Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(TF-18290) Early validation for Terraform Stack files #1776

Merged
merged 2 commits into from
Aug 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changes/unreleased/ENHANCEMENTS-20240724-122601.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: ENHANCEMENTS
body: Enable early validation for Terraform Stack files
time: 2024-07-24T12:26:01.677192-04:00
custom:
Issue: "1776"
Repository: terraform-ls
8 changes: 8 additions & 0 deletions internal/features/stacks/ast/stacks.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,14 @@ func (sd Diagnostics) Count() int {
return count
}

func DiagnosticsFromMap(m map[string]hcl.Diagnostics) Diagnostics {
mf := make(Diagnostics, len(m))
for name, file := range m {
mf[FilenameFromName(name)] = file
}
return mf
}

type SourceDiagnostics map[globalAst.DiagnosticSource]Diagnostics

func (svd SourceDiagnostics) Count() int {
Expand Down
2 changes: 2 additions & 0 deletions internal/features/stacks/decoder/path_reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ func stackPathContext(record *state.StackRecord, stateReader CombinedReader) (*d
ReferenceTargets: make(reference.Targets, 0),
Files: make(map[string]*hcl.File, 0),
Functions: mustFunctionsForVersion(version),
Validators: stackValidators,
}

// TODO: Add reference origins and targets if needed
Expand Down Expand Up @@ -148,6 +149,7 @@ func deployPathContext(record *state.StackRecord) (*decoder.PathContext, error)
ReferenceOrigins: make(reference.Origins, 0),
ReferenceTargets: make(reference.Targets, 0),
Files: make(map[string]*hcl.File, 0),
Validators: stackValidators,
}

// TODO: Add reference origins and targets if needed
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: MPL-2.0

package validations

import (
"context"
"fmt"

"github.com/hashicorp/hcl-lang/schema"
"github.com/hashicorp/hcl-lang/schemacontext"
"github.com/hashicorp/hcl/v2"
"github.com/hashicorp/hcl/v2/hclsyntax"
)

type MissingRequiredAttribute struct{}

func (mra MissingRequiredAttribute) Visit(ctx context.Context, node hclsyntax.Node, nodeSchema schema.Schema) (context.Context, hcl.Diagnostics) {
var diags hcl.Diagnostics
if HasUnknownRequiredAttributes(ctx) {
return ctx, diags
}

switch nodeType := node.(type) {
case *hclsyntax.Block:
// Providers were excluded from Terraform validation due to https://github.com/hashicorp/vscode-terraform/issues/1616
// TODO: See if we can remove this exclusion for Terraform Stacks
jpogran marked this conversation as resolved.
Show resolved Hide resolved
// We would need to check if the provider can be configured via environment variables in Stacks. Since you can
// have multiple configurations for the same provider, this may be challenging.
// If it is possible, we may need to update this code to reflect the nested config { } block. But we're not sure right now.
nestingLvl, nestingOk := schemacontext.BlockNestingLevel(ctx)
if nodeType.Type == "provider" && (nestingOk && nestingLvl == 0) {
ctx = WithUnknownRequiredAttributes(ctx)
}
case *hclsyntax.Body:
if nodeSchema == nil {
return ctx, diags
}

bodySchema := nodeSchema.(*schema.BodySchema)
if bodySchema.Attributes == nil {
return ctx, diags
}

for name, attr := range bodySchema.Attributes {
if attr.IsRequired {
_, ok := nodeType.Attributes[name]
if !ok {
diags = append(diags, &hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: fmt.Sprintf("Required attribute %q not specified", name),
Detail: fmt.Sprintf("An attribute named %q is required here", name),
Subject: nodeType.SrcRange.Ptr(),
})
}
}
}
}

return ctx, diags
}

type unknownRequiredAttrsCtxKey struct{}

func HasUnknownRequiredAttributes(ctx context.Context) bool {
jpogran marked this conversation as resolved.
Show resolved Hide resolved
_, ok := ctx.Value(unknownRequiredAttrsCtxKey{}).(bool)
return ok
}

func WithUnknownRequiredAttributes(ctx context.Context) context.Context {
return context.WithValue(ctx, unknownRequiredAttrsCtxKey{}, true)
}
71 changes: 71 additions & 0 deletions internal/features/stacks/decoder/validations/valid_name.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: MPL-2.0

package validations

import (
"context"
"fmt"

"github.com/hashicorp/hcl-lang/schema"
"github.com/hashicorp/hcl/v2"
"github.com/hashicorp/hcl/v2/hclsyntax"
)

type StackBlockValidName struct{}

func (sb StackBlockValidName) Visit(ctx context.Context, node hclsyntax.Node, nodeSchema schema.Schema) (context.Context, hcl.Diagnostics) {
var diags hcl.Diagnostics

block, ok := node.(*hclsyntax.Block)
if !ok {
return ctx, diags
}

if nodeSchema == nil {
return ctx, diags
}

// TODO: Revist checking Stack concepts in the static validators
// Other validators are more generic and can be applied to any block/attribute/etc, but
// this one is specific to the stack block types. I'm sure this could be done in a more generic way,
// but I'm not sure if it's worth it at the moment. The names of these blocks are used as identifiers
// in the stack and are used in the UI, so they have to be valid Terraform identifiers or else the stack
// will not be able to be created. This is a very basic check to ensure that the names are valid until we can
// do more advanced checks or use the cloud api.
// I tried adding this to the earlydecoder, but we do not seem to report the diagnostics there at all currently,
// so nothing was being reported. I'm not sure if that's a bug or intended.

switch block.Type {
// stack
case "component":
diags = hasValidNameLabel(block, diags)
// case "provider":
// case "required_providers":
case "variable":
diags = hasValidNameLabel(block, diags)
case "output":
diags = hasValidNameLabel(block, diags)
// deployment
case "deployment":
diags = hasValidNameLabel(block, diags)
case "identity_token":
diags = hasValidNameLabel(block, diags)
case "orchestrate":
diags = hasValidNameLabel(block, diags)
}

return ctx, diags
}

func hasValidNameLabel(block *hclsyntax.Block, diags hcl.Diagnostics) hcl.Diagnostics {
if !hclsyntax.ValidIdentifier(block.Labels[0]) {
diags = append(diags, &hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: fmt.Sprintf("Invalid %q name %q", block.Type, block.Labels[0]),
Detail: "Names must be valid identifiers: beginning with a letter or underscore, followed by zero or more letters, digits, or underscores.",
Subject: block.LabelRanges[0].Ptr(),
})
}
return diags
}
21 changes: 21 additions & 0 deletions internal/features/stacks/decoder/validators.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: MPL-2.0

package decoder

import (
"github.com/hashicorp/hcl-lang/validator"
"github.com/hashicorp/terraform-ls/internal/features/stacks/decoder/validations"
)

var stackValidators = []validator.Validator{
validator.BlockLabelsLength{},
validator.DeprecatedAttribute{},
validator.DeprecatedBlock{},
validator.MaxBlocks{},
validator.MinBlocks{},
validator.UnexpectedAttribute{},
validator.UnexpectedBlock{},
validations.MissingRequiredAttribute{},
validations.StackBlockValidName{},
}
21 changes: 21 additions & 0 deletions internal/features/stacks/events.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"os"
"path/filepath"

lsctx "github.com/hashicorp/terraform-ls/internal/context"
"github.com/hashicorp/terraform-ls/internal/document"
"github.com/hashicorp/terraform-ls/internal/eventbus"
"github.com/hashicorp/terraform-ls/internal/features/stacks/ast"
Expand Down Expand Up @@ -191,6 +192,10 @@ func (f *StacksFeature) decodeStack(ctx context.Context, dir document.DirHandle,
}
ids = append(ids, parseId)

// this needs to be here because the setting context
// is not available in the validate job
validationOptions, _ := lsctx.ValidationOptions(ctx)

metaId, err := f.stateStore.JobStore.EnqueueJob(ctx, job.Job{
Dir: dir,
Func: func(ctx context.Context) error {
Expand Down Expand Up @@ -233,6 +238,22 @@ func (f *StacksFeature) decodeStack(ctx context.Context, dir document.DirHandle,
}
deferIds = append(deferIds, eSchemaId)

if validationOptions.EnableEnhancedValidation {
validationId, err := f.stateStore.JobStore.EnqueueJob(ctx, job.Job{
Dir: dir,
Func: func(ctx context.Context) error {
return jobs.SchemaStackValidation(ctx, f.store, f.moduleFeature, dir.Path())
},
Type: operation.OpTypeSchemaStackValidation.String(),
DependsOn: deferIds,
IgnoreState: ignoreState,
})
if err != nil {
return deferIds, err
}
deferIds = append(deferIds, validationId)
}

return deferIds, nil
},
})
Expand Down
118 changes: 118 additions & 0 deletions internal/features/stacks/jobs/validation.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: MPL-2.0

package jobs

import (
"context"
"path"

"github.com/hashicorp/hcl-lang/decoder"
"github.com/hashicorp/hcl-lang/lang"
"github.com/hashicorp/hcl/v2"
lsctx "github.com/hashicorp/terraform-ls/internal/context"
idecoder "github.com/hashicorp/terraform-ls/internal/decoder"
"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/state"
"github.com/hashicorp/terraform-ls/internal/job"
ilsp "github.com/hashicorp/terraform-ls/internal/lsp"
globalAst "github.com/hashicorp/terraform-ls/internal/terraform/ast"
"github.com/hashicorp/terraform-ls/internal/terraform/module/operation"
)

func SchemaStackValidation(ctx context.Context, stackStore *state.StackStore, moduleFeature stackDecoder.ModuleReader, stackPath string) error {
rpcContext := lsctx.DocumentContext(ctx)

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.SchemaValidationSource] != operation.OpStateUnknown && !job.IgnoreState(ctx) {
return job.StateNotChangedErr{Dir: document.DirHandleFromPath(stackPath)}
}

err = stackStore.SetDiagnosticsState(stackPath, globalAst.SchemaValidationSource, operation.OpStateLoading)
if err != nil {
return err
}

d := decoder.NewDecoder(&stackDecoder.PathReader{
StateReader: stackStore,
ModuleReader: moduleFeature,
})
d.SetContext(idecoder.DecoderContext(ctx))

var rErr error
if rpcContext.Method == "textDocument/didChange" {
// We validate only the file that has changed
// This means only creating a decoder for the file type that has changed
decoder, err := d.Path(lang.Path{
Path: stackPath,
LanguageID: rpcContext.LanguageID,
})
if err != nil {
return err
}

filename := path.Base(rpcContext.URI)

var fileDiags hcl.Diagnostics
fileDiags, rErr = decoder.ValidateFile(ctx, filename)

diags, ok := record.Diagnostics[globalAst.SchemaValidationSource]
if !ok {
diags = make(ast.Diagnostics)
}
diags[ast.FilenameFromName(filename)] = fileDiags

sErr := stackStore.UpdateDiagnostics(stackPath, globalAst.SchemaValidationSource, diags)
if sErr != nil {
return sErr
}
} else {
// We validate the whole stack, and so need to create decoders for
// all the file types in the stack
stackDecoder, err := d.Path(lang.Path{
Path: stackPath,
LanguageID: ilsp.Stacks.String(),
})
if err != nil {
return err
}
deployDecoder, err := d.Path(lang.Path{
Path: stackPath,
LanguageID: ilsp.Deploy.String(),
})
if err != nil {
return err
}

diags := make(lang.DiagnosticsMap)

stacksDiags, err := stackDecoder.Validate(ctx)
if err != nil {
// TODO: Should we really return here or continue with the other decoders?
// Is this really a complete fail case? Shouldn't a failure in a stack file
// not prevent the deploy file from being validated?
jpogran marked this conversation as resolved.
Show resolved Hide resolved
return err
}
diags = diags.Extend(stacksDiags)

deployDiags, err := deployDecoder.Validate(ctx)
if err != nil {
return err
}
diags = diags.Extend(deployDiags)

sErr := stackStore.UpdateDiagnostics(stackPath, globalAst.SchemaValidationSource, ast.DiagnosticsFromMap(diags))
if sErr != nil {
return sErr
}
}

return rErr
}
17 changes: 9 additions & 8 deletions internal/terraform/module/operation/op_type_string.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading