From daaa726c9ef975085f7f770951b3c24782011b5f Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Mon, 25 Sep 2023 10:59:44 +0100 Subject: [PATCH] Introduce validation of `*.tfvars` files (#1413) * Introduce validation of tfvars files * clarify existing job name/type * add tests --- internal/decoder/decoder.go | 1 + internal/decoder/validators.go | 5 + internal/indexer/document_change.go | 24 +++- internal/indexer/document_open.go | 21 ++++ internal/state/module.go | 2 +- internal/terraform/module/module_ops.go | 69 ++++++++++- internal/terraform/module/module_ops_test.go | 117 +++++++++++++++++- .../module/operation/op_type_string.go | 11 +- .../terraform/module/operation/operation.go | 3 +- .../testdata/invalid-tfvars/foo.auto.tfvars | 1 + .../testdata/invalid-tfvars/terraform.tfvars | 2 + .../testdata/invalid-tfvars/variables.tf | 1 + 12 files changed, 241 insertions(+), 16 deletions(-) create mode 100644 internal/terraform/module/testdata/invalid-tfvars/foo.auto.tfvars create mode 100644 internal/terraform/module/testdata/invalid-tfvars/terraform.tfvars create mode 100644 internal/terraform/module/testdata/invalid-tfvars/variables.tf diff --git a/internal/decoder/decoder.go b/internal/decoder/decoder.go index 318cd488e..ed1a20c3b 100644 --- a/internal/decoder/decoder.go +++ b/internal/decoder/decoder.go @@ -64,6 +64,7 @@ func varsPathContext(mod *state.Module) (*decoder.PathContext, error) { ReferenceOrigins: make(reference.Origins, 0), ReferenceTargets: make(reference.Targets, 0), Files: make(map[string]*hcl.File), + Validators: varsValidators, } for _, origin := range mod.VarsRefOrigins { diff --git a/internal/decoder/validators.go b/internal/decoder/validators.go index 7bb3bf7b3..0b1473ba3 100644 --- a/internal/decoder/validators.go +++ b/internal/decoder/validators.go @@ -17,3 +17,8 @@ var moduleValidators = []validator.Validator{ validator.UnexpectedAttribute{}, validator.UnexpectedBlock{}, } + +var varsValidators = []validator.Validator{ + validator.UnexpectedAttribute{}, + validator.UnexpectedBlock{}, +} diff --git a/internal/indexer/document_change.go b/internal/indexer/document_change.go index 2dc243fff..414717963 100644 --- a/internal/indexer/document_change.go +++ b/internal/indexer/document_change.go @@ -49,6 +49,26 @@ func (idx *Indexer) DocumentChanged(ctx context.Context, modHandle document.DirH } ids = append(ids, parseVarsId) + validationOptions, err := lsctx.ValidationOptions(ctx) + if err != nil { + return ids, err + } + + if validationOptions.EarlyValidation { + _, err = idx.jobStore.EnqueueJob(ctx, job.Job{ + Dir: modHandle, + Func: func(ctx context.Context) error { + return module.SchemaVariablesValidation(ctx, idx.modStore, idx.schemaStore, modHandle.Path()) + }, + Type: op.OpTypeSchemaVarsValidation.String(), + DependsOn: append(modIds, parseVarsId), + IgnoreState: true, + }) + if err != nil { + return ids, err + } + } + varsRefsId, err := idx.jobStore.EnqueueJob(ctx, job.Job{ Dir: modHandle, Func: func(ctx context.Context) error { @@ -131,9 +151,9 @@ func (idx *Indexer) decodeModule(ctx context.Context, modHandle document.DirHand _, err = idx.jobStore.EnqueueJob(ctx, job.Job{ Dir: modHandle, Func: func(ctx context.Context) error { - return module.SchemaValidation(ctx, idx.modStore, idx.schemaStore, modHandle.Path()) + return module.SchemaModuleValidation(ctx, idx.modStore, idx.schemaStore, modHandle.Path()) }, - Type: op.OpTypeSchemaValidation.String(), + Type: op.OpTypeSchemaModuleValidation.String(), DependsOn: job.IDs{metaId}, IgnoreState: ignoreState, }) diff --git a/internal/indexer/document_open.go b/internal/indexer/document_open.go index 159c2ddec..043840d05 100644 --- a/internal/indexer/document_open.go +++ b/internal/indexer/document_open.go @@ -7,6 +7,7 @@ import ( "context" "github.com/hashicorp/go-multierror" + lsctx "github.com/hashicorp/terraform-ls/internal/context" "github.com/hashicorp/terraform-ls/internal/document" "github.com/hashicorp/terraform-ls/internal/job" "github.com/hashicorp/terraform-ls/internal/terraform/exec" @@ -72,6 +73,26 @@ func (idx *Indexer) DocumentOpened(ctx context.Context, modHandle document.DirHa } ids = append(ids, parseVarsId) + validationOptions, err := lsctx.ValidationOptions(ctx) + if err != nil { + return ids, err + } + + if validationOptions.EarlyValidation { + _, err = idx.jobStore.EnqueueJob(ctx, job.Job{ + Dir: modHandle, + Func: func(ctx context.Context) error { + return module.SchemaVariablesValidation(ctx, idx.modStore, idx.schemaStore, modHandle.Path()) + }, + Type: op.OpTypeSchemaVarsValidation.String(), + DependsOn: append(modIds, parseVarsId), + IgnoreState: true, + }) + if err != nil { + return ids, err + } + } + varsRefsId, err := idx.jobStore.EnqueueJob(ctx, job.Job{ Dir: modHandle, Func: func(ctx context.Context) error { diff --git a/internal/state/module.go b/internal/state/module.go index b06765b36..10838bc06 100644 --- a/internal/state/module.go +++ b/internal/state/module.go @@ -988,7 +988,7 @@ func (s *ModuleStore) SetModuleDiagnosticsState(path string, source ast.Diagnost func (s *ModuleStore) UpdateVarsDiagnostics(path string, source ast.DiagnosticSource, diags ast.VarsDiags) error { txn := s.db.Txn(true) txn.Defer(func() { - s.SetVarsDiagnosticsState(path, ast.HCLParsingSource, op.OpStateLoaded) + s.SetVarsDiagnosticsState(path, source, op.OpStateLoaded) }) defer txn.Abort() diff --git a/internal/terraform/module/module_ops.go b/internal/terraform/module/module_ops.go index 866e1263d..210205327 100644 --- a/internal/terraform/module/module_ops.go +++ b/internal/terraform/module/module_ops.go @@ -668,7 +668,7 @@ func DecodeVarsReferences(ctx context.Context, modStore *state.ModuleStore, sche return rErr } -func SchemaValidation(ctx context.Context, modStore *state.ModuleStore, schemaReader state.SchemaReader, modPath string) error { +func SchemaModuleValidation(ctx context.Context, modStore *state.ModuleStore, schemaReader state.SchemaReader, modPath string) error { mod, err := modStore.ModuleByPath(modPath) if err != nil { return err @@ -688,6 +688,7 @@ func SchemaValidation(ctx context.Context, modStore *state.ModuleStore, schemaRe ModuleReader: modStore, SchemaReader: schemaReader, }) + d.SetContext(idecoder.DecoderContext(ctx)) moduleDecoder, err := d.Path(lang.Path{ @@ -700,8 +701,7 @@ func SchemaValidation(ctx context.Context, modStore *state.ModuleStore, schemaRe var rErr error rpcContext := lsctx.RPCContext(ctx) - isSingleFileChange := rpcContext.Method == "textDocument/didChange" - if isSingleFileChange { + if rpcContext.Method == "textDocument/didChange" && lsctx.IsLanguageId(ctx, ilsp.Terraform.String()) { filename := path.Base(rpcContext.URI) // We only revalidate a single file that changed var fileDiags hcl.Diagnostics @@ -731,6 +731,69 @@ func SchemaValidation(ctx context.Context, modStore *state.ModuleStore, schemaRe return rErr } +func SchemaVariablesValidation(ctx context.Context, modStore *state.ModuleStore, schemaReader state.SchemaReader, modPath string) error { + mod, err := modStore.ModuleByPath(modPath) + if err != nil { + return err + } + + // Avoid validation if it is already in progress or already finished + if mod.VarsDiagnosticsState[ast.SchemaValidationSource] != op.OpStateUnknown && !job.IgnoreState(ctx) { + return job.StateNotChangedErr{Dir: document.DirHandleFromPath(modPath)} + } + + err = modStore.SetVarsDiagnosticsState(modPath, ast.SchemaValidationSource, op.OpStateLoading) + if err != nil { + return err + } + + d := decoder.NewDecoder(&idecoder.PathReader{ + ModuleReader: modStore, + SchemaReader: schemaReader, + }) + + d.SetContext(idecoder.DecoderContext(ctx)) + + moduleDecoder, err := d.Path(lang.Path{ + Path: modPath, + LanguageID: ilsp.Tfvars.String(), + }) + if err != nil { + return err + } + + var rErr error + rpcContext := lsctx.RPCContext(ctx) + if rpcContext.Method == "textDocument/didChange" && lsctx.IsLanguageId(ctx, ilsp.Tfvars.String()) { + filename := path.Base(rpcContext.URI) + // We only revalidate a single file that changed + var fileDiags hcl.Diagnostics + fileDiags, rErr = moduleDecoder.ValidateFile(ctx, filename) + + varsDiags, ok := mod.VarsDiagnostics[ast.SchemaValidationSource] + if !ok { + varsDiags = make(ast.VarsDiags) + } + varsDiags[ast.VarsFilename(filename)] = fileDiags + + sErr := modStore.UpdateVarsDiagnostics(modPath, ast.SchemaValidationSource, varsDiags) + if sErr != nil { + return sErr + } + } else { + // We validate the whole module, e.g. on open + var diags lang.DiagnosticsMap + diags, rErr = moduleDecoder.Validate(ctx) + + sErr := modStore.UpdateVarsDiagnostics(modPath, ast.SchemaValidationSource, ast.VarsDiagsFromMap(diags)) + if sErr != nil { + return sErr + } + } + + return rErr +} + func ReferenceValidation(ctx context.Context, modStore *state.ModuleStore, schemaReader state.SchemaReader, modPath string) error { mod, err := modStore.ModuleByPath(modPath) if err != nil { diff --git a/internal/terraform/module/module_ops_test.go b/internal/terraform/module/module_ops_test.go index c5b5c43cb..15ca3b309 100644 --- a/internal/terraform/module/module_ops_test.go +++ b/internal/terraform/module/module_ops_test.go @@ -27,6 +27,7 @@ import ( "github.com/hashicorp/terraform-ls/internal/document" "github.com/hashicorp/terraform-ls/internal/filesystem" "github.com/hashicorp/terraform-ls/internal/job" + ilsp "github.com/hashicorp/terraform-ls/internal/lsp" "github.com/hashicorp/terraform-ls/internal/registry" "github.com/hashicorp/terraform-ls/internal/state" "github.com/hashicorp/terraform-ls/internal/terraform/ast" @@ -969,7 +970,7 @@ var randomSchemaJSON = `{ } }` -func TestSchemaValidation_FullModule(t *testing.T) { +func TestSchemaModuleValidation_FullModule(t *testing.T) { ctx := context.Background() ss, err := state.NewStateStore() if err != nil { @@ -996,7 +997,8 @@ func TestSchemaValidation_FullModule(t *testing.T) { Method: "textDocument/didOpen", URI: "file:///test/variables.tf", }) - err = SchemaValidation(ctx, ss.Modules, ss.ProviderSchemas, modPath) + ctx = lsctx.WithLanguageId(ctx, ilsp.Terraform.String()) + err = SchemaModuleValidation(ctx, ss.Modules, ss.ProviderSchemas, modPath) if err != nil { t.Fatal(err) } @@ -1013,7 +1015,7 @@ func TestSchemaValidation_FullModule(t *testing.T) { } } -func TestSchemaValidation_SingleFile(t *testing.T) { +func TestSchemaModuleValidation_SingleFile(t *testing.T) { ctx := context.Background() ss, err := state.NewStateStore() if err != nil { @@ -1040,7 +1042,8 @@ func TestSchemaValidation_SingleFile(t *testing.T) { Method: "textDocument/didChange", URI: "file:///test/variables.tf", }) - err = SchemaValidation(ctx, ss.Modules, ss.ProviderSchemas, modPath) + ctx = lsctx.WithLanguageId(ctx, ilsp.Terraform.String()) + err = SchemaModuleValidation(ctx, ss.Modules, ss.ProviderSchemas, modPath) if err != nil { t.Fatal(err) } @@ -1056,3 +1059,109 @@ func TestSchemaValidation_SingleFile(t *testing.T) { t.Fatalf("expected %d diagnostics, %d given", expectedCount, diagsCount) } } + +func TestSchemaVarsValidation_FullModule(t *testing.T) { + ctx := context.Background() + ss, err := state.NewStateStore() + if err != nil { + t.Fatal(err) + } + + testData, err := filepath.Abs("testdata") + if err != nil { + t.Fatal(err) + } + modPath := filepath.Join(testData, "invalid-tfvars") + + err = ss.Modules.Add(modPath) + if err != nil { + t.Fatal(err) + } + + fs := filesystem.NewFilesystem(ss.DocumentStore) + err = ParseModuleConfiguration(ctx, fs, ss.Modules, modPath) + if err != nil { + t.Fatal(err) + } + err = LoadModuleMetadata(ctx, ss.Modules, modPath) + if err != nil { + t.Fatal(err) + } + err = ParseVariables(ctx, fs, ss.Modules, modPath) + if err != nil { + t.Fatal(err) + } + ctx = lsctx.WithRPCContext(ctx, lsctx.RPCContextData{ + Method: "textDocument/didOpen", + URI: "file:///test/terraform.tfvars", + }) + ctx = lsctx.WithLanguageId(ctx, ilsp.Tfvars.String()) + err = SchemaVariablesValidation(ctx, ss.Modules, ss.ProviderSchemas, modPath) + if err != nil { + t.Fatal(err) + } + + mod, err := ss.Modules.ModuleByPath(modPath) + if err != nil { + t.Fatal(err) + } + + expectedCount := 2 + diagsCount := mod.VarsDiagnostics[ast.SchemaValidationSource].Count() + if diagsCount != expectedCount { + t.Fatalf("expected %d diagnostics, %d given", expectedCount, diagsCount) + } +} + +func TestSchemaVarsValidation_SingleFile(t *testing.T) { + ctx := context.Background() + ss, err := state.NewStateStore() + if err != nil { + t.Fatal(err) + } + + testData, err := filepath.Abs("testdata") + if err != nil { + t.Fatal(err) + } + modPath := filepath.Join(testData, "invalid-tfvars") + + err = ss.Modules.Add(modPath) + if err != nil { + t.Fatal(err) + } + + fs := filesystem.NewFilesystem(ss.DocumentStore) + err = ParseModuleConfiguration(ctx, fs, ss.Modules, modPath) + if err != nil { + t.Fatal(err) + } + err = LoadModuleMetadata(ctx, ss.Modules, modPath) + if err != nil { + t.Fatal(err) + } + err = ParseVariables(ctx, fs, ss.Modules, modPath) + if err != nil { + t.Fatal(err) + } + ctx = lsctx.WithRPCContext(ctx, lsctx.RPCContextData{ + Method: "textDocument/didChange", + URI: "file:///test/terraform.tfvars", + }) + ctx = lsctx.WithLanguageId(ctx, ilsp.Tfvars.String()) + err = SchemaVariablesValidation(ctx, ss.Modules, ss.ProviderSchemas, modPath) + if err != nil { + t.Fatal(err) + } + + mod, err := ss.Modules.ModuleByPath(modPath) + if err != nil { + t.Fatal(err) + } + + expectedCount := 1 + diagsCount := mod.VarsDiagnostics[ast.SchemaValidationSource].Count() + if diagsCount != expectedCount { + t.Fatalf("expected %d diagnostics, %d given", expectedCount, diagsCount) + } +} diff --git a/internal/terraform/module/operation/op_type_string.go b/internal/terraform/module/operation/op_type_string.go index 5b4ac8744..1c4a10fcc 100644 --- a/internal/terraform/module/operation/op_type_string.go +++ b/internal/terraform/module/operation/op_type_string.go @@ -21,14 +21,15 @@ func _() { _ = x[OpTypeGetModuleDataFromRegistry-10] _ = x[OpTypeParseProviderVersions-11] _ = x[OpTypePreloadEmbeddedSchema-12] - _ = x[OpTypeSchemaValidation-13] - _ = x[OpTypeReferenceValidation-14] - _ = x[OpTypeTerraformValidate-15] + _ = x[OpTypeSchemaModuleValidation-13] + _ = x[OpTypeSchemaVarsValidation-14] + _ = x[OpTypeReferenceValidation-15] + _ = x[OpTypeTerraformValidate-16] } -const _OpType_name = "OpTypeUnknownOpTypeGetTerraformVersionOpTypeObtainSchemaOpTypeParseModuleConfigurationOpTypeParseVariablesOpTypeParseModuleManifestOpTypeLoadModuleMetadataOpTypeDecodeReferenceTargetsOpTypeDecodeReferenceOriginsOpTypeDecodeVarsReferencesOpTypeGetModuleDataFromRegistryOpTypeParseProviderVersionsOpTypePreloadEmbeddedSchemaOpTypeSchemaValidationOpTypeReferenceValidationOpTypeTerraformValidate" +const _OpType_name = "OpTypeUnknownOpTypeGetTerraformVersionOpTypeObtainSchemaOpTypeParseModuleConfigurationOpTypeParseVariablesOpTypeParseModuleManifestOpTypeLoadModuleMetadataOpTypeDecodeReferenceTargetsOpTypeDecodeReferenceOriginsOpTypeDecodeVarsReferencesOpTypeGetModuleDataFromRegistryOpTypeParseProviderVersionsOpTypePreloadEmbeddedSchemaOpTypeSchemaModuleValidationOpTypeSchemaVarsValidationOpTypeReferenceValidationOpTypeTerraformValidate" -var _OpType_index = [...]uint16{0, 13, 38, 56, 86, 106, 131, 155, 183, 211, 237, 268, 295, 322, 344, 369, 392} +var _OpType_index = [...]uint16{0, 13, 38, 56, 86, 106, 131, 155, 183, 211, 237, 268, 295, 322, 350, 376, 401, 424} func (i OpType) String() string { if i >= OpType(len(_OpType_index)-1) { diff --git a/internal/terraform/module/operation/operation.go b/internal/terraform/module/operation/operation.go index fe408450d..3485bc096 100644 --- a/internal/terraform/module/operation/operation.go +++ b/internal/terraform/module/operation/operation.go @@ -30,7 +30,8 @@ const ( OpTypeGetModuleDataFromRegistry OpTypeParseProviderVersions OpTypePreloadEmbeddedSchema - OpTypeSchemaValidation + OpTypeSchemaModuleValidation + OpTypeSchemaVarsValidation OpTypeReferenceValidation OpTypeTerraformValidate ) diff --git a/internal/terraform/module/testdata/invalid-tfvars/foo.auto.tfvars b/internal/terraform/module/testdata/invalid-tfvars/foo.auto.tfvars new file mode 100644 index 000000000..60a5c57f4 --- /dev/null +++ b/internal/terraform/module/testdata/invalid-tfvars/foo.auto.tfvars @@ -0,0 +1 @@ +noot = "noot" diff --git a/internal/terraform/module/testdata/invalid-tfvars/terraform.tfvars b/internal/terraform/module/testdata/invalid-tfvars/terraform.tfvars new file mode 100644 index 000000000..79892b8ea --- /dev/null +++ b/internal/terraform/module/testdata/invalid-tfvars/terraform.tfvars @@ -0,0 +1,2 @@ +foo = "foo" +bar = "noot" diff --git a/internal/terraform/module/testdata/invalid-tfvars/variables.tf b/internal/terraform/module/testdata/invalid-tfvars/variables.tf new file mode 100644 index 000000000..91084f7a1 --- /dev/null +++ b/internal/terraform/module/testdata/invalid-tfvars/variables.tf @@ -0,0 +1 @@ +variable "foo" {}