8000 feat: allow new immutable parameters for existing workspaces (#18579) · coder/coder@e396b06 · GitHub
[go: up one dir, main page]
Skip to content

Commit e396b06

Browse files
EmyrkEmyrk
authored
feat: allow new immutable parameters for existing workspaces (#18579)
Closes #18578
1 parent 072c81c commit e396b06

File tree

8 files changed

+237
-5
lines changed

8 files changed

+237
-5
lines changed

‎coderd/coderdtest/dynamicparameters.go

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@ type DynamicParameterTemplateParams struct {
2222

2323
// StaticParams is used if the provisioner daemon version does not support dynamic parameters.
2424
StaticParams []*proto.RichParameter
25+
26+
// TemplateID is used to update an existing template instead of creating a new one.
27+
TemplateID uuid.UUID
2528
}
2629

2730
func DynamicParameterTemplate(t *testing.T, client *codersdk.Client, org uuid.UUID, args DynamicParameterTemplateParams) (codersdk.Template, codersdk.TemplateVersion) {
@@ -40,16 +43,30 @@ func DynamicParameterTemplate(t *testing.T, client *codersdk.Client, org uuid.UU
4043
},
4144
}}
4245

43-
version := CreateTemplateVersion(t, client, org, files)
46+
version := CreateTemplateVersion(t, client, org, files, func(request *codersdk.CreateTemplateVersionRequest) {
47+
if args.TemplateID != uuid.Nil {
48+
request.TemplateID = args.TemplateID
49+
}
50+
})
4451
AwaitTemplateVersionJobCompleted(t, client, version.ID)
45-
tpl := CreateTemplate(t, client, org, version.ID)
52+
53+
tplID := args.TemplateID
54+
if args.TemplateID == uuid.Nil {
55+
tpl := CreateTemplate(t, client, org, version.ID)
56+
tplID = tpl.ID
57+
}
4658

4759
var err error
48-
tpl, err = client.UpdateTemplateMeta(t.Context(), tpl.ID, codersdk.UpdateTemplateMeta{
60+
tpl, err := client.UpdateTemplateMeta(t.Context(), tplID, codersdk.UpdateTemplateMeta{
4961
UseClassicParameterFlow: ptr.Ref(false),
5062
})
5163
require.NoError(t, err)
5264

65+
err = client.UpdateActiveTemplateVersion(t.Context(), tpl.ID, codersdk.UpdateActiveTemplateVersion{
66+
ID: version.ID,
67+
})
68+
require.NoError(t, err)
69+
5370
return tpl, version
5471
}
5572

‎coderd/dynamicparameters/rendermock/mock.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
//go:generate mockgen -destination ./rendermock.go -package rendermock github.com/coder/coder/v2/coderd/dynamicparameters Renderer
2+
package rendermock

‎coderd/dynamicparameters/rendermock/rendermock.go

Lines changed: 71 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/dynamicparameters/resolver.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -169,9 +169,15 @@ func ResolveParameters(
169169
parameterNames[parameter.Name] = struct{}{}
170170

171171
if !firstBuild && !parameter.Mutable {
172+
originalValue, ok := originalValues[parameter.Name]
172173
// Immutable parameters should not be changed after the first build.
173-
// They can match the original value though!
174-
if parameter.Value.AsString() != originalValues[parameter.Name].Value {
174+
// If the value matches the original value, that is fine.
175+
//
176+
// If the original value is not set, that means this is a new parameter. New
177+
// immutable parameters are allowed. This is an opinionated choice to prevent
178+
// workspaces failing to update or delete. Ideally we would block this, as
179+
// immutable parameters should only be able to be set at creation time.
180+
if ok && parameter.Value.AsString() != originalValue.Value {
175181
var src *hcl.Range
176182
if parameter.Source != nil {
177183
src = &parameter.Source.HCLBlock().TypeRange

‎coderd/dynamicparameters/resolver_test.go

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
package dynamicparameters_test
2+
3+
import (
4+
"testing"
5+
6+
"github.com/google/uuid"
7+
"github.com/stretchr/testify/require"
8+
"go.uber.org/mock/gomock"
9+
10+
"github.com/coder/coder/v2/coderd/database"
11+
"github.com/coder/coder/v2/coderd/dynamicparameters"
12+
"github.com/coder/coder/v2/coderd/dynamicparameters/rendermock"
13+
"github.com/coder/coder/v2/codersdk"
14+
"github.com/coder/coder/v2/testutil"
15+
"github.com/coder/preview"
16+
previewtypes "github.com/coder/preview/types"
17+
"github.com/coder/terraform-provider-coder/v2/provider"
18+
)
19+
20+
func TestResolveParameters(t *testing.T) {
21+
t.Parallel()
22+
23+
t.Run("NewImmutable", func(t *testing.T) {
24+
t.Parallel()
25+
26+
ctrl := gomock.NewController(t)
27+
render := rendermock.NewMockRenderer(ctrl)
28+
29+
// A single immutable parameter with no previous value.
30+
render.EXPECT().
31+
Render(gomock.Any(), gomock.Any(), gomock.Any()).
32+
AnyTimes().
33+
Return(&preview.Output{
34+
Parameters: []previewtypes.Parameter{
35+
{
36+
ParameterData: previewtypes.ParameterData{
37+
Name: "immutable",
38+
Type: previewtypes.ParameterTypeString,
39+
FormType: provider.ParameterFormTypeInput,
40+
Mutable: false,
41+
DefaultValue: previewtypes.StringLiteral("foo"),
42+
Required: true,
43+
},
44+
Value: previewtypes.StringLiteral("foo"),
45+
Diagnostics: nil,
46+
},
47+
},
48+
}, nil)
49+
50+
ctx := testutil.Context(t, testutil.WaitShort)
51+
values, err := dynamicparameters.ResolveParameters(ctx, uuid.New(), render, false,
52+
[]database.WorkspaceBuildParameter{}, // No previous values
53+
[]codersdk.WorkspaceBuildParameter{}, // No new build values
54+
[]database.TemplateVersionPresetParameter{}, // No preset values
55+
)
56+
require.NoError(t, err)
57+
require.Equal(t, map[string]string{"immutable": "foo"}, values)
58+
})
59+
}

‎enterprise/coderd/dynamicparameters_test.go

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,57 @@ func TestDynamicParameterBuild(t *testing.T) {
302302
require.ErrorContains(t, err, "Number must be between 0 and 10")
303303
})
304304
})
305+
306+
t.Run("ImmutableValidation", func(t *testing.T) {
307+
t.Parallel()
308+
309+
// NewImmutable tests the case where a new immutable parameter is added to a template
310+
// after a workspace has been created with an older version of the template.
311+
// The test tries to delete the workspace, which should succeed.
312+
t.Run("NewImmutable", func(t *testing.T) {
313+
t.Parallel()
314+
315+
ctx := testutil.Context(t, testutil.WaitShort)
316+
// Start with a new template that has 0 parameters
317+
empty, _ := coderdtest.DynamicParameterTemplate(t, templateAdmin, orgID, coderdtest.DynamicParameterTemplateParams{
318+
MainTF: string(must(os.ReadFile("testdata/parameters/none/main.tf"))),
319+
})
320+
321+
// Create the workspace with 0 parameters
322+
wrk, err := templateAdmin.CreateUserWorkspace(ctx, codersdk.Me, codersdk.CreateWorkspaceRequest{
323+
TemplateID: empty.ID,
324+
Name: coderdtest.RandomUsername(t),
325+
RichParameterValues: []codersdk.WorkspaceBuildParameter{},
326+
})
327+
require.NoError(t, err)
328+
coderdtest.AwaitWorkspaceBuildJobCompleted(t, templateAdmin, wrk.LatestBuild.ID)
329+
330+
// Update the template with a new immutable parameter
331+
_, immutable := coderdtest.DynamicParameterTemplate(t, templateAdmin, orgID, coderdtest.DynamicParameterTemplateParams{
332+
MainTF: string(must(os.ReadFile("testdata/parameters/immutable/main.tf"))),
333+
TemplateID: empty.ID,
334+
})
335+
336+
bld, err := templateAdmin.CreateWorkspaceBuild(ctx, wrk.ID, codersdk.CreateWorkspaceBuildRequest{
337+
TemplateVersionID: immutable.ID, // Use the new template version with the immutable parameter
338+
Transition: codersdk.WorkspaceTransitionDelete,
339+
DryRun: false,
340+
})
341+
require.NoError(t, err)
342+
coderdtest.AwaitWorkspaceBuildJobCompleted(t, templateAdmin, bld.ID)
343+
344+
// Verify the immutable parameter is set on the workspace build
345+
params, err := templateAdmin.WorkspaceBuildParameters(ctx, bld.ID)
346+
require.NoError(t, err)
347+
require.Len(t, params, 1)
348+
require.Equal(t, "Hello World", params[0].Value)
349+
350+
// Verify the workspace is deleted
351+
deleted, err := templateAdmin.DeletedWorkspace(ctx, wrk.ID)
352+< 1C6A div class="diff-text-inner"> require.NoError(t, err)
353+
require.Equal(t, wrk.ID, deleted.ID, "workspace should be deleted")
354+
})
355+
})
305356
}
306357

307358
// TestDynamicParameterTemplate uses a template with some dynamic elements, and

‎enterprise/coderd/testdata/parameters/immutable/main.tf

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
terraform {
2+
required_providers {
3+
coder = {
4+
source = "coder/coder"
5+
}
6+
}
7+
}
8+
9+
data "coder_workspace_owner" "me" {}
10+
11+
data "coder_parameter" "immutable" {
12+
name = "immutable"
13+
type = "string"
14+
mutable = false
15+
default = "Hello World"
16+
}

‎enterprise/coderd/testdata/parameters/none/main.tf

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
terraform {
2+
required_providers {
3+
coder = {
4+
source = "coder/coder"
5+
}
6+
}
7+
}
8+
9+
data "coder_workspace_owner" "me" {}
10+

0 commit comments

Comments
 (0)
0