From c9bc7e8479744a073b4910ba0bc4975181023f12 Mon Sep 17 00:00:00 2001 From: Liam Cervante Date: Mon, 10 Jul 2023 12:33:45 +0200 Subject: [PATCH] Add input validation into the 'checks' outputs and tracking (#33481) --- internal/addrs/check_rule.go | 9 + internal/addrs/checkable.go | 17 +- internal/addrs/checkablekind_string.go | 12 +- internal/addrs/checkruletype_string.go | 5 +- internal/addrs/input_variable.go | 67 ++++ internal/checks/state_init.go | 16 + internal/command/jsonchecks/objects.go | 15 + .../plans/internal/planproto/planfile.pb.go | 143 +++---- .../plans/internal/planproto/planfile.proto | 1 + internal/plans/planfile/tfplan.go | 4 + internal/states/statefile/version4.go | 4 + internal/terraform/eval_variable.go | 349 ++++++++++-------- internal/terraform/eval_variable_test.go | 23 ++ internal/terraform/graph_builder_eval.go | 4 +- internal/terraform/graph_builder_plan.go | 4 +- internal/terraform/node_module_variable.go | 28 +- internal/terraform/node_root_variable.go | 15 +- internal/terraform/node_root_variable_test.go | 13 + .../terraform/transform_module_variable.go | 17 +- internal/terraform/transform_variable.go | 5 + 20 files changed, 501 insertions(+), 250 deletions(-) diff --git a/internal/addrs/check_rule.go b/internal/addrs/check_rule.go index 6d4c1dbc22..4a60e5e8db 100644 --- a/internal/addrs/check_rule.go +++ b/internal/addrs/check_rule.go @@ -44,6 +44,12 @@ func (c CheckRule) String() string { return fmt.Sprintf("%s.postcondition[%d]", container, c.Index) case OutputPrecondition: return fmt.Sprintf("%s.precondition[%d]", container, c.Index) + case CheckDataResource: + return fmt.Sprintf("%s.data[%d]", container, c.Index) + case CheckAssertion: + return fmt.Sprintf("%s.assert[%d]", container, c.Index) + case InputValidation: + return fmt.Sprintf("%s.validation[%d]", container, c.Index) default: // This should not happen return fmt.Sprintf("%s.condition[%d]", container, c.Index) @@ -85,6 +91,7 @@ const ( OutputPrecondition CheckRuleType = 3 CheckDataResource CheckRuleType = 4 CheckAssertion CheckRuleType = 5 + InputValidation CheckRuleType = 6 ) // Description returns a human-readable description of the check type. This is @@ -101,6 +108,8 @@ func (c CheckRuleType) Description() string { return "Check block data resource" case CheckAssertion: return "Check block assertion" + case InputValidation: + return "Input variable validation" default: // This should not happen return "Condition" diff --git a/internal/addrs/checkable.go b/internal/addrs/checkable.go index 55d41098b9..908ff671db 100644 --- a/internal/addrs/checkable.go +++ b/internal/addrs/checkable.go @@ -52,10 +52,11 @@ 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' + CheckableKindInvalid CheckableKind = 0 + CheckableResource CheckableKind = 'R' + CheckableOutputValue CheckableKind = 'O' + CheckableCheck CheckableKind = 'C' + CheckableInputVariable CheckableKind = 'I' ) // ConfigCheckable is an interfaces implemented by address types that represent @@ -179,6 +180,14 @@ func ParseCheckableStr(kind CheckableKind, src string) (Checkable, tfdiags.Diagn } return Check{Name: name}.Absolute(path), diags + case CheckableInputVariable: + name, nameDiags := getCheckableName("var", "variable value") + diags = diags.Append(nameDiags) + if diags.HasErrors() { + return nil, diags + } + return InputVariable{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 e0657cb6a1..80578d4b7e 100644 --- a/internal/addrs/checkablekind_string.go +++ b/internal/addrs/checkablekind_string.go @@ -12,13 +12,15 @@ func _() { _ = x[CheckableResource-82] _ = x[CheckableOutputValue-79] _ = x[CheckableCheck-67] + _ = x[CheckableInputVariable-73] } const ( _CheckableKind_name_0 = "CheckableKindInvalid" _CheckableKind_name_1 = "CheckableCheck" - _CheckableKind_name_2 = "CheckableOutputValue" - _CheckableKind_name_3 = "CheckableResource" + _CheckableKind_name_2 = "CheckableInputVariable" + _CheckableKind_name_3 = "CheckableOutputValue" + _CheckableKind_name_4 = "CheckableResource" ) func (i CheckableKind) String() string { @@ -27,10 +29,12 @@ func (i CheckableKind) String() string { return _CheckableKind_name_0 case i == 67: return _CheckableKind_name_1 - case i == 79: + case i == 73: return _CheckableKind_name_2 - case i == 82: + case i == 79: return _CheckableKind_name_3 + case i == 82: + return _CheckableKind_name_4 default: return "CheckableKind(" + strconv.FormatInt(int64(i), 10) + ")" } diff --git a/internal/addrs/checkruletype_string.go b/internal/addrs/checkruletype_string.go index 7865793665..18726e211a 100644 --- a/internal/addrs/checkruletype_string.go +++ b/internal/addrs/checkruletype_string.go @@ -14,11 +14,12 @@ func _() { _ = x[OutputPrecondition-3] _ = x[CheckDataResource-4] _ = x[CheckAssertion-5] + _ = x[InputValidation-6] } -const _CheckRuleType_name = "InvalidConditionResourcePreconditionResourcePostconditionOutputPreconditionCheckDataResourceCheckAssertion" +const _CheckRuleType_name = "InvalidConditionResourcePreconditionResourcePostconditionOutputPreconditionCheckDataResourceCheckAssertionInputValidation" -var _CheckRuleType_index = [...]uint8{0, 16, 36, 57, 75, 92, 106} +var _CheckRuleType_index = [...]uint8{0, 16, 36, 57, 75, 92, 106, 121} func (i CheckRuleType) String() string { if i < 0 || i >= CheckRuleType(len(_CheckRuleType_index)-1) { diff --git a/internal/addrs/input_variable.go b/internal/addrs/input_variable.go index c38d55e975..0e59d3f79d 100644 --- a/internal/addrs/input_variable.go +++ b/internal/addrs/input_variable.go @@ -32,6 +32,13 @@ func (v InputVariable) Absolute(m ModuleInstance) AbsInputVariableInstance { } } +func (v InputVariable) InModule(module Module) ConfigInputVariable { + return ConfigInputVariable{ + Module: module, + Variable: v, + } +} + // AbsInputVariableInstance is the address of an input variable within a // particular module instance. type AbsInputVariableInstance struct { @@ -39,6 +46,8 @@ type AbsInputVariableInstance struct { Variable InputVariable } +var _ Checkable = AbsInputVariableInstance{} + // InputVariable returns the absolute address of the input variable of the // given name inside the receiving module instance. func (m ModuleInstance) InputVariable(name string) AbsInputVariableInstance { @@ -57,3 +66,61 @@ func (v AbsInputVariableInstance) String() string { return fmt.Sprintf("%s.%s", v.Module.String(), v.Variable.String()) } + +func (v AbsInputVariableInstance) UniqueKey() UniqueKey { + return absInputVariableInstanceUniqueKey(v.String()) +} + +func (v AbsInputVariableInstance) checkableSigil() {} + +func (v AbsInputVariableInstance) CheckRule(typ CheckRuleType, i int) CheckRule { + return CheckRule{ + Container: v, + Type: typ, + Index: i, + } +} + +func (v AbsInputVariableInstance) ConfigCheckable() ConfigCheckable { + return ConfigInputVariable{ + Module: v.Module.Module(), + Variable: v.Variable, + } +} + +func (v AbsInputVariableInstance) CheckableKind() CheckableKind { + return CheckableInputVariable +} + +type ConfigInputVariable struct { + Module Module + Variable InputVariable +} + +var _ ConfigCheckable = ConfigInputVariable{} + +func (v ConfigInputVariable) UniqueKey() UniqueKey { + return configInputVariableUniqueKey(v.String()) +} + +func (v ConfigInputVariable) configCheckableSigil() {} + +func (v ConfigInputVariable) CheckableKind() CheckableKind { + return CheckableInputVariable +} + +func (v ConfigInputVariable) String() string { + if len(v.Module) == 0 { + return v.Variable.String() + } + + return fmt.Sprintf("%s.%s", v.Module.String(), v.Variable.String()) +} + +type configInputVariableUniqueKey string + +func (k configInputVariableUniqueKey) uniqueKeySigil() {} + +type absInputVariableInstanceUniqueKey string + +func (k absInputVariableInstanceUniqueKey) uniqueKeySigil() {} diff --git a/internal/checks/state_init.go b/internal/checks/state_init.go index 9acd40d7a0..8e2713d037 100644 --- a/internal/checks/state_init.go +++ b/internal/checks/state_init.go @@ -68,6 +68,22 @@ func collectInitialStatuses(into addrs.Map[addrs.ConfigCheckable, *configCheckab into.Put(addr, st) } + for _, v := range cfg.Module.Variables { + addr := v.Addr().InModule(moduleAddr) + + vs := len(v.Validations) + if vs == 0 { + continue + } + + st := &configCheckableState{} + st.checkTypes = map[addrs.CheckRuleType]int{ + addrs.InputValidation: vs, + } + + into.Put(addr, st) + } + // Must also visit child modules to collect everything for _, child := range cfg.Children { collectInitialStatuses(into, child) diff --git a/internal/command/jsonchecks/objects.go b/internal/command/jsonchecks/objects.go index 32c8a6637d..30a56abf8a 100644 --- a/internal/command/jsonchecks/objects.go +++ b/internal/command/jsonchecks/objects.go @@ -59,6 +59,17 @@ func makeStaticObjectAddr(addr addrs.ConfigCheckable) staticObjectAddr { if !addr.Module.IsRoot() { ret["module"] = addr.Module.String() } + case addrs.ConfigInputVariable: + if kind := addr.CheckableKind(); kind != addrs.CheckableInputVariable { + // Something has gone very wrong + panic(fmt.Sprintf("%T has CheckableKind %s", addr, kind)) + } + + ret["kind"] = "var" + ret["name"] = addr.Variable.Name + if !addr.Module.IsRoot() { + ret["module"] = addr.Module.String() + } default: panic(fmt.Sprintf("unsupported ConfigCheckable implementation %T", addr)) } @@ -89,6 +100,10 @@ func makeDynamicObjectAddr(addr addrs.Checkable) dynamicObjectAddr { if !addr.Module.IsRoot() { ret["module"] = addr.Module.String() } + case addrs.AbsInputVariableInstance: + if !addr.Module.IsRoot() { + ret["module"] = addr.Module.String() + } default: panic(fmt.Sprintf("unsupported Checkable implementation %T", addr)) } diff --git a/internal/plans/internal/planproto/planfile.pb.go b/internal/plans/internal/planproto/planfile.pb.go index 0c3e65ba32..92cb68ced6 100644 --- a/internal/plans/internal/planproto/planfile.pb.go +++ b/internal/plans/internal/planproto/planfile.pb.go @@ -279,10 +279,11 @@ func (CheckResults_Status) EnumDescriptor() ([]byte, []int) { type CheckResults_ObjectKind int32 const ( - CheckResults_UNSPECIFIED CheckResults_ObjectKind = 0 - CheckResults_RESOURCE CheckResults_ObjectKind = 1 - CheckResults_OUTPUT_VALUE CheckResults_ObjectKind = 2 - CheckResults_CHECK CheckResults_ObjectKind = 3 + CheckResults_UNSPECIFIED CheckResults_ObjectKind = 0 + CheckResults_RESOURCE CheckResults_ObjectKind = 1 + CheckResults_OUTPUT_VALUE CheckResults_ObjectKind = 2 + CheckResults_CHECK CheckResults_ObjectKind = 3 + CheckResults_INPUT_VARIABLE CheckResults_ObjectKind = 4 ) // Enum value maps for CheckResults_ObjectKind. @@ -292,12 +293,14 @@ var ( 1: "RESOURCE", 2: "OUTPUT_VALUE", 3: "CHECK", + 4: "INPUT_VARIABLE", } CheckResults_ObjectKind_value = map[string]int32{ - "UNSPECIFIED": 0, - "RESOURCE": 1, - "OUTPUT_VALUE": 2, - "CHECK": 3, + "UNSPECIFIED": 0, + "RESOURCE": 1, + "OUTPUT_VALUE": 2, + "CHECK": 3, + "INPUT_VARIABLE": 4, } ) @@ -1449,7 +1452,7 @@ var file_planfile_proto_rawDesc = []byte{ 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, 0xe8, 0x03, 0x0a, 0x0c, 0x43, 0x68, 0x65, + 0x65, 0x6e, 0x73, 0x69, 0x74, 0x69, 0x76, 0x65, 0x22, 0xfc, 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, 0x6c, 0x74, 0x73, 0x2e, 0x4f, 0x62, @@ -1475,70 +1478,72 @@ var file_planfile_proto_rawDesc = []byte{ 0x67, 0x65, 0x73, 0x22, 0x34, 0x0a, 0x06, 0x53, 0x74, 0x61, 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, 0x48, 0x0a, 0x0a, 0x4f, 0x62, 0x6a, + 0x0a, 0x05, 0x45, 0x52, 0x52, 0x4f, 0x52, 0x10, 0x03, 0x22, 0x5c, 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, 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, 0x22, 0x1b, 0x0a, 0x09, 0x49, 0x6d, 0x70, 0x6f, 0x72, 0x74, 0x69, - 0x6e, 0x67, 0x12, 0x0e, 0x0a, 0x02, 0x69, 0x64, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x02, - 0x69, 0x64, 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, + 0x4b, 0x10, 0x03, 0x12, 0x12, 0x0a, 0x0e, 0x49, 0x4e, 0x50, 0x55, 0x54, 0x5f, 0x56, 0x41, 0x52, + 0x49, 0x41, 0x42, 0x4c, 0x45, 0x10, 0x04, 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, 0x22, 0x1b, 0x0a, 0x09, 0x49, 0x6d, 0x70, + 0x6f, 0x72, 0x74, 0x69, 0x6e, 0x67, 0x12, 0x0e, 0x0a, 0x02, 0x69, 0x64, 0x18, 0x01, 0x20, 0x01, + 0x28, 0x09, 0x52, 0x02, 0x69, 0x64, 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 47b5478353..784608ab0f 100644 --- a/internal/plans/internal/planproto/planfile.proto +++ b/internal/plans/internal/planproto/planfile.proto @@ -253,6 +253,7 @@ message CheckResults { RESOURCE = 1; OUTPUT_VALUE = 2; CHECK = 3; + INPUT_VARIABLE = 4; } message ObjectResult { diff --git a/internal/plans/planfile/tfplan.go b/internal/plans/planfile/tfplan.go index d7f5cb0ff3..dd53fc63a4 100644 --- a/internal/plans/planfile/tfplan.go +++ b/internal/plans/planfile/tfplan.go @@ -120,6 +120,8 @@ func readTfplan(r io.Reader) (*plans.Plan, error) { objKind = addrs.CheckableOutputValue case planproto.CheckResults_CHECK: objKind = addrs.CheckableCheck + case planproto.CheckResults_INPUT_VARIABLE: + objKind = addrs.CheckableInputVariable default: return nil, fmt.Errorf("aggregate check results for %s have unsupported object kind %s", rawCRs.ConfigAddr, objKind) } @@ -545,6 +547,8 @@ func writeTfplan(plan *plans.Plan, w io.Writer) error { pcrs.Kind = planproto.CheckResults_OUTPUT_VALUE case addrs.CheckableCheck: pcrs.Kind = planproto.CheckResults_CHECK + case addrs.CheckableInputVariable: + pcrs.Kind = planproto.CheckResults_INPUT_VARIABLE default: return fmt.Errorf("checkable configuration %s has unsupported object type kind %s", configElem.Key, kind) } diff --git a/internal/states/statefile/version4.go b/internal/states/statefile/version4.go index 6191e350b7..ddddaa0258 100644 --- a/internal/states/statefile/version4.go +++ b/internal/states/statefile/version4.go @@ -643,6 +643,8 @@ func decodeCheckableObjectKindV4(in string) addrs.CheckableKind { return addrs.CheckableOutputValue case "check": return addrs.CheckableCheck + case "var": + return addrs.CheckableInputVariable default: // We'll treat anything else as invalid just as a concession to // forward-compatible parsing, in case a later version of Terraform @@ -659,6 +661,8 @@ func encodeCheckableObjectKindV4(in addrs.CheckableKind) string { return "output" case addrs.CheckableCheck: return "check" + case addrs.CheckableInputVariable: + return "var" default: panic(fmt.Sprintf("unsupported checkable object kind %s", in)) } diff --git a/internal/terraform/eval_variable.go b/internal/terraform/eval_variable.go index 7f291be0e7..f998ea0fad 100644 --- a/internal/terraform/eval_variable.go +++ b/internal/terraform/eval_variable.go @@ -10,12 +10,14 @@ import ( "github.com/hashicorp/hcl/v2" "github.com/hashicorp/hcl/v2/gohcl" + "github.com/zclconf/go-cty/cty" + "github.com/zclconf/go-cty/cty/convert" + "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/checks" "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/lang/marks" "github.com/hashicorp/terraform/internal/tfdiags" - "github.com/zclconf/go-cty/cty" - "github.com/zclconf/go-cty/cty/convert" ) func prepareFinalInputVariableValue(addr addrs.AbsInputVariableInstance, raw *InputValue, cfg *configs.Variable) (cty.Value, tfdiags.Diagnostics) { @@ -202,6 +204,16 @@ func evalVariableValidations(addr addrs.AbsInputVariableInstance, config *config } log.Printf("[TRACE] evalVariableValidations: validating %s", addr) + checkState := ctx.Checks() + if !checkState.ConfigHasChecks(addr.ConfigCheckable()) { + // We have nothing to do if this object doesn't have any checks, + // but the "rules" slice should agree that we don't. + if ct := len(config.Validations); ct != 0 { + panic(fmt.Sprintf("check state says that %s should have no rules, but it has %d", addr, ct)) + } + return diags + } + // Variable nodes evaluate in the parent module to where they were declared // because the value expression (n.Expr, if set) comes from the calling // "module" block in the parent module. @@ -229,169 +241,186 @@ func evalVariableValidations(addr addrs.AbsInputVariableInstance, config *config Functions: ctx.EvaluationScope(nil, nil, EvalDataForNoInstanceKey).Functions(), } - for _, validation := range config.Validations { - const errInvalidCondition = "Invalid variable validation result" - const errInvalidValue = "Invalid value for variable" - var ruleDiags tfdiags.Diagnostics - - result, moreDiags := validation.Condition.Value(hclCtx) - ruleDiags = ruleDiags.Append(moreDiags) - errorValue, errorDiags := validation.ErrorMessage.Value(hclCtx) - - // The following error handling is a workaround to preserve backwards - // compatibility. Due to an implementation quirk, all prior versions of - // Terraform would treat error messages specified using JSON - // configuration syntax (.tf.json) as string literals, even if they - // contained the "${" template expression operator. This behaviour did - // not match that of HCL configuration syntax, where a template - // expression would result in a validation error. - // - // As a result, users writing or generating JSON configuration syntax - // may have specified error messages which are invalid template - // expressions. As we add support for error message expressions, we are - // unable to perfectly distinguish between these two cases. - // - // To ensure that we don't break backwards compatibility, we have the - // below fallback logic if the error message fails to evaluate. This - // should only have any effect for JSON configurations. The gohcl - // DecodeExpression function behaves differently when the source of the - // expression is a JSON configuration file and a nil context is passed. - if errorDiags.HasErrors() { - // Attempt to decode the expression as a string literal. Passing - // nil as the context forces a JSON syntax string value to be - // interpreted as a string literal. - var errorString string - moreErrorDiags := gohcl.DecodeExpression(validation.ErrorMessage, nil, &errorString) - if !moreErrorDiags.HasErrors() { - // Decoding succeeded, meaning that this is a JSON syntax - // string value. We rewrap that as a cty value to allow later - // decoding to succeed. - errorValue = cty.StringVal(errorString) - - // This warning diagnostic explains this odd behaviour, while - // giving us an escape hatch to change this to a hard failure - // in some future Terraform 1.x version. - errorDiags = hcl.Diagnostics{ - &hcl.Diagnostic{ - Severity: hcl.DiagWarning, - Summary: "Validation error message expression is invalid", - Detail: fmt.Sprintf("The error message provided could not be evaluated as an expression, so Terraform is interpreting it as a string literal.\n\nIn future versions of Terraform, this will be considered an error. Please file a GitHub issue if this would break your workflow.\n\n%s", errorDiags.Error()), - Subject: validation.ErrorMessage.Range().Ptr(), - Context: validation.DeclRange.Ptr(), - Expression: validation.ErrorMessage, - EvalContext: hclCtx, - }, - } - } - - // We want to either report the original diagnostics if the - // fallback failed, or the warning generated above if it succeeded. - ruleDiags = ruleDiags.Append(errorDiags) - } - + for ix, validation := range config.Validations { + result, ruleDiags := evalVariableValidation(validation, hclCtx, addr, config, expr) diags = diags.Append(ruleDiags) - if ruleDiags.HasErrors() { - log.Printf("[TRACE] evalVariableValidations: %s rule %s check rule evaluation failed: %s", addr, validation.DeclRange, ruleDiags.Err().Error()) - } - if !result.IsKnown() { - log.Printf("[TRACE] evalVariableValidations: %s rule %s condition value is unknown, so skipping validation for now", addr, validation.DeclRange) - continue // We'll wait until we've learned more, then. - } - if result.IsNull() { - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: errInvalidCondition, - Detail: "Validation condition expression must return either true or false, not null.", - Subject: validation.Condition.Range().Ptr(), - Expression: validation.Condition, - EvalContext: hclCtx, - }) - continue - } - var err error - result, err = convert.Convert(result, cty.Bool) - if err != nil { - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: errInvalidCondition, - Detail: fmt.Sprintf("Invalid validation condition result value: %s.", tfdiags.FormatError(err)), - Subject: validation.Condition.Range().Ptr(), - Expression: validation.Condition, - EvalContext: hclCtx, - }) - continue - } - - // Validation condition may be marked if the input variable is bound to - // a sensitive value. This is irrelevant to the validation process, so - // we discard the marks now. - result, _ = result.Unmark() - - if result.True() { - continue - } - - var errorMessage string - if !errorDiags.HasErrors() && errorValue.IsKnown() && !errorValue.IsNull() { - var err error - errorValue, err = convert.Convert(errorValue, cty.String) - if err != nil { - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid error message", - Detail: fmt.Sprintf("Unsuitable value for error message: %s.", tfdiags.FormatError(err)), - Subject: validation.ErrorMessage.Range().Ptr(), - Expression: validation.ErrorMessage, - EvalContext: hclCtx, - }) - } else { - if marks.Has(errorValue, marks.Sensitive) { - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - - Summary: "Error message refers to sensitive values", - Detail: `The error expression used to explain this condition refers to sensitive values. Terraform will not display the resulting message. - -You can correct this by removing references to sensitive values, or by carefully using the nonsensitive() function if the expression will not reveal the sensitive data.`, - - Subject: validation.ErrorMessage.Range().Ptr(), - Expression: validation.ErrorMessage, - EvalContext: hclCtx, - }) - errorMessage = "The error message included a sensitive value, so it will not be displayed." - } else { - errorMessage = strings.TrimSpace(errorValue.AsString()) - } - } - } - if errorMessage == "" { - errorMessage = "Failed to evaluate condition error message." - } - - if expr != nil { - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: errInvalidValue, - Detail: fmt.Sprintf("%s\n\nThis was checked by the validation rule at %s.", errorMessage, validation.DeclRange.String()), - Subject: expr.Range().Ptr(), - Expression: validation.Condition, - EvalContext: hclCtx, - }) + log.Printf("[TRACE] evalVariableValidations: %s status is now %s", addr, result.Status) + if result.Status == checks.StatusFail { + checkState.ReportCheckFailure(addr, addrs.InputValidation, ix, result.FailureMessage) } else { - // Since we don't have a source expression for a root module - // variable, we'll just report the error from the perspective - // of the variable declaration itself. - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: errInvalidValue, - Detail: fmt.Sprintf("%s\n\nThis was checked by the validation rule at %s.", errorMessage, validation.DeclRange.String()), - Subject: config.DeclRange.Ptr(), - Expression: validation.Condition, - EvalContext: hclCtx, - }) + checkState.ReportCheckResult(addr, addrs.InputValidation, ix, result.Status) } } return diags } + +func evalVariableValidation(validation *configs.CheckRule, hclCtx *hcl.EvalContext, addr addrs.AbsInputVariableInstance, config *configs.Variable, expr hcl.Expression) (checkResult, tfdiags.Diagnostics) { + const errInvalidCondition = "Invalid variable validation result" + const errInvalidValue = "Invalid value for variable" + var diags tfdiags.Diagnostics + + result, moreDiags := validation.Condition.Value(hclCtx) + diags = diags.Append(moreDiags) + errorValue, errorDiags := validation.ErrorMessage.Value(hclCtx) + + // The following error handling is a workaround to preserve backwards + // compatibility. Due to an implementation quirk, all prior versions of + // Terraform would treat error messages specified using JSON + // configuration syntax (.tf.json) as string literals, even if they + // contained the "${" template expression operator. This behaviour did + // not match that of HCL configuration syntax, where a template + // expression would result in a validation error. + // + // As a result, users writing or generating JSON configuration syntax + // may have specified error messages which are invalid template + // expressions. As we add support for error message expressions, we are + // unable to perfectly distinguish between these two cases. + // + // To ensure that we don't break backwards compatibility, we have the + // below fallback logic if the error message fails to evaluate. This + // should only have any effect for JSON configurations. The gohcl + // DecodeExpression function behaves differently when the source of the + // expression is a JSON configuration file and a nil context is passed. + if errorDiags.HasErrors() { + // Attempt to decode the expression as a string literal. Passing + // nil as the context forces a JSON syntax string value to be + // interpreted as a string literal. + var errorString string + moreErrorDiags := gohcl.DecodeExpression(validation.ErrorMessage, nil, &errorString) + if !moreErrorDiags.HasErrors() { + // Decoding succeeded, meaning that this is a JSON syntax + // string value. We rewrap that as a cty value to allow later + // decoding to succeed. + errorValue = cty.StringVal(errorString) + + // This warning diagnostic explains this odd behaviour, while + // giving us an escape hatch to change this to a hard failure + // in some future Terraform 1.x version. + errorDiags = hcl.Diagnostics{ + &hcl.Diagnostic{ + Severity: hcl.DiagWarning, + Summary: "Validation error message expression is invalid", + Detail: fmt.Sprintf("The error message provided could not be evaluated as an expression, so Terraform is interpreting it as a string literal.\n\nIn future versions of Terraform, this will be considered an error. Please file a GitHub issue if this would break your workflow.\n\n%s", errorDiags.Error()), + Subject: validation.ErrorMessage.Range().Ptr(), + Context: validation.DeclRange.Ptr(), + Expression: validation.ErrorMessage, + EvalContext: hclCtx, + }, + } + } + + // We want to either report the original diagnostics if the + // fallback failed, or the warning generated above if it succeeded. + diags = diags.Append(errorDiags) + } + + if diags.HasErrors() { + log.Printf("[TRACE] evalVariableValidations: %s rule %s check rule evaluation failed: %s", addr, validation.DeclRange, diags.Err().Error()) + } + if !result.IsKnown() { + log.Printf("[TRACE] evalVariableValidations: %s rule %s condition value is unknown, so skipping validation for now", addr, validation.DeclRange) + + return checkResult{Status: checks.StatusUnknown}, diags // We'll wait until we've learned more, then. + } + if result.IsNull() { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: errInvalidCondition, + Detail: "Validation condition expression must return either true or false, not null.", + Subject: validation.Condition.Range().Ptr(), + Expression: validation.Condition, + EvalContext: hclCtx, + }) + return checkResult{Status: checks.StatusError}, diags + } + var err error + result, err = convert.Convert(result, cty.Bool) + if err != nil { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: errInvalidCondition, + Detail: fmt.Sprintf("Invalid validation condition result value: %s.", tfdiags.FormatError(err)), + Subject: validation.Condition.Range().Ptr(), + Expression: validation.Condition, + EvalContext: hclCtx, + }) + return checkResult{Status: checks.StatusError}, diags + } + + // Validation condition may be marked if the input variable is bound to + // a sensitive value. This is irrelevant to the validation process, so + // we discard the marks now. + result, _ = result.Unmark() + status := checks.StatusForCtyValue(result) + + if status != checks.StatusFail { + return checkResult{Status: status}, diags + } + + var errorMessage string + if !errorDiags.HasErrors() && errorValue.IsKnown() && !errorValue.IsNull() { + var err error + errorValue, err = convert.Convert(errorValue, cty.String) + if err != nil { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid error message", + Detail: fmt.Sprintf("Unsuitable value for error message: %s.", tfdiags.FormatError(err)), + Subject: validation.ErrorMessage.Range().Ptr(), + Expression: validation.ErrorMessage, + EvalContext: hclCtx, + }) + } else { + if marks.Has(errorValue, marks.Sensitive) { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + + Summary: "Error message refers to sensitive values", + Detail: `The error expression used to explain this condition refers to sensitive values. Terraform will not display the resulting message. + +You can correct this by removing references to sensitive values, or by carefully using the nonsensitive() function if the expression will not reveal the sensitive data.`, + + Subject: validation.ErrorMessage.Range().Ptr(), + Expression: validation.ErrorMessage, + EvalContext: hclCtx, + }) + errorMessage = "The error message included a sensitive value, so it will not be displayed." + } else { + errorMessage = strings.TrimSpace(errorValue.AsString()) + } + } + } + if errorMessage == "" { + errorMessage = "Failed to evaluate condition error message." + } + + if expr != nil { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: errInvalidValue, + Detail: fmt.Sprintf("%s\n\nThis was checked by the validation rule at %s.", errorMessage, validation.DeclRange.String()), + Subject: expr.Range().Ptr(), + Expression: validation.Condition, + EvalContext: hclCtx, + }) + } else { + // Since we don't have a source expression for a root module + // variable, we'll just report the error from the perspective + // of the variable declaration itself. + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: errInvalidValue, + Detail: fmt.Sprintf("%s\n\nThis was checked by the validation rule at %s.", errorMessage, validation.DeclRange.String()), + Subject: config.DeclRange.Ptr(), + Expression: validation.Condition, + EvalContext: hclCtx, + }) + } + + return checkResult{ + Status: status, + FailureMessage: errorMessage, + }, diags +} diff --git a/internal/terraform/eval_variable_test.go b/internal/terraform/eval_variable_test.go index aca3b0df71..ae473bdd13 100644 --- a/internal/terraform/eval_variable_test.go +++ b/internal/terraform/eval_variable_test.go @@ -12,6 +12,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/lang" "github.com/hashicorp/terraform/internal/lang/marks" "github.com/hashicorp/terraform/internal/tfdiags" @@ -1107,12 +1108,14 @@ func TestEvalVariableValidations_jsonErrorMessageEdgeCase(t *testing.T) { given cty.Value wantErr []string wantWarn []string + status checks.Status }{ // Valid variable validation declaration, assigned value which passes // the condition generates no diagnostics. { varName: "valid", given: cty.StringVal("foo"), + status: checks.StatusPass, }, // Assigning a value which fails the condition generates an error // message with the expression successfully evaluated. @@ -1123,6 +1126,7 @@ func TestEvalVariableValidations_jsonErrorMessageEdgeCase(t *testing.T) { "Invalid value for variable", "Valid template string bar", }, + status: checks.StatusFail, }, // Invalid variable validation declaration due to an unparseable // template string. Assigning a value which passes the condition @@ -1134,6 +1138,7 @@ func TestEvalVariableValidations_jsonErrorMessageEdgeCase(t *testing.T) { "Validation error message expression is invalid", "Missing expression; Expected the start of an expression, but found the end of the file.", }, + status: checks.StatusPass, }, // Assigning a value which fails the condition generates an error // message including the configured string interpreted as a literal @@ -1149,6 +1154,7 @@ func TestEvalVariableValidations_jsonErrorMessageEdgeCase(t *testing.T) { "Validation error message expression is invalid", "Missing expression; Expected the start of an expression, but found the end of the file.", }, + status: checks.StatusFail, }, } @@ -1173,11 +1179,17 @@ func TestEvalVariableValidations_jsonErrorMessageEdgeCase(t *testing.T) { } return test.given } + ctx.ChecksState = checks.NewState(cfg) + ctx.ChecksState.ReportCheckableObjects(varAddr.ConfigCheckable(), addrs.MakeSet[addrs.Checkable](varAddr)) gotDiags := evalVariableValidations( varAddr, varCfg, nil, ctx, ) + if ctx.ChecksState.ObjectCheckStatus(varAddr) != test.status { + t.Errorf("expected check result %s but instead %s", test.status, ctx.ChecksState.ObjectCheckStatus(varAddr)) + } + if len(test.wantErr) == 0 && len(test.wantWarn) == 0 { if len(gotDiags) > 0 { t.Errorf("no diags expected, got %s", gotDiags.Err().Error()) @@ -1258,12 +1270,14 @@ variable "bar" { varName string given cty.Value wantErr []string + status checks.Status }{ // Validations pass on a sensitive variable with an error message which // would generate a sensitive value { varName: "foo", given: cty.StringVal("boop"), + status: checks.StatusPass, }, // Assigning a value which fails the condition generates a sensitive // error message, which is elided and generates another error @@ -1275,12 +1289,14 @@ variable "bar" { "The error message included a sensitive value, so it will not be displayed.", "Error message refers to sensitive values", }, + status: checks.StatusFail, }, // Validations pass on a sensitive variable with a correctly defined // error message { varName: "bar", given: cty.StringVal("boop"), + status: checks.StatusPass, }, // Assigning a value which fails the condition generates a nonsensitive // error message, which is displayed @@ -1291,6 +1307,7 @@ variable "bar" { "Invalid value for variable", "Bar must be 4 characters, not 3.", }, + status: checks.StatusFail, }, } @@ -1319,11 +1336,17 @@ variable "bar" { return test.given } } + ctx.ChecksState = checks.NewState(cfg) + ctx.ChecksState.ReportCheckableObjects(varAddr.ConfigCheckable(), addrs.MakeSet[addrs.Checkable](varAddr)) gotDiags := evalVariableValidations( varAddr, varCfg, nil, ctx, ) + if ctx.ChecksState.ObjectCheckStatus(varAddr) != test.status { + t.Errorf("expected check result %s but instead %s", test.status, ctx.ChecksState.ObjectCheckStatus(varAddr)) + } + if len(test.wantErr) == 0 { if len(gotDiags) > 0 { t.Errorf("no diags expected, got %s", gotDiags.Err().Error()) diff --git a/internal/terraform/graph_builder_eval.go b/internal/terraform/graph_builder_eval.go index 303538c63c..a0cb4527b1 100644 --- a/internal/terraform/graph_builder_eval.go +++ b/internal/terraform/graph_builder_eval.go @@ -67,8 +67,8 @@ func (b *EvalGraphBuilder) Steps() []GraphTransformer { }, // Add dynamic values - &RootVariableTransformer{Config: b.Config, RawValues: b.RootVariableValues}, - &ModuleVariableTransformer{Config: b.Config}, + &RootVariableTransformer{Config: b.Config, RawValues: b.RootVariableValues, Planning: true}, + &ModuleVariableTransformer{Config: b.Config, Planning: true}, &LocalTransformer{Config: b.Config}, &OutputTransformer{ Config: b.Config, diff --git a/internal/terraform/graph_builder_plan.go b/internal/terraform/graph_builder_plan.go index 4cea55e1fe..768e8b9bce 100644 --- a/internal/terraform/graph_builder_plan.go +++ b/internal/terraform/graph_builder_plan.go @@ -128,8 +128,8 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer { }, // Add dynamic values - &RootVariableTransformer{Config: b.Config, RawValues: b.RootVariableValues}, - &ModuleVariableTransformer{Config: b.Config}, + &RootVariableTransformer{Config: b.Config, RawValues: b.RootVariableValues, Planning: true}, + &ModuleVariableTransformer{Config: b.Config, Planning: true}, &LocalTransformer{Config: b.Config}, &OutputTransformer{ Config: b.Config, diff --git a/internal/terraform/node_module_variable.go b/internal/terraform/node_module_variable.go index e1b5407452..105d786a54 100644 --- a/internal/terraform/node_module_variable.go +++ b/internal/terraform/node_module_variable.go @@ -25,6 +25,10 @@ type nodeExpandModuleVariable struct { Module addrs.Module Config *configs.Variable Expr hcl.Expression + + // Planning must be set to true when building a planning graph, and must be + // false when building an apply graph. + Planning bool } var ( @@ -44,10 +48,27 @@ func (n *nodeExpandModuleVariable) temporaryValue() bool { func (n *nodeExpandModuleVariable) DynamicExpand(ctx EvalContext) (*Graph, error) { var g Graph + + // If this variable has preconditions, we need to report these checks now. + // + // We should only do this during planning as the apply phase starts with + // all the same checkable objects that were registered during the plan. + var checkableAddrs addrs.Set[addrs.Checkable] + if n.Planning { + if checkState := ctx.Checks(); checkState.ConfigHasChecks(n.Addr.InModule(n.Module)) { + checkableAddrs = addrs.MakeSet[addrs.Checkable]() + } + } + expander := ctx.InstanceExpander() for _, module := range expander.ExpandModule(n.Module) { + addr := n.Addr.Absolute(module) + if checkableAddrs != nil { + checkableAddrs.Add(addr) + } + o := &nodeModuleVariable{ - Addr: n.Addr.Absolute(module), + Addr: addr, Config: n.Config, Expr: n.Expr, ModuleInstance: module, @@ -55,6 +76,11 @@ func (n *nodeExpandModuleVariable) DynamicExpand(ctx EvalContext) (*Graph, error g.Add(o) } addRootNodeToGraph(&g) + + if checkableAddrs != nil { + ctx.Checks().ReportCheckableObjects(n.Addr.InModule(n.Module), checkableAddrs) + } + return &g, nil } diff --git a/internal/terraform/node_root_variable.go b/internal/terraform/node_root_variable.go index c18d052ee2..08cb44d331 100644 --- a/internal/terraform/node_root_variable.go +++ b/internal/terraform/node_root_variable.go @@ -6,11 +6,12 @@ package terraform import ( "log" + "github.com/zclconf/go-cty/cty" + "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/dag" "github.com/hashicorp/terraform/internal/tfdiags" - "github.com/zclconf/go-cty/cty" ) // NodeRootVariable represents a root variable input. @@ -24,6 +25,10 @@ type NodeRootVariable struct { // converted or validated, and can be nil for a variable that isn't // set at all. RawValue *InputValue + + // Planning must be set to true when building a planning graph, and must be + // false when building an apply graph. + Planning bool } var ( @@ -82,6 +87,14 @@ func (n *NodeRootVariable) Execute(ctx EvalContext, op walkOperation) tfdiags.Di } } + if n.Planning { + if checkState := ctx.Checks(); checkState.ConfigHasChecks(n.Addr.InModule(addrs.RootModule)) { + ctx.Checks().ReportCheckableObjects( + n.Addr.InModule(addrs.RootModule), + addrs.MakeSet[addrs.Checkable](n.Addr.Absolute(addrs.RootModuleInstance))) + } + } + finalVal, moreDiags := prepareFinalInputVariableValue( addr, givenVal, diff --git a/internal/terraform/node_root_variable_test.go b/internal/terraform/node_root_variable_test.go index 7e8eb6a3d3..2909c282d8 100644 --- a/internal/terraform/node_root_variable_test.go +++ b/internal/terraform/node_root_variable_test.go @@ -11,6 +11,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/lang" ) @@ -115,8 +116,17 @@ func TestNodeRootVariableExecute(t *testing.T) { Value: cty.StringVal("5"), SourceType: ValueFromUnknown, }, + Planning: true, } + ctx.ChecksState = checks.NewState(&configs.Config{ + Module: &configs.Module{ + Variables: map[string]*configs.Variable{ + "foo": n.Config, + }, + }, + }) + diags := n.Execute(ctx, walkApply) if diags.HasErrors() { t.Fatalf("unexpected error: %s", diags.Err()) @@ -134,6 +144,9 @@ func TestNodeRootVariableExecute(t *testing.T) { // as part of preparing the "final value". t.Errorf("wrong value for ctx.SetRootModuleArgument\ngot: %#v\nwant: %#v", got, want) } + if status := ctx.Checks().ObjectCheckStatus(n.Addr.Absolute(addrs.RootModuleInstance)); status != checks.StatusPass { + t.Errorf("expected checks to pass but go %s instead", status) + } }) } diff --git a/internal/terraform/transform_module_variable.go b/internal/terraform/transform_module_variable.go index a4df222a89..3cae76bda9 100644 --- a/internal/terraform/transform_module_variable.go +++ b/internal/terraform/transform_module_variable.go @@ -6,11 +6,13 @@ package terraform import ( "fmt" - "github.com/hashicorp/terraform/internal/addrs" - "github.com/hashicorp/terraform/internal/tfdiags" "github.com/zclconf/go-cty/cty" + "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/tfdiags" + "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/terraform/internal/configs" ) @@ -26,6 +28,10 @@ import ( // steps for validating module blocks, separate from this transform. type ModuleVariableTransformer struct { Config *configs.Config + + // Planning must be set to true when building a planning graph, and must be + // false when building an apply graph. + Planning bool } func (t *ModuleVariableTransformer) Transform(g *Graph) error { @@ -104,9 +110,10 @@ func (t *ModuleVariableTransformer) transformSingle(g *Graph, parent, c *configs Addr: addrs.InputVariable{ Name: v.Name, }, - Module: c.Path, - Config: v, - Expr: expr, + Module: c.Path, + Config: v, + Expr: expr, + Planning: t.Planning, } g.Add(node) } diff --git a/internal/terraform/transform_variable.go b/internal/terraform/transform_variable.go index 7df3257e2e..da75fc3a8a 100644 --- a/internal/terraform/transform_variable.go +++ b/internal/terraform/transform_variable.go @@ -18,6 +18,10 @@ type RootVariableTransformer struct { Config *configs.Config RawValues InputValues + + // Planning must be set to true when building a planning graph, and must be + // false when building an apply graph. + Planning bool } func (t *RootVariableTransformer) Transform(g *Graph) error { @@ -38,6 +42,7 @@ func (t *RootVariableTransformer) Transform(g *Graph) error { }, Config: v, RawValue: t.RawValues[v.Name], + Planning: t.Planning, } g.Add(node) }