From c003c6b7e9113bceee95e1f62eb2371bf9820877 Mon Sep 17 00:00:00 2001 From: James Pogran Date: Mon, 19 Aug 2024 10:58:21 -0400 Subject: [PATCH] (TF-18673) Validate Stack and Deployment files for unreferenced origins (#1797) * Validate Stack and Deployment files for unreferenced origins This commit adds a new validation to check for unreferenced origins in stack and deployment files. This validation is performed after the reference targets and origins are decoded. The validation checks for references to variables and local values that do not have a corresponding target. The validation is performed for variables, local values, providers and identity_tokens only, as components can have unknown schema. * refactor: stop validating components as we see false positives for referenced outputs that don't match the type constraint for where they're used --------- Co-authored-by: Ansgar Mertens --- .../ENHANCEMENTS-20240815-135108.yaml | 6 ++ .../validations/unreferenced_origins.go | 81 +++++++++++++++++++ internal/features/stacks/events.go | 14 ++++ internal/features/stacks/jobs/validation.go | 51 ++++++++++++ .../module/operation/op_type_string.go | 13 +-- .../terraform/module/operation/operation.go | 1 + 6 files changed, 160 insertions(+), 6 deletions(-) create mode 100644 .changes/unreleased/ENHANCEMENTS-20240815-135108.yaml create mode 100644 internal/features/stacks/decoder/validations/unreferenced_origins.go diff --git a/.changes/unreleased/ENHANCEMENTS-20240815-135108.yaml b/.changes/unreleased/ENHANCEMENTS-20240815-135108.yaml new file mode 100644 index 000000000..9c6cb773c --- /dev/null +++ b/.changes/unreleased/ENHANCEMENTS-20240815-135108.yaml @@ -0,0 +1,6 @@ +kind: ENHANCEMENTS +body: Validate Stack and Deployment files for unreferenced origins +time: 2024-08-15T13:51:08.906805-04:00 +custom: + Issue: "1797" + Repository: terraform-ls diff --git a/internal/features/stacks/decoder/validations/unreferenced_origins.go b/internal/features/stacks/decoder/validations/unreferenced_origins.go new file mode 100644 index 000000000..593b2dc3e --- /dev/null +++ b/internal/features/stacks/decoder/validations/unreferenced_origins.go @@ -0,0 +1,81 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package validations + +import ( + "context" + "fmt" + "slices" + + "github.com/hashicorp/hcl-lang/decoder" + "github.com/hashicorp/hcl-lang/lang" + "github.com/hashicorp/hcl-lang/reference" + "github.com/hashicorp/hcl/v2" +) + +func UnreferencedOrigins(ctx context.Context, pathCtx *decoder.PathContext) lang.DiagnosticsMap { + diagsMap := make(lang.DiagnosticsMap) + + for _, origin := range pathCtx.ReferenceOrigins { + localOrigin, ok := origin.(reference.LocalOrigin) + if !ok { + // We avoid reporting on other origin types. + // + // DirectOrigin is represented as module's source + // and we already validate existence of the local module + // and avoiding linking to a non-existent module in terraform-schema + // https://github.com/hashicorp/terraform-schema/blob/b39f3de0/schema/module_schema.go#L212-L232 + // + // PathOrigin is represented as module inputs + // and we can validate module inputs more meaningfully + // as attributes in body (module block), e.g. raise that + // an input is required or unknown, rather than "reference" + // lacking a corresponding target. + continue + } + + address := localOrigin.Address() + + if len(address) > 2 { + // We temporarily ignore references with more than 2 segments + // as these indicate references to complex types + // which we do not fully support yet. + // TODO: revisit as part of https://github.com/hashicorp/terraform-ls/issues/653 + + // However, we still want to validate references to component provider and identity_token + // for Stacks. This is relatively safe as we know the structure of the references + // and can validate them without needing to know the schema of the referenced object. + // TODO: revisit after user feedback + supported := []string{"provider", "identity_token"} + if !slices.Contains(supported, address[0].String()) { + continue + } + } + + // we only initially validate variables, providers, and identity_tokens + // resources can have unknown schema and will be researched at a later point + // TODO: revisit as part of https://github.com/hashicorp/terraform-ls/issues/1364 + supported := []string{"var", "provider", "identity_token"} + firstStep := address[0].String() + if !slices.Contains(supported, firstStep) { + continue + } + + _, ok = pathCtx.ReferenceTargets.Match(localOrigin) + if !ok { + // target not found + fileName := origin.OriginRange().Filename + d := &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: fmt.Sprintf("No declaration found for %q", address), + Subject: origin.OriginRange().Ptr(), + } + diagsMap[fileName] = diagsMap[fileName].Append(d) + + continue + } + } + + return diagsMap +} diff --git a/internal/features/stacks/events.go b/internal/features/stacks/events.go index f7a614cdd..e8aabd510 100644 --- a/internal/features/stacks/events.go +++ b/internal/features/stacks/events.go @@ -289,6 +289,20 @@ func (f *StacksFeature) decodeStack(ctx context.Context, dir document.DirHandle, } } + _, err = f.stateStore.JobStore.EnqueueJob(ctx, job.Job{ + Dir: dir, + Func: func(ctx context.Context) error { + + return jobs.ReferenceValidation(ctx, f.store, f.moduleFeature, dir.Path()) + }, + Type: operation.OpTypeReferenceStackValidation.String(), + DependsOn: job.IDs{refOriginsId, refTargetsId}, + IgnoreState: ignoreState, + }) + if err != nil { + return deferIds, err + } + return deferIds, nil }, }) diff --git a/internal/features/stacks/jobs/validation.go b/internal/features/stacks/jobs/validation.go index 975226273..505f98a5d 100644 --- a/internal/features/stacks/jobs/validation.go +++ b/internal/features/stacks/jobs/validation.go @@ -15,6 +15,7 @@ import ( "github.com/hashicorp/terraform-ls/internal/document" "github.com/hashicorp/terraform-ls/internal/features/stacks/ast" stackDecoder "github.com/hashicorp/terraform-ls/internal/features/stacks/decoder" + "github.com/hashicorp/terraform-ls/internal/features/stacks/decoder/validations" "github.com/hashicorp/terraform-ls/internal/features/stacks/state" "github.com/hashicorp/terraform-ls/internal/job" ilsp "github.com/hashicorp/terraform-ls/internal/lsp" @@ -116,3 +117,53 @@ func SchemaStackValidation(ctx context.Context, stackStore *state.StackStore, mo return rErr } + +// ReferenceValidation does validation based on (mis)matched +// reference origins and targets, to flag up "orphaned" references. +// +// It relies on [DecodeReferenceTargets] and [DecodeReferenceOrigins] +// to supply both origins and targets to compare. +func ReferenceValidation(ctx context.Context, stackStore *state.StackStore, moduleFeature stackDecoder.ModuleReader, stackPath string) error { + record, err := stackStore.StackRecordByPath(stackPath) + if err != nil { + return err + } + + // Avoid validation if it is already in progress or already finished + if record.DiagnosticsState[globalAst.ReferenceValidationSource] != operation.OpStateUnknown && !job.IgnoreState(ctx) { + return job.StateNotChangedErr{Dir: document.DirHandleFromPath(stackPath)} + } + + err = stackStore.SetDiagnosticsState(stackPath, globalAst.ReferenceValidationSource, operation.OpStateLoading) + if err != nil { + return err + } + + pathReader := &stackDecoder.PathReader{ + StateReader: stackStore, + ModuleReader: moduleFeature, + } + + stackDecoder, err := pathReader.PathContext(lang.Path{ + Path: stackPath, + LanguageID: ilsp.Stacks.String(), + }) + if err != nil { + return err + } + + deployDecoder, err := pathReader.PathContext(lang.Path{ + Path: stackPath, + LanguageID: ilsp.Deploy.String(), + }) + if err != nil { + return err + } + + diags := validations.UnreferencedOrigins(ctx, stackDecoder) + + deployDiags := validations.UnreferencedOrigins(ctx, deployDecoder) + diags = diags.Extend(deployDiags) + + return stackStore.UpdateDiagnostics(stackPath, globalAst.ReferenceValidationSource, ast.DiagnosticsFromMap(diags)) +} diff --git a/internal/terraform/module/operation/op_type_string.go b/internal/terraform/module/operation/op_type_string.go index b116ecb6d..afa48295f 100644 --- a/internal/terraform/module/operation/op_type_string.go +++ b/internal/terraform/module/operation/op_type_string.go @@ -27,15 +27,16 @@ func _() { _ = x[OpTypeSchemaStackValidation-16] _ = x[OpTypeSchemaVarsValidation-17] _ = x[OpTypeReferenceValidation-18] - _ = x[OpTypeTerraformValidate-19] - _ = x[OpTypeParseStackConfiguration-20] - _ = x[OpTypeLoadStackMetadata-21] - _ = x[OpTypeLoadStackRequiredTerraformVersion-22] + _ = x[OpTypeReferenceStackValidation-19] + _ = x[OpTypeTerraformValidate-20] + _ = x[OpTypeParseStackConfiguration-21] + _ = x[OpTypeLoadStackMetadata-22] + _ = x[OpTypeLoadStackRequiredTerraformVersion-23] } -const _OpType_name = "OpTypeUnknownOpTypeGetTerraformVersionOpTypeGetInstalledTerraformVersionOpTypeObtainSchemaOpTypeParseModuleConfigurationOpTypeParseVariablesOpTypeParseModuleManifestOpTypeLoadModuleMetadataOpTypeDecodeReferenceTargetsOpTypeDecodeReferenceOriginsOpTypeDecodeVarsReferencesOpTypeGetModuleDataFromRegistryOpTypeParseProviderVersionsOpTypePreloadEmbeddedSchemaOpTypeStacksPreloadEmbeddedSchemaOpTypeSchemaModuleValidationOpTypeSchemaStackValidationOpTypeSchemaVarsValidationOpTypeReferenceValidationOpTypeTerraformValidateOpTypeParseStackConfigurationOpTypeLoadStackMetadataOpTypeLoadStackRequiredTerraformVersion" +const _OpType_name = "OpTypeUnknownOpTypeGetTerraformVersionOpTypeGetInstalledTerraformVersionOpTypeObtainSchemaOpTypeParseModuleConfigurationOpTypeParseVariablesOpTypeParseModuleManifestOpTypeLoadModuleMetadataOpTypeDecodeReferenceTargetsOpTypeDecodeReferenceOriginsOpTypeDecodeVarsReferencesOpTypeGetModuleDataFromRegistryOpTypeParseProviderVersionsOpTypePreloadEmbeddedSchemaOpTypeStacksPreloadEmbeddedSchemaOpTypeSchemaModuleValidationOpTypeSchemaStackValidationOpTypeSchemaVarsValidationOpTypeReferenceValidationOpTypeReferenceStackValidationOpTypeTerraformValidateOpTypeParseStackConfigurationOpTypeLoadStackMetadataOpTypeLoadStackRequiredTerraformVersion" -var _OpType_index = [...]uint16{0, 13, 38, 72, 90, 120, 140, 165, 189, 217, 245, 271, 302, 329, 356, 389, 417, 444, 470, 495, 518, 547, 570, 609} +var _OpType_index = [...]uint16{0, 13, 38, 72, 90, 120, 140, 165, 189, 217, 245, 271, 302, 329, 356, 389, 417, 444, 470, 495, 525, 548, 577, 600, 639} 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 a4f1ed81b..5d68ab962 100644 --- a/internal/terraform/module/operation/operation.go +++ b/internal/terraform/module/operation/operation.go @@ -36,6 +36,7 @@ const ( OpTypeSchemaStackValidation OpTypeSchemaVarsValidation OpTypeReferenceValidation + OpTypeReferenceStackValidation OpTypeTerraformValidate OpTypeParseStackConfiguration OpTypeLoadStackMetadata