mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix(coderd): declare project_id variable in GCP template builder bases (#27015)
## Summary
GCP base templates (`gcp-linux`, `gcp-windows`) in the Template Builder
had a Terraform `variable "project_id"` with no default, but their
`base.json` manifests didn't declare it. The UI never prompted for it,
so the provisioner import always failed with:
```
required template variables need values: project_id
```
## Changes
- Add `project_id` as a required variable in both GCP `base.json`
manifests
- Convert templates from raw Terraform variable blocks to Go template
injection (`{{ .Variables.project_id }}`), matching the existing
kubernetes pattern
- Fix `DefaultBaseRenderContext` to supply a `"REQUIRED"` placeholder
for required variables without defaults (previously rendered as `<no
value>`)
- Replace duplicate test subtests with proper GCP coverage including a
missing-variable error case
<details>
<summary>Implementation plan</summary>
### Root cause
The GCP base templates contained `variable "project_id" {}` (no default
= required) in their `.tf.tmpl` files, but the `base.json` manifests had
an empty `variables` array. The Template Builder UI
(`BaseTemplateParametersStep`) is data-driven from `base.json`, so it
never showed a field for `project_id`. The composed Terraform output
still contained the required variable, causing the provisioner import to
fail.
### Fix approach
Follow the pattern established by the kubernetes base template:
1. Declare variables in `base.json` so the UI prompts for them
2. Use Go template syntax (`{{ .Variables.project_id }}`) to inject
values at compose time
3. Remove raw Terraform `variable` blocks from the template since the
value is now baked in
### Files changed
| File | Change |
|---|---|
| `bases/gcp-linux/base.json` | Added `project_id` as a required
variable |
| `bases/gcp-windows/base.json` | Added `project_id` as a required
variable |
| `bases/gcp-linux/main.tf.tmpl` | Removed Terraform variable block, use
Go template injection |
| `bases/gcp-windows/main.tf.tmpl` | Same |
| `bases.go` | `DefaultBaseRenderContext` supplies placeholder for
required vars without defaults |
| `compose_test.go` | Replaced duplicate subtests with proper GCP tests
|
| `templatebuilder_handler_test.go` | Updated `gcp-windows` spec to
expect `project_id` variable |
| Golden files | Regenerated |
</details>
> 🤖 Generated by Coder Agents on behalf of @jeremyruppel
This commit is contained in:
@@ -2,5 +2,15 @@
|
||||
"id": "gcp-linux",
|
||||
"display_name": "Google Compute Engine (Linux)",
|
||||
"os": "linux",
|
||||
"default_context": {}
|
||||
"default_context": {},
|
||||
"variables": [
|
||||
{
|
||||
"name": "project_id",
|
||||
"type": "string",
|
||||
"description": "Which Google Compute Project should your workspace live in?",
|
||||
"required": true,
|
||||
"sensitive": false,
|
||||
"computed": false
|
||||
}
|
||||
]
|
||||
}
|
||||
|
||||
@@ -13,6 +13,7 @@ provider "coder" {}
|
||||
|
||||
variable "project_id" {
|
||||
description = "Which Google Compute Project should your workspace live in?"
|
||||
default = {{ .Variables.project_id }}
|
||||
}
|
||||
|
||||
# See https://registry.coder.com/modules/coder/gcp-region
|
||||
|
||||
@@ -2,5 +2,15 @@
|
||||
"id": "gcp-windows",
|
||||
"display_name": "Google Compute Engine (Windows)",
|
||||
"os": "windows",
|
||||
"default_context": {}
|
||||
"default_context": {},
|
||||
"variables": [
|
||||
{
|
||||
"name": "project_id",
|
||||
"type": "string",
|
||||
"description": "Which Google Compute Project should your workspace live in?",
|
||||
"required": true,
|
||||
"sensitive": false,
|
||||
"computed": false
|
||||
}
|
||||
]
|
||||
}
|
||||
|
||||
@@ -13,6 +13,7 @@ provider "coder" {}
|
||||
|
||||
variable "project_id" {
|
||||
description = "Which Google Compute Project should your workspace live in?"
|
||||
default = {{ .Variables.project_id }}
|
||||
}
|
||||
|
||||
# See https://registry.coder.com/modules/coder/gcp-region
|
||||
|
||||
@@ -78,36 +78,44 @@ func TestCompose(t *testing.T) {
|
||||
require.Contains(t, result.ExtraFiles, "cloud-init/userdata.sh.tftpl")
|
||||
})
|
||||
|
||||
t.Run("GCPWindowsBase", func(t *testing.T) {
|
||||
t.Run("GCPLinuxBaseWithProjectID", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
result, err := templatebuilder.Compose(templatebuilder.ComposeRequest{
|
||||
BaseTemplateID: "gcp-windows",
|
||||
RegistryURL: "https://registry.coder.com",
|
||||
BaseTemplateID: "gcp-linux",
|
||||
BaseVariableValues: map[string]string{"project_id": "my-gcp-project"},
|
||||
RegistryURL: "https://registry.coder.com",
|
||||
})
|
||||
require.NoError(t, err)
|
||||
require.NotEmpty(t, result.MainTF)
|
||||
require.Contains(t, string(result.MainTF), `resource "coder_agent" "main"`)
|
||||
})
|
||||
|
||||
t.Run("AzureLinuxExtraFiles", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
result, err := templatebuilder.Compose(templatebuilder.ComposeRequest{
|
||||
BaseTemplateID: "azure-linux",
|
||||
RegistryURL: "https://registry.coder.com",
|
||||
})
|
||||
require.NoError(t, err)
|
||||
require.Contains(t, result.ExtraFiles, "cloud-init/cloud-config.yaml.tftpl")
|
||||
mainTF := string(result.MainTF)
|
||||
require.Contains(t, mainTF, `resource "coder_agent" "main"`)
|
||||
require.Contains(t, mainTF, `default = "my-gcp-project"`)
|
||||
require.Contains(t, mainTF, `project = var.project_id`)
|
||||
})
|
||||
|
||||
t.Run("GCPWindowsBase", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
result, err := templatebuilder.Compose(templatebuilder.ComposeRequest{
|
||||
BaseTemplateID: "gcp-windows",
|
||||
RegistryURL: "https://registry.coder.com",
|
||||
BaseTemplateID: "gcp-windows",
|
||||
BaseVariableValues: map[string]string{"project_id": "my-gcp-project"},
|
||||
RegistryURL: "https://registry.coder.com",
|
||||
})
|
||||
require.NoError(t, err)
|
||||
require.NotEmpty(t, result.MainTF)
|
||||
require.Contains(t, string(result.MainTF), `resource "coder_agent" "main"`)
|
||||
mainTF := string(result.MainTF)
|
||||
require.Contains(t, mainTF, `resource "coder_agent" "main"`)
|
||||
require.Contains(t, mainTF, `default = "my-gcp-project"`)
|
||||
require.Contains(t, mainTF, `project = var.project_id`)
|
||||
})
|
||||
|
||||
t.Run("GCPMissingRequiredBaseVariable", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
_, err := templatebuilder.Compose(templatebuilder.ComposeRequest{
|
||||
BaseTemplateID: "gcp-linux",
|
||||
RegistryURL: "https://registry.coder.com",
|
||||
})
|
||||
require.Error(t, err)
|
||||
require.Contains(t, err.Error(), `variable "project_id" is required`)
|
||||
})
|
||||
|
||||
t.Run("AzureLinuxExtraFiles", func(t *testing.T) {
|
||||
|
||||
@@ -2,6 +2,7 @@ package templatebuilder_test
|
||||
|
||||
import (
|
||||
"flag"
|
||||
"fmt"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
@@ -14,6 +15,26 @@ import (
|
||||
|
||||
var updateGolden = flag.Bool("update", false, "update golden files")
|
||||
|
||||
// testRenderContext builds a BaseRenderContext from defaults and fills
|
||||
// in deterministic test values for any required variables that have no
|
||||
// default. This keeps the all-bases test loops working without silently
|
||||
// producing broken HCL.
|
||||
func testRenderContext(exampleID string) templatebuilder.BaseRenderContext {
|
||||
rc := templatebuilder.DefaultBaseRenderContext(exampleID)
|
||||
if rc.Variables == nil {
|
||||
rc.Variables = make(map[string]string)
|
||||
}
|
||||
for _, v := range templatebuilder.BaseVariables(exampleID) {
|
||||
if v.Computed || v.Sensitive {
|
||||
continue
|
||||
}
|
||||
if _, ok := rc.Variables[v.Name]; !ok {
|
||||
rc.Variables[v.Name] = fmt.Sprintf("%q", "test-"+v.Name)
|
||||
}
|
||||
}
|
||||
return rc
|
||||
}
|
||||
|
||||
func TestRenderBaseTemplate(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
@@ -297,7 +318,7 @@ func TestAllBasesRenderAndExtractAgent(t *testing.T) {
|
||||
for _, id := range templatebuilder.BaseTemplateIDs() {
|
||||
t.Run(id, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
renderCtx := templatebuilder.DefaultBaseRenderContext(id)
|
||||
renderCtx := testRenderContext(id)
|
||||
rendered, err := templatebuilder.RenderBaseTemplate(id, "main.tf.tmpl", renderCtx)
|
||||
require.NoError(t, err, "base %q should render without error", id)
|
||||
require.NotEmpty(t, rendered)
|
||||
@@ -330,7 +351,7 @@ func TestBaseTemplateSnapshot(t *testing.T) {
|
||||
t.Run(tc.exampleID, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
renderCtx := templatebuilder.DefaultBaseRenderContext(tc.exampleID)
|
||||
renderCtx := testRenderContext(tc.exampleID)
|
||||
rendered, err := templatebuilder.RenderBaseTemplate(tc.exampleID, "main.tf.tmpl", renderCtx)
|
||||
require.NoError(t, err)
|
||||
require.NotEmpty(t, rendered)
|
||||
|
||||
@@ -13,6 +13,7 @@ provider "coder" {}
|
||||
|
||||
variable "project_id" {
|
||||
description = "Which Google Compute Project should your workspace live in?"
|
||||
default = "test-project_id"
|
||||
}
|
||||
|
||||
# See https://registry.coder.com/modules/coder/gcp-region
|
||||
|
||||
@@ -13,6 +13,7 @@ provider "coder" {}
|
||||
|
||||
variable "project_id" {
|
||||
description = "Which Google Compute Project should your workspace live in?"
|
||||
default = "test-project_id"
|
||||
}
|
||||
|
||||
# See https://registry.coder.com/modules/coder/gcp-region
|
||||
|
||||
+2
-2
@@ -153,7 +153,7 @@ resource "coder_agent" "main" {
|
||||
resource "kubernetes_persistent_volume_claim_v1" "home" {
|
||||
metadata {
|
||||
name = "coder-${data.coder_workspace.me.id}-home"
|
||||
namespace = <no value>
|
||||
namespace = "test-namespace"
|
||||
labels = {
|
||||
"app.kubernetes.io/name" = "coder-pvc"
|
||||
"app.kubernetes.io/instance" = "coder-pvc-${data.coder_workspace.me.id}"
|
||||
@@ -188,7 +188,7 @@ resource "kubernetes_deployment_v1" "main" {
|
||||
wait_for_rollout = false
|
||||
metadata {
|
||||
name = "coder-${data.coder_workspace.me.id}"
|
||||
namespace = <no value>
|
||||
namespace = "test-namespace"
|
||||
labels = {
|
||||
"app.kubernetes.io/name" = "coder-workspace"
|
||||
"app.kubernetes.io/instance" = "coder-workspace-${data.coder_workspace.me.id}"
|
||||
|
||||
@@ -67,7 +67,8 @@ func TestTemplateBuilderBases(t *testing.T) {
|
||||
{
|
||||
id: "gcp-windows",
|
||||
expectedOS: "windows",
|
||||
hasVariables: false,
|
||||
hasVariables: true,
|
||||
expectedVars: []string{"project_id"},
|
||||
},
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user