diff --git a/coderd/templatebuilder/bases.go b/coderd/templatebuilder/bases.go index 093b632a5d..5b5d79a60f 100644 --- a/coderd/templatebuilder/bases.go +++ b/coderd/templatebuilder/bases.go @@ -174,9 +174,24 @@ func DefaultBaseRenderContext(exampleID string) BaseRenderContext { if err != nil || bases[exampleID] == nil { return BaseRenderContext{} } - dc := bases[exampleID].Manifest.DefaultContext + base := bases[exampleID] + dc := base.Manifest.DefaultContext + + // Populate Variables from manifest defaults so that Go template + // rendering succeeds even without caller-supplied values. + vars := make(map[string]string, len(base.Manifest.Variables)) + for _, v := range base.Manifest.Variables { + if v.Computed || v.Sensitive { + continue + } + if len(v.Default) > 0 && isSimpleJSONValue(v.Default) { + vars[v.Name] = string(v.Default) + } + } + return BaseRenderContext{ ContainerImage: dc.ContainerImage, + Variables: vars, } } diff --git a/coderd/templatebuilder/bases/kubernetes/main.tf.tmpl b/coderd/templatebuilder/bases/kubernetes/main.tf.tmpl index ab63956317..b50495ffb3 100644 --- a/coderd/templatebuilder/bases/kubernetes/main.tf.tmpl +++ b/coderd/templatebuilder/bases/kubernetes/main.tf.tmpl @@ -12,23 +12,9 @@ terraform { provider "coder" { } -variable "use_kubeconfig" { - type = bool - description = <<-EOF - Use host kubeconfig? (true/false) - - Set this to false if the Coder host is itself running as a Pod on the same - Kubernetes cluster as you are deploying workspaces to. - - Set this to true if the Coder host is running outside the Kubernetes cluster - for workspaces. A valid "~/.kube/config" must be present on the Coder host. - EOF - default = false -} - -variable "namespace" { - type = string - description = "The Kubernetes namespace to create workspaces in (must exist prior to creating workspaces). If the Coder host is itself running as a Pod on the same Kubernetes cluster as you are deploying workspaces to, set this to the same namespace." +provider "kubernetes" { + # Authenticate via ~/.kube/config or a Coder-specific ServiceAccount, depending on admin preferences + config_path = {{ .Variables.use_kubeconfig }} == true ? "~/.kube/config" : null } data "coder_parameter" "cpu" { @@ -95,11 +81,6 @@ data "coder_parameter" "home_disk_size" { } } -provider "kubernetes" { - # Authenticate via ~/.kube/config or a Coder-specific ServiceAccount, depending on admin preferences - config_path = var.use_kubeconfig == true ? "~/.kube/config" : null -} - data "coder_workspace" "me" {} data "coder_workspace_owner" "me" {} {{ if .ImageOptions }} @@ -185,7 +166,7 @@ resource "coder_agent" "main" { resource "kubernetes_persistent_volume_claim_v1" "home" { metadata { name = "coder-${data.coder_workspace.me.id}-home" - namespace = var.namespace + namespace = {{ .Variables.namespace }} labels = { "app.kubernetes.io/name" = "coder-pvc" "app.kubernetes.io/instance" = "coder-pvc-${data.coder_workspace.me.id}" @@ -220,7 +201,7 @@ resource "kubernetes_deployment_v1" "main" { wait_for_rollout = false metadata { name = "coder-${data.coder_workspace.me.id}" - namespace = var.namespace + namespace = {{ .Variables.namespace }} labels = { "app.kubernetes.io/name" = "coder-workspace" "app.kubernetes.io/instance" = "coder-workspace-${data.coder_workspace.me.id}" diff --git a/coderd/templatebuilder/compose.go b/coderd/templatebuilder/compose.go index 2b1699fe39..9a400bd00e 100644 --- a/coderd/templatebuilder/compose.go +++ b/coderd/templatebuilder/compose.go @@ -104,7 +104,13 @@ func renderBase(baseTemplateID string, baseVars map[string]string) ([]byte, erro if renderCtx.Variables == nil { renderCtx.Variables = make(map[string]string) } - maps.Copy(renderCtx.Variables, baseVars) + + vars, err := mergeBaseVariables(baseTemplateID, baseVars) + if err != nil { + return nil, xerrors.Errorf("base %q: %w", baseTemplateID, err) + } + maps.Copy(renderCtx.Variables, vars) + mainTF, err := RenderBaseTemplate(baseTemplateID, "main.tf.tmpl", renderCtx) if err != nil { return nil, xerrors.Errorf("render base template: %w", err) @@ -112,6 +118,63 @@ func renderBase(baseTemplateID string, baseVars map[string]string) ([]byte, erro return mainTF, nil } +// mergeBaseVariables builds the final Variables map for a base template. +// It starts with manifest defaults, overlays caller-supplied values, +// validates types, and converts to HCL literals. +func mergeBaseVariables(baseTemplateID string, callerVars map[string]string) (map[string]string, error) { + allVars := BaseVariables(baseTemplateID) + if len(allVars) == 0 && len(callerVars) == 0 { + return make(map[string]string), nil + } + + allowedVars := make(map[string]ModuleVariable, len(allVars)) + for _, v := range allVars { + if v.Computed || v.Sensitive { + continue + } + allowedVars[v.Name] = v + } + + // Validate caller-supplied keys and values. + for k, val := range callerVars { + v, ok := allowedVars[k] + if !ok { + return nil, xerrors.Errorf("unknown variable %q", k) + } + if err := validateVariableValue(v, val); err != nil { + return nil, xerrors.Errorf("variable %q: %w", k, err) + } + } + + // Build merged map from manifest defaults. + merged := make(map[string]string, len(allVars)) + for _, v := range allVars { + if v.Computed || v.Sensitive { + continue + } + if len(v.Default) > 0 && isSimpleJSONValue(v.Default) { + merged[v.Name] = string(v.Default) + } + } + + // Overlay validated caller values, converting to HCL literals. + for k, val := range callerVars { + merged[k] = toHCLLiteral(allowedVars[k], val) + } + + // Ensure all required variables without defaults have a value. + for _, v := range allVars { + if v.Computed || v.Sensitive { + continue + } + if v.Required && merged[v.Name] == "" { + return nil, xerrors.Errorf("variable %q is required", v.Name) + } + } + + return merged, nil +} + // loadCatalogMap loads the module catalog and returns it as a map keyed // by module ID. func loadCatalogMap() (map[string]ModuleManifest, error) { @@ -248,6 +311,17 @@ func mergeModuleVariables(manifest ModuleManifest, callerVars map[string]string) for k, val := range callerVars { merged[k] = toHCLLiteral(allowedVars[k], val) } + + // Ensure all required variables without defaults have a value. + for _, v := range manifest.Variables { + if v.Computed || v.Sensitive { + continue + } + if v.Required && merged[v.Name] == "" { + return nil, xerrors.Errorf("variable %q is required", v.Name) + } + } + return merged, nil } diff --git a/coderd/templatebuilder/compose_internal_test.go b/coderd/templatebuilder/compose_internal_test.go index 4a919aefdf..3849c32f11 100644 --- a/coderd/templatebuilder/compose_internal_test.go +++ b/coderd/templatebuilder/compose_internal_test.go @@ -55,9 +55,13 @@ func TestMergeModuleVariables(t *testing.T) { }, } + requiredVars := map[string]string{ + "required_no_default": "value", + } + t.Run("DefaultsApplied", func(t *testing.T) { t.Parallel() - merged, err := mergeModuleVariables(manifest, nil) + merged, err := mergeModuleVariables(manifest, requiredVars) require.NoError(t, err) require.Equal(t, "13337", merged["port"]) require.Equal(t, "false", merged["enabled"]) @@ -65,7 +69,7 @@ func TestMergeModuleVariables(t *testing.T) { t.Run("ComputedAndSensitiveSkipped", func(t *testing.T) { t.Parallel() - merged, err := mergeModuleVariables(manifest, nil) + merged, err := mergeModuleVariables(manifest, requiredVars) require.NoError(t, err) require.NotContains(t, merged, "agent_id") require.NotContains(t, merged, "api_key") @@ -73,22 +77,24 @@ func TestMergeModuleVariables(t *testing.T) { t.Run("NonRequiredWithoutDefaultGetsNull", func(t *testing.T) { t.Parallel() - merged, err := mergeModuleVariables(manifest, nil) + merged, err := mergeModuleVariables(manifest, requiredVars) require.NoError(t, err) require.Equal(t, "null", merged["optional_no_default"]) }) - t.Run("RequiredWithoutDefaultOmitted", func(t *testing.T) { + t.Run("RequiredWithoutDefaultIsRequired", func(t *testing.T) { t.Parallel() - merged, err := mergeModuleVariables(manifest, nil) - require.NoError(t, err) - require.NotContains(t, merged, "required_no_default") + _, err := mergeModuleVariables(manifest, nil) + require.Error(t, err) + require.Contains(t, err.Error(), `variable "required_no_default"`) + require.Contains(t, err.Error(), "is required") }) t.Run("CallerOverridesDefault", func(t *testing.T) { t.Parallel() merged, err := mergeModuleVariables(manifest, map[string]string{ - "port": "9999", + "port": "9999", + "required_no_default": "value", }) require.NoError(t, err) require.Equal(t, "9999", merged["port"]) @@ -166,6 +172,7 @@ func TestMergeModuleVariables(t *testing.T) { "port": "null", "enabled": "null", "optional_no_default": "null", + "required_no_default": "null", }) require.NoError(t, err) require.Equal(t, "null", merged["port"]) @@ -173,9 +180,11 @@ func TestMergeModuleVariables(t *testing.T) { require.Equal(t, "null", merged["optional_no_default"]) }) - t.Run("EmptyCallerVarsNoError", func(t *testing.T) { + t.Run("EmptyCallerVarsUsesDefaults", func(t *testing.T) { t.Parallel() - merged, err := mergeModuleVariables(manifest, map[string]string{}) + merged, err := mergeModuleVariables(manifest, map[string]string{ + "required_no_default": "value", + }) require.NoError(t, err) require.Equal(t, "13337", merged["port"]) }) diff --git a/coderd/templatebuilder/compose_test.go b/coderd/templatebuilder/compose_test.go index c718f94b6d..9eae66e326 100644 --- a/coderd/templatebuilder/compose_test.go +++ b/coderd/templatebuilder/compose_test.go @@ -237,7 +237,8 @@ func TestCompose(t *testing.T) { }, }) require.Error(t, err) - require.Contains(t, err.Error(), "render module") + require.Contains(t, err.Error(), `variable "url"`) + require.Contains(t, err.Error(), "is required") }) } diff --git a/coderd/templatebuilder/render_test.go b/coderd/templatebuilder/render_test.go index 0c616a7148..2534aa8ce9 100644 --- a/coderd/templatebuilder/render_test.go +++ b/coderd/templatebuilder/render_test.go @@ -58,6 +58,10 @@ func TestRenderBaseTemplate(t *testing.T) { renderCtx := templatebuilder.BaseRenderContext{ ContainerImage: "custom/image:latest", ImageOptions: imageOpts, + Variables: map[string]string{ + "namespace": `"test-ns"`, + "use_kubeconfig": "false", + }, } out, err := templatebuilder.RenderBaseTemplate("kubernetes", "main.tf.tmpl", renderCtx) require.NoError(t, err) diff --git a/coderd/templatebuilder/testdata/kubernetes.tf.golden b/coderd/templatebuilder/testdata/kubernetes.tf.golden index c9d18a34c8..876f3c1dfc 100644 --- a/coderd/templatebuilder/testdata/kubernetes.tf.golden +++ b/coderd/templatebuilder/testdata/kubernetes.tf.golden @@ -12,23 +12,9 @@ terraform { provider "coder" { } -variable "use_kubeconfig" { - type = bool - description = <<-EOF - Use host kubeconfig? (true/false) - - Set this to false if the Coder host is itself running as a Pod on the same - Kubernetes cluster as you are deploying workspaces to. - - Set this to true if the Coder host is running outside the Kubernetes cluster - for workspaces. A valid "~/.kube/config" must be present on the Coder host. - EOF - default = false -} - -variable "namespace" { - type = string - description = "The Kubernetes namespace to create workspaces in (must exist prior to creating workspaces). If the Coder host is itself running as a Pod on the same Kubernetes cluster as you are deploying workspaces to, set this to the same namespace." +provider "kubernetes" { + # Authenticate via ~/.kube/config or a Coder-specific ServiceAccount, depending on admin preferences + config_path = false == true ? "~/.kube/config" : null } data "coder_parameter" "cpu" { @@ -95,11 +81,6 @@ data "coder_parameter" "home_disk_size" { } } -provider "kubernetes" { - # Authenticate via ~/.kube/config or a Coder-specific ServiceAccount, depending on admin preferences - config_path = var.use_kubeconfig == true ? "~/.kube/config" : null -} - data "coder_workspace" "me" {} data "coder_workspace_owner" "me" {} @@ -172,7 +153,7 @@ resource "coder_agent" "main" { resource "kubernetes_persistent_volume_claim_v1" "home" { metadata { name = "coder-${data.coder_workspace.me.id}-home" - namespace = var.namespace + namespace = labels = { "app.kubernetes.io/name" = "coder-pvc" "app.kubernetes.io/instance" = "coder-pvc-${data.coder_workspace.me.id}" @@ -207,7 +188,7 @@ resource "kubernetes_deployment_v1" "main" { wait_for_rollout = false metadata { name = "coder-${data.coder_workspace.me.id}" - namespace = var.namespace + namespace = labels = { "app.kubernetes.io/name" = "coder-workspace" "app.kubernetes.io/instance" = "coder-workspace-${data.coder_workspace.me.id}"