From d31af62318148744949c88650a363ac1860f1383 Mon Sep 17 00:00:00 2001 From: Daniel Banck Date: Wed, 6 Apr 2022 14:30:03 +0200 Subject: [PATCH] Make use of the Targets body schema field for modules (#101) * update hcl-lang * Add list of filenames to module meta earlydecoder now collects a sorted list of filenames for each module * Make use of the Targets body schema field for modules The target will point to the first file of a module * target main.tf in module if file exists --- earlydecoder/decoder.go | 8 ++- earlydecoder/decoder_test.go | 38 ++++++++++-- go.mod | 2 +- go.sum | 6 +- module/meta.go | 3 +- schema/module_schema.go | 32 +++++++++++ schema/module_schema_test.go | 108 +++++++++++++++++++++++++++++++++++ 7 files changed, 184 insertions(+), 13 deletions(-) diff --git a/earlydecoder/decoder.go b/earlydecoder/decoder.go index 7cf95dbd..f2bcfd45 100644 --- a/earlydecoder/decoder.go +++ b/earlydecoder/decoder.go @@ -2,6 +2,7 @@ package earlydecoder import ( "fmt" + "sort" "github.com/hashicorp/go-version" "github.com/hashicorp/hcl/v2" @@ -11,13 +12,17 @@ import ( func LoadModule(path string, files map[string]*hcl.File) (*module.Meta, hcl.Diagnostics) { var diags hcl.Diagnostics + filenames := make([]string, 0) mod := newDecodedModule() - for _, f := range files { + for filename, f := range files { + filenames = append(filenames, filename) fDiags := loadModuleFromFile(f, mod) diags = append(diags, fDiags...) } + sort.Strings(filenames) + var coreRequirements version.Constraints for _, rc := range mod.RequiredCore { c, err := version.NewConstraint(rc) @@ -166,5 +171,6 @@ func LoadModule(path string, files map[string]*hcl.File) (*module.Meta, hcl.Diag CoreRequirements: coreRequirements, Variables: variables, Outputs: outputs, + Filenames: filenames, }, diags } diff --git a/earlydecoder/decoder_test.go b/earlydecoder/decoder_test.go index 8ae913e0..616e87a3 100644 --- a/earlydecoder/decoder_test.go +++ b/earlydecoder/decoder_test.go @@ -40,6 +40,7 @@ func TestLoadModule(t *testing.T) { ProviderRequirements: map[tfaddr.Provider]version.Constraints{}, Variables: map[string]module.Variable{}, Outputs: map[string]module.Output{}, + Filenames: []string{"test.tf"}, }, nil, }, @@ -56,6 +57,7 @@ terraform { ProviderRequirements: map[tfaddr.Provider]version.Constraints{}, Variables: map[string]module.Variable{}, Outputs: map[string]module.Output{}, + Filenames: []string{"test.tf"}, }, nil, }, @@ -95,6 +97,7 @@ provider "grafana" { }, Variables: map[string]module.Variable{}, Outputs: map[string]module.Output{}, + Filenames: []string{"test.tf"}, }, nil, }, @@ -134,6 +137,7 @@ provider "grafana" { }, Variables: map[string]module.Variable{}, Outputs: map[string]module.Output{}, + Filenames: []string{"test.tf"}, }, nil, }, @@ -177,6 +181,7 @@ provider "grafana" { }, Variables: map[string]module.Variable{}, Outputs: map[string]module.Output{}, + Filenames: []string{"test.tf"}, }, nil, }, @@ -250,6 +255,7 @@ provider "grafana" { }, Variables: map[string]module.Variable{}, Outputs: map[string]module.Output{}, + Filenames: []string{"test.tf"}, }, nil, }, @@ -313,6 +319,7 @@ resource "google_storage_bucket" "bucket" { }, Variables: map[string]module.Variable{}, Outputs: map[string]module.Output{}, + Filenames: []string{"test.tf"}, }, nil, }, @@ -377,6 +384,7 @@ resource "google_storage_bucket" "bucket" { }, Variables: map[string]module.Variable{}, Outputs: map[string]module.Output{}, + Filenames: []string{"test.tf"}, }, nil, }, @@ -433,6 +441,7 @@ provider "aws" { }, Variables: map[string]module.Variable{}, Outputs: map[string]module.Output{}, + Filenames: []string{"test.tf"}, }, nil, }, @@ -495,6 +504,7 @@ provider "aws" { }, Variables: map[string]module.Variable{}, Outputs: map[string]module.Output{}, + Filenames: []string{"test.tf"}, }, nil, }, @@ -532,6 +542,7 @@ resource "google_something" "test" { }, Variables: map[string]module.Variable{}, Outputs: map[string]module.Output{}, + Filenames: []string{"test.tf"}, }, nil, }, @@ -555,6 +566,7 @@ variable "" { ProviderRequirements: map[tfaddr.Provider]version.Constraints{}, Variables: map[string]module.Variable{}, Outputs: map[string]module.Output{}, + Filenames: []string{"test.tf"}, }, nil, }, @@ -569,6 +581,7 @@ variable { ProviderRequirements: map[tfaddr.Provider]version.Constraints{}, Variables: map[string]module.Variable{}, Outputs: map[string]module.Output{}, + Filenames: []string{"test.tf"}, }, hcl.Diagnostics{ &hcl.Diagnostic{ @@ -615,6 +628,7 @@ variable "one" "two" { ProviderRequirements: map[tfaddr.Provider]version.Constraints{}, Variables: map[string]module.Variable{}, Outputs: map[string]module.Output{}, + Filenames: []string{"test.tf"}, }, hcl.Diagnostics{ &hcl.Diagnostic{ @@ -664,7 +678,8 @@ variable "name" { Type: cty.DynamicPseudoType, }, }, - Outputs: map[string]module.Output{}, + Outputs: map[string]module.Output{}, + Filenames: []string{"test.tf"}, }, nil, }, @@ -683,7 +698,8 @@ variable "name" { Type: cty.String, }, }, - Outputs: map[string]module.Output{}, + Outputs: map[string]module.Output{}, + Filenames: []string{"test.tf"}, }, nil, }, @@ -703,7 +719,8 @@ variable "name" { Description: "description", }, }, - Outputs: map[string]module.Output{}, + Outputs: map[string]module.Output{}, + Filenames: []string{"test.tf"}, }, nil, }, @@ -723,7 +740,8 @@ variable "name" { IsSensitive: true, }, }, - Outputs: map[string]module.Output{}, + Outputs: map[string]module.Output{}, + Filenames: []string{"test.tf"}, }, nil, }, @@ -746,7 +764,8 @@ variable "name" { IsSensitive: true, }, }, - Outputs: map[string]module.Output{}, + Outputs: map[string]module.Output{}, + Filenames: []string{"test.tf"}, }, nil, }, @@ -766,7 +785,8 @@ variable "name" { DefaultValue: cty.EmptyObjectVal, }, }, - Outputs: map[string]module.Output{}, + Outputs: map[string]module.Output{}, + Filenames: []string{"test.tf"}, }, nil, }, @@ -784,6 +804,7 @@ output "name" { Outputs: map[string]module.Output{ "name": {Value: cty.NilVal}, }, + Filenames: []string{"test.tf"}, }, nil, }, @@ -809,6 +830,7 @@ terraform { ProviderRequirements: map[tfaddr.Provider]version.Constraints{}, Variables: map[string]module.Variable{}, Outputs: map[string]module.Output{}, + Filenames: []string{"test.tf"}, }, nil, }, @@ -830,6 +852,7 @@ terraform { ProviderRequirements: map[tfaddr.Provider]version.Constraints{}, Variables: map[string]module.Variable{}, Outputs: map[string]module.Output{}, + Filenames: []string{"test.tf"}, }, nil, }, @@ -849,6 +872,7 @@ terraform { ProviderRequirements: map[tfaddr.Provider]version.Constraints{}, Variables: map[string]module.Variable{}, Outputs: map[string]module.Output{}, + Filenames: []string{"test.tf"}, }, nil, }, @@ -870,6 +894,7 @@ terraform { ProviderRequirements: map[tfaddr.Provider]version.Constraints{}, Variables: map[string]module.Variable{}, Outputs: map[string]module.Output{}, + Filenames: []string{"test.tf"}, }, nil, }, @@ -896,6 +921,7 @@ terraform { ProviderRequirements: map[tfaddr.Provider]version.Constraints{}, Variables: map[string]module.Variable{}, Outputs: map[string]module.Output{}, + Filenames: []string{"test.tf"}, }, nil, }, diff --git a/go.mod b/go.mod index f4a9698d..56b5c3c5 100644 --- a/go.mod +++ b/go.mod @@ -5,7 +5,7 @@ go 1.14 require ( github.com/google/go-cmp v0.5.7 github.com/hashicorp/go-version v1.4.0 - github.com/hashicorp/hcl-lang v0.0.0-20220322100058-bdead81fb0c9 + github.com/hashicorp/hcl-lang v0.0.0-20220406121211-c20527a75592 github.com/hashicorp/hcl/v2 v2.11.1 github.com/hashicorp/terraform-json v0.13.0 github.com/hashicorp/terraform-registry-address v0.0.0-20210412075316-9b2996cce896 diff --git a/go.sum b/go.sum index 3f33f948..75173adc 100644 --- a/go.sum +++ b/go.sum @@ -29,10 +29,8 @@ github.com/hashicorp/go-version v1.2.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09 github.com/hashicorp/go-version v1.3.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA= github.com/hashicorp/go-version v1.4.0 h1:aAQzgqIrRKRa7w75CKpbBxYsmUoPjzVm1W59ca1L0J4= github.com/hashicorp/go-version v1.4.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA= -github.com/hashicorp/hcl-lang v0.0.0-20220322093838-7d146ea70bc9 h1:KOUGusoj5MubxgHn5VqDvZfTBHZKV7h0b216xyfqeuo= -github.com/hashicorp/hcl-lang v0.0.0-20220322093838-7d146ea70bc9/go.mod h1:oQgcOV8OizFyZfZh3FbQSsQtvtTv8hD23MLAxfn3E+E= -github.com/hashicorp/hcl-lang v0.0.0-20220322100058-bdead81fb0c9 h1:Oj1j/Fl1pFz+4+Th5/y2iUbKfF+UguMkqlF9NMD/s+Q= -github.com/hashicorp/hcl-lang v0.0.0-20220322100058-bdead81fb0c9/go.mod h1:oQgcOV8OizFyZfZh3FbQSsQtvtTv8hD23MLAxfn3E+E= +github.com/hashicorp/hcl-lang v0.0.0-20220406121211-c20527a75592 h1:pSTtkCAbU+SLxw6J59ihqFDX5lJ9xR/fhqaOng1kQXY= +github.com/hashicorp/hcl-lang v0.0.0-20220406121211-c20527a75592/go.mod h1:oQgcOV8OizFyZfZh3FbQSsQtvtTv8hD23MLAxfn3E+E= github.com/hashicorp/hcl/v2 v2.11.1 h1:yTyWcXcm9XB0TEkyU/JCRU6rYy4K+mgLtzn2wlrJbcc= github.com/hashicorp/hcl/v2 v2.11.1/go.mod h1:FwWsfWEjyV/CMj8s/gqAuiviY72rJ1/oayI9WftqcKg= github.com/hashicorp/terraform-json v0.13.0 h1:Li9L+lKD1FO5RVFRM1mMMIBDoUHslOniyEi5CM+FWGY= diff --git a/module/meta.go b/module/meta.go index 2becf258..d26e02e1 100644 --- a/module/meta.go +++ b/module/meta.go @@ -7,7 +7,8 @@ import ( ) type Meta struct { - Path string + Path string + Filenames []string Backend *Backend ProviderReferences map[ProviderRef]tfaddr.Provider diff --git a/schema/module_schema.go b/schema/module_schema.go index 2d8b127f..59f9c70a 100644 --- a/schema/module_schema.go +++ b/schema/module_schema.go @@ -5,6 +5,7 @@ import ( "github.com/hashicorp/hcl-lang/lang" "github.com/hashicorp/hcl-lang/schema" + "github.com/hashicorp/hcl/v2" "github.com/hashicorp/terraform-schema/internal/schema/refscope" "github.com/hashicorp/terraform-schema/module" "github.com/zclconf/go-cty/cty" @@ -90,5 +91,36 @@ func schemaForDependentModuleBlock(localName string, modMeta *module.Meta) (*sch NestedTargetables: targetableOutputs, }) + if len(modMeta.Filenames) > 0 { + filename := modMeta.Filenames[0] + + // Prioritize main.tf based on best practices as documented at + // https://learn.hashicorp.com/tutorials/terraform/module-create + if sliceContains(modMeta.Filenames, "main.tf") { + filename = "main.tf" + } + + bodySchema.Targets = &schema.Target{ + Path: lang.Path{ + Path: modMeta.Path, + LanguageID: "terraform", + }, + Range: hcl.Range{ + Filename: filename, + Start: hcl.InitialPos, + End: hcl.InitialPos, + }, + } + } + return bodySchema, nil } + +func sliceContains(slice []string, value string) bool { + for _, val := range slice { + if val == value { + return true + } + } + return false +} diff --git a/schema/module_schema_test.go b/schema/module_schema_test.go index 63c7acb2..1d3f1311 100644 --- a/schema/module_schema_test.go +++ b/schema/module_schema_test.go @@ -6,6 +6,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/hashicorp/hcl-lang/lang" "github.com/hashicorp/hcl-lang/schema" + "github.com/hashicorp/hcl/v2" "github.com/hashicorp/terraform-schema/internal/schema/refscope" "github.com/hashicorp/terraform-schema/module" "github.com/zclconf/go-cty-debug/ctydebug" @@ -166,3 +167,110 @@ func TestSchemaForDependentModuleBlock_basic(t *testing.T) { t.Fatalf("schema mismatch: %s", diff) } } + +func TestSchemaForDependentModuleBlock_Target(t *testing.T) { + type testCase struct { + name string + meta *module.Meta + expectedSchema *schema.BodySchema + } + + testCases := []testCase{ + { + "no target", + &module.Meta{ + Path: "./local", + Variables: map[string]module.Variable{}, + Outputs: map[string]module.Output{}, + Filenames: nil, + }, + &schema.BodySchema{ + Attributes: map[string]*schema.AttributeSchema{}, + TargetableAs: []*schema.Targetable{ + { + Address: lang.Address{ + lang.RootStep{Name: "module"}, + lang.AttrStep{Name: "refname"}, + }, + ScopeId: refscope.ModuleScope, + AsType: cty.Object(map[string]cty.Type{}), + NestedTargetables: []*schema.Targetable{}, + }, + }, + Targets: nil, + }, + }, + { + "without main.tf", + &module.Meta{ + Path: "./local", + Variables: map[string]module.Variable{}, + Outputs: map[string]module.Output{}, + Filenames: []string{"a_file.tf", "b_file.tf"}, + }, + &schema.BodySchema{ + Attributes: map[string]*schema.AttributeSchema{}, + TargetableAs: []*schema.Targetable{ + { + Address: lang.Address{ + lang.RootStep{Name: "module"}, + lang.AttrStep{Name: "refname"}, + }, + ScopeId: refscope.ModuleScope, + AsType: cty.Object(map[string]cty.Type{}), + NestedTargetables: []*schema.Targetable{}, + }, + }, + Targets: &schema.Target{ + Path: lang.Path{Path: "./local", LanguageID: "terraform"}, + Range: hcl.Range{ + Filename: "a_file.tf", + Start: hcl.InitialPos, + End: hcl.InitialPos, + }, + }, + }, + }, + { + "with main.tf", + &module.Meta{ + Path: "./local", + Variables: map[string]module.Variable{}, + Outputs: map[string]module.Output{}, + Filenames: []string{"a_file.tf", "main.tf"}, + }, + &schema.BodySchema{ + Attributes: map[string]*schema.AttributeSchema{}, + TargetableAs: []*schema.Targetable{ + { + Address: lang.Address{ + lang.RootStep{Name: "module"}, + lang.AttrStep{Name: "refname"}, + }, + ScopeId: refscope.ModuleScope, + AsType: cty.Object(map[string]cty.Type{}), + NestedTargetables: []*schema.Targetable{}, + }, + }, + Targets: &schema.Target{ + Path: lang.Path{Path: "./local", LanguageID: "terraform"}, + Range: hcl.Range{ + Filename: "main.tf", + Start: hcl.InitialPos, + End: hcl.InitialPos, + }, + }, + }, + }, + } + + for _, tc := range testCases { + depSchema, err := schemaForDependentModuleBlock("refname", tc.meta) + if err != nil { + t.Fatal(err) + } + if diff := cmp.Diff(tc.expectedSchema, depSchema, ctydebug.CmpOptions); diff != "" { + t.Fatalf("schema mismatch: %s", diff) + } + } +}