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

* Improves tfparse test coverage to include more parameter types and values
* Adds tests with unrelated parameters that should be ignored by tfparse
* Modifies tfparse to only attempt evaluation of parameters referenced by coder_workspace_tags
This commit is contained in:
Cian Johnston
2025-01-03 19:32:17 +00:00
committed by GitHub
parent 4e0963966d
commit 1ab10cf80c
5 changed files with 330 additions and 78 deletions
+86 -27
View File
@@ -293,6 +293,11 @@ func TestPostTemplateVersionsByOrganization(t *testing.T) {
type = string
default = "2"
}
data "coder_parameter" "unrelated" {
name = "unrelated"
type = "list(string)"
default = jsonencode(["a", "b"])
}
resource "null_resource" "test" {}`,
},
wantTags: map[string]string{"owner": "", "scope": "organization"},
@@ -301,18 +306,23 @@ func TestPostTemplateVersionsByOrganization(t *testing.T) {
name: "main.tf with empty workspace tags",
files: map[string]string{
`main.tf`: `
variable "a" {
type = string
default = "1"
}
data "coder_parameter" "b" {
type = string
default = "2"
}
resource "null_resource" "test" {}
data "coder_workspace_tags" "tags" {
tags = {}
}`,
variable "a" {
type = string
default = "1"
}
data "coder_parameter" "b" {
type = string
default = "2"
}
data "coder_parameter" "unrelated" {
name = "unrelated"
type = "list(string)"
default = jsonencode(["a", "b"])
}
resource "null_resource" "test" {}
data "coder_workspace_tags" "tags" {
tags = {}
}`,
},
wantTags: map[string]string{"owner": "", "scope": "organization"},
},
@@ -328,6 +338,11 @@ func TestPostTemplateVersionsByOrganization(t *testing.T) {
type = string
default = "2"
}
data "coder_parameter" "unrelated" {
name = "unrelated"
type = "list(string)"
default = jsonencode(["a", "b"])
}
resource "null_resource" "test" {}
data "coder_workspace_tags" "tags" {
tags = {
@@ -343,22 +358,28 @@ func TestPostTemplateVersionsByOrganization(t *testing.T) {
name: "main.tf with workspace tags and request tags",
files: map[string]string{
`main.tf`: `
variable "a" {
type = string
default = "1"
}
data "coder_parameter" "b" {
type = string
default = "2"
}
resource "null_resource" "test" {}
data "coder_workspace_tags" "tags" {
tags = {
"foo": "bar",
"a": var.a,
"b": data.coder_parameter.b.value,
// This file is the same as the above, except for this comment.
variable "a" {
type = string
default = "1"
}
}`,
data "coder_parameter" "b" {
type = string
default = "2"
}
data "coder_parameter" "unrelated" {
name = "unrelated"
type = "list(string)"
default = jsonencode(["a", "b"])
}
resource "null_resource" "test" {}
data "coder_workspace_tags" "tags" {
tags = {
"foo": "bar",
"a": var.a,
"b": data.coder_parameter.b.value,
}
}`,
},
reqTags: map[string]string{"baz": "zap", "foo": "noclobber"},
wantTags: map[string]string{"owner": "", "scope": "organization", "foo": "bar", "baz": "zap", "a": "1", "b": "2"},
@@ -375,6 +396,11 @@ func TestPostTemplateVersionsByOrganization(t *testing.T) {
type = string
default = "2"
}
data "coder_parameter" "unrelated" {
name = "unrelated"
type = "list(string)"
default = jsonencode(["a", "b"])
}
resource "null_resource" "test" {
name = "foo"
}
@@ -401,6 +427,11 @@ func TestPostTemplateVersionsByOrganization(t *testing.T) {
type = string
default = "2"
}
data "coder_parameter" "unrelated" {
name = "unrelated"
type = "list(string)"
default = jsonencode(["a", "b"])
}
resource "null_resource" "test" {
name = "foo"
}
@@ -423,6 +454,11 @@ func TestPostTemplateVersionsByOrganization(t *testing.T) {
name: "main.tf with workspace tags that attempts to set user scope",
files: map[string]string{
`main.tf`: `
data "coder_parameter" "unrelated" {
name = "unrelated"
type = "list(string)"
default = jsonencode(["a", "b"])
}
resource "null_resource" "test" {}
data "coder_workspace_tags" "tags" {
tags = {
@@ -437,6 +473,11 @@ func TestPostTemplateVersionsByOrganization(t *testing.T) {
name: "main.tf with workspace tags that attempt to clobber org ID",
files: map[string]string{
`main.tf`: `
data "coder_parameter" "unrelated" {
name = "unrelated"
type = "list(string)"
default = jsonencode(["a", "b"])
}
resource "null_resource" "test" {}
data "coder_workspace_tags" "tags" {
tags = {
@@ -451,6 +492,11 @@ func TestPostTemplateVersionsByOrganization(t *testing.T) {
name: "main.tf with workspace tags that set scope=user",
files: map[string]string{
`main.tf`: `
data "coder_parameter" "unrelated" {
name = "unrelated"
type = "list(string)"
default = jsonencode(["a", "b"])
}
resource "null_resource" "test" {}
data "coder_workspace_tags" "tags" {
tags = {
@@ -460,6 +506,19 @@ func TestPostTemplateVersionsByOrganization(t *testing.T) {
},
wantTags: map[string]string{"owner": templateAdminUser.ID.String(), "scope": "user"},
},
// Ref: https://github.com/coder/coder/issues/16021
{
name: "main.tf with no workspace_tags and a function call in a parameter default",
files: map[string]string{
`main.tf`: `
data "coder_parameter" "unrelated" {
name = "unrelated"
type = "list(string)"
default = jsonencode(["a", "b"])
}`,
},
wantTags: map[string]string{"owner": "", "scope": "organization"},
},
} {
tt := tt
t.Run(tt.name, func(t *testing.T) {
+5
View File
@@ -1412,6 +1412,11 @@ func TestWorkspaceTagsTerraform(t *testing.T) {
provider "coder" {}
data "coder_workspace" "me" {}
data "coder_workspace_owner" "me" {}
data "coder_parameter" "unrelated" {
name = "unrelated"
type = "list(string)"
default = jsonencode(["a", "b"])
}
%s
`
+1 -1
View File
@@ -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)
}
+87 -23
View File
@@ -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. Use
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}, "."))
@@ -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.
@@ -117,7 +119,7 @@ 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 {
@@ -125,54 +127,106 @@ func (p *Parser) WorkspaceTags(ctx context.Context) (map[string]string, error) {
}
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)
}
if len(tags) == 0 {
return map[string]string{}, nil
}
// To evaluate the expressions, we need to load the default values for
// variables and parameters.
varsDefaults, err := p.VariableDefaults(ctx)
if err != nil {
return nil, xerrors.Errorf("load variable defaults: %w", err)
}
paramsDefaults, err := p.CoderParameterDefaults(ctx, varsDefaults)
paramsDefaults, err := p.CoderParameterDefaults(ctx, varsDefaults, requiredVars)
if err != nil {
return nil, xerrors.Errorf("load parameter defaults: %w", err)
}
@@ -247,10 +301,10 @@ func WriteArchive(bs []byte, mimetype string, path string) error {
return nil
}
// VariableDefaults returns the default values for all variables passed to it.
// 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
// variables.
// required variables.
m := make(map[string]string)
for _, v := range p.module.Variables {
if v == nil {
@@ -268,7 +322,7 @@ func (p *Parser) VariableDefaults(ctx context.Context) (map[string]string, error
// 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) (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
@@ -281,12 +335,18 @@ 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}, "."))
if !strings.HasSuffix(dataResource.Pos.Filename, ".tf") {
continue
}
if !strings.HasSuffix(dataResource.Pos.Filename, ".tf") {
needle := strings.Join([]string{"data", dataResource.Type, dataResource.Name}, ".")
if dataResource.Type != "coder_parameter" {
skipped = append(skipped, needle)
continue
}
if _, found := names[needle]; !found {
skipped = append(skipped, needle)
continue
}
@@ -538,7 +598,11 @@ func interfaceToString(i interface{}) (string, error) {
return strconv.FormatFloat(v, 'f', -1, 64), nil
case bool:
return strconv.FormatBool(v), nil
default:
return "", xerrors.Errorf("unsupported type %T", v)
default: // just try to JSON-encode it.
var sb strings.Builder
if err := json.NewEncoder(&sb).Encode(i); err != nil {
return "", xerrors.Errorf("convert %T: %w", v, err)
}
return strings.TrimSpace(sb.String()), nil
}
}
+151 -27
View File
@@ -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: "",
@@ -49,8 +49,10 @@ func Test_WorkspaceTagDefaultsFromFile(t *testing.T) {
type = string
default = "us"
}
data "base" "ours" {
all = true
data "coder_parameter" "unrelated" {
name = "unrelated"
type = "list(string)"
default = jsonencode(["a", "b"])
}
data "coder_parameter" "az" {
name = "az"
@@ -71,8 +73,10 @@ func Test_WorkspaceTagDefaultsFromFile(t *testing.T) {
type = string
default = "us"
}
data "base" "ours" {
all = true
data "coder_parameter" "unrelated" {
name = "unrelated"
type = "list(string)"
default = jsonencode(["a", "b"])
}
data "coder_parameter" "az" {
name = "az"
@@ -94,8 +98,13 @@ func Test_WorkspaceTagDefaultsFromFile(t *testing.T) {
type = string
default = "us"
}
data "base" "ours" {
all = true
variable "unrelated" {
type = bool
}
data "coder_parameter" "unrelated" {
name = "unrelated"
type = "list(string)"
default = jsonencode(["a", "b"])
}
data "coder_parameter" "az" {
name = "az"
@@ -128,8 +137,10 @@ func Test_WorkspaceTagDefaultsFromFile(t *testing.T) {
type = string
default = "${""}${"a"}"
}
data "base" "ours" {
all = true
data "coder_parameter" "unrelated" {
name = "unrelated"
type = "list(string)"
default = jsonencode(["a", "b"])
}
data "coder_parameter" "az" {
name = "az"
@@ -158,8 +169,10 @@ func Test_WorkspaceTagDefaultsFromFile(t *testing.T) {
type = string
default = "us"
}
data "base" "ours" {
all = true
data "coder_parameter" "unrelated" {
name = "unrelated"
type = "list(string)"
default = jsonencode(["a", "b"])
}
data "coder_parameter" "az" {
type = string
@@ -195,8 +208,10 @@ func Test_WorkspaceTagDefaultsFromFile(t *testing.T) {
type = string
default = "eu"
}
data "base" "ours" {
all = true
data "coder_parameter" "unrelated" {
name = "unrelated"
type = "list(string)"
default = jsonencode(["a", "b"])
}
data "coder_parameter" "az" {
name = "az"
@@ -235,8 +250,10 @@ func Test_WorkspaceTagDefaultsFromFile(t *testing.T) {
type = string
default = "us"
}
data "base" "ours" {
all = true
data "coder_parameter" "unrelated" {
name = "unrelated"
type = "list(string)"
default = jsonencode(["a", "b"])
}
data "coder_parameter" "az" {
name = "az"
@@ -263,8 +280,10 @@ func Test_WorkspaceTagDefaultsFromFile(t *testing.T) {
type = string
default = "us"
}
data "base" "ours" {
all = true
data "coder_parameter" "unrelated" {
name = "unrelated"
type = "list(string)"
default = jsonencode(["a", "b"])
}
data "coder_parameter" "az" {
name = "az"
@@ -300,8 +319,10 @@ func Test_WorkspaceTagDefaultsFromFile(t *testing.T) {
variable "notregion" {
type = string
}
data "base" "ours" {
all = true
data "coder_parameter" "unrelated" {
name = "unrelated"
type = "list(string)"
default = jsonencode(["a", "b"])
}
data "coder_parameter" "az" {
name = "az"
@@ -332,8 +353,10 @@ func Test_WorkspaceTagDefaultsFromFile(t *testing.T) {
type = string
default = "us"
}
data "base" "ours" {
all = true
data "coder_parameter" "unrelated" {
name = "unrelated"
type = "list(string)"
default = jsonencode(["a", "b"])
}
data "coder_parameter" "az" {
name = "az"
@@ -368,8 +391,10 @@ func Test_WorkspaceTagDefaultsFromFile(t *testing.T) {
type = string
default = "us"
}
data "base" "ours" {
all = true
data "coder_parameter" "unrelated" {
name = "unrelated"
type = "list(string)"
default = jsonencode(["a", "b"])
}
data "coder_parameter" "az" {
name = "az"
@@ -402,8 +427,10 @@ func Test_WorkspaceTagDefaultsFromFile(t *testing.T) {
type = string
default = "region.us"
}
data "base" "ours" {
all = true
data "coder_parameter" "unrelated" {
name = "unrelated"
type = "list(string)"
default = jsonencode(["a", "b"])
}
data "coder_parameter" "az" {
name = "az"
@@ -422,6 +449,103 @@ func Test_WorkspaceTagDefaultsFromFile(t *testing.T) {
expectTags: nil,
expectError: `Function calls not allowed; Functions may not be called here.`,
},
{
name: "supported types",
files: map[string]string{
"main.tf": `
variable "stringvar" {
type = string
default = "a"
}
variable "numvar" {
type = number
default = 1
}
variable "boolvar" {
type = bool
default = true
}
variable "listvar" {
type = list(string)
default = ["a"]
}
variable "mapvar" {
type = map(string)
default = {"a": "b"}
}
data "coder_parameter" "stringparam" {
name = "stringparam"
type = "string"
default = "a"
}
data "coder_parameter" "numparam" {
name = "numparam"
type = "number"
default = 1
}
data "coder_parameter" "boolparam" {
name = "boolparam"
type = "bool"
default = true
}
data "coder_parameter" "listparam" {
name = "listparam"
type = "list(string)"
default = "[\"a\", \"b\"]"
}
data "coder_workspace_tags" "tags" {
tags = {
"stringvar" = var.stringvar
"numvar" = var.numvar
"boolvar" = var.boolvar
"listvar" = var.listvar
"mapvar" = var.mapvar
"stringparam" = data.coder_parameter.stringparam.value
"numparam" = data.coder_parameter.numparam.value
"boolparam" = data.coder_parameter.boolparam.value
"listparam" = data.coder_parameter.listparam.value
}
}`,
},
expectTags: map[string]string{
"stringvar": "a",
"numvar": "1",
"boolvar": "true",
"listvar": `["a"]`,
"mapvar": `{"a":"b"}`,
"stringparam": "a",
"numparam": "1",
"boolparam": "true",
"listparam": `["a", "b"]`,
},
expectError: ``,
},
{
name: "overlapping var name",
files: map[string]string{
`main.tf`: `
variable "a" {
type = string
default = "1"
}
variable "unused" {
type = map(string)
default = {"a" : "b"}
}
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) {
@@ -505,7 +629,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)
}
@@ -519,7 +643,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)
}