From fb2c92ff9f2bfe34fa859feb536f24fa65f8b5bd Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Tue, 21 May 2024 12:36:59 +0200 Subject: [PATCH 1/9] feat: evaluate provisioner tags --- coderd/wsbuilder/wsbuilder.go | 81 +++++++++++++++++++++++++++++++---- 1 file changed, 72 insertions(+), 9 deletions(-) diff --git a/coderd/wsbuilder/wsbuilder.go b/coderd/wsbuilder/wsbuilder.go index b34eb9ce3c858..ee69891356ab2 100644 --- a/coderd/wsbuilder/wsbuilder.go +++ b/coderd/wsbuilder/wsbuilder.go @@ -7,6 +7,7 @@ import ( "database/sql" "encoding/json" "fmt" + "log" "net/http" "time" @@ -55,14 +56,17 @@ type Builder struct { store database.Store // cache of objects, so we only fetch once - template *database.Template - templateVersion *database.TemplateVersion - templateVersionJob *database.ProvisionerJob - templateVersionParameters *[]database.TemplateVersionParameter - lastBuild *database.WorkspaceBuild - lastBuildErr *error - lastBuildParameters *[]database.WorkspaceBuildParameter - lastBuildJob *database.ProvisionerJob + template *database.Template + templateVersion *database.TemplateVersion + templateVersionJob *database.ProvisionerJob + templateVersionParameters *[]database.TemplateVersionParameter + templateVersionWorkspaceTags *[]database.TemplateVersionWorkspaceTag + lastBuild *database.WorkspaceBuild + lastBuildErr *error + lastBuildParameters *[]database.WorkspaceBuildParameter + lastBuildJob *database.ProvisionerJob + parameterNames *[]string + parameterValues *[]string verifyNoLegacyParametersOnce bool } @@ -297,7 +301,11 @@ func (b *Builder) buildTx(authFunc func(action policy.Action, object rbac.Object if err != nil { return nil, nil, BuildError{http.StatusInternalServerError, "marshal metadata", err} } - tags := provisionersdk.MutateTags(b.workspace.OwnerID, templateVersionJob.Tags) + + tags, err := b.getProvisionerTags() + if err != nil { + return nil, nil, err // already wrapped BuildError + } now := dbtime.Now() provisionerJob, err := b.store.InsertProvisionerJob(b.ctx, database.InsertProvisionerJobParams{ @@ -364,6 +372,7 @@ func (b *Builder) buildTx(authFunc func(action policy.Action, object rbac.Object // getParameters already wraps errors in BuildError return err } + err = store.InsertWorkspaceBuildParameters(b.ctx, database.InsertWorkspaceBuildParametersParams{ WorkspaceBuildID: workspaceBuildID, Name: names, @@ -502,6 +511,10 @@ func (b *Builder) getState() ([]byte, error) { } func (b *Builder) getParameters() (names, values []string, err error) { + if b.parameterNames != nil { + return *b.parameterNames, *b.parameterValues, nil + } + templateVersionParameters, err := b.getTemplateVersionParameters() if err != nil { return nil, nil, BuildError{http.StatusInternalServerError, "failed to fetch template version parameters", err} @@ -535,6 +548,9 @@ func (b *Builder) getParameters() (names, values []string, err error) { names = append(names, templateVersionParameter.Name) values = append(values, value) } + + b.parameterNames = &names + b.parameterValues = &values return names, values, nil } @@ -632,6 +648,53 @@ func (b *Builder) getLastBuildJob() (*database.ProvisionerJob, error) { return b.lastBuildJob, nil } +func (b *Builder) getProvisionerTags() (map[string]string, error) { + // Step 1: Fetch required data + workspaceTags, err := b.getTemplateVersionWorkspaceTags() + if err != nil { + return nil, BuildError{http.StatusInternalServerError, "failed to fetch template version workspace tags", err} + } + parameterNames, parameterValues, err := b.getParameters() + if err != nil { + return nil, err // already wrapped BuildError + } + templateVersionJob, err := b.getTemplateVersionJob() + if err != nil { + return nil, BuildError{http.StatusInternalServerError, "failed to fetch template version job", err} + } + annotationTags := provisionersdk.MutateTags(b.workspace.OwnerID, templateVersionJob.Tags) + + // Step 2: Evaluate provisioner tags + tags := map[string]string{} + for name, value := range annotationTags { + tags[name] = value + } + + // FIXME evaluate and merge workspace tags + log.Println(workspaceTags, parameterNames, parameterValues) + + return tags, nil +} + +func (b *Builder) getTemplateVersionWorkspaceTags() ([]database.TemplateVersionWorkspaceTag, error) { + if b.templateVersionWorkspaceTags != nil { + return *b.templateVersionWorkspaceTags, nil + } + + templateVersion, err := b.getTemplateVersion() + if err != nil { + return nil, xerrors.Errorf("get template version: %w", err) + } + + workspaceTags, err := b.store.GetTemplateVersionWorkspaceTags(b.ctx, templateVersion.ID) + if err != nil { + return nil, xerrors.Errorf("get template version workspace tags: %w", err) + } + + b.templateVersionWorkspaceTags = &workspaceTags + return *b.templateVersionWorkspaceTags, nil +} + // authorize performs build authorization pre-checks using the provided authFunc func (b *Builder) authorize(authFunc func(action policy.Action, object rbac.Objecter) bool) error { // Doing this up front saves a lot of work if the user doesn't have permission. From 98e623ae48e5b5fa3ea529f4b055689bd516800b Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Tue, 21 May 2024 13:25:24 +0200 Subject: [PATCH 2/9] fix: tests --- coderd/wsbuilder/wsbuilder.go | 2 +- coderd/wsbuilder/wsbuilder_test.go | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/coderd/wsbuilder/wsbuilder.go b/coderd/wsbuilder/wsbuilder.go index ee69891356ab2..9993b69531aa4 100644 --- a/coderd/wsbuilder/wsbuilder.go +++ b/coderd/wsbuilder/wsbuilder.go @@ -687,7 +687,7 @@ func (b *Builder) getTemplateVersionWorkspaceTags() ([]database.TemplateVersionW } workspaceTags, err := b.store.GetTemplateVersionWorkspaceTags(b.ctx, templateVersion.ID) - if err != nil { + if err != nil && !xerrors.Is(err, sql.ErrNoRows) { return nil, xerrors.Errorf("get template version workspace tags: %w", err) } diff --git a/coderd/wsbuilder/wsbuilder_test.go b/coderd/wsbuilder/wsbuilder_test.go index f1c7e6b62a493..343bc48562760 100644 --- a/coderd/wsbuilder/wsbuilder_test.go +++ b/coderd/wsbuilder/wsbuilder_test.go @@ -60,6 +60,7 @@ func TestBuilder_NoOptions(t *testing.T) { withLastBuildFound, withRichParameters(nil), withParameterSchemas(inactiveJobID, nil), + withWorkspaceTags(inactiveVersionID, nil), // Outputs expectProvisionerJob(func(job database.InsertProvisionerJobParams) { @@ -112,6 +113,7 @@ func TestBuilder_Initiator(t *testing.T) { withLastBuildFound, withRichParameters(nil), withParameterSchemas(inactiveJobID, nil), + withWorkspaceTags(inactiveVersionID, nil), // Outputs expectProvisionerJob(func(job database.InsertProvisionerJobParams) { @@ -154,6 +156,7 @@ func TestBuilder_Baggage(t *testing.T) { withLastBuildFound, withRichParameters(nil), withParameterSchemas(inactiveJobID, nil), + withWorkspaceTags(inactiveVersionID, nil), // Outputs expectProvisionerJob(func(job database.InsertProvisionerJobParams) { @@ -188,6 +191,7 @@ func TestBuilder_Reason(t *testing.T) { withLastBuildFound, withRichParameters(nil), withParameterSchemas(inactiveJobID, nil), + withWorkspaceTags(inactiveVersionID, nil), // Outputs expectProvisionerJob(func(job database.InsertProvisionerJobParams) { @@ -813,6 +817,18 @@ func withRichParameters(params []database.WorkspaceBuildParameter) func(mTx *dbm } } +func withWorkspaceTags(versionID uuid.UUID, tags []database.TemplateVersionWorkspaceTag) func(mTx *dbmock.MockStore) { + return func(mTx *dbmock.MockStore) { + c := mTx.EXPECT().GetTemplateVersionWorkspaceTags(gomock.Any(), versionID). + Times(1) + if len(tags) > 0 { + c.Return(tags, nil) + } else { + c.Return(nil, sql.ErrNoRows) + } + } +} + // Since there is expected to be only one each of job, build, and build-parameters inserted, instead // of building matchers, we match any call and then assert its parameters. This will feel // more familiar to the way we write other tests. From 031aeb6cdc684fc1f7dec389afe74465594445f0 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Tue, 21 May 2024 14:15:42 +0200 Subject: [PATCH 3/9] fix: tests --- coderd/wsbuilder/wsbuilder.go | 2 +- coderd/wsbuilder/wsbuilder_test.go | 13 ++++++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/coderd/wsbuilder/wsbuilder.go b/coderd/wsbuilder/wsbuilder.go index 9993b69531aa4..f0b059ab06d86 100644 --- a/coderd/wsbuilder/wsbuilder.go +++ b/coderd/wsbuilder/wsbuilder.go @@ -670,7 +670,7 @@ func (b *Builder) getProvisionerTags() (map[string]string, error) { tags[name] = value } - // FIXME evaluate and merge workspace tags + // TODO: Take "workspaceTags", evaluate expressions using parameters, update "tags" map log.Println(workspaceTags, parameterNames, parameterValues) return tags, nil diff --git a/coderd/wsbuilder/wsbuilder_test.go b/coderd/wsbuilder/wsbuilder_test.go index 343bc48562760..cebf1c0cc3a64 100644 --- a/coderd/wsbuilder/wsbuilder_test.go +++ b/coderd/wsbuilder/wsbuilder_test.go @@ -225,6 +225,7 @@ func TestBuilder_ActiveVersion(t *testing.T) { withActiveVersion(nil), withLastBuildNotFound, withParameterSchemas(activeJobID, nil), + withWorkspaceTags(activeVersionID, nil), // previous rich parameters are not queried because there is no previous build. // Outputs @@ -306,6 +307,7 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) { withLastBuildFound, withRichParameters(initialBuildParameters), withParameterSchemas(inactiveJobID, nil), + withWorkspaceTags(inactiveVersionID, nil), // Outputs expectProvisionerJob(func(job database.InsertProvisionerJobParams) {}), @@ -349,6 +351,7 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) { withLastBuildFound, withRichParameters(initialBuildParameters), withParameterSchemas(inactiveJobID, nil), + withWorkspaceTags(inactiveVersionID, nil), // Outputs expectProvisionerJob(func(job database.InsertProvisionerJobParams) {}), @@ -398,6 +401,7 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) { withLastBuildFound, withRichParameters(nil), withParameterSchemas(inactiveJobID, schemas), + withWorkspaceTags(inactiveVersionID, nil), // Outputs expectProvisionerJob(func(job database.InsertProvisionerJobParams) {}), @@ -433,13 +437,10 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) { withLastBuildFound, withRichParameters(initialBuildParameters), withParameterSchemas(inactiveJobID, nil), + withWorkspaceTags(inactiveVersionID, nil), // Outputs - expectProvisionerJob(func(job database.InsertProvisionerJobParams) {}), - withInTx, - expectBuild(func(bld database.InsertWorkspaceBuildParams) {}), - // no build parameters, since we hit an error validating. - // expectBuildParameters(func(params database.InsertWorkspaceBuildParametersParams) {}), + // no transaction, since we failed fast while validation build parameters ) ws := database.Workspace{ID: workspaceID, TemplateID: templateID, OwnerID: userID} @@ -486,6 +487,7 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) { withLastBuildFound, withRichParameters(initialBuildParameters), withParameterSchemas(activeJobID, nil), + withWorkspaceTags(activeVersionID, nil), // Outputs expectProvisionerJob(func(job database.InsertProvisionerJobParams) {}), @@ -546,6 +548,7 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) { withLastBuildFound, withRichParameters(initialBuildParameters), withParameterSchemas(activeJobID, nil), + withWorkspaceTags(activeVersionID, nil), // Outputs expectProvisionerJob(func(job database.InsertProvisionerJobParams) {}), From 8706ac2d44137323ed933d26f774887dc46d67a3 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Tue, 21 May 2024 14:51:58 +0200 Subject: [PATCH 4/9] WIP --- coderd/wsbuilder/wsbuilder_test.go | 51 +++++++++++++++++++++++++++--- 1 file changed, 46 insertions(+), 5 deletions(-) diff --git a/coderd/wsbuilder/wsbuilder_test.go b/coderd/wsbuilder/wsbuilder_test.go index cebf1c0cc3a64..2cd4aa3232cbd 100644 --- a/coderd/wsbuilder/wsbuilder_test.go +++ b/coderd/wsbuilder/wsbuilder_test.go @@ -251,6 +251,51 @@ func TestBuilder_ActiveVersion(t *testing.T) { req.NoError(err) } +func TestWorkspaceBuildWithTags(t *testing.T) { + t.Parallel() + + req := require.New(t) + + richParameters := []database.TemplateVersionParameter{ + {Name: "cluster", Description: "This is first parameter", DefaultValue: "developers", Mutable: false}, + // Parameters can be mutable although it is discouraged as the workspace can be moved between provisioner nodes. + {Name: "project", Description: "This is second parameter", Mutable: true}, + {Name: "is_debug_build", Type: "bool", Description: "This is third parameter", Mutable: false}, + } + + buildParameters := []codersdk.WorkspaceBuildParameter{ + {Name: "cluster", Value: "developers"}, + {Name: "project", Value: "foobar-foobaz"}, + {Name: "is_debug_build", Value: "true"}, + } + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + mDB := expectDB(t, + // Inputs + withTemplate, + withInactiveVersion(richParameters), + withLastBuildFound, + withRichParameters(nil), + withParameterSchemas(inactiveJobID, nil), + withWorkspaceTags(inactiveVersionID, nil), + + // Outputs + expectProvisionerJob(func(_ database.InsertProvisionerJobParams) {}), + withInTx, + expectBuild(func(_ database.InsertWorkspaceBuildParams) {}), + expectBuildParameters(func(_ database.InsertWorkspaceBuildParametersParams) { + }), + withBuild, + ) + + ws := database.Workspace{ID: workspaceID, TemplateID: templateID, OwnerID: userID} + uut := wsbuilder.New(ws, database.WorkspaceTransitionStart).RichParameterValues(buildParameters) + _, _, err := uut.Build(ctx, mDB, nil, audit.WorkspaceBuildBaggage{}) + req.NoError(err) +} + func TestWorkspaceBuildWithRichParameters(t *testing.T) { t.Parallel() @@ -402,11 +447,6 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) { withRichParameters(nil), withParameterSchemas(inactiveJobID, schemas), withWorkspaceTags(inactiveVersionID, nil), - - // Outputs - expectProvisionerJob(func(job database.InsertProvisionerJobParams) {}), - withInTx, - expectBuild(func(bld database.InsertWorkspaceBuildParams) {}), ) ws := database.Workspace{ID: workspaceID, TemplateID: templateID, OwnerID: userID} @@ -607,6 +647,7 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) { withLastBuildFound, withRichParameters(initialBuildParameters), withParameterSchemas(activeJobID, nil), + withWorkspaceTags(activeVersionID, nil), // Outputs expectProvisionerJob(func(job database.InsertProvisionerJobParams) {}), From 6fb78e4d7edef0279a7a66c5aad62e7bd5a1bdf4 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Tue, 21 May 2024 17:00:53 +0200 Subject: [PATCH 5/9] WIP --- coderd/wsbuilder/wsbuilder_test.go | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/coderd/wsbuilder/wsbuilder_test.go b/coderd/wsbuilder/wsbuilder_test.go index 2cd4aa3232cbd..1cfa8c6ad5124 100644 --- a/coderd/wsbuilder/wsbuilder_test.go +++ b/coderd/wsbuilder/wsbuilder_test.go @@ -256,15 +256,28 @@ func TestWorkspaceBuildWithTags(t *testing.T) { req := require.New(t) + workspaceTags := []database.TemplateVersionWorkspaceTag{ + { + Key: "cluster_tag", + Value: "developers", + }, + { + Key: "project_tag", + Value: `"${data.coder_parameter.project.value}+12345"`, + }, + { + Key: "is_debug_build", + Value: `data.coder_parameter.is_debug_build.value == "true" ? "in-debug-mode" : "no-debug"`, + }, + } + richParameters := []database.TemplateVersionParameter{ - {Name: "cluster", Description: "This is first parameter", DefaultValue: "developers", Mutable: false}, // Parameters can be mutable although it is discouraged as the workspace can be moved between provisioner nodes. - {Name: "project", Description: "This is second parameter", Mutable: true}, - {Name: "is_debug_build", Type: "bool", Description: "This is third parameter", Mutable: false}, + {Name: "project", Description: "This is second parameter", Mutable: true, Options: json.RawMessage("[]")}, + {Name: "is_debug_build", Type: "bool", Description: "This is third parameter", Mutable: false, Options: json.RawMessage("[]")}, } buildParameters := []codersdk.WorkspaceBuildParameter{ - {Name: "cluster", Value: "developers"}, {Name: "project", Value: "foobar-foobaz"}, {Name: "is_debug_build", Value: "true"}, } @@ -279,7 +292,7 @@ func TestWorkspaceBuildWithTags(t *testing.T) { withLastBuildFound, withRichParameters(nil), withParameterSchemas(inactiveJobID, nil), - withWorkspaceTags(inactiveVersionID, nil), + withWorkspaceTags(inactiveVersionID, workspaceTags), // Outputs expectProvisionerJob(func(_ database.InsertProvisionerJobParams) {}), From a77a2dc22c15d3f41e9d03471cdac3c5caab14df Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Tue, 21 May 2024 17:04:06 +0200 Subject: [PATCH 6/9] Stub for eval --- coderd/wsbuilder/wsbuilder_test.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/coderd/wsbuilder/wsbuilder_test.go b/coderd/wsbuilder/wsbuilder_test.go index 1cfa8c6ad5124..47f93e74512af 100644 --- a/coderd/wsbuilder/wsbuilder_test.go +++ b/coderd/wsbuilder/wsbuilder_test.go @@ -265,6 +265,10 @@ func TestWorkspaceBuildWithTags(t *testing.T) { Key: "project_tag", Value: `"${data.coder_parameter.project.value}+12345"`, }, + { + Key: "team_tag", + Value: `"data.coder_parameter.team.value`, + }, { Key: "is_debug_build", Value: `data.coder_parameter.is_debug_build.value == "true" ? "in-debug-mode" : "no-debug"`, @@ -273,12 +277,14 @@ func TestWorkspaceBuildWithTags(t *testing.T) { richParameters := []database.TemplateVersionParameter{ // Parameters can be mutable although it is discouraged as the workspace can be moved between provisioner nodes. - {Name: "project", Description: "This is second parameter", Mutable: true, Options: json.RawMessage("[]")}, + {Name: "project", Description: "This is first parameter", Mutable: true, Options: json.RawMessage("[]")}, + {Name: "team", Description: "This is second parameter", Mutable: true, DefaultValue: "godzilla", Options: json.RawMessage("[]")}, {Name: "is_debug_build", Type: "bool", Description: "This is third parameter", Mutable: false, Options: json.RawMessage("[]")}, } buildParameters := []codersdk.WorkspaceBuildParameter{ {Name: "project", Value: "foobar-foobaz"}, + // "team" parameter is skipped, so default value is selected {Name: "is_debug_build", Value: "true"}, } From 00533fa67de0e7b971735b4293cfb9702dafe02c Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 22 May 2024 14:56:49 +0200 Subject: [PATCH 7/9] Works --- coderd/wsbuilder/wsbuilder.go | 58 ++++++++++++++++++++++++++++-- coderd/wsbuilder/wsbuilder_test.go | 44 +++++++++++++++++++---- 2 files changed, 93 insertions(+), 9 deletions(-) diff --git a/coderd/wsbuilder/wsbuilder.go b/coderd/wsbuilder/wsbuilder.go index f0b059ab06d86..6795746d06b8b 100644 --- a/coderd/wsbuilder/wsbuilder.go +++ b/coderd/wsbuilder/wsbuilder.go @@ -7,10 +7,13 @@ import ( "database/sql" "encoding/json" "fmt" - "log" "net/http" "time" + "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/hcl/v2/hclsyntax" + "github.com/zclconf/go-cty/cty" + "github.com/coder/coder/v2/coderd/rbac/policy" "github.com/coder/coder/v2/provisionersdk" @@ -670,12 +673,61 @@ func (b *Builder) getProvisionerTags() (map[string]string, error) { tags[name] = value } - // TODO: Take "workspaceTags", evaluate expressions using parameters, update "tags" map - log.Println(workspaceTags, parameterNames, parameterValues) + evalCtx := buildParametersEvalContext(parameterNames, parameterValues) + for _, workspaceTag := range workspaceTags { + expr, diags := hclsyntax.ParseExpression([]byte(workspaceTag.Value), "partial.hcl", hcl.InitialPos) + if diags.HasErrors() { + return nil, BuildError{http.StatusBadRequest, "failed to parse workspace tag value", xerrors.Errorf(diags.Error())} + } + val, diags := expr.Value(evalCtx) + if diags.HasErrors() { + return nil, BuildError{http.StatusBadRequest, "failed to evaluate workspace tag value", xerrors.Errorf(diags.Error())} + } + + // Do not use "val.AsString()" as it can panic + str, err := ctyValueString(val) + if err != nil { + return nil, BuildError{http.StatusBadRequest, "failed to marshal cty.Value as string", err} + } + tags[workspaceTag.Key] = str + } return tags, nil } +func buildParametersEvalContext(names, values []string) *hcl.EvalContext { + m := map[string]cty.Value{} + for i, name := range names { + m[name] = cty.MapVal(map[string]cty.Value{ + "value": cty.StringVal(values[i]), + }) + } + return &hcl.EvalContext{ + Variables: map[string]cty.Value{ + "data": cty.MapVal(map[string]cty.Value{ + "coder_parameter": cty.MapVal(m), + }), + }, + } +} + +func ctyValueString(val cty.Value) (string, error) { + switch val.Type() { + case cty.Bool: + if val.True() { + return "true", nil + } else { + return "false", nil + } + case cty.Number: + return val.AsBigFloat().String(), nil + case cty.String: + return val.AsString(), nil + default: + return "", xerrors.Errorf("only primitive types are supported - bool, number, and string") + } +} + func (b *Builder) getTemplateVersionWorkspaceTags() ([]database.TemplateVersionWorkspaceTag, error) { if b.templateVersionWorkspaceTags != nil { return *b.templateVersionWorkspaceTags, nil diff --git a/coderd/wsbuilder/wsbuilder_test.go b/coderd/wsbuilder/wsbuilder_test.go index 47f93e74512af..ad53cd7d45609 100644 --- a/coderd/wsbuilder/wsbuilder_test.go +++ b/coderd/wsbuilder/wsbuilder_test.go @@ -194,7 +194,7 @@ func TestBuilder_Reason(t *testing.T) { withWorkspaceTags(inactiveVersionID, nil), // Outputs - expectProvisionerJob(func(job database.InsertProvisionerJobParams) { + expectProvisionerJob(func(_ database.InsertProvisionerJobParams) { }), withInTx, expectBuild(func(bld database.InsertWorkspaceBuildParams) { @@ -254,12 +254,17 @@ func TestBuilder_ActiveVersion(t *testing.T) { func TestWorkspaceBuildWithTags(t *testing.T) { t.Parallel() + asrt := assert.New(t) req := require.New(t) workspaceTags := []database.TemplateVersionWorkspaceTag{ + { + Key: "fruits_tag", + Value: "data.coder_parameter.number_of_apples.value + data.coder_parameter.number_of_oranges.value", + }, { Key: "cluster_tag", - Value: "developers", + Value: `"best_developers"`, }, { Key: "project_tag", @@ -267,7 +272,15 @@ func TestWorkspaceBuildWithTags(t *testing.T) { }, { Key: "team_tag", - Value: `"data.coder_parameter.team.value`, + Value: `data.coder_parameter.team.value`, + }, + { + Key: "yes_or_no", + Value: `data.coder_parameter.is_debug_build.value`, + }, + { + Key: "actually_no", + Value: `!data.coder_parameter.is_debug_build.value`, }, { Key: "is_debug_build", @@ -279,13 +292,15 @@ func TestWorkspaceBuildWithTags(t *testing.T) { // Parameters can be mutable although it is discouraged as the workspace can be moved between provisioner nodes. {Name: "project", Description: "This is first parameter", Mutable: true, Options: json.RawMessage("[]")}, {Name: "team", Description: "This is second parameter", Mutable: true, DefaultValue: "godzilla", Options: json.RawMessage("[]")}, - {Name: "is_debug_build", Type: "bool", Description: "This is third parameter", Mutable: false, Options: json.RawMessage("[]")}, + {Name: "is_debug_build", Type: "bool", Description: "This is third parameter", Mutable: false, DefaultValue: "false", Options: json.RawMessage("[]")}, + {Name: "number_of_apples", Type: "number", Description: "This is fourth parameter", Mutable: false, DefaultValue: "4", Options: json.RawMessage("[]")}, + {Name: "number_of_oranges", Type: "number", Description: "This is fifth parameter", Mutable: false, DefaultValue: "6", Options: json.RawMessage("[]")}, } buildParameters := []codersdk.WorkspaceBuildParameter{ {Name: "project", Value: "foobar-foobaz"}, - // "team" parameter is skipped, so default value is selected {Name: "is_debug_build", Value: "true"}, + // Parameters "team", "number_of_apples", "number_of_oranges" are skipped, so default value is selected } ctx, cancel := context.WithCancel(context.Background()) @@ -301,7 +316,24 @@ func TestWorkspaceBuildWithTags(t *testing.T) { withWorkspaceTags(inactiveVersionID, workspaceTags), // Outputs - expectProvisionerJob(func(_ database.InsertProvisionerJobParams) {}), + expectProvisionerJob(func(job database.InsertProvisionerJobParams) { + asrt.Len(job.Tags, 10) + + expected := database.StringMap{ + "actually_no": "false", + "cluster_tag": "best_developers", + "fruits_tag": "10", + "is_debug_build": "in-debug-mode", + "project_tag": "foobar-foobaz+12345", + "team_tag": "godzilla", + "yes_or_no": "true", + + "scope": "user", + "version": "inactive", + "owner": userID.String(), + } + asrt.Equal(job.Tags, expected) + }), withInTx, expectBuild(func(_ database.InsertWorkspaceBuildParams) {}), expectBuildParameters(func(_ database.InsertWorkspaceBuildParametersParams) { From 92c999200711913ad7c216d108dcd62b19da7385 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 22 May 2024 15:11:04 +0200 Subject: [PATCH 8/9] fix: panic --- coderd/wsbuilder/wsbuilder.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/coderd/wsbuilder/wsbuilder.go b/coderd/wsbuilder/wsbuilder.go index 6795746d06b8b..0ec39ccfd6ef0 100644 --- a/coderd/wsbuilder/wsbuilder.go +++ b/coderd/wsbuilder/wsbuilder.go @@ -675,7 +675,7 @@ func (b *Builder) getProvisionerTags() (map[string]string, error) { evalCtx := buildParametersEvalContext(parameterNames, parameterValues) for _, workspaceTag := range workspaceTags { - expr, diags := hclsyntax.ParseExpression([]byte(workspaceTag.Value), "partial.hcl", hcl.InitialPos) + expr, diags := hclsyntax.ParseExpression([]byte(workspaceTag.Value), "expression.hcl", hcl.InitialPos) if diags.HasErrors() { return nil, BuildError{http.StatusBadRequest, "failed to parse workspace tag value", xerrors.Errorf(diags.Error())} } @@ -702,6 +702,11 @@ func buildParametersEvalContext(names, values []string) *hcl.EvalContext { "value": cty.StringVal(values[i]), }) } + + if len(m) == 0 { + return nil // otherwise, panic: must not call MapVal with empty map + } + return &hcl.EvalContext{ Variables: map[string]cty.Value{ "data": cty.MapVal(map[string]cty.Value{ From ac1040eb3eb6ce8a4c924bc327d8bd16f4a0d465 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Thu, 23 May 2024 09:43:48 +0200 Subject: [PATCH 9/9] rearrange --- coderd/wsbuilder/wsbuilder.go | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/coderd/wsbuilder/wsbuilder.go b/coderd/wsbuilder/wsbuilder.go index 0ec39ccfd6ef0..9e8de1d688768 100644 --- a/coderd/wsbuilder/wsbuilder.go +++ b/coderd/wsbuilder/wsbuilder.go @@ -652,27 +652,28 @@ func (b *Builder) getLastBuildJob() (*database.ProvisionerJob, error) { } func (b *Builder) getProvisionerTags() (map[string]string, error) { - // Step 1: Fetch required data - workspaceTags, err := b.getTemplateVersionWorkspaceTags() - if err != nil { - return nil, BuildError{http.StatusInternalServerError, "failed to fetch template version workspace tags", err} - } - parameterNames, parameterValues, err := b.getParameters() - if err != nil { - return nil, err // already wrapped BuildError - } + // Step 1: Mutate template version tags templateVersionJob, err := b.getTemplateVersionJob() if err != nil { return nil, BuildError{http.StatusInternalServerError, "failed to fetch template version job", err} } annotationTags := provisionersdk.MutateTags(b.workspace.OwnerID, templateVersionJob.Tags) - // Step 2: Evaluate provisioner tags tags := map[string]string{} for name, value := range annotationTags { tags[name] = value } + // Step 2: Mutate workspace tags + workspaceTags, err := b.getTemplateVersionWorkspaceTags() + if err != nil { + return nil, BuildError{http.StatusInternalServerError, "failed to fetch template version workspace tags", err} + } + parameterNames, parameterValues, err := b.getParameters() + if err != nil { + return nil, err // already wrapped BuildError + } + evalCtx := buildParametersEvalContext(parameterNames, parameterValues) for _, workspaceTag := range workspaceTags { expr, diags := hclsyntax.ParseExpression([]byte(workspaceTag.Value), "expression.hcl", hcl.InitialPos)