From 361474dc75afce77a4d385ce6495cdd006abb1db Mon Sep 17 00:00:00 2001 From: Samsondeen Dare Date: Fri, 13 Dec 2024 15:35:50 +0100 Subject: [PATCH 01/18] allow mocked values override during planning --- internal/configs/mock_provider.go | 35 +++++++++++++++++++ internal/moduletest/mocking/values.go | 35 +++++++++++++------ internal/providers/mock.go | 4 ++- .../internal/stackeval/stubs/unknown.go | 4 +-- .../node_resource_abstract_instance.go | 15 ++++++-- 5 files changed, 78 insertions(+), 15 deletions(-) diff --git a/internal/configs/mock_provider.go b/internal/configs/mock_provider.go index f367e0051635..d0dab193592c 100644 --- a/internal/configs/mock_provider.go +++ b/internal/configs/mock_provider.go @@ -7,6 +7,7 @@ import ( "github.com/hashicorp/hcl/v2/gohcl" "github.com/hashicorp/hcl/v2/hclsyntax" "github.com/zclconf/go-cty/cty" + "github.com/zclconf/go-cty/cty/gocty" "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/tfdiags" @@ -171,6 +172,7 @@ const ( type Override struct { Target *addrs.Target Values cty.Value + Ignore bool // Source tells us where this Override was defined. Source OverrideSource @@ -393,12 +395,18 @@ func decodeOverrideDataBlock(block *hcl.Block, source OverrideSource) (*Override return override, diags } +var ( + // triggerWhenPlan is the attribute name for specifying whether to trigger overrides when planning. + triggerWhenPlan = "trigger_when_plan" +) + func decodeOverrideBlock(block *hcl.Block, attributeName string, blockName string, source OverrideSource) (*Override, hcl.Diagnostics) { var diags hcl.Diagnostics content, contentDiags := block.Body.Content(&hcl.BodySchema{ Attributes: []hcl.AttributeSchema{ {Name: "target"}, + {Name: triggerWhenPlan}, {Name: attributeName}, }, }) @@ -440,6 +448,33 @@ func decodeOverrideBlock(block *hcl.Block, attributeName string, blockName strin override.Values = cty.EmptyObjectVal } + if attribute, exists := content.Attributes[triggerWhenPlan]; exists { + var valueDiags hcl.Diagnostics + val, valueDiags := attribute.Expr.Value(nil) + diags = append(diags, valueDiags...) + if val.Type().Equals(cty.Bool) { + var triggerOverride bool + err := gocty.FromCtyValue(val, &triggerOverride) + if err != nil { + // should not happen as we already checked the type + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid trigger_when_plan attribute", + Detail: fmt.Sprintf("The %s attribute must be a boolean.", triggerWhenPlan), + Subject: attribute.Range.Ptr(), + }) + } + override.Ignore = !triggerOverride + } else { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid trigger_when_plan attribute", + Detail: fmt.Sprintf("The %s attribute must be a boolean.", triggerWhenPlan), + Subject: attribute.Range.Ptr(), + }) + } + } + if !override.Values.Type().IsObjectType() { var attributePreposition string diff --git a/internal/moduletest/mocking/values.go b/internal/moduletest/mocking/values.go index 76e216fe3e91..54cffa95330c 100644 --- a/internal/moduletest/mocking/values.go +++ b/internal/moduletest/mocking/values.go @@ -13,13 +13,18 @@ import ( "github.com/hashicorp/terraform/internal/tfdiags" ) -// PlanComputedValuesForResource accepts a target value, and populates it with -// cty.UnknownValues wherever a value should be computed during the apply stage. +// PlanComputedValuesForResource accepts a target value, and populates computed +// values with values from the provider 'with' argument, and if not provided, +// sets them to cty.UnknownVal. // -// This method basically simulates the behaviour of a plan request in a real +// The latter behaviour simulates the behaviour of a plan request in a real // provider. -func PlanComputedValuesForResource(original cty.Value, schema *configschema.Block) (cty.Value, tfdiags.Diagnostics) { - return populateComputedValues(original, MockedData{}, schema, isNull, makeUnknown) +func PlanComputedValuesForResource(original cty.Value, with *MockedData, schema *configschema.Block) (cty.Value, tfdiags.Diagnostics) { + mocked := MockedData{} + if with != nil { + mocked = *with + } + return populateComputedValues(original, mocked, schema, isNull) } // ApplyComputedValuesForResource accepts a target value, and populates it @@ -30,7 +35,7 @@ func PlanComputedValuesForResource(original cty.Value, schema *configschema.Bloc // This method basically simulates the behaviour of an apply request in a real // provider. func ApplyComputedValuesForResource(original cty.Value, with MockedData, schema *configschema.Block) (cty.Value, tfdiags.Diagnostics) { - return populateComputedValues(original, with, schema, isUnknown, with.makeKnown) + return populateComputedValues(original, with, schema, isUnknown) } // ComputedValuesForDataSource accepts a target value, and populates it either @@ -44,16 +49,25 @@ func ApplyComputedValuesForResource(original cty.Value, with MockedData, schema // This method basically simulates the behaviour of a get data source request // in a real provider. func ComputedValuesForDataSource(original cty.Value, with MockedData, schema *configschema.Block) (cty.Value, tfdiags.Diagnostics) { - return populateComputedValues(original, with, schema, isNull, with.makeKnown) + return populateComputedValues(original, with, schema, isNull) } type processValue func(value cty.Value) bool type generateValue func(attribute *configschema.Attribute, with cty.Value, path cty.Path) (cty.Value, tfdiags.Diagnostics) -func populateComputedValues(target cty.Value, with MockedData, schema *configschema.Block, processValue processValue, generateValue generateValue) (cty.Value, tfdiags.Diagnostics) { +func populateComputedValues(target cty.Value, with MockedData, schema *configschema.Block, processValue processValue) (cty.Value, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics + var generateValue generateValue + // If we're ignoring the values, then we should generate + // unknown values for the computed attributes. + if with.Ignore { + generateValue = makeUnknown + } else { + generateValue = with.makeKnown + } + if !with.validate() { // This is actually a user error, it means the user wrote something like // `values = "not an object"` when defining the replacement values for @@ -159,8 +173,9 @@ func makeUnknown(target *configschema.Attribute, _ cty.Value, _ cty.Path) (cty.V // MockedData wraps the value and the source location of the value into a single // struct for easy access. type MockedData struct { - Value cty.Value - Range hcl.Range + Value cty.Value + Ignore bool + Range hcl.Range } // makeKnown produces a valid value for the given attribute. The input value diff --git a/internal/providers/mock.go b/internal/providers/mock.go index 55af9ce808d1..6f5f888200b8 100644 --- a/internal/providers/mock.go +++ b/internal/providers/mock.go @@ -182,7 +182,9 @@ func (m *Mock) PlanResourceChange(request PlanResourceChangeRequest) PlanResourc panic(fmt.Errorf("failed to retrieve schema for resource %s", request.TypeName)) } - value, diags := mocking.PlanComputedValuesForResource(request.ProposedNewState, resource.Block) + // If the provider was overriden in the test (via override_*), the mock provider is not called at all, + // so we can be certain that this provider is not mocked. + value, diags := mocking.PlanComputedValuesForResource(request.ProposedNewState, nil, resource.Block) response.Diagnostics = response.Diagnostics.Append(diags) response.PlannedState = value response.PlannedPrivate = []byte("create") diff --git a/internal/stacks/stackruntime/internal/stackeval/stubs/unknown.go b/internal/stacks/stackruntime/internal/stackeval/stubs/unknown.go index a84942b5d54a..7d4d8167d4a1 100644 --- a/internal/stacks/stackruntime/internal/stackeval/stubs/unknown.go +++ b/internal/stacks/stackruntime/internal/stackeval/stubs/unknown.go @@ -115,7 +115,7 @@ func (u *unknownProvider) PlanResourceChange(request providers.PlanResourceChang // library, but it is doing exactly what we need it to do. schema := u.GetProviderSchema().ResourceTypes[request.TypeName] - val, diags := mocking.PlanComputedValuesForResource(request.ProposedNewState, schema.Block) + val, diags := mocking.PlanComputedValuesForResource(request.ProposedNewState, nil, schema.Block) if diags.HasErrors() { // All the potential errors we get back from this function are // related to the user badly defining mocks. We should never hit @@ -213,7 +213,7 @@ func (u *unknownProvider) ReadDataSource(request providers.ReadDataSourceRequest // library, but it is doing exactly what we need it to do. schema := u.GetProviderSchema().DataSources[request.TypeName] - val, diags := mocking.PlanComputedValuesForResource(request.Config, schema.Block) + val, diags := mocking.PlanComputedValuesForResource(request.Config, nil, schema.Block) if diags.HasErrors() { // All the potential errors we get back from this function are // related to the user badly defining mocks. We should never hit diff --git a/internal/terraform/node_resource_abstract_instance.go b/internal/terraform/node_resource_abstract_instance.go index 6d721cc85311..99ac12077bf9 100644 --- a/internal/terraform/node_resource_abstract_instance.go +++ b/internal/terraform/node_resource_abstract_instance.go @@ -904,7 +904,7 @@ func (n *NodeAbstractResourceInstance) plan( if priorVal.IsNull() { // Then we are actually creating something, so let's populate the // computed values from our override value. - override, overrideDiags := mocking.PlanComputedValuesForResource(proposedNewVal, schema) + override, overrideDiags := mocking.PlanComputedValuesForResource(proposedNewVal, getPlanMockedData(n.override), schema) resp = providers.PlanResourceChangeResponse{ PlannedState: override, Diagnostics: overrideDiags, @@ -1082,7 +1082,7 @@ func (n *NodeAbstractResourceInstance) plan( if n.override != nil { // In this case, we are always creating the resource so we don't // do any validation, and just call out to the mocking library. - override, overrideDiags := mocking.PlanComputedValuesForResource(proposedNewVal, schema) + override, overrideDiags := mocking.PlanComputedValuesForResource(proposedNewVal, getPlanMockedData(n.override), schema) resp = providers.PlanResourceChangeResponse{ PlannedState: override, Diagnostics: overrideDiags, @@ -1231,6 +1231,17 @@ func (n *NodeAbstractResourceInstance) plan( return plan, state, deferred, keyData, diags } +func getPlanMockedData(override *configs.Override) *mocking.MockedData { + if override == nil { + return nil + } + return &mocking.MockedData{ + Value: override.Values, + Range: override.Range, + Ignore: override.Ignore, + } +} + func (n *NodeAbstractResource) processIgnoreChanges(prior, config cty.Value, schema *configschema.Block) (cty.Value, tfdiags.Diagnostics) { // ignore_changes only applies when an object already exists, since we // can't ignore changes to a thing we've not created yet. From 3d9a1829448050cf7ace40a95e94835c518b5f6a Mon Sep 17 00:00:00 2001 From: Samsondeen Dare Date: Mon, 16 Dec 2024 12:08:23 +0100 Subject: [PATCH 02/18] change param to pointer --- internal/configs/mock_provider.go | 4 +- internal/moduletest/mocking/values.go | 45 ++++++++++++------- internal/moduletest/mocking/values_test.go | 2 +- internal/providers/mock.go | 4 +- .../node_resource_abstract_instance.go | 22 ++++----- .../terraform/node_resource_plan_instance.go | 5 +-- 6 files changed, 42 insertions(+), 40 deletions(-) diff --git a/internal/configs/mock_provider.go b/internal/configs/mock_provider.go index d0dab193592c..5bb570e3fe80 100644 --- a/internal/configs/mock_provider.go +++ b/internal/configs/mock_provider.go @@ -459,7 +459,7 @@ func decodeOverrideBlock(block *hcl.Block, attributeName string, blockName strin // should not happen as we already checked the type diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, - Summary: "Invalid trigger_when_plan attribute", + Summary: fmt.Sprintf("Invalid %s value", triggerWhenPlan), Detail: fmt.Sprintf("The %s attribute must be a boolean.", triggerWhenPlan), Subject: attribute.Range.Ptr(), }) @@ -468,7 +468,7 @@ func decodeOverrideBlock(block *hcl.Block, attributeName string, blockName strin } else { diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, - Summary: "Invalid trigger_when_plan attribute", + Summary: fmt.Sprintf("Invalid %s value", triggerWhenPlan), Detail: fmt.Sprintf("The %s attribute must be a boolean.", triggerWhenPlan), Subject: attribute.Range.Ptr(), }) diff --git a/internal/moduletest/mocking/values.go b/internal/moduletest/mocking/values.go index 54cffa95330c..95c15d324b69 100644 --- a/internal/moduletest/mocking/values.go +++ b/internal/moduletest/mocking/values.go @@ -13,18 +13,14 @@ import ( "github.com/hashicorp/terraform/internal/tfdiags" ) -// PlanComputedValuesForResource accepts a target value, and populates computed -// values with values from the provider 'with' argument, and if not provided, -// sets them to cty.UnknownVal. +// PlanComputedValuesForResource accepts a target value, and populates its computed +// values with values from the provider 'with' argument, and if 'with' is not provided, +// it sets the computed values to cty.UnknownVal. // // The latter behaviour simulates the behaviour of a plan request in a real // provider. func PlanComputedValuesForResource(original cty.Value, with *MockedData, schema *configschema.Block) (cty.Value, tfdiags.Diagnostics) { - mocked := MockedData{} - if with != nil { - mocked = *with - } - return populateComputedValues(original, mocked, schema, isNull) + return populateComputedValues(original, with, schema, isNull) } // ApplyComputedValuesForResource accepts a target value, and populates it @@ -34,7 +30,7 @@ func PlanComputedValuesForResource(original cty.Value, with *MockedData, schema // // This method basically simulates the behaviour of an apply request in a real // provider. -func ApplyComputedValuesForResource(original cty.Value, with MockedData, schema *configschema.Block) (cty.Value, tfdiags.Diagnostics) { +func ApplyComputedValuesForResource(original cty.Value, with *MockedData, schema *configschema.Block) (cty.Value, tfdiags.Diagnostics) { return populateComputedValues(original, with, schema, isUnknown) } @@ -48,7 +44,7 @@ func ApplyComputedValuesForResource(original cty.Value, with MockedData, schema // // This method basically simulates the behaviour of a get data source request // in a real provider. -func ComputedValuesForDataSource(original cty.Value, with MockedData, schema *configschema.Block) (cty.Value, tfdiags.Diagnostics) { +func ComputedValuesForDataSource(original cty.Value, with *MockedData, schema *configschema.Block) (cty.Value, tfdiags.Diagnostics) { return populateComputedValues(original, with, schema, isNull) } @@ -56,13 +52,19 @@ type processValue func(value cty.Value) bool type generateValue func(attribute *configschema.Attribute, with cty.Value, path cty.Path) (cty.Value, tfdiags.Diagnostics) -func populateComputedValues(target cty.Value, with MockedData, schema *configschema.Block, processValue processValue) (cty.Value, tfdiags.Diagnostics) { +func populateComputedValues(target cty.Value, mocked *MockedData, schema *configschema.Block, processValue processValue) (cty.Value, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics + with := MockedData{} + if mocked != nil { + with = *mocked + } + var generateValue generateValue - // If we're ignoring the values, then we should generate - // unknown values for the computed attributes. - if with.Ignore { + // If the mocked data should be ignored, then we will generate + // unknown values for the computed attributes, otherwise we will + // generate values based on the mocked data. + if with.IgnoreComputed { generateValue = makeUnknown } else { generateValue = with.makeKnown @@ -173,9 +175,18 @@ func makeUnknown(target *configschema.Attribute, _ cty.Value, _ cty.Path) (cty.V // MockedData wraps the value and the source location of the value into a single // struct for easy access. type MockedData struct { - Value cty.Value - Ignore bool - Range hcl.Range + Value cty.Value + Range hcl.Range + IgnoreComputed bool // If true, computed values will be ignored and replaced with unknown values. +} + +// NewMockedData creates a new MockedData struct with the given value and range. +func NewMockedData(value cty.Value, ignoreComputed bool, range_ hcl.Range) MockedData { + return MockedData{ + Value: value, + IgnoreComputed: ignoreComputed, + Range: range_, + } } // makeKnown produces a valid value for the given attribute. The input value diff --git a/internal/moduletest/mocking/values_test.go b/internal/moduletest/mocking/values_test.go index 29b5993c7c82..76b6bb232c6b 100644 --- a/internal/moduletest/mocking/values_test.go +++ b/internal/moduletest/mocking/values_test.go @@ -983,7 +983,7 @@ func TestComputedValuesForDataSource(t *testing.T) { testRand = nil }() - actual, diags := ComputedValuesForDataSource(tc.target, MockedData{ + actual, diags := ComputedValuesForDataSource(tc.target, &MockedData{ Value: tc.with, }, tc.schema) diff --git a/internal/providers/mock.go b/internal/providers/mock.go index 6f5f888200b8..2590005a72f4 100644 --- a/internal/providers/mock.go +++ b/internal/providers/mock.go @@ -222,7 +222,7 @@ func (m *Mock) ApplyResourceChange(request ApplyResourceChangeRequest) ApplyReso panic(fmt.Errorf("failed to retrieve schema for resource %s", request.TypeName)) } - replacement := mocking.MockedData{ + replacement := &mocking.MockedData{ Value: cty.NilVal, // If we have no data then we use cty.NilVal. } if mockedResource, exists := m.Data.MockResources[request.TypeName]; exists { @@ -277,7 +277,7 @@ func (m *Mock) ReadDataSource(request ReadDataSourceRequest) ReadDataSourceRespo panic(fmt.Errorf("failed to retrieve schema for data source %s", request.TypeName)) } - mockedData := mocking.MockedData{ + mockedData := &mocking.MockedData{ Value: cty.NilVal, // If we have no mocked data we use cty.NilVal. } if mockedDataSource, exists := m.Data.MockDataSources[request.TypeName]; exists { diff --git a/internal/terraform/node_resource_abstract_instance.go b/internal/terraform/node_resource_abstract_instance.go index 99ac12077bf9..bbb64e0e583e 100644 --- a/internal/terraform/node_resource_abstract_instance.go +++ b/internal/terraform/node_resource_abstract_instance.go @@ -904,7 +904,7 @@ func (n *NodeAbstractResourceInstance) plan( if priorVal.IsNull() { // Then we are actually creating something, so let's populate the // computed values from our override value. - override, overrideDiags := mocking.PlanComputedValuesForResource(proposedNewVal, getPlanMockedData(n.override), schema) + override, overrideDiags := mocking.PlanComputedValuesForResource(proposedNewVal, getMockedData(n.override), schema) resp = providers.PlanResourceChangeResponse{ PlannedState: override, Diagnostics: overrideDiags, @@ -1082,7 +1082,7 @@ func (n *NodeAbstractResourceInstance) plan( if n.override != nil { // In this case, we are always creating the resource so we don't // do any validation, and just call out to the mocking library. - override, overrideDiags := mocking.PlanComputedValuesForResource(proposedNewVal, getPlanMockedData(n.override), schema) + override, overrideDiags := mocking.PlanComputedValuesForResource(proposedNewVal, getMockedData(n.override), schema) resp = providers.PlanResourceChangeResponse{ PlannedState: override, Diagnostics: overrideDiags, @@ -1231,14 +1231,14 @@ func (n *NodeAbstractResourceInstance) plan( return plan, state, deferred, keyData, diags } -func getPlanMockedData(override *configs.Override) *mocking.MockedData { +func getMockedData(override *configs.Override) *mocking.MockedData { if override == nil { return nil } return &mocking.MockedData{ - Value: override.Values, - Range: override.Range, - Ignore: override.Ignore, + Value: override.Values, + Range: override.Range, + IgnoreComputed: override.Ignore, } } @@ -1554,10 +1554,7 @@ func (n *NodeAbstractResourceInstance) readDataSource(ctx EvalContext, configVal var resp providers.ReadDataSourceResponse if n.override != nil { - override, overrideDiags := mocking.ComputedValuesForDataSource(configVal, mocking.MockedData{ - Value: n.override.Values, - Range: n.override.ValuesRange, - }, schema) + override, overrideDiags := mocking.ComputedValuesForDataSource(configVal, getMockedData(n.override), schema) resp = providers.ReadDataSourceResponse{ State: override, Diagnostics: overrideDiags, @@ -2509,10 +2506,7 @@ func (n *NodeAbstractResourceInstance) apply( // values the first time the object is created. Otherwise, we're happy // to just apply whatever the user asked for. if change.Action == plans.Create { - override, overrideDiags := mocking.ApplyComputedValuesForResource(unmarkedAfter, mocking.MockedData{ - Value: n.override.Values, - Range: n.override.ValuesRange, - }, schema) + override, overrideDiags := mocking.ApplyComputedValuesForResource(unmarkedAfter, getMockedData(n.override), schema) resp = providers.ApplyResourceChangeResponse{ NewState: override, Diagnostics: overrideDiags, diff --git a/internal/terraform/node_resource_plan_instance.go b/internal/terraform/node_resource_plan_instance.go index a7513cca646e..52ae58a847cf 100644 --- a/internal/terraform/node_resource_plan_instance.go +++ b/internal/terraform/node_resource_plan_instance.go @@ -634,10 +634,7 @@ func (n *NodePlannableResourceInstance) importState(ctx EvalContext, addr addrs. // Let's pretend we're reading the value as a data source so we // pre-compute values now as if the resource has already been created. - override, overrideDiags := mocking.ComputedValuesForDataSource(configVal, mocking.MockedData{ - Value: n.override.Values, - Range: n.override.ValuesRange, - }, schema) + override, overrideDiags := mocking.ComputedValuesForDataSource(configVal, getMockedData(n.override), schema) resp = providers.ImportResourceStateResponse{ ImportedResources: []providers.ImportedResource{ { From 1ee9e7e490f88c00cbb42e805b8e800c60711031 Mon Sep 17 00:00:00 2001 From: Samsondeen Dare Date: Mon, 16 Dec 2024 22:06:44 +0100 Subject: [PATCH 03/18] fix tests --- .../primary_mocked_overridden.tftest.hcl | 6 +++ internal/moduletest/mocking/values.go | 47 +++++++++++++------ internal/providers/mock.go | 4 +- .../node_resource_abstract_instance.go | 18 +++---- .../terraform/node_resource_plan_instance.go | 2 +- 5 files changed, 53 insertions(+), 24 deletions(-) diff --git a/internal/command/testdata/test/mocking/tests/primary_mocked_overridden.tftest.hcl b/internal/command/testdata/test/mocking/tests/primary_mocked_overridden.tftest.hcl index 16f92305cf54..6d4eeff4c0ff 100644 --- a/internal/command/testdata/test/mocking/tests/primary_mocked_overridden.tftest.hcl +++ b/internal/command/testdata/test/mocking/tests/primary_mocked_overridden.tftest.hcl @@ -49,4 +49,10 @@ run "test" { error_message = "did not apply mocks" } + assert { + // Override should not affect the other instances + condition = !contains(["aaaa", "cccc"], test_resource.secondary[0].id) + error_message = "override from another instance affected this instance" + } + } diff --git a/internal/moduletest/mocking/values.go b/internal/moduletest/mocking/values.go index 95c15d324b69..4ec5c7acdf25 100644 --- a/internal/moduletest/mocking/values.go +++ b/internal/moduletest/mocking/values.go @@ -20,7 +20,14 @@ import ( // The latter behaviour simulates the behaviour of a plan request in a real // provider. func PlanComputedValuesForResource(original cty.Value, with *MockedData, schema *configschema.Block) (cty.Value, tfdiags.Diagnostics) { - return populateComputedValues(original, with, schema, isNull) + mocked := with + if with == nil { + mocked = &MockedData{ + Value: cty.NilVal, + ComputedAsUnknown: true, + } + } + return populateComputedValues(original, mocked, schema, isNull) } // ApplyComputedValuesForResource accepts a target value, and populates it @@ -31,7 +38,13 @@ func PlanComputedValuesForResource(original cty.Value, with *MockedData, schema // This method basically simulates the behaviour of an apply request in a real // provider. func ApplyComputedValuesForResource(original cty.Value, with *MockedData, schema *configschema.Block) (cty.Value, tfdiags.Diagnostics) { - return populateComputedValues(original, with, schema, isUnknown) + mocked := with + if with == nil { + mocked = &MockedData{ + Value: cty.NilVal, + } + } + return populateComputedValues(original, mocked, schema, isUnknown) } // ComputedValuesForDataSource accepts a target value, and populates it either @@ -45,7 +58,13 @@ func ApplyComputedValuesForResource(original cty.Value, with *MockedData, schema // This method basically simulates the behaviour of a get data source request // in a real provider. func ComputedValuesForDataSource(original cty.Value, with *MockedData, schema *configschema.Block) (cty.Value, tfdiags.Diagnostics) { - return populateComputedValues(original, with, schema, isNull) + mocked := with + if with == nil { + mocked = &MockedData{ + Value: cty.NilVal, + } + } + return populateComputedValues(original, mocked, schema, isNull) } type processValue func(value cty.Value) bool @@ -61,10 +80,10 @@ func populateComputedValues(target cty.Value, mocked *MockedData, schema *config } var generateValue generateValue - // If the mocked data should be ignored, then we will generate - // unknown values for the computed attributes, otherwise we will - // generate values based on the mocked data. - if with.IgnoreComputed { + // If the computed attributes should be ignored, then we will generate + // unknown values for them, otherwise we will + // generate their values based on the mocked data. + if with.ComputedAsUnknown { generateValue = makeUnknown } else { generateValue = with.makeKnown @@ -175,17 +194,17 @@ func makeUnknown(target *configschema.Attribute, _ cty.Value, _ cty.Path) (cty.V // MockedData wraps the value and the source location of the value into a single // struct for easy access. type MockedData struct { - Value cty.Value - Range hcl.Range - IgnoreComputed bool // If true, computed values will be ignored and replaced with unknown values. + Value cty.Value + Range hcl.Range + ComputedAsUnknown bool // If true, computed values are replaced with unknown. } // NewMockedData creates a new MockedData struct with the given value and range. -func NewMockedData(value cty.Value, ignoreComputed bool, range_ hcl.Range) MockedData { +func NewMockedData(value cty.Value, computedAsUnknown bool, range_ hcl.Range) MockedData { return MockedData{ - Value: value, - IgnoreComputed: ignoreComputed, - Range: range_, + Value: value, + ComputedAsUnknown: computedAsUnknown, + Range: range_, } } diff --git a/internal/providers/mock.go b/internal/providers/mock.go index 2590005a72f4..9af422e96d20 100644 --- a/internal/providers/mock.go +++ b/internal/providers/mock.go @@ -184,7 +184,9 @@ func (m *Mock) PlanResourceChange(request PlanResourceChangeRequest) PlanResourc // If the provider was overriden in the test (via override_*), the mock provider is not called at all, // so we can be certain that this provider is not mocked. - value, diags := mocking.PlanComputedValuesForResource(request.ProposedNewState, nil, resource.Block) + value, diags := mocking.PlanComputedValuesForResource(request.ProposedNewState, &mocking.MockedData{ + ComputedAsUnknown: true, + }, resource.Block) response.Diagnostics = response.Diagnostics.Append(diags) response.PlannedState = value response.PlannedPrivate = []byte("create") diff --git a/internal/terraform/node_resource_abstract_instance.go b/internal/terraform/node_resource_abstract_instance.go index bbb64e0e583e..ad11e9050170 100644 --- a/internal/terraform/node_resource_abstract_instance.go +++ b/internal/terraform/node_resource_abstract_instance.go @@ -904,7 +904,7 @@ func (n *NodeAbstractResourceInstance) plan( if priorVal.IsNull() { // Then we are actually creating something, so let's populate the // computed values from our override value. - override, overrideDiags := mocking.PlanComputedValuesForResource(proposedNewVal, getMockedData(n.override), schema) + override, overrideDiags := mocking.PlanComputedValuesForResource(proposedNewVal, getMockedData(n.override, true), schema) resp = providers.PlanResourceChangeResponse{ PlannedState: override, Diagnostics: overrideDiags, @@ -1082,7 +1082,7 @@ func (n *NodeAbstractResourceInstance) plan( if n.override != nil { // In this case, we are always creating the resource so we don't // do any validation, and just call out to the mocking library. - override, overrideDiags := mocking.PlanComputedValuesForResource(proposedNewVal, getMockedData(n.override), schema) + override, overrideDiags := mocking.PlanComputedValuesForResource(proposedNewVal, getMockedData(n.override, true), schema) resp = providers.PlanResourceChangeResponse{ PlannedState: override, Diagnostics: overrideDiags, @@ -1231,14 +1231,16 @@ func (n *NodeAbstractResourceInstance) plan( return plan, state, deferred, keyData, diags } -func getMockedData(override *configs.Override) *mocking.MockedData { +func getMockedData(override *configs.Override, isPlan bool) *mocking.MockedData { if override == nil { return nil } return &mocking.MockedData{ - Value: override.Values, - Range: override.Range, - IgnoreComputed: override.Ignore, + Value: override.Values, + Range: override.Range, + // Apply never ignores computed values. This attribute only matters + // when we are planning. + ComputedAsUnknown: override.Ignore && isPlan, } } @@ -1554,7 +1556,7 @@ func (n *NodeAbstractResourceInstance) readDataSource(ctx EvalContext, configVal var resp providers.ReadDataSourceResponse if n.override != nil { - override, overrideDiags := mocking.ComputedValuesForDataSource(configVal, getMockedData(n.override), schema) + override, overrideDiags := mocking.ComputedValuesForDataSource(configVal, getMockedData(n.override, false), schema) resp = providers.ReadDataSourceResponse{ State: override, Diagnostics: overrideDiags, @@ -2506,7 +2508,7 @@ func (n *NodeAbstractResourceInstance) apply( // values the first time the object is created. Otherwise, we're happy // to just apply whatever the user asked for. if change.Action == plans.Create { - override, overrideDiags := mocking.ApplyComputedValuesForResource(unmarkedAfter, getMockedData(n.override), schema) + override, overrideDiags := mocking.ApplyComputedValuesForResource(unmarkedAfter, getMockedData(n.override, false), schema) resp = providers.ApplyResourceChangeResponse{ NewState: override, Diagnostics: overrideDiags, diff --git a/internal/terraform/node_resource_plan_instance.go b/internal/terraform/node_resource_plan_instance.go index 52ae58a847cf..aa1163f5f857 100644 --- a/internal/terraform/node_resource_plan_instance.go +++ b/internal/terraform/node_resource_plan_instance.go @@ -634,7 +634,7 @@ func (n *NodePlannableResourceInstance) importState(ctx EvalContext, addr addrs. // Let's pretend we're reading the value as a data source so we // pre-compute values now as if the resource has already been created. - override, overrideDiags := mocking.ComputedValuesForDataSource(configVal, getMockedData(n.override), schema) + override, overrideDiags := mocking.ComputedValuesForDataSource(configVal, getMockedData(n.override, false), schema) resp = providers.ImportResourceStateResponse{ ImportedResources: []providers.ImportedResource{ { From 54c2b6c10cf5164399d3fb520a549fb76f7642b9 Mon Sep 17 00:00:00 2001 From: Samsondeen Dare Date: Mon, 16 Dec 2024 22:13:45 +0100 Subject: [PATCH 04/18] change pointer param --- internal/moduletest/mocking/values.go | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/internal/moduletest/mocking/values.go b/internal/moduletest/mocking/values.go index 4ec5c7acdf25..1ff9f12ddb19 100644 --- a/internal/moduletest/mocking/values.go +++ b/internal/moduletest/mocking/values.go @@ -20,14 +20,13 @@ import ( // The latter behaviour simulates the behaviour of a plan request in a real // provider. func PlanComputedValuesForResource(original cty.Value, with *MockedData, schema *configschema.Block) (cty.Value, tfdiags.Diagnostics) { - mocked := with if with == nil { - mocked = &MockedData{ + with = &MockedData{ Value: cty.NilVal, ComputedAsUnknown: true, } } - return populateComputedValues(original, mocked, schema, isNull) + return populateComputedValues(original, *with, schema, isNull) } // ApplyComputedValuesForResource accepts a target value, and populates it @@ -38,13 +37,12 @@ func PlanComputedValuesForResource(original cty.Value, with *MockedData, schema // This method basically simulates the behaviour of an apply request in a real // provider. func ApplyComputedValuesForResource(original cty.Value, with *MockedData, schema *configschema.Block) (cty.Value, tfdiags.Diagnostics) { - mocked := with if with == nil { - mocked = &MockedData{ + with = &MockedData{ Value: cty.NilVal, } } - return populateComputedValues(original, mocked, schema, isUnknown) + return populateComputedValues(original, *with, schema, isUnknown) } // ComputedValuesForDataSource accepts a target value, and populates it either @@ -58,27 +56,21 @@ func ApplyComputedValuesForResource(original cty.Value, with *MockedData, schema // This method basically simulates the behaviour of a get data source request // in a real provider. func ComputedValuesForDataSource(original cty.Value, with *MockedData, schema *configschema.Block) (cty.Value, tfdiags.Diagnostics) { - mocked := with if with == nil { - mocked = &MockedData{ + with = &MockedData{ Value: cty.NilVal, } } - return populateComputedValues(original, mocked, schema, isNull) + return populateComputedValues(original, *with, schema, isNull) } type processValue func(value cty.Value) bool type generateValue func(attribute *configschema.Attribute, with cty.Value, path cty.Path) (cty.Value, tfdiags.Diagnostics) -func populateComputedValues(target cty.Value, mocked *MockedData, schema *configschema.Block, processValue processValue) (cty.Value, tfdiags.Diagnostics) { +func populateComputedValues(target cty.Value, with MockedData, schema *configschema.Block, processValue processValue) (cty.Value, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics - with := MockedData{} - if mocked != nil { - with = *mocked - } - var generateValue generateValue // If the computed attributes should be ignored, then we will generate // unknown values for them, otherwise we will From f69fcfd3fd6f5511fc9b6591f3108122d3c9e4d4 Mon Sep 17 00:00:00 2001 From: Samsondeen Dare Date: Tue, 17 Dec 2024 15:30:30 +0100 Subject: [PATCH 05/18] add tests to show that plan needs flag to use overrides --- .../testdata/test/mocking-error/child/main.tf | 30 ++++++++++++ .../testdata/test/mocking-error/main.tf | 46 +++++++++++++++++++ .../tests/plan_mocked_overridden.tftest.hcl | 33 +++++++++++++ .../tests/plan_mocked_overridden.tftest.hcl | 32 +++++++++++++ internal/configs/mock_provider.go | 15 +++--- .../node_resource_abstract_instance.go | 2 +- 6 files changed, 151 insertions(+), 7 deletions(-) create mode 100644 internal/command/testdata/test/mocking-error/child/main.tf create mode 100644 internal/command/testdata/test/mocking-error/main.tf create mode 100644 internal/command/testdata/test/mocking-error/tests/plan_mocked_overridden.tftest.hcl create mode 100644 internal/command/testdata/test/mocking/tests/plan_mocked_overridden.tftest.hcl diff --git a/internal/command/testdata/test/mocking-error/child/main.tf b/internal/command/testdata/test/mocking-error/child/main.tf new file mode 100644 index 000000000000..2ef4e4d97991 --- /dev/null +++ b/internal/command/testdata/test/mocking-error/child/main.tf @@ -0,0 +1,30 @@ +terraform { + required_providers { + test = { + source = "hashicorp/test" + configuration_aliases = [test.primary, test.secondary] + } + } +} + +variable "instances" { + type = number +} + +resource "test_resource" "primary" { + provider = test.primary + count = var.instances +} + +resource "test_resource" "secondary" { + provider = test.secondary + count = var.instances +} + +output "primary" { + value = test_resource.primary +} + +output "secondary" { + value = test_resource.secondary +} diff --git a/internal/command/testdata/test/mocking-error/main.tf b/internal/command/testdata/test/mocking-error/main.tf new file mode 100644 index 000000000000..49506e06c38f --- /dev/null +++ b/internal/command/testdata/test/mocking-error/main.tf @@ -0,0 +1,46 @@ +terraform { + required_providers { + test = { + source = "hashicorp/test" + } + } +} + +provider "test" { + alias = "primary" +} + +provider "test" { + alias = "secondary" +} + +variable "instances" { + type = number +} + +variable "child_instances" { + type = number +} + +resource "test_resource" "primary" { + provider = test.primary + count = var.instances +} + +resource "test_resource" "secondary" { + provider = test.secondary + count = var.instances +} + +module "child" { + count = var.instances + + source = "./child" + + providers = { + test.primary = test.primary + test.secondary = test.secondary + } + + instances = var.child_instances +} diff --git a/internal/command/testdata/test/mocking-error/tests/plan_mocked_overridden.tftest.hcl b/internal/command/testdata/test/mocking-error/tests/plan_mocked_overridden.tftest.hcl new file mode 100644 index 000000000000..5efea37e18a5 --- /dev/null +++ b/internal/command/testdata/test/mocking-error/tests/plan_mocked_overridden.tftest.hcl @@ -0,0 +1,33 @@ +mock_provider "test" { + alias = "primary" + + mock_resource "test_resource" { + defaults = { + id = "aaaa" + } + } + + override_resource { + target = test_resource.primary + values = { + id = "bbbb" + } + } +} + +variables { + instances = 1 + child_instances = 1 +} + +// This test will fail because the plan command does not use the +// overridden values, making the left-hand side of the condition unknown. +run "test" { + command = plan + + assert { + condition = test_resource.primary[0].id != "bbbb" + error_message = "plan should not have the overridden value" + } + +} diff --git a/internal/command/testdata/test/mocking/tests/plan_mocked_overridden.tftest.hcl b/internal/command/testdata/test/mocking/tests/plan_mocked_overridden.tftest.hcl new file mode 100644 index 000000000000..f764786079bc --- /dev/null +++ b/internal/command/testdata/test/mocking/tests/plan_mocked_overridden.tftest.hcl @@ -0,0 +1,32 @@ +mock_provider "test" { + alias = "primary" + + mock_resource "test_resource" { + defaults = { + id = "aaaa" + } + } + + override_resource { + target = test_resource.primary + trigger_when_plan = true + values = { + id = "bbbb" + } + } +} + +variables { + instances = 1 + child_instances = 1 +} + +run "test" { + command = plan + + assert { + condition = test_resource.primary[0].id == "bbbb" + error_message = "plan should override the value when trigger_when_plan is true" + } + +} diff --git a/internal/configs/mock_provider.go b/internal/configs/mock_provider.go index 5bb570e3fe80..f923cb9275e6 100644 --- a/internal/configs/mock_provider.go +++ b/internal/configs/mock_provider.go @@ -170,9 +170,9 @@ const ( // replacement values that should be used in place of whatever the underlying // provider would normally do. type Override struct { - Target *addrs.Target - Values cty.Value - Ignore bool + Target *addrs.Target + Values cty.Value + IgnoreValues bool // Source tells us where this Override was defined. Source OverrideSource @@ -448,13 +448,16 @@ func decodeOverrideBlock(block *hcl.Block, attributeName string, blockName strin override.Values = cty.EmptyObjectVal } + // By default, the override values are ignored when planning. + // This can be overridden by setting the triggerWhenPlan attribute to true. + override.IgnoreValues = true if attribute, exists := content.Attributes[triggerWhenPlan]; exists { var valueDiags hcl.Diagnostics val, valueDiags := attribute.Expr.Value(nil) diags = append(diags, valueDiags...) if val.Type().Equals(cty.Bool) { - var triggerOverride bool - err := gocty.FromCtyValue(val, &triggerOverride) + var useOverrideValues bool + err := gocty.FromCtyValue(val, &useOverrideValues) if err != nil { // should not happen as we already checked the type diags = diags.Append(&hcl.Diagnostic{ @@ -464,7 +467,7 @@ func decodeOverrideBlock(block *hcl.Block, attributeName string, blockName strin Subject: attribute.Range.Ptr(), }) } - override.Ignore = !triggerOverride + override.IgnoreValues = !useOverrideValues } else { diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, diff --git a/internal/terraform/node_resource_abstract_instance.go b/internal/terraform/node_resource_abstract_instance.go index ad11e9050170..3bb7eb79484b 100644 --- a/internal/terraform/node_resource_abstract_instance.go +++ b/internal/terraform/node_resource_abstract_instance.go @@ -1240,7 +1240,7 @@ func getMockedData(override *configs.Override, isPlan bool) *mocking.MockedData Range: override.Range, // Apply never ignores computed values. This attribute only matters // when we are planning. - ComputedAsUnknown: override.Ignore && isPlan, + ComputedAsUnknown: override.IgnoreValues && isPlan, } } From bb0644e59ee6764bd5e732a2e20264824da152ca Mon Sep 17 00:00:00 2001 From: Samsondeen Dare Date: Tue, 17 Dec 2024 15:44:34 +0100 Subject: [PATCH 06/18] some clarity improvement --- internal/moduletest/mocking/values.go | 2 +- .../node_resource_abstract_instance.go | 48 ++++++++++++------- .../terraform/node_resource_plan_instance.go | 6 ++- 3 files changed, 38 insertions(+), 18 deletions(-) diff --git a/internal/moduletest/mocking/values.go b/internal/moduletest/mocking/values.go index 1ff9f12ddb19..d374756ca35b 100644 --- a/internal/moduletest/mocking/values.go +++ b/internal/moduletest/mocking/values.go @@ -188,7 +188,7 @@ func makeUnknown(target *configschema.Attribute, _ cty.Value, _ cty.Path) (cty.V type MockedData struct { Value cty.Value Range hcl.Range - ComputedAsUnknown bool // If true, computed values are replaced with unknown. + ComputedAsUnknown bool // If true, computed values are replaced with unknown, otherwise they are replaced with generated values. } // NewMockedData creates a new MockedData struct with the given value and range. diff --git a/internal/terraform/node_resource_abstract_instance.go b/internal/terraform/node_resource_abstract_instance.go index 3bb7eb79484b..99f20a262d74 100644 --- a/internal/terraform/node_resource_abstract_instance.go +++ b/internal/terraform/node_resource_abstract_instance.go @@ -904,7 +904,11 @@ func (n *NodeAbstractResourceInstance) plan( if priorVal.IsNull() { // Then we are actually creating something, so let's populate the // computed values from our override value. - override, overrideDiags := mocking.PlanComputedValuesForResource(proposedNewVal, getMockedData(n.override, true), schema) + override, overrideDiags := mocking.PlanComputedValuesForResource(proposedNewVal, &mocking.MockedData{ + Value: n.override.Values, + Range: n.override.Range, + ComputedAsUnknown: n.override.IgnoreValues, + }, schema) resp = providers.PlanResourceChangeResponse{ PlannedState: override, Diagnostics: overrideDiags, @@ -1082,7 +1086,11 @@ func (n *NodeAbstractResourceInstance) plan( if n.override != nil { // In this case, we are always creating the resource so we don't // do any validation, and just call out to the mocking library. - override, overrideDiags := mocking.PlanComputedValuesForResource(proposedNewVal, getMockedData(n.override, true), schema) + override, overrideDiags := mocking.PlanComputedValuesForResource(proposedNewVal, &mocking.MockedData{ + Value: n.override.Values, + Range: n.override.Range, + ComputedAsUnknown: n.override.IgnoreValues, + }, schema) resp = providers.PlanResourceChangeResponse{ PlannedState: override, Diagnostics: overrideDiags, @@ -1231,18 +1239,18 @@ func (n *NodeAbstractResourceInstance) plan( return plan, state, deferred, keyData, diags } -func getMockedData(override *configs.Override, isPlan bool) *mocking.MockedData { - if override == nil { - return nil - } - return &mocking.MockedData{ - Value: override.Values, - Range: override.Range, - // Apply never ignores computed values. This attribute only matters - // when we are planning. - ComputedAsUnknown: override.IgnoreValues && isPlan, - } -} +// func getMockedData(override *configs.Override, isPlan bool) *mocking.MockedData { +// if override == nil { +// return nil +// } +// return &mocking.MockedData{ +// Value: override.Values, +// Range: override.Range, +// // Apply never ignores computed values. This attribute only matters +// // when we are planning. +// ComputedAsUnknown: override.IgnoreValues && isPlan, +// } +// } func (n *NodeAbstractResource) processIgnoreChanges(prior, config cty.Value, schema *configschema.Block) (cty.Value, tfdiags.Diagnostics) { // ignore_changes only applies when an object already exists, since we @@ -1556,7 +1564,11 @@ func (n *NodeAbstractResourceInstance) readDataSource(ctx EvalContext, configVal var resp providers.ReadDataSourceResponse if n.override != nil { - override, overrideDiags := mocking.ComputedValuesForDataSource(configVal, getMockedData(n.override, false), schema) + override, overrideDiags := mocking.ComputedValuesForDataSource(configVal, &mocking.MockedData{ + Value: n.override.Values, + Range: n.override.Range, + ComputedAsUnknown: false, + }, schema) resp = providers.ReadDataSourceResponse{ State: override, Diagnostics: overrideDiags, @@ -2508,7 +2520,11 @@ func (n *NodeAbstractResourceInstance) apply( // values the first time the object is created. Otherwise, we're happy // to just apply whatever the user asked for. if change.Action == plans.Create { - override, overrideDiags := mocking.ApplyComputedValuesForResource(unmarkedAfter, getMockedData(n.override, false), schema) + override, overrideDiags := mocking.ApplyComputedValuesForResource(unmarkedAfter, &mocking.MockedData{ + Value: n.override.Values, + Range: n.override.Range, + ComputedAsUnknown: false, + }, schema) resp = providers.ApplyResourceChangeResponse{ NewState: override, Diagnostics: overrideDiags, diff --git a/internal/terraform/node_resource_plan_instance.go b/internal/terraform/node_resource_plan_instance.go index aa1163f5f857..c3f6cd50d912 100644 --- a/internal/terraform/node_resource_plan_instance.go +++ b/internal/terraform/node_resource_plan_instance.go @@ -634,7 +634,11 @@ func (n *NodePlannableResourceInstance) importState(ctx EvalContext, addr addrs. // Let's pretend we're reading the value as a data source so we // pre-compute values now as if the resource has already been created. - override, overrideDiags := mocking.ComputedValuesForDataSource(configVal, getMockedData(n.override, false), schema) + override, overrideDiags := mocking.ComputedValuesForDataSource(configVal, &mocking.MockedData{ + Value: n.override.Values, + Range: n.override.Range, + ComputedAsUnknown: false, + }, schema) resp = providers.ImportResourceStateResponse{ ImportedResources: []providers.ImportedResource{ { From c0cbb3da31292b333a82aa5bbaa0a13cff6a569f Mon Sep 17 00:00:00 2001 From: Samsondeen Dare Date: Tue, 17 Dec 2024 18:14:47 +0100 Subject: [PATCH 07/18] fix tests --- internal/command/test_test.go | 8 +++++++- .../tests/plan_mocked_overridden.tftest.hcl | 2 +- .../terraform/node_resource_abstract_instance.go | 13 ------------- 3 files changed, 8 insertions(+), 15 deletions(-) diff --git a/internal/command/test_test.go b/internal/command/test_test.go index 9f0e6c0130e2..346b8f1a5d13 100644 --- a/internal/command/test_test.go +++ b/internal/command/test_test.go @@ -219,13 +219,19 @@ func TestTest_Runs(t *testing.T) { code: 0, }, "mocking": { - expectedOut: []string{"6 passed, 0 failed."}, + expectedOut: []string{"7 passed, 0 failed."}, code: 0, }, "mocking-invalid": { expectedErr: []string{"Invalid outputs attribute"}, initCode: 1, }, + "mocking-error": { + expectedErr: []string{"Unknown condition value", + "test_resource.primary[0].id", + }, + code: 1, + }, "dangling_data_block": { expectedOut: []string{"2 passed, 0 failed."}, code: 0, diff --git a/internal/command/testdata/test/mocking-error/tests/plan_mocked_overridden.tftest.hcl b/internal/command/testdata/test/mocking-error/tests/plan_mocked_overridden.tftest.hcl index 5efea37e18a5..85b9df6657c3 100644 --- a/internal/command/testdata/test/mocking-error/tests/plan_mocked_overridden.tftest.hcl +++ b/internal/command/testdata/test/mocking-error/tests/plan_mocked_overridden.tftest.hcl @@ -26,7 +26,7 @@ run "test" { command = plan assert { - condition = test_resource.primary[0].id != "bbbb" + condition = test_resource.primary[0].id == "bbbb" error_message = "plan should not have the overridden value" } diff --git a/internal/terraform/node_resource_abstract_instance.go b/internal/terraform/node_resource_abstract_instance.go index 99f20a262d74..a94da59d27fd 100644 --- a/internal/terraform/node_resource_abstract_instance.go +++ b/internal/terraform/node_resource_abstract_instance.go @@ -1239,19 +1239,6 @@ func (n *NodeAbstractResourceInstance) plan( return plan, state, deferred, keyData, diags } -// func getMockedData(override *configs.Override, isPlan bool) *mocking.MockedData { -// if override == nil { -// return nil -// } -// return &mocking.MockedData{ -// Value: override.Values, -// Range: override.Range, -// // Apply never ignores computed values. This attribute only matters -// // when we are planning. -// ComputedAsUnknown: override.IgnoreValues && isPlan, -// } -// } - func (n *NodeAbstractResource) processIgnoreChanges(prior, config cty.Value, schema *configschema.Block) (cty.Value, tfdiags.Diagnostics) { // ignore_changes only applies when an object already exists, since we // can't ignore changes to a thing we've not created yet. From 1e01fdf92b0ac6cd8baa991772b01fc0c940159f Mon Sep 17 00:00:00 2001 From: Samsondeen Dare Date: Wed, 18 Dec 2024 11:41:36 +0100 Subject: [PATCH 08/18] change flag name --- .../tests/plan_mocked_overridden.tftest.hcl | 4 +- internal/configs/mock_provider.go | 42 ++++++++++++------- internal/moduletest/mocking/values.go | 2 +- .../node_resource_abstract_instance.go | 4 +- 4 files changed, 32 insertions(+), 20 deletions(-) diff --git a/internal/command/testdata/test/mocking/tests/plan_mocked_overridden.tftest.hcl b/internal/command/testdata/test/mocking/tests/plan_mocked_overridden.tftest.hcl index f764786079bc..57a3220567a5 100644 --- a/internal/command/testdata/test/mocking/tests/plan_mocked_overridden.tftest.hcl +++ b/internal/command/testdata/test/mocking/tests/plan_mocked_overridden.tftest.hcl @@ -9,7 +9,7 @@ mock_provider "test" { override_resource { target = test_resource.primary - trigger_when_plan = true + force_computed_override = true values = { id = "bbbb" } @@ -26,7 +26,7 @@ run "test" { assert { condition = test_resource.primary[0].id == "bbbb" - error_message = "plan should override the value when trigger_when_plan is true" + error_message = "plan should override the value when force_computed_override is true" } } diff --git a/internal/configs/mock_provider.go b/internal/configs/mock_provider.go index f923cb9275e6..f04f982edc6c 100644 --- a/internal/configs/mock_provider.go +++ b/internal/configs/mock_provider.go @@ -170,9 +170,15 @@ const ( // replacement values that should be used in place of whatever the underlying // provider would normally do. type Override struct { - Target *addrs.Target - Values cty.Value - IgnoreValues bool + Target *addrs.Target + Values cty.Value + + // By default, overridden computed values are ignored during planning, + // and the computed values are set to unknown to simulate the behavior + // of a real plan. This attribute indicates that the computed values + // should be overridden with the values specified in the override block, + // even when planning. + planComputedOverride *bool // Source tells us where this Override was defined. Source OverrideSource @@ -183,6 +189,12 @@ type Override struct { ValuesRange hcl.Range } +// UseOverridesForPlan returns true if the computed values in the target +// resource should be overridden with the values specified in the override block. +func (o *Override) UseOverridesForPlan() bool { + return o.planComputedOverride != nil && *o.planComputedOverride +} + func decodeMockDataBody(body hcl.Body, source OverrideSource) (*MockData, hcl.Diagnostics) { var diags hcl.Diagnostics @@ -396,8 +408,10 @@ func decodeOverrideDataBlock(block *hcl.Block, source OverrideSource) (*Override } var ( - // triggerWhenPlan is the attribute name for specifying whether to trigger overrides when planning. - triggerWhenPlan = "trigger_when_plan" + // When this attribute is set to true, the values specified in the override + // block will be used for computed attributes even when planning. Otherwise, + // the computed values will be set to unknown, just like in a real plan. + forceComputedOverride = "force_computed_override" ) func decodeOverrideBlock(block *hcl.Block, attributeName string, blockName string, source OverrideSource) (*Override, hcl.Diagnostics) { @@ -406,7 +420,7 @@ func decodeOverrideBlock(block *hcl.Block, attributeName string, blockName strin content, contentDiags := block.Body.Content(&hcl.BodySchema{ Attributes: []hcl.AttributeSchema{ {Name: "target"}, - {Name: triggerWhenPlan}, + {Name: forceComputedOverride}, {Name: attributeName}, }, }) @@ -448,10 +462,8 @@ func decodeOverrideBlock(block *hcl.Block, attributeName string, blockName strin override.Values = cty.EmptyObjectVal } - // By default, the override values are ignored when planning. - // This can be overridden by setting the triggerWhenPlan attribute to true. - override.IgnoreValues = true - if attribute, exists := content.Attributes[triggerWhenPlan]; exists { + // Override computed values during planning if force_computed_override is true. + if attribute, exists := content.Attributes[forceComputedOverride]; exists { var valueDiags hcl.Diagnostics val, valueDiags := attribute.Expr.Value(nil) diags = append(diags, valueDiags...) @@ -462,17 +474,17 @@ func decodeOverrideBlock(block *hcl.Block, attributeName string, blockName strin // should not happen as we already checked the type diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, - Summary: fmt.Sprintf("Invalid %s value", triggerWhenPlan), - Detail: fmt.Sprintf("The %s attribute must be a boolean.", triggerWhenPlan), + Summary: fmt.Sprintf("Invalid %s value", forceComputedOverride), + Detail: fmt.Sprintf("The %s attribute must be a boolean.", forceComputedOverride), Subject: attribute.Range.Ptr(), }) } - override.IgnoreValues = !useOverrideValues + override.planComputedOverride = &useOverrideValues } else { diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, - Summary: fmt.Sprintf("Invalid %s value", triggerWhenPlan), - Detail: fmt.Sprintf("The %s attribute must be a boolean.", triggerWhenPlan), + Summary: fmt.Sprintf("Invalid %s value", forceComputedOverride), + Detail: fmt.Sprintf("The %s attribute must be a boolean.", forceComputedOverride), Subject: attribute.Range.Ptr(), }) } diff --git a/internal/moduletest/mocking/values.go b/internal/moduletest/mocking/values.go index d374756ca35b..5b09fe9cbc9a 100644 --- a/internal/moduletest/mocking/values.go +++ b/internal/moduletest/mocking/values.go @@ -188,7 +188,7 @@ func makeUnknown(target *configschema.Attribute, _ cty.Value, _ cty.Path) (cty.V type MockedData struct { Value cty.Value Range hcl.Range - ComputedAsUnknown bool // If true, computed values are replaced with unknown, otherwise they are replaced with generated values. + ComputedAsUnknown bool // If true, computed values are replaced with unknown, otherwise they are replaced with overridden or generated values. } // NewMockedData creates a new MockedData struct with the given value and range. diff --git a/internal/terraform/node_resource_abstract_instance.go b/internal/terraform/node_resource_abstract_instance.go index a94da59d27fd..d115820cd42c 100644 --- a/internal/terraform/node_resource_abstract_instance.go +++ b/internal/terraform/node_resource_abstract_instance.go @@ -907,7 +907,7 @@ func (n *NodeAbstractResourceInstance) plan( override, overrideDiags := mocking.PlanComputedValuesForResource(proposedNewVal, &mocking.MockedData{ Value: n.override.Values, Range: n.override.Range, - ComputedAsUnknown: n.override.IgnoreValues, + ComputedAsUnknown: !n.override.UseOverridesForPlan(), }, schema) resp = providers.PlanResourceChangeResponse{ PlannedState: override, @@ -1089,7 +1089,7 @@ func (n *NodeAbstractResourceInstance) plan( override, overrideDiags := mocking.PlanComputedValuesForResource(proposedNewVal, &mocking.MockedData{ Value: n.override.Values, Range: n.override.Range, - ComputedAsUnknown: n.override.IgnoreValues, + ComputedAsUnknown: !n.override.UseOverridesForPlan(), }, schema) resp = providers.PlanResourceChangeResponse{ PlannedState: override, From 1e5d9ec41735ce18680980d633510dfcb2b2cbf0 Mon Sep 17 00:00:00 2001 From: Samsondeen Dare Date: Wed, 18 Dec 2024 12:40:19 +0100 Subject: [PATCH 09/18] change flag name (2) --- .../tests/plan_mocked_overridden.tftest.hcl | 3 ++- .../tests/plan_mocked_overridden.tftest.hcl | 4 ++-- internal/configs/mock_provider.go | 16 ++++++++-------- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/internal/command/testdata/test/mocking-error/tests/plan_mocked_overridden.tftest.hcl b/internal/command/testdata/test/mocking-error/tests/plan_mocked_overridden.tftest.hcl index 85b9df6657c3..5003cab76564 100644 --- a/internal/command/testdata/test/mocking-error/tests/plan_mocked_overridden.tftest.hcl +++ b/internal/command/testdata/test/mocking-error/tests/plan_mocked_overridden.tftest.hcl @@ -21,7 +21,8 @@ variables { } // This test will fail because the plan command does not use the -// overridden values, making the left-hand side of the condition unknown. +// overridden values for computed properties, +// making the left-hand side of the condition unknown. run "test" { command = plan diff --git a/internal/command/testdata/test/mocking/tests/plan_mocked_overridden.tftest.hcl b/internal/command/testdata/test/mocking/tests/plan_mocked_overridden.tftest.hcl index 57a3220567a5..a65be82f4a77 100644 --- a/internal/command/testdata/test/mocking/tests/plan_mocked_overridden.tftest.hcl +++ b/internal/command/testdata/test/mocking/tests/plan_mocked_overridden.tftest.hcl @@ -9,7 +9,7 @@ mock_provider "test" { override_resource { target = test_resource.primary - force_computed_override = true + override_computed = true values = { id = "bbbb" } @@ -26,7 +26,7 @@ run "test" { assert { condition = test_resource.primary[0].id == "bbbb" - error_message = "plan should override the value when force_computed_override is true" + error_message = "plan should override the value when override_computed is true" } } diff --git a/internal/configs/mock_provider.go b/internal/configs/mock_provider.go index f04f982edc6c..97cbce3040c4 100644 --- a/internal/configs/mock_provider.go +++ b/internal/configs/mock_provider.go @@ -411,7 +411,7 @@ var ( // When this attribute is set to true, the values specified in the override // block will be used for computed attributes even when planning. Otherwise, // the computed values will be set to unknown, just like in a real plan. - forceComputedOverride = "force_computed_override" + overrideComputed = "override_computed" ) func decodeOverrideBlock(block *hcl.Block, attributeName string, blockName string, source OverrideSource) (*Override, hcl.Diagnostics) { @@ -420,7 +420,7 @@ func decodeOverrideBlock(block *hcl.Block, attributeName string, blockName strin content, contentDiags := block.Body.Content(&hcl.BodySchema{ Attributes: []hcl.AttributeSchema{ {Name: "target"}, - {Name: forceComputedOverride}, + {Name: overrideComputed}, {Name: attributeName}, }, }) @@ -462,8 +462,8 @@ func decodeOverrideBlock(block *hcl.Block, attributeName string, blockName strin override.Values = cty.EmptyObjectVal } - // Override computed values during planning if force_computed_override is true. - if attribute, exists := content.Attributes[forceComputedOverride]; exists { + // Override computed values during planning if override_computed is true. + if attribute, exists := content.Attributes[overrideComputed]; exists { var valueDiags hcl.Diagnostics val, valueDiags := attribute.Expr.Value(nil) diags = append(diags, valueDiags...) @@ -474,8 +474,8 @@ func decodeOverrideBlock(block *hcl.Block, attributeName string, blockName strin // should not happen as we already checked the type diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, - Summary: fmt.Sprintf("Invalid %s value", forceComputedOverride), - Detail: fmt.Sprintf("The %s attribute must be a boolean.", forceComputedOverride), + Summary: fmt.Sprintf("Invalid %s value", overrideComputed), + Detail: fmt.Sprintf("The %s attribute must be a boolean.", overrideComputed), Subject: attribute.Range.Ptr(), }) } @@ -483,8 +483,8 @@ func decodeOverrideBlock(block *hcl.Block, attributeName string, blockName strin } else { diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, - Summary: fmt.Sprintf("Invalid %s value", forceComputedOverride), - Detail: fmt.Sprintf("The %s attribute must be a boolean.", forceComputedOverride), + Summary: fmt.Sprintf("Invalid %s value", overrideComputed), + Detail: fmt.Sprintf("The %s attribute must be a boolean.", overrideComputed), Subject: attribute.Range.Ptr(), }) } From 7818c94357c1ad8412a1db431277e1a5fe9d06be Mon Sep 17 00:00:00 2001 From: Samsondeen Dare Date: Wed, 18 Dec 2024 13:39:44 +0100 Subject: [PATCH 10/18] support provider_level override_computed flag --- internal/command/test_test.go | 2 +- ...plan_mocked_provider_overridden.tftest.hcl | 40 ++++++++++ internal/configs/mock_provider.go | 78 ++++++++++++------- 3 files changed, 93 insertions(+), 27 deletions(-) create mode 100644 internal/command/testdata/test/mocking/tests/plan_mocked_provider_overridden.tftest.hcl diff --git a/internal/command/test_test.go b/internal/command/test_test.go index 346b8f1a5d13..313f20b72050 100644 --- a/internal/command/test_test.go +++ b/internal/command/test_test.go @@ -219,7 +219,7 @@ func TestTest_Runs(t *testing.T) { code: 0, }, "mocking": { - expectedOut: []string{"7 passed, 0 failed."}, + expectedOut: []string{"8 passed, 0 failed."}, code: 0, }, "mocking-invalid": { diff --git a/internal/command/testdata/test/mocking/tests/plan_mocked_provider_overridden.tftest.hcl b/internal/command/testdata/test/mocking/tests/plan_mocked_provider_overridden.tftest.hcl new file mode 100644 index 000000000000..665fb77503f5 --- /dev/null +++ b/internal/command/testdata/test/mocking/tests/plan_mocked_provider_overridden.tftest.hcl @@ -0,0 +1,40 @@ +mock_provider "test" { + alias = "primary" + override_computed = true + + mock_resource "test_resource" { + defaults = { + id = "aaaa" + } + } + + override_resource { + target = test_resource.primary + values = { + id = "bbbb" + } + } + + override_resource { + target = test_resource.primary[1] + override_computed = false // this should take precedence over the provider-level override_computed + values = { + id = "bbbb" + } + } +} + +variables { + instances = 2 + child_instances = 1 +} + +run "test" { + command = plan + + assert { + condition = test_resource.primary[0].id == "bbbb" + error_message = "plan should override the value when override_computed is true" + } + +} diff --git a/internal/configs/mock_provider.go b/internal/configs/mock_provider.go index 97cbce3040c4..00aabce77613 100644 --- a/internal/configs/mock_provider.go +++ b/internal/configs/mock_provider.go @@ -63,9 +63,51 @@ func decodeMockProviderBlock(block *hcl.Block) (*Provider, hcl.Diagnostics) { diags = append(diags, sourceDiags...) } + overrideComputedBool, valueDiags := extractOverrideComputed(content) + diags = append(diags, valueDiags...) + // Set the overrideComputed value for all the elements in the MockData + // if it is not already set. + for _, elem := range provider.MockData.Overrides.Elements() { + if elem.Value.planComputedOverride == nil { + elem.Value.planComputedOverride = overrideComputedBool + } + provider.MockData.Overrides.Put(elem.Key, elem.Value) + } + return provider, diags } +func extractOverrideComputed(content *hcl.BodyContent) (*bool, hcl.Diagnostics) { + var diags hcl.Diagnostics + + if attr, exists := content.Attributes[overrideComputed]; exists { + val, valueDiags := attr.Expr.Value(nil) + diags = append(diags, valueDiags...) + if val.Type().Equals(cty.Bool) { + var overrideComputedBool bool + err := gocty.FromCtyValue(val, &overrideComputedBool) + if err != nil { + // should not happen as we already checked the type + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: fmt.Sprintf("Invalid %s value", overrideComputed), + Detail: fmt.Sprintf("The %s attribute must be a boolean.", overrideComputed), + Subject: attr.Range.Ptr(), + }) + } + return &overrideComputedBool, diags + } + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: fmt.Sprintf("Invalid %s value", overrideComputed), + Detail: fmt.Sprintf("The %s attribute must be a boolean.", overrideComputed), + Subject: attr.Range.Ptr(), + }) + } + + return nil, diags +} + // MockData packages up all the available mock and override data available to // a mocked provider. type MockData struct { @@ -463,32 +505,9 @@ func decodeOverrideBlock(block *hcl.Block, attributeName string, blockName strin } // Override computed values during planning if override_computed is true. - if attribute, exists := content.Attributes[overrideComputed]; exists { - var valueDiags hcl.Diagnostics - val, valueDiags := attribute.Expr.Value(nil) - diags = append(diags, valueDiags...) - if val.Type().Equals(cty.Bool) { - var useOverrideValues bool - err := gocty.FromCtyValue(val, &useOverrideValues) - if err != nil { - // should not happen as we already checked the type - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: fmt.Sprintf("Invalid %s value", overrideComputed), - Detail: fmt.Sprintf("The %s attribute must be a boolean.", overrideComputed), - Subject: attribute.Range.Ptr(), - }) - } - override.planComputedOverride = &useOverrideValues - } else { - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: fmt.Sprintf("Invalid %s value", overrideComputed), - Detail: fmt.Sprintf("The %s attribute must be a boolean.", overrideComputed), - Subject: attribute.Range.Ptr(), - }) - } - } + overrideComputedBool, valueDiags := extractOverrideComputed(content) + diags = append(diags, valueDiags...) + override.planComputedOverride = overrideComputedBool if !override.Values.Type().IsObjectType() { @@ -516,6 +535,9 @@ var mockProviderSchema = &hcl.BodySchema{ { Name: "alias", }, + { + Name: overrideComputed, + }, { Name: "source", }, @@ -523,6 +545,10 @@ var mockProviderSchema = &hcl.BodySchema{ } var mockDataSchema = &hcl.BodySchema{ + Attributes: []hcl.AttributeSchema{ + {Name: overrideComputed}, + {Name: "alias"}, + }, Blocks: []hcl.BlockHeaderSchema{ {Type: "mock_resource", LabelNames: []string{"type"}}, {Type: "mock_data", LabelNames: []string{"type"}}, From b1049f75aec400c42573ac351d0df83288a428ec Mon Sep 17 00:00:00 2001 From: Samsondeen Dare Date: Wed, 18 Dec 2024 13:48:49 +0100 Subject: [PATCH 11/18] more tests --- internal/command/test_test.go | 10 ++++-- ...erride_computed_invalid_boolean.tftest.hcl | 30 +++++++++++++++++ .../primary_mocked_overridden.tftest.hcl | 6 ++++ internal/configs/mock_provider.go | 33 ++++++------------- 4 files changed, 53 insertions(+), 26 deletions(-) create mode 100644 internal/command/testdata/test/mocking-invalid/tests/override_computed_invalid_boolean.tftest.hcl diff --git a/internal/command/test_test.go b/internal/command/test_test.go index 313f20b72050..94af10732550 100644 --- a/internal/command/test_test.go +++ b/internal/command/test_test.go @@ -223,11 +223,15 @@ func TestTest_Runs(t *testing.T) { code: 0, }, "mocking-invalid": { - expectedErr: []string{"Invalid outputs attribute"}, - initCode: 1, + expectedErr: []string{ + "Invalid outputs attribute", + "The override_computed attribute must be a boolean.", + }, + initCode: 1, }, "mocking-error": { - expectedErr: []string{"Unknown condition value", + expectedErr: []string{ + "Unknown condition value", "test_resource.primary[0].id", }, code: 1, diff --git a/internal/command/testdata/test/mocking-invalid/tests/override_computed_invalid_boolean.tftest.hcl b/internal/command/testdata/test/mocking-invalid/tests/override_computed_invalid_boolean.tftest.hcl new file mode 100644 index 000000000000..59ce59f3f3e8 --- /dev/null +++ b/internal/command/testdata/test/mocking-invalid/tests/override_computed_invalid_boolean.tftest.hcl @@ -0,0 +1,30 @@ +mock_provider "test" { + alias = "primary" + override_computed = foo // This should be a boolean value, therefore this test should fail + + mock_resource "test_resource" { + defaults = { + id = "aaaa" + } + } + + override_resource { + target = test_resource.primary + values = { + id = "bbbb" + } + } +} + +variables { + instances = 1 + child_instances = 1 +} + +run "test" { + + assert { + condition = test_resource.primary[0].id == "bbbb" + error_message = "mock not applied" + } +} diff --git a/internal/command/testdata/test/mocking/tests/primary_mocked_overridden.tftest.hcl b/internal/command/testdata/test/mocking/tests/primary_mocked_overridden.tftest.hcl index 6d4eeff4c0ff..62fca3e29c3f 100644 --- a/internal/command/testdata/test/mocking/tests/primary_mocked_overridden.tftest.hcl +++ b/internal/command/testdata/test/mocking/tests/primary_mocked_overridden.tftest.hcl @@ -55,4 +55,10 @@ run "test" { error_message = "override from another instance affected this instance" } + assert { + // Provider Override should propagate to the child module + condition = module.child[0].primary[0].id == "aaaa" + error_message = "did not apply mocks" + } + } diff --git a/internal/configs/mock_provider.go b/internal/configs/mock_provider.go index 00aabce77613..f537b50113aa 100644 --- a/internal/configs/mock_provider.go +++ b/internal/configs/mock_provider.go @@ -83,26 +83,17 @@ func extractOverrideComputed(content *hcl.BodyContent) (*bool, hcl.Diagnostics) if attr, exists := content.Attributes[overrideComputed]; exists { val, valueDiags := attr.Expr.Value(nil) diags = append(diags, valueDiags...) - if val.Type().Equals(cty.Bool) { - var overrideComputedBool bool - err := gocty.FromCtyValue(val, &overrideComputedBool) - if err != nil { - // should not happen as we already checked the type - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: fmt.Sprintf("Invalid %s value", overrideComputed), - Detail: fmt.Sprintf("The %s attribute must be a boolean.", overrideComputed), - Subject: attr.Range.Ptr(), - }) - } - return &overrideComputedBool, diags + var overrideComputedBool bool + err := gocty.FromCtyValue(val, &overrideComputedBool) + if err != nil { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: fmt.Sprintf("Invalid %s value", overrideComputed), + Detail: fmt.Sprintf("The %s attribute must be a boolean.", overrideComputed), + Subject: attr.Range.Ptr(), + }) } - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: fmt.Sprintf("Invalid %s value", overrideComputed), - Detail: fmt.Sprintf("The %s attribute must be a boolean.", overrideComputed), - Subject: attr.Range.Ptr(), - }) + return &overrideComputedBool, diags } return nil, diags @@ -545,10 +536,6 @@ var mockProviderSchema = &hcl.BodySchema{ } var mockDataSchema = &hcl.BodySchema{ - Attributes: []hcl.AttributeSchema{ - {Name: overrideComputed}, - {Name: "alias"}, - }, Blocks: []hcl.BlockHeaderSchema{ {Type: "mock_resource", LabelNames: []string{"type"}}, {Type: "mock_data", LabelNames: []string{"type"}}, From 60aad1cf75b4a6bada2c3ec8da71ef38585eb816 Mon Sep 17 00:00:00 2001 From: Samsondeen Dare Date: Thu, 19 Dec 2024 09:36:34 +0100 Subject: [PATCH 12/18] minor improvements --- .../plan_mocked_provider_overridden.tftest.hcl | 15 +++++++++++++++ internal/moduletest/eval_context.go | 2 +- internal/providers/mock.go | 4 ++-- 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/internal/command/testdata/test/mocking/tests/plan_mocked_provider_overridden.tftest.hcl b/internal/command/testdata/test/mocking/tests/plan_mocked_provider_overridden.tftest.hcl index 665fb77503f5..354ce2b7b2cf 100644 --- a/internal/command/testdata/test/mocking/tests/plan_mocked_provider_overridden.tftest.hcl +++ b/internal/command/testdata/test/mocking/tests/plan_mocked_provider_overridden.tftest.hcl @@ -24,6 +24,16 @@ mock_provider "test" { } } + +override_resource { + target = test_resource.secondary[0] + override_computed = true + values = { + id = "ssss" + } +} + + variables { instances = 2 child_instances = 1 @@ -37,4 +47,9 @@ run "test" { error_message = "plan should override the value when override_computed is true" } + assert { + condition = test_resource.secondary[0].id == "ssss" + error_message = "plan should override the value when override_computed is true" + } + } diff --git a/internal/moduletest/eval_context.go b/internal/moduletest/eval_context.go index 0c4583b706a8..623d71f4fb4a 100644 --- a/internal/moduletest/eval_context.go +++ b/internal/moduletest/eval_context.go @@ -141,7 +141,7 @@ func (ec *EvalContext) Evaluate() (Status, cty.Value, tfdiags.Diagnostics) { diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, Summary: "Unknown condition value", - Detail: "Condition expression could not be evaluated at this time. This means you have executed a `run` block with `command = plan` and one of the values your condition depended on is not known until after the plan has been applied. Either remove this value from your condition, or execute an `apply` command from this `run` block.", + Detail: "Condition expression could not be evaluated at this time. This means you have executed a `run` block with `command = plan` and one of the values your condition depended on is not known until after the plan has been applied. Either remove this value from your condition, or execute an `apply` command from this `run` block. Alternatively, if there is an override for this value, you can make it available during the plan phase by setting `override_computed = true` in the `override_` block.", Subject: rule.Condition.Range().Ptr(), Expression: rule.Condition, EvalContext: hclCtx, diff --git a/internal/providers/mock.go b/internal/providers/mock.go index 9af422e96d20..c23dacf09442 100644 --- a/internal/providers/mock.go +++ b/internal/providers/mock.go @@ -182,8 +182,8 @@ func (m *Mock) PlanResourceChange(request PlanResourceChangeRequest) PlanResourc panic(fmt.Errorf("failed to retrieve schema for resource %s", request.TypeName)) } - // If the provider was overriden in the test (via override_*), the mock provider is not called at all, - // so we can be certain that this provider is not mocked. + // If the provider was overriden via override_* blocks, the mock provider is not called at all, + // so we can continue to make computed values as unknown here (until mock defaults support plan command). value, diags := mocking.PlanComputedValuesForResource(request.ProposedNewState, &mocking.MockedData{ ComputedAsUnknown: true, }, resource.Block) From 4ccfe7c4052b367e536a26e4ffbc2902ff722168 Mon Sep 17 00:00:00 2001 From: Samsondeen Dare Date: Thu, 19 Dec 2024 11:53:32 +0100 Subject: [PATCH 13/18] minor improvements (2) --- internal/configs/mock_provider.go | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/internal/configs/mock_provider.go b/internal/configs/mock_provider.go index f537b50113aa..aacc62b516e6 100644 --- a/internal/configs/mock_provider.go +++ b/internal/configs/mock_provider.go @@ -63,17 +63,6 @@ func decodeMockProviderBlock(block *hcl.Block) (*Provider, hcl.Diagnostics) { diags = append(diags, sourceDiags...) } - overrideComputedBool, valueDiags := extractOverrideComputed(content) - diags = append(diags, valueDiags...) - // Set the overrideComputed value for all the elements in the MockData - // if it is not already set. - for _, elem := range provider.MockData.Overrides.Elements() { - if elem.Value.planComputedOverride == nil { - elem.Value.planComputedOverride = overrideComputedBool - } - provider.MockData.Overrides.Put(elem.Key, elem.Value) - } - return provider, diags } @@ -234,6 +223,10 @@ func decodeMockDataBody(body hcl.Body, source OverrideSource) (*MockData, hcl.Di content, contentDiags := body.Content(mockDataSchema) diags = append(diags, contentDiags...) + // provider-level setting for overrideComputed + providerOverrideComputed, valueDiags := extractOverrideComputed(content) + diags = append(diags, valueDiags...) + data := &MockData{ MockResources: make(map[string]*MockResource), MockDataSources: make(map[string]*MockResource), @@ -309,6 +302,14 @@ func decodeMockDataBody(body hcl.Body, source OverrideSource) (*MockData, hcl.Di } } + for _, elem := range data.Overrides.Elements() { + // use the provider-level setting if there is none set for this override + if elem.Value.planComputedOverride == nil { + elem.Value.planComputedOverride = providerOverrideComputed + } + data.Overrides.Put(elem.Key, elem.Value) + } + return data, diags } @@ -526,9 +527,6 @@ var mockProviderSchema = &hcl.BodySchema{ { Name: "alias", }, - { - Name: overrideComputed, - }, { Name: "source", }, @@ -536,6 +534,9 @@ var mockProviderSchema = &hcl.BodySchema{ } var mockDataSchema = &hcl.BodySchema{ + Attributes: []hcl.AttributeSchema{ + {Name: overrideComputed}, + }, Blocks: []hcl.BlockHeaderSchema{ {Type: "mock_resource", LabelNames: []string{"type"}}, {Type: "mock_data", LabelNames: []string{"type"}}, From b75db068fa91826bf2cd39bc08a0f82df393ab27 Mon Sep 17 00:00:00 2001 From: Samsondeen Dare Date: Thu, 19 Dec 2024 12:17:59 +0100 Subject: [PATCH 14/18] allow mocked values override during planning --- internal/command/test_test.go | 5 +++- .../tests/plan_mocked_provider.tftest.hcl | 25 ++++++++++++++++++ .../tests/plan_mocked_provider.tftest.hcl | 26 +++++++++++++++++++ internal/configs/mock_provider.go | 11 +++++++- internal/providers/mock.go | 15 ++++++++--- 5 files changed, 76 insertions(+), 6 deletions(-) create mode 100644 internal/command/testdata/test/mocking-error/tests/plan_mocked_provider.tftest.hcl create mode 100644 internal/command/testdata/test/mocking/tests/plan_mocked_provider.tftest.hcl diff --git a/internal/command/test_test.go b/internal/command/test_test.go index 94af10732550..a640329fa3ef 100644 --- a/internal/command/test_test.go +++ b/internal/command/test_test.go @@ -219,7 +219,7 @@ func TestTest_Runs(t *testing.T) { code: 0, }, "mocking": { - expectedOut: []string{"8 passed, 0 failed."}, + expectedOut: []string{"9 passed, 0 failed."}, code: 0, }, "mocking-invalid": { @@ -232,7 +232,10 @@ func TestTest_Runs(t *testing.T) { "mocking-error": { expectedErr: []string{ "Unknown condition value", + "plan_mocked_overridden.tftest.hcl", "test_resource.primary[0].id", + "plan_mocked_provider.tftest.hcl", + "test_resource.secondary[0].id", }, code: 1, }, diff --git a/internal/command/testdata/test/mocking-error/tests/plan_mocked_provider.tftest.hcl b/internal/command/testdata/test/mocking-error/tests/plan_mocked_provider.tftest.hcl new file mode 100644 index 000000000000..70754a888b3d --- /dev/null +++ b/internal/command/testdata/test/mocking-error/tests/plan_mocked_provider.tftest.hcl @@ -0,0 +1,25 @@ +mock_provider "test" { + alias = "secondary" + + mock_resource "test_resource" { + defaults = { + id = "ffff" + } + } +} + + +variables { + instances = 2 + child_instances = 1 +} + +run "test" { + command = plan + + assert { + condition = test_resource.secondary[0].id == "ffff" + error_message = "plan should use the mocked provider value when override_computed is true" + } + +} diff --git a/internal/command/testdata/test/mocking/tests/plan_mocked_provider.tftest.hcl b/internal/command/testdata/test/mocking/tests/plan_mocked_provider.tftest.hcl new file mode 100644 index 000000000000..bae107b89175 --- /dev/null +++ b/internal/command/testdata/test/mocking/tests/plan_mocked_provider.tftest.hcl @@ -0,0 +1,26 @@ +mock_provider "test" { + alias = "secondary" + override_computed = true + + mock_resource "test_resource" { + defaults = { + id = "ffff" + } + } +} + + +variables { + instances = 2 + child_instances = 1 +} + +run "test" { + command = plan + + assert { + condition = test_resource.secondary[0].id == "ffff" + error_message = "plan should use the mocked provider value when override_computed is true" + } + +} diff --git a/internal/configs/mock_provider.go b/internal/configs/mock_provider.go index aacc62b516e6..6760135d0e75 100644 --- a/internal/configs/mock_provider.go +++ b/internal/configs/mock_provider.go @@ -94,6 +94,14 @@ type MockData struct { MockResources map[string]*MockResource MockDataSources map[string]*MockResource Overrides addrs.Map[addrs.Targetable, *Override] + + useForPlan *bool // If true, the mock data can be used for planning. +} + +// UseForPlan returns true if the provider-level setting for overrideComputed +// is true, meaning that computed values can be overridden with the mocked values. +func (data *MockData) UseForPlan() bool { + return data.useForPlan != nil && *data.useForPlan } // Merge will merge the target MockData object into the current MockData. @@ -212,7 +220,7 @@ type Override struct { } // UseOverridesForPlan returns true if the computed values in the target -// resource should be overridden with the values specified in the override block. +// resource can be overridden with the values specified in the override block. func (o *Override) UseOverridesForPlan() bool { return o.planComputedOverride != nil && *o.planComputedOverride } @@ -231,6 +239,7 @@ func decodeMockDataBody(body hcl.Body, source OverrideSource) (*MockData, hcl.Di MockResources: make(map[string]*MockResource), MockDataSources: make(map[string]*MockResource), Overrides: addrs.MakeMap[addrs.Targetable, *Override](), + useForPlan: providerOverrideComputed, } for _, block := range content.Blocks { diff --git a/internal/providers/mock.go b/internal/providers/mock.go index c23dacf09442..f65882d40fa3 100644 --- a/internal/providers/mock.go +++ b/internal/providers/mock.go @@ -182,11 +182,18 @@ func (m *Mock) PlanResourceChange(request PlanResourceChangeRequest) PlanResourc panic(fmt.Errorf("failed to retrieve schema for resource %s", request.TypeName)) } - // If the provider was overriden via override_* blocks, the mock provider is not called at all, - // so we can continue to make computed values as unknown here (until mock defaults support plan command). - value, diags := mocking.PlanComputedValuesForResource(request.ProposedNewState, &mocking.MockedData{ + replacement := &mocking.MockedData{ + Value: cty.NilVal, // If we have no data then we use cty.NilVal. ComputedAsUnknown: true, - }, resource.Block) + } + // if we are allowed to use the mock defaults for plan, we can populate the computed fields with the mock defaults. + if mockedResource, exists := m.Data.MockResources[request.TypeName]; exists && m.Data.UseForPlan() { + replacement.Value = mockedResource.Defaults + replacement.Range = mockedResource.DefaultsRange + replacement.ComputedAsUnknown = false + } + + value, diags := mocking.PlanComputedValuesForResource(request.ProposedNewState, replacement, resource.Block) response.Diagnostics = response.Diagnostics.Append(diags) response.PlannedState = value response.PlannedPrivate = []byte("create") From 9c1113821fea1d11bffd30404c74510a86a7c2db Mon Sep 17 00:00:00 2001 From: Samsondeen Dare Date: Thu, 19 Dec 2024 12:42:09 +0100 Subject: [PATCH 15/18] minor improvements (3) --- internal/configs/mock_provider.go | 14 +++++++------- .../terraform/node_resource_abstract_instance.go | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/internal/configs/mock_provider.go b/internal/configs/mock_provider.go index 6760135d0e75..b54d9dc974d7 100644 --- a/internal/configs/mock_provider.go +++ b/internal/configs/mock_provider.go @@ -208,7 +208,7 @@ type Override struct { // of a real plan. This attribute indicates that the computed values // should be overridden with the values specified in the override block, // even when planning. - planComputedOverride *bool + useForPlan *bool // Source tells us where this Override was defined. Source OverrideSource @@ -219,10 +219,10 @@ type Override struct { ValuesRange hcl.Range } -// UseOverridesForPlan returns true if the computed values in the target +// UseForPlan returns true if the computed values in the target // resource can be overridden with the values specified in the override block. -func (o *Override) UseOverridesForPlan() bool { - return o.planComputedOverride != nil && *o.planComputedOverride +func (o *Override) UseForPlan() bool { + return o.useForPlan != nil && *o.useForPlan } func decodeMockDataBody(body hcl.Body, source OverrideSource) (*MockData, hcl.Diagnostics) { @@ -313,8 +313,8 @@ func decodeMockDataBody(body hcl.Body, source OverrideSource) (*MockData, hcl.Di for _, elem := range data.Overrides.Elements() { // use the provider-level setting if there is none set for this override - if elem.Value.planComputedOverride == nil { - elem.Value.planComputedOverride = providerOverrideComputed + if elem.Value.useForPlan == nil { + elem.Value.useForPlan = providerOverrideComputed } data.Overrides.Put(elem.Key, elem.Value) } @@ -508,7 +508,7 @@ func decodeOverrideBlock(block *hcl.Block, attributeName string, blockName strin // Override computed values during planning if override_computed is true. overrideComputedBool, valueDiags := extractOverrideComputed(content) diags = append(diags, valueDiags...) - override.planComputedOverride = overrideComputedBool + override.useForPlan = overrideComputedBool if !override.Values.Type().IsObjectType() { diff --git a/internal/terraform/node_resource_abstract_instance.go b/internal/terraform/node_resource_abstract_instance.go index d115820cd42c..5985c7786f31 100644 --- a/internal/terraform/node_resource_abstract_instance.go +++ b/internal/terraform/node_resource_abstract_instance.go @@ -907,7 +907,7 @@ func (n *NodeAbstractResourceInstance) plan( override, overrideDiags := mocking.PlanComputedValuesForResource(proposedNewVal, &mocking.MockedData{ Value: n.override.Values, Range: n.override.Range, - ComputedAsUnknown: !n.override.UseOverridesForPlan(), + ComputedAsUnknown: !n.override.UseForPlan(), }, schema) resp = providers.PlanResourceChangeResponse{ PlannedState: override, @@ -1089,7 +1089,7 @@ func (n *NodeAbstractResourceInstance) plan( override, overrideDiags := mocking.PlanComputedValuesForResource(proposedNewVal, &mocking.MockedData{ Value: n.override.Values, Range: n.override.Range, - ComputedAsUnknown: !n.override.UseOverridesForPlan(), + ComputedAsUnknown: !n.override.UseForPlan(), }, schema) resp = providers.PlanResourceChangeResponse{ PlannedState: override, From 71a23ef8733817d171f63b9e15ab7ace4c9acfd5 Mon Sep 17 00:00:00 2001 From: Samsondeen Dare Date: Thu, 19 Dec 2024 16:48:01 +0100 Subject: [PATCH 16/18] fix tests --- internal/command/test_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/command/test_test.go b/internal/command/test_test.go index a640329fa3ef..87327a00fc42 100644 --- a/internal/command/test_test.go +++ b/internal/command/test_test.go @@ -1761,8 +1761,9 @@ Condition expression could not be evaluated at this time. This means you have executed a %s block with %s and one of the values your condition depended on is not known until after the plan has been applied. Either remove this value from your condition, or execute an %s command -from this %s block. -`, "`run`", "`command = plan`", "`apply`", "`run`"), +from this %s block. Alternatively, if there is an override for this value, +you can make it available during the plan phase by setting %s in the %s block. +`, "`run`", "`command = plan`", "`apply`", "`run`", "`override_computed\n= true`", "`override_`"), }, "unknown_value_in_vars": { code: 1, From 2f014ac3c9f488f5a8bf30749810282d5f7d1fbd Mon Sep 17 00:00:00 2001 From: Samsondeen Dare Date: Tue, 7 Jan 2025 12:02:07 +0100 Subject: [PATCH 17/18] address comments, change attr name --- internal/command/test_test.go | 4 +- .../tests/plan_mocked_provider.tftest.hcl | 2 +- ...erride_computed_invalid_boolean.tftest.hcl | 2 +- .../tests/plan_mocked_overridden.tftest.hcl | 4 +- .../tests/plan_mocked_provider.tftest.hcl | 4 +- ...plan_mocked_provider_overridden.tftest.hcl | 10 ++-- internal/configs/mock_provider.go | 52 +++++++++---------- internal/moduletest/eval_context.go | 2 +- internal/moduletest/mocking/values.go | 4 +- 9 files changed, 42 insertions(+), 42 deletions(-) diff --git a/internal/command/test_test.go b/internal/command/test_test.go index 87327a00fc42..a41a39a9dede 100644 --- a/internal/command/test_test.go +++ b/internal/command/test_test.go @@ -225,7 +225,7 @@ func TestTest_Runs(t *testing.T) { "mocking-invalid": { expectedErr: []string{ "Invalid outputs attribute", - "The override_computed attribute must be a boolean.", + "The override_target attribute must be a value of plan or apply.", }, initCode: 1, }, @@ -1763,7 +1763,7 @@ condition depended on is not known until after the plan has been applied. Either remove this value from your condition, or execute an %s command from this %s block. Alternatively, if there is an override for this value, you can make it available during the plan phase by setting %s in the %s block. -`, "`run`", "`command = plan`", "`apply`", "`run`", "`override_computed\n= true`", "`override_`"), +`, "`run`", "`command = plan`", "`apply`", "`run`", "`override_target\n= plan`", "`override_`"), }, "unknown_value_in_vars": { code: 1, diff --git a/internal/command/testdata/test/mocking-error/tests/plan_mocked_provider.tftest.hcl b/internal/command/testdata/test/mocking-error/tests/plan_mocked_provider.tftest.hcl index 70754a888b3d..f1cd355ca98a 100644 --- a/internal/command/testdata/test/mocking-error/tests/plan_mocked_provider.tftest.hcl +++ b/internal/command/testdata/test/mocking-error/tests/plan_mocked_provider.tftest.hcl @@ -19,7 +19,7 @@ run "test" { assert { condition = test_resource.secondary[0].id == "ffff" - error_message = "plan should use the mocked provider value when override_computed is true" + error_message = "plan should use the mocked provider value when override_target is plan" } } diff --git a/internal/command/testdata/test/mocking-invalid/tests/override_computed_invalid_boolean.tftest.hcl b/internal/command/testdata/test/mocking-invalid/tests/override_computed_invalid_boolean.tftest.hcl index 59ce59f3f3e8..d35a7eecfff3 100644 --- a/internal/command/testdata/test/mocking-invalid/tests/override_computed_invalid_boolean.tftest.hcl +++ b/internal/command/testdata/test/mocking-invalid/tests/override_computed_invalid_boolean.tftest.hcl @@ -1,6 +1,6 @@ mock_provider "test" { alias = "primary" - override_computed = foo // This should be a boolean value, therefore this test should fail + override_target = baz // This should either be plan or apply, therefore this test should fail mock_resource "test_resource" { defaults = { diff --git a/internal/command/testdata/test/mocking/tests/plan_mocked_overridden.tftest.hcl b/internal/command/testdata/test/mocking/tests/plan_mocked_overridden.tftest.hcl index a65be82f4a77..2e2372894817 100644 --- a/internal/command/testdata/test/mocking/tests/plan_mocked_overridden.tftest.hcl +++ b/internal/command/testdata/test/mocking/tests/plan_mocked_overridden.tftest.hcl @@ -9,7 +9,7 @@ mock_provider "test" { override_resource { target = test_resource.primary - override_computed = true + override_target = plan values = { id = "bbbb" } @@ -26,7 +26,7 @@ run "test" { assert { condition = test_resource.primary[0].id == "bbbb" - error_message = "plan should override the value when override_computed is true" + error_message = "plan should override the value when override_target is plan" } } diff --git a/internal/command/testdata/test/mocking/tests/plan_mocked_provider.tftest.hcl b/internal/command/testdata/test/mocking/tests/plan_mocked_provider.tftest.hcl index bae107b89175..d293d4248581 100644 --- a/internal/command/testdata/test/mocking/tests/plan_mocked_provider.tftest.hcl +++ b/internal/command/testdata/test/mocking/tests/plan_mocked_provider.tftest.hcl @@ -1,6 +1,6 @@ mock_provider "test" { alias = "secondary" - override_computed = true + override_target = plan mock_resource "test_resource" { defaults = { @@ -20,7 +20,7 @@ run "test" { assert { condition = test_resource.secondary[0].id == "ffff" - error_message = "plan should use the mocked provider value when override_computed is true" + error_message = "plan should use the mocked provider value when override_target is plan" } } diff --git a/internal/command/testdata/test/mocking/tests/plan_mocked_provider_overridden.tftest.hcl b/internal/command/testdata/test/mocking/tests/plan_mocked_provider_overridden.tftest.hcl index 354ce2b7b2cf..847438b47b48 100644 --- a/internal/command/testdata/test/mocking/tests/plan_mocked_provider_overridden.tftest.hcl +++ b/internal/command/testdata/test/mocking/tests/plan_mocked_provider_overridden.tftest.hcl @@ -1,6 +1,6 @@ mock_provider "test" { alias = "primary" - override_computed = true + override_target = plan mock_resource "test_resource" { defaults = { @@ -17,7 +17,7 @@ mock_provider "test" { override_resource { target = test_resource.primary[1] - override_computed = false // this should take precedence over the provider-level override_computed + override_target = apply // this should take precedence over the provider-level override_target values = { id = "bbbb" } @@ -27,7 +27,7 @@ mock_provider "test" { override_resource { target = test_resource.secondary[0] - override_computed = true + override_target = plan values = { id = "ssss" } @@ -44,12 +44,12 @@ run "test" { assert { condition = test_resource.primary[0].id == "bbbb" - error_message = "plan should override the value when override_computed is true" + error_message = "plan should override the value when override_target is plan" } assert { condition = test_resource.secondary[0].id == "ssss" - error_message = "plan should override the value when override_computed is true" + error_message = "plan should override the value when override_target is plan" } } diff --git a/internal/configs/mock_provider.go b/internal/configs/mock_provider.go index b54d9dc974d7..3cc288c52754 100644 --- a/internal/configs/mock_provider.go +++ b/internal/configs/mock_provider.go @@ -7,12 +7,18 @@ import ( "github.com/hashicorp/hcl/v2/gohcl" "github.com/hashicorp/hcl/v2/hclsyntax" "github.com/zclconf/go-cty/cty" - "github.com/zclconf/go-cty/cty/gocty" "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/tfdiags" ) +var ( + // When this attribute is set to plan, the values specified in the override + // block will be used for computed attributes even when planning. It defaults + // to apply, meaning that the values will only be used during apply. + overrideTargetCommand = "override_target" +) + func decodeMockProviderBlock(block *hcl.Block) (*Provider, hcl.Diagnostics) { var diags hcl.Diagnostics @@ -66,23 +72,20 @@ func decodeMockProviderBlock(block *hcl.Block) (*Provider, hcl.Diagnostics) { return provider, diags } -func extractOverrideComputed(content *hcl.BodyContent) (*bool, hcl.Diagnostics) { +func extractOverrideComputed(content *hcl.BodyContent) (*string, hcl.Diagnostics) { var diags hcl.Diagnostics - if attr, exists := content.Attributes[overrideComputed]; exists { - val, valueDiags := attr.Expr.Value(nil) - diags = append(diags, valueDiags...) - var overrideComputedBool bool - err := gocty.FromCtyValue(val, &overrideComputedBool) - if err != nil { + if attr, exists := content.Attributes[overrideTargetCommand]; exists { + overrideComputedStr := hcl.ExprAsKeyword(attr.Expr) + if overrideComputedStr != "plan" && overrideComputedStr != "apply" { diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, - Summary: fmt.Sprintf("Invalid %s value", overrideComputed), - Detail: fmt.Sprintf("The %s attribute must be a boolean.", overrideComputed), + Summary: fmt.Sprintf("Invalid %s value", overrideTargetCommand), + Detail: fmt.Sprintf("The %s attribute must be a value of plan or apply.", overrideTargetCommand), Subject: attr.Range.Ptr(), }) } - return &overrideComputedBool, diags + return &overrideComputedStr, diags } return nil, diags @@ -234,12 +237,12 @@ func decodeMockDataBody(body hcl.Body, source OverrideSource) (*MockData, hcl.Di // provider-level setting for overrideComputed providerOverrideComputed, valueDiags := extractOverrideComputed(content) diags = append(diags, valueDiags...) - + useForPlan := providerOverrideComputed != nil && *providerOverrideComputed == "plan" data := &MockData{ MockResources: make(map[string]*MockResource), MockDataSources: make(map[string]*MockResource), Overrides: addrs.MakeMap[addrs.Targetable, *Override](), - useForPlan: providerOverrideComputed, + useForPlan: &useForPlan, } for _, block := range content.Blocks { @@ -313,8 +316,9 @@ func decodeMockDataBody(body hcl.Body, source OverrideSource) (*MockData, hcl.Di for _, elem := range data.Overrides.Elements() { // use the provider-level setting if there is none set for this override + useForPlan := providerOverrideComputed != nil && *providerOverrideComputed == "plan" if elem.Value.useForPlan == nil { - elem.Value.useForPlan = providerOverrideComputed + elem.Value.useForPlan = &useForPlan } data.Overrides.Put(elem.Key, elem.Value) } @@ -450,20 +454,13 @@ func decodeOverrideDataBlock(block *hcl.Block, source OverrideSource) (*Override return override, diags } -var ( - // When this attribute is set to true, the values specified in the override - // block will be used for computed attributes even when planning. Otherwise, - // the computed values will be set to unknown, just like in a real plan. - overrideComputed = "override_computed" -) - func decodeOverrideBlock(block *hcl.Block, attributeName string, blockName string, source OverrideSource) (*Override, hcl.Diagnostics) { var diags hcl.Diagnostics content, contentDiags := block.Body.Content(&hcl.BodySchema{ Attributes: []hcl.AttributeSchema{ {Name: "target"}, - {Name: overrideComputed}, + {Name: overrideTargetCommand}, {Name: attributeName}, }, }) @@ -505,10 +502,13 @@ func decodeOverrideBlock(block *hcl.Block, attributeName string, blockName strin override.Values = cty.EmptyObjectVal } - // Override computed values during planning if override_computed is true. - overrideComputedBool, valueDiags := extractOverrideComputed(content) + // Override computed values during planning if override_target is plan. + overrideComputedStr, valueDiags := extractOverrideComputed(content) diags = append(diags, valueDiags...) - override.useForPlan = overrideComputedBool + if overrideComputedStr != nil { + useForPlan := *overrideComputedStr == "plan" + override.useForPlan = &useForPlan + } if !override.Values.Type().IsObjectType() { @@ -544,7 +544,7 @@ var mockProviderSchema = &hcl.BodySchema{ var mockDataSchema = &hcl.BodySchema{ Attributes: []hcl.AttributeSchema{ - {Name: overrideComputed}, + {Name: overrideTargetCommand}, }, Blocks: []hcl.BlockHeaderSchema{ {Type: "mock_resource", LabelNames: []string{"type"}}, diff --git a/internal/moduletest/eval_context.go b/internal/moduletest/eval_context.go index 623d71f4fb4a..7a7b8a3fea12 100644 --- a/internal/moduletest/eval_context.go +++ b/internal/moduletest/eval_context.go @@ -141,7 +141,7 @@ func (ec *EvalContext) Evaluate() (Status, cty.Value, tfdiags.Diagnostics) { diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, Summary: "Unknown condition value", - Detail: "Condition expression could not be evaluated at this time. This means you have executed a `run` block with `command = plan` and one of the values your condition depended on is not known until after the plan has been applied. Either remove this value from your condition, or execute an `apply` command from this `run` block. Alternatively, if there is an override for this value, you can make it available during the plan phase by setting `override_computed = true` in the `override_` block.", + Detail: "Condition expression could not be evaluated at this time. This means you have executed a `run` block with `command = plan` and one of the values your condition depended on is not known until after the plan has been applied. Either remove this value from your condition, or execute an `apply` command from this `run` block. Alternatively, if there is an override for this value, you can make it available during the plan phase by setting `override_target = plan` in the `override_` block.", Subject: rule.Condition.Range().Ptr(), Expression: rule.Condition, EvalContext: hclCtx, diff --git a/internal/moduletest/mocking/values.go b/internal/moduletest/mocking/values.go index 5b09fe9cbc9a..f06e2e34fb87 100644 --- a/internal/moduletest/mocking/values.go +++ b/internal/moduletest/mocking/values.go @@ -192,11 +192,11 @@ type MockedData struct { } // NewMockedData creates a new MockedData struct with the given value and range. -func NewMockedData(value cty.Value, computedAsUnknown bool, range_ hcl.Range) MockedData { +func NewMockedData(value cty.Value, computedAsUnknown bool, rng hcl.Range) MockedData { return MockedData{ Value: value, ComputedAsUnknown: computedAsUnknown, - Range: range_, + Range: rng, } } From 0cdeaa999a7248d9a262792b1eadf4639861e08d Mon Sep 17 00:00:00 2001 From: Samsondeen Dare Date: Tue, 7 Jan 2025 12:28:18 +0100 Subject: [PATCH 18/18] rename to override_during --- internal/command/test_test.go | 4 ++-- .../tests/plan_mocked_provider.tftest.hcl | 2 +- ...erride_computed_invalid_boolean.tftest.hcl | 2 +- .../tests/plan_mocked_overridden.tftest.hcl | 4 ++-- .../tests/plan_mocked_provider.tftest.hcl | 4 ++-- ...plan_mocked_provider_overridden.tftest.hcl | 10 +++++----- internal/configs/mock_provider.go | 20 +++++++++---------- internal/moduletest/eval_context.go | 2 +- 8 files changed, 24 insertions(+), 24 deletions(-) diff --git a/internal/command/test_test.go b/internal/command/test_test.go index a41a39a9dede..843d9c44837e 100644 --- a/internal/command/test_test.go +++ b/internal/command/test_test.go @@ -225,7 +225,7 @@ func TestTest_Runs(t *testing.T) { "mocking-invalid": { expectedErr: []string{ "Invalid outputs attribute", - "The override_target attribute must be a value of plan or apply.", + "The override_during attribute must be a value of plan or apply.", }, initCode: 1, }, @@ -1763,7 +1763,7 @@ condition depended on is not known until after the plan has been applied. Either remove this value from your condition, or execute an %s command from this %s block. Alternatively, if there is an override for this value, you can make it available during the plan phase by setting %s in the %s block. -`, "`run`", "`command = plan`", "`apply`", "`run`", "`override_target\n= plan`", "`override_`"), +`, "`run`", "`command = plan`", "`apply`", "`run`", "`override_during =\nplan`", "`override_`"), }, "unknown_value_in_vars": { code: 1, diff --git a/internal/command/testdata/test/mocking-error/tests/plan_mocked_provider.tftest.hcl b/internal/command/testdata/test/mocking-error/tests/plan_mocked_provider.tftest.hcl index f1cd355ca98a..ed387c5be822 100644 --- a/internal/command/testdata/test/mocking-error/tests/plan_mocked_provider.tftest.hcl +++ b/internal/command/testdata/test/mocking-error/tests/plan_mocked_provider.tftest.hcl @@ -19,7 +19,7 @@ run "test" { assert { condition = test_resource.secondary[0].id == "ffff" - error_message = "plan should use the mocked provider value when override_target is plan" + error_message = "plan should use the mocked provider value when override_during is plan" } } diff --git a/internal/command/testdata/test/mocking-invalid/tests/override_computed_invalid_boolean.tftest.hcl b/internal/command/testdata/test/mocking-invalid/tests/override_computed_invalid_boolean.tftest.hcl index d35a7eecfff3..ce346c6d5476 100644 --- a/internal/command/testdata/test/mocking-invalid/tests/override_computed_invalid_boolean.tftest.hcl +++ b/internal/command/testdata/test/mocking-invalid/tests/override_computed_invalid_boolean.tftest.hcl @@ -1,6 +1,6 @@ mock_provider "test" { alias = "primary" - override_target = baz // This should either be plan or apply, therefore this test should fail + override_during = baz // This should either be plan or apply, therefore this test should fail mock_resource "test_resource" { defaults = { diff --git a/internal/command/testdata/test/mocking/tests/plan_mocked_overridden.tftest.hcl b/internal/command/testdata/test/mocking/tests/plan_mocked_overridden.tftest.hcl index 2e2372894817..f5dd295096a0 100644 --- a/internal/command/testdata/test/mocking/tests/plan_mocked_overridden.tftest.hcl +++ b/internal/command/testdata/test/mocking/tests/plan_mocked_overridden.tftest.hcl @@ -9,7 +9,7 @@ mock_provider "test" { override_resource { target = test_resource.primary - override_target = plan + override_during = plan values = { id = "bbbb" } @@ -26,7 +26,7 @@ run "test" { assert { condition = test_resource.primary[0].id == "bbbb" - error_message = "plan should override the value when override_target is plan" + error_message = "plan should override the value when override_during is plan" } } diff --git a/internal/command/testdata/test/mocking/tests/plan_mocked_provider.tftest.hcl b/internal/command/testdata/test/mocking/tests/plan_mocked_provider.tftest.hcl index d293d4248581..c47755d4b577 100644 --- a/internal/command/testdata/test/mocking/tests/plan_mocked_provider.tftest.hcl +++ b/internal/command/testdata/test/mocking/tests/plan_mocked_provider.tftest.hcl @@ -1,6 +1,6 @@ mock_provider "test" { alias = "secondary" - override_target = plan + override_during = plan mock_resource "test_resource" { defaults = { @@ -20,7 +20,7 @@ run "test" { assert { condition = test_resource.secondary[0].id == "ffff" - error_message = "plan should use the mocked provider value when override_target is plan" + error_message = "plan should use the mocked provider value when override_during is plan" } } diff --git a/internal/command/testdata/test/mocking/tests/plan_mocked_provider_overridden.tftest.hcl b/internal/command/testdata/test/mocking/tests/plan_mocked_provider_overridden.tftest.hcl index 847438b47b48..9c553deda2a2 100644 --- a/internal/command/testdata/test/mocking/tests/plan_mocked_provider_overridden.tftest.hcl +++ b/internal/command/testdata/test/mocking/tests/plan_mocked_provider_overridden.tftest.hcl @@ -1,6 +1,6 @@ mock_provider "test" { alias = "primary" - override_target = plan + override_during = plan mock_resource "test_resource" { defaults = { @@ -17,7 +17,7 @@ mock_provider "test" { override_resource { target = test_resource.primary[1] - override_target = apply // this should take precedence over the provider-level override_target + override_during = apply // this should take precedence over the provider-level override_during values = { id = "bbbb" } @@ -27,7 +27,7 @@ mock_provider "test" { override_resource { target = test_resource.secondary[0] - override_target = plan + override_during = plan values = { id = "ssss" } @@ -44,12 +44,12 @@ run "test" { assert { condition = test_resource.primary[0].id == "bbbb" - error_message = "plan should override the value when override_target is plan" + error_message = "plan should override the value when override_during is plan" } assert { condition = test_resource.secondary[0].id == "ssss" - error_message = "plan should override the value when override_target is plan" + error_message = "plan should override the value when override_during is plan" } } diff --git a/internal/configs/mock_provider.go b/internal/configs/mock_provider.go index 3cc288c52754..f6d05bfcdcba 100644 --- a/internal/configs/mock_provider.go +++ b/internal/configs/mock_provider.go @@ -16,7 +16,7 @@ var ( // When this attribute is set to plan, the values specified in the override // block will be used for computed attributes even when planning. It defaults // to apply, meaning that the values will only be used during apply. - overrideTargetCommand = "override_target" + overrideDuringCommand = "override_during" ) func decodeMockProviderBlock(block *hcl.Block) (*Provider, hcl.Diagnostics) { @@ -72,16 +72,16 @@ func decodeMockProviderBlock(block *hcl.Block) (*Provider, hcl.Diagnostics) { return provider, diags } -func extractOverrideComputed(content *hcl.BodyContent) (*string, hcl.Diagnostics) { +func extractOverrideDuring(content *hcl.BodyContent) (*string, hcl.Diagnostics) { var diags hcl.Diagnostics - if attr, exists := content.Attributes[overrideTargetCommand]; exists { + if attr, exists := content.Attributes[overrideDuringCommand]; exists { overrideComputedStr := hcl.ExprAsKeyword(attr.Expr) if overrideComputedStr != "plan" && overrideComputedStr != "apply" { diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, - Summary: fmt.Sprintf("Invalid %s value", overrideTargetCommand), - Detail: fmt.Sprintf("The %s attribute must be a value of plan or apply.", overrideTargetCommand), + Summary: fmt.Sprintf("Invalid %s value", overrideDuringCommand), + Detail: fmt.Sprintf("The %s attribute must be a value of plan or apply.", overrideDuringCommand), Subject: attr.Range.Ptr(), }) } @@ -235,7 +235,7 @@ func decodeMockDataBody(body hcl.Body, source OverrideSource) (*MockData, hcl.Di diags = append(diags, contentDiags...) // provider-level setting for overrideComputed - providerOverrideComputed, valueDiags := extractOverrideComputed(content) + providerOverrideComputed, valueDiags := extractOverrideDuring(content) diags = append(diags, valueDiags...) useForPlan := providerOverrideComputed != nil && *providerOverrideComputed == "plan" data := &MockData{ @@ -460,7 +460,7 @@ func decodeOverrideBlock(block *hcl.Block, attributeName string, blockName strin content, contentDiags := block.Body.Content(&hcl.BodySchema{ Attributes: []hcl.AttributeSchema{ {Name: "target"}, - {Name: overrideTargetCommand}, + {Name: overrideDuringCommand}, {Name: attributeName}, }, }) @@ -502,8 +502,8 @@ func decodeOverrideBlock(block *hcl.Block, attributeName string, blockName strin override.Values = cty.EmptyObjectVal } - // Override computed values during planning if override_target is plan. - overrideComputedStr, valueDiags := extractOverrideComputed(content) + // Override computed values during planning if override_during is plan. + overrideComputedStr, valueDiags := extractOverrideDuring(content) diags = append(diags, valueDiags...) if overrideComputedStr != nil { useForPlan := *overrideComputedStr == "plan" @@ -544,7 +544,7 @@ var mockProviderSchema = &hcl.BodySchema{ var mockDataSchema = &hcl.BodySchema{ Attributes: []hcl.AttributeSchema{ - {Name: overrideTargetCommand}, + {Name: overrideDuringCommand}, }, Blocks: []hcl.BlockHeaderSchema{ {Type: "mock_resource", LabelNames: []string{"type"}}, diff --git a/internal/moduletest/eval_context.go b/internal/moduletest/eval_context.go index 7a7b8a3fea12..ba627ecf5196 100644 --- a/internal/moduletest/eval_context.go +++ b/internal/moduletest/eval_context.go @@ -141,7 +141,7 @@ func (ec *EvalContext) Evaluate() (Status, cty.Value, tfdiags.Diagnostics) { diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, Summary: "Unknown condition value", - Detail: "Condition expression could not be evaluated at this time. This means you have executed a `run` block with `command = plan` and one of the values your condition depended on is not known until after the plan has been applied. Either remove this value from your condition, or execute an `apply` command from this `run` block. Alternatively, if there is an override for this value, you can make it available during the plan phase by setting `override_target = plan` in the `override_` block.", + Detail: "Condition expression could not be evaluated at this time. This means you have executed a `run` block with `command = plan` and one of the values your condition depended on is not known until after the plan has been applied. Either remove this value from your condition, or execute an `apply` command from this `run` block. Alternatively, if there is an override for this value, you can make it available during the plan phase by setting `override_during = plan` in the `override_` block.", Subject: rule.Condition.Range().Ptr(), Expression: rule.Condition, EvalContext: hclCtx,