8000 fix(provisioner/terraform/tfparse): skip evaluation of unrelated parameters by johnstcn · Pull Request #16023 · coder/coder · GitHub
[go: up one dir, main page]
Skip to content

fix(provisioner/terraform/tfparse): skip evaluation of unrelated parameters #16023

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jan 3, 2025
Merged

fix(provisioner/terraform/tfparse): skip evaluation of unrelated parameters #16023

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e56cac7
fix(provisioner/terraform/tfparse): skip evaluation of defaults if no…
johnstcn Jan 3, 2025
78d88fe
do some more intelligent HCL parsing to determine required variables
johnstcn Jan 3, 2025
222fd90
fallback to JSON-encoding default variable values
johnstcn Jan 3, 2025
de5541b
ensure unrelated variables without default values are allowed
johnstcn Jan 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
do some more intelligent HCL parsing to determine required variables
  • Loading branch information
@johnstcn
johnstcn committed Jan 3, 2025
commit 78d88fed357e834d1bb2db468b0f7db4f6cd7053
2 changes: 1 addition & 1 deletion provisioner/terraform/parse.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func (s *server) Parse(sess *provisionersdk.Session, _ *proto.ParseRequest, _ <-
return provisionersdk.ParseErrorf("load module: %s", formatDiagnostics(sess.WorkDirectory, diags))
}

workspaceTags, err := parser.WorkspaceTags(ctx)
workspaceTags, _, err := parser.WorkspaceTags(ctx)
if err != nil {
return provisionersdk.ParseErrorf("can't load workspace tags: %v", err)
}
Expand Down
117 changes: 75 additions & 42 deletions provisioner/terraform/tfparse/tfparse.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,12 @@ func New(workdir string, opts ...Option) (*Parser, tfconfig.Diagnostics) {
}

// WorkspaceTags looks for all coder_workspace_tags datasource in the module
// and returns the raw values for the tags.
func (p *Parser) WorkspaceTags(ctx context.Context) (map[string]string, error) {
// and returns the raw values for the tags. It also returns the set of
// variables referenced by any expressions in the raw values of tags.
func (p *Parser) WorkspaceTags(ctx context.Context) (map[string]string, map[string]struct{}, error) {
tags := map[string]string{}
var skipped []string
skipped := []string{}
requiredVars := map[string]struct{}{}
for _, dataResource := range p.module.DataResources {
if dataResource.Type != "coder_workspace_tags" {
skipped = append(skipped, strings.Join([]string{"data", dataResource.Type, dataResource.Name}, "."))
Expand All @@ -99,13 +101,13 @@ func (p *Parser) WorkspaceTags(ctx context.Context) (map[string]string, error) {
// We know in which HCL file is the data resource defined.
file, diags = p.underlying.ParseHCLFile(dataResource.Pos.Filename)
if diags.HasErrors() {
return nil, xerrors.Errorf("can't parse the resource file: %s", diags.Error())
return nil, nil, xerrors.Errorf("can't parse the resource file: %s", diags.Error())
}

// Parse root to find "coder_workspace_tags".
content, _, diags := file.Body.PartialContent(rootTemplateSchema)
if diags.HasErrors() {
return nil, xerrors.Errorf("can't parse the resource file: %s", diags.Error())
return nil, nil, xerrors.Errorf("can't parse the resource file: %s", diags.Error())
}

// Iterate over blocks to locate the exact "coder_workspace_tags" data resource.
Expand All @@ -117,51 +119,99 @@ func (p *Parser) WorkspaceTags(ctx context.Context) (map[string]string, error) {
// Parse "coder_workspace_tags" to find all key-value tags.
resContent, _, diags := block.Body.PartialContent(coderWorkspaceTagsSchema)
if diags.HasErrors() {
return nil, xerrors.Errorf(`can't parse the resource coder_workspace_tags: %s`, diags.Error())
return nil, nil, xerrors.Errorf(`can't parse the resource coder_workspace_tags: %s`, diags.Error())
}

if resContent == nil {
continue // workspace tags are not present
}

if _, ok := resContent.Attributes["tags"]; !ok {
return nil, xerrors.Errorf(`"tags" attribute is required by coder_workspace_tags`)
return nil, nil, xerrors.Errorf(`"tags" attribute is required by coder_workspace_tags`)
}

expr := resContent.Attributes["tags"].Expr
tagsExpr, ok := expr.(*hclsyntax.ObjectConsExpr)
if !ok {
return nil, xerrors.Errorf(`"tags" attribute is expected to be a key-value map`)
return nil, nil, xerrors.Errorf(`"tags" attribute is expected to be a key-value map`)
}

// Parse key-value entries in "coder_workspace_tags"
for _, tagItem := range tagsExpr.Items {
key, err := previewFileContent(tagItem.KeyExpr.Range())
if err != nil {
return nil, xerrors.Errorf("can't preview the resource file: %v", err)
return nil, nil, xerrors.Errorf("can't preview the resource file: %v", err)
}
key = strings.Trim(key, `"`)

value, err := previewFileContent(tagItem.ValueExpr.Range())
if err != nil {
return nil, xerrors.Errorf("can't preview the resource file: %v", err)
return nil, nil, xerrors.Errorf("can't preview the resource file: %v", err)
}

if _, ok := tags[key]; ok {
return nil, xerrors.Errorf(`workspace tag %q is defined multiple times`, key)
return nil, nil, xerrors.Errorf(`workspace tag %q is defined multiple times`, key)
}
tags[key] = value

// Find values referenced by the expression.
refVars := referencedVariablesExpr(tagItem.ValueExpr)
for _, refVar := range refVars {
requiredVars[refVar] = struct{}{}
}
}
}
}
p.logger.Debug(ctx, "found workspace tags", slog.F("tags", maps.Keys(tags)), slog.F("skipped", skipped))
return tags, nil

requiredVarNames := maps.Keys(requiredVars)
slices.Sort(requiredVarNames)
p.logger.Debug(ctx, "found workspace tags", slog.F("tags", maps.Keys(tags)), slog.F("skipped", skipped), slog.F("required_vars", requiredVarNames))
return tags, requiredVars, nil
}

// referencedVariablesExpr determines the variables referenced in expr
// and returns the names of those variables.
func referencedVariablesExpr(expr hclsyntax.Expression) (names []string) {
var parts []string
for _, expVar := range expr.Variables() {
for _, tr := range expVar {
switch v := tr.(type) {
case hcl.TraverseRoot:
parts = append(parts, v.Name)
case hcl.TraverseAttr:
parts = append(parts, v.Name)
default: // skip
}
}

cleaned := cleanupTraversalName(parts)
names = append(names, strings.Join(cleaned, "."))
}
return names
}

// cleanupTraversalName chops off extraneous pieces of the traversal.
// for example:
// - var.foo -> unchanged
// - data.coder_parameter.bar.value -> data.coder_parameter.bar
// - null_resource.baz.zap -> null_resource.baz
func cleanupTraversalName(parts []string) []string {
if len(parts) == 0 {
return parts
}
if len(parts) > 3 && parts[0] == "data" {
return parts[:3]
}
if len(parts) > 2 {
return parts[:2]
}
return parts
}

func (p *Parser) WorkspaceTagDefaults(ctx context.Context) (map[string]string, error) {
// This only gets us the expressions. We need to evaluate them.
// Example: var.region -> "us"
tags, err := p.WorkspaceTags(ctx)
tags, requiredVars, err := p.WorkspaceTags(ctx)
if err != nil {
return nil, xerrors.Errorf("extract workspace tags: %w", err)
}
Expand All @@ -172,11 +222,11 @@ func (p *Parser) WorkspaceTagDefaults(ctx context.Context) (map[string]string, e

// To evaluate the expressions, we need to load the default values for
// variables and parameters.
varsDefaults, err := p.VariableDefaults(ctx, tags)
varsDefaults, err := p.VariableDefaults(ctx)
if err != nil {
return nil, xerrors.Errorf("load variable defaults: %w", err)
}
paramsDefaults, err := p.CoderParameterDefaults(ctx, varsDefaults, tags)
paramsDefaults, err := p.CoderParameterDefaults(ctx, varsDefaults, requiredVars)
if err != nil {
return nil, xerrors.Errorf("load parameter defaults: %w", err)
}
Expand Down Expand Up @@ -251,39 +301,28 @@ func WriteArchive(bs []byte, mimetype string, path string) error {
return nil
}

// VariableDefaults returns the default values for all variables referenced in the values of tags.
func (p *Parser) VariableDefaults(ctx context.Context, tags map[string]string) (map[string]string, error) {
var skipped []string
// VariableDefaults returns the default values for all variables in the module.
func (p *Parser) VariableDefaults(ctx context.Context) (map[string]string, error) {
// iterate through vars to get the default values for all
// required variables.
m := make(map[string]string)
for _, v := range p.module.Variables {
if v == nil {
continue
}
var found bool
for _, tv := range tags {
if strings.Contains(tv, v.Name) {
found = true
}
}
if !found {
skipped = append(skipped, "var."+v.Name)
continue
}
sv, err := interfaceToString(v.Default)
if err != nil {
return nil, xerrors.Errorf("can't convert variable default value to string: %v", err)
}
m[v.Name] = strings.Trim(sv, `"`)
}
p.logger.Debug(ctx, "found default values for variables", slog.F("defaults", m), slog.F("skipped", skipped))
p.logger.Debug(ctx, "found default values for variables", slog.F("defaults", m))
return m, nil
}

// CoderParameterDefaults returns the default values of all coder_parameter data sources
// in the parsed module.
func (p *Parser) CoderParameterDefaults(ctx context.Context, varsDefaults map[string]string, tags map[string]string) (map[string]string, error) {
func (p *Parser) CoderParameterDefaults(ctx context.Context, varsDefaults map[string]string, names map[string]struct{}) (map[string]string, error) {
defaultsM := make(map[string]string)
var (
skipped []string
Expand All @@ -296,23 +335,17 @@ func (p *Parser) CoderParameterDefaults(ctx context.Context, varsDefaults map[st
continue
}

if dataResource.Type != "coder_parameter" {
skipped = append(skipped, strings.Join([]string{"data", dataResource.Type, dataResource.Name}, "."))
continue
}

if !strings.HasSuffix(dataResource.Pos.Filename, ".tf") {
continue
}

var found bool
needle := strings.Join([]string{"data", dataResource.Type, dataResource.Name}, ".")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I can tell, there is no way to break a variable reference over multiple lines e.g.

data.\
coder_parameter.\
foo.\
value

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

correct 👍

for _, tv := range tags {
if strings.Contains(tv, needle) {
found = true
}
if dataResource.Type != "coder_parameter" {
skipped = append(skipped, needle)
continue
}
if !found {

if _, found := names[needle]; !found {
skipped = append(skipped, needle)
continue
}
Expand Down
28 changes: 25 additions & 3 deletions provisioner/terraform/tfparse/tfparse_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func Test_WorkspaceTagDefaultsFromFile(t *testing.T) {
name: "single text file",
files: map[string]string{
"file.txt": `
hello world`,
hello world`,
},
expectTags: map[string]string{},
expectError: "",
Expand Down Expand Up @@ -539,6 +539,28 @@ func Test_WorkspaceTagDefaultsFromFile(t *testing.T) {
expectTags: nil,
expectError: `can't convert variable default value to string: unsupported type map[string]interface {}`,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

review: also not allowed by the provider:

│   on main.tf line 38, in data "coder_workspace_tags" "tags":
│   38:   tags = {
│   39:     "mapvar" = {a = "b"}
│   40:   }
│
│ Inappropriate value for attribute "tags": element "mapvar": string required.

},
{
name: "overlapping var name",
files: map[string]string{
`main.tf`: `
variable "a" {
type = string
default = "1"
}
variable "ab" {
description = "This is a variable of type string"
type = string
default = "ab"
}
data "coder_workspace_tags" "tags" {
tags = {
"foo": "bar",
"a": var.a,
}
}`,
},
expectTags: map[string]string{"foo": "bar", "a": "1"},
},
} {
tc := tc
t.Run(tc.name+"/tar", func(t *testing.T) {
Expand Down Expand Up @@ -622,7 +644,7 @@ func BenchmarkWorkspaceTagDefaultsFromFile(b *testing.B) {
tfparse.WriteArchive(tarFile, "application/x-tar", tmpDir)
parser, diags := tfparse.New(tmpDir, tfparse.WithLogger(logger))
require.NoError(b, diags.Err())
_, err := parser.WorkspaceTags(ctx)
_, _, err := parser.WorkspaceTags(ctx)
if err != nil {
b.Fatal(err)
}
Expand All @@ -636,7 +658,7 @@ func BenchmarkWorkspaceTagDefaultsFromFile(b *testing.B) {
tfparse.WriteArchive(zipFile, "application/zip", tmpDir)
parser, diags := tfparse.New(tmpDir, tfparse.WithLogger(logger))
require.NoError(b, diags.Err())
_, err := parser.WorkspaceTags(ctx)
_, _, err := parser.WorkspaceTags(ctx)
if err != nil {
b.Fatal(err)
}
Expand Down
0