From de03bec203aa9690e94faa353af2142a359bf987 Mon Sep 17 00:00:00 2001 From: Liam Cervante Date: Fri, 17 Feb 2023 13:13:52 +0100 Subject: [PATCH 1/8] Add support for scoped resources --- internal/configs/container.go | 16 ++++++++++++++ internal/configs/resource.go | 7 +++++++ internal/lang/data.go | 2 +- internal/lang/data_test.go | 2 +- internal/lang/eval.go | 2 +- internal/lang/scope.go | 8 +++++++ internal/terraform/context_eval.go | 2 +- internal/terraform/eval_conditions.go | 2 +- internal/terraform/eval_context.go | 2 +- internal/terraform/eval_context_builtin.go | 8 +++---- internal/terraform/eval_context_mock.go | 2 +- internal/terraform/eval_for_each.go | 2 +- internal/terraform/eval_variable.go | 2 +- internal/terraform/evaluate.go | 11 +++++----- internal/terraform/evaluate_test.go | 16 +++++++------- internal/terraform/evaluate_valid.go | 21 +++++++++++++------ internal/terraform/evaluate_valid_test.go | 2 +- internal/terraform/node_module_variable.go | 2 +- .../node_resource_abstract_instance.go | 2 +- internal/terraform/node_resource_validate.go | 4 ++-- 20 files changed, 78 insertions(+), 37 deletions(-) create mode 100644 internal/configs/container.go diff --git a/internal/configs/container.go b/internal/configs/container.go new file mode 100644 index 000000000000..b43c2c17a288 --- /dev/null +++ b/internal/configs/container.go @@ -0,0 +1,16 @@ +package configs + +import "github.com/hashicorp/terraform/internal/addrs" + +// Container provides an interface for scoped resources. +// +// Any resources contained within a Container should not be accessible from +// outside the container. +type Container interface { + // Accessible should return true if the resource specified by addr can + // reference other items within this Container. + // + // Typically, that means that addr will either be the container itself or + // something within the container. + Accessible(addr addrs.Referenceable) bool +} diff --git a/internal/configs/resource.go b/internal/configs/resource.go index 1f67c6c40f68..f0d809ab1f3a 100644 --- a/internal/configs/resource.go +++ b/internal/configs/resource.go @@ -37,6 +37,13 @@ type Resource struct { // For all other resource modes, this field is nil. Managed *ManagedResource + // Container links a scoped resource back up to the resources that contains + // it. This field is referenced during static analysis to check whether any + // references are also made from within the same container. + // + // If this is nil, then this resource is essentially public. + Container Container + DeclRange hcl.Range TypeRange hcl.Range } diff --git a/internal/lang/data.go b/internal/lang/data.go index 710fccedc8c5..f79298e03bf1 100644 --- a/internal/lang/data.go +++ b/internal/lang/data.go @@ -20,7 +20,7 @@ import ( // cases where it's not possible to even determine a suitable result type, // cty.DynamicVal is returned along with errors describing the problem. type Data interface { - StaticValidateReferences(refs []*addrs.Reference, self addrs.Referenceable) tfdiags.Diagnostics + StaticValidateReferences(refs []*addrs.Reference, self addrs.Referenceable, source addrs.Referenceable) tfdiags.Diagnostics GetCountAttr(addrs.CountAttr, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) GetForEachAttr(addrs.ForEachAttr, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) diff --git a/internal/lang/data_test.go b/internal/lang/data_test.go index e86a8561839b..010c243fa95a 100644 --- a/internal/lang/data_test.go +++ b/internal/lang/data_test.go @@ -19,7 +19,7 @@ type dataForTests struct { var _ Data = &dataForTests{} -func (d *dataForTests) StaticValidateReferences(refs []*addrs.Reference, self addrs.Referenceable) tfdiags.Diagnostics { +func (d *dataForTests) StaticValidateReferences(refs []*addrs.Reference, self addrs.Referenceable, source addrs.Referenceable) tfdiags.Diagnostics { return nil // does nothing in this stub implementation } diff --git a/internal/lang/eval.go b/internal/lang/eval.go index 5c82392bcc44..f25398e7c937 100644 --- a/internal/lang/eval.go +++ b/internal/lang/eval.go @@ -259,7 +259,7 @@ func (s *Scope) evalContext(refs []*addrs.Reference, selfAddr addrs.Referenceabl // First we'll do static validation of the references. This catches things // early that might otherwise not get caught due to unknown values being // present in the scope during planning. - staticDiags := s.Data.StaticValidateReferences(refs, selfAddr) + staticDiags := s.Data.StaticValidateReferences(refs, selfAddr, s.SourceAddr) diags = diags.Append(staticDiags) if staticDiags.HasErrors() { return ctx, diags diff --git a/internal/lang/scope.go b/internal/lang/scope.go index 6c229e25d90a..fea113269b7d 100644 --- a/internal/lang/scope.go +++ b/internal/lang/scope.go @@ -20,6 +20,14 @@ type Scope struct { // or nil if the "self" object should not be available at all. SelfAddr addrs.Referenceable + // SourceAddr is the address of the source item for the scope. This will + // affect any scoped resources that can be accessed from within this scope. + // + // If nil, access is assumed to be at the module level. So, in practice this + // only needs to be set for items that should be able to access something + // hidden in their own scope. + SourceAddr addrs.Referenceable + // BaseDir is the base directory used by any interpolation functions that // accept filesystem paths as arguments. BaseDir string diff --git a/internal/terraform/context_eval.go b/internal/terraform/context_eval.go index f9d0f649338b..29875bec3843 100644 --- a/internal/terraform/context_eval.go +++ b/internal/terraform/context_eval.go @@ -92,5 +92,5 @@ func (c *Context) Eval(config *configs.Config, state *states.State, moduleAddr a // caches its contexts, so we should get hold of the context that was // previously used for evaluation here, unless we skipped walking. evalCtx := walker.EnterPath(moduleAddr) - return evalCtx.EvaluationScope(nil, EvalDataForNoInstanceKey), diags + return evalCtx.EvaluationScope(nil, nil, EvalDataForNoInstanceKey), diags } diff --git a/internal/terraform/eval_conditions.go b/internal/terraform/eval_conditions.go index 58877011ce5c..3f2e3352fb9c 100644 --- a/internal/terraform/eval_conditions.go +++ b/internal/terraform/eval_conditions.go @@ -87,7 +87,7 @@ func evalCheckRule(typ addrs.CheckType, rule *configs.CheckRule, ctx EvalContext panic(fmt.Sprintf("Invalid self reference type %t", self)) } } - scope := ctx.EvaluationScope(selfReference, keyData) + scope := ctx.EvaluationScope(selfReference, nil, keyData) hclCtx, moreDiags := scope.EvalContext(refs) diags = diags.Append(moreDiags) diff --git a/internal/terraform/eval_context.go b/internal/terraform/eval_context.go index fedf223051ab..61320ef32f07 100644 --- a/internal/terraform/eval_context.go +++ b/internal/terraform/eval_context.go @@ -125,7 +125,7 @@ type EvalContext interface { // EvaluationScope returns a scope that can be used to evaluate reference // addresses in this context. - EvaluationScope(self addrs.Referenceable, keyData InstanceKeyEvalData) *lang.Scope + EvaluationScope(self addrs.Referenceable, source addrs.Referenceable, keyData InstanceKeyEvalData) *lang.Scope // SetRootModuleArgument defines the value for one variable of the root // module. The caller must ensure that given value is a suitable diff --git a/internal/terraform/eval_context_builtin.go b/internal/terraform/eval_context_builtin.go index d66b9dd08060..2953e008b605 100644 --- a/internal/terraform/eval_context_builtin.go +++ b/internal/terraform/eval_context_builtin.go @@ -270,7 +270,7 @@ func (ctx *BuiltinEvalContext) CloseProvisioners() error { func (ctx *BuiltinEvalContext) EvaluateBlock(body hcl.Body, schema *configschema.Block, self addrs.Referenceable, keyData InstanceKeyEvalData) (cty.Value, hcl.Body, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics - scope := ctx.EvaluationScope(self, keyData) + scope := ctx.EvaluationScope(self, nil, keyData) body, evalDiags := scope.ExpandBlock(body, schema) diags = diags.Append(evalDiags) val, evalDiags := scope.EvalBlock(body, schema) @@ -279,7 +279,7 @@ func (ctx *BuiltinEvalContext) EvaluateBlock(body hcl.Body, schema *configschema } func (ctx *BuiltinEvalContext) EvaluateExpr(expr hcl.Expression, wantType cty.Type, self addrs.Referenceable) (cty.Value, tfdiags.Diagnostics) { - scope := ctx.EvaluationScope(self, EvalDataForNoInstanceKey) + scope := ctx.EvaluationScope(self, nil, EvalDataForNoInstanceKey) return scope.EvalExpr(expr, wantType) } @@ -397,7 +397,7 @@ func (ctx *BuiltinEvalContext) EvaluateReplaceTriggeredBy(expr hcl.Expression, r return ref, replace, diags } -func (ctx *BuiltinEvalContext) EvaluationScope(self addrs.Referenceable, keyData instances.RepetitionData) *lang.Scope { +func (ctx *BuiltinEvalContext) EvaluationScope(self addrs.Referenceable, source addrs.Referenceable, keyData InstanceKeyEvalData) *lang.Scope { if !ctx.pathSet { panic("context path not set") } @@ -407,7 +407,7 @@ func (ctx *BuiltinEvalContext) EvaluationScope(self addrs.Referenceable, keyData InstanceKeyData: keyData, Operation: ctx.Evaluator.Operation, } - scope := ctx.Evaluator.Scope(data, self) + scope := ctx.Evaluator.Scope(data, self, source) // ctx.PathValue is the path of the module that contains whatever // expression the caller will be trying to evaluate, so this will diff --git a/internal/terraform/eval_context_mock.go b/internal/terraform/eval_context_mock.go index 24159ef95502..23f0ea8648e1 100644 --- a/internal/terraform/eval_context_mock.go +++ b/internal/terraform/eval_context_mock.go @@ -319,7 +319,7 @@ func (c *MockEvalContext) installSimpleEval() { } } -func (c *MockEvalContext) EvaluationScope(self addrs.Referenceable, keyData InstanceKeyEvalData) *lang.Scope { +func (c *MockEvalContext) EvaluationScope(self addrs.Referenceable, source addrs.Referenceable, keyData InstanceKeyEvalData) *lang.Scope { c.EvaluationScopeCalled = true c.EvaluationScopeSelf = self c.EvaluationScopeKeyData = keyData diff --git a/internal/terraform/eval_for_each.go b/internal/terraform/eval_for_each.go index 3c80ebff01d3..9e89439a31b2 100644 --- a/internal/terraform/eval_for_each.go +++ b/internal/terraform/eval_for_each.go @@ -43,7 +43,7 @@ func evaluateForEachExpressionValue(expr hcl.Expression, ctx EvalContext, allowU refs, moreDiags := lang.ReferencesInExpr(expr) diags = diags.Append(moreDiags) - scope := ctx.EvaluationScope(nil, EvalDataForNoInstanceKey) + scope := ctx.EvaluationScope(nil, nil, EvalDataForNoInstanceKey) var hclCtx *hcl.EvalContext if scope != nil { hclCtx, moreDiags = scope.EvalContext(refs) diff --git a/internal/terraform/eval_variable.go b/internal/terraform/eval_variable.go index 8878886383e4..bd98698a1b8c 100644 --- a/internal/terraform/eval_variable.go +++ b/internal/terraform/eval_variable.go @@ -223,7 +223,7 @@ func evalVariableValidations(addr addrs.AbsInputVariableInstance, config *config config.Name: val, }), }, - Functions: ctx.EvaluationScope(nil, EvalDataForNoInstanceKey).Functions(), + Functions: ctx.EvaluationScope(nil, nil, EvalDataForNoInstanceKey).Functions(), } for _, validation := range config.Validations { diff --git a/internal/terraform/evaluate.go b/internal/terraform/evaluate.go index d680136d8dcd..0beb29b21b53 100644 --- a/internal/terraform/evaluate.go +++ b/internal/terraform/evaluate.go @@ -68,12 +68,13 @@ type Evaluator struct { // If the "self" argument is nil then the "self" object is not available // in evaluated expressions. Otherwise, it behaves as an alias for the given // address. -func (e *Evaluator) Scope(data lang.Data, self addrs.Referenceable) *lang.Scope { +func (e *Evaluator) Scope(data lang.Data, self addrs.Referenceable, source addrs.Referenceable) *lang.Scope { return &lang.Scope{ - Data: data, - SelfAddr: self, - PureOnly: e.Operation != walkApply && e.Operation != walkDestroy && e.Operation != walkEval, - BaseDir: ".", // Always current working directory for now. + Data: data, + SelfAddr: self, + SourceAddr: source, + PureOnly: e.Operation != walkApply && e.Operation != walkDestroy && e.Operation != walkEval, + BaseDir: ".", // Always current working directory for now. } } diff --git a/internal/terraform/evaluate_test.go b/internal/terraform/evaluate_test.go index 765efded6859..87ab4ca73cbc 100644 --- a/internal/terraform/evaluate_test.go +++ b/internal/terraform/evaluate_test.go @@ -25,7 +25,7 @@ func TestEvaluatorGetTerraformAttr(t *testing.T) { data := &evaluationStateData{ Evaluator: evaluator, } - scope := evaluator.Scope(data, nil) + scope := evaluator.Scope(data, nil, nil) t.Run("workspace", func(t *testing.T) { want := cty.StringVal("foo") @@ -55,7 +55,7 @@ func TestEvaluatorGetPathAttr(t *testing.T) { data := &evaluationStateData{ Evaluator: evaluator, } - scope := evaluator.Scope(data, nil) + scope := evaluator.Scope(data, nil, nil) t.Run("module", func(t *testing.T) { want := cty.StringVal("bar/baz") @@ -124,7 +124,7 @@ func TestEvaluatorGetInputVariable(t *testing.T) { data := &evaluationStateData{ Evaluator: evaluator, } - scope := evaluator.Scope(data, nil) + scope := evaluator.Scope(data, nil, nil) want := cty.StringVal("bar").Mark(marks.Sensitive) got, diags := scope.Data.GetInputVariable(addrs.InputVariable{ @@ -273,7 +273,7 @@ func TestEvaluatorGetResource(t *testing.T) { data := &evaluationStateData{ Evaluator: evaluator, } - scope := evaluator.Scope(data, nil) + scope := evaluator.Scope(data, nil, nil) want := cty.ObjectVal(map[string]cty.Value{ "id": cty.StringVal("foo"), @@ -438,7 +438,7 @@ func TestEvaluatorGetResource_changes(t *testing.T) { data := &evaluationStateData{ Evaluator: evaluator, } - scope := evaluator.Scope(data, nil) + scope := evaluator.Scope(data, nil, nil) want := cty.ObjectVal(map[string]cty.Value{ "id": cty.StringVal("foo"), @@ -473,7 +473,7 @@ func TestEvaluatorGetModule(t *testing.T) { data := &evaluationStateData{ Evaluator: evaluator, } - scope := evaluator.Scope(data, nil) + scope := evaluator.Scope(data, nil, nil) want := cty.ObjectVal(map[string]cty.Value{"out": cty.StringVal("bar").Mark(marks.Sensitive)}) got, diags := scope.Data.GetModule(addrs.ModuleCall{ Name: "mod", @@ -501,7 +501,7 @@ func TestEvaluatorGetModule(t *testing.T) { data = &evaluationStateData{ Evaluator: evaluator, } - scope = evaluator.Scope(data, nil) + scope = evaluator.Scope(data, nil, nil) want = cty.ObjectVal(map[string]cty.Value{"out": cty.StringVal("baz").Mark(marks.Sensitive)}) got, diags = scope.Data.GetModule(addrs.ModuleCall{ Name: "mod", @@ -519,7 +519,7 @@ func TestEvaluatorGetModule(t *testing.T) { data = &evaluationStateData{ Evaluator: evaluator, } - scope = evaluator.Scope(data, nil) + scope = evaluator.Scope(data, nil, nil) want = cty.ObjectVal(map[string]cty.Value{"out": cty.StringVal("baz").Mark(marks.Sensitive)}) got, diags = scope.Data.GetModule(addrs.ModuleCall{ Name: "mod", diff --git a/internal/terraform/evaluate_valid.go b/internal/terraform/evaluate_valid.go index 1d43cc4fce64..30331a756a6b 100644 --- a/internal/terraform/evaluate_valid.go +++ b/internal/terraform/evaluate_valid.go @@ -28,16 +28,16 @@ import ( // // The result may include warning diagnostics if, for example, deprecated // features are referenced. -func (d *evaluationStateData) StaticValidateReferences(refs []*addrs.Reference, self addrs.Referenceable) tfdiags.Diagnostics { +func (d *evaluationStateData) StaticValidateReferences(refs []*addrs.Reference, self addrs.Referenceable, source addrs.Referenceable) tfdiags.Diagnostics { var diags tfdiags.Diagnostics for _, ref := range refs { - moreDiags := d.staticValidateReference(ref, self) + moreDiags := d.staticValidateReference(ref, self, source) diags = diags.Append(moreDiags) } return diags } -func (d *evaluationStateData) staticValidateReference(ref *addrs.Reference, self addrs.Referenceable) tfdiags.Diagnostics { +func (d *evaluationStateData) staticValidateReference(ref *addrs.Reference, self addrs.Referenceable, source addrs.Referenceable) tfdiags.Diagnostics { modCfg := d.Evaluator.Config.DescendentForInstance(d.ModulePath) if modCfg == nil { // This is a bug in the caller rather than a problem with the @@ -78,12 +78,12 @@ func (d *evaluationStateData) staticValidateReference(ref *addrs.Reference, self case addrs.Resource: var diags tfdiags.Diagnostics diags = diags.Append(d.staticValidateSingleResourceReference(modCfg, addr, ref.Remaining, ref.SourceRange)) - diags = diags.Append(d.staticValidateResourceReference(modCfg, addr, ref.Remaining, ref.SourceRange)) + diags = diags.Append(d.staticValidateResourceReference(modCfg, addr, source, ref.Remaining, ref.SourceRange)) return diags case addrs.ResourceInstance: var diags tfdiags.Diagnostics diags = diags.Append(d.staticValidateMultiResourceReference(modCfg, addr, ref.Remaining, ref.SourceRange)) - diags = diags.Append(d.staticValidateResourceReference(modCfg, addr.ContainingResource(), ref.Remaining, ref.SourceRange)) + diags = diags.Append(d.staticValidateResourceReference(modCfg, addr.ContainingResource(), source, ref.Remaining, ref.SourceRange)) return diags // We also handle all module call references the same way, disregarding index. @@ -187,7 +187,7 @@ func (d *evaluationStateData) staticValidateMultiResourceReference(modCfg *confi return diags } -func (d *evaluationStateData) staticValidateResourceReference(modCfg *configs.Config, addr addrs.Resource, remain hcl.Traversal, rng tfdiags.SourceRange) tfdiags.Diagnostics { +func (d *evaluationStateData) staticValidateResourceReference(modCfg *configs.Config, addr addrs.Resource, source addrs.Referenceable, remain hcl.Traversal, rng tfdiags.SourceRange) tfdiags.Diagnostics { var diags tfdiags.Diagnostics var modeAdjective string @@ -223,6 +223,15 @@ func (d *evaluationStateData) staticValidateResourceReference(modCfg *configs.Co return diags } + if cfg.Container != nil && (source == nil || !cfg.Container.Accessible(source)) { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: `Reference to scoped resource`, + Detail: fmt.Sprintf(`The referenced %s resource %q %q is not available from this context.`, modeAdjective, addr.Type, addr.Name), + Subject: rng.ToHCL().Ptr(), + }) + } + providerFqn := modCfg.Module.ProviderForLocalConfig(cfg.ProviderConfigAddr()) schema, _, err := d.Evaluator.Plugins.ResourceTypeSchema(providerFqn, addr.Mode, addr.Type) if err != nil { diff --git a/internal/terraform/evaluate_valid_test.go b/internal/terraform/evaluate_valid_test.go index cfdfdea1f5e1..920bdbef2d7b 100644 --- a/internal/terraform/evaluate_valid_test.go +++ b/internal/terraform/evaluate_valid_test.go @@ -100,7 +100,7 @@ For example, to correlate with indices of a referring resource, use: Evaluator: evaluator, } - diags = data.StaticValidateReferences(refs, nil) + diags = data.StaticValidateReferences(refs, nil, nil) if diags.HasErrors() { if test.WantErr == "" { t.Fatalf("Unexpected diagnostics: %s", diags.Err()) diff --git a/internal/terraform/node_module_variable.go b/internal/terraform/node_module_variable.go index 6d5ae2af89cb..429529808650 100644 --- a/internal/terraform/node_module_variable.go +++ b/internal/terraform/node_module_variable.go @@ -214,7 +214,7 @@ func (n *nodeModuleVariable) evalModuleVariable(ctx EvalContext, validateOnly bo moduleInstanceRepetitionData = ctx.InstanceExpander().GetModuleInstanceRepetitionData(n.ModuleInstance) } - scope := ctx.EvaluationScope(nil, moduleInstanceRepetitionData) + scope := ctx.EvaluationScope(nil, nil, moduleInstanceRepetitionData) val, moreDiags := scope.EvalExpr(expr, cty.DynamicPseudoType) diags = diags.Append(moreDiags) if moreDiags.HasErrors() { diff --git a/internal/terraform/node_resource_abstract_instance.go b/internal/terraform/node_resource_abstract_instance.go index 4dabb7bedb5c..709298ee519a 100644 --- a/internal/terraform/node_resource_abstract_instance.go +++ b/internal/terraform/node_resource_abstract_instance.go @@ -2049,7 +2049,7 @@ func (n *NodeAbstractResourceInstance) evalDestroyProvisionerConfig(ctx EvalCont // destroy-time provisioners. keyData := EvalDataForInstanceKey(n.ResourceInstanceAddr().Resource.Key, nil) - evalScope := ctx.EvaluationScope(n.ResourceInstanceAddr().Resource, keyData) + evalScope := ctx.EvaluationScope(n.ResourceInstanceAddr().Resource, nil, keyData) config, evalDiags := evalScope.EvalSelfBlock(body, self, schema, keyData) diags = diags.Append(evalDiags) diff --git a/internal/terraform/node_resource_validate.go b/internal/terraform/node_resource_validate.go index a70bcdc4bc85..b0ea8c70a1f8 100644 --- a/internal/terraform/node_resource_validate.go +++ b/internal/terraform/node_resource_validate.go @@ -465,7 +465,7 @@ func (n *NodeValidatableResource) evaluateExpr(ctx EvalContext, expr hcl.Express refs, refDiags := lang.ReferencesInExpr(expr) diags = diags.Append(refDiags) - scope := ctx.EvaluationScope(self, keyData) + scope := ctx.EvaluationScope(self, nil, keyData) hclCtx, moreDiags := scope.EvalContext(refs) diags = diags.Append(moreDiags) @@ -581,7 +581,7 @@ func validateDependsOn(ctx EvalContext, dependsOn []hcl.Traversal) (diags tfdiag // we'll just eval it and count on the fact that our evaluator will // detect references to non-existent objects. if !diags.HasErrors() { - scope := ctx.EvaluationScope(nil, EvalDataForNoInstanceKey) + scope := ctx.EvaluationScope(nil, nil, EvalDataForNoInstanceKey) if scope != nil { // sometimes nil in tests, due to incomplete mocks _, refDiags = scope.EvalReference(ref, cty.DynamicPseudoType) diags = diags.Append(refDiags) From 0ecb54c565959914c6be9c079d334d73d1987f47 Mon Sep 17 00:00:00 2001 From: Liam Cervante Date: Fri, 17 Feb 2023 13:31:12 +0100 Subject: [PATCH 2/8] refactor existing checks addrs and add check block addr --- internal/addrs/check.go | 310 +++++++------------------ internal/addrs/check_rule.go | 105 +++++++++ internal/addrs/checkable.go | 182 +++++++++++++++ internal/addrs/checkablekind_string.go | 14 +- internal/addrs/checkruletype_string.go | 28 +++ internal/addrs/checktype_string.go | 26 --- internal/addrs/output_value.go | 5 +- internal/addrs/resource.go | 4 +- internal/checks/state.go | 8 +- internal/checks/state_init.go | 4 +- internal/checks/state_report.go | 14 +- internal/terraform/eval_conditions.go | 4 +- 12 files changed, 427 insertions(+), 277 deletions(-) create mode 100644 internal/addrs/check_rule.go create mode 100644 internal/addrs/checkable.go create mode 100644 internal/addrs/checkruletype_string.go delete mode 100644 internal/addrs/checktype_string.go diff --git a/internal/addrs/check.go b/internal/addrs/check.go index 430b50c990d8..fb5655479343 100644 --- a/internal/addrs/check.go +++ b/internal/addrs/check.go @@ -1,251 +1,107 @@ package addrs -import ( - "fmt" - - "github.com/hashicorp/hcl/v2" - "github.com/hashicorp/hcl/v2/hclsyntax" - "github.com/hashicorp/terraform/internal/tfdiags" -) - -// Check is the address of a check rule within a checkable object. -// -// This represents the check rule globally within a configuration, and is used -// during graph evaluation to identify a condition result object to update with -// the result of check rule evaluation. -// -// The check address is not distinct from resource traversals, and check rule -// values are not intended to be available to the language, so the address is -// not Referenceable. -// -// Note also that the check address is only relevant within the scope of a run, -// as reordering check blocks between runs will result in their addresses -// changing. Check is therefore for internal use only and should not be exposed -// in durable artifacts such as state snapshots. +import "fmt" + type Check struct { - Container Checkable - Type CheckType - Index int + referenceable + Name string } -func NewCheck(container Checkable, typ CheckType, index int) Check { - return Check{ - Container: container, - Type: typ, - Index: index, +func (c Check) String() string { + return fmt.Sprintf("check.%s", c.Name) +} + +func (c Check) InModule(modAddr Module) ConfigCheck { + return ConfigCheck{ + Module: modAddr, + Check: c, } } -func (c Check) String() string { - container := c.Container.String() - switch c.Type { - case ResourcePrecondition: - return fmt.Sprintf("%s.precondition[%d]", container, c.Index) - case ResourcePostcondition: - return fmt.Sprintf("%s.postcondition[%d]", container, c.Index) - case OutputPrecondition: - return fmt.Sprintf("%s.precondition[%d]", container, c.Index) - default: - // This should not happen - return fmt.Sprintf("%s.condition[%d]", container, c.Index) +func (c Check) Absolute(modAddr ModuleInstance) AbsCheck { + return AbsCheck{ + Module: modAddr, + Check: c, } } +func (c Check) Equal(o Check) bool { + return c.Name == o.Name +} + func (c Check) UniqueKey() UniqueKey { - return checkKey{ - ContainerKey: c.Container.UniqueKey(), - Type: c.Type, - Index: c.Index, - } + return c // A Check is its own UniqueKey } -type checkKey struct { - ContainerKey UniqueKey - Type CheckType - Index int -} - -func (k checkKey) uniqueKeySigil() {} - -// CheckType describes a category of check. We use this only to establish -// uniqueness for Check values, and do not expose this concept of "check types" -// (which is subject to change in future) in any durable artifacts such as -// state snapshots. -// -// (See [CheckableKind] for an enumeration that we _do_ use externally, to -// describe the type of object being checked rather than the type of the check -// itself.) -type CheckType int - -//go:generate go run golang.org/x/tools/cmd/stringer -type=CheckType check.go - -const ( - InvalidCondition CheckType = 0 - ResourcePrecondition CheckType = 1 - ResourcePostcondition CheckType = 2 - OutputPrecondition CheckType = 3 -) - -// Description returns a human-readable description of the check type. This is -// presented in the user interface through a diagnostic summary. -func (c CheckType) Description() string { - switch c { - case ResourcePrecondition: - return "Resource precondition" - case ResourcePostcondition: - return "Resource postcondition" - case OutputPrecondition: - return "Module output value precondition" - default: - // This should not happen - return "Condition" - } +func (c Check) uniqueKeySigil() {} + +type ConfigCheck struct { + Module Module + Check Check +} + +var _ ConfigCheckable = ConfigCheck{} + +func (c ConfigCheck) UniqueKey() UniqueKey { + return configCheckUniqueKey(c.String()) +} + +func (c ConfigCheck) configCheckableSigil() {} + +func (c ConfigCheck) CheckableKind() CheckableKind { + return CheckableCheck } -// Checkable is an interface implemented by all address types that can contain -// condition blocks. -type Checkable interface { - UniqueKeyer - - checkableSigil() - - // Check returns the address of an individual check rule of a specified - // type and index within this checkable container. - Check(CheckType, int) Check - - // ConfigCheckable returns the address of the configuration construct that - // this Checkable belongs to. - // - // Checkable objects can potentially be dynamically declared during a - // plan operation using constructs like resource for_each, and so - // ConfigCheckable gives us a way to talk about the static containers - // those dynamic objects belong to, in case we wish to group together - // dynamic checkable objects into their static checkable for reporting - // purposes. - ConfigCheckable() ConfigCheckable - - CheckableKind() CheckableKind - String() string -} - -var ( - _ Checkable = AbsResourceInstance{} - _ Checkable = AbsOutputValue{} -) - -// CheckableKind describes the different kinds of checkable objects. -type CheckableKind rune - -//go:generate go run golang.org/x/tools/cmd/stringer -type=CheckableKind check.go - -const ( - CheckableKindInvalid CheckableKind = 0 - CheckableResource CheckableKind = 'R' - CheckableOutputValue CheckableKind = 'O' -) - -// ConfigCheckable is an interfaces implemented by address types that represent -// configuration constructs that can have Checkable addresses associated with -// them. -// -// This address type therefore in a sense represents a container for zero or -// more checkable objects all declared by the same configuration construct, -// so that we can talk about these groups of checkable objects before we're -// ready to decide how many checkable objects belong to each one. -type ConfigCheckable interface { - UniqueKeyer - - configCheckableSigil() - - CheckableKind() CheckableKind - String() string -} - -var ( - _ ConfigCheckable = ConfigResource{} - _ ConfigCheckable = ConfigOutputValue{} -) - -// ParseCheckableStr attempts to parse the given string as a Checkable address -// of the given kind. -// -// This should be the opposite of Checkable.String for any Checkable address -// type, as long as "kind" is set to the value returned by the address's -// CheckableKind method. -// -// We do not typically expect users to write out checkable addresses as input, -// but we use them as part of some of our wire formats for persisting check -// results between runs. -func ParseCheckableStr(kind CheckableKind, src string) (Checkable, tfdiags.Diagnostics) { - var diags tfdiags.Diagnostics - - traversal, parseDiags := hclsyntax.ParseTraversalAbs([]byte(src), "", hcl.InitialPos) - diags = diags.Append(parseDiags) - if parseDiags.HasErrors() { - return nil, diags +func (c ConfigCheck) String() string { + if len(c.Module) == 0 { + return c.Check.String() } + return fmt.Sprintf("%s.%s", c.Module, c.Check) +} + +type AbsCheck struct { + Module ModuleInstance + Check Check +} + +var _ Checkable = AbsCheck{} - path, remain, diags := parseModuleInstancePrefix(traversal) - if diags.HasErrors() { - return nil, diags +func (c AbsCheck) UniqueKey() UniqueKey { + return absCheckUniqueKey(c.String()) +} + +func (c AbsCheck) checkableSigil() {} + +func (c AbsCheck) CheckRule(typ CheckRuleType, i int) CheckRule { + return CheckRule{ + Container: c, + Type: typ, + Index: i, } +} - if remain.IsRelative() { - // (relative means that there's either nothing left or what's next isn't an identifier) - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid checkable address", - Detail: "Module path must be followed by either a resource instance address or an output value address.", - Subject: remain.SourceRange().Ptr(), - }) - return nil, diags +func (c AbsCheck) ConfigCheckable() ConfigCheckable { + return ConfigCheck{ + Module: c.Module.Module(), + Check: c.Check, } +} + +func (c AbsCheck) CheckableKind() CheckableKind { + return CheckableCheck +} - // We use "kind" to disambiguate here because unfortunately we've - // historically never reserved "output" as a possible resource type name - // and so it is in principle possible -- albeit unlikely -- that there - // might be a resource whose type is literally "output". - switch kind { - case CheckableResource: - riAddr, moreDiags := parseResourceInstanceUnderModule(path, remain) - diags = diags.Append(moreDiags) - if diags.HasErrors() { - return nil, diags - } - return riAddr, diags - - case CheckableOutputValue: - if len(remain) != 2 { - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid checkable address", - Detail: "Output address must have only one attribute part after the keyword 'output', giving the name of the output value.", - Subject: remain.SourceRange().Ptr(), - }) - return nil, diags - } - if remain.RootName() != "output" { - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid checkable address", - Detail: "Output address must follow the module address with the keyword 'output'.", - Subject: remain.SourceRange().Ptr(), - }) - return nil, diags - } - if step, ok := remain[1].(hcl.TraverseAttr); !ok { - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid checkable address", - Detail: "Output address must have only one attribute part after the keyword 'output', giving the name of the output value.", - Subject: remain.SourceRange().Ptr(), - }) - return nil, diags - } else { - return OutputValue{Name: step.Name}.Absolute(path), diags - } - - default: - panic(fmt.Sprintf("unsupported CheckableKind %s", kind)) +func (c AbsCheck) String() string { + if len(c.Module) == 0 { + return c.Check.String() } + return fmt.Sprintf("%s.%s", c.Module, c.Check) } + +type configCheckUniqueKey string + +func (k configCheckUniqueKey) uniqueKeySigil() {} + +type absCheckUniqueKey string + +func (k absCheckUniqueKey) uniqueKeySigil() {} diff --git a/internal/addrs/check_rule.go b/internal/addrs/check_rule.go new file mode 100644 index 000000000000..a096e7e4b6c6 --- /dev/null +++ b/internal/addrs/check_rule.go @@ -0,0 +1,105 @@ +package addrs + +import ( + "fmt" +) + +// CheckRule is the address of a check rule within a checkable object. +// +// This represents the check rule globally within a configuration, and is used +// during graph evaluation to identify a condition result object to update with +// the result of check rule evaluation. +// +// The check address is not distinct from resource traversals, and check rule +// values are not intended to be available to the language, so the address is +// not Referenceable. +// +// Note also that the check address is only relevant within the scope of a run, +// as reordering check blocks between runs will result in their addresses +// changing. CheckRule is therefore for internal use only and should not be +// exposed in durable artifacts such as state snapshots. +type CheckRule struct { + Container Checkable + Type CheckRuleType + Index int +} + +func NewCheckRule(container Checkable, typ CheckRuleType, index int) CheckRule { + return CheckRule{ + Container: container, + Type: typ, + Index: index, + } +} + +func (c CheckRule) String() string { + container := c.Container.String() + switch c.Type { + case ResourcePrecondition: + return fmt.Sprintf("%s.precondition[%d]", container, c.Index) + case ResourcePostcondition: + return fmt.Sprintf("%s.postcondition[%d]", container, c.Index) + case OutputPrecondition: + return fmt.Sprintf("%s.precondition[%d]", container, c.Index) + default: + // This should not happen + return fmt.Sprintf("%s.condition[%d]", container, c.Index) + } +} + +func (c CheckRule) UniqueKey() UniqueKey { + return checkRuleKey{ + ContainerKey: c.Container.UniqueKey(), + Type: c.Type, + Index: c.Index, + } +} + +type checkRuleKey struct { + ContainerKey UniqueKey + Type CheckRuleType + Index int +} + +func (k checkRuleKey) uniqueKeySigil() {} + +// CheckRuleType describes a category of check. We use this only to establish +// uniqueness for Check values, and do not expose this concept of "check types" +// (which is subject to change in future) in any durable artifacts such as +// state snapshots. +// +// (See [CheckableKind] for an enumeration that we _do_ use externally, to +// describe the type of object being checked rather than the type of the check +// itself.) +type CheckRuleType int + +//go:generate go run golang.org/x/tools/cmd/stringer -type=CheckRuleType check_rule.go + +const ( + InvalidCondition CheckRuleType = 0 + ResourcePrecondition CheckRuleType = 1 + ResourcePostcondition CheckRuleType = 2 + OutputPrecondition CheckRuleType = 3 + CheckDataResource CheckRuleType = 4 + CheckAssertion CheckRuleType = 5 +) + +// Description returns a human-readable description of the check type. This is +// presented in the user interface through a diagnostic summary. +func (c CheckRuleType) Description() string { + switch c { + case ResourcePrecondition: + return "Resource precondition" + case ResourcePostcondition: + return "Resource postcondition" + case OutputPrecondition: + return "Module output value precondition" + case CheckDataResource: + return "Check block data resource" + case CheckAssertion: + return "Check block assertion" + default: + // This should not happen + return "Condition" + } +} diff --git a/internal/addrs/checkable.go b/internal/addrs/checkable.go new file mode 100644 index 000000000000..c8de300f66ca --- /dev/null +++ b/internal/addrs/checkable.go @@ -0,0 +1,182 @@ +package addrs + +import ( + "fmt" + + "golang.org/x/text/cases" + "golang.org/x/text/language" + + "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/hcl/v2/hclsyntax" + + "github.com/hashicorp/terraform/internal/tfdiags" +) + +// Checkable is an interface implemented by all address types that can contain +// condition blocks. +type Checkable interface { + UniqueKeyer + + checkableSigil() + + // CheckRule returns the address of an individual check rule of a specified + // type and index within this checkable container. + CheckRule(CheckRuleType, int) CheckRule + + // ConfigCheckable returns the address of the configuration construct that + // this Checkable belongs to. + // + // Checkable objects can potentially be dynamically declared during a + // plan operation using constructs like resource for_each, and so + // ConfigCheckable gives us a way to talk about the static containers + // those dynamic objects belong to, in case we wish to group together + // dynamic checkable objects into their static checkable for reporting + // purposes. + ConfigCheckable() ConfigCheckable + + CheckableKind() CheckableKind + String() string +} + +var ( + _ Checkable = AbsResourceInstance{} + _ Checkable = AbsOutputValue{} +) + +// CheckableKind describes the different kinds of checkable objects. +type CheckableKind rune + +//go:generate go run golang.org/x/tools/cmd/stringer -type=CheckableKind checkable.go + +const ( + CheckableKindInvalid CheckableKind = 0 + CheckableResource CheckableKind = 'R' + CheckableOutputValue CheckableKind = 'O' + CheckableCheck CheckableKind = 'C' +) + +// ConfigCheckable is an interfaces implemented by address types that represent +// configuration constructs that can have Checkable addresses associated with +// them. +// +// This address type therefore in a sense represents a container for zero or +// more checkable objects all declared by the same configuration construct, +// so that we can talk about these groups of checkable objects before we're +// ready to decide how many checkable objects belong to each one. +type ConfigCheckable interface { + UniqueKeyer + + configCheckableSigil() + + CheckableKind() CheckableKind + String() string +} + +var ( + _ ConfigCheckable = ConfigResource{} + _ ConfigCheckable = ConfigOutputValue{} +) + +// ParseCheckableStr attempts to parse the given string as a Checkable address +// of the given kind. +// +// This should be the opposite of Checkable.String for any Checkable address +// type, as long as "kind" is set to the value returned by the address's +// CheckableKind method. +// +// We do not typically expect users to write out checkable addresses as input, +// but we use them as part of some of our wire formats for persisting check +// results between runs. +func ParseCheckableStr(kind CheckableKind, src string) (Checkable, tfdiags.Diagnostics) { + var diags tfdiags.Diagnostics + + traversal, parseDiags := hclsyntax.ParseTraversalAbs([]byte(src), "", hcl.InitialPos) + diags = diags.Append(parseDiags) + if parseDiags.HasErrors() { + return nil, diags + } + + path, remain, diags := parseModuleInstancePrefix(traversal) + if diags.HasErrors() { + return nil, diags + } + + if remain.IsRelative() { + // (relative means that there's either nothing left or what's next isn't an identifier) + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid checkable address", + Detail: "Module path must be followed by either a resource instance address or an output value address.", + Subject: remain.SourceRange().Ptr(), + }) + return nil, diags + } + + getCheckableName := func(keyword string, descriptor string) (string, tfdiags.Diagnostics) { + var diags tfdiags.Diagnostics + var name string + + if len(remain) != 2 { + diags = diags.Append(hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid checkable address", + Detail: fmt.Sprintf("%s address must have only one attribute part after the keyword '%s', giving the name of the %s.", cases.Title(language.English, cases.NoLower).String(keyword), keyword, descriptor), + Subject: remain.SourceRange().Ptr(), + }) + } + + if remain.RootName() != keyword { + diags = diags.Append(hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid checkable address", + Detail: fmt.Sprintf("%s address must follow the module address with the keyword '%s'.", cases.Title(language.English, cases.NoLower).String(keyword), keyword), + Subject: remain.SourceRange().Ptr(), + }) + } + if step, ok := remain[1].(hcl.TraverseAttr); !ok { + diags = diags.Append(hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid checkable address", + Detail: fmt.Sprintf("%s address must have only one attribute part after the keyword '%s', giving the name of the %s.", cases.Title(language.English, cases.NoLower).String(keyword), keyword, descriptor), + Subject: remain.SourceRange().Ptr(), + }) + } else { + name = step.Name + } + + return name, diags + } + + // We use "kind" to disambiguate here because unfortunately we've + // historically never reserved "output" as a possible resource type name + // and so it is in principle possible -- albeit unlikely -- that there + // might be a resource whose type is literally "output". + switch kind { + case CheckableResource: + riAddr, moreDiags := parseResourceInstanceUnderModule(path, remain) + diags = diags.Append(moreDiags) + if diags.HasErrors() { + return nil, diags + } + return riAddr, diags + + case CheckableOutputValue: + name, nameDiags := getCheckableName("output", "output value") + diags = diags.Append(nameDiags) + if diags.HasErrors() { + return nil, diags + } + return OutputValue{Name: name}.Absolute(path), diags + + case CheckableCheck: + name, nameDiags := getCheckableName("check", "check block") + diags = diags.Append(nameDiags) + if diags.HasErrors() { + return nil, diags + } + return Check{Name: name}.Absolute(path), diags + + default: + panic(fmt.Sprintf("unsupported CheckableKind %s", kind)) + } +} diff --git a/internal/addrs/checkablekind_string.go b/internal/addrs/checkablekind_string.go index 8987cd02bff1..e0657cb6a1f6 100644 --- a/internal/addrs/checkablekind_string.go +++ b/internal/addrs/checkablekind_string.go @@ -1,4 +1,4 @@ -// Code generated by "stringer -type=CheckableKind check.go"; DO NOT EDIT. +// Code generated by "stringer -type=CheckableKind checkable.go"; DO NOT EDIT. package addrs @@ -11,22 +11,26 @@ func _() { _ = x[CheckableKindInvalid-0] _ = x[CheckableResource-82] _ = x[CheckableOutputValue-79] + _ = x[CheckableCheck-67] } const ( _CheckableKind_name_0 = "CheckableKindInvalid" - _CheckableKind_name_1 = "CheckableOutputValue" - _CheckableKind_name_2 = "CheckableResource" + _CheckableKind_name_1 = "CheckableCheck" + _CheckableKind_name_2 = "CheckableOutputValue" + _CheckableKind_name_3 = "CheckableResource" ) func (i CheckableKind) String() string { switch { case i == 0: return _CheckableKind_name_0 - case i == 79: + case i == 67: return _CheckableKind_name_1 - case i == 82: + case i == 79: return _CheckableKind_name_2 + case i == 82: + return _CheckableKind_name_3 default: return "CheckableKind(" + strconv.FormatInt(int64(i), 10) + ")" } diff --git a/internal/addrs/checkruletype_string.go b/internal/addrs/checkruletype_string.go new file mode 100644 index 000000000000..786579366579 --- /dev/null +++ b/internal/addrs/checkruletype_string.go @@ -0,0 +1,28 @@ +// Code generated by "stringer -type=CheckRuleType check_rule.go"; DO NOT EDIT. + +package addrs + +import "strconv" + +func _() { + // An "invalid array index" compiler error signifies that the constant values have changed. + // Re-run the stringer command to generate them again. + var x [1]struct{} + _ = x[InvalidCondition-0] + _ = x[ResourcePrecondition-1] + _ = x[ResourcePostcondition-2] + _ = x[OutputPrecondition-3] + _ = x[CheckDataResource-4] + _ = x[CheckAssertion-5] +} + +const _CheckRuleType_name = "InvalidConditionResourcePreconditionResourcePostconditionOutputPreconditionCheckDataResourceCheckAssertion" + +var _CheckRuleType_index = [...]uint8{0, 16, 36, 57, 75, 92, 106} + +func (i CheckRuleType) String() string { + if i < 0 || i >= CheckRuleType(len(_CheckRuleType_index)-1) { + return "CheckRuleType(" + strconv.FormatInt(int64(i), 10) + ")" + } + return _CheckRuleType_name[_CheckRuleType_index[i]:_CheckRuleType_index[i+1]] +} diff --git a/internal/addrs/checktype_string.go b/internal/addrs/checktype_string.go deleted file mode 100644 index 8c2fcebd4ffa..000000000000 --- a/internal/addrs/checktype_string.go +++ /dev/null @@ -1,26 +0,0 @@ -// Code generated by "stringer -type=CheckType check.go"; DO NOT EDIT. - -package addrs - -import "strconv" - -func _() { - // An "invalid array index" compiler error signifies that the constant values have changed. - // Re-run the stringer command to generate them again. - var x [1]struct{} - _ = x[InvalidCondition-0] - _ = x[ResourcePrecondition-1] - _ = x[ResourcePostcondition-2] - _ = x[OutputPrecondition-3] -} - -const _CheckType_name = "InvalidConditionResourcePreconditionResourcePostconditionOutputPrecondition" - -var _CheckType_index = [...]uint8{0, 16, 36, 57, 75} - -func (i CheckType) String() string { - if i < 0 || i >= CheckType(len(_CheckType_index)-1) { - return "CheckType(" + strconv.FormatInt(int64(i), 10) + ")" - } - return _CheckType_name[_CheckType_index[i]:_CheckType_index[i+1]] -} diff --git a/internal/addrs/output_value.go b/internal/addrs/output_value.go index ff76556b2ac9..3fd7639893df 100644 --- a/internal/addrs/output_value.go +++ b/internal/addrs/output_value.go @@ -5,6 +5,7 @@ import ( "github.com/hashicorp/hcl/v2" "github.com/hashicorp/hcl/v2/hclsyntax" + "github.com/hashicorp/terraform/internal/tfdiags" ) @@ -62,8 +63,8 @@ func (m ModuleInstance) OutputValue(name string) AbsOutputValue { } } -func (v AbsOutputValue) Check(t CheckType, i int) Check { - return Check{ +func (v AbsOutputValue) CheckRule(t CheckRuleType, i int) CheckRule { + return CheckRule{ Container: v, Type: t, Index: i, diff --git a/internal/addrs/resource.go b/internal/addrs/resource.go index bf19b2d2471f..7ef5390c0c34 100644 --- a/internal/addrs/resource.go +++ b/internal/addrs/resource.go @@ -329,8 +329,8 @@ func (r AbsResourceInstance) AffectedAbsResource() AbsResource { } } -func (r AbsResourceInstance) Check(t CheckType, i int) Check { - return Check{ +func (r AbsResourceInstance) CheckRule(t CheckRuleType, i int) CheckRule { + return CheckRule{ Container: r, Type: t, Index: i, diff --git a/internal/checks/state.go b/internal/checks/state.go index 2d9d7e188dba..05b4c65b5454 100644 --- a/internal/checks/state.go +++ b/internal/checks/state.go @@ -33,7 +33,7 @@ type State struct { mu sync.Mutex statuses addrs.Map[addrs.ConfigCheckable, *configCheckableState] - failureMsgs addrs.Map[addrs.Check, string] + failureMsgs addrs.Map[addrs.CheckRule, string] } // configCheckableState is an internal part of type State that represents @@ -53,7 +53,7 @@ type configCheckableState struct { // associated with object declared by this configuration construct. Since // checks are statically declared (even though the checkable objects // aren't) we can compute this only from the configuration. - checkTypes map[addrs.CheckType]int + checkTypes map[addrs.CheckRuleType]int // objects represents the set of dynamic checkable objects associated // with this configuration construct. This is initially nil to represent @@ -64,7 +64,7 @@ type configCheckableState struct { // The leaf Status values will initially be StatusUnknown // and then gradually updated by Terraform Core as it visits the // individual checkable objects and reports their status. - objects addrs.Map[addrs.Checkable, map[addrs.CheckType][]Status] + objects addrs.Map[addrs.Checkable, map[addrs.CheckRuleType][]Status] } // NOTE: For the "Report"-prefixed methods that we use to gradually update @@ -253,7 +253,7 @@ func (c *State) ObjectFailureMessages(addr addrs.Checkable) []string { for checkType, checks := range checksByType { for i, status := range checks { if status == StatusFail { - checkAddr := addrs.NewCheck(addr, checkType, i) + checkAddr := addrs.NewCheckRule(addr, checkType, i) msg := c.failureMsgs.Get(checkAddr) if msg != "" { ret = append(ret, msg) diff --git a/internal/checks/state_init.go b/internal/checks/state_init.go index 6714243b5c3b..a2af43c92453 100644 --- a/internal/checks/state_init.go +++ b/internal/checks/state_init.go @@ -42,7 +42,7 @@ func collectInitialStatuses(into addrs.Map[addrs.ConfigCheckable, *configCheckab st := &configCheckableState{} - st.checkTypes = map[addrs.CheckType]int{ + st.checkTypes = map[addrs.CheckRuleType]int{ addrs.OutputPrecondition: ct, } @@ -63,7 +63,7 @@ func collectInitialStatusForResource(into addrs.Map[addrs.ConfigCheckable, *conf } st := &configCheckableState{ - checkTypes: make(map[addrs.CheckType]int), + checkTypes: make(map[addrs.CheckRuleType]int), } if ct := len(rc.Preconditions); ct > 0 { diff --git a/internal/checks/state_report.go b/internal/checks/state_report.go index ccf35ac13826..d3938c1a0fca 100644 --- a/internal/checks/state_report.go +++ b/internal/checks/state_report.go @@ -32,14 +32,14 @@ func (c *State) ReportCheckableObjects(configAddr addrs.ConfigCheckable, objectA // At this point we pre-populate all of the check results as StatusUnknown, // so that even if we never hear from Terraform Core again we'll still // remember that these results were all pending. - st.objects = addrs.MakeMap[addrs.Checkable, map[addrs.CheckType][]Status]() + st.objects = addrs.MakeMap[addrs.Checkable, map[addrs.CheckRuleType][]Status]() for _, objectAddr := range objectAddrs { if gotConfigAddr := objectAddr.ConfigCheckable(); !addrs.Equivalent(configAddr, gotConfigAddr) { // All of the given object addresses must belong to the specified configuration address panic(fmt.Sprintf("%s belongs to %s, not %s", objectAddr, gotConfigAddr, configAddr)) } - checks := make(map[addrs.CheckType][]Status, len(st.checkTypes)) + checks := make(map[addrs.CheckRuleType][]Status, len(st.checkTypes)) for checkType, count := range st.checkTypes { // NOTE: This is intentionally a slice of count of the zero value // of Status, which is StatusUnknown to represent that we don't @@ -61,7 +61,7 @@ func (c *State) ReportCheckableObjects(configAddr addrs.ConfigCheckable, objectA // // This method will also panic if the specified check already had a known // status; each check should have its result reported only once. -func (c *State) ReportCheckResult(objectAddr addrs.Checkable, checkType addrs.CheckType, index int, status Status) { +func (c *State) ReportCheckResult(objectAddr addrs.Checkable, checkType addrs.CheckRuleType, index int, status Status) { c.mu.Lock() defer c.mu.Unlock() @@ -76,21 +76,21 @@ func (c *State) ReportCheckResult(objectAddr addrs.Checkable, checkType addrs.Ch // situations where the check condition was itself invalid, because that // should be represented by StatusError instead, and the error signalled via // diagnostics as normal. -func (c *State) ReportCheckFailure(objectAddr addrs.Checkable, checkType addrs.CheckType, index int, errorMessage string) { +func (c *State) ReportCheckFailure(objectAddr addrs.Checkable, checkType addrs.CheckRuleType, index int, errorMessage string) { c.mu.Lock() defer c.mu.Unlock() c.reportCheckResult(objectAddr, checkType, index, StatusFail) if c.failureMsgs.Elems == nil { - c.failureMsgs = addrs.MakeMap[addrs.Check, string]() + c.failureMsgs = addrs.MakeMap[addrs.CheckRule, string]() } - checkAddr := addrs.NewCheck(objectAddr, checkType, index) + checkAddr := addrs.NewCheckRule(objectAddr, checkType, index) c.failureMsgs.Put(checkAddr, errorMessage) } // reportCheckResult is shared between both ReportCheckResult and // ReportCheckFailure, and assumes its caller already holds the mutex. -func (c *State) reportCheckResult(objectAddr addrs.Checkable, checkType addrs.CheckType, index int, status Status) { +func (c *State) reportCheckResult(objectAddr addrs.Checkable, checkType addrs.CheckRuleType, index int, status Status) { configAddr := objectAddr.ConfigCheckable() st, ok := c.statuses.GetOk(configAddr) diff --git a/internal/terraform/eval_conditions.go b/internal/terraform/eval_conditions.go index 3f2e3352fb9c..9edd9199be24 100644 --- a/internal/terraform/eval_conditions.go +++ b/internal/terraform/eval_conditions.go @@ -27,7 +27,7 @@ import ( // // If any of the rules do not pass, the returned diagnostics will contain // errors. Otherwise, it will either be empty or contain only warnings. -func evalCheckRules(typ addrs.CheckType, rules []*configs.CheckRule, ctx EvalContext, self addrs.Checkable, keyData instances.RepetitionData, diagSeverity tfdiags.Severity) tfdiags.Diagnostics { +func evalCheckRules(typ addrs.CheckRuleType, rules []*configs.CheckRule, ctx EvalContext, self addrs.Checkable, keyData instances.RepetitionData, diagSeverity tfdiags.Severity) tfdiags.Diagnostics { var diags tfdiags.Diagnostics checkState := ctx.Checks() @@ -67,7 +67,7 @@ type checkResult struct { FailureMessage string } -func evalCheckRule(typ addrs.CheckType, rule *configs.CheckRule, ctx EvalContext, self addrs.Checkable, keyData instances.RepetitionData, severity hcl.DiagnosticSeverity) (checkResult, tfdiags.Diagnostics) { +func evalCheckRule(typ addrs.CheckRuleType, rule *configs.CheckRule, ctx EvalContext, self addrs.Checkable, keyData instances.RepetitionData, severity hcl.DiagnosticSeverity) (checkResult, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics const errInvalidCondition = "Invalid condition result" From 6db570cd56b5944d7650e5795a989e91c72e833d Mon Sep 17 00:00:00 2001 From: Liam Cervante Date: Fri, 17 Feb 2023 14:24:21 +0100 Subject: [PATCH 3/8] Add configuration for check blocks --- 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 417dff45eece..079d37de194a 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 6ecae9c2903d..8149745fbe5f 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 2e08580b5bc9..79186dce3926 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 f0d809ab1f3a..cdba4e9987ab 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, From 01dcda3ddd0e7a6988993c995bdbac6cf8010717 Mon Sep 17 00:00:00 2001 From: Liam Cervante Date: Mon, 20 Feb 2023 13:06:21 +0100 Subject: [PATCH 4/8] introduce check blocks into the terraform node and transform graph --- internal/checks/state_init.go | 16 + internal/checks/state_test.go | 18 +- .../testdata/happypath/checks-happypath.tf | 7 + internal/command/format/diff.go | 2 + internal/command/jsonchecks/checks_test.go | 35 + internal/command/jsonchecks/objects.go | 15 + internal/command/jsonformat/plan.go | 2 + internal/command/jsonplan/plan.go | 3 + internal/command/views/json/change.go | 3 + internal/plans/changes.go | 6 + .../plans/internal/planproto/planfile.pb.go | 125 ++-- .../plans/internal/planproto/planfile.proto | 2 + internal/plans/planfile/tfplan.go | 10 +- internal/plans/planfile/tfplan_test.go | 19 + ...sourceinstancechangeactionreason_string.go | 36 +- internal/states/statefile/version4.go | 4 + .../terraform/context_apply_checks_test.go | 689 ++++++++++++++++++ internal/terraform/eval_conditions.go | 50 +- internal/terraform/evaluate_valid_test.go | 59 +- internal/terraform/graph_builder.go | 5 + internal/terraform/graph_builder_apply.go | 7 + internal/terraform/graph_builder_plan.go | 7 + internal/terraform/node_check.go | 164 +++++ .../node_resource_abstract_instance.go | 121 ++- .../terraform/node_resource_plan_instance.go | 7 +- .../static-validate-refs.tf | 9 + internal/terraform/transform_check.go | 114 +++ internal/tfdiags/errors_as_warnings.go | 41 ++ 28 files changed, 1460 insertions(+), 116 deletions(-) create mode 100644 internal/terraform/context_apply_checks_test.go create mode 100644 internal/terraform/node_check.go create mode 100644 internal/terraform/transform_check.go create mode 100644 internal/tfdiags/errors_as_warnings.go diff --git a/internal/checks/state_init.go b/internal/checks/state_init.go index a2af43c92453..4ab043740b96 100644 --- a/internal/checks/state_init.go +++ b/internal/checks/state_init.go @@ -49,6 +49,22 @@ func collectInitialStatuses(into addrs.Map[addrs.ConfigCheckable, *configCheckab into.Put(addr, st) } + for _, c := range cfg.Module.Checks { + addr := c.Addr().InModule(moduleAddr) + + st := &configCheckableState{ + checkTypes: map[addrs.CheckRuleType]int{ + addrs.CheckAssertion: len(c.Asserts), + }, + } + + if c.DataResource != nil { + st.checkTypes[addrs.CheckDataResource] = 1 + } + + into.Put(addr, st) + } + // Must also visit child modules to collect everything for _, child := range cfg.Children { collectInitialStatuses(into, child) diff --git a/internal/checks/state_test.go b/internal/checks/state_test.go index 03168daebf05..4829e04c38b8 100644 --- a/internal/checks/state_test.go +++ b/internal/checks/state_test.go @@ -62,6 +62,9 @@ func TestChecksHappyPath(t *testing.T) { childOutput := addrs.OutputValue{ Name: "b", }.InModule(moduleChild) + checkBlock := addrs.Check{ + Name: "check", + }.InModule(addrs.RootModule) // First some consistency checks to make sure our configuration is the // shape we are relying on it to be. @@ -77,6 +80,9 @@ func TestChecksHappyPath(t *testing.T) { if addr := resourceNonExist; cfg.Module.ResourceByAddr(addr.Resource) != nil { t.Fatalf("configuration includes %s, which is not supposed to exist", addr) } + if addr := checkBlock; cfg.Module.Checks[addr.Check.Name] == nil { + t.Fatalf("configuration does not include %s", addr) + } ///////////////////////////////////////////////////////////////////////// @@ -109,6 +115,10 @@ func TestChecksHappyPath(t *testing.T) { if addr := resourceNonExist; checks.ConfigHasChecks(addr) { t.Errorf("checks detected for %s, even though it doesn't exist", addr) } + if addr := checkBlock; !checks.ConfigHasChecks(addr) { + t.Errorf("checks not detected for %s", addr) + missing++ + } if missing > 0 { t.Fatalf("missing some configuration objects we'd need for subsequent testing") } @@ -124,6 +134,7 @@ func TestChecksHappyPath(t *testing.T) { resourceC, rootOutput, childOutput, + checkBlock, ) gotConfigAddrs := checks.AllConfigAddrs() if diff := cmp.Diff(wantConfigAddrs, gotConfigAddrs); diff != "" { @@ -153,6 +164,7 @@ func TestChecksHappyPath(t *testing.T) { resourceInstC0 := resourceC.Resource.Absolute(moduleChildInst).Instance(addrs.IntKey(0)) resourceInstC1 := resourceC.Resource.Absolute(moduleChildInst).Instance(addrs.IntKey(1)) childOutputInst := childOutput.OutputValue.Absolute(moduleChildInst) + checkBlockInst := checkBlock.Check.Absolute(addrs.RootModuleInstance) checks.ReportCheckableObjects(resourceA, addrs.MakeSet[addrs.Checkable](resourceInstA)) checks.ReportCheckResult(resourceInstA, addrs.ResourcePrecondition, 0, StatusPass) @@ -172,6 +184,9 @@ func TestChecksHappyPath(t *testing.T) { checks.ReportCheckableObjects(rootOutput, addrs.MakeSet[addrs.Checkable](rootOutputInst)) checks.ReportCheckResult(rootOutputInst, addrs.OutputPrecondition, 0, StatusPass) + checks.ReportCheckableObjects(checkBlock, addrs.MakeSet[addrs.Checkable](checkBlockInst)) + checks.ReportCheckResult(checkBlockInst, addrs.CheckAssertion, 0, StatusPass) + ///////////////////////////////////////////////////////////////////////// // This "section" is simulating what we might do to report the results @@ -185,7 +200,7 @@ func TestChecksHappyPath(t *testing.T) { t.Errorf("incorrect final aggregate check status for %s: %s, but want %s", configAddr, got, want) } } - if got, want := configCount, 5; got != want { + if got, want := configCount, 6; got != want { t.Errorf("incorrect number of known config addresses %d; want %d", got, want) } } @@ -198,6 +213,7 @@ func TestChecksHappyPath(t *testing.T) { resourceInstC0, resourceInstC1, childOutputInst, + checkBlockInst, ) for _, addr := range objAddrs { if got, want := checks.ObjectCheckStatus(addr), StatusPass; got != want { diff --git a/internal/checks/testdata/happypath/checks-happypath.tf b/internal/checks/testdata/happypath/checks-happypath.tf index 4a6ca46fca16..a9cd055b7ee3 100644 --- a/internal/checks/testdata/happypath/checks-happypath.tf +++ b/internal/checks/testdata/happypath/checks-happypath.tf @@ -30,3 +30,10 @@ output "a" { error_message = "A has no id." } } + +check "check" { + assert { + condition = null_resource.a.id != "" + error_message = "check block: A has no id" + } +} diff --git a/internal/command/format/diff.go b/internal/command/format/diff.go index ed30e3214aff..fe8ec31e7ef7 100644 --- a/internal/command/format/diff.go +++ b/internal/command/format/diff.go @@ -77,6 +77,8 @@ func ResourceChange( buf.WriteString("\n # (config refers to values not yet known)") case plans.ResourceInstanceReadBecauseDependencyPending: buf.WriteString("\n # (depends on a resource or a module with changes pending)") + case plans.ResourceInstanceReadBecauseCheckNested: + buf.WriteString("\n # (data will be read during apply for a check block)") } case plans.Update: switch language { diff --git a/internal/command/jsonchecks/checks_test.go b/internal/command/jsonchecks/checks_test.go index 6e0f52da4faa..55a947dc9a6f 100644 --- a/internal/command/jsonchecks/checks_test.go +++ b/internal/command/jsonchecks/checks_test.go @@ -36,6 +36,8 @@ func TestMarshalCheckStates(t *testing.T) { outputAInstAddr := addrs.Checkable(addrs.OutputValue{Name: "a"}.Absolute(addrs.RootModuleInstance)) outputBAddr := addrs.ConfigCheckable(addrs.OutputValue{Name: "b"}.InModule(moduleChildAddr.Module())) outputBInstAddr := addrs.Checkable(addrs.OutputValue{Name: "b"}.Absolute(moduleChildAddr)) + checkBlockAAddr := addrs.ConfigCheckable(addrs.Check{Name: "a"}.InModule(addrs.RootModule)) + checkBlockAInstAddr := addrs.Checkable(addrs.Check{Name: "a"}.Absolute(addrs.RootModuleInstance)) tests := map[string]struct { Input *states.CheckResults @@ -90,9 +92,42 @@ func TestMarshalCheckStates(t *testing.T) { }), ), }), + addrs.MakeMapElem(checkBlockAAddr, &states.CheckResultAggregate{ + Status: checks.StatusFail, + ObjectResults: addrs.MakeMap( + addrs.MakeMapElem(checkBlockAInstAddr, &states.CheckResultObject{ + Status: checks.StatusFail, + FailureMessages: []string{ + "Couldn't reverse the polarity.", + }, + }), + ), + }), ), }, []any{ + map[string]any{ + "//": "EXPERIMENTAL: see docs for details", + "address": map[string]any{ + "kind": "check", + "to_display": "check.a", + "name": "a", + }, + "instances": []any{ + map[string]any{ + "address": map[string]any{ + "to_display": `check.a`, + }, + "problems": []any{ + map[string]any{ + "message": "Couldn't reverse the polarity.", + }, + }, + "status": "fail", + }, + }, + "status": "fail", + }, map[string]any{ "//": "EXPERIMENTAL: see docs for details", "address": map[string]any{ diff --git a/internal/command/jsonchecks/objects.go b/internal/command/jsonchecks/objects.go index d7a5014fee13..ec0185755876 100644 --- a/internal/command/jsonchecks/objects.go +++ b/internal/command/jsonchecks/objects.go @@ -45,6 +45,17 @@ func makeStaticObjectAddr(addr addrs.ConfigCheckable) staticObjectAddr { if !addr.Module.IsRoot() { ret["module"] = addr.Module.String() } + case addrs.ConfigCheck: + if kind := addr.CheckableKind(); kind != addrs.CheckableCheck { + // Something has gone very wrong + panic(fmt.Sprintf("%T has CheckableKind %s", addr, kind)) + } + + ret["kind"] = "check" + ret["name"] = addr.Check.Name + if !addr.Module.IsRoot() { + ret["module"] = addr.Module.String() + } default: panic(fmt.Sprintf("unsupported ConfigCheckable implementation %T", addr)) } @@ -71,6 +82,10 @@ func makeDynamicObjectAddr(addr addrs.Checkable) dynamicObjectAddr { if !addr.Module.IsRoot() { ret["module"] = addr.Module.String() } + case addrs.AbsCheck: + if !addr.Module.IsRoot() { + ret["module"] = addr.Module.String() + } default: panic(fmt.Sprintf("unsupported Checkable implementation %T", addr)) } diff --git a/internal/command/jsonformat/plan.go b/internal/command/jsonformat/plan.go index 5deaeca54467..c214b0ad213b 100644 --- a/internal/command/jsonformat/plan.go +++ b/internal/command/jsonformat/plan.go @@ -361,6 +361,8 @@ func resourceChangeComment(resource jsonplan.ResourceChange, action plans.Action buf.WriteString("\n # (config refers to values not yet known)") case jsonplan.ResourceInstanceReadBecauseDependencyPending: buf.WriteString("\n # (depends on a resource or a module with changes pending)") + case jsonplan.ResourceInstanceReadBecauseCheckNested: + buf.WriteString("\n # (config will be reloaded to verify a check block)") } case plans.Update: switch changeCause { diff --git a/internal/command/jsonplan/plan.go b/internal/command/jsonplan/plan.go index fca01a1d28a7..5e960083344f 100644 --- a/internal/command/jsonplan/plan.go +++ b/internal/command/jsonplan/plan.go @@ -39,6 +39,7 @@ const ( ResourceInstanceDeleteBecauseNoMoveTarget = "delete_because_no_move_target" ResourceInstanceReadBecauseConfigUnknown = "read_because_config_unknown" ResourceInstanceReadBecauseDependencyPending = "read_because_dependency_pending" + ResourceInstanceReadBecauseCheckNested = "read_because_check_nested" ) // Plan is the top-level representation of the json format of a plan. It includes @@ -492,6 +493,8 @@ func MarshalResourceChanges(resources []*plans.ResourceInstanceChangeSrc, schema r.ActionReason = ResourceInstanceReadBecauseConfigUnknown case plans.ResourceInstanceReadBecauseDependencyPending: r.ActionReason = ResourceInstanceReadBecauseDependencyPending + case plans.ResourceInstanceReadBecauseCheckNested: + r.ActionReason = ResourceInstanceReadBecauseCheckNested default: return nil, fmt.Errorf("resource %s has an unsupported action reason %s", r.Address, rc.ActionReason) } diff --git a/internal/command/views/json/change.go b/internal/command/views/json/change.go index 60439e509017..fb4ec26e3df7 100644 --- a/internal/command/views/json/change.go +++ b/internal/command/views/json/change.go @@ -83,6 +83,7 @@ const ( ReasonDeleteBecauseNoMoveTarget ChangeReason = "delete_because_no_move_target" ReasonReadBecauseConfigUnknown ChangeReason = "read_because_config_unknown" ReasonReadBecauseDependencyPending ChangeReason = "read_because_dependency_pending" + ReasonReadBecauseCheckNested ChangeReason = "read_because_check_nested" ) func changeReason(reason plans.ResourceInstanceChangeActionReason) ChangeReason { @@ -113,6 +114,8 @@ func changeReason(reason plans.ResourceInstanceChangeActionReason) ChangeReason return ReasonDeleteBecauseNoMoveTarget case plans.ResourceInstanceReadBecauseDependencyPending: return ReasonReadBecauseDependencyPending + case plans.ResourceInstanceReadBecauseCheckNested: + return ReasonReadBecauseCheckNested default: // This should never happen, but there's no good way to guarantee // exhaustive handling of the enum, so a generic fall back is better diff --git a/internal/plans/changes.go b/internal/plans/changes.go index 7c54928331e6..7206d4f4645f 100644 --- a/internal/plans/changes.go +++ b/internal/plans/changes.go @@ -444,6 +444,12 @@ const ( // depends on a managed resource instance which has its own changes // pending. ResourceInstanceReadBecauseDependencyPending ResourceInstanceChangeActionReason = '!' + + // ResourceInstanceReadBecauseCheckNested indicates that the resource must + // be read during apply (as well as during planning) because it is inside + // a check block and when the check assertions execute we want them to use + // the most up-to-date data. + ResourceInstanceReadBecauseCheckNested ResourceInstanceChangeActionReason = '#' ) // OutputChange describes a change to an output value. diff --git a/internal/plans/internal/planproto/planfile.pb.go b/internal/plans/internal/planproto/planfile.pb.go index ec329d3ba198..df4ff03b73aa 100644 --- a/internal/plans/internal/planproto/planfile.pb.go +++ b/internal/plans/internal/planproto/planfile.pb.go @@ -152,6 +152,7 @@ const ( ResourceInstanceActionReason_REPLACE_BY_TRIGGERS ResourceInstanceActionReason = 9 ResourceInstanceActionReason_READ_BECAUSE_CONFIG_UNKNOWN ResourceInstanceActionReason = 10 ResourceInstanceActionReason_READ_BECAUSE_DEPENDENCY_PENDING ResourceInstanceActionReason = 11 + ResourceInstanceActionReason_READ_BECAUSE_CHECK_NESTED ResourceInstanceActionReason = 13 ResourceInstanceActionReason_DELETE_BECAUSE_NO_MOVE_TARGET ResourceInstanceActionReason = 12 ) @@ -170,6 +171,7 @@ var ( 9: "REPLACE_BY_TRIGGERS", 10: "READ_BECAUSE_CONFIG_UNKNOWN", 11: "READ_BECAUSE_DEPENDENCY_PENDING", + 13: "READ_BECAUSE_CHECK_NESTED", 12: "DELETE_BECAUSE_NO_MOVE_TARGET", } ResourceInstanceActionReason_value = map[string]int32{ @@ -185,6 +187,7 @@ var ( "REPLACE_BY_TRIGGERS": 9, "READ_BECAUSE_CONFIG_UNKNOWN": 10, "READ_BECAUSE_DEPENDENCY_PENDING": 11, + "READ_BECAUSE_CHECK_NESTED": 13, "DELETE_BECAUSE_NO_MOVE_TARGET": 12, } ) @@ -276,6 +279,7 @@ const ( CheckResults_UNSPECIFIED CheckResults_ObjectKind = 0 CheckResults_RESOURCE CheckResults_ObjectKind = 1 CheckResults_OUTPUT_VALUE CheckResults_ObjectKind = 2 + CheckResults_CHECK CheckResults_ObjectKind = 3 ) // Enum value maps for CheckResults_ObjectKind. @@ -284,11 +288,13 @@ var ( 0: "UNSPECIFIED", 1: "RESOURCE", 2: "OUTPUT_VALUE", + 3: "CHECK", } CheckResults_ObjectKind_value = map[string]int32{ "UNSPECIFIED": 0, "RESOURCE": 1, "OUTPUT_VALUE": 2, + "CHECK": 3, } ) @@ -1353,7 +1359,7 @@ var file_planfile_proto_rawDesc = []byte{ 0x67, 0x65, 0x18, 0x02, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x0e, 0x2e, 0x74, 0x66, 0x70, 0x6c, 0x61, 0x6e, 0x2e, 0x43, 0x68, 0x61, 0x6e, 0x67, 0x65, 0x52, 0x06, 0x63, 0x68, 0x61, 0x6e, 0x67, 0x65, 0x12, 0x1c, 0x0a, 0x09, 0x73, 0x65, 0x6e, 0x73, 0x69, 0x74, 0x69, 0x76, 0x65, 0x18, 0x03, 0x20, - 0x01, 0x28, 0x08, 0x52, 0x09, 0x73, 0x65, 0x6e, 0x73, 0x69, 0x74, 0x69, 0x76, 0x65, 0x22, 0xdd, + 0x01, 0x28, 0x08, 0x52, 0x09, 0x73, 0x65, 0x6e, 0x73, 0x69, 0x74, 0x69, 0x76, 0x65, 0x22, 0xe8, 0x03, 0x0a, 0x0c, 0x43, 0x68, 0x65, 0x63, 0x6b, 0x52, 0x65, 0x73, 0x75, 0x6c, 0x74, 0x73, 0x12, 0x33, 0x0a, 0x04, 0x6b, 0x69, 0x6e, 0x64, 0x18, 0x01, 0x20, 0x01, 0x28, 0x0e, 0x32, 0x1f, 0x2e, 0x74, 0x66, 0x70, 0x6c, 0x61, 0x6e, 0x2e, 0x43, 0x68, 0x65, 0x63, 0x6b, 0x52, 0x65, 0x73, 0x75, @@ -1380,65 +1386,68 @@ var file_planfile_proto_rawDesc = []byte{ 0x74, 0x75, 0x73, 0x12, 0x0b, 0x0a, 0x07, 0x55, 0x4e, 0x4b, 0x4e, 0x4f, 0x57, 0x4e, 0x10, 0x00, 0x12, 0x08, 0x0a, 0x04, 0x50, 0x41, 0x53, 0x53, 0x10, 0x01, 0x12, 0x08, 0x0a, 0x04, 0x46, 0x41, 0x49, 0x4c, 0x10, 0x02, 0x12, 0x09, 0x0a, 0x05, 0x45, 0x52, 0x52, 0x4f, 0x52, 0x10, 0x03, 0x22, - 0x3d, 0x0a, 0x0a, 0x4f, 0x62, 0x6a, 0x65, 0x63, 0x74, 0x4b, 0x69, 0x6e, 0x64, 0x12, 0x0f, 0x0a, + 0x48, 0x0a, 0x0a, 0x4f, 0x62, 0x6a, 0x65, 0x63, 0x74, 0x4b, 0x69, 0x6e, 0x64, 0x12, 0x0f, 0x0a, 0x0b, 0x55, 0x4e, 0x53, 0x50, 0x45, 0x43, 0x49, 0x46, 0x49, 0x45, 0x44, 0x10, 0x00, 0x12, 0x0c, 0x0a, 0x08, 0x52, 0x45, 0x53, 0x4f, 0x55, 0x52, 0x43, 0x45, 0x10, 0x01, 0x12, 0x10, 0x0a, 0x0c, - 0x4f, 0x55, 0x54, 0x50, 0x55, 0x54, 0x5f, 0x56, 0x41, 0x4c, 0x55, 0x45, 0x10, 0x02, 0x22, 0x28, - 0x0a, 0x0c, 0x44, 0x79, 0x6e, 0x61, 0x6d, 0x69, 0x63, 0x56, 0x61, 0x6c, 0x75, 0x65, 0x12, 0x18, - 0x0a, 0x07, 0x6d, 0x73, 0x67, 0x70, 0x61, 0x63, 0x6b, 0x18, 0x01, 0x20, 0x01, 0x28, 0x0c, 0x52, - 0x07, 0x6d, 0x73, 0x67, 0x70, 0x61, 0x63, 0x6b, 0x22, 0xa5, 0x01, 0x0a, 0x04, 0x50, 0x61, 0x74, - 0x68, 0x12, 0x27, 0x0a, 0x05, 0x73, 0x74, 0x65, 0x70, 0x73, 0x18, 0x01, 0x20, 0x03, 0x28, 0x0b, - 0x32, 0x11, 0x2e, 0x74, 0x66, 0x70, 0x6c, 0x61, 0x6e, 0x2e, 0x50, 0x61, 0x74, 0x68, 0x2e, 0x53, - 0x74, 0x65, 0x70, 0x52, 0x05, 0x73, 0x74, 0x65, 0x70, 0x73, 0x1a, 0x74, 0x0a, 0x04, 0x53, 0x74, - 0x65, 0x70, 0x12, 0x27, 0x0a, 0x0e, 0x61, 0x74, 0x74, 0x72, 0x69, 0x62, 0x75, 0x74, 0x65, 0x5f, - 0x6e, 0x61, 0x6d, 0x65, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x48, 0x00, 0x52, 0x0d, 0x61, 0x74, - 0x74, 0x72, 0x69, 0x62, 0x75, 0x74, 0x65, 0x4e, 0x61, 0x6d, 0x65, 0x12, 0x37, 0x0a, 0x0b, 0x65, - 0x6c, 0x65, 0x6d, 0x65, 0x6e, 0x74, 0x5f, 0x6b, 0x65, 0x79, 0x18, 0x02, 0x20, 0x01, 0x28, 0x0b, - 0x32, 0x14, 0x2e, 0x74, 0x66, 0x70, 0x6c, 0x61, 0x6e, 0x2e, 0x44, 0x79, 0x6e, 0x61, 0x6d, 0x69, - 0x63, 0x56, 0x61, 0x6c, 0x75, 0x65, 0x48, 0x00, 0x52, 0x0a, 0x65, 0x6c, 0x65, 0x6d, 0x65, 0x6e, - 0x74, 0x4b, 0x65, 0x79, 0x42, 0x0a, 0x0a, 0x08, 0x73, 0x65, 0x6c, 0x65, 0x63, 0x74, 0x6f, 0x72, - 0x2a, 0x31, 0x0a, 0x04, 0x4d, 0x6f, 0x64, 0x65, 0x12, 0x0a, 0x0a, 0x06, 0x4e, 0x4f, 0x52, 0x4d, - 0x41, 0x4c, 0x10, 0x00, 0x12, 0x0b, 0x0a, 0x07, 0x44, 0x45, 0x53, 0x54, 0x52, 0x4f, 0x59, 0x10, - 0x01, 0x12, 0x10, 0x0a, 0x0c, 0x52, 0x45, 0x46, 0x52, 0x45, 0x53, 0x48, 0x5f, 0x4f, 0x4e, 0x4c, - 0x59, 0x10, 0x02, 0x2a, 0x70, 0x0a, 0x06, 0x41, 0x63, 0x74, 0x69, 0x6f, 0x6e, 0x12, 0x08, 0x0a, - 0x04, 0x4e, 0x4f, 0x4f, 0x50, 0x10, 0x00, 0x12, 0x0a, 0x0a, 0x06, 0x43, 0x52, 0x45, 0x41, 0x54, - 0x45, 0x10, 0x01, 0x12, 0x08, 0x0a, 0x04, 0x52, 0x45, 0x41, 0x44, 0x10, 0x02, 0x12, 0x0a, 0x0a, - 0x06, 0x55, 0x50, 0x44, 0x41, 0x54, 0x45, 0x10, 0x03, 0x12, 0x0a, 0x0a, 0x06, 0x44, 0x45, 0x4c, - 0x45, 0x54, 0x45, 0x10, 0x05, 0x12, 0x16, 0x0a, 0x12, 0x44, 0x45, 0x4c, 0x45, 0x54, 0x45, 0x5f, - 0x54, 0x48, 0x45, 0x4e, 0x5f, 0x43, 0x52, 0x45, 0x41, 0x54, 0x45, 0x10, 0x06, 0x12, 0x16, 0x0a, - 0x12, 0x43, 0x52, 0x45, 0x41, 0x54, 0x45, 0x5f, 0x54, 0x48, 0x45, 0x4e, 0x5f, 0x44, 0x45, 0x4c, - 0x45, 0x54, 0x45, 0x10, 0x07, 0x2a, 0xa9, 0x03, 0x0a, 0x1c, 0x52, 0x65, 0x73, 0x6f, 0x75, 0x72, - 0x63, 0x65, 0x49, 0x6e, 0x73, 0x74, 0x61, 0x6e, 0x63, 0x65, 0x41, 0x63, 0x74, 0x69, 0x6f, 0x6e, - 0x52, 0x65, 0x61, 0x73, 0x6f, 0x6e, 0x12, 0x08, 0x0a, 0x04, 0x4e, 0x4f, 0x4e, 0x45, 0x10, 0x00, - 0x12, 0x1b, 0x0a, 0x17, 0x52, 0x45, 0x50, 0x4c, 0x41, 0x43, 0x45, 0x5f, 0x42, 0x45, 0x43, 0x41, - 0x55, 0x53, 0x45, 0x5f, 0x54, 0x41, 0x49, 0x4e, 0x54, 0x45, 0x44, 0x10, 0x01, 0x12, 0x16, 0x0a, - 0x12, 0x52, 0x45, 0x50, 0x4c, 0x41, 0x43, 0x45, 0x5f, 0x42, 0x59, 0x5f, 0x52, 0x45, 0x51, 0x55, - 0x45, 0x53, 0x54, 0x10, 0x02, 0x12, 0x21, 0x0a, 0x1d, 0x52, 0x45, 0x50, 0x4c, 0x41, 0x43, 0x45, - 0x5f, 0x42, 0x45, 0x43, 0x41, 0x55, 0x53, 0x45, 0x5f, 0x43, 0x41, 0x4e, 0x4e, 0x4f, 0x54, 0x5f, - 0x55, 0x50, 0x44, 0x41, 0x54, 0x45, 0x10, 0x03, 0x12, 0x25, 0x0a, 0x21, 0x44, 0x45, 0x4c, 0x45, - 0x54, 0x45, 0x5f, 0x42, 0x45, 0x43, 0x41, 0x55, 0x53, 0x45, 0x5f, 0x4e, 0x4f, 0x5f, 0x52, 0x45, - 0x53, 0x4f, 0x55, 0x52, 0x43, 0x45, 0x5f, 0x43, 0x4f, 0x4e, 0x46, 0x49, 0x47, 0x10, 0x04, 0x12, - 0x23, 0x0a, 0x1f, 0x44, 0x45, 0x4c, 0x45, 0x54, 0x45, 0x5f, 0x42, 0x45, 0x43, 0x41, 0x55, 0x53, - 0x45, 0x5f, 0x57, 0x52, 0x4f, 0x4e, 0x47, 0x5f, 0x52, 0x45, 0x50, 0x45, 0x54, 0x49, 0x54, 0x49, - 0x4f, 0x4e, 0x10, 0x05, 0x12, 0x1e, 0x0a, 0x1a, 0x44, 0x45, 0x4c, 0x45, 0x54, 0x45, 0x5f, 0x42, - 0x45, 0x43, 0x41, 0x55, 0x53, 0x45, 0x5f, 0x43, 0x4f, 0x55, 0x4e, 0x54, 0x5f, 0x49, 0x4e, 0x44, - 0x45, 0x58, 0x10, 0x06, 0x12, 0x1b, 0x0a, 0x17, 0x44, 0x45, 0x4c, 0x45, 0x54, 0x45, 0x5f, 0x42, - 0x45, 0x43, 0x41, 0x55, 0x53, 0x45, 0x5f, 0x45, 0x41, 0x43, 0x48, 0x5f, 0x4b, 0x45, 0x59, 0x10, - 0x07, 0x12, 0x1c, 0x0a, 0x18, 0x44, 0x45, 0x4c, 0x45, 0x54, 0x45, 0x5f, 0x42, 0x45, 0x43, 0x41, - 0x55, 0x53, 0x45, 0x5f, 0x4e, 0x4f, 0x5f, 0x4d, 0x4f, 0x44, 0x55, 0x4c, 0x45, 0x10, 0x08, 0x12, - 0x17, 0x0a, 0x13, 0x52, 0x45, 0x50, 0x4c, 0x41, 0x43, 0x45, 0x5f, 0x42, 0x59, 0x5f, 0x54, 0x52, - 0x49, 0x47, 0x47, 0x45, 0x52, 0x53, 0x10, 0x09, 0x12, 0x1f, 0x0a, 0x1b, 0x52, 0x45, 0x41, 0x44, - 0x5f, 0x42, 0x45, 0x43, 0x41, 0x55, 0x53, 0x45, 0x5f, 0x43, 0x4f, 0x4e, 0x46, 0x49, 0x47, 0x5f, - 0x55, 0x4e, 0x4b, 0x4e, 0x4f, 0x57, 0x4e, 0x10, 0x0a, 0x12, 0x23, 0x0a, 0x1f, 0x52, 0x45, 0x41, - 0x44, 0x5f, 0x42, 0x45, 0x43, 0x41, 0x55, 0x53, 0x45, 0x5f, 0x44, 0x45, 0x50, 0x45, 0x4e, 0x44, - 0x45, 0x4e, 0x43, 0x59, 0x5f, 0x50, 0x45, 0x4e, 0x44, 0x49, 0x4e, 0x47, 0x10, 0x0b, 0x12, 0x21, - 0x0a, 0x1d, 0x44, 0x45, 0x4c, 0x45, 0x54, 0x45, 0x5f, 0x42, 0x45, 0x43, 0x41, 0x55, 0x53, 0x45, - 0x5f, 0x4e, 0x4f, 0x5f, 0x4d, 0x4f, 0x56, 0x45, 0x5f, 0x54, 0x41, 0x52, 0x47, 0x45, 0x54, 0x10, - 0x0c, 0x42, 0x42, 0x5a, 0x40, 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, - 0x68, 0x61, 0x73, 0x68, 0x69, 0x63, 0x6f, 0x72, 0x70, 0x2f, 0x74, 0x65, 0x72, 0x72, 0x61, 0x66, - 0x6f, 0x72, 0x6d, 0x2f, 0x69, 0x6e, 0x74, 0x65, 0x72, 0x6e, 0x61, 0x6c, 0x2f, 0x70, 0x6c, 0x61, - 0x6e, 0x73, 0x2f, 0x69, 0x6e, 0x74, 0x65, 0x72, 0x6e, 0x61, 0x6c, 0x2f, 0x70, 0x6c, 0x61, 0x6e, - 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, + 0x4f, 0x55, 0x54, 0x50, 0x55, 0x54, 0x5f, 0x56, 0x41, 0x4c, 0x55, 0x45, 0x10, 0x02, 0x12, 0x09, + 0x0a, 0x05, 0x43, 0x48, 0x45, 0x43, 0x4b, 0x10, 0x03, 0x22, 0x28, 0x0a, 0x0c, 0x44, 0x79, 0x6e, + 0x61, 0x6d, 0x69, 0x63, 0x56, 0x61, 0x6c, 0x75, 0x65, 0x12, 0x18, 0x0a, 0x07, 0x6d, 0x73, 0x67, + 0x70, 0x61, 0x63, 0x6b, 0x18, 0x01, 0x20, 0x01, 0x28, 0x0c, 0x52, 0x07, 0x6d, 0x73, 0x67, 0x70, + 0x61, 0x63, 0x6b, 0x22, 0xa5, 0x01, 0x0a, 0x04, 0x50, 0x61, 0x74, 0x68, 0x12, 0x27, 0x0a, 0x05, + 0x73, 0x74, 0x65, 0x70, 0x73, 0x18, 0x01, 0x20, 0x03, 0x28, 0x0b, 0x32, 0x11, 0x2e, 0x74, 0x66, + 0x70, 0x6c, 0x61, 0x6e, 0x2e, 0x50, 0x61, 0x74, 0x68, 0x2e, 0x53, 0x74, 0x65, 0x70, 0x52, 0x05, + 0x73, 0x74, 0x65, 0x70, 0x73, 0x1a, 0x74, 0x0a, 0x04, 0x53, 0x74, 0x65, 0x70, 0x12, 0x27, 0x0a, + 0x0e, 0x61, 0x74, 0x74, 0x72, 0x69, 0x62, 0x75, 0x74, 0x65, 0x5f, 0x6e, 0x61, 0x6d, 0x65, 0x18, + 0x01, 0x20, 0x01, 0x28, 0x09, 0x48, 0x00, 0x52, 0x0d, 0x61, 0x74, 0x74, 0x72, 0x69, 0x62, 0x75, + 0x74, 0x65, 0x4e, 0x61, 0x6d, 0x65, 0x12, 0x37, 0x0a, 0x0b, 0x65, 0x6c, 0x65, 0x6d, 0x65, 0x6e, + 0x74, 0x5f, 0x6b, 0x65, 0x79, 0x18, 0x02, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x14, 0x2e, 0x74, 0x66, + 0x70, 0x6c, 0x61, 0x6e, 0x2e, 0x44, 0x79, 0x6e, 0x61, 0x6d, 0x69, 0x63, 0x56, 0x61, 0x6c, 0x75, + 0x65, 0x48, 0x00, 0x52, 0x0a, 0x65, 0x6c, 0x65, 0x6d, 0x65, 0x6e, 0x74, 0x4b, 0x65, 0x79, 0x42, + 0x0a, 0x0a, 0x08, 0x73, 0x65, 0x6c, 0x65, 0x63, 0x74, 0x6f, 0x72, 0x2a, 0x31, 0x0a, 0x04, 0x4d, + 0x6f, 0x64, 0x65, 0x12, 0x0a, 0x0a, 0x06, 0x4e, 0x4f, 0x52, 0x4d, 0x41, 0x4c, 0x10, 0x00, 0x12, + 0x0b, 0x0a, 0x07, 0x44, 0x45, 0x53, 0x54, 0x52, 0x4f, 0x59, 0x10, 0x01, 0x12, 0x10, 0x0a, 0x0c, + 0x52, 0x45, 0x46, 0x52, 0x45, 0x53, 0x48, 0x5f, 0x4f, 0x4e, 0x4c, 0x59, 0x10, 0x02, 0x2a, 0x70, + 0x0a, 0x06, 0x41, 0x63, 0x74, 0x69, 0x6f, 0x6e, 0x12, 0x08, 0x0a, 0x04, 0x4e, 0x4f, 0x4f, 0x50, + 0x10, 0x00, 0x12, 0x0a, 0x0a, 0x06, 0x43, 0x52, 0x45, 0x41, 0x54, 0x45, 0x10, 0x01, 0x12, 0x08, + 0x0a, 0x04, 0x52, 0x45, 0x41, 0x44, 0x10, 0x02, 0x12, 0x0a, 0x0a, 0x06, 0x55, 0x50, 0x44, 0x41, + 0x54, 0x45, 0x10, 0x03, 0x12, 0x0a, 0x0a, 0x06, 0x44, 0x45, 0x4c, 0x45, 0x54, 0x45, 0x10, 0x05, + 0x12, 0x16, 0x0a, 0x12, 0x44, 0x45, 0x4c, 0x45, 0x54, 0x45, 0x5f, 0x54, 0x48, 0x45, 0x4e, 0x5f, + 0x43, 0x52, 0x45, 0x41, 0x54, 0x45, 0x10, 0x06, 0x12, 0x16, 0x0a, 0x12, 0x43, 0x52, 0x45, 0x41, + 0x54, 0x45, 0x5f, 0x54, 0x48, 0x45, 0x4e, 0x5f, 0x44, 0x45, 0x4c, 0x45, 0x54, 0x45, 0x10, 0x07, + 0x2a, 0xc8, 0x03, 0x0a, 0x1c, 0x52, 0x65, 0x73, 0x6f, 0x75, 0x72, 0x63, 0x65, 0x49, 0x6e, 0x73, + 0x74, 0x61, 0x6e, 0x63, 0x65, 0x41, 0x63, 0x74, 0x69, 0x6f, 0x6e, 0x52, 0x65, 0x61, 0x73, 0x6f, + 0x6e, 0x12, 0x08, 0x0a, 0x04, 0x4e, 0x4f, 0x4e, 0x45, 0x10, 0x00, 0x12, 0x1b, 0x0a, 0x17, 0x52, + 0x45, 0x50, 0x4c, 0x41, 0x43, 0x45, 0x5f, 0x42, 0x45, 0x43, 0x41, 0x55, 0x53, 0x45, 0x5f, 0x54, + 0x41, 0x49, 0x4e, 0x54, 0x45, 0x44, 0x10, 0x01, 0x12, 0x16, 0x0a, 0x12, 0x52, 0x45, 0x50, 0x4c, + 0x41, 0x43, 0x45, 0x5f, 0x42, 0x59, 0x5f, 0x52, 0x45, 0x51, 0x55, 0x45, 0x53, 0x54, 0x10, 0x02, + 0x12, 0x21, 0x0a, 0x1d, 0x52, 0x45, 0x50, 0x4c, 0x41, 0x43, 0x45, 0x5f, 0x42, 0x45, 0x43, 0x41, + 0x55, 0x53, 0x45, 0x5f, 0x43, 0x41, 0x4e, 0x4e, 0x4f, 0x54, 0x5f, 0x55, 0x50, 0x44, 0x41, 0x54, + 0x45, 0x10, 0x03, 0x12, 0x25, 0x0a, 0x21, 0x44, 0x45, 0x4c, 0x45, 0x54, 0x45, 0x5f, 0x42, 0x45, + 0x43, 0x41, 0x55, 0x53, 0x45, 0x5f, 0x4e, 0x4f, 0x5f, 0x52, 0x45, 0x53, 0x4f, 0x55, 0x52, 0x43, + 0x45, 0x5f, 0x43, 0x4f, 0x4e, 0x46, 0x49, 0x47, 0x10, 0x04, 0x12, 0x23, 0x0a, 0x1f, 0x44, 0x45, + 0x4c, 0x45, 0x54, 0x45, 0x5f, 0x42, 0x45, 0x43, 0x41, 0x55, 0x53, 0x45, 0x5f, 0x57, 0x52, 0x4f, + 0x4e, 0x47, 0x5f, 0x52, 0x45, 0x50, 0x45, 0x54, 0x49, 0x54, 0x49, 0x4f, 0x4e, 0x10, 0x05, 0x12, + 0x1e, 0x0a, 0x1a, 0x44, 0x45, 0x4c, 0x45, 0x54, 0x45, 0x5f, 0x42, 0x45, 0x43, 0x41, 0x55, 0x53, + 0x45, 0x5f, 0x43, 0x4f, 0x55, 0x4e, 0x54, 0x5f, 0x49, 0x4e, 0x44, 0x45, 0x58, 0x10, 0x06, 0x12, + 0x1b, 0x0a, 0x17, 0x44, 0x45, 0x4c, 0x45, 0x54, 0x45, 0x5f, 0x42, 0x45, 0x43, 0x41, 0x55, 0x53, + 0x45, 0x5f, 0x45, 0x41, 0x43, 0x48, 0x5f, 0x4b, 0x45, 0x59, 0x10, 0x07, 0x12, 0x1c, 0x0a, 0x18, + 0x44, 0x45, 0x4c, 0x45, 0x54, 0x45, 0x5f, 0x42, 0x45, 0x43, 0x41, 0x55, 0x53, 0x45, 0x5f, 0x4e, + 0x4f, 0x5f, 0x4d, 0x4f, 0x44, 0x55, 0x4c, 0x45, 0x10, 0x08, 0x12, 0x17, 0x0a, 0x13, 0x52, 0x45, + 0x50, 0x4c, 0x41, 0x43, 0x45, 0x5f, 0x42, 0x59, 0x5f, 0x54, 0x52, 0x49, 0x47, 0x47, 0x45, 0x52, + 0x53, 0x10, 0x09, 0x12, 0x1f, 0x0a, 0x1b, 0x52, 0x45, 0x41, 0x44, 0x5f, 0x42, 0x45, 0x43, 0x41, + 0x55, 0x53, 0x45, 0x5f, 0x43, 0x4f, 0x4e, 0x46, 0x49, 0x47, 0x5f, 0x55, 0x4e, 0x4b, 0x4e, 0x4f, + 0x57, 0x4e, 0x10, 0x0a, 0x12, 0x23, 0x0a, 0x1f, 0x52, 0x45, 0x41, 0x44, 0x5f, 0x42, 0x45, 0x43, + 0x41, 0x55, 0x53, 0x45, 0x5f, 0x44, 0x45, 0x50, 0x45, 0x4e, 0x44, 0x45, 0x4e, 0x43, 0x59, 0x5f, + 0x50, 0x45, 0x4e, 0x44, 0x49, 0x4e, 0x47, 0x10, 0x0b, 0x12, 0x1d, 0x0a, 0x19, 0x52, 0x45, 0x41, + 0x44, 0x5f, 0x42, 0x45, 0x43, 0x41, 0x55, 0x53, 0x45, 0x5f, 0x43, 0x48, 0x45, 0x43, 0x4b, 0x5f, + 0x4e, 0x45, 0x53, 0x54, 0x45, 0x44, 0x10, 0x0d, 0x12, 0x21, 0x0a, 0x1d, 0x44, 0x45, 0x4c, 0x45, + 0x54, 0x45, 0x5f, 0x42, 0x45, 0x43, 0x41, 0x55, 0x53, 0x45, 0x5f, 0x4e, 0x4f, 0x5f, 0x4d, 0x4f, + 0x56, 0x45, 0x5f, 0x54, 0x41, 0x52, 0x47, 0x45, 0x54, 0x10, 0x0c, 0x42, 0x42, 0x5a, 0x40, 0x67, + 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x68, 0x61, 0x73, 0x68, 0x69, 0x63, + 0x6f, 0x72, 0x70, 0x2f, 0x74, 0x65, 0x72, 0x72, 0x61, 0x66, 0x6f, 0x72, 0x6d, 0x2f, 0x69, 0x6e, + 0x74, 0x65, 0x72, 0x6e, 0x61, 0x6c, 0x2f, 0x70, 0x6c, 0x61, 0x6e, 0x73, 0x2f, 0x69, 0x6e, 0x74, + 0x65, 0x72, 0x6e, 0x61, 0x6c, 0x2f, 0x70, 0x6c, 0x61, 0x6e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x62, + 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, } var ( diff --git a/internal/plans/internal/planproto/planfile.proto b/internal/plans/internal/planproto/planfile.proto index fa7cb774c958..f7a10353b045 100644 --- a/internal/plans/internal/planproto/planfile.proto +++ b/internal/plans/internal/planproto/planfile.proto @@ -156,6 +156,7 @@ enum ResourceInstanceActionReason { REPLACE_BY_TRIGGERS = 9; READ_BECAUSE_CONFIG_UNKNOWN = 10; READ_BECAUSE_DEPENDENCY_PENDING = 11; + READ_BECAUSE_CHECK_NESTED = 13; DELETE_BECAUSE_NO_MOVE_TARGET = 12; } @@ -237,6 +238,7 @@ message CheckResults { UNSPECIFIED = 0; RESOURCE = 1; OUTPUT_VALUE = 2; + CHECK = 3; } message ObjectResult { diff --git a/internal/plans/planfile/tfplan.go b/internal/plans/planfile/tfplan.go index 19866b6714e9..f8b982497d79 100644 --- a/internal/plans/planfile/tfplan.go +++ b/internal/plans/planfile/tfplan.go @@ -5,6 +5,7 @@ import ( "io" "io/ioutil" + "github.com/zclconf/go-cty/cty" "google.golang.org/protobuf/proto" "github.com/hashicorp/terraform/internal/addrs" @@ -15,7 +16,6 @@ import ( "github.com/hashicorp/terraform/internal/plans/internal/planproto" "github.com/hashicorp/terraform/internal/states" "github.com/hashicorp/terraform/version" - "github.com/zclconf/go-cty/cty" ) const tfplanFormatVersion = 3 @@ -114,6 +114,8 @@ func readTfplan(r io.Reader) (*plans.Plan, error) { objKind = addrs.CheckableResource case planproto.CheckResults_OUTPUT_VALUE: objKind = addrs.CheckableOutputValue + case planproto.CheckResults_CHECK: + objKind = addrs.CheckableCheck default: return nil, fmt.Errorf("aggregate check results for %s have unsupported object kind %s", rawCRs.ConfigAddr, objKind) } @@ -333,6 +335,8 @@ func resourceChangeFromTfplan(rawChange *planproto.ResourceInstanceChange) (*pla ret.ActionReason = plans.ResourceInstanceReadBecauseConfigUnknown case planproto.ResourceInstanceActionReason_READ_BECAUSE_DEPENDENCY_PENDING: ret.ActionReason = plans.ResourceInstanceReadBecauseDependencyPending + case planproto.ResourceInstanceActionReason_READ_BECAUSE_CHECK_NESTED: + ret.ActionReason = plans.ResourceInstanceReadBecauseCheckNested case planproto.ResourceInstanceActionReason_DELETE_BECAUSE_NO_MOVE_TARGET: ret.ActionReason = plans.ResourceInstanceDeleteBecauseNoMoveTarget default: @@ -524,6 +528,8 @@ func writeTfplan(plan *plans.Plan, w io.Writer) error { pcrs.Kind = planproto.CheckResults_RESOURCE case addrs.CheckableOutputValue: pcrs.Kind = planproto.CheckResults_OUTPUT_VALUE + case addrs.CheckableCheck: + pcrs.Kind = planproto.CheckResults_CHECK default: return fmt.Errorf("checkable configuration %s has unsupported object type kind %s", configElem.Key, kind) } @@ -711,6 +717,8 @@ func resourceChangeToTfplan(change *plans.ResourceInstanceChangeSrc) (*planproto ret.ActionReason = planproto.ResourceInstanceActionReason_READ_BECAUSE_CONFIG_UNKNOWN case plans.ResourceInstanceReadBecauseDependencyPending: ret.ActionReason = planproto.ResourceInstanceActionReason_READ_BECAUSE_DEPENDENCY_PENDING + case plans.ResourceInstanceReadBecauseCheckNested: + ret.ActionReason = planproto.ResourceInstanceActionReason_READ_BECAUSE_CHECK_NESTED case plans.ResourceInstanceDeleteBecauseNoMoveTarget: ret.ActionReason = planproto.ResourceInstanceActionReason_DELETE_BECAUSE_NO_MOVE_TARGET default: diff --git a/internal/plans/planfile/tfplan_test.go b/internal/plans/planfile/tfplan_test.go index 6984ceafdcad..217319373b9e 100644 --- a/internal/plans/planfile/tfplan_test.go +++ b/internal/plans/planfile/tfplan_test.go @@ -196,6 +196,25 @@ func TestTFPlanRoundTrip(t *testing.T) { ), }, ), + addrs.MakeMapElem[addrs.ConfigCheckable]( + addrs.Check{ + Name: "check", + }.InModule(addrs.RootModule), + &states.CheckResultAggregate{ + Status: checks.StatusFail, + ObjectResults: addrs.MakeMap( + addrs.MakeMapElem[addrs.Checkable]( + addrs.Check{ + Name: "check", + }.Absolute(addrs.RootModuleInstance), + &states.CheckResultObject{ + Status: checks.StatusFail, + FailureMessages: []string{"check failed"}, + }, + ), + ), + }, + ), ), }, TargetAddrs: []addrs.Targetable{ diff --git a/internal/plans/resourceinstancechangeactionreason_string.go b/internal/plans/resourceinstancechangeactionreason_string.go index 742aae235651..9915ec85bc1f 100644 --- a/internal/plans/resourceinstancechangeactionreason_string.go +++ b/internal/plans/resourceinstancechangeactionreason_string.go @@ -21,23 +21,25 @@ func _() { _ = x[ResourceInstanceDeleteBecauseNoMoveTarget-65] _ = x[ResourceInstanceReadBecauseConfigUnknown-63] _ = x[ResourceInstanceReadBecauseDependencyPending-33] + _ = x[ResourceInstanceReadBecauseCheckNested-35] } const ( _ResourceInstanceChangeActionReason_name_0 = "ResourceInstanceChangeNoReason" _ResourceInstanceChangeActionReason_name_1 = "ResourceInstanceReadBecauseDependencyPending" - _ResourceInstanceChangeActionReason_name_2 = "ResourceInstanceReadBecauseConfigUnknown" - _ResourceInstanceChangeActionReason_name_3 = "ResourceInstanceDeleteBecauseNoMoveTarget" - _ResourceInstanceChangeActionReason_name_4 = "ResourceInstanceDeleteBecauseCountIndexResourceInstanceReplaceByTriggersResourceInstanceDeleteBecauseEachKeyResourceInstanceReplaceBecauseCannotUpdate" - _ResourceInstanceChangeActionReason_name_5 = "ResourceInstanceDeleteBecauseNoModuleResourceInstanceDeleteBecauseNoResourceConfig" - _ResourceInstanceChangeActionReason_name_6 = "ResourceInstanceReplaceByRequest" - _ResourceInstanceChangeActionReason_name_7 = "ResourceInstanceReplaceBecauseTainted" - _ResourceInstanceChangeActionReason_name_8 = "ResourceInstanceDeleteBecauseWrongRepetition" + _ResourceInstanceChangeActionReason_name_2 = "ResourceInstanceReadBecauseCheckNested" + _ResourceInstanceChangeActionReason_name_3 = "ResourceInstanceReadBecauseConfigUnknown" + _ResourceInstanceChangeActionReason_name_4 = "ResourceInstanceDeleteBecauseNoMoveTarget" + _ResourceInstanceChangeActionReason_name_5 = "ResourceInstanceDeleteBecauseCountIndexResourceInstanceReplaceByTriggersResourceInstanceDeleteBecauseEachKeyResourceInstanceReplaceBecauseCannotUpdate" + _ResourceInstanceChangeActionReason_name_6 = "ResourceInstanceDeleteBecauseNoModuleResourceInstanceDeleteBecauseNoResourceConfig" + _ResourceInstanceChangeActionReason_name_7 = "ResourceInstanceReplaceByRequest" + _ResourceInstanceChangeActionReason_name_8 = "ResourceInstanceReplaceBecauseTainted" + _ResourceInstanceChangeActionReason_name_9 = "ResourceInstanceDeleteBecauseWrongRepetition" ) var ( - _ResourceInstanceChangeActionReason_index_4 = [...]uint8{0, 39, 72, 108, 150} - _ResourceInstanceChangeActionReason_index_5 = [...]uint8{0, 37, 82} + _ResourceInstanceChangeActionReason_index_5 = [...]uint8{0, 39, 72, 108, 150} + _ResourceInstanceChangeActionReason_index_6 = [...]uint8{0, 37, 82} ) func (i ResourceInstanceChangeActionReason) String() string { @@ -46,22 +48,24 @@ func (i ResourceInstanceChangeActionReason) String() string { return _ResourceInstanceChangeActionReason_name_0 case i == 33: return _ResourceInstanceChangeActionReason_name_1 - case i == 63: + case i == 35: return _ResourceInstanceChangeActionReason_name_2 - case i == 65: + case i == 63: return _ResourceInstanceChangeActionReason_name_3 + case i == 65: + return _ResourceInstanceChangeActionReason_name_4 case 67 <= i && i <= 70: i -= 67 - return _ResourceInstanceChangeActionReason_name_4[_ResourceInstanceChangeActionReason_index_4[i]:_ResourceInstanceChangeActionReason_index_4[i+1]] + return _ResourceInstanceChangeActionReason_name_5[_ResourceInstanceChangeActionReason_index_5[i]:_ResourceInstanceChangeActionReason_index_5[i+1]] case 77 <= i && i <= 78: i -= 77 - return _ResourceInstanceChangeActionReason_name_5[_ResourceInstanceChangeActionReason_index_5[i]:_ResourceInstanceChangeActionReason_index_5[i+1]] + return _ResourceInstanceChangeActionReason_name_6[_ResourceInstanceChangeActionReason_index_6[i]:_ResourceInstanceChangeActionReason_index_6[i+1]] case i == 82: - return _ResourceInstanceChangeActionReason_name_6 - case i == 84: return _ResourceInstanceChangeActionReason_name_7 - case i == 87: + case i == 84: return _ResourceInstanceChangeActionReason_name_8 + case i == 87: + return _ResourceInstanceChangeActionReason_name_9 default: return "ResourceInstanceChangeActionReason(" + strconv.FormatInt(int64(i), 10) + ")" } diff --git a/internal/states/statefile/version4.go b/internal/states/statefile/version4.go index 2cdde4be7107..18a775bd5f62 100644 --- a/internal/states/statefile/version4.go +++ b/internal/states/statefile/version4.go @@ -638,6 +638,8 @@ func decodeCheckableObjectKindV4(in string) addrs.CheckableKind { return addrs.CheckableResource case "output": return addrs.CheckableOutputValue + case "check": + return addrs.CheckableCheck default: // We'll treat anything else as invalid just as a concession to // forward-compatible parsing, in case a later version of Terraform @@ -652,6 +654,8 @@ func encodeCheckableObjectKindV4(in addrs.CheckableKind) string { return "resource" case addrs.CheckableOutputValue: return "output" + case addrs.CheckableCheck: + return "check" default: panic(fmt.Sprintf("unsupported checkable object kind %s", in)) } diff --git a/internal/terraform/context_apply_checks_test.go b/internal/terraform/context_apply_checks_test.go new file mode 100644 index 000000000000..b18642605690 --- /dev/null +++ b/internal/terraform/context_apply_checks_test.go @@ -0,0 +1,689 @@ +package terraform + +import ( + "testing" + + "github.com/zclconf/go-cty/cty" + + "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/checks" + "github.com/hashicorp/terraform/internal/configs/configschema" + "github.com/hashicorp/terraform/internal/plans" + "github.com/hashicorp/terraform/internal/providers" + "github.com/hashicorp/terraform/internal/states" + "github.com/hashicorp/terraform/internal/tfdiags" +) + +// This file contains 'integration' tests for the Terraform check blocks. +// +// These tests could live in context_apply_test or context_apply2_test but given +// the size of those files, it makes sense to keep these check related tests +// grouped together. + +type checksTestingStatus struct { + status checks.Status + messages []string +} + +func TestContextChecks(t *testing.T) { + tests := map[string]struct { + configs map[string]string + plan map[string]checksTestingStatus + planError string + apply map[string]checksTestingStatus + applyError string + state *states.State + provider *MockProvider + providerHook func(*MockProvider) + }{ + "passing": { + configs: map[string]string{ + "main.tf": ` +provider "checks" {} + +check "passing" { + data "checks_object" "positive" {} + + assert { + condition = data.checks_object.positive.number >= 0 + error_message = "negative number" + } +} +`, + }, + plan: map[string]checksTestingStatus{ + "passing": { + status: checks.StatusPass, + }, + }, + apply: map[string]checksTestingStatus{ + "passing": { + status: checks.StatusPass, + }, + }, + provider: &MockProvider{ + Meta: "checks", + GetProviderSchemaResponse: &providers.GetProviderSchemaResponse{ + DataSources: map[string]providers.Schema{ + "checks_object": { + Block: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "number": { + Type: cty.Number, + Computed: true, + }, + }, + }, + }, + }, + }, + ReadDataSourceFn: func(request providers.ReadDataSourceRequest) providers.ReadDataSourceResponse { + return providers.ReadDataSourceResponse{ + State: cty.ObjectVal(map[string]cty.Value{ + "number": cty.NumberIntVal(0), + }), + } + }, + }, + }, + "failing": { + configs: map[string]string{ + "main.tf": ` +provider "checks" {} + +check "failing" { + data "checks_object" "positive" {} + + assert { + condition = data.checks_object.positive.number >= 0 + error_message = "negative number" + } +} +`, + }, + plan: map[string]checksTestingStatus{ + "failing": { + status: checks.StatusFail, + messages: []string{"negative number"}, + }, + }, + apply: map[string]checksTestingStatus{ + "failing": { + status: checks.StatusFail, + messages: []string{"negative number"}, + }, + }, + provider: &MockProvider{ + Meta: "checks", + GetProviderSchemaResponse: &providers.GetProviderSchemaResponse{ + DataSources: map[string]providers.Schema{ + "checks_object": { + Block: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "number": { + Type: cty.Number, + Computed: true, + }, + }, + }, + }, + }, + }, + ReadDataSourceFn: func(request providers.ReadDataSourceRequest) providers.ReadDataSourceResponse { + return providers.ReadDataSourceResponse{ + State: cty.ObjectVal(map[string]cty.Value{ + "number": cty.NumberIntVal(-1), + }), + } + }, + }, + }, + "mixed": { + configs: map[string]string{ + "main.tf": ` +provider "checks" {} + +check "failing" { + data "checks_object" "neutral" {} + + assert { + condition = data.checks_object.neutral.number >= 0 + error_message = "negative number" + } + + assert { + condition = data.checks_object.neutral.number < 0 + error_message = "positive number" + } +} +`, + }, + plan: map[string]checksTestingStatus{ + "failing": { + status: checks.StatusFail, + messages: []string{"positive number"}, + }, + }, + apply: map[string]checksTestingStatus{ + "failing": { + status: checks.StatusFail, + messages: []string{"positive number"}, + }, + }, + provider: &MockProvider{ + Meta: "checks", + GetProviderSchemaResponse: &providers.GetProviderSchemaResponse{ + DataSources: map[string]providers.Schema{ + "checks_object": { + Block: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "number": { + Type: cty.Number, + Computed: true, + }, + }, + }, + }, + }, + }, + ReadDataSourceFn: func(request providers.ReadDataSourceRequest) providers.ReadDataSourceResponse { + return providers.ReadDataSourceResponse{ + State: cty.ObjectVal(map[string]cty.Value{ + "number": cty.NumberIntVal(0), + }), + } + }, + }, + }, + "nested data blocks reload during apply": { + configs: map[string]string{ + "main.tf": ` +provider "checks" {} + +data "checks_object" "data_block" {} + +check "data_block" { + assert { + condition = data.checks_object.data_block.number >= 0 + error_message = "negative number" + } +} + +check "nested_data_block" { + data "checks_object" "nested_data_block" {} + + assert { + condition = data.checks_object.nested_data_block.number >= 0 + error_message = "negative number" + } +} +`, + }, + plan: map[string]checksTestingStatus{ + "nested_data_block": { + status: checks.StatusFail, + messages: []string{"negative number"}, + }, + "data_block": { + status: checks.StatusFail, + messages: []string{"negative number"}, + }, + }, + apply: map[string]checksTestingStatus{ + "nested_data_block": { + status: checks.StatusPass, + }, + "data_block": { + status: checks.StatusFail, + messages: []string{"negative number"}, + }, + }, + provider: &MockProvider{ + Meta: "checks", + GetProviderSchemaResponse: &providers.GetProviderSchemaResponse{ + DataSources: map[string]providers.Schema{ + "checks_object": { + Block: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "number": { + Type: cty.Number, + Computed: true, + }, + }, + }, + }, + }, + }, + ReadDataSourceFn: func(request providers.ReadDataSourceRequest) providers.ReadDataSourceResponse { + return providers.ReadDataSourceResponse{ + State: cty.ObjectVal(map[string]cty.Value{ + "number": cty.NumberIntVal(-1), + }), + } + }, + }, + providerHook: func(provider *MockProvider) { + provider.ReadDataSourceFn = func(request providers.ReadDataSourceRequest) providers.ReadDataSourceResponse { + // The data returned by the data sources are changing + // between the plan and apply stage. The nested data block + // will update to reflect this while the normal data block + // will not detect the change. + return providers.ReadDataSourceResponse{ + State: cty.ObjectVal(map[string]cty.Value{ + "number": cty.NumberIntVal(0), + }), + } + } + }, + }, + "returns unknown for unknown config": { + configs: map[string]string{ + "main.tf": ` +provider "checks" {} + +resource "checks_object" "resource_block" {} + +check "resource_block" { + data "checks_object" "data_block" { + id = checks_object.resource_block.id + } + + assert { + condition = data.checks_object.data_block.number >= 0 + error_message = "negative number" + } +} +`, + }, + plan: map[string]checksTestingStatus{ + "resource_block": { + status: checks.StatusUnknown, + }, + }, + apply: map[string]checksTestingStatus{ + "resource_block": { + status: checks.StatusPass, + }, + }, + provider: &MockProvider{ + Meta: "checks", + GetProviderSchemaResponse: &providers.GetProviderSchemaResponse{ + ResourceTypes: map[string]providers.Schema{ + "checks_object": { + Block: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "id": { + Type: cty.String, + Computed: true, + }, + }, + }, + }, + }, + DataSources: map[string]providers.Schema{ + "checks_object": { + Block: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "id": { + Type: cty.String, + Required: true, + }, + "number": { + Type: cty.Number, + Computed: true, + }, + }, + }, + }, + }, + }, + PlanResourceChangeFn: func(request providers.PlanResourceChangeRequest) providers.PlanResourceChangeResponse { + return providers.PlanResourceChangeResponse{ + PlannedState: cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + }), + } + }, + ApplyResourceChangeFn: func(request providers.ApplyResourceChangeRequest) providers.ApplyResourceChangeResponse { + return providers.ApplyResourceChangeResponse{ + NewState: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("7A9F887D-44C7-4281-80E5-578E41F99DFC"), + }), + } + }, + ReadDataSourceFn: func(request providers.ReadDataSourceRequest) providers.ReadDataSourceResponse { + values := request.Config.AsValueMap() + if id, ok := values["id"]; ok { + if id.IsKnown() && id.AsString() == "7A9F887D-44C7-4281-80E5-578E41F99DFC" { + return providers.ReadDataSourceResponse{ + State: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("7A9F887D-44C7-4281-80E5-578E41F99DFC"), + "number": cty.NumberIntVal(0), + }), + } + } + } + + return providers.ReadDataSourceResponse{ + Diagnostics: tfdiags.Diagnostics{tfdiags.Sourceless(tfdiags.Error, "shouldn't make it here", "really shouldn't make it here")}, + } + }, + }, + }, + "failing nested data source doesn't block the plan": { + configs: map[string]string{ + "main.tf": ` +provider "checks" {} + +check "error" { + data "checks_object" "data_block" {} + + assert { + condition = data.checks_object.data_block.number >= 0 + error_message = "negative number" + } +} +`, + }, + plan: map[string]checksTestingStatus{ + "error": { + status: checks.StatusFail, + messages: []string{ + "data source read failed: something bad happened and the provider couldn't read the data source", + }, + }, + }, + apply: map[string]checksTestingStatus{ + "error": { + status: checks.StatusFail, + messages: []string{ + "data source read failed: something bad happened and the provider couldn't read the data source", + }, + }, + }, + provider: &MockProvider{ + Meta: "checks", + GetProviderSchemaResponse: &providers.GetProviderSchemaResponse{ + DataSources: map[string]providers.Schema{ + "checks_object": { + Block: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "number": { + Type: cty.Number, + Computed: true, + }, + }, + }, + }, + }, + }, + ReadDataSourceFn: func(request providers.ReadDataSourceRequest) providers.ReadDataSourceResponse { + return providers.ReadDataSourceResponse{ + Diagnostics: tfdiags.Diagnostics{tfdiags.Sourceless(tfdiags.Error, "data source read failed", "something bad happened and the provider couldn't read the data source")}, + } + }, + }, + }, + "check failing in state and passing after plan and apply": { + configs: map[string]string{ + "main.tf": ` +provider "checks" {} + +resource "checks_object" "resource" { + number = 0 +} + +check "passing" { + assert { + condition = checks_object.resource.number >= 0 + error_message = "negative number" + } +} +`, + }, + state: states.BuildState(func(state *states.SyncState) { + state.SetResourceInstanceCurrent( + addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "checks_object", + Name: "resource", + }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), + &states.ResourceInstanceObjectSrc{ + Status: states.ObjectReady, + AttrsJSON: []byte(`{"number": -1}`), + }, + addrs.AbsProviderConfig{ + Provider: addrs.NewDefaultProvider("test"), + Module: addrs.RootModule, + }) + }), + plan: map[string]checksTestingStatus{ + "passing": { + status: checks.StatusPass, + }, + }, + apply: map[string]checksTestingStatus{ + "passing": { + status: checks.StatusPass, + }, + }, + provider: &MockProvider{ + Meta: "checks", + GetProviderSchemaResponse: &providers.GetProviderSchemaResponse{ + ResourceTypes: map[string]providers.Schema{ + "checks_object": { + Block: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "number": { + Type: cty.Number, + Required: true, + }, + }, + }, + }, + }, + }, + PlanResourceChangeFn: func(request providers.PlanResourceChangeRequest) providers.PlanResourceChangeResponse { + return providers.PlanResourceChangeResponse{ + PlannedState: cty.ObjectVal(map[string]cty.Value{ + "number": cty.NumberIntVal(0), + }), + } + }, + ApplyResourceChangeFn: func(request providers.ApplyResourceChangeRequest) providers.ApplyResourceChangeResponse { + return providers.ApplyResourceChangeResponse{ + NewState: cty.ObjectVal(map[string]cty.Value{ + "number": cty.NumberIntVal(0), + }), + } + }, + }, + }, + "failing data source does block the plan": { + configs: map[string]string{ + "main.tf": ` +provider "checks" {} + +data "checks_object" "data_block" {} + +check "error" { + assert { + condition = data.checks_object.data_block.number >= 0 + error_message = "negative number" + } +} +`, + }, + planError: "data source read failed: something bad happened and the provider couldn't read the data source", + provider: &MockProvider{ + Meta: "checks", + GetProviderSchemaResponse: &providers.GetProviderSchemaResponse{ + DataSources: map[string]providers.Schema{ + "checks_object": { + Block: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "number": { + Type: cty.Number, + Computed: true, + }, + }, + }, + }, + }, + }, + ReadDataSourceFn: func(request providers.ReadDataSourceRequest) providers.ReadDataSourceResponse { + return providers.ReadDataSourceResponse{ + Diagnostics: tfdiags.Diagnostics{tfdiags.Sourceless(tfdiags.Error, "data source read failed", "something bad happened and the provider couldn't read the data source")}, + } + }, + }, + }, + "invalid reference into check block": { + configs: map[string]string{ + "main.tf": ` +provider "checks" {} + +data "checks_object" "data_block" { + id = data.checks_object.nested_data_block.id +} + +check "error" { + data "checks_object" "nested_data_block" {} + + assert { + condition = data.checks_object.data_block.number >= 0 + error_message = "negative number" + } +} +`, + }, + planError: "Reference to scoped resource: The referenced data resource \"checks_object\" \"nested_data_block\" is not available from this context.", + provider: &MockProvider{ + Meta: "checks", + GetProviderSchemaResponse: &providers.GetProviderSchemaResponse{ + DataSources: map[string]providers.Schema{ + "checks_object": { + Block: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "id": { + Type: cty.String, + Computed: true, + Optional: true, + }, + }, + }, + }, + }, + }, + ReadDataSourceFn: func(request providers.ReadDataSourceRequest) providers.ReadDataSourceResponse { + input := request.Config.AsValueMap() + if _, ok := input["id"]; ok { + return providers.ReadDataSourceResponse{ + State: request.Config, + } + } + + return providers.ReadDataSourceResponse{ + State: cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + }), + } + }, + }, + }, + } + for name, test := range tests { + t.Run(name, func(t *testing.T) { + configs := testModuleInline(t, test.configs) + ctx := testContext2(t, &ContextOpts{ + Providers: map[addrs.Provider]providers.Factory{ + addrs.NewDefaultProvider(test.provider.Meta.(string)): testProviderFuncFixed(test.provider), + }, + }) + + initialState := states.NewState() + if test.state != nil { + initialState = test.state + } + + plan, diags := ctx.Plan(configs, initialState, &PlanOpts{ + Mode: plans.NormalMode, + }) + if validateError(t, "planning", test.planError, diags) { + return + } + validateCheckResults(t, "planning", test.plan, plan.Checks) + + if test.providerHook != nil { + // This gives an opportunity to change the behaviour of the + // provider between the plan and apply stages. + test.providerHook(test.provider) + } + + state, diags := ctx.Apply(plan, configs) + if validateError(t, "apply", test.applyError, diags) { + return + } + validateCheckResults(t, "apply", test.apply, state.CheckResults) + }) + } +} + +func validateError(t *testing.T, stage string, expected string, actual tfdiags.Diagnostics) bool { + if expected != "" { + if !actual.HasErrors() { + t.Errorf("expected %s to error with \"%s\", but no errors were returned", stage, expected) + } else if expected != actual.Err().Error() { + t.Errorf("expected %s to error with \"%s\" but found \"%s\"", stage, expected, actual.Err()) + } + return true + } + + assertNoErrors(t, actual) + return false +} + +func validateCheckResults(t *testing.T, stage string, expected map[string]checksTestingStatus, actual *states.CheckResults) { + + // Just a quick sanity check that the plan or apply process didn't create + // some non-existent checks. + if len(expected) != len(actual.ConfigResults.Keys()) { + t.Errorf("expected %d check results but found %d after %s", len(expected), len(actual.ConfigResults.Keys()), stage) + } + + // Now, lets make sure the checks all match what we expect. + for check, want := range expected { + results := actual.GetObjectResult(addrs.Check{ + Name: check, + }.Absolute(addrs.RootModuleInstance)) + + if results.Status != want.status { + t.Errorf("%s: wanted %s but got %s after %s", check, want.status, results.Status, stage) + } + + if len(want.messages) != len(results.FailureMessages) { + t.Errorf("%s: expected %d failure messages but had %d after %s", check, len(want.messages), len(results.FailureMessages), stage) + } + + max := len(want.messages) + if len(results.FailureMessages) > max { + max = len(results.FailureMessages) + } + + for ix := 0; ix < max; ix++ { + var expected, actual string + if ix < len(want.messages) { + expected = want.messages[ix] + } + if ix < len(results.FailureMessages) { + actual = results.FailureMessages[ix] + } + + // Order matters! + if actual != expected { + t.Errorf("%s: expected failure message at %d to be \"%s\" but was \"%s\" after %s", check, ix, expected, actual, stage) + } + } + + } +} diff --git a/internal/terraform/eval_conditions.go b/internal/terraform/eval_conditions.go index 9edd9199be24..881f5dd59520 100644 --- a/internal/terraform/eval_conditions.go +++ b/internal/terraform/eval_conditions.go @@ -67,9 +67,8 @@ type checkResult struct { FailureMessage string } -func evalCheckRule(typ addrs.CheckRuleType, rule *configs.CheckRule, ctx EvalContext, self addrs.Checkable, keyData instances.RepetitionData, severity hcl.DiagnosticSeverity) (checkResult, tfdiags.Diagnostics) { +func validateCheckRule(typ addrs.CheckRuleType, rule *configs.CheckRule, ctx EvalContext, self addrs.Checkable, keyData instances.RepetitionData) (string, *hcl.EvalContext, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics - const errInvalidCondition = "Invalid condition result" refs, moreDiags := lang.ReferencesInExpr(rule.Condition) diags = diags.Append(moreDiags) @@ -77,29 +76,47 @@ func evalCheckRule(typ addrs.CheckRuleType, rule *configs.CheckRule, ctx EvalCon diags = diags.Append(moreDiags) refs = append(refs, moreRefs...) - var selfReference addrs.Referenceable - // Only resource postconditions can refer to self - if typ == addrs.ResourcePostcondition { + var selfReference, sourceReference addrs.Referenceable + switch typ { + case addrs.ResourcePostcondition: switch s := self.(type) { case addrs.AbsResourceInstance: + // Only resource postconditions can refer to self selfReference = s.Resource default: panic(fmt.Sprintf("Invalid self reference type %t", self)) } + case addrs.CheckAssertion: + switch s := self.(type) { + case addrs.AbsCheck: + // Only check blocks have scoped resources so need to specify their + // source. + sourceReference = s.Check + default: + panic(fmt.Sprintf("Invalid source reference type %t", self)) + } } - scope := ctx.EvaluationScope(selfReference, nil, keyData) + scope := ctx.EvaluationScope(selfReference, sourceReference, keyData) hclCtx, moreDiags := scope.EvalContext(refs) diags = diags.Append(moreDiags) - resultVal, hclDiags := rule.Condition.Value(hclCtx) - diags = diags.Append(hclDiags) + errorMessage, moreDiags := evalCheckErrorMessage(rule.ErrorMessage, hclCtx) + diags = diags.Append(moreDiags) + + return errorMessage, hclCtx, diags +} +func evalCheckRule(typ addrs.CheckRuleType, rule *configs.CheckRule, ctx EvalContext, self addrs.Checkable, keyData instances.RepetitionData, severity hcl.DiagnosticSeverity) (checkResult, tfdiags.Diagnostics) { // NOTE: Intentionally not passing the caller's selected severity in here, // because this reports errors in the configuration itself, not the failure // of an otherwise-valid condition. - errorMessage, moreDiags := evalCheckErrorMessage(rule.ErrorMessage, hclCtx) - diags = diags.Append(moreDiags) + errorMessage, hclCtx, diags := validateCheckRule(typ, rule, ctx, self, keyData) + + const errInvalidCondition = "Invalid condition result" + + resultVal, hclDiags := rule.Condition.Value(hclCtx) + diags = diags.Append(hclDiags) if diags.HasErrors() { log.Printf("[TRACE] evalCheckRule: %s: %s", typ, diags.Err().Error()) @@ -107,6 +124,19 @@ func evalCheckRule(typ addrs.CheckRuleType, rule *configs.CheckRule, ctx EvalCon } if !resultVal.IsKnown() { + + // Check assertions warn if a status is unknown. + if typ == addrs.CheckAssertion { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagWarning, + Summary: fmt.Sprintf("%s known after apply", typ.Description()), + Detail: "The condition could not be evaluated at this time, a result will be known when this plan is applied.", + Subject: rule.Condition.Range().Ptr(), + Expression: rule.Condition, + EvalContext: hclCtx, + }) + } + // We'll wait until we've learned more, then. return checkResult{Status: checks.StatusUnknown}, diags } diff --git a/internal/terraform/evaluate_valid_test.go b/internal/terraform/evaluate_valid_test.go index 920bdbef2d7b..2d8d5734b900 100644 --- a/internal/terraform/evaluate_valid_test.go +++ b/internal/terraform/evaluate_valid_test.go @@ -5,6 +5,7 @@ import ( "github.com/hashicorp/hcl/v2" "github.com/hashicorp/hcl/v2/hclsyntax" + "github.com/zclconf/go-cty/cty" "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/configs/configschema" @@ -14,41 +15,42 @@ import ( func TestStaticValidateReferences(t *testing.T) { tests := []struct { Ref string + Src addrs.Referenceable WantErr string }{ { - "aws_instance.no_count", - ``, + Ref: "aws_instance.no_count", + WantErr: ``, }, { - "aws_instance.count", - ``, + Ref: "aws_instance.count", + WantErr: ``, }, { - "aws_instance.count[0]", - ``, + Ref: "aws_instance.count[0]", + WantErr: ``, }, { - "aws_instance.nonexist", - `Reference to undeclared resource: A managed resource "aws_instance" "nonexist" has not been declared in the root module.`, + Ref: "aws_instance.nonexist", + WantErr: `Reference to undeclared resource: A managed resource "aws_instance" "nonexist" has not been declared in the root module.`, }, { - "beep.boop", - `Reference to undeclared resource: A managed resource "beep" "boop" has not been declared in the root module. + Ref: "beep.boop", + WantErr: `Reference to undeclared resource: A managed resource "beep" "boop" has not been declared in the root module. Did you mean the data resource data.beep.boop?`, }, { - "aws_instance.no_count[0]", - `Unexpected resource instance key: Because aws_instance.no_count does not have "count" or "for_each" set, references to it must not include an index key. Remove the bracketed index to refer to the single instance of this resource.`, + Ref: "aws_instance.no_count[0]", + WantErr: `Unexpected resource instance key: Because aws_instance.no_count does not have "count" or "for_each" set, references to it must not include an index key. Remove the bracketed index to refer to the single instance of this resource.`, }, { - "aws_instance.count.foo", + Ref: "aws_instance.count.foo", // In this case we return two errors that are somewhat redundant with // one another, but we'll accept that because they both report the // problem from different perspectives and so give the user more // opportunity to understand what's going on here. - `2 problems: + WantErr: `2 problems: - Missing resource instance key: Because aws_instance.count has "count" set, its attributes must be accessed on specific instances. @@ -57,12 +59,21 @@ For example, to correlate with indices of a referring resource, use: - Unsupported attribute: This object has no argument, nested block, or exported attribute named "foo".`, }, { - "boop_instance.yep", - ``, + Ref: "boop_instance.yep", + WantErr: ``, }, { - "boop_whatever.nope", - `Invalid resource type: A managed resource type "boop_whatever" is not supported by provider "registry.terraform.io/foobar/beep".`, + Ref: "boop_whatever.nope", + WantErr: `Invalid resource type: A managed resource type "boop_whatever" is not supported by provider "registry.terraform.io/foobar/beep".`, + }, + { + Ref: "data.boop_data.boop_nested", + WantErr: `Reference to scoped resource: The referenced data resource "boop_data" "boop_nested" is not available from this context.`, + }, + { + Ref: "data.boop_data.boop_nested", + WantErr: ``, + Src: addrs.Check{Name: "foo"}, }, } @@ -80,6 +91,16 @@ For example, to correlate with indices of a referring resource, use: // intentional mismatch between resource type prefix and provider type "boop_instance": {}, }, + DataSources: map[string]*configschema.Block{ + "boop_data": { + Attributes: map[string]*configschema.Attribute{ + "id": { + Type: cty.String, + Optional: true, + }, + }, + }, + }, }, }), } @@ -100,7 +121,7 @@ For example, to correlate with indices of a referring resource, use: Evaluator: evaluator, } - diags = data.StaticValidateReferences(refs, nil, nil) + diags = data.StaticValidateReferences(refs, nil, test.Src) if diags.HasErrors() { if test.WantErr == "" { t.Fatalf("Unexpected diagnostics: %s", diags.Err()) diff --git a/internal/terraform/graph_builder.go b/internal/terraform/graph_builder.go index 1c69ee41f82d..0e6d01277dca 100644 --- a/internal/terraform/graph_builder.go +++ b/internal/terraform/graph_builder.go @@ -32,6 +32,11 @@ func (b *BasicGraphBuilder) Build(path addrs.ModuleInstance) (*Graph, tfdiags.Di var lastStepStr string for _, step := range b.Steps { + + if _, ok := step.(*checkTransformer); ok { + log.Printf("my line") + } + if step == nil { continue } diff --git a/internal/terraform/graph_builder_apply.go b/internal/terraform/graph_builder_apply.go index 29e60ff6a0d8..57c1660a845b 100644 --- a/internal/terraform/graph_builder_apply.go +++ b/internal/terraform/graph_builder_apply.go @@ -110,6 +110,13 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { Config: b.Config, }, + // Add nodes and edges for check block assertions. Check block data + // sources were added earlier. + &checkTransformer{ + Config: b.Config, + Operation: b.Operation, + }, + // Attach the state &AttachStateTransformer{State: b.State}, diff --git a/internal/terraform/graph_builder_plan.go b/internal/terraform/graph_builder_plan.go index f085e9aa2dad..ffa88207cd62 100644 --- a/internal/terraform/graph_builder_plan.go +++ b/internal/terraform/graph_builder_plan.go @@ -127,6 +127,13 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer { Planning: true, }, + // Add nodes and edges for the check block assertions. Check block data + // sources were added earlier. + &checkTransformer{ + Config: b.Config, + Operation: b.Operation, + }, + // Add orphan resources &OrphanResourceInstanceTransformer{ Concrete: b.ConcreteResourceOrphan, diff --git a/internal/terraform/node_check.go b/internal/terraform/node_check.go new file mode 100644 index 000000000000..486bfe107c0d --- /dev/null +++ b/internal/terraform/node_check.go @@ -0,0 +1,164 @@ +package terraform + +import ( + "log" + + "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/checks" + "github.com/hashicorp/terraform/internal/configs" + "github.com/hashicorp/terraform/internal/dag" + "github.com/hashicorp/terraform/internal/lang" + "github.com/hashicorp/terraform/internal/tfdiags" +) + +var ( + _ GraphNodeModulePath = (*nodeReportCheck)(nil) + _ GraphNodeExecutable = (*nodeReportCheck)(nil) +) + +// nodeReportCheck calls the ReportCheckableObjects function for our assertions +// within the check blocks. +// +// We need this to happen before the checks are actually verified and before any +// nested data blocks, so the creator of this structure should make sure this +// node is a parent of any nested data blocks. +// +// This needs to be separate to nodeExpandCheck, because the actual checks +// should happen after referenced data blocks rather than before. +type nodeReportCheck struct { + addr addrs.ConfigCheck +} + +func (n *nodeReportCheck) ModulePath() addrs.Module { + return n.addr.Module +} + +func (n *nodeReportCheck) Execute(ctx EvalContext, _ walkOperation) tfdiags.Diagnostics { + exp := ctx.InstanceExpander() + modInsts := exp.ExpandModule(n.ModulePath()) + + instAddrs := addrs.MakeSet[addrs.Checkable]() + for _, modAddr := range modInsts { + instAddrs.Add(n.addr.Check.Absolute(modAddr)) + } + ctx.Checks().ReportCheckableObjects(n.addr, instAddrs) + return nil +} + +func (n *nodeReportCheck) Name() string { + return n.addr.String() + " (report)" +} + +var ( + _ GraphNodeModulePath = (*nodeExpandCheck)(nil) + _ GraphNodeDynamicExpandable = (*nodeExpandCheck)(nil) + _ GraphNodeReferencer = (*nodeExpandCheck)(nil) +) + +// nodeExpandCheck creates child nodes that actually execute the assertions for +// a given check block. +// +// This must happen after any other nodes/resources/data sources that are +// referenced, so we implement GraphNodeReferencer. +// +// This needs to be separate to nodeReportCheck as nodeReportCheck must happen +// first, while nodeExpandCheck must execute after any referenced blocks. +type nodeExpandCheck struct { + addr addrs.ConfigCheck + config *configs.Check + + makeInstance func(addrs.AbsCheck, *configs.Check) dag.Vertex +} + +func (n *nodeExpandCheck) ModulePath() addrs.Module { + return n.addr.Module +} + +func (n *nodeExpandCheck) DynamicExpand(ctx EvalContext) (*Graph, error) { + exp := ctx.InstanceExpander() + modInsts := exp.ExpandModule(n.ModulePath()) + + var g Graph + for _, modAddr := range modInsts { + testAddr := n.addr.Check.Absolute(modAddr) + log.Printf("[TRACE] nodeExpandCheck: Node for %s", testAddr) + g.Add(n.makeInstance(testAddr, n.config)) + } + addRootNodeToGraph(&g) + + return &g, nil +} + +func (n *nodeExpandCheck) References() []*addrs.Reference { + var refs []*addrs.Reference + for _, assert := range n.config.Asserts { + condition, _ := lang.ReferencesInExpr(assert.Condition) + message, _ := lang.ReferencesInExpr(assert.ErrorMessage) + refs = append(refs, condition...) + refs = append(refs, message...) + } + return refs +} + +func (n *nodeExpandCheck) Name() string { + return n.addr.String() + " (expand)" +} + +var ( + _ GraphNodeModuleInstance = (*nodeCheckAssert)(nil) + _ GraphNodeExecutable = (*nodeCheckAssert)(nil) +) + +type nodeCheckAssert struct { + addr addrs.AbsCheck + config *configs.Check + + // We only want to actually execute the checks during the plan and apply + // operations, but we still want to validate our config during + // other operations. + executeChecks bool +} + +func (n *nodeCheckAssert) ModulePath() addrs.Module { + return n.Path().Module() +} + +func (n *nodeCheckAssert) Path() addrs.ModuleInstance { + return n.addr.Module +} + +func (n *nodeCheckAssert) Execute(ctx EvalContext, _ walkOperation) tfdiags.Diagnostics { + + // We only want to actually execute the checks during specific + // operations, such as plan and applies. + if n.executeChecks { + if status := ctx.Checks().ObjectCheckStatus(n.addr); status == checks.StatusFail || status == checks.StatusError { + // This check is already failing, so we won't try and evaluate it. + // This typically means there was an error in a data block within + // the check block. + return nil + } + + return evalCheckRules( + addrs.CheckAssertion, + n.config.Asserts, + ctx, + n.addr, + EvalDataForNoInstanceKey, + tfdiags.Warning) + + } + + // Otherwise let's still validate the config and references and return + // diagnostics if references do not exist etc. + var diags tfdiags.Diagnostics + for _, assert := range n.config.Asserts { + _, _, moreDiags := validateCheckRule(addrs.CheckAssertion, assert, ctx, n.addr, EvalDataForNoInstanceKey) + diags = diags.Append(moreDiags) + } + return diags +} + +func (n *nodeCheckAssert) Name() string { + return n.addr.String() + " (assertions)" +} diff --git a/internal/terraform/node_resource_abstract_instance.go b/internal/terraform/node_resource_abstract_instance.go index 709298ee519a..0d458fd29cb6 100644 --- a/internal/terraform/node_resource_abstract_instance.go +++ b/internal/terraform/node_resource_abstract_instance.go @@ -9,6 +9,7 @@ import ( "github.com/zclconf/go-cty/cty" "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/checks" "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/configs/configschema" "github.com/hashicorp/terraform/internal/instances" @@ -1587,7 +1588,31 @@ func (n *NodeAbstractResourceInstance) planDataSource(ctx EvalContext, checkRule return nil, nil, keyData, diags } - unmarkedConfigVal, configMarkPaths := configVal.UnmarkDeepWithPaths() + check, nested := n.nestedInCheckBlock() + if nested { + // Going forward from this point, the only reason we will fail is + // that the data source fails to load its data. Normally, this would + // cancel the entire plan and this error message would bubble its way + // back up to the user. + // + // But, if we are in a check block then we don't want this data block to + // cause the plan to fail. We also need to report a status on the data + // block so the check processing later on knows whether to attempt to + // process the checks. Either we'll report the data block as failed + // if/when we load the data block later, or we want to report it as a + // success overall. + // + // Therefore, we create a deferred function here that will check if the + // status for the check has been updated yet, and if not we will set it + // to be StatusPass. The rest of this function will only update the + // status if it should be StatusFail. + defer func() { + status := ctx.Checks().ObjectCheckStatus(check.Addr().Absolute(n.Addr.Module)) + if status == checks.StatusUnknown { + ctx.Checks().ReportCheckResult(check.Addr().Absolute(n.Addr.Module), addrs.CheckDataResource, 0, checks.StatusPass) + } + }() + } configKnown := configVal.IsWhollyKnown() depsPending := n.dependenciesHavePendingChanges(ctx) @@ -1620,6 +1645,7 @@ func (n *NodeAbstractResourceInstance) planDataSource(ctx EvalContext, checkRule reason = plans.ResourceInstanceReadBecauseDependencyPending } + unmarkedConfigVal, configMarkPaths := configVal.UnmarkDeepWithPaths() proposedNewVal := objchange.PlannedDataResourceObject(schema, unmarkedConfigVal) proposedNewVal = proposedNewVal.MarkWithPaths(configMarkPaths) @@ -1653,16 +1679,83 @@ func (n *NodeAbstractResourceInstance) planDataSource(ctx EvalContext, checkRule // can read the data source into the state. newVal, readDiags := n.readDataSource(ctx, configVal) diags = diags.Append(readDiags) - if diags.HasErrors() { - return nil, nil, keyData, diags + + // Now we've loaded the data, and diags tells us whether we were successful + // or not, we are going to create our plannedChange and our + // proposedNewState. + var plannedChange *plans.ResourceInstanceChange + var plannedNewState *states.ResourceInstanceObject + + // If we are a nested block, then we want to create a plannedChange that + // tells Terraform to reload the data block during the apply stage even if + // we managed to get the data now. + // Another consideration is that if we failed to load the data, we need to + // disguise that for a nested block. Nested blocks will report the overall + // check as failed but won't affect the rest of the plan operation or block + // an apply operation. + + if nested { + // Let's fix things up for a nested data block. + // + // A nested data block doesn't error, and creates a planned change. So, + // if we encountered an error we'll tidy up newVal so it makes sense + // and handle the error. We'll also create the plannedChange if + // appropriate. + + if diags.HasErrors() { + // If we had errors, then we can cover that up by marking the new + // state as unknown. + unmarkedConfigVal, configMarkPaths := configVal.UnmarkDeepWithPaths() + newVal = objchange.PlannedDataResourceObject(schema, unmarkedConfigVal) + newVal = newVal.MarkWithPaths(configMarkPaths) + + // We still want to report the check as failed even if we are still + // letting it run again during the apply stage. + ctx.Checks().ReportCheckFailure(check.Addr().Absolute(n.Addr.Module), addrs.CheckDataResource, 0, diags.Err().Error()) + + // Also, let's hide the errors so that execution can continue as + // normal. + diags = tfdiags.WithErrorsAsWarnings(diags) + } + + if !skipPlanChanges { + // refreshOnly plans cannot produce planned changes, so we only do + // this if skipPlanChanges is false. + plannedChange = &plans.ResourceInstanceChange{ + Addr: n.Addr, + PrevRunAddr: n.prevRunAddr(ctx), + ProviderAddr: n.ResolvedProvider, + Change: plans.Change{ + Action: plans.Read, + Before: priorVal, + After: newVal, + }, + ActionReason: plans.ResourceInstanceReadBecauseCheckNested, + } + } + } - plannedNewState := &states.ResourceInstanceObject{ - Value: newVal, - Status: states.ObjectReady, + if !diags.HasErrors() { + // Finally, let's make our new state. + plannedNewState = &states.ResourceInstanceObject{ + Value: newVal, + Status: states.ObjectReady, + } } - return nil, plannedNewState, keyData, diags + return plannedChange, plannedNewState, keyData, diags +} + +// nestedInCheckBlock determines if this resource is nested in a Check config +// block. If so, this resource will be loaded during both plan and apply +// operations to make sure the check is always giving the latest information. +func (n *NodeAbstractResourceInstance) nestedInCheckBlock() (*configs.Check, bool) { + if n.Config.Container != nil { + check, ok := n.Config.Container.(*configs.Check) + return check, ok + } + return nil, false } // dependenciesHavePendingChanges determines whether any managed resource the @@ -1784,7 +1877,19 @@ func (n *NodeAbstractResourceInstance) applyDataSource(ctx EvalContext, planned newVal, readDiags := n.readDataSource(ctx, configVal) diags = diags.Append(readDiags) - if diags.HasErrors() { + + if check, nested := n.nestedInCheckBlock(); nested { + // We're just going to jump in here and hide away any erros for nested + // data blocks. + if diags.HasErrors() { + ctx.Checks().ReportCheckFailure(check.Addr().Absolute(n.Addr.Module), addrs.CheckDataResource, 0, diags.Err().Error()) + return nil, keyData, tfdiags.WithErrorsAsWarnings(diags) + } + + // If no errors, just remember to report this as a success and continue + // as normal. + ctx.Checks().ReportCheckResult(check.Addr().Absolute(n.Addr.Module), addrs.CheckDataResource, 0, checks.StatusPass) + } else if diags.HasErrors() { return nil, keyData, diags } diff --git a/internal/terraform/node_resource_plan_instance.go b/internal/terraform/node_resource_plan_instance.go index aac13bedc82d..7d7211fe834c 100644 --- a/internal/terraform/node_resource_plan_instance.go +++ b/internal/terraform/node_resource_plan_instance.go @@ -5,13 +5,13 @@ import ( "log" "sort" + "github.com/zclconf/go-cty/cty" + + "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/instances" "github.com/hashicorp/terraform/internal/plans" "github.com/hashicorp/terraform/internal/states" "github.com/hashicorp/terraform/internal/tfdiags" - "github.com/zclconf/go-cty/cty" - - "github.com/hashicorp/terraform/internal/addrs" ) // NodePlannableResourceInstance represents a _single_ resource @@ -66,6 +66,7 @@ func (n *NodePlannableResourceInstance) Execute(ctx EvalContext, op walkOperatio } func (n *NodePlannableResourceInstance) dataResourceExecute(ctx EvalContext) (diags tfdiags.Diagnostics) { + config := n.Config addr := n.ResourceInstanceAddr() diff --git a/internal/terraform/testdata/static-validate-refs/static-validate-refs.tf b/internal/terraform/testdata/static-validate-refs/static-validate-refs.tf index 3667a4e11f35..2f71e21713d6 100644 --- a/internal/terraform/testdata/static-validate-refs/static-validate-refs.tf +++ b/internal/terraform/testdata/static-validate-refs/static-validate-refs.tf @@ -21,3 +21,12 @@ resource "boop_whatever" "nope" { data "beep" "boop" { } + +check "foo" { + data "boop_data" "boop_nested" {} + + assert { + condition = data.boop_data.boop_nested.id == null + error_message = "check failed" + } +} diff --git a/internal/terraform/transform_check.go b/internal/terraform/transform_check.go new file mode 100644 index 000000000000..ac4a73aed1bd --- /dev/null +++ b/internal/terraform/transform_check.go @@ -0,0 +1,114 @@ +package terraform + +import ( + "log" + + "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/configs" + "github.com/hashicorp/terraform/internal/dag" +) + +type checkTransformer struct { + // Config for the entire module. + Config *configs.Config + + // Operation is the current operation this node will be part of. + Operation walkOperation +} + +var _ GraphTransformer = (*checkTransformer)(nil) + +func (t *checkTransformer) Transform(graph *Graph) error { + return t.transform(graph, t.Config, graph.Vertices()) +} + +func (t *checkTransformer) transform(g *Graph, cfg *configs.Config, allNodes []dag.Vertex) error { + moduleAddr := cfg.Path + + for _, check := range cfg.Module.Checks { + configAddr := check.Addr().InModule(moduleAddr) + + // We want to create a node for each check block. This node will execute + // after anything it references, and will update the checks object + // embedded in the plan and/or state. + + log.Printf("[TRACE] checkTransformer: Nodes and edges for %s", configAddr) + expand := &nodeExpandCheck{ + addr: configAddr, + config: check, + makeInstance: func(addr addrs.AbsCheck, cfg *configs.Check) dag.Vertex { + return &nodeCheckAssert{ + addr: addr, + config: cfg, + executeChecks: t.ExecuteChecks(), + } + }, + } + g.Add(expand) + + // We also need to report the checks we are going to execute before we + // try and execute them. + if t.ReportChecks() { + report := &nodeReportCheck{ + addr: configAddr, + } + g.Add(report) + + // This part ensures we report our checks before our nested data + // block executes and attempts to report on a check. + for _, other := range allNodes { + if resource, isResource := other.(GraphNodeConfigResource); isResource { + resourceAddr := resource.ResourceAddr() + if !resourceAddr.Module.Equal(moduleAddr) { + // This resource isn't in the same module as our check + // so skip it. + continue + } + + resourceCfg := cfg.Module.ResourceByAddr(resourceAddr.Resource) + if resourceCfg != nil && resourceCfg.Container != nil && resourceCfg.Container.Accessible(check.Addr()) { + // Make sure we report our checks before we execute any + // embedded data resource. + g.Connect(dag.BasicEdge(other, report)) + continue + } + } + } + } + } + + for _, child := range cfg.Children { + if err := t.transform(g, child, allNodes); err != nil { + return err + } + } + + return nil +} + +// ReportChecks returns true if this operation should report any check blocks +// that it is about to execute. +// +// This is generally only true for planning operations, as apply operations +// recreate the expected checks from the plan. +func (t *checkTransformer) ReportChecks() bool { + return t.Operation == walkPlan || t.Operation == walkPlanDestroy +} + +// ExecuteChecks returns true if this operation should actually execute any +// check blocks in the config. +// +// If this returns false we will still create and execute check nodes in the +// graph, but they will only validate things like references and syntax. +func (t *checkTransformer) ExecuteChecks() bool { + switch t.Operation { + case walkPlan, walkPlanDestroy, walkApply, walkDestroy: + // We only actually execute the checks for plan and apply operations. + return true + default: + // For everything else, we still want to validate the checks make sense + // logically and syntactically, but we won't actually resolve the check + // conditions. + return false + } +} diff --git a/internal/tfdiags/errors_as_warnings.go b/internal/tfdiags/errors_as_warnings.go new file mode 100644 index 000000000000..c208a3601431 --- /dev/null +++ b/internal/tfdiags/errors_as_warnings.go @@ -0,0 +1,41 @@ +package tfdiags + +type diagForceWarningSeverity struct { + wrapped Diagnostic +} + +func WithErrorsAsWarnings(diags Diagnostics) Diagnostics { + if len(diags) == 0 { + return nil + } + + ret := make(Diagnostics, len(diags)) + for i, diag := range diags { + if diag.Severity() == Error { + ret[i] = diagForceWarningSeverity{diag} + } else { + ret[i] = diag + } + } + return ret +} + +func (diag diagForceWarningSeverity) Severity() Severity { + return Warning +} + +func (diag diagForceWarningSeverity) Description() Description { + return diag.wrapped.Description() +} + +func (diag diagForceWarningSeverity) Source() Source { + return diag.wrapped.Source() +} + +func (diag diagForceWarningSeverity) FromExpr() *FromExpr { + return diag.wrapped.FromExpr() +} + +func (diag diagForceWarningSeverity) ExtraInfo() any { + return diag.wrapped.ExtraInfo() +} From 40b2ad79842896b00efcbd9a1aecbb09e4f912dc Mon Sep 17 00:00:00 2001 From: Liam Cervante Date: Wed, 22 Mar 2023 15:21:30 +0100 Subject: [PATCH 5/8] address comments --- internal/addrs/check.go | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/internal/addrs/check.go b/internal/addrs/check.go index fb5655479343..74b20d620809 100644 --- a/internal/addrs/check.go +++ b/internal/addrs/check.go @@ -2,6 +2,10 @@ package addrs import "fmt" +// Check is the address of a check block within a module. +// +// For now, checks do not support meta arguments such as "count" or "for_each" +// so this address uniquely describes a single check within a module. type Check struct { referenceable Name string @@ -11,6 +15,8 @@ func (c Check) String() string { return fmt.Sprintf("check.%s", c.Name) } +// InModule returns a ConfigCheck from the receiver and the given module +// address. func (c Check) InModule(modAddr Module) ConfigCheck { return ConfigCheck{ Module: modAddr, @@ -18,6 +24,8 @@ func (c Check) InModule(modAddr Module) ConfigCheck { } } +// Absolute returns an AbsCheck from the receiver and the given module instance +// address. func (c Check) Absolute(modAddr ModuleInstance) AbsCheck { return AbsCheck{ Module: modAddr, @@ -35,6 +43,10 @@ func (c Check) UniqueKey() UniqueKey { func (c Check) uniqueKeySigil() {} +// ConfigCheck is an address for a check block within a configuration. +// +// This contains a Check address and a Module address, meaning this describes +// a check block within the entire configuration. type ConfigCheck struct { Module Module Check Check @@ -59,6 +71,12 @@ func (c ConfigCheck) String() string { return fmt.Sprintf("%s.%s", c.Module, c.Check) } +// AbsCheck is an absolute address for a check block under a given module path. +// +// This contains an actual ModuleInstance address (compared to the Module within +// a ConfigCheck), meaning this uniquely describes a check block within the +// entire configuration after any "count" or "foreach" meta arguments have been +// evaluated on the containing module. type AbsCheck struct { Module ModuleInstance Check Check @@ -72,6 +90,11 @@ func (c AbsCheck) UniqueKey() UniqueKey { func (c AbsCheck) checkableSigil() {} +// CheckRule returns an address for a given rule type within the check block. +// +// There will be at most one CheckDataResource rule within a check block (with +// an index of 0). There will be at least one, but potentially many, +// CheckAssertion rules within a check block. func (c AbsCheck) CheckRule(typ CheckRuleType, i int) CheckRule { return CheckRule{ Container: c, @@ -80,6 +103,7 @@ func (c AbsCheck) CheckRule(typ CheckRuleType, i int) CheckRule { } } +// ConfigCheckable returns the ConfigCheck address for this absolute reference. func (c AbsCheck) ConfigCheckable() ConfigCheckable { return ConfigCheck{ Module: c.Module.Module(), From 9debe8cc4ee83b87cbf65a77d8f7fd16907c6c5f Mon Sep 17 00:00:00 2001 From: Liam Cervante Date: Thu, 23 Mar 2023 09:24:41 +0100 Subject: [PATCH 6/8] address comments --- internal/terraform/graph_builder.go | 5 ----- internal/terraform/node_resource_plan_instance.go | 7 +++---- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/internal/terraform/graph_builder.go b/internal/terraform/graph_builder.go index 0e6d01277dca..1c69ee41f82d 100644 --- a/internal/terraform/graph_builder.go +++ b/internal/terraform/graph_builder.go @@ -32,11 +32,6 @@ func (b *BasicGraphBuilder) Build(path addrs.ModuleInstance) (*Graph, tfdiags.Di var lastStepStr string for _, step := range b.Steps { - - if _, ok := step.(*checkTransformer); ok { - log.Printf("my line") - } - if step == nil { continue } diff --git a/internal/terraform/node_resource_plan_instance.go b/internal/terraform/node_resource_plan_instance.go index 7d7211fe834c..aac13bedc82d 100644 --- a/internal/terraform/node_resource_plan_instance.go +++ b/internal/terraform/node_resource_plan_instance.go @@ -5,13 +5,13 @@ import ( "log" "sort" - "github.com/zclconf/go-cty/cty" - - "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/instances" "github.com/hashicorp/terraform/internal/plans" "github.com/hashicorp/terraform/internal/states" "github.com/hashicorp/terraform/internal/tfdiags" + "github.com/zclconf/go-cty/cty" + + "github.com/hashicorp/terraform/internal/addrs" ) // NodePlannableResourceInstance represents a _single_ resource @@ -66,7 +66,6 @@ func (n *NodePlannableResourceInstance) Execute(ctx EvalContext, op walkOperatio } func (n *NodePlannableResourceInstance) dataResourceExecute(ctx EvalContext) (diags tfdiags.Diagnostics) { - config := n.Config addr := n.ResourceInstanceAddr() From 1c74f84e4b7611e2825348fa3d9a52e3a6467a0c Mon Sep 17 00:00:00 2001 From: Liam Cervante Date: Thu, 23 Mar 2023 13:43:35 +0100 Subject: [PATCH 7/8] don't execute checks during destroy operations --- internal/terraform/transform_check.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/terraform/transform_check.go b/internal/terraform/transform_check.go index ac4a73aed1bd..db39e3816802 100644 --- a/internal/terraform/transform_check.go +++ b/internal/terraform/transform_check.go @@ -92,7 +92,7 @@ func (t *checkTransformer) transform(g *Graph, cfg *configs.Config, allNodes []d // This is generally only true for planning operations, as apply operations // recreate the expected checks from the plan. func (t *checkTransformer) ReportChecks() bool { - return t.Operation == walkPlan || t.Operation == walkPlanDestroy + return t.Operation == walkPlan } // ExecuteChecks returns true if this operation should actually execute any @@ -102,7 +102,7 @@ func (t *checkTransformer) ReportChecks() bool { // graph, but they will only validate things like references and syntax. func (t *checkTransformer) ExecuteChecks() bool { switch t.Operation { - case walkPlan, walkPlanDestroy, walkApply, walkDestroy: + case walkPlan, walkApply: // We only actually execute the checks for plan and apply operations. return true default: From f355d0b69b1b5e10657aa42d0dbbca53b9e0f1cf Mon Sep 17 00:00:00 2001 From: Liam Cervante Date: Thu, 23 Mar 2023 15:37:31 +0100 Subject: [PATCH 8/8] don't even include check nodes for destroy operations --- internal/terraform/transform_check.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/internal/terraform/transform_check.go b/internal/terraform/transform_check.go index db39e3816802..5f469b8ad3b0 100644 --- a/internal/terraform/transform_check.go +++ b/internal/terraform/transform_check.go @@ -23,6 +23,18 @@ func (t *checkTransformer) Transform(graph *Graph) error { } func (t *checkTransformer) transform(g *Graph, cfg *configs.Config, allNodes []dag.Vertex) error { + + if t.Operation == walkDestroy || t.Operation == walkPlanDestroy { + // Don't include anything about checks during destroy operations. + // + // For other plan and normal apply operations we do everything, for + // destroy operations we do nothing. For any other operations we still + // include the check nodes, but we don't actually execute the checks + // instead we still validate their references and make sure their + // conditions make sense etc. + return nil + } + moduleAddr := cfg.Path for _, check := range cfg.Module.Checks {