From ce2a0091f0e2ab8ee6f9803010b247bafdc9ca66 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Thu, 6 Jul 2023 11:14:45 +0200 Subject: [PATCH 1/3] fixes --- docs/templates/parameters.md | 6 +- provisioner/terraform/parse.go | 107 ++-------------------------- provisioner/terraform/parse_test.go | 40 +++-------- 3 files changed, 17 insertions(+), 136 deletions(-) diff --git a/docs/templates/parameters.md b/docs/templates/parameters.md index 5dbf6a72614b3..321ee5f79ff9f 100644 --- 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/provisioner/terraform/parse.go b/provisioner/terraform/parse.go index 237954e4447db..3460077e8d6b9 100644 --- a/provisioner/terraform/parse.go +++ b/provisioner/terraform/parse.go @@ -3,15 +3,11 @@ 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,8 +16,6 @@ import ( "github.com/coder/coder/provisionersdk/proto" ) -const featureUseManagedVariables = "feature_use_managed_variables" - var terraformWithFeaturesSchema = &hcl.BodySchema{ Blocks: []hcl.BlockHeaderSchema{ { @@ -31,14 +25,6 @@ var terraformWithFeaturesSchema = &hcl.BodySchema{ }, } -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 +36,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 +47,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 := convertTerraformVariableToManagedVariable(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,79 +63,6 @@ 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) { var defaultData string diff --git a/provisioner/terraform/parse_test.go b/provisioner/terraform/parse_test.go index 9e2767216806c..e82bcba470857 100644 --- 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{ From cbb59bd4bd5e5244ec2a82132afb733bab447c27 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Thu, 6 Jul 2023 11:18:29 +0200 Subject: [PATCH 2/3] Clean templates --- examples/templates/aws-ecs-container/main.tf | 1 - examples/templates/do-linux/main.tf | 1 - examples/templates/envbox/main.tf | 1 - examples/templates/fly-docker-image/main.tf | 1 - examples/templates/gcp-linux/main.tf | 1 - examples/templates/gcp-vm-container/main.tf | 1 - examples/templates/gcp-windows/main.tf | 1 - examples/templates/kubernetes/main.tf | 1 - 8 files changed, 8 deletions(-) diff --git a/examples/templates/aws-ecs-container/main.tf b/examples/templates/aws-ecs-container/main.tf index f07c0925ec018..f7f2249ee8807 100644 --- 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 index 798e5ca106f02..3080c9228b368 100644 --- 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 index 1e4e49b5e280c..f5460c622b4eb 100644 --- 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 index 539bb01530e84..2b4fdf850d8d6 100644 --- a/examples/templates/fly-docker-image/main.tf +++ b/examples/templates/fly-docker-image/main.tf @@ -19,7 +19,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 index a6d6f86eb0ede..e4bba1bcbe58f 100644 --- 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 index 5a9fc311d9dd8..45a76c63a4431 100644 --- 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 index ca7c4ff5f73a9..16148ef407e1a 100644 --- 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 index 074565b89b5c3..5d414032382c9 100644 --- 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" { From ba8a748a01e480a9e29ce8310939136d4a67cc78 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Thu, 6 Jul 2023 11:26:16 +0200 Subject: [PATCH 3/3] linting --- provisioner/terraform/parse.go | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/provisioner/terraform/parse.go b/provisioner/terraform/parse.go index 3460077e8d6b9..dc57cde28a591 100644 --- a/provisioner/terraform/parse.go +++ b/provisioner/terraform/parse.go @@ -7,7 +7,6 @@ import ( "sort" "strings" - "github.com/hashicorp/hcl/v2" "github.com/hashicorp/terraform-config-inspect/tfconfig" "github.com/mitchellh/go-wordwrap" "golang.org/x/xerrors" @@ -16,15 +15,6 @@ import ( "github.com/coder/coder/provisionersdk/proto" ) -var terraformWithFeaturesSchema = &hcl.BodySchema{ - Blocks: []hcl.BlockHeaderSchema{ - { - Type: "provider", - LabelNames: []string{"type"}, - }, - }, -} - // 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()) @@ -48,7 +38,7 @@ func (s *server) Parse(request *proto.Parse_Request, stream proto.DRPCProvisione var templateVariables []*proto.TemplateVariable for _, v := range variables { - mv, err := convertTerraformVariableToManagedVariable(v) + mv, err := convertTerraformVariable(v) if err != nil { return xerrors.Errorf("can't convert the Terraform variable to a managed one: %w", err) } @@ -63,8 +53,8 @@ func (s *server) Parse(request *proto.Parse_Request, stream proto.DRPCProvisione }) } -// 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