From 3827120c258f40e0438d4d772dd4f05aa219532d Mon Sep 17 00:00:00 2001 From: Liam Cervante Date: Thu, 23 Mar 2023 09:12:53 +0100 Subject: [PATCH] Checks: Add configuration for check blocks (#32734) * Add support for scoped resources * refactor existing checks addrs and add check block addr * Add configuration for check blocks * address comments --- internal/configs/checks.go | 116 ++++++++++++++++++++++++++++++ internal/configs/module.go | 38 ++++++++++ internal/configs/parser_config.go | 13 +++- internal/configs/resource.go | 33 ++++++++- 4 files changed, 196 insertions(+), 4 deletions(-) diff --git a/internal/configs/checks.go b/internal/configs/checks.go index 417dff45ee..079d37de19 100644 --- a/internal/configs/checks.go +++ b/internal/configs/checks.go @@ -4,6 +4,8 @@ import ( "fmt" "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/hcl/v2/hclsyntax" + "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/lang" ) @@ -139,3 +141,117 @@ var checkRuleBlockSchema = &hcl.BodySchema{ }, }, } + +// Check represents a configuration defined check block. +// +// A check block contains 0-1 data blocks, and 0-n assert blocks. The check +// block will load the data block, and execute the assert blocks as check rules +// during the plan and apply Terraform operations. +type Check struct { + Name string + + DataResource *Resource + Asserts []*CheckRule + + DeclRange hcl.Range +} + +func (c Check) Addr() addrs.Check { + return addrs.Check{ + Name: c.Name, + } +} + +func (c Check) Accessible(addr addrs.Referenceable) bool { + if check, ok := addr.(addrs.Check); ok { + return check.Equal(c.Addr()) + } + return false +} + +func decodeCheckBlock(block *hcl.Block, override bool) (*Check, hcl.Diagnostics) { + var diags hcl.Diagnostics + + check := &Check{ + Name: block.Labels[0], + DeclRange: block.DefRange, + } + + if override { + // For now we'll just forbid overriding check blocks, to simplify + // the initial design. If we can find a clear use-case for overriding + // checks in override files and there's a way to define it that + // isn't confusing then we could relax this. + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Can't override check blocks", + Detail: "Override files cannot override check blocks.", + Subject: check.DeclRange.Ptr(), + }) + return check, diags + } + + content, moreDiags := block.Body.Content(checkBlockSchema) + diags = append(diags, moreDiags...) + + if !hclsyntax.ValidIdentifier(check.Name) { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid check block name", + Detail: badIdentifierDetail, + Subject: &block.LabelRanges[0], + }) + } + + for _, block := range content.Blocks { + switch block.Type { + case "data": + + if check.DataResource != nil { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Multiple data resource blocks", + Detail: fmt.Sprintf("This check block already has a data resource defined at %s.", check.DataResource.DeclRange.Ptr()), + Subject: block.DefRange.Ptr(), + }) + continue + } + + data, moreDiags := decodeDataBlock(block, override, true) + diags = append(diags, moreDiags...) + if !moreDiags.HasErrors() { + // Connect this data block back up to this check block. + data.Container = check + + // Finally, save the data block. + check.DataResource = data + } + case "assert": + assert, moreDiags := decodeCheckRuleBlock(block, override) + diags = append(diags, moreDiags...) + if !moreDiags.HasErrors() { + check.Asserts = append(check.Asserts, assert) + } + default: + panic(fmt.Sprintf("unhandled check nested block %q", block.Type)) + } + } + + if len(check.Asserts) == 0 { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Zero assert blocks", + Detail: "Check blocks must have at least one assert block.", + Subject: check.DeclRange.Ptr(), + }) + } + + return check, diags +} + +var checkBlockSchema = &hcl.BodySchema{ + Blocks: []hcl.BlockHeaderSchema{ + {Type: "data", LabelNames: []string{"type", "name"}}, + {Type: "assert"}, + }, +} diff --git a/internal/configs/module.go b/internal/configs/module.go index 6ecae9c290..8149745fbe 100644 --- a/internal/configs/module.go +++ b/internal/configs/module.go @@ -47,6 +47,8 @@ type Module struct { DataResources map[string]*Resource Moved []*Moved + + Checks map[string]*Check } // File describes the contents of a single configuration file. @@ -81,6 +83,8 @@ type File struct { DataResources []*Resource Moved []*Moved + + Checks []*Check } // NewModule takes a list of primary files and a list of override files and @@ -102,6 +106,7 @@ func NewModule(primaryFiles, overrideFiles []*File) (*Module, hcl.Diagnostics) { ModuleCalls: map[string]*ModuleCall{}, ManagedResources: map[string]*Resource{}, DataResources: map[string]*Resource{}, + Checks: map[string]*Check{}, ProviderMetas: map[addrs.Provider]*ProviderMeta{}, } @@ -330,6 +335,9 @@ func (m *Module) appendFile(file *File) hcl.Diagnostics { } } + // Data sources can either be defined at the module root level, or within a + // single check block. We'll merge the data sources from both into the + // single module level DataResources map. for _, r := range file.DataResources { key := r.moduleUniqueKey() if existing, exists := m.DataResources[key]; exists { @@ -342,7 +350,37 @@ func (m *Module) appendFile(file *File) hcl.Diagnostics { continue } m.DataResources[key] = r + } + for _, c := range file.Checks { + if c.DataResource != nil { + key := c.DataResource.moduleUniqueKey() + if existing, exists := m.DataResources[key]; exists { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: fmt.Sprintf("Duplicate data %q configuration", existing.Type), + Detail: fmt.Sprintf("A %s data resource named %q was already declared at %s. Resource names must be unique per type in each module, including within check blocks.", existing.Type, existing.Name, existing.DeclRange), + Subject: &c.DataResource.DeclRange, + }) + continue + } + m.DataResources[key] = c.DataResource + } + + if existing, exists := m.Checks[c.Name]; exists { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: fmt.Sprintf("Duplicate check %q configuration", existing.Name), + Detail: fmt.Sprintf("A check block named %q was already declared at %s. Check blocks must be unique within each module.", existing.Name, existing.DeclRange), + Subject: &c.DeclRange, + }) + continue + } + m.Checks[c.Name] = c + } + + // Handle the provider associations for all data resources together. + for _, r := range m.DataResources { // set the provider FQN for the resource if r.ProviderConfigRef != nil { r.Provider = m.ProviderForLocalConfig(r.ProviderConfigAddr()) diff --git a/internal/configs/parser_config.go b/internal/configs/parser_config.go index 2e08580b5b..79186dce39 100644 --- a/internal/configs/parser_config.go +++ b/internal/configs/parser_config.go @@ -149,7 +149,7 @@ func (p *Parser) loadConfigFile(path string, override bool) (*File, hcl.Diagnost } case "data": - cfg, cfgDiags := decodeDataBlock(block, override) + cfg, cfgDiags := decodeDataBlock(block, override, false) diags = append(diags, cfgDiags...) if cfg != nil { file.DataResources = append(file.DataResources, cfg) @@ -162,6 +162,13 @@ func (p *Parser) loadConfigFile(path string, override bool) (*File, hcl.Diagnost file.Moved = append(file.Moved, cfg) } + case "check": + cfg, cfgDiags := decodeCheckBlock(block, override) + diags = append(diags, cfgDiags...) + if cfg != nil { + file.Checks = append(file.Checks, cfg) + } + default: // Should never happen because the above cases should be exhaustive // for all block type names in our schema. @@ -252,6 +259,10 @@ var configFileSchema = &hcl.BodySchema{ { Type: "moved", }, + { + Type: "check", + LabelNames: []string{"name"}, + }, }, } diff --git a/internal/configs/resource.go b/internal/configs/resource.go index f0d809ab1f..cdba4e9987 100644 --- a/internal/configs/resource.go +++ b/internal/configs/resource.go @@ -356,7 +356,7 @@ func decodeResourceBlock(block *hcl.Block, override bool) (*Resource, hcl.Diagno return r, diags } -func decodeDataBlock(block *hcl.Block, override bool) (*Resource, hcl.Diagnostics) { +func decodeDataBlock(block *hcl.Block, override, nested bool) (*Resource, hcl.Diagnostics) { var diags hcl.Diagnostics r := &Resource{ Mode: addrs.DataResourceMode, @@ -387,11 +387,19 @@ func decodeDataBlock(block *hcl.Block, override bool) (*Resource, hcl.Diagnostic }) } - if attr, exists := content.Attributes["count"]; exists { + if attr, exists := content.Attributes["count"]; exists && !nested { r.Count = attr.Expr + } else if exists && nested { + // We don't allow count attributes in nested data blocks. + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: `Invalid "count" attribute`, + Detail: `The "count" and "for_each" meta-arguments are not supported within nested data blocks.`, + Subject: &attr.NameRange, + }) } - if attr, exists := content.Attributes["for_each"]; exists { + if attr, exists := content.Attributes["for_each"]; exists && !nested { r.ForEach = attr.Expr // Cannot have count and for_each on the same data block if r.Count != nil { @@ -402,6 +410,14 @@ func decodeDataBlock(block *hcl.Block, override bool) (*Resource, hcl.Diagnostic Subject: &attr.NameRange, }) } + } else if exists && nested { + // We don't allow for_each attributes in nested data blocks. + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: `Invalid "for_each" attribute`, + Detail: `The "count" and "for_each" meta-arguments are not supported within nested data blocks.`, + Subject: &attr.NameRange, + }) } if attr, exists := content.Attributes["provider"]; exists { @@ -442,6 +458,17 @@ func decodeDataBlock(block *hcl.Block, override bool) (*Resource, hcl.Diagnostic r.Config = hcl.MergeBodies([]hcl.Body{r.Config, block.Body}) case "lifecycle": + if nested { + // We don't allow lifecycle arguments in nested data blocks, + // the lifecycle is managed by the parent block. + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid lifecycle block", + Detail: `Nested data blocks do not support "lifecycle" blocks as the lifecycle is managed by the containing block.`, + Subject: block.DefRange.Ptr(), + }) + } + if seenLifecycle != nil { diags = append(diags, &hcl.Diagnostic{ Severity: hcl.DiagError,