mirror of
https://github.com/coder/coder.git
synced 2026-09-22 13:10:21 +08:00
## 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
241 lines
6.3 KiB
Go
241 lines
6.3 KiB
Go
package coderd_test
|
|
|
|
import (
|
|
"context"
|
|
"net/http"
|
|
"testing"
|
|
|
|
"github.com/stretchr/testify/require"
|
|
|
|
"github.com/coder/coder/v2/coderd/coderdtest"
|
|
"github.com/coder/coder/v2/coderd/templatebuilder"
|
|
"github.com/coder/coder/v2/codersdk"
|
|
"github.com/coder/coder/v2/testutil"
|
|
)
|
|
|
|
func TestTemplateBuilderBases(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
t.Run("OK", func(t *testing.T) {
|
|
t.Parallel()
|
|
client := coderdtest.New(t, nil)
|
|
_ = coderdtest.CreateFirstUser(t, client)
|
|
|
|
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
|
|
defer cancel()
|
|
|
|
resp, err := client.TemplateBuilderBases(ctx)
|
|
require.NoError(t, err)
|
|
require.NotEmpty(t, resp.Bases)
|
|
require.Len(t, resp.Bases, len(templatebuilder.BaseTemplateIDs()))
|
|
|
|
basesByID := make(map[string]codersdk.TemplateBuilderBase, len(resp.Bases))
|
|
for _, b := range resp.Bases {
|
|
basesByID[b.ID] = b
|
|
}
|
|
|
|
type baseSpec struct {
|
|
id string
|
|
expectedOS string
|
|
expectedVars []string
|
|
hasVariables bool
|
|
}
|
|
|
|
specs := []baseSpec{
|
|
{
|
|
id: "docker",
|
|
expectedOS: "linux",
|
|
hasVariables: true,
|
|
expectedVars: []string{"container_image"},
|
|
},
|
|
{
|
|
id: "kubernetes",
|
|
expectedOS: "linux",
|
|
hasVariables: true,
|
|
expectedVars: []string{"container_image", "namespace", "use_kubeconfig"},
|
|
},
|
|
{
|
|
id: "aws-linux",
|
|
expectedOS: "linux",
|
|
hasVariables: false,
|
|
},
|
|
{
|
|
id: "aws-windows",
|
|
expectedOS: "windows",
|
|
hasVariables: false,
|
|
},
|
|
{
|
|
id: "gcp-windows",
|
|
expectedOS: "windows",
|
|
hasVariables: true,
|
|
expectedVars: []string{"project_id"},
|
|
},
|
|
}
|
|
|
|
for _, spec := range specs {
|
|
b, ok := basesByID[spec.id]
|
|
require.True(t, ok, "base %q missing from response", spec.id)
|
|
require.NotEmpty(t, b.Name, "base %q should have a name", spec.id)
|
|
require.NotEmpty(t, b.Icon, "base %q should have an icon", spec.id)
|
|
require.Equal(t, spec.expectedOS, b.OS, "base %q OS mismatch", spec.id)
|
|
require.NotNil(t, b.Variables, "base %q should have non-nil variables slice", spec.id)
|
|
|
|
if spec.hasVariables {
|
|
require.NotEmpty(t, b.Variables, "base %q should have variables", spec.id)
|
|
varNames := make(map[string]bool, len(b.Variables))
|
|
for _, v := range b.Variables {
|
|
varNames[v.Name] = true
|
|
}
|
|
for _, expected := range spec.expectedVars {
|
|
require.True(t, varNames[expected],
|
|
"base %q should have variable %q", spec.id, expected)
|
|
}
|
|
} else {
|
|
require.Empty(t, b.Variables, "base %q should have no variables", spec.id)
|
|
}
|
|
}
|
|
})
|
|
|
|
t.Run("Sorted", func(t *testing.T) {
|
|
t.Parallel()
|
|
client := coderdtest.New(t, nil)
|
|
_ = coderdtest.CreateFirstUser(t, client)
|
|
|
|
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
|
|
defer cancel()
|
|
|
|
resp, err := client.TemplateBuilderBases(ctx)
|
|
require.NoError(t, err)
|
|
|
|
for i := 1; i < len(resp.Bases); i++ {
|
|
require.LessOrEqual(t, resp.Bases[i-1].Name, resp.Bases[i].Name,
|
|
"bases should be sorted by name")
|
|
}
|
|
})
|
|
|
|
t.Run("DisabledReturns404", func(t *testing.T) {
|
|
t.Parallel()
|
|
dv := coderdtest.DeploymentValues(t)
|
|
dv.TemplateBuilder.Disabled = true
|
|
|
|
client := coderdtest.New(t, &coderdtest.Options{
|
|
DeploymentValues: dv,
|
|
})
|
|
_ = coderdtest.CreateFirstUser(t, client)
|
|
|
|
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
|
|
defer cancel()
|
|
|
|
_, err := client.TemplateBuilderBases(ctx)
|
|
require.Error(t, err)
|
|
|
|
var sdkErr *codersdk.Error
|
|
require.ErrorAs(t, err, &sdkErr)
|
|
require.Equal(t, http.StatusNotFound, sdkErr.StatusCode())
|
|
})
|
|
}
|
|
|
|
func TestTemplateBuilderModules(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
t.Run("OK", func(t *testing.T) {
|
|
t.Parallel()
|
|
client := coderdtest.New(t, nil)
|
|
_ = coderdtest.CreateFirstUser(t, client)
|
|
|
|
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
|
|
defer cancel()
|
|
|
|
resp, err := client.TemplateBuilderModules(ctx, "")
|
|
require.NoError(t, err)
|
|
require.NotEmpty(t, resp.Modules)
|
|
|
|
for _, m := range resp.Modules {
|
|
require.NotEmpty(t, m.ID)
|
|
require.NotEmpty(t, m.Version)
|
|
}
|
|
})
|
|
|
|
t.Run("FilteredByBase", func(t *testing.T) {
|
|
t.Parallel()
|
|
client := coderdtest.New(t, nil)
|
|
_ = coderdtest.CreateFirstUser(t, client)
|
|
|
|
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
|
|
defer cancel()
|
|
|
|
resp, err := client.TemplateBuilderModules(ctx, "docker")
|
|
require.NoError(t, err)
|
|
|
|
for _, m := range resp.Modules {
|
|
if len(m.CompatibleOS) > 0 {
|
|
require.Contains(t, m.CompatibleOS, "linux",
|
|
"module %q should be compatible with linux when filtered by docker base", m.ID)
|
|
}
|
|
}
|
|
})
|
|
|
|
t.Run("ComputedVariablesExcluded", func(t *testing.T) {
|
|
t.Parallel()
|
|
client := coderdtest.New(t, nil)
|
|
_ = coderdtest.CreateFirstUser(t, client)
|
|
|
|
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
|
|
defer cancel()
|
|
|
|
resp, err := client.TemplateBuilderModules(ctx, "")
|
|
require.NoError(t, err)
|
|
|
|
// The embedded code-server module has agent_id with computed=true.
|
|
// It must not appear in the API response.
|
|
var found bool
|
|
for _, m := range resp.Modules {
|
|
if m.ID == "code-server" {
|
|
found = true
|
|
for _, v := range m.Variables {
|
|
require.NotEqual(t, "agent_id", v.Name,
|
|
"computed variable agent_id must not appear in API response")
|
|
}
|
|
}
|
|
}
|
|
require.True(t, found, "code-server module must be in the catalog")
|
|
})
|
|
|
|
t.Run("UnknownBaseReturns400", func(t *testing.T) {
|
|
t.Parallel()
|
|
client := coderdtest.New(t, nil)
|
|
_ = coderdtest.CreateFirstUser(t, client)
|
|
|
|
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
|
|
defer cancel()
|
|
|
|
_, err := client.TemplateBuilderModules(ctx, "nonexistent")
|
|
require.Error(t, err)
|
|
|
|
var sdkErr *codersdk.Error
|
|
require.ErrorAs(t, err, &sdkErr)
|
|
require.Equal(t, http.StatusBadRequest, sdkErr.StatusCode())
|
|
})
|
|
|
|
t.Run("DisabledReturns404", func(t *testing.T) {
|
|
t.Parallel()
|
|
dv := coderdtest.DeploymentValues(t)
|
|
dv.TemplateBuilder.Disabled = true
|
|
|
|
client := coderdtest.New(t, &coderdtest.Options{
|
|
DeploymentValues: dv,
|
|
})
|
|
_ = coderdtest.CreateFirstUser(t, client)
|
|
|
|
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
|
|
defer cancel()
|
|
|
|
_, err := client.TemplateBuilderModules(ctx, "")
|
|
require.Error(t, err)
|
|
|
|
var sdkErr *codersdk.Error
|
|
require.ErrorAs(t, err, &sdkErr)
|
|
require.Equal(t, http.StatusNotFound, sdkErr.StatusCode())
|
|
})
|
|
}
|