feat: enable Terraform template-wide variables by default (#8334)

mtojek · web-flow · commit 64687631aa81 · 2023-07-07T11:49:02.000+02:00
diff --git a/docs/templates/parameters.md b/docs/templates/parameters.md
@@ -248,12 +248,14 @@ data "coder_parameter" "cpu" {
 
 As a template improvement, the template author can consider making some of the new `coder_parameter` resources `mutable`.
 
-## Managed Terraform variables
+## Terraform template-wide variables
+
+> ⚠️ Flag `feature_use_managed_variables` is available until v0.25.0 (Jul 2023) release. After this release, template-wide Terraform variables will be enabled by default.
 
 As parameters are intended to be used only for workspace customization purposes, Terraform variables can be freely managed by the template author to build templates. Workspace users are not able to modify
 template variables.
 
-The template author can enable managed Terraform variables mode by specifying the following flag:
+The template author can enable Terraform template-wide variables mode by specifying the following flag:
 
 ```hcl
 provider "coder" {
diff --git a/examples/templates/aws-ecs-container/main.tf b/examples/templates/aws-ecs-container/main.tf
@@ -12,7 +12,6 @@ terraform {
 }
 
 provider "coder" {
-  feature_use_managed_variables = true
 }
 
 variable "ecs-cluster" {
diff --git a/examples/templates/do-linux/main.tf b/examples/templates/do-linux/main.tf
@@ -12,7 +12,6 @@ terraform {
 }
 
 provider "coder" {
-  feature_use_managed_variables = true
 }
 
 variable "step1_do_project_id" {
diff --git a/examples/templates/envbox/main.tf b/examples/templates/envbox/main.tf
@@ -38,7 +38,6 @@ variable "use_kubeconfig" {
 }
 
 provider "coder" {
-  feature_use_managed_variables = "true"
 }
 
 variable "namespace" {
diff --git a/examples/templates/fly-docker-image/main.tf b/examples/templates/fly-docker-image/main.tf
@@ -16,7 +16,6 @@ provider "fly" {
 }
 
 provider "coder" {
-  feature_use_managed_variables = true
 }
 
 resource "fly_app" "workspace" {
diff --git a/examples/templates/gcp-linux/main.tf b/examples/templates/gcp-linux/main.tf
@@ -12,7 +12,6 @@ terraform {
 }
 
 provider "coder" {
-  feature_use_managed_variables = true
 }
 
 variable "project_id" {
diff --git a/examples/templates/gcp-vm-container/main.tf b/examples/templates/gcp-vm-container/main.tf
@@ -12,7 +12,6 @@ terraform {
 }
 
 provider "coder" {
-  feature_use_managed_variables = true
 }
 
 variable "project_id" {
diff --git a/examples/templates/gcp-windows/main.tf b/examples/templates/gcp-windows/main.tf
@@ -12,7 +12,6 @@ terraform {
 }
 
 provider "coder" {
-  feature_use_managed_variables = true
 }
 
 variable "project_id" {
diff --git a/examples/templates/kubernetes/main.tf b/examples/templates/kubernetes/main.tf
@@ -12,7 +12,6 @@ terraform {
 }
 
 provider "coder" {
-  feature_use_managed_variables = true
 }
 
 variable "use_kubeconfig" {
diff --git a/provisioner/terraform/parse.go b/provisioner/terraform/parse.go
@@ -3,15 +3,10 @@ package terraform
 import (
 	"encoding/json"
 	"fmt"
-	"os"
-	"path"
 	"path/filepath"
 	"sort"
 	"strings"
 
-	"github.com/hashicorp/hcl/v2"
-	"github.com/hashicorp/hcl/v2/gohcl"
-	"github.com/hashicorp/hcl/v2/hclparse"
 	"github.com/hashicorp/terraform-config-inspect/tfconfig"
 	"github.com/mitchellh/go-wordwrap"
 	"golang.org/x/xerrors"
@@ -20,25 +15,6 @@ import (
 	"github.com/coder/coder/provisionersdk/proto"
 )
 
-const featureUseManagedVariables = "feature_use_managed_variables"
-
-var terraformWithFeaturesSchema = &hcl.BodySchema{
-	Blocks: []hcl.BlockHeaderSchema{
-		{
-			Type:       "provider",
-			LabelNames: []string{"type"},
-		},
-	},
-}
-
-var providerFeaturesConfigSchema = &hcl.BodySchema{
-	Attributes: []hcl.AttributeSchema{
-		{
-			Name: featureUseManagedVariables,
-		},
-	},
-}
-
 // Parse extracts Terraform variables from source-code.
 func (s *server) Parse(request *proto.Parse_Request, stream proto.DRPCProvisioner_ParseStream) error {
 	_, span := s.startTrace(stream.Context(), tracing.FuncName())
@@ -50,11 +26,6 @@ func (s *server) Parse(request *proto.Parse_Request, stream proto.DRPCProvisione
 		return xerrors.Errorf("load module: %s", formatDiagnostics(request.Directory, diags))
 	}
 
-	flags, flagsDiags := loadEnabledFeatures(request.Directory)
-	if flagsDiags.HasErrors() {
-		return xerrors.Errorf("load coder provider features: %s", formatDiagnostics(request.Directory, diags))
-	}
-
 	// Sort variables by (filename, line) to make the ordering consistent
 	variables := make([]*tfconfig.Variable, 0, len(module.Variables))
 	for _, v := range module.Variables {
@@ -66,17 +37,12 @@ func (s *server) Parse(request *proto.Parse_Request, stream proto.DRPCProvisione
 
 	var templateVariables []*proto.TemplateVariable
 
-	useManagedVariables := flags != nil && flags[featureUseManagedVariables]
-	if useManagedVariables {
-		for _, v := range variables {
-			mv, err := convertTerraformVariableToManagedVariable(v)
-			if err != nil {
-				return xerrors.Errorf("can't convert the Terraform variable to a managed one: %w", err)
-			}
-			templateVariables = append(templateVariables, mv)
+	for _, v := range variables {
+		mv, err := convertTerraformVariable(v)
+		if err != nil {
+			return xerrors.Errorf("can't convert the Terraform variable to a managed one: %w", err)
 		}
-	} else if len(variables) > 0 {
-		return xerrors.Errorf("legacy parameters are not supported anymore, use %q flag to enable managed Terraform variables", featureUseManagedVariables)
+		templateVariables = append(templateVariables, mv)
 	}
 	return stream.Send(&proto.Parse_Response{
 		Type: &proto.Parse_Response_Complete{
@@ -87,81 +53,8 @@ func (s *server) Parse(request *proto.Parse_Request, stream proto.DRPCProvisione
 	})
 }
 
-func loadEnabledFeatures(moduleDir string) (map[string]bool, hcl.Diagnostics) {
-	flags := map[string]bool{}
-	var diags hcl.Diagnostics
-
-	entries, err := os.ReadDir(moduleDir)
-	if err != nil {
-		diags = append(diags, &hcl.Diagnostic{
-			Severity: hcl.DiagError,
-			Summary:  "Failed to read module directory",
-			Detail:   fmt.Sprintf("Module directory %s does not exist or cannot be read.", moduleDir),
-		})
-		return flags, diags
-	}
-
-	var found bool
-	for _, entry := range entries {
-		if !strings.HasSuffix(entry.Name(), ".tf") && !strings.HasSuffix(entry.Name(), ".tf.json") {
-			continue
-		}
-
-		flags, found, diags = parseFeatures(path.Join(moduleDir, entry.Name()))
-		if found {
-			break
-		}
-	}
-	return flags, diags
-}
-
-func parseFeatures(hclFilepath string) (map[string]bool, bool, hcl.Diagnostics) {
-	flags := map[string]bool{}
-	var diags hcl.Diagnostics
-
-	_, err := os.Stat(hclFilepath)
-	if os.IsNotExist(err) {
-		return flags, false, diags
-	} else if err != nil {
-		diags = append(diags, &hcl.Diagnostic{
-			Severity: hcl.DiagError,
-			Summary:  fmt.Sprintf("Failed to open %q file", hclFilepath),
-		})
-		return flags, false, diags
-	}
-
-	parser := hclparse.NewParser()
-	var parsedHCL *hcl.File
-	if strings.HasSuffix(hclFilepath, ".tf.json") {
-		parsedHCL, diags = parser.ParseJSONFile(hclFilepath)
-	} else {
-		parsedHCL, diags = parser.ParseHCLFile(hclFilepath)
-	}
-	if diags.HasErrors() {
-		return flags, false, diags
-	}
-
-	var found bool
-	content, _ := parsedHCL.Body.Content(terraformWithFeaturesSchema)
-	for _, block := range content.Blocks {
-		if block.Type == "provider" && block.Labels[0] == "coder" {
-			content, _, partialDiags := block.Body.PartialContent(providerFeaturesConfigSchema)
-			diags = append(diags, partialDiags...)
-			if attr, defined := content.Attributes[featureUseManagedVariables]; defined {
-				found = true
-
-				var useManagedVariables bool
-				partialDiags := gohcl.DecodeExpression(attr.Expr, nil, &useManagedVariables)
-				diags = append(diags, partialDiags...)
-				flags[featureUseManagedVariables] = useManagedVariables
-			}
-		}
-	}
-	return flags, found, diags
-}
-
-// Converts a Terraform variable to a managed variable.
-func convertTerraformVariableToManagedVariable(variable *tfconfig.Variable) (*proto.TemplateVariable, error) {
+// Converts a Terraform variable to a template-wide variable, processed by Coder.
+func convertTerraformVariable(variable *tfconfig.Variable) (*proto.TemplateVariable, error) {
 	var defaultData string
 	if variable.Default != nil {
 		var valid bool
diff --git a/provisioner/terraform/parse_test.go b/provisioner/terraform/parse_test.go
@@ -31,9 +31,7 @@ func TestParse(t *testing.T) {
 			Files: map[string]string{
 				"main.tf": `variable "A" {
 				description = "Testing!"
-			}
-
-			provider "coder" { feature_use_managed_variables = "true" }`,
+			}`,
 			},
 			Response: &proto.Parse_Response{
 				Type: &proto.Parse_Response_Complete{
@@ -54,9 +52,7 @@ func TestParse(t *testing.T) {
 			Files: map[string]string{
 				"main.tf": `variable "A" {
 				default = "wow"
-			}
-
-			provider "coder" { feature_use_managed_variables = "true" }`,
+			}`,
 			},
 			Response: &proto.Parse_Response{
 				Type: &proto.Parse_Response_Complete{
@@ -78,9 +74,7 @@ func TestParse(t *testing.T) {
 				validation {
 					condition = var.A == "value"
 				}
-			}
-
-			provider "coder" { feature_use_managed_variables = "true" }`,
+			}`,
 			},
 			Response: &proto.Parse_Response{
 				Type: &proto.Parse_Response_Complete{
@@ -106,9 +100,7 @@ func TestParse(t *testing.T) {
 			Name: "multiple-variables",
 			Files: map[string]string{
 				"main1.tf": `variable "foo" { }
-				variable "bar" { }
-
-				provider "coder" { feature_use_managed_variables = "true" }`,
+				variable "bar" { }`,
 				"main2.tf": `variable "baz" { }
 				variable "quux" { }`,
 			},
@@ -138,17 +130,13 @@ func TestParse(t *testing.T) {
 			},
 		},
 		{
-			Name: "enable-managed-variables-with-default-bool",
+			Name: "template-variables-with-default-bool",
 			Files: map[string]string{
 				"main.tf": `variable "A" {
 				description = "Testing!"
 				type = 	bool
 				default = true
 				sensitive = true
-			}
-
-			provider "coder" {
-				feature_use_managed_variables = true
 			}`,
 			},
 			Response: &proto.Parse_Response{
@@ -169,17 +157,13 @@ func TestParse(t *testing.T) {
 			},
 		},
 		{
-			Name: "enable-managed-variables-with-default-string",
+			Name: "template-variables-with-default-string",
 			Files: map[string]string{
 				"main.tf": `variable "A" {
 				description = "Testing!"
 				type = 	string
 				default = "abc"
 				sensitive = true
-			}
-
-			provider "coder" {
-				feature_use_managed_variables = true
 			}`,
 			},
 			Response: &proto.Parse_Response{
@@ -200,17 +184,13 @@ func TestParse(t *testing.T) {
 			},
 		},
 		{
-			Name: "enable-managed-variables-with-default-empty-string",
+			Name: "template-variables-with-default-empty-string",
 			Files: map[string]string{
 				"main.tf": `variable "A" {
 				description = "Testing!"
 				type = 	string
 				default = ""
 				sensitive = true
-			}
-
-			provider "coder" {
-				feature_use_managed_variables = true
 			}`,
 			},
 			Response: &proto.Parse_Response{
@@ -231,16 +211,12 @@ func TestParse(t *testing.T) {
 			},
 		},
 		{
-			Name: "enable-managed-variables-without-default",
+			Name: "template-variables-without-default",
 			Files: map[string]string{
 				"main2.tf": `variable "A" {
 				description = "Testing!"
 				type = string
 				sensitive = true
-			}
-
-			provider "coder" {
-				feature_use_managed_variables = true
 			}`,
 			},
 			Response: &proto.Parse_Response{

Original file line number	Diff line number	Diff line change
`@@ -12,7 +12,6 @@ terraform {`
`12`	`12`	`}`
`13`	`13`
`14`	`14`	`provider "coder" {`
`15`		`- feature_use_managed_variables = true`
`16`	`15`	`}`
`17`	`16`
`18`	`17`	`variable "ecs-cluster" {`
Original file line number	Diff line number	Diff line change
`@@ -38,7 +38,6 @@ variable "use_kubeconfig" {`
`38`	`38`	`}`
`39`	`39`
`40`	`40`	`provider "coder" {`
`41`		`- feature_use_managed_variables = "true"`
`42`	`41`	`}`
`43`	`42`
`44`	`43`	`variable "namespace" {`
Original file line number	Diff line number	Diff line change
`@@ -16,7 +16,6 @@ provider "fly" {`
`16`	`16`	`}`
`17`	`17`
`18`	`18`	`provider "coder" {`
`19`		`- feature_use_managed_variables = true`
`20`	`19`	`}`
`21`	`20`
`22`	`21`	`resource "fly_app" "workspace" {`