From 7f76707dd04fe49c8d6ebbbf980d58e417e84a37 Mon Sep 17 00:00:00 2001 From: Andrei Ciobanu Date: Mon, 25 Aug 2025 13:57:11 +0300 Subject: [PATCH] Ephemeral write only attributes (#3171) Signed-off-by: Andrei Ciobanu Signed-off-by: Christian Mesh --- internal/command/e2etest/primary_test.go | 12 +- .../testdata/ephemeral-workflow/mod/main.tf | 10 +- .../jsonformat/computed/renderers/testing.go | 20 + .../computed/renderers/write_only.go | 24 + .../command/jsonformat/differ/attribute.go | 21 +- internal/command/jsonformat/differ/block.go | 6 +- internal/command/jsonformat/differ/differ.go | 8 + .../command/jsonformat/differ/differ_test.go | 535 ++++++++++++++++-- internal/command/jsonformat/differ/object.go | 10 +- internal/command/jsonprovider/attribute.go | 2 + .../command/jsonprovider/attribute_test.go | 4 +- internal/configs/configschema/schema.go | 3 + internal/configs/configschema/write_only.go | 188 ++++++ .../configs/configschema/write_only_test.go | 387 +++++++++++++ internal/plans/objchange/plan_valid.go | 12 +- internal/plans/objchange/plan_valid_test.go | 384 +++++++++++++ internal/plugin/convert/schema_test.go | 36 ++ internal/plugin/grpc_provider.go | 18 +- internal/plugin/grpc_provider_test.go | 255 ++++++++- internal/plugin/validation/write_only.go | 48 ++ internal/plugin6/convert/schema.go | 1 + internal/plugin6/convert/schema_test.go | 83 +++ internal/plugin6/grpc_provider.go | 18 +- internal/plugin6/grpc_provider_test.go | 256 ++++++++- internal/plugin6/validation/write_only.go | 48 ++ internal/provider-simple-v6/provider.go | 29 +- internal/provider-simple/provider.go | 27 +- internal/tofu/context_plan2_test.go | 4 +- .../tofu/node_resource_abstract_instance.go | 29 +- 29 files changed, 2389 insertions(+), 89 deletions(-) create mode 100644 internal/command/jsonformat/computed/renderers/write_only.go create mode 100644 internal/configs/configschema/write_only.go create mode 100644 internal/configs/configschema/write_only_test.go create mode 100644 internal/plugin/validation/write_only.go create mode 100644 internal/plugin6/validation/write_only.go diff --git a/internal/command/e2etest/primary_test.go b/internal/command/e2etest/primary_test.go index 5e115ade3c..a3bb5916b8 100644 --- a/internal/command/e2etest/primary_test.go +++ b/internal/command/e2etest/primary_test.go @@ -274,7 +274,6 @@ func TestEphemeralWorkflowAndOutput(t *testing.T) { if err != nil { t.Fatalf("unexpected plan error: %s\nstderr:\n%s", err, stderr) } - // TODO ephemeral - this "value_wo" should be shown something like (write-only attribute). This will be handled during the work on the write-only attributes. expectedChangesOutput := `OpenTofu used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols: + create @@ -285,18 +284,21 @@ OpenTofu will perform the following actions: # data.simple_resource.test_data2 will be read during apply # (depends on a resource or a module with changes pending) <= data "simple_resource" "test_data2" { - + id = (known after apply) - + value = "test" + + id = (known after apply) + + value = "test" + + value_wo = (write-only attribute) } # simple_resource.test_res will be created + resource "simple_resource" "test_res" { - + value = "test value" + + value = "test value" + + value_wo = (write-only attribute) } # simple_resource.test_res_second_provider will be created + resource "simple_resource" "test_res_second_provider" { - + value = "just a simple resource to ensure that the second provider it's working fine" + + value = "just a simple resource to ensure that the second provider it's working fine" + + value_wo = (write-only attribute) } Plan: 2 to add, 0 to change, 0 to destroy. diff --git a/internal/command/e2etest/testdata/ephemeral-workflow/mod/main.tf b/internal/command/e2etest/testdata/ephemeral-workflow/mod/main.tf index 6618d9682b..cd868c95b3 100644 --- a/internal/command/e2etest/testdata/ephemeral-workflow/mod/main.tf +++ b/internal/command/e2etest/testdata/ephemeral-workflow/mod/main.tf @@ -1,10 +1,12 @@ variable "in" { - type = string + type = string description = "Variable that is marked as ephemeral and doesn't matter what value is given in, ephemeral or not, the value evaluated for this variable will be marked as ephemeral" - ephemeral = true + ephemeral = true } output "out1" { - value = var.in - ephemeral = true // NOTE: because + value = var.in + // NOTE: because this output gets its value from referencing an ephemeral variable, + // it needs to be configured as ephemeral too. + ephemeral = true } diff --git a/internal/command/jsonformat/computed/renderers/testing.go b/internal/command/jsonformat/computed/renderers/testing.go index 73a6ff7a54..aff6ae8685 100644 --- a/internal/command/jsonformat/computed/renderers/testing.go +++ b/internal/command/jsonformat/computed/renderers/testing.go @@ -6,6 +6,8 @@ package renderers import ( + "maps" + "slices" "sort" "testing" @@ -42,6 +44,18 @@ func ValidatePrimitive(before, after interface{}, action plans.Action, replace b } } +func ValidateWriteOnly(action plans.Action, replace bool) ValidateDiffFunction { + return func(t *testing.T, diff computed.Diff) { + validateDiff(t, diff, action, replace) + + _, ok := diff.Renderer.(*writeOnlyRenderer) + if !ok { + t.Errorf("invalid renderer type: %T", diff.Renderer) + return + } + } +} + func ValidateObject(attributes map[string]ValidateDiffFunction, action plans.Action, replace bool) ValidateDiffFunction { return func(t *testing.T, diff computed.Diff) { validateDiff(t, diff, action, replace) @@ -121,6 +135,12 @@ func validateKeys[C, V any](t *testing.T, actual map[string]C, expected map[stri if diff := cmp.Diff(actualAttributes, expectedAttributes); len(diff) > 0 { t.Errorf("actual and expected attributes did not match: %s", diff) } + } else { + gotKeys := slices.Sorted(maps.Keys(actual)) + wantKeys := slices.Sorted(maps.Keys(expected)) + if diff := cmp.Diff(wantKeys, gotKeys); len(diff) > 0 { + t.Errorf("keys not match: %s", diff) + } } } diff --git a/internal/command/jsonformat/computed/renderers/write_only.go b/internal/command/jsonformat/computed/renderers/write_only.go new file mode 100644 index 0000000000..954003d738 --- /dev/null +++ b/internal/command/jsonformat/computed/renderers/write_only.go @@ -0,0 +1,24 @@ +// Copyright (c) The OpenTofu Authors +// SPDX-License-Identifier: MPL-2.0 +// Copyright (c) 2023 HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package renderers + +import ( + "fmt" + + "github.com/opentofu/opentofu/internal/command/jsonformat/computed" +) + +func WriteOnly() computed.DiffRenderer { + return &writeOnlyRenderer{} +} + +type writeOnlyRenderer struct { + NoWarningsRenderer +} + +func (renderer writeOnlyRenderer) RenderHuman(diff computed.Diff, _ int, opts computed.RenderHumanOpts) string { + return fmt.Sprintf("(write-only attribute)%s%s", forcesReplacement(diff.Replace, opts), nullSuffix(diff.Action, opts)) +} diff --git a/internal/command/jsonformat/differ/attribute.go b/internal/command/jsonformat/differ/attribute.go index 7196dd3e0d..ba0d129d59 100644 --- a/internal/command/jsonformat/differ/attribute.go +++ b/internal/command/jsonformat/differ/attribute.go @@ -6,7 +6,9 @@ package differ import ( + "github.com/opentofu/opentofu/internal/command/jsonformat/computed/renderers" "github.com/opentofu/opentofu/internal/command/jsonformat/structured" + "github.com/opentofu/opentofu/internal/plans" "github.com/zclconf/go-cty/cty" ctyjson "github.com/zclconf/go-cty/cty/json" @@ -15,7 +17,20 @@ import ( "github.com/opentofu/opentofu/internal/command/jsonprovider" ) -func ComputeDiffForAttribute(change structured.Change, attribute *jsonprovider.Attribute) computed.Diff { +// ComputeDiffForAttribute generates the diff for the change. +// It handles 3 specific cases: +// - When the attribute for which the change is generated is a nested object, +// it generates the diff for each attribute +// of the nested object. +// - If the attribute is write-only, due to the fact that its changes will always be null, we want +// to return a diff with the same action as the parent's. +// If we use change.CalculateAction(), then the action will always be NoOp because of the +// which will skip from showing this in the diff. +// - If none above, it tries to generate the diff by using the specific generator for the attr type. +func ComputeDiffForAttribute(change structured.Change, attribute *jsonprovider.Attribute, parentAction plans.Action) computed.Diff { + if attribute.WriteOnly { + return computeAttributeDiffAsWriteOnly(change, parentAction) + } if attribute.AttributeNestedType != nil { return computeDiffForNestedAttribute(change, attribute.AttributeNestedType) } @@ -87,3 +102,7 @@ func unmarshalAttribute(attribute *jsonprovider.Attribute) cty.Type { } return ctyType } + +func computeAttributeDiffAsWriteOnly(change structured.Change, parentAction plans.Action) computed.Diff { + return asDiffWithInheritedAction(change, parentAction, renderers.WriteOnly()) +} diff --git a/internal/command/jsonformat/differ/block.go b/internal/command/jsonformat/differ/block.go index 10d9891ff6..4fafcf0f75 100644 --- a/internal/command/jsonformat/differ/block.go +++ b/internal/command/jsonformat/differ/block.go @@ -49,7 +49,11 @@ func ComputeDiffForBlock(change structured.Change, block *jsonprovider.Block) co childValue.BeforeExplicit = false childValue.AfterExplicit = false - childChange := ComputeDiffForAttribute(childValue, attr) + // Because we want to print also the write-only attributes, we need to pass in the parent block + // action instead of the child one. + // This is because the child action will always result in NoOp since for write-only attributes, the + // values returned will be null. + childChange := ComputeDiffForAttribute(childValue, attr, current) if childChange.Action == plans.NoOp && childValue.Before == nil && childValue.After == nil { // Don't record nil values at all in blocks. continue diff --git a/internal/command/jsonformat/differ/differ.go b/internal/command/jsonformat/differ/differ.go index 5c79d69b1a..92b7d72728 100644 --- a/internal/command/jsonformat/differ/differ.go +++ b/internal/command/jsonformat/differ/differ.go @@ -8,6 +8,7 @@ package differ import ( "github.com/opentofu/opentofu/internal/command/jsonformat/computed" "github.com/opentofu/opentofu/internal/command/jsonformat/structured" + "github.com/opentofu/opentofu/internal/plans" ) // asDiff is a helper function to abstract away some simple and common @@ -15,3 +16,10 @@ import ( func asDiff(change structured.Change, renderer computed.DiffRenderer) computed.Diff { return computed.NewDiff(renderer, change.CalculateAction(), change.ReplacePaths.Matches()) } + +// asDiffWithInheritedAction is a specific implementation of asDiff that gets also a parentAction plans.Action. +// This is used when the given change is known to always generate a NoOp diff, but it still should be shown +// in the printed diff. +func asDiffWithInheritedAction(change structured.Change, parentAction plans.Action, renderer computed.DiffRenderer) computed.Diff { + return computed.NewDiff(renderer, parentAction, change.ReplacePaths.Matches()) +} diff --git a/internal/command/jsonformat/differ/differ_test.go b/internal/command/jsonformat/differ/differ_test.go index 7a563496ff..2d1f8fceb7 100644 --- a/internal/command/jsonformat/differ/differ_test.go +++ b/internal/command/jsonformat/differ/differ_test.go @@ -101,6 +101,71 @@ func TestValue_SimpleBlocks(t *testing.T) { "normal_attribute": renderers.ValidatePrimitive(nil, "some value", plans.Create, false), }, nil, nil, nil, nil, plans.Create, false), }, + + "delete_with_write_only_value": { + input: structured.Change{ + Before: map[string]any{}, + After: nil, + BeforeSensitive: false, + AfterSensitive: false, + }, + block: &jsonprovider.Block{ + Attributes: map[string]*jsonprovider.Attribute{ + "write_only_attribute": { + AttributeType: unmarshalType(t, cty.String), + WriteOnly: true, + }, + }, + BlockTypes: map[string]*jsonprovider.BlockType{ + "nested_with_write_only": { + NestingMode: "single", + Block: &jsonprovider.Block{ + Attributes: map[string]*jsonprovider.Attribute{ + "inner_write_only": { + AttributeType: unmarshalType(t, cty.String), + WriteOnly: true, + }, + }, + }, + }, + }, + }, + validate: renderers.ValidateBlock(map[string]renderers.ValidateDiffFunction{ + "write_only_attribute": renderers.ValidateWriteOnly(plans.Delete, false), + }, nil, nil, nil, nil, plans.Delete, false), + }, + "create_with_write_only_value": { + input: structured.Change{ + Before: nil, + After: map[string]any{}, + BeforeSensitive: false, + AfterSensitive: false, + }, + block: &jsonprovider.Block{ + Attributes: map[string]*jsonprovider.Attribute{ + "write_only_attribute": { + AttributeType: unmarshalType(t, cty.String), + WriteOnly: true, + }, + }, + BlockTypes: map[string]*jsonprovider.BlockType{ + "nested_with_write_only": { + NestingMode: "single", + Block: &jsonprovider.Block{ + Attributes: map[string]*jsonprovider.Attribute{ + "inner_write_only": { + AttributeType: unmarshalType(t, cty.String), + WriteOnly: true, + }, + }, + }, + }, + }, + }, + validate: renderers.ValidateBlock(map[string]renderers.ValidateDiffFunction{ + "write_only_attribute": renderers.ValidateWriteOnly(plans.Create, false), + }, nil, nil, nil, nil, plans.Create, false), + }, } for name, tc := range tcs { // Set some default values @@ -746,17 +811,17 @@ func TestValue_ObjectAttributes(t *testing.T) { } if tc.validateObject != nil { - tc.validateObject(t, ComputeDiffForAttribute(tc.input, attribute)) + tc.validateObject(t, ComputeDiffForAttribute(tc.input, attribute, tc.input.CalculateAction())) return } if tc.validateSingleDiff != nil { - tc.validateSingleDiff(t, ComputeDiffForAttribute(tc.input, attribute)) + tc.validateSingleDiff(t, ComputeDiffForAttribute(tc.input, attribute, tc.input.CalculateAction())) return } validate := renderers.ValidateObject(tc.validateDiffs, tc.validateAction, tc.validateReplace) - validate(t, ComputeDiffForAttribute(tc.input, attribute)) + validate(t, ComputeDiffForAttribute(tc.input, attribute, tc.input.CalculateAction())) }) t.Run("map", func(t *testing.T) { @@ -770,7 +835,7 @@ func TestValue_ObjectAttributes(t *testing.T) { validate := renderers.ValidateMap(map[string]renderers.ValidateDiffFunction{ "element": tc.validateObject, }, collectionDefaultAction, false) - validate(t, ComputeDiffForAttribute(input, attribute)) + validate(t, ComputeDiffForAttribute(input, attribute, input.CalculateAction())) return } @@ -778,14 +843,14 @@ func TestValue_ObjectAttributes(t *testing.T) { validate := renderers.ValidateMap(map[string]renderers.ValidateDiffFunction{ "element": tc.validateSingleDiff, }, collectionDefaultAction, false) - validate(t, ComputeDiffForAttribute(input, attribute)) + validate(t, ComputeDiffForAttribute(input, attribute, input.CalculateAction())) return } validate := renderers.ValidateMap(map[string]renderers.ValidateDiffFunction{ "element": renderers.ValidateObject(tc.validateDiffs, tc.validateAction, tc.validateReplace), }, collectionDefaultAction, false) - validate(t, ComputeDiffForAttribute(input, attribute)) + validate(t, ComputeDiffForAttribute(input, attribute, input.CalculateAction())) }) t.Run("list", func(t *testing.T) { @@ -796,7 +861,7 @@ func TestValue_ObjectAttributes(t *testing.T) { input := wrapChangeInSlice(tc.input) if tc.validateList != nil { - tc.validateList(t, ComputeDiffForAttribute(input, attribute)) + tc.validateList(t, ComputeDiffForAttribute(input, attribute, input.CalculateAction())) return } @@ -804,7 +869,7 @@ func TestValue_ObjectAttributes(t *testing.T) { validate := renderers.ValidateList([]renderers.ValidateDiffFunction{ tc.validateObject, }, collectionDefaultAction, false) - validate(t, ComputeDiffForAttribute(input, attribute)) + validate(t, ComputeDiffForAttribute(input, attribute, input.CalculateAction())) return } @@ -812,14 +877,14 @@ func TestValue_ObjectAttributes(t *testing.T) { validate := renderers.ValidateList([]renderers.ValidateDiffFunction{ tc.validateSingleDiff, }, collectionDefaultAction, false) - validate(t, ComputeDiffForAttribute(input, attribute)) + validate(t, ComputeDiffForAttribute(input, attribute, input.CalculateAction())) return } validate := renderers.ValidateList([]renderers.ValidateDiffFunction{ renderers.ValidateObject(tc.validateDiffs, tc.validateAction, tc.validateReplace), }, collectionDefaultAction, false) - validate(t, ComputeDiffForAttribute(input, attribute)) + validate(t, ComputeDiffForAttribute(input, attribute, input.CalculateAction())) }) t.Run("set", func(t *testing.T) { @@ -836,7 +901,7 @@ func TestValue_ObjectAttributes(t *testing.T) { ret = append(ret, tc.validateSetDiffs.After.Validate(renderers.ValidateObject)) return ret }(), collectionDefaultAction, false) - validate(t, ComputeDiffForAttribute(input, attribute)) + validate(t, ComputeDiffForAttribute(input, attribute, input.CalculateAction())) return } @@ -844,7 +909,7 @@ func TestValue_ObjectAttributes(t *testing.T) { validate := renderers.ValidateSet([]renderers.ValidateDiffFunction{ tc.validateObject, }, collectionDefaultAction, false) - validate(t, ComputeDiffForAttribute(input, attribute)) + validate(t, ComputeDiffForAttribute(input, attribute, input.CalculateAction())) return } @@ -852,14 +917,14 @@ func TestValue_ObjectAttributes(t *testing.T) { validate := renderers.ValidateSet([]renderers.ValidateDiffFunction{ tc.validateSingleDiff, }, collectionDefaultAction, false) - validate(t, ComputeDiffForAttribute(input, attribute)) + validate(t, ComputeDiffForAttribute(input, attribute, input.CalculateAction())) return } validate := renderers.ValidateSet([]renderers.ValidateDiffFunction{ renderers.ValidateObject(tc.validateDiffs, tc.validateAction, tc.validateReplace), }, collectionDefaultAction, false) - validate(t, ComputeDiffForAttribute(input, attribute)) + validate(t, ComputeDiffForAttribute(input, attribute, input.CalculateAction())) }) }) @@ -881,17 +946,17 @@ func TestValue_ObjectAttributes(t *testing.T) { } if tc.validateNestedObject != nil { - tc.validateNestedObject(t, ComputeDiffForAttribute(tc.input, attribute)) + tc.validateNestedObject(t, ComputeDiffForAttribute(tc.input, attribute, tc.input.CalculateAction())) return } if tc.validateSingleDiff != nil { - tc.validateSingleDiff(t, ComputeDiffForAttribute(tc.input, attribute)) + tc.validateSingleDiff(t, ComputeDiffForAttribute(tc.input, attribute, tc.input.CalculateAction())) return } validate := renderers.ValidateNestedObject(tc.validateDiffs, tc.validateAction, tc.validateReplace) - validate(t, ComputeDiffForAttribute(tc.input, attribute)) + validate(t, ComputeDiffForAttribute(tc.input, attribute, tc.input.CalculateAction())) }) t.Run("map", func(t *testing.T) { @@ -916,7 +981,7 @@ func TestValue_ObjectAttributes(t *testing.T) { validate := renderers.ValidateMap(map[string]renderers.ValidateDiffFunction{ "element": tc.validateNestedObject, }, collectionDefaultAction, false) - validate(t, ComputeDiffForAttribute(input, attribute)) + validate(t, ComputeDiffForAttribute(input, attribute, input.CalculateAction())) return } @@ -924,14 +989,14 @@ func TestValue_ObjectAttributes(t *testing.T) { validate := renderers.ValidateMap(map[string]renderers.ValidateDiffFunction{ "element": tc.validateSingleDiff, }, collectionDefaultAction, false) - validate(t, ComputeDiffForAttribute(input, attribute)) + validate(t, ComputeDiffForAttribute(input, attribute, input.CalculateAction())) return } validate := renderers.ValidateMap(map[string]renderers.ValidateDiffFunction{ "element": renderers.ValidateNestedObject(tc.validateDiffs, tc.validateAction, tc.validateReplace), }, collectionDefaultAction, false) - validate(t, ComputeDiffForAttribute(input, attribute)) + validate(t, ComputeDiffForAttribute(input, attribute, input.CalculateAction())) }) t.Run("list", func(t *testing.T) { @@ -956,7 +1021,7 @@ func TestValue_ObjectAttributes(t *testing.T) { validate := renderers.ValidateNestedList([]renderers.ValidateDiffFunction{ tc.validateNestedObject, }, collectionDefaultAction, false) - validate(t, ComputeDiffForAttribute(input, attribute)) + validate(t, ComputeDiffForAttribute(input, attribute, input.CalculateAction())) return } @@ -964,14 +1029,14 @@ func TestValue_ObjectAttributes(t *testing.T) { validate := renderers.ValidateNestedList([]renderers.ValidateDiffFunction{ tc.validateSingleDiff, }, collectionDefaultAction, false) - validate(t, ComputeDiffForAttribute(input, attribute)) + validate(t, ComputeDiffForAttribute(input, attribute, input.CalculateAction())) return } validate := renderers.ValidateNestedList([]renderers.ValidateDiffFunction{ renderers.ValidateNestedObject(tc.validateDiffs, tc.validateAction, tc.validateReplace), }, collectionDefaultAction, false) - validate(t, ComputeDiffForAttribute(input, attribute)) + validate(t, ComputeDiffForAttribute(input, attribute, input.CalculateAction())) }) t.Run("set", func(t *testing.T) { @@ -999,7 +1064,7 @@ func TestValue_ObjectAttributes(t *testing.T) { ret = append(ret, tc.validateSetDiffs.After.Validate(renderers.ValidateNestedObject)) return ret }(), collectionDefaultAction, false) - validate(t, ComputeDiffForAttribute(input, attribute)) + validate(t, ComputeDiffForAttribute(input, attribute, input.CalculateAction())) return } @@ -1007,7 +1072,7 @@ func TestValue_ObjectAttributes(t *testing.T) { validate := renderers.ValidateSet([]renderers.ValidateDiffFunction{ tc.validateNestedObject, }, collectionDefaultAction, false) - validate(t, ComputeDiffForAttribute(input, attribute)) + validate(t, ComputeDiffForAttribute(input, attribute, input.CalculateAction())) return } @@ -1015,14 +1080,14 @@ func TestValue_ObjectAttributes(t *testing.T) { validate := renderers.ValidateSet([]renderers.ValidateDiffFunction{ tc.validateSingleDiff, }, collectionDefaultAction, false) - validate(t, ComputeDiffForAttribute(input, attribute)) + validate(t, ComputeDiffForAttribute(input, attribute, input.CalculateAction())) return } validate := renderers.ValidateSet([]renderers.ValidateDiffFunction{ renderers.ValidateNestedObject(tc.validateDiffs, tc.validateAction, tc.validateReplace), }, collectionDefaultAction, false) - validate(t, ComputeDiffForAttribute(input, attribute)) + validate(t, ComputeDiffForAttribute(input, attribute, input.CalculateAction())) }) }) } @@ -1840,9 +1905,9 @@ func TestValue_PrimitiveAttributes(t *testing.T) { t.Run(name, func(t *testing.T) { t.Run("direct", func(t *testing.T) { - tc.validateDiff(t, ComputeDiffForAttribute(tc.input, &jsonprovider.Attribute{ - AttributeType: unmarshalType(t, tc.attribute), - })) + attr := &jsonprovider.Attribute{AttributeType: unmarshalType(t, tc.attribute)} + diff := ComputeDiffForAttribute(tc.input, attr, tc.input.CalculateAction()) + tc.validateDiff(t, diff) }) t.Run("map", func(t *testing.T) { @@ -1854,7 +1919,7 @@ func TestValue_PrimitiveAttributes(t *testing.T) { validate := renderers.ValidateMap(map[string]renderers.ValidateDiffFunction{ "element": tc.validateDiff, }, defaultCollectionsAction, false) - validate(t, ComputeDiffForAttribute(input, attribute)) + validate(t, ComputeDiffForAttribute(input, attribute, input.CalculateAction())) }) t.Run("list", func(t *testing.T) { @@ -1865,14 +1930,14 @@ func TestValue_PrimitiveAttributes(t *testing.T) { if tc.validateSliceDiffs != nil { validate := renderers.ValidateList(tc.validateSliceDiffs, defaultCollectionsAction, false) - validate(t, ComputeDiffForAttribute(input, attribute)) + validate(t, ComputeDiffForAttribute(input, attribute, input.CalculateAction())) return } validate := renderers.ValidateList([]renderers.ValidateDiffFunction{ tc.validateDiff, }, defaultCollectionsAction, false) - validate(t, ComputeDiffForAttribute(input, attribute)) + validate(t, ComputeDiffForAttribute(input, attribute, input.CalculateAction())) }) t.Run("set", func(t *testing.T) { @@ -1883,14 +1948,14 @@ func TestValue_PrimitiveAttributes(t *testing.T) { if tc.validateSliceDiffs != nil { validate := renderers.ValidateSet(tc.validateSliceDiffs, defaultCollectionsAction, false) - validate(t, ComputeDiffForAttribute(input, attribute)) + validate(t, ComputeDiffForAttribute(input, attribute, input.CalculateAction())) return } validate := renderers.ValidateSet([]renderers.ValidateDiffFunction{ tc.validateDiff, }, defaultCollectionsAction, false) - validate(t, ComputeDiffForAttribute(input, attribute)) + validate(t, ComputeDiffForAttribute(input, attribute, input.CalculateAction())) }) }) } @@ -2261,7 +2326,7 @@ func TestValue_CollectionAttributes(t *testing.T) { } t.Run(name, func(t *testing.T) { - tc.validateDiff(t, ComputeDiffForAttribute(tc.input, tc.attribute)) + tc.validateDiff(t, ComputeDiffForAttribute(tc.input, tc.attribute, tc.input.CalculateAction())) }) } } @@ -3005,6 +3070,404 @@ func TestSpecificCases(t *testing.T) { } } +func TestWriteOnly_ComputeDiffForBlock(t *testing.T) { + // NOTE: The write-only change will *always* have the after change null. + // We set specific values in this test for the after change to ensure that, + // based on the attribute's schema, we use the correct renderers for these attributes. + cases := map[string]struct { + block jsonprovider.Block + change structured.Change + validator renderers.ValidateDiffFunction + }{ + // ---> Attributes with basic types + // attributes with basic types + "write_only_primitive_attribute": { + block: jsonprovider.Block{ + Attributes: map[string]*jsonprovider.Attribute{ + "write_only_attribute": { + AttributeType: unmarshalType(t, cty.String), + Optional: true, + WriteOnly: true, + }, + }, + }, + change: structured.Change{ + After: map[string]any{ + "write_only_attribute": "dummy value", // NOTE: since this is WO attr, IRL it's nil + }, + }, + validator: renderers.ValidateBlock(map[string]renderers.ValidateDiffFunction{ + "write_only_attribute": renderers.ValidateWriteOnly(plans.Create, false), + }, nil, nil, nil, nil, plans.Create, false), + }, + "write_only_list": { + block: jsonprovider.Block{ + Attributes: map[string]*jsonprovider.Attribute{ + "write_only_list": { + AttributeType: unmarshalType(t, cty.List(cty.Object(map[string]cty.Type{"val": cty.String}))), + Optional: true, + WriteOnly: true, + }, + }, + }, + change: structured.Change{ + After: map[string]any{ + "write_only_list": []string{"val", "b"}, // NOTE: since this is WO attr, IRL it's nil + }, + }, + validator: renderers.ValidateBlock(map[string]renderers.ValidateDiffFunction{ + "write_only_list": renderers.ValidateWriteOnly(plans.Create, false), + }, nil, nil, nil, nil, plans.Create, false), + }, + "write_only_set": { + block: jsonprovider.Block{ + Attributes: map[string]*jsonprovider.Attribute{ + "write_only_set": { + AttributeType: unmarshalType(t, cty.Set(cty.Object(map[string]cty.Type{"val": cty.String}))), + Optional: true, + WriteOnly: true, + }, + }, + }, + change: structured.Change{ + After: map[string]any{ + "write_only_set": []string{"val", "b"}, // NOTE: since this is WO attr, IRL it's nil + }, + }, + validator: renderers.ValidateBlock(map[string]renderers.ValidateDiffFunction{ + "write_only_set": renderers.ValidateWriteOnly(plans.Create, false), + }, nil, nil, nil, nil, plans.Create, false), + }, + "write_only_map": { + block: jsonprovider.Block{ + Attributes: map[string]*jsonprovider.Attribute{ + "write_only_map": { + AttributeType: unmarshalType(t, cty.Map(cty.String)), + Optional: true, + WriteOnly: true, + }, + }, + }, + change: structured.Change{ + After: map[string]any{ + "write_only_map": map[string]string{"bar": "barv", "baz": "bazv"}, // NOTE: since this is WO attr, IRL it's nil + }, + }, + validator: renderers.ValidateBlock(map[string]renderers.ValidateDiffFunction{ + "write_only_map": renderers.ValidateWriteOnly(plans.Create, false), + }, nil, nil, nil, nil, plans.Create, false), + }, + "write_only_object": { + block: jsonprovider.Block{ + Attributes: map[string]*jsonprovider.Attribute{ + "write_only_obj": { + AttributeType: unmarshalType(t, cty.Object(map[string]cty.Type{"val": cty.String})), + Optional: true, + WriteOnly: true, + }, + }, + }, + change: structured.Change{ + After: map[string]any{ + "write_only_obj": map[string]string{"bar": "barv"}, // NOTE: since this is WO attr, IRL it's nil + }, + }, + validator: renderers.ValidateBlock(map[string]renderers.ValidateDiffFunction{ + "write_only_obj": renderers.ValidateWriteOnly(plans.Create, false), + }, nil, nil, nil, nil, plans.Create, false), + }, + // ---> Attributes with nested types + "attribute_single_nested_contains_write_only_attribute": { + block: jsonprovider.Block{ + Attributes: map[string]*jsonprovider.Attribute{ + "write_only_obj": { + AttributeNestedType: &jsonprovider.NestedType{ + Attributes: map[string]*jsonprovider.Attribute{ + "val": { + AttributeType: unmarshalType(t, cty.Object(map[string]cty.Type{"val": cty.String})), + Optional: true, + WriteOnly: true, + }, + }, + NestingMode: "single", + }, + }, + }, + }, + change: structured.Change{ + After: map[string]any{ + "write_only_obj": map[string]any{"val": "foo"}, // NOTE: since this is WO attr, IRL it's nil + }, + }, + validator: renderers.ValidateBlock( + map[string]renderers.ValidateDiffFunction{ + "write_only_obj": renderers.ValidateNestedObject( + map[string]renderers.ValidateDiffFunction{ + "val": renderers.ValidateWriteOnly(plans.Create, false), + }, + plans.Create, false), + }, + nil, nil, nil, nil, plans.Create, false), + }, + "attribute_map_nested_contains_write_only_attribute": { + block: jsonprovider.Block{ + Attributes: map[string]*jsonprovider.Attribute{ + "write_only_map": { + AttributeNestedType: &jsonprovider.NestedType{ + Attributes: map[string]*jsonprovider.Attribute{ + "val": { + AttributeType: unmarshalType(t, cty.Object(map[string]cty.Type{"val": cty.String})), + Optional: true, + WriteOnly: true, + }, + }, + NestingMode: "map", + }, + }, + }, + }, + change: structured.Change{ + After: map[string]any{ + "write_only_map": map[string]any{"val": "val value"}, // NOTE: since this is WO attr, IRL it's nil + }, + }, + validator: renderers.ValidateBlock( + map[string]renderers.ValidateDiffFunction{ + "write_only_map": renderers.ValidateMap( + map[string]renderers.ValidateDiffFunction{ + "val": renderers.ValidateNestedObject( + map[string]renderers.ValidateDiffFunction{ + "val": renderers.ValidateWriteOnly(plans.Create, false), + }, + plans.Create, false), + }, + plans.Create, false), + }, + nil, nil, nil, nil, plans.Create, false), + }, + "attribute_list_nested_contains_write_only_attribute": { + block: jsonprovider.Block{ + Attributes: map[string]*jsonprovider.Attribute{ + "write_only_list": { + AttributeNestedType: &jsonprovider.NestedType{ + Attributes: map[string]*jsonprovider.Attribute{ + "val": { + AttributeType: unmarshalType(t, cty.Object(map[string]cty.Type{"val": cty.String})), + Optional: true, + WriteOnly: true, + }, + }, + NestingMode: "list", + }, + }, + }, + }, + change: structured.Change{ + After: map[string]any{ + "write_only_list": []any{map[string]any{"val": "foo value"}}, // NOTE: since this is WO attr, IRL it's nil + }, + }, + validator: renderers.ValidateBlock( + map[string]renderers.ValidateDiffFunction{ + "write_only_list": renderers.ValidateNestedList( + []renderers.ValidateDiffFunction{ + renderers.ValidateNestedObject( + map[string]renderers.ValidateDiffFunction{ + "val": renderers.ValidateWriteOnly(plans.Create, false), + }, + plans.Create, false), + }, + plans.Create, false), + }, + nil, nil, nil, nil, plans.Create, false), + }, + "attribute_set_nested_contains_write_only_attribute": { + block: jsonprovider.Block{ + Attributes: map[string]*jsonprovider.Attribute{ + "write_only_set": { + AttributeNestedType: &jsonprovider.NestedType{ + Attributes: map[string]*jsonprovider.Attribute{ + "val": { + AttributeType: unmarshalType(t, cty.Object(map[string]cty.Type{"val": cty.String})), + Optional: true, + WriteOnly: true, + }, + }, + NestingMode: "set", + }, + }, + }, + }, + change: structured.Change{ + After: map[string]any{ + "write_only_set": []any{map[string]any{"val": "foo value"}}, // NOTE: since this is WO attr, IRL it's nil + }, + }, + validator: renderers.ValidateBlock( + map[string]renderers.ValidateDiffFunction{ + "write_only_set": renderers.ValidateSet( + []renderers.ValidateDiffFunction{ + renderers.ValidateNestedObject( + map[string]renderers.ValidateDiffFunction{ + "val": renderers.ValidateWriteOnly(plans.Create, false), + }, + plans.Create, false), + }, + plans.Create, false), + }, + nil, nil, nil, nil, plans.Create, false), + }, + // ---> Nested blocks + "single_nested_block": { + block: jsonprovider.Block{ + BlockTypes: map[string]*jsonprovider.BlockType{ + "nested_block": { + NestingMode: "single", + Block: &jsonprovider.Block{ + Attributes: map[string]*jsonprovider.Attribute{ + "write_only_attr": { + AttributeType: unmarshalType(t, cty.String), + WriteOnly: true, + }, + }, + }, + }, + }, + }, + change: structured.Change{ + After: map[string]any{ + "nested_block": map[string]any{"write_only_attr": "foo"}, // NOTE: since this is WO attr, IRL it's nil + }, + }, + validator: renderers.ValidateBlock( + nil, + map[string]renderers.ValidateDiffFunction{ + "nested_block": renderers.ValidateBlock( + map[string]renderers.ValidateDiffFunction{ + "write_only_attr": renderers.ValidateWriteOnly(plans.Create, false), + }, + nil, nil, nil, nil, plans.Create, false), + }, + nil, nil, nil, plans.Create, false), + }, + "map_nested_block": { + block: jsonprovider.Block{ + BlockTypes: map[string]*jsonprovider.BlockType{ + "nested_block": { + NestingMode: "map", + Block: &jsonprovider.Block{ + Attributes: map[string]*jsonprovider.Attribute{ + "write_only_attr": { + AttributeType: unmarshalType(t, cty.String), + WriteOnly: true, + }, + }, + }, + }, + }, + }, + change: structured.Change{ + After: map[string]any{ + "nested_block": map[string]any{"write_only_attr": "foo"}, // NOTE: since this is WO attr, IRL it's nil + }, + }, + validator: renderers.ValidateBlock( + nil, nil, nil, + map[string]map[string]renderers.ValidateDiffFunction{ + "nested_block": { + "write_only_attr": renderers.ValidateBlock( + map[string]renderers.ValidateDiffFunction{ + "write_only_attr": renderers.ValidateWriteOnly(plans.Create, false), + }, + nil, nil, nil, nil, plans.Create, false), + }, + }, + nil, plans.Create, false), + }, + "list_nested_block": { + block: jsonprovider.Block{ + BlockTypes: map[string]*jsonprovider.BlockType{ + "nested_block": { + NestingMode: "list", + Block: &jsonprovider.Block{ + Attributes: map[string]*jsonprovider.Attribute{ + "write_only_attr": { + AttributeType: unmarshalType(t, cty.String), + WriteOnly: true, + }, + }, + }, + }, + }, + }, + change: structured.Change{ + After: map[string]any{ + "nested_block": []any{"value"}, // NOTE: since this is WO attr, IRL it's nil + }, + }, + validator: renderers.ValidateBlock( + nil, nil, + map[string][]renderers.ValidateDiffFunction{ + "nested_block": { + renderers.ValidateBlock( + map[string]renderers.ValidateDiffFunction{ + "write_only_attr": renderers.ValidateWriteOnly(plans.Create, false), + }, + nil, nil, nil, nil, plans.Create, false), + }, + }, + nil, nil, plans.Create, false), + }, + "set_nested_block": { + block: jsonprovider.Block{ + BlockTypes: map[string]*jsonprovider.BlockType{ + "nested_block": { + NestingMode: "set", + Block: &jsonprovider.Block{ + Attributes: map[string]*jsonprovider.Attribute{ + "write_only_attr": { + AttributeType: unmarshalType(t, cty.String), + WriteOnly: true, + }, + }, + }, + }, + }, + }, + change: structured.Change{ + After: map[string]any{ + "nested_block": []any{"value"}, // NOTE: since this is WO attr, IRL it's nil + }, + }, + validator: renderers.ValidateBlock( + nil, nil, nil, nil, + map[string][]renderers.ValidateDiffFunction{ + "nested_block": { + renderers.ValidateBlock( + map[string]renderers.ValidateDiffFunction{ + "write_only_attr": renderers.ValidateWriteOnly(plans.Create, false), + }, + nil, nil, nil, nil, plans.Create, false), + }, + }, plans.Create, false), + }, + } + + for name, tt := range cases { + t.Run(name, func(t *testing.T) { + if tt.change.ReplacePaths == nil { + tt.change.ReplacePaths = &attribute_path.PathMatcher{} + } + if tt.change.RelevantAttributes == nil { + tt.change.RelevantAttributes = attribute_path.AlwaysMatcher() + + } + diff := ComputeDiffForBlock(tt.change, &tt.block) + tt.validator(t, diff) + }) + } +} + // unmarshalType converts a cty.Type into a json.RawMessage understood by the // schema. It also lets the testing framework handle any errors to keep the API // clean. diff --git a/internal/command/jsonformat/differ/object.go b/internal/command/jsonformat/differ/object.go index dfb6ccc025..ba6b2dfd42 100644 --- a/internal/command/jsonformat/differ/object.go +++ b/internal/command/jsonformat/differ/object.go @@ -17,15 +17,15 @@ import ( ) func computeAttributeDiffAsObject(change structured.Change, attributes map[string]cty.Type) computed.Diff { - attributeDiffs, action := processObject(change, attributes, func(value structured.Change, ctype cty.Type) computed.Diff { + attributeDiffs, action := processObject(change, attributes, func(value structured.Change, ctype cty.Type, _ plans.Action) computed.Diff { return ComputeDiffForType(value, ctype) }) return computed.NewDiff(renderers.Object(attributeDiffs), action, change.ReplacePaths.Matches()) } func computeAttributeDiffAsNestedObject(change structured.Change, attributes map[string]*jsonprovider.Attribute) computed.Diff { - attributeDiffs, action := processObject(change, attributes, func(value structured.Change, attribute *jsonprovider.Attribute) computed.Diff { - return ComputeDiffForAttribute(value, attribute) + attributeDiffs, action := processObject(change, attributes, func(value structured.Change, attribute *jsonprovider.Attribute, parentAction plans.Action) computed.Diff { + return ComputeDiffForAttribute(value, attribute, parentAction) }) return computed.NewDiff(renderers.NestedObject(attributeDiffs), action, change.ReplacePaths.Matches()) } @@ -41,7 +41,7 @@ func computeAttributeDiffAsNestedObject(change structured.Change, attributes map // Also, as it generic we cannot make this function a method on Change as you // can't create generic methods on structs. Instead, we make this a generic // function that receives the value as an argument. -func processObject[T any](v structured.Change, attributes map[string]T, computeDiff func(structured.Change, T) computed.Diff) (map[string]computed.Diff, plans.Action) { +func processObject[T any](v structured.Change, attributes map[string]T, computeDiff func(structured.Change, T, plans.Action) computed.Diff) (map[string]computed.Diff, plans.Action) { attributeDiffs := make(map[string]computed.Diff) mapValue := v.AsMap() @@ -58,7 +58,7 @@ func processObject[T any](v structured.Change, attributes map[string]T, computeD attributeValue.BeforeExplicit = false attributeValue.AfterExplicit = false - attributeDiff := computeDiff(attributeValue, attribute) + attributeDiff := computeDiff(attributeValue, attribute, currentAction) if attributeDiff.Action == plans.NoOp && attributeValue.Before == nil && attributeValue.After == nil { // We skip attributes of objects that are null both before and // after. We don't even count these as unchanged attributes. diff --git a/internal/command/jsonprovider/attribute.go b/internal/command/jsonprovider/attribute.go index 4bf380cda2..a921588e9b 100644 --- a/internal/command/jsonprovider/attribute.go +++ b/internal/command/jsonprovider/attribute.go @@ -22,6 +22,7 @@ type Attribute struct { Optional bool `json:"optional,omitempty"` Computed bool `json:"computed,omitempty"` Sensitive bool `json:"sensitive,omitempty"` + WriteOnly bool `json:"write_only,omitempty"` } type NestedType struct { @@ -47,6 +48,7 @@ func marshalAttribute(attr *configschema.Attribute) *Attribute { Computed: attr.Computed, Sensitive: attr.Sensitive, Deprecated: attr.Deprecated, + WriteOnly: attr.WriteOnly, } // we're not concerned about errors because at this point the schema has diff --git a/internal/command/jsonprovider/attribute_test.go b/internal/command/jsonprovider/attribute_test.go index 372c80b719..540ad435e5 100644 --- a/internal/command/jsonprovider/attribute_test.go +++ b/internal/command/jsonprovider/attribute_test.go @@ -21,11 +21,13 @@ func TestMarshalAttribute(t *testing.T) { Want *Attribute }{ { - &configschema.Attribute{Type: cty.String, Optional: true, Computed: true}, + &configschema.Attribute{Type: cty.String, Optional: true, Computed: true, Sensitive: true, WriteOnly: true}, &Attribute{ AttributeType: json.RawMessage(`"string"`), Optional: true, Computed: true, + Sensitive: true, + WriteOnly: true, DescriptionKind: "plain", }, }, diff --git a/internal/configs/configschema/schema.go b/internal/configs/configschema/schema.go index a79ffca5f4..85f475d795 100644 --- a/internal/configs/configschema/schema.go +++ b/internal/configs/configschema/schema.go @@ -100,6 +100,9 @@ type Attribute struct { Deprecated bool + // WriteOnly indicates that such an attribute can receive ephemeral values. + // When configured as true, these attributes cannot have values returned by + // the provider, and an error will be returned if OpenTofu detects such a thing. WriteOnly bool } diff --git a/internal/configs/configschema/write_only.go b/internal/configs/configschema/write_only.go new file mode 100644 index 0000000000..dcc0f475f4 --- /dev/null +++ b/internal/configs/configschema/write_only.go @@ -0,0 +1,188 @@ +// Copyright (c) The OpenTofu Authors +// SPDX-License-Identifier: MPL-2.0 +// Copyright (c) 2023 HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package configschema + +import ( + "fmt" + + "github.com/zclconf/go-cty/cty" +) + +// PathSetContainsWriteOnly checks that the given cty.PathSet contains *at least* one +// of write-only attribute from the given value's schema. +func (b *Block) PathSetContainsWriteOnly(v cty.Value, ps cty.PathSet) bool { + paths := b.WriteOnlyPaths(v, nil) + for _, path := range paths { + if ps.Has(path) { + return true + } + } + return false +} + +// WriteOnlyPaths returns the list of paths where write-only attributes +// exist in the given value. +// This logic is similar to the Block.ValueMarks since the logic of drilling into the +// value is similar. +func (b *Block) WriteOnlyPaths(val cty.Value, path cty.Path) []cty.Path { + var res []cty.Path + + // No need to get the paths since the value has no values inside. + if val.IsNull() || !val.IsKnown() { + return res + } + + for name, attrS := range b.Attributes { + if attrS.WriteOnly { + // Create a copy of the path, with this step added, to add to our paths slice + attrPath := copyAndExtendPath(path, cty.GetAttrStep{Name: name}) + res = append(res, attrPath) + } + } + + // Extract paths for nested attribute type values + for name, attrS := range b.Attributes { + // If the attribute has no nested type, or the nested type doesn't + // contain any write-only attributes, skip inspecting it + if attrS.NestedType == nil || !attrS.NestedType.ContainsWriteOnly() { + continue + } + + // Create a copy of the path, with this step added, to add to our paths slice + attrPath := copyAndExtendPath(path, cty.GetAttrStep{Name: name}) + + res = append(res, attrS.NestedType.WriteOnlyPaths(val.GetAttr(name), attrPath)...) + } + + // Extract paths from nested blocks + for name, blockS := range b.BlockTypes { + // If our block doesn't contain any write-only attributes, skip inspecting it + if !blockS.Block.ContainsWriteOnly() { + continue + } + + blockV := val.GetAttr(name) + if blockV.IsNull() || !blockV.IsKnown() { + continue + } + + // Create a copy of the path, with this step added, to add to our paths slice + blockPath := copyAndExtendPath(path, cty.GetAttrStep{Name: name}) + + switch blockS.Nesting { + case NestingSingle, NestingGroup: + res = append(res, blockS.Block.WriteOnlyPaths(blockV, blockPath)...) + case NestingList, NestingMap, NestingSet: + for it := blockV.ElementIterator(); it.Next(); { + idx, blockEV := it.Element() + // Create a copy of the path, with this block instance's index + // step added, to add to our paths slice + blockInstancePath := copyAndExtendPath(blockPath, cty.IndexStep{Key: idx}) + morePaths := blockS.Block.WriteOnlyPaths(blockEV, blockInstancePath) + res = append(res, morePaths...) + } + default: + panic(fmt.Sprintf("unsupported nesting mode %s", blockS.Nesting)) + } + } + return res +} + +// WriteOnlyPaths returns a slice of paths pointing to the attributes that are +// configured as write-only. +func (o *Object) WriteOnlyPaths(val cty.Value, path cty.Path) []cty.Path { + var res []cty.Path + + if val.IsNull() || !val.IsKnown() { + return res + } + + for name, attrS := range o.Attributes { + // Skip attributes which can never produce write-only paths + if !attrS.WriteOnly && (attrS.NestedType == nil || !attrS.NestedType.ContainsWriteOnly()) { + continue + } + + switch o.Nesting { + case NestingSingle, NestingGroup: + // Create a path to this attribute + attrPath := copyAndExtendPath(path, cty.GetAttrStep{Name: name}) + + if attrS.WriteOnly { + // If the entire attribute is write-only save that path fully + res = append(res, attrPath) + } else { + // The attribute has a nested type which contains write-only + // attributes, so recurse + res = append(res, attrS.NestedType.WriteOnlyPaths(val.GetAttr(name), attrPath)...) + } + case NestingList, NestingMap, NestingSet: + // For nested attribute types which have a non-single nesting mode, + // we add write-only paths for each element. + for it := val.ElementIterator(); it.Next(); { + idx, attrEV := it.Element() + attrV := attrEV.GetAttr(name) + + // Create a path to this element of the attribute's collection. Note + // that the path is extended in opposite order to the iteration order + // of the loops: index into the collection, then the contained + // attribute name. This is because we have one type + // representing multiple collection elements. + attrPath := copyAndExtendPath(path, cty.IndexStep{Key: idx}, cty.GetAttrStep{Name: name}) + + if attrS.WriteOnly { + // If the entire attribute is configured as write only, mark it so + res = append(res, attrPath) + } else { + // The attribute has a nested type which contains write-only + // attributes, so recurse + res = append(res, attrS.NestedType.WriteOnlyPaths(attrV, attrPath)...) + } + } + default: + panic(fmt.Sprintf("unsupported nesting mode %s", attrS.NestedType.Nesting)) + } + } + return res +} + +// ContainsWriteOnly returns true if any of the attributes of the receiving +// block or any of its descendent blocks are marked as write-only. +// +// Blocks themselves cannot be write-only as a whole -- write-only is a +// per-attribute idea -- but sometimes we want to include a whole object +// decoded from a block in some UI output, and that is safe to do only if +// none of the contained attributes are write-only. +func (b *Block) ContainsWriteOnly() bool { + for _, attrS := range b.Attributes { + if attrS.WriteOnly { + return true + } + if attrS.NestedType != nil && attrS.NestedType.ContainsWriteOnly() { + return true + } + } + for _, blockS := range b.BlockTypes { + if blockS.ContainsWriteOnly() { + return true + } + } + return false +} + +// ContainsWriteOnly returns true if any of the attributes of the receiving +// Object are marked as write-only. +func (o *Object) ContainsWriteOnly() bool { + for _, attrS := range o.Attributes { + if attrS.WriteOnly { + return true + } + if attrS.NestedType != nil && attrS.NestedType.ContainsWriteOnly() { + return true + } + } + return false +} diff --git a/internal/configs/configschema/write_only_test.go b/internal/configs/configschema/write_only_test.go new file mode 100644 index 0000000000..2d52389c4e --- /dev/null +++ b/internal/configs/configschema/write_only_test.go @@ -0,0 +1,387 @@ +// Copyright (c) The OpenTofu Authors +// SPDX-License-Identifier: MPL-2.0 +// Copyright (c) 2023 HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package configschema + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + "github.com/zclconf/go-cty/cty" +) + +func TestBlock_WriteOnlyPaths(t *testing.T) { + complexBlock := &Block{ + Attributes: map[string]*Attribute{ + "regular_attr": {}, + "sensitive_attr": {Sensitive: true}, + "wo_attr": {WriteOnly: true}, + "nested_single_attribute": { + NestedType: &Object{ + Attributes: map[string]*Attribute{ + "regular_attr": {}, + "sensitive_attr": {Sensitive: true}, + "wo_attr": {WriteOnly: true}, + }, + Nesting: NestingSingle, + }, + }, + "nested_set_attribute": { + NestedType: &Object{ + Attributes: map[string]*Attribute{ + "regular_attr": {}, + "sensitive_attr": {Sensitive: true}, + "wo_attr": {WriteOnly: true}, + }, + Nesting: NestingSet, + }, + }, + "nested_map_attribute": { + NestedType: &Object{ + Attributes: map[string]*Attribute{ + "regular_attr": {}, + "sensitive_attr": {Sensitive: true}, + "wo_attr": {WriteOnly: true}, + }, + Nesting: NestingMap, + }, + }, + }, + BlockTypes: map[string]*NestedBlock{ + "nested_single_block": { + Block: Block{ + Attributes: map[string]*Attribute{ + "regular_attr": {}, + "sensitive_attr": {Sensitive: true}, + "wo_attr": {WriteOnly: true}, + }, + }, + Nesting: NestingSingle, + }, + "nested_set_block": { + Block: Block{ + Attributes: map[string]*Attribute{ + "wo_attr": {WriteOnly: true}, + }, + }, + Nesting: NestingSet, + }, + "nested_map_block": { + Block: Block{ + Attributes: map[string]*Attribute{ + "wo_attr": {WriteOnly: true}, + }, + }, + Nesting: NestingMap, + }, + }, + } + cases := map[string]struct { + block *Block + val cty.Value + want []cty.Path + }{ + "only attributes": { + block: complexBlock, + val: cty.ObjectVal(map[string]cty.Value{ + "regular_attr": cty.StringVal("foo"), + "sensitive_attr": cty.StringVal("bar"), + "wo_attr": cty.StringVal("baz"), + "nested_single_attribute": cty.NullVal(cty.Object(map[string]cty.Type{})), + "nested_set_attribute": cty.NullVal(cty.Set(cty.String)), + "nested_map_attribute": cty.NullVal(cty.Map(cty.String)), + "nested_single_block": cty.NullVal(cty.Object(map[string]cty.Type{})), + "nested_set_block": cty.NullVal(cty.Set(cty.String)), + "nested_map_block": cty.NullVal(cty.Map(cty.String)), + }), + want: []cty.Path{ + cty.GetAttrPath("wo_attr"), + }, + }, + "single nested attribute": { + block: complexBlock, + val: cty.ObjectVal(map[string]cty.Value{ + "regular_attr": cty.NullVal(cty.String), + "sensitive_attr": cty.NullVal(cty.String), + "wo_attr": cty.StringVal("baz"), + "nested_single_attribute": cty.ObjectVal(map[string]cty.Value{ + "regular_attr": cty.StringVal("foo"), + "sensitive_attr": cty.StringVal("bar"), + "wo_attr": cty.StringVal("baz"), + }), + "nested_set_attribute": cty.NullVal(cty.Set(cty.String)), + "nested_map_attribute": cty.NullVal(cty.Map(cty.String)), + "nested_single_block": cty.NullVal(cty.Object(map[string]cty.Type{})), + "nested_set_block": cty.NullVal(cty.Set(cty.String)), + "nested_map_block": cty.NullVal(cty.Map(cty.String)), + }), + want: []cty.Path{ + cty.GetAttrPath("wo_attr"), + cty.GetAttrPath("nested_single_attribute").GetAttr("wo_attr"), + }, + }, + "set nested attribute": { + block: complexBlock, + val: cty.ObjectVal(map[string]cty.Value{ + "regular_attr": cty.NullVal(cty.String), + "sensitive_attr": cty.NullVal(cty.String), + "wo_attr": cty.StringVal("baz"), + "nested_single_attribute": cty.NullVal(cty.Object(map[string]cty.Type{})), + "nested_set_attribute": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{"wo_attr": cty.StringVal("foo")}), + }), + "nested_map_attribute": cty.NullVal(cty.Map(cty.String)), + "nested_single_block": cty.NullVal(cty.Object(map[string]cty.Type{})), + "nested_set_block": cty.NullVal(cty.Set(cty.String)), + "nested_map_block": cty.NullVal(cty.Map(cty.String)), + }), + want: []cty.Path{ + cty.GetAttrPath("wo_attr"), + cty.GetAttrPath("nested_set_attribute").Index(cty.ObjectVal(map[string]cty.Value{"wo_attr": cty.StringVal("foo")})).GetAttr("wo_attr"), + }, + }, + "map nested attribute": { + block: complexBlock, + val: cty.ObjectVal(map[string]cty.Value{ + "regular_attr": cty.NullVal(cty.String), + "sensitive_attr": cty.NullVal(cty.String), + "wo_attr": cty.StringVal("baz"), + "nested_single_attribute": cty.NullVal(cty.Object(map[string]cty.Type{})), + "nested_set_attribute": cty.NullVal(cty.Set(cty.String)), + "nested_map_attribute": cty.MapVal(map[string]cty.Value{ + "wo_attr": cty.ObjectVal(map[string]cty.Value{"wo_attr": cty.StringVal("foo")}), + }), + "nested_single_block": cty.NullVal(cty.Object(map[string]cty.Type{})), + "nested_set_block": cty.NullVal(cty.Set(cty.String)), + "nested_map_block": cty.NullVal(cty.Map(cty.String)), + }), + want: []cty.Path{ + cty.GetAttrPath("wo_attr"), + cty.GetAttrPath("nested_map_attribute").Index(cty.StringVal("wo_attr")).GetAttr("wo_attr"), + }, + }, + "single nested block": { + block: complexBlock, + val: cty.ObjectVal(map[string]cty.Value{ + "regular_attr": cty.NullVal(cty.String), + "sensitive_attr": cty.NullVal(cty.String), + "wo_attr": cty.StringVal("baz"), + "nested_single_attribute": cty.NullVal(cty.Object(map[string]cty.Type{})), + "nested_set_attribute": cty.NullVal(cty.Set(cty.String)), + "nested_map_attribute": cty.NullVal(cty.Map(cty.String)), + "nested_single_block": cty.ObjectVal(map[string]cty.Value{ + "regular_attr": cty.StringVal("foo"), + "sensitive_attr": cty.StringVal("bar"), + "wo_attr": cty.StringVal("baz"), + }), + "nested_set_block": cty.NullVal(cty.Set(cty.String)), + "nested_map_block": cty.NullVal(cty.Map(cty.String)), + }), + want: []cty.Path{ + cty.GetAttrPath("wo_attr"), + cty.GetAttrPath("nested_single_block").GetAttr("wo_attr"), + }, + }, + "set nested block": { + block: complexBlock, + val: cty.ObjectVal(map[string]cty.Value{ + "regular_attr": cty.NullVal(cty.String), + "sensitive_attr": cty.NullVal(cty.String), + "wo_attr": cty.StringVal("baz"), + "nested_single_attribute": cty.NullVal(cty.Object(map[string]cty.Type{})), + "nested_set_attribute": cty.NullVal(cty.Set(cty.String)), + "nested_map_attribute": cty.NullVal(cty.Map(cty.String)), + "nested_single_block": cty.NullVal(cty.Object(map[string]cty.Type{})), + "nested_set_block": cty.SetVal([]cty.Value{ + cty.StringVal("foo"), + }), + "nested_map_block": cty.NullVal(cty.Map(cty.String)), + }), + want: []cty.Path{ + cty.GetAttrPath("wo_attr"), + cty.GetAttrPath("nested_set_block").IndexString("foo").GetAttr("wo_attr"), + }, + }, + "map nested block": { + block: complexBlock, + val: cty.ObjectVal(map[string]cty.Value{ + "regular_attr": cty.NullVal(cty.String), + "sensitive_attr": cty.NullVal(cty.String), + "wo_attr": cty.StringVal("baz"), + "nested_single_attribute": cty.NullVal(cty.Object(map[string]cty.Type{})), + "nested_set_attribute": cty.NullVal(cty.Set(cty.String)), + "nested_map_attribute": cty.NullVal(cty.Map(cty.String)), + "nested_single_block": cty.NullVal(cty.Object(map[string]cty.Type{})), + "nested_set_block": cty.NullVal(cty.Set(cty.String)), + "nested_map_block": cty.MapVal(map[string]cty.Value{ + "wo_attr": cty.StringVal("foo"), + }), + }), + want: []cty.Path{ + cty.GetAttrPath("wo_attr"), + cty.GetAttrPath("nested_map_block").Index(cty.StringVal("wo_attr")).GetAttr("wo_attr"), + }, + }, + } + for name, tt := range cases { + t.Run(name, func(t *testing.T) { + got := tt.block.WriteOnlyPaths(tt.val, nil) + gotPs := cty.NewPathSet(got...) + wantPs := cty.NewPathSet(tt.want...) + if !gotPs.Equal(wantPs) { + diff := cmp.Diff(wantPs.List(), gotPs.List(), cmpopts.EquateComparable(cty.GetAttrStep{}, cty.IndexStep{})) + t.Errorf("paths returned are not as expected:\n%s", diff) + } + }) + } +} + +func TestBlock_PathSetContainsWriteOnly(t *testing.T) { + complexBlock := &Block{ + Attributes: map[string]*Attribute{ + "regular_attr": {}, + "sensitive_attr": {Sensitive: true}, + "wo_attr": {WriteOnly: true}, + "nested_set_attribute": { + NestedType: &Object{ + Attributes: map[string]*Attribute{ + "regular_attr": {}, + "sensitive_attr": {Sensitive: true}, + "wo_attr": {WriteOnly: true}, + }, + Nesting: NestingSet, + }, + }, + "nested_map_attribute": { + NestedType: &Object{ + Attributes: map[string]*Attribute{ + "regular_attr": {}, + "sensitive_attr": {Sensitive: true}, + "wo_attr": {WriteOnly: true}, + }, + Nesting: NestingMap, + }, + }, + }, + BlockTypes: map[string]*NestedBlock{ + "nested_set_block": { + Block: Block{ + Attributes: map[string]*Attribute{ + "wo_attr": {WriteOnly: true}, + }, + }, + Nesting: NestingSet, + }, + }, + } + cases := map[string]struct { + block *Block + val cty.Value + pathSet cty.PathSet + want bool + }{ + "PathSet pointing to a simple root attribute": { + block: complexBlock, + val: cty.ObjectVal(map[string]cty.Value{ + "regular_attr": cty.StringVal("foo"), + "sensitive_attr": cty.StringVal("bar"), + "wo_attr": cty.StringVal("baz"), + "nested_set_attribute": cty.NullVal(cty.Set(cty.String)), + "nested_map_attribute": cty.NullVal(cty.Map(cty.String)), + "nested_set_block": cty.NullVal(cty.Set(cty.String)), + }), + pathSet: cty.NewPathSet([]cty.Path{ + cty.GetAttrPath("wo_attr"), + }...), + want: true, + }, + "PathSet points to a path in a existing nested object": { + block: complexBlock, + val: cty.ObjectVal(map[string]cty.Value{ + "regular_attr": cty.NullVal(cty.String), + "sensitive_attr": cty.NullVal(cty.String), + "wo_attr": cty.StringVal("baz"), + "nested_set_attribute": cty.NullVal(cty.Set(cty.String)), + "nested_map_attribute": cty.MapVal(map[string]cty.Value{ + "wo_attr": cty.ObjectVal(map[string]cty.Value{"wo_attr": cty.StringVal("foo")}), + }), + "nested_single_block": cty.NullVal(cty.Object(map[string]cty.Type{})), + "nested_set_block": cty.NullVal(cty.Set(cty.String)), + "nested_map_block": cty.NullVal(cty.Map(cty.String)), + }), + pathSet: cty.NewPathSet([]cty.Path{ + cty.GetAttrPath("nested_map_attribute").Index(cty.StringVal("wo_attr")).GetAttr("wo_attr"), + }...), + want: true, + }, + "empty PathSet": { + block: complexBlock, + val: cty.ObjectVal(map[string]cty.Value{ + "regular_attr": cty.NullVal(cty.String), + "sensitive_attr": cty.NullVal(cty.String), + "wo_attr": cty.StringVal("baz"), + "nested_set_attribute": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{"wo_attr": cty.ObjectVal(map[string]cty.Value{"wo_attr": cty.StringVal("foo")})}), + }), + "nested_map_attribute": cty.NullVal(cty.Map(cty.String)), + "nested_single_block": cty.NullVal(cty.Object(map[string]cty.Type{})), + "nested_set_block": cty.NullVal(cty.Set(cty.String)), + "nested_map_block": cty.NullVal(cty.Map(cty.String)), + }), + pathSet: cty.NewPathSet(), + want: false, + }, + "PathSet points to unset root value": { + block: complexBlock, + val: cty.ObjectVal(map[string]cty.Value{ + "regular_attr": cty.NullVal(cty.String), + "sensitive_attr": cty.NullVal(cty.String), + "wo_attr": cty.NullVal(cty.String), + "nested_single_attribute": cty.NullVal(cty.Object(map[string]cty.Type{})), + "nested_set_attribute": cty.NullVal(cty.Set(cty.String)), + "nested_map_attribute": cty.MapVal(map[string]cty.Value{ + "wo_attr": cty.ObjectVal(map[string]cty.Value{"wo_attr": cty.StringVal("foo")}), + }), + "nested_single_block": cty.NullVal(cty.Object(map[string]cty.Type{})), + "nested_set_block": cty.NullVal(cty.Set(cty.String)), + "nested_map_block": cty.NullVal(cty.Map(cty.String)), + }), + pathSet: cty.NewPathSet([]cty.Path{ + cty.GetAttrPath("wo_attr"), + }...), + want: true, + }, + "PathSet points to unset value from a nil block": { + block: complexBlock, + val: cty.ObjectVal(map[string]cty.Value{ + "regular_attr": cty.NullVal(cty.String), + "sensitive_attr": cty.NullVal(cty.String), + "wo_attr": cty.StringVal("baz"), + "nested_single_attribute": cty.NullVal(cty.Object(map[string]cty.Type{})), + "nested_set_attribute": cty.NullVal(cty.Set(cty.String)), + "nested_map_attribute": cty.NullVal(cty.Map(cty.String)), + "nested_single_block": cty.NullVal(cty.Object(map[string]cty.Type{})), + "nested_set_block": cty.NullVal(cty.Set(cty.String)), + "nested_map_block": cty.NullVal(cty.Map(cty.String)), + }), + pathSet: cty.NewPathSet([]cty.Path{ + cty.GetAttrPath("nested_single_block").GetAttr("wo_attr"), + }...), + want: false, + }, + } + for name, tt := range cases { + t.Run(name, func(t *testing.T) { + got := tt.block.PathSetContainsWriteOnly(tt.val, tt.pathSet) + if got != tt.want { + existingPaths := tt.block.WriteOnlyPaths(tt.val, nil) + gotPs := cty.NewPathSet(existingPaths...) + diff := cmp.Diff(tt.pathSet.List(), gotPs.List(), cmpopts.EquateComparable(cty.GetAttrStep{}, cty.IndexStep{})) + t.Errorf("wanted %t but got %t:\n%s", tt.want, got, diff) + } + }) + } +} diff --git a/internal/plans/objchange/plan_valid.go b/internal/plans/objchange/plan_valid.go index 72ed9785ef..77259e5250 100644 --- a/internal/plans/objchange/plan_valid.go +++ b/internal/plans/objchange/plan_valid.go @@ -308,14 +308,20 @@ func assertPlannedValueValid(attrS *configschema.Attribute, priorV, configV, pla return errs } + // If there is a config value but there is nothing planned, we allow this in case of write only attributes. + // This validation covers all the cases: + // - root resource attributes + // - nested attributes object attributes + // - nested blocks attributes + if !configV.IsNull() && plannedV.IsNull() && attrS.WriteOnly { + return errs + } + // If this attribute has a NestedType, validate the nested object if attrS.NestedType != nil { return assertPlannedObjectValid(attrS.NestedType, priorV, configV, plannedV, path) } - if !configV.IsNull() && plannedV.IsNull() && attrS.WriteOnly { - return errs // TODO ephemeral - check other places that might need a validation like this (part of the write-only attributes work) - } // If none of the above conditions match, the provider has made an invalid // change to this attribute. if priorV.IsNull() { diff --git a/internal/plans/objchange/plan_valid_test.go b/internal/plans/objchange/plan_valid_test.go index 0654789316..3fc39375c6 100644 --- a/internal/plans/objchange/plan_valid_test.go +++ b/internal/plans/objchange/plan_valid_test.go @@ -1938,6 +1938,390 @@ func TestAssertPlanValid(t *testing.T) { }), []string{}, }, + + "write-only attributes": { + &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "string_wo": { + Optional: true, + WriteOnly: true, + Type: cty.String, + }, + "object_attribute": { + Optional: true, + WriteOnly: true, + Type: cty.Object(map[string]cty.Type{ + "inner": cty.String, + }), + }, + "map_attribute": { + Optional: true, + WriteOnly: true, + Type: cty.Object(map[string]cty.Type{ + "inner": cty.String, + }), + }, + "list_attribute": { + Optional: true, + WriteOnly: true, + Type: cty.Object(map[string]cty.Type{ + "inner": cty.String, + }), + }, + }, + }, + cty.NilVal, + cty.ObjectVal(map[string]cty.Value{ + "string_wo": cty.StringVal("test value"), + "object_attribute": cty.ObjectVal(map[string]cty.Value{ + "inner": cty.StringVal("object_attribute inner value"), + }), + "map_attribute": cty.ObjectVal(map[string]cty.Value{ + "inner": cty.StringVal("map_attribute inner value"), + }), + "list_attribute": cty.ListVal([]cty.Value{ + cty.StringVal("list_attribute inner value"), + }), + }), + cty.ObjectVal(map[string]cty.Value{ + "string_wo": cty.NullVal(cty.String), + "object_attribute": cty.NullVal( + cty.Object( + map[string]cty.Type{"inner": cty.Number}, + ), + ), + "map_attribute": cty.NullVal( + cty.Map(cty.String), + ), + "list_attribute": cty.NullVal( + cty.List(cty.String), + ), + }), + []string{}, + }, + + "write-only nested attributes": { + &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "single_nested_attribute_wo": { + Optional: true, + WriteOnly: true, + NestedType: &configschema.Object{ + Nesting: configschema.NestingSingle, + Attributes: map[string]*configschema.Attribute{ + "inner": { + Type: cty.String, + WriteOnly: true, + }, + }, + }, + }, + "single_nested_attribute": { + Optional: true, + NestedType: &configschema.Object{ + Nesting: configschema.NestingSingle, + Attributes: map[string]*configschema.Attribute{ + "inner": { + Type: cty.String, + WriteOnly: true, + }, + }, + }, + }, + "list_nested_attribute_wo": { + Optional: true, + WriteOnly: true, + NestedType: &configschema.Object{ + Nesting: configschema.NestingList, + Attributes: map[string]*configschema.Attribute{ + "inner": { + Type: cty.String, + WriteOnly: true, + }, + }, + }, + }, + "list_nested_attribute": { + Optional: true, + NestedType: &configschema.Object{ + Nesting: configschema.NestingList, + Attributes: map[string]*configschema.Attribute{ + "inner": { + Type: cty.String, + WriteOnly: true, + }, + }, + }, + }, + "map_nested_attribute_wo": { + Optional: true, + WriteOnly: true, + NestedType: &configschema.Object{ + Nesting: configschema.NestingMap, + Attributes: map[string]*configschema.Attribute{ + "inner": { + Type: cty.String, + WriteOnly: true, + }, + }, + }, + }, + "map_nested_attribute": { + Optional: true, + NestedType: &configschema.Object{ + Nesting: configschema.NestingMap, + Attributes: map[string]*configschema.Attribute{ + "inner": { + Type: cty.String, + WriteOnly: true, + }, + }, + }, + }, + }, + }, + cty.NilVal, + cty.ObjectVal(map[string]cty.Value{ + "single_nested_attribute_wo": cty.ObjectVal(map[string]cty.Value{ + "inner": cty.StringVal("a"), + }), + "single_nested_attribute": cty.ObjectVal(map[string]cty.Value{ + "inner": cty.StringVal("a"), + }), + "list_nested_attribute_wo": cty.ListVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "inner": cty.StringVal("a"), + }), + cty.ObjectVal(map[string]cty.Value{ + "inner": cty.StringVal("b"), + }), + }), + "list_nested_attribute": cty.ListVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "inner": cty.StringVal("a"), + }), + cty.ObjectVal(map[string]cty.Value{ + "inner": cty.StringVal("b"), + }), + }), + "map_nested_attribute_wo": cty.MapVal(map[string]cty.Value{ + "first": cty.ObjectVal(map[string]cty.Value{ + "inner": cty.StringVal("a"), + }), + "second": cty.ObjectVal(map[string]cty.Value{ + "inner": cty.StringVal("b"), + }), + }), + "map_nested_attribute": cty.MapVal(map[string]cty.Value{ + "first": cty.ObjectVal(map[string]cty.Value{ + "inner": cty.StringVal("a"), + }), + "second": cty.ObjectVal(map[string]cty.Value{ + "inner": cty.StringVal("b"), + }), + }), + }), + cty.ObjectVal(map[string]cty.Value{ + "single_nested_attribute_wo": cty.NullVal(cty.Object(map[string]cty.Type{"inner": cty.String})), + "single_nested_attribute": cty.ObjectVal(map[string]cty.Value{ + "inner": cty.NullVal(cty.String), + }), + "list_nested_attribute_wo": cty.NullVal(cty.List(cty.Object(map[string]cty.Type{"inner": cty.String}))), + "list_nested_attribute": cty.ListVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "inner": cty.NullVal(cty.String), + }), + cty.ObjectVal(map[string]cty.Value{ + "inner": cty.NullVal(cty.String), + }), + }), + "map_nested_attribute_wo": cty.NullVal(cty.Map(cty.Object(map[string]cty.Type{"inner": cty.String}))), + "map_nested_attribute": cty.MapVal(map[string]cty.Value{ + "first": cty.ObjectVal(map[string]cty.Value{ + "inner": cty.NullVal(cty.String), + }), + "second": cty.ObjectVal(map[string]cty.Value{ + "inner": cty.NullVal(cty.String), + }), + }), + }), + []string{}, + }, + "write-only nested blocks": { + &configschema.Block{ + BlockTypes: map[string]*configschema.NestedBlock{ + "single_nested_block": { + Nesting: configschema.NestingSingle, + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "inner": { + Type: cty.String, + WriteOnly: true, + }, + }, + }, + }, + "list_nested_block": { + Nesting: configschema.NestingList, + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "inner": { + Type: cty.String, + WriteOnly: true, + }, + }, + }, + }, + "map_nested_block": { + Nesting: configschema.NestingMap, + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "inner": { + Type: cty.String, + WriteOnly: true, + }, + }, + }, + }, + }, + }, + cty.NilVal, + cty.ObjectVal(map[string]cty.Value{ + "single_nested_block": cty.ObjectVal(map[string]cty.Value{ + "inner": cty.StringVal("a"), + }), + "list_nested_block": cty.ListVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "inner": cty.StringVal("a"), + }), + cty.ObjectVal(map[string]cty.Value{ + "inner": cty.StringVal("b"), + }), + }), + "map_nested_block": cty.MapVal(map[string]cty.Value{ + "first": cty.ObjectVal(map[string]cty.Value{ + "inner": cty.StringVal("a"), + }), + "second": cty.ObjectVal(map[string]cty.Value{ + "inner": cty.StringVal("b"), + }), + }), + }), + cty.ObjectVal(map[string]cty.Value{ + "single_nested_block": cty.ObjectVal(map[string]cty.Value{ + "inner": cty.NullVal(cty.String), + }), + "list_nested_block": cty.ListVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "inner": cty.NullVal(cty.String), + }), + cty.ObjectVal(map[string]cty.Value{ + "inner": cty.NullVal(cty.String), + }), + }), + "map_nested_block": cty.MapVal(map[string]cty.Value{ + "first": cty.ObjectVal(map[string]cty.Value{ + "inner": cty.NullVal(cty.String), + }), + "second": cty.ObjectVal(map[string]cty.Value{ + "inner": cty.NullVal(cty.String), + }), + }), + }), + []string{}, + }, + // IRL, set-type attributes, and nested attributes/blocks cannot be or contain write-only attributes. + // This is checking that the AssertPlanValid works correctly with these types. + "write-only set attributes and blocks": { + Schema: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "set_attribute": { + Optional: true, + WriteOnly: true, + Type: cty.Set(cty.String), + }, + "set_nested_attribute": { + Optional: true, + WriteOnly: true, + NestedType: &configschema.Object{ + Nesting: configschema.NestingSet, + Attributes: map[string]*configschema.Attribute{ + "inner": { + Type: cty.String, + WriteOnly: true, + }, + }, + }, + }, + }, + BlockTypes: map[string]*configschema.NestedBlock{ + "set_nested_block": { + Nesting: configschema.NestingSet, + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "val": { + Type: cty.String, + }, + }, + }, + }, + }, + }, + Prior: cty.NilVal, + Config: cty.ObjectVal(map[string]cty.Value{ + "set_attribute": cty.ListVal([]cty.Value{ + cty.StringVal("set_attribute inner value"), + cty.StringVal("set_attribute inner value 2"), + }), + "set_nested_attribute": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "inner": cty.StringVal("a"), + }), + cty.ObjectVal(map[string]cty.Value{ + "inner": cty.StringVal("b"), + }), + }), + "set_nested_attribute_wo": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "inner": cty.StringVal("a"), + }), + cty.ObjectVal(map[string]cty.Value{ + "inner": cty.StringVal("b"), + }), + }), + "set_nested_block": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "val": cty.StringVal("a"), + }), + cty.ObjectVal(map[string]cty.Value{ + "val": cty.StringVal("b"), + }), + }), + }), + Planned: cty.ObjectVal(map[string]cty.Value{ + "set_attribute": cty.NullVal( + cty.Set(cty.String), + ), + "set_nested_attribute_wo": cty.NullVal(cty.Set(cty.Object(map[string]cty.Type{"inner": cty.String}))), + "set_nested_attribute": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "inner": cty.NullVal(cty.String), + }), + cty.ObjectVal(map[string]cty.Value{ + "inner": cty.NullVal(cty.String), + }), + }), + "set_nested_block": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "val": cty.NullVal(cty.String), + }), + cty.ObjectVal(map[string]cty.Value{ + "val": cty.NullVal(cty.String), + }), + }), + }), + WantErrs: []string{".set_nested_attribute: count in plan (cty.NumberIntVal(1)) disagrees with count in config (cty.NumberIntVal(2))"}, + }, } for name, test := range tests { diff --git a/internal/plugin/convert/schema_test.go b/internal/plugin/convert/schema_test.go index 97cb1f4c75..f7c13636ba 100644 --- a/internal/plugin/convert/schema_test.go +++ b/internal/plugin/convert/schema_test.go @@ -51,6 +51,11 @@ func TestConvertSchemaBlocks(t *testing.T) { Type: []byte(`"number"`), Required: true, }, + { + Name: "write_only", + Type: []byte(`"number"`), + WriteOnly: true, + }, }, }, &configschema.Block{ @@ -72,6 +77,10 @@ func TestConvertSchemaBlocks(t *testing.T) { Type: cty.Number, Required: true, }, + "write_only": { + Type: cty.Number, + WriteOnly: true, + }, }, }, }, @@ -103,6 +112,11 @@ func TestConvertSchemaBlocks(t *testing.T) { Type: []byte(`"dynamic"`), Required: true, }, + { + Name: "write_only", + Type: []byte(`"dynamic"`), + WriteOnly: true, + }, }, }, }, @@ -127,6 +141,10 @@ func TestConvertSchemaBlocks(t *testing.T) { Type: cty.DynamicPseudoType, Required: true, }, + "write_only": { + Type: cty.DynamicPseudoType, + WriteOnly: true, + }, }, }, }, @@ -223,6 +241,11 @@ func TestConvertProtoSchemaBlocks(t *testing.T) { Type: []byte(`"number"`), Required: true, }, + { + Name: "write_only", + Type: []byte(`"number"`), + WriteOnly: true, + }, }, }, &configschema.Block{ @@ -244,6 +267,10 @@ func TestConvertProtoSchemaBlocks(t *testing.T) { Type: cty.Number, Required: true, }, + "write_only": { + Type: cty.Number, + WriteOnly: true, + }, }, }, }, @@ -275,6 +302,11 @@ func TestConvertProtoSchemaBlocks(t *testing.T) { Type: []byte(`"dynamic"`), Required: true, }, + { + Name: "write_only", + Type: []byte(`"dynamic"`), + WriteOnly: true, + }, }, }, }, @@ -299,6 +331,10 @@ func TestConvertProtoSchemaBlocks(t *testing.T) { Type: cty.DynamicPseudoType, Required: true, }, + "write_only": { + Type: cty.DynamicPseudoType, + WriteOnly: true, + }, }, }, }, diff --git a/internal/plugin/grpc_provider.go b/internal/plugin/grpc_provider.go index 58a65ddc5c..f3ca086a23 100644 --- a/internal/plugin/grpc_provider.go +++ b/internal/plugin/grpc_provider.go @@ -12,6 +12,7 @@ import ( "sync" plugin "github.com/hashicorp/go-plugin" + "github.com/opentofu/opentofu/internal/plugin/validation" "github.com/zclconf/go-cty/cty" ctyjson "github.com/zclconf/go-cty/cty/json" "github.com/zclconf/go-cty/cty/msgpack" @@ -40,7 +41,12 @@ var clientCapabilities = &proto.ClientCapabilities{ // satisfy the request. Setting this means that we need to be prepared // for there to be a "deferred" object in the response from various // other provider RPC functions. - DeferralAllowed: true, + DeferralAllowed: true, + // WriteOnlyAttributesAllowed indicates that the current system version + // supports write-only attributes. + // This enables the SDK to run specific validations and enable the + // nullification of such configured attributes before returning the + // response back to the system. WriteOnlyAttributesAllowed: true, } @@ -391,6 +397,8 @@ func (p *GRPCProvider) UpgradeResourceState(ctx context.Context, r providers.Upg } resp.UpgradedState = state + resp.Diagnostics = resp.Diagnostics.Append(validation.WriteOnlyAttributes(resSchema.Block, resp.UpgradedState, r.TypeName)) + return resp } @@ -508,6 +516,8 @@ func (p *GRPCProvider) ReadResource(ctx context.Context, r providers.ReadResourc resp.NewState = state resp.Private = protoResp.Private + resp.Diagnostics = resp.Diagnostics.Append(validation.WriteOnlyAttributes(resSchema.Block, resp.NewState, r.TypeName)) + return resp } @@ -600,6 +610,8 @@ func (p *GRPCProvider) PlanResourceChange(ctx context.Context, r providers.PlanR resp.LegacyTypeSystem = protoResp.LegacyTypeSystem + resp.Diagnostics = resp.Diagnostics.Append(validation.WriteOnlyAttributes(resSchema.Block, resp.PlannedState, r.TypeName)) + return resp } @@ -678,6 +690,8 @@ func (p *GRPCProvider) ApplyResourceChange(ctx context.Context, r providers.Appl resp.LegacyTypeSystem = protoResp.LegacyTypeSystem + resp.Diagnostics = resp.Diagnostics.Append(validation.WriteOnlyAttributes(resSchema.Block, resp.NewState, r.TypeName)) + return resp } @@ -775,6 +789,8 @@ func (p *GRPCProvider) MoveResourceState(ctx context.Context, r providers.MoveRe resp.TargetState = state resp.TargetPrivate = protoResp.TargetPrivate + resp.Diagnostics = resp.Diagnostics.Append(validation.WriteOnlyAttributes(resourceSchema.Block, resp.TargetState, r.TargetTypeName)) + return resp } diff --git a/internal/plugin/grpc_provider_test.go b/internal/plugin/grpc_provider_test.go index f4b0f3cdbb..eb47ec6656 100644 --- a/internal/plugin/grpc_provider_test.go +++ b/internal/plugin/grpc_provider_test.go @@ -7,6 +7,7 @@ package plugin import ( "bytes" + "context" "fmt" "slices" "strings" @@ -16,7 +17,9 @@ import ( "github.com/davecgh/go-spew/spew" "github.com/google/go-cmp/cmp" "github.com/zclconf/go-cty/cty" + "github.com/zclconf/go-cty/cty/msgpack" "go.uber.org/mock/gomock" + "google.golang.org/grpc" "google.golang.org/protobuf/types/known/timestamppb" "github.com/opentofu/opentofu/internal/addrs" @@ -29,7 +32,24 @@ import ( var _ providers.Interface = (*GRPCProvider)(nil) +func mutateSchemaResponse(response *proto.GetProviderSchema_Response, mut ...func(schemaResponse *proto.GetProviderSchema_Response)) *proto.GetProviderSchema_Response { + for _, f := range mut { + f(response) + } + return response +} + +func addAttributeToResource(resourceName string, attr *proto.Schema_Attribute) func(response *proto.GetProviderSchema_Response) { + return func(schemaResponse *proto.GetProviderSchema_Response) { + schemaResponse.ResourceSchemas[resourceName].Block.Attributes = append(schemaResponse.ResourceSchemas[resourceName].Block.Attributes, attr) + } +} + func mockProviderClient(t *testing.T) *mockproto.MockProviderClient { + return mockProviderClientWithSchema(t, providerProtoSchema()) +} + +func mockProviderClientWithSchema(t *testing.T, schema *proto.GetProviderSchema_Response) *mockproto.MockProviderClient { ctrl := gomock.NewController(t) client := mockproto.NewMockProviderClient(ctrl) @@ -38,7 +58,7 @@ func mockProviderClient(t *testing.T) *mockproto.MockProviderClient { gomock.Any(), gomock.Any(), gomock.Any(), - ).Return(providerProtoSchema(), nil) + ).Return(schema, nil) return client } @@ -442,6 +462,48 @@ func TestGRPCProvider_UpgradeResourceStateJSON(t *testing.T) { } } +func TestGRPCProvider_UpgradeResourceStateWithWriteOnlyReturned(t *testing.T) { + client := mockProviderClientWithSchema(t, mutateSchemaResponse(providerProtoSchema(), addAttributeToResource("resource", &proto.Schema_Attribute{ + Name: "write_only_attr", + Type: []byte(`"string"`), + Optional: true, + WriteOnly: true, + }))) + p := &GRPCProvider{ + client: client, + } + + client.EXPECT().UpgradeResourceState( + gomock.Any(), + gomock.Any(), + ).DoAndReturn(func(_ context.Context, _ *proto.UpgradeResourceState_Request, _ ...grpc.CallOption) (*proto.UpgradeResourceState_Response, error) { + b, err := msgpack.Marshal( + cty.ObjectVal(map[string]cty.Value{"attr": cty.StringVal("bar"), "write_only_attr": cty.StringVal("val")}), + cty.Object(map[string]cty.Type{"attr": cty.String, "write_only_attr": cty.String}), + ) + if err != nil { + return nil, err + } + return &proto.UpgradeResourceState_Response{ + UpgradedState: &proto.DynamicValue{ + Msgpack: b, + }, + }, nil + }) + + resp := p.UpgradeResourceState(t.Context(), providers.UpgradeResourceStateRequest{ + TypeName: "resource", + Version: 0, + RawStateJSON: []byte(`{"attr":"bar"}`), + }) + checkDiagsHasError(t, resp.Diagnostics) + + expectedErr := `Resource type "resource" returned an actual value for the write-only attribute ".write_only_attr" while it is meant to be nil. This is an issue in the provider SDK.` + if gotErr := resp.Diagnostics[0].Description().Detail; expectedErr != gotErr { + t.Errorf("the expected error is not the same with the one returned.\nexpected: %s\ngot: %s", expectedErr, gotErr) + } +} + func TestGRPCProvider_MoveResourceState(t *testing.T) { client := mockProviderClient(t) p := &GRPCProvider{ @@ -478,6 +540,49 @@ func TestGRPCProvider_MoveResourceState(t *testing.T) { } } +func TestGRPCProvider_MoveResourceStateReturnsWriteOnlyValue(t *testing.T) { + client := mockProviderClientWithSchema(t, mutateSchemaResponse(providerProtoSchema(), addAttributeToResource("resource", &proto.Schema_Attribute{ + Name: "write_only_attr", + Type: []byte(`"string"`), + Optional: true, + WriteOnly: true, + }))) + p := &GRPCProvider{ + client: client, + } + + client.EXPECT().MoveResourceState( + gomock.Any(), + gomock.Any(), + ).DoAndReturn(func(_ context.Context, _ *proto.MoveResourceState_Request, _ ...grpc.CallOption) (*proto.MoveResourceState_Response, error) { + b, err := msgpack.Marshal( + cty.ObjectVal(map[string]cty.Value{"attr": cty.StringVal("bar"), "write_only_attr": cty.StringVal("val")}), + cty.Object(map[string]cty.Type{"attr": cty.String, "write_only_attr": cty.String}), + ) + if err != nil { + return nil, err + } + + return &proto.MoveResourceState_Response{ + TargetState: &proto.DynamicValue{ + Msgpack: b, + }, + }, nil + }) + + resp := p.MoveResourceState(t.Context(), providers.MoveResourceStateRequest{ + SourceTypeName: "resource_old", + SourceSchemaVersion: 0, + TargetTypeName: "resource", + }) + checkDiagsHasError(t, resp.Diagnostics) + + expectedErr := `Resource type "resource" returned an actual value for the write-only attribute ".write_only_attr" while it is meant to be nil. This is an issue in the provider SDK.` + if gotErr := resp.Diagnostics[0].Description().Detail; expectedErr != gotErr { + t.Errorf("the expected error is not the same with the one returned.\nexpected: %s\ngot: %s", expectedErr, gotErr) + } +} + func TestGRPCProvider_Configure(t *testing.T) { client := mockProviderClient(t) p := &GRPCProvider{ @@ -581,6 +686,51 @@ func TestGRPCProvider_ReadResourceJSON(t *testing.T) { } } +func TestGRPCProvider_ReadResourceReturnsWriteOnlyValue(t *testing.T) { + client := mockProviderClientWithSchema(t, mutateSchemaResponse(providerProtoSchema(), addAttributeToResource("resource", &proto.Schema_Attribute{ + Name: "write_only_attr", + Type: []byte(`"string"`), + Optional: true, + WriteOnly: true, + }))) + p := &GRPCProvider{ + client: client, + } + + client.EXPECT().ReadResource( + gomock.Any(), + gomock.Any(), + ).DoAndReturn(func(_ context.Context, _ *proto.ReadResource_Request, opts ...grpc.CallOption) (*proto.ReadResource_Response, error) { + b, err := msgpack.Marshal( + cty.ObjectVal(map[string]cty.Value{"attr": cty.StringVal("bar"), "write_only_attr": cty.StringVal("val")}), + cty.Object(map[string]cty.Type{"attr": cty.String, "write_only_attr": cty.String}), + ) + if err != nil { + return nil, err + } + return &proto.ReadResource_Response{ + NewState: &proto.DynamicValue{ + Msgpack: b, + }, + }, nil + }) + + resp := p.ReadResource(t.Context(), providers.ReadResourceRequest{ + TypeName: "resource", + PriorState: cty.ObjectVal(map[string]cty.Value{ + "attr": cty.StringVal("foo"), + "write_only_attr": cty.NullVal(cty.String), + }), + }) + + checkDiagsHasError(t, resp.Diagnostics) + + expectedErr := `Resource type "resource" returned an actual value for the write-only attribute ".write_only_attr" while it is meant to be nil. This is an issue in the provider SDK.` + if gotErr := resp.Diagnostics[0].Description().Detail; expectedErr != gotErr { + t.Errorf("the expected error is not the same with the one returned.\nexpected: %s\ngot: %s", expectedErr, gotErr) + } +} + func TestGRPCProvider_ReadEmptyJSON(t *testing.T) { client := mockProviderClient(t) p := &GRPCProvider{ @@ -785,6 +935,56 @@ func TestGRPCProvider_PlanResourceChangeJSON(t *testing.T) { } } +func TestGRPCProvider_PlanResourceChangeReturnsWriteOnlyValue(t *testing.T) { + client := mockProviderClientWithSchema(t, mutateSchemaResponse(providerProtoSchema(), addAttributeToResource("resource", &proto.Schema_Attribute{ + Name: "write_only_attr", + Type: []byte(`"string"`), + Optional: true, + WriteOnly: true, + }))) + p := &GRPCProvider{ + client: client, + } + + client.EXPECT().PlanResourceChange( + gomock.Any(), + gomock.Any(), + ).DoAndReturn(func(_ context.Context, _ *proto.PlanResourceChange_Request, opts ...grpc.CallOption) (*proto.PlanResourceChange_Response, error) { + b, err := msgpack.Marshal( + cty.ObjectVal(map[string]cty.Value{"attr": cty.StringVal("bar"), "write_only_attr": cty.StringVal("val")}), + cty.Object(map[string]cty.Type{"attr": cty.String, "write_only_attr": cty.String}), + ) + if err != nil { + return nil, err + } + return &proto.PlanResourceChange_Response{ + PlannedState: &proto.DynamicValue{ + Msgpack: b, + }, + }, nil + }) + resp := p.PlanResourceChange(t.Context(), providers.PlanResourceChangeRequest{ + TypeName: "resource", + PriorState: cty.ObjectVal(map[string]cty.Value{ + "attr": cty.StringVal("foo"), + "write_only_attr": cty.NullVal(cty.String), + }), + ProposedNewState: cty.ObjectVal(map[string]cty.Value{ + "attr": cty.StringVal("bar"), + "write_only_attr": cty.NullVal(cty.String), + }), + Config: cty.ObjectVal(map[string]cty.Value{ + "attr": cty.StringVal("bar"), + "write_only_attr": cty.NullVal(cty.String), + }), + }) + checkDiagsHasError(t, resp.Diagnostics) + expectedErr := `Resource type "resource" returned an actual value for the write-only attribute ".write_only_attr" while it is meant to be nil. This is an issue in the provider SDK.` + if gotErr := resp.Diagnostics[0].Description().Detail; expectedErr != gotErr { + t.Errorf("the expected error is not the same with the one returned.\nexpected: %s\ngot: %s", expectedErr, gotErr) + } +} + func TestGRPCProvider_ApplyResourceChange(t *testing.T) { client := mockProviderClient(t) p := &GRPCProvider{ @@ -831,6 +1031,7 @@ func TestGRPCProvider_ApplyResourceChange(t *testing.T) { t.Fatalf("expected %q, got %q", expectedPrivate, resp.Private) } } + func TestGRPCProvider_ApplyResourceChangeJSON(t *testing.T) { client := mockProviderClient(t) p := &GRPCProvider{ @@ -878,6 +1079,58 @@ func TestGRPCProvider_ApplyResourceChangeJSON(t *testing.T) { } } +func TestGRPCProvider_ApplyResourceChangeReturnsWriteOnlyValue(t *testing.T) { + client := mockProviderClientWithSchema(t, mutateSchemaResponse(providerProtoSchema(), addAttributeToResource("resource", &proto.Schema_Attribute{ + Name: "write_only_attr", + Type: []byte(`"string"`), + Optional: true, + WriteOnly: true, + }))) + p := &GRPCProvider{ + client: client, + } + + client.EXPECT().ApplyResourceChange( + gomock.Any(), + gomock.Any(), + ).DoAndReturn(func(_ context.Context, _ *proto.ApplyResourceChange_Request, opts ...grpc.CallOption) (*proto.ApplyResourceChange_Response, error) { + b, err := msgpack.Marshal( + cty.ObjectVal(map[string]cty.Value{"attr": cty.StringVal("bar"), "write_only_attr": cty.StringVal("val")}), + cty.Object(map[string]cty.Type{"attr": cty.String, "write_only_attr": cty.String}), + ) + if err != nil { + return nil, err + } + return &proto.ApplyResourceChange_Response{ + NewState: &proto.DynamicValue{ + Msgpack: b, + }, + }, nil + }) + + resp := p.ApplyResourceChange(t.Context(), providers.ApplyResourceChangeRequest{ + TypeName: "resource", + PriorState: cty.ObjectVal(map[string]cty.Value{ + "attr": cty.StringVal("foo"), + "write_only_attr": cty.NullVal(cty.String), + }), + PlannedState: cty.ObjectVal(map[string]cty.Value{ + "attr": cty.StringVal("bar"), + "write_only_attr": cty.NullVal(cty.String), + }), + Config: cty.ObjectVal(map[string]cty.Value{ + "attr": cty.StringVal("bar"), + "write_only_attr": cty.StringVal("foo"), + }), + }) + checkDiagsHasError(t, resp.Diagnostics) + + expectedErr := `Resource type "resource" returned an actual value for the write-only attribute ".write_only_attr" while it is meant to be nil. This is an issue in the provider SDK.` + if gotErr := resp.Diagnostics[0].Description().Detail; expectedErr != gotErr { + t.Errorf("the expected error is not the same with the one returned.\nexpected: %s\ngot: %s", expectedErr, gotErr) + } +} + func TestGRPCProvider_ImportResourceState(t *testing.T) { client := mockProviderClient(t) p := &GRPCProvider{ diff --git a/internal/plugin/validation/write_only.go b/internal/plugin/validation/write_only.go new file mode 100644 index 0000000000..b35e9f4053 --- /dev/null +++ b/internal/plugin/validation/write_only.go @@ -0,0 +1,48 @@ +// Copyright (c) The OpenTofu Authors +// SPDX-License-Identifier: MPL-2.0 +// Copyright (c) 2023 HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package validation + +import ( + "fmt" + "log" + + "github.com/hashicorp/hcl/v2" + "github.com/opentofu/opentofu/internal/configs/configschema" + "github.com/opentofu/opentofu/internal/tfdiags" + "github.com/zclconf/go-cty/cty" +) + +// WriteOnlyAttributes checks that the write-only attributes are not returned back with an actual value. +// This particular validation does not require to return the diags right away, but we can leave the +// flow move on. +// The diagnostics generated by this validation ensure that the provider works correctly +// and there is no issue in the provider SDK when it comes to the write-only attributes. +// Returning those with actual values can create unknown behavior leading to possible confidential +// information exposure. +func WriteOnlyAttributes(schema *configschema.Block, v cty.Value, resAddr string) (diags tfdiags.Diagnostics) { + if !schema.ContainsWriteOnly() { + return diags + } + paths := schema.WriteOnlyPaths(v, nil) + for _, path := range paths { + pathAsString := tfdiags.FormatCtyPath(path) + + pathVal, err := path.Apply(v) + if err != nil { + log.Printf("[WARN] Error when tried to get the path (%s) value from the given object: %s", pathAsString, err) + continue + } + if pathVal.IsNull() { + continue + } + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid provider response", + Detail: fmt.Sprintf("Resource type %q returned an actual value for the write-only attribute %q while it is meant to be nil. This is an issue in the provider SDK.", resAddr, pathAsString), + }) + } + return diags +} diff --git a/internal/plugin6/convert/schema.go b/internal/plugin6/convert/schema.go index 3ca95e9902..452d93a257 100644 --- a/internal/plugin6/convert/schema.go +++ b/internal/plugin6/convert/schema.go @@ -289,6 +289,7 @@ func configschemaObjectToProto(b *configschema.Object) *proto.Schema_Object { Computed: a.Computed, Required: a.Required, Sensitive: a.Sensitive, + WriteOnly: a.WriteOnly, Deprecated: a.Deprecated, } diff --git a/internal/plugin6/convert/schema_test.go b/internal/plugin6/convert/schema_test.go index 4746a8a3c0..236842d070 100644 --- a/internal/plugin6/convert/schema_test.go +++ b/internal/plugin6/convert/schema_test.go @@ -51,6 +51,11 @@ func TestConvertSchemaBlocks(t *testing.T) { Type: []byte(`"number"`), Required: true, }, + { + Name: "write_only", + Type: []byte(`"number"`), + WriteOnly: true, + }, { Name: "nested_type", NestedType: &proto.Schema_Object{ @@ -77,6 +82,11 @@ func TestConvertSchemaBlocks(t *testing.T) { Type: []byte(`"number"`), Required: true, }, + { + Name: "write_only", + Type: []byte(`"number"`), + WriteOnly: true, + }, }, }, Required: true, @@ -112,6 +122,11 @@ func TestConvertSchemaBlocks(t *testing.T) { Type: []byte(`"number"`), Required: true, }, + { + Name: "write_only", + Type: []byte(`"number"`), + WriteOnly: true, + }, }, }, Computed: true, @@ -130,6 +145,11 @@ func TestConvertSchemaBlocks(t *testing.T) { Type: []byte(`"string"`), Computed: true, }, + { + Name: "write_only", + Type: []byte(`"string"`), + WriteOnly: true, + }, }, }, Required: true, @@ -144,6 +164,13 @@ func TestConvertSchemaBlocks(t *testing.T) { Type: []byte(`"string"`), Computed: true, }, + { + // Even though the set types do not accept write-only attributes, we want + // to test the generic convertion of this. + Name: "write_only", + Type: []byte(`"string"`), + WriteOnly: true, + }, }, }, Required: true, @@ -158,6 +185,11 @@ func TestConvertSchemaBlocks(t *testing.T) { Type: []byte(`"string"`), Computed: true, }, + { + Name: "write_only", + Type: []byte(`"string"`), + WriteOnly: true, + }, }, }, Required: true, @@ -183,6 +215,10 @@ func TestConvertSchemaBlocks(t *testing.T) { Type: cty.Number, Required: true, }, + "write_only": { + Type: cty.Number, + WriteOnly: true, + }, "nested_type": { NestedType: &configschema.Object{ Attributes: map[string]*configschema.Attribute{ @@ -203,6 +239,10 @@ func TestConvertSchemaBlocks(t *testing.T) { Type: cty.Number, Required: true, }, + "write_only": { + Type: cty.Number, + WriteOnly: true, + }, }, Nesting: configschema.NestingSingle, }, @@ -232,6 +272,10 @@ func TestConvertSchemaBlocks(t *testing.T) { Type: cty.Number, Required: true, }, + "write_only": { + Type: cty.Number, + WriteOnly: true, + }, }, }, Computed: true, @@ -249,6 +293,10 @@ func TestConvertSchemaBlocks(t *testing.T) { Type: cty.String, Computed: true, }, + "write_only": { + Type: cty.String, + WriteOnly: true, + }, }, }, Required: true, @@ -261,6 +309,10 @@ func TestConvertSchemaBlocks(t *testing.T) { Type: cty.String, Computed: true, }, + "write_only": { + Type: cty.String, + WriteOnly: true, + }, }, }, Required: true, @@ -273,6 +325,10 @@ func TestConvertSchemaBlocks(t *testing.T) { Type: cty.String, Computed: true, }, + "write_only": { + Type: cty.String, + WriteOnly: true, + }, }, }, Required: true, @@ -308,6 +364,11 @@ func TestConvertSchemaBlocks(t *testing.T) { Type: []byte(`"dynamic"`), Required: true, }, + { + Name: "write_only", + Type: []byte(`"dynamic"`), + WriteOnly: true, + }, }, }, }, @@ -332,6 +393,10 @@ func TestConvertSchemaBlocks(t *testing.T) { Type: cty.DynamicPseudoType, Required: true, }, + "write_only": { + Type: cty.DynamicPseudoType, + WriteOnly: true, + }, }, }, }, @@ -428,6 +493,11 @@ func TestConvertProtoSchemaBlocks(t *testing.T) { Type: []byte(`"number"`), Required: true, }, + { + Name: "write_only", + Type: []byte(`"number"`), + WriteOnly: true, + }, }, }, &configschema.Block{ @@ -449,6 +519,10 @@ func TestConvertProtoSchemaBlocks(t *testing.T) { Type: cty.Number, Required: true, }, + "write_only": { + Type: cty.Number, + WriteOnly: true, + }, }, }, }, @@ -480,6 +554,11 @@ func TestConvertProtoSchemaBlocks(t *testing.T) { Type: []byte(`"dynamic"`), Required: true, }, + { + Name: "write_only", + Type: []byte(`"dynamic"`), + WriteOnly: true, + }, }, }, }, @@ -504,6 +583,10 @@ func TestConvertProtoSchemaBlocks(t *testing.T) { Type: cty.DynamicPseudoType, Required: true, }, + "write_only": { + Type: cty.DynamicPseudoType, + WriteOnly: true, + }, }, }, }, diff --git a/internal/plugin6/grpc_provider.go b/internal/plugin6/grpc_provider.go index bb0bdcb406..1b0c7c7e22 100644 --- a/internal/plugin6/grpc_provider.go +++ b/internal/plugin6/grpc_provider.go @@ -12,6 +12,7 @@ import ( "sync" plugin "github.com/hashicorp/go-plugin" + "github.com/opentofu/opentofu/internal/plugin6/validation" "github.com/zclconf/go-cty/cty" ctyjson "github.com/zclconf/go-cty/cty/json" "github.com/zclconf/go-cty/cty/msgpack" @@ -40,7 +41,12 @@ var clientCapabilities = &proto6.ClientCapabilities{ // satisfy the request. Setting this means that we need to be prepared // for there to be a "deferred" object in the response from various // other provider RPC functions. - DeferralAllowed: true, + DeferralAllowed: true, + // WriteOnlyAttributesAllowed indicates that the current system version + // supports write-only attributes. + // This enables the SDK to run specific validations and enable the + // nullification of such configured attributes before returning the + // response back to the system. WriteOnlyAttributesAllowed: true, } @@ -385,6 +391,8 @@ func (p *GRPCProvider) UpgradeResourceState(ctx context.Context, r providers.Upg } resp.UpgradedState = state + resp.Diagnostics = resp.Diagnostics.Append(validation.WriteOnlyAttributes(resSchema.Block, resp.UpgradedState, r.TypeName)) + return resp } @@ -498,6 +506,8 @@ func (p *GRPCProvider) ReadResource(ctx context.Context, r providers.ReadResourc resp.NewState = state resp.Private = protoResp.Private + resp.Diagnostics = resp.Diagnostics.Append(validation.WriteOnlyAttributes(resSchema.Block, resp.NewState, r.TypeName)) + return resp } @@ -590,6 +600,8 @@ func (p *GRPCProvider) PlanResourceChange(ctx context.Context, r providers.PlanR resp.LegacyTypeSystem = protoResp.LegacyTypeSystem + resp.Diagnostics = resp.Diagnostics.Append(validation.WriteOnlyAttributes(resSchema.Block, resp.PlannedState, r.TypeName)) + return resp } @@ -668,6 +680,8 @@ func (p *GRPCProvider) ApplyResourceChange(ctx context.Context, r providers.Appl resp.LegacyTypeSystem = protoResp.LegacyTypeSystem + resp.Diagnostics = resp.Diagnostics.Append(validation.WriteOnlyAttributes(resSchema.Block, resp.NewState, r.TypeName)) + return resp } @@ -765,6 +779,8 @@ func (p *GRPCProvider) MoveResourceState(ctx context.Context, r providers.MoveRe resp.TargetState = state resp.TargetPrivate = protoResp.TargetPrivate + resp.Diagnostics = resp.Diagnostics.Append(validation.WriteOnlyAttributes(resourceSchema.Block, resp.TargetState, r.TargetTypeName)) + return resp } diff --git a/internal/plugin6/grpc_provider_test.go b/internal/plugin6/grpc_provider_test.go index 1934f6a678..e5e4ef8427 100644 --- a/internal/plugin6/grpc_provider_test.go +++ b/internal/plugin6/grpc_provider_test.go @@ -7,6 +7,7 @@ package plugin6 import ( "bytes" + "context" "fmt" "slices" "strings" @@ -17,7 +18,9 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" "github.com/zclconf/go-cty/cty" + "github.com/zclconf/go-cty/cty/msgpack" "go.uber.org/mock/gomock" + "google.golang.org/grpc" "google.golang.org/protobuf/types/known/timestamppb" "github.com/opentofu/opentofu/internal/addrs" @@ -36,7 +39,24 @@ var ( valueComparer = cmp.Comparer(cty.Value.RawEquals) ) +func mutateSchemaResponse(response *proto.GetProviderSchema_Response, mut ...func(schemaResponse *proto.GetProviderSchema_Response)) *proto.GetProviderSchema_Response { + for _, f := range mut { + f(response) + } + return response +} + +func addAttributeToResource(resourceName string, attr *proto.Schema_Attribute) func(response *proto.GetProviderSchema_Response) { + return func(schemaResponse *proto.GetProviderSchema_Response) { + schemaResponse.ResourceSchemas[resourceName].Block.Attributes = append(schemaResponse.ResourceSchemas[resourceName].Block.Attributes, attr) + } +} + func mockProviderClient(t *testing.T) *mockproto.MockProviderClient { + return mockProviderClientWithSchema(t, providerProtoSchema()) +} + +func mockProviderClientWithSchema(t *testing.T, schema *proto.GetProviderSchema_Response) *mockproto.MockProviderClient { ctrl := gomock.NewController(t) client := mockproto.NewMockProviderClient(ctrl) @@ -45,7 +65,7 @@ func mockProviderClient(t *testing.T) *mockproto.MockProviderClient { gomock.Any(), gomock.Any(), gomock.Any(), - ).Return(providerProtoSchema(), nil) + ).Return(schema, nil) return client } @@ -449,6 +469,48 @@ func TestGRPCProvider_UpgradeResourceStateJSON(t *testing.T) { } } +func TestGRPCProvider_UpgradeResourceStateWithWriteOnlyReturned(t *testing.T) { + client := mockProviderClientWithSchema(t, mutateSchemaResponse(providerProtoSchema(), addAttributeToResource("resource", &proto.Schema_Attribute{ + Name: "write_only_attr", + Type: []byte(`"string"`), + Optional: true, + WriteOnly: true, + }))) + p := &GRPCProvider{ + client: client, + } + + client.EXPECT().UpgradeResourceState( + gomock.Any(), + gomock.Any(), + ).DoAndReturn(func(_ context.Context, _ *proto.UpgradeResourceState_Request, _ ...grpc.CallOption) (*proto.UpgradeResourceState_Response, error) { + b, err := msgpack.Marshal( + cty.ObjectVal(map[string]cty.Value{"attr": cty.StringVal("bar"), "write_only_attr": cty.StringVal("val")}), + cty.Object(map[string]cty.Type{"attr": cty.String, "write_only_attr": cty.String}), + ) + if err != nil { + return nil, err + } + return &proto.UpgradeResourceState_Response{ + UpgradedState: &proto.DynamicValue{ + Msgpack: b, + }, + }, nil + }) + + resp := p.UpgradeResourceState(t.Context(), providers.UpgradeResourceStateRequest{ + TypeName: "resource", + Version: 0, + RawStateJSON: []byte(`{"attr":"bar"}`), + }) + checkDiagsHasError(t, resp.Diagnostics) + + expectedErr := `Resource type "resource" returned an actual value for the write-only attribute ".write_only_attr" while it is meant to be nil. This is an issue in the provider SDK.` + if gotErr := resp.Diagnostics[0].Description().Detail; expectedErr != gotErr { + t.Errorf("the expected error is not the same with the one returned.\nexpected: %s\ngot: %s", expectedErr, gotErr) + } +} + func TestGRPCProvider_MoveResourceState(t *testing.T) { client := mockProviderClient(t) p := &GRPCProvider{ @@ -484,6 +546,50 @@ func TestGRPCProvider_MoveResourceState(t *testing.T) { t.Fatalf("expected %q, got %q", expectedPrivate, resp.TargetPrivate) } } + +func TestGRPCProvider_MoveResourceStateReturnsWriteOnlyValue(t *testing.T) { + client := mockProviderClientWithSchema(t, mutateSchemaResponse(providerProtoSchema(), addAttributeToResource("resource", &proto.Schema_Attribute{ + Name: "write_only_attr", + Type: []byte(`"string"`), + Optional: true, + WriteOnly: true, + }))) + p := &GRPCProvider{ + client: client, + } + + client.EXPECT().MoveResourceState( + gomock.Any(), + gomock.Any(), + ).DoAndReturn(func(_ context.Context, _ *proto.MoveResourceState_Request, _ ...grpc.CallOption) (*proto.MoveResourceState_Response, error) { + b, err := msgpack.Marshal( + cty.ObjectVal(map[string]cty.Value{"attr": cty.StringVal("bar"), "write_only_attr": cty.StringVal("val")}), + cty.Object(map[string]cty.Type{"attr": cty.String, "write_only_attr": cty.String}), + ) + if err != nil { + return nil, err + } + + return &proto.MoveResourceState_Response{ + TargetState: &proto.DynamicValue{ + Msgpack: b, + }, + }, nil + }) + + resp := p.MoveResourceState(t.Context(), providers.MoveResourceStateRequest{ + SourceTypeName: "resource_old", + SourceSchemaVersion: 0, + TargetTypeName: "resource", + }) + checkDiagsHasError(t, resp.Diagnostics) + + expectedErr := `Resource type "resource" returned an actual value for the write-only attribute ".write_only_attr" while it is meant to be nil. This is an issue in the provider SDK.` + if gotErr := resp.Diagnostics[0].Description().Detail; expectedErr != gotErr { + t.Errorf("the expected error is not the same with the one returned.\nexpected: %s\ngot: %s", expectedErr, gotErr) + } +} + func TestGRPCProvider_Configure(t *testing.T) { client := mockProviderClient(t) p := &GRPCProvider{ @@ -587,6 +693,51 @@ func TestGRPCProvider_ReadResourceJSON(t *testing.T) { } } +func TestGRPCProvider_ReadResourceReturnsWriteOnlyValue(t *testing.T) { + client := mockProviderClientWithSchema(t, mutateSchemaResponse(providerProtoSchema(), addAttributeToResource("resource", &proto.Schema_Attribute{ + Name: "write_only_attr", + Type: []byte(`"string"`), + Optional: true, + WriteOnly: true, + }))) + p := &GRPCProvider{ + client: client, + } + + client.EXPECT().ReadResource( + gomock.Any(), + gomock.Any(), + ).DoAndReturn(func(_ context.Context, _ *proto.ReadResource_Request, opts ...grpc.CallOption) (*proto.ReadResource_Response, error) { + b, err := msgpack.Marshal( + cty.ObjectVal(map[string]cty.Value{"attr": cty.StringVal("bar"), "write_only_attr": cty.StringVal("val")}), + cty.Object(map[string]cty.Type{"attr": cty.String, "write_only_attr": cty.String}), + ) + if err != nil { + return nil, err + } + return &proto.ReadResource_Response{ + NewState: &proto.DynamicValue{ + Msgpack: b, + }, + }, nil + }) + + resp := p.ReadResource(t.Context(), providers.ReadResourceRequest{ + TypeName: "resource", + PriorState: cty.ObjectVal(map[string]cty.Value{ + "attr": cty.StringVal("foo"), + "write_only_attr": cty.NullVal(cty.String), + }), + }) + + checkDiagsHasError(t, resp.Diagnostics) + + expectedErr := `Resource type "resource" returned an actual value for the write-only attribute ".write_only_attr" while it is meant to be nil. This is an issue in the provider SDK.` + if gotErr := resp.Diagnostics[0].Description().Detail; expectedErr != gotErr { + t.Errorf("the expected error is not the same with the one returned.\nexpected: %s\ngot: %s", expectedErr, gotErr) + } +} + func TestGRPCProvider_ReadEmptyJSON(t *testing.T) { client := mockProviderClient(t) p := &GRPCProvider{ @@ -791,6 +942,56 @@ func TestGRPCProvider_PlanResourceChangeJSON(t *testing.T) { } } +func TestGRPCProvider_PlanResourceChangeReturnsWriteOnlyValue(t *testing.T) { + client := mockProviderClientWithSchema(t, mutateSchemaResponse(providerProtoSchema(), addAttributeToResource("resource", &proto.Schema_Attribute{ + Name: "write_only_attr", + Type: []byte(`"string"`), + Optional: true, + WriteOnly: true, + }))) + p := &GRPCProvider{ + client: client, + } + + client.EXPECT().PlanResourceChange( + gomock.Any(), + gomock.Any(), + ).DoAndReturn(func(_ context.Context, _ *proto.PlanResourceChange_Request, opts ...grpc.CallOption) (*proto.PlanResourceChange_Response, error) { + b, err := msgpack.Marshal( + cty.ObjectVal(map[string]cty.Value{"attr": cty.StringVal("bar"), "write_only_attr": cty.StringVal("val")}), + cty.Object(map[string]cty.Type{"attr": cty.String, "write_only_attr": cty.String}), + ) + if err != nil { + return nil, err + } + return &proto.PlanResourceChange_Response{ + PlannedState: &proto.DynamicValue{ + Msgpack: b, + }, + }, nil + }) + resp := p.PlanResourceChange(t.Context(), providers.PlanResourceChangeRequest{ + TypeName: "resource", + PriorState: cty.ObjectVal(map[string]cty.Value{ + "attr": cty.StringVal("foo"), + "write_only_attr": cty.NullVal(cty.String), + }), + ProposedNewState: cty.ObjectVal(map[string]cty.Value{ + "attr": cty.StringVal("bar"), + "write_only_attr": cty.NullVal(cty.String), + }), + Config: cty.ObjectVal(map[string]cty.Value{ + "attr": cty.StringVal("bar"), + "write_only_attr": cty.NullVal(cty.String), + }), + }) + checkDiagsHasError(t, resp.Diagnostics) + expectedErr := `Resource type "resource" returned an actual value for the write-only attribute ".write_only_attr" while it is meant to be nil. This is an issue in the provider SDK.` + if gotErr := resp.Diagnostics[0].Description().Detail; expectedErr != gotErr { + t.Errorf("the expected error is not the same with the one returned.\nexpected: %s\ngot: %s", expectedErr, gotErr) + } +} + func TestGRPCProvider_ApplyResourceChange(t *testing.T) { client := mockProviderClient(t) p := &GRPCProvider{ @@ -837,6 +1038,7 @@ func TestGRPCProvider_ApplyResourceChange(t *testing.T) { t.Fatalf("expected %q, got %q", expectedPrivate, resp.Private) } } + func TestGRPCProvider_ApplyResourceChangeJSON(t *testing.T) { client := mockProviderClient(t) p := &GRPCProvider{ @@ -884,6 +1086,58 @@ func TestGRPCProvider_ApplyResourceChangeJSON(t *testing.T) { } } +func TestGRPCProvider_ApplyResourceChangeReturnsWriteOnlyValue(t *testing.T) { + client := mockProviderClientWithSchema(t, mutateSchemaResponse(providerProtoSchema(), addAttributeToResource("resource", &proto.Schema_Attribute{ + Name: "write_only_attr", + Type: []byte(`"string"`), + Optional: true, + WriteOnly: true, + }))) + p := &GRPCProvider{ + client: client, + } + + client.EXPECT().ApplyResourceChange( + gomock.Any(), + gomock.Any(), + ).DoAndReturn(func(_ context.Context, _ *proto.ApplyResourceChange_Request, opts ...grpc.CallOption) (*proto.ApplyResourceChange_Response, error) { + b, err := msgpack.Marshal( + cty.ObjectVal(map[string]cty.Value{"attr": cty.StringVal("bar"), "write_only_attr": cty.StringVal("val")}), + cty.Object(map[string]cty.Type{"attr": cty.String, "write_only_attr": cty.String}), + ) + if err != nil { + return nil, err + } + return &proto.ApplyResourceChange_Response{ + NewState: &proto.DynamicValue{ + Msgpack: b, + }, + }, nil + }) + + resp := p.ApplyResourceChange(t.Context(), providers.ApplyResourceChangeRequest{ + TypeName: "resource", + PriorState: cty.ObjectVal(map[string]cty.Value{ + "attr": cty.StringVal("foo"), + "write_only_attr": cty.NullVal(cty.String), + }), + PlannedState: cty.ObjectVal(map[string]cty.Value{ + "attr": cty.StringVal("bar"), + "write_only_attr": cty.NullVal(cty.String), + }), + Config: cty.ObjectVal(map[string]cty.Value{ + "attr": cty.StringVal("bar"), + "write_only_attr": cty.StringVal("foo"), + }), + }) + checkDiagsHasError(t, resp.Diagnostics) + + expectedErr := `Resource type "resource" returned an actual value for the write-only attribute ".write_only_attr" while it is meant to be nil. This is an issue in the provider SDK.` + if gotErr := resp.Diagnostics[0].Description().Detail; expectedErr != gotErr { + t.Errorf("the expected error is not the same with the one returned.\nexpected: %s\ngot: %s", expectedErr, gotErr) + } +} + func TestGRPCProvider_ImportResourceState(t *testing.T) { client := mockProviderClient(t) p := &GRPCProvider{ diff --git a/internal/plugin6/validation/write_only.go b/internal/plugin6/validation/write_only.go new file mode 100644 index 0000000000..b35e9f4053 --- /dev/null +++ b/internal/plugin6/validation/write_only.go @@ -0,0 +1,48 @@ +// Copyright (c) The OpenTofu Authors +// SPDX-License-Identifier: MPL-2.0 +// Copyright (c) 2023 HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package validation + +import ( + "fmt" + "log" + + "github.com/hashicorp/hcl/v2" + "github.com/opentofu/opentofu/internal/configs/configschema" + "github.com/opentofu/opentofu/internal/tfdiags" + "github.com/zclconf/go-cty/cty" +) + +// WriteOnlyAttributes checks that the write-only attributes are not returned back with an actual value. +// This particular validation does not require to return the diags right away, but we can leave the +// flow move on. +// The diagnostics generated by this validation ensure that the provider works correctly +// and there is no issue in the provider SDK when it comes to the write-only attributes. +// Returning those with actual values can create unknown behavior leading to possible confidential +// information exposure. +func WriteOnlyAttributes(schema *configschema.Block, v cty.Value, resAddr string) (diags tfdiags.Diagnostics) { + if !schema.ContainsWriteOnly() { + return diags + } + paths := schema.WriteOnlyPaths(v, nil) + for _, path := range paths { + pathAsString := tfdiags.FormatCtyPath(path) + + pathVal, err := path.Apply(v) + if err != nil { + log.Printf("[WARN] Error when tried to get the path (%s) value from the given object: %s", pathAsString, err) + continue + } + if pathVal.IsNull() { + continue + } + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid provider response", + Detail: fmt.Sprintf("Resource type %q returned an actual value for the write-only attribute %q while it is meant to be nil. This is an issue in the provider SDK.", resAddr, pathAsString), + }) + } + return diags +} diff --git a/internal/provider-simple-v6/provider.go b/internal/provider-simple-v6/provider.go index e9bbb496ff..c8e43efedc 100644 --- a/internal/provider-simple-v6/provider.go +++ b/internal/provider-simple-v6/provider.go @@ -39,7 +39,7 @@ func Provider() providers.Interface { }, } // Only managed resource should have write-only arguments. - withWriteOnlyAttribute := func(s providers.Schema) providers.Schema { + withWriteOnlyBlocks := func(s providers.Schema) providers.Schema { b := *s.Block b.Attributes["value_wo"] = &configschema.Attribute{ @@ -47,6 +47,20 @@ func Provider() providers.Interface { Type: cty.String, WriteOnly: true, } + b.BlockTypes = map[string]*configschema.NestedBlock{ + "nested_block": { + Nesting: configschema.NestingSingle, + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "nested_block_attr": { + Type: cty.String, + Optional: true, + WriteOnly: true, + }, + }, + }, + }, + } return providers.Schema{Block: &b} } @@ -70,7 +84,7 @@ func Provider() providers.Interface { }, }, ResourceTypes: map[string]providers.Schema{ - "simple_resource": withWriteOnlyAttribute(simpleResource), + "simple_resource": withWriteOnlyBlocks(simpleResource), }, DataSources: map[string]providers.Schema{ "simple_resource": simpleResource, @@ -155,13 +169,6 @@ func (s simple) PlanResourceChange(_ context.Context, req providers.PlanResource if !ok { m["id"] = cty.UnknownVal(cty.String) } - // TODO ephemeral - remove this line after work will be done on write-only arguments. - // The problem now is that the value sent to ApplyResourceChange is always null as returned by the plan call. - // When the work on write-only arguments will be done, OpenTofu should send the actual value to - // the ApplyResourceChange too. - // To confirm that everything is ok, by removing this "waitIfRequested" call from here, theTestEphemeralWorkflowAndOutput - // should still work correctly without any warn logs in the test output - waitIfRequested(m) // Simulate what the terraform-plugin-go should do. Nullify the write-only attributes. m["value_wo"] = cty.NullVal(cty.String) @@ -186,8 +193,8 @@ func (s simple) ApplyResourceChange(_ context.Context, req providers.ApplyResour if !ok { m["id"] = cty.StringVal(time.Now().String()) } - waitIfRequested(m) - + waitIfRequested(req.Config.AsValueMap()) + // Simulate what the terraform-plugin-go should do. Nullify the write-only attributes. m["value_wo"] = cty.NullVal(cty.String) resp.NewState = cty.ObjectVal(m) diff --git a/internal/provider-simple/provider.go b/internal/provider-simple/provider.go index 11a6527158..8929c29c47 100644 --- a/internal/provider-simple/provider.go +++ b/internal/provider-simple/provider.go @@ -38,9 +38,23 @@ func Provider() providers.Interface { }, } // Only managed resource should have write-only arguments. - withWriteOnlyAttribute := func(s providers.Schema) providers.Schema { + withWriteOnlyBlocks := func(s providers.Schema) providers.Schema { b := *s.Block + b.BlockTypes = map[string]*configschema.NestedBlock{ + "nested_block": { + Nesting: configschema.NestingSingle, + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "nested_block_attr": { + Type: cty.String, + Optional: true, + WriteOnly: true, + }, + }, + }, + }, + } b.Attributes["value_wo"] = &configschema.Attribute{ Optional: true, Type: cty.String, @@ -69,7 +83,7 @@ func Provider() providers.Interface { }, }, ResourceTypes: map[string]providers.Schema{ - "simple_resource": withWriteOnlyAttribute(simpleResource), + "simple_resource": withWriteOnlyBlocks(simpleResource), }, DataSources: map[string]providers.Schema{ "simple_resource": simpleResource, @@ -152,13 +166,6 @@ func (s simple) PlanResourceChange(_ context.Context, req providers.PlanResource m["id"] = cty.UnknownVal(cty.String) } - // TODO ephemeral - remove this line after work will be done on write-only arguments. - // The problem now is that the value sent to ApplyResourceChange is always null as returned by the plan call. - // When the work on write-only arguments will be done, OpenTofu should send the actual value to - // the ApplyResourceChange too. - // To confirm that everything is ok, by removing this "waitIfRequested" call from here, theTestEphemeralWorkflowAndOutput - // should still work correctly without any warn logs in the test output - waitIfRequested(m) // Simulate what the terraform-plugin-go should do. Nullify the write-only attributes. m["value_wo"] = cty.NullVal(cty.String) @@ -177,7 +184,7 @@ func (s simple) ApplyResourceChange(_ context.Context, req providers.ApplyResour if !ok { m["id"] = cty.StringVal(time.Now().String()) } - waitIfRequested(m) + waitIfRequested(req.Config.AsValueMap()) // Simulate what the terraform-plugin-go should do. Nullify the write-only attributes. m["value_wo"] = cty.NullVal(cty.String) diff --git a/internal/tofu/context_plan2_test.go b/internal/tofu/context_plan2_test.go index 731aea28ac..f2896c5aa6 100644 --- a/internal/tofu/context_plan2_test.go +++ b/internal/tofu/context_plan2_test.go @@ -8773,10 +8773,10 @@ ephemeral "test_ephemeral_resource" "a" { } } -// TestContext2Apply_ephemeralResourcesLifecycleCheck checks that the +// TestContext2Plan_ephemeralVariablesInPlan checks that the // ephemeral variables get configured correctly in the plan // to be used later to exclude values from being written into the plan object. -func TestContext2Apply_ephemeralVariablesInPlan(t *testing.T) { +func TestContext2Plan_ephemeralVariablesInPlan(t *testing.T) { m := testModuleInline(t, map[string]string{ `main.tf`: ` variable "regular_var" { diff --git a/internal/tofu/node_resource_abstract_instance.go b/internal/tofu/node_resource_abstract_instance.go index baece69134..34b128891b 100644 --- a/internal/tofu/node_resource_abstract_instance.go +++ b/internal/tofu/node_resource_abstract_instance.go @@ -1222,6 +1222,18 @@ func (n *NodeAbstractResourceInstance) plan( eqV := unmarkedPlannedChangedVal.Equals(priorChangedVal) if !eqV.IsKnown() || eqV.False() { reqRep.Add(path) + // we continue here to avoid the lookup for the attribute on the next section + continue + } + + // If a write-only requests the replacement of the resource, we add that to the + // reqRep just because it's write-only. + // Needed because there is no way to apply the path based on the equivalence + // of the before/after values of this, since both are meant to always be null. + schemaAttr := schema.AttributeByPath(path) + isWo := schemaAttr != nil && schemaAttr.WriteOnly + if isWo { + reqRep.Add(path) } } if diags.HasErrors() { @@ -1254,12 +1266,7 @@ func (n *NodeAbstractResourceInstance) plan( var action plans.Action var actionReason plans.ResourceInstanceChangeActionReason - switch { - case priorVal.IsNull(): - action = plans.Create - case eq && !matchedForceReplace: - action = plans.NoOp - case matchedForceReplace || !reqRep.Empty(): + replaceResAction := func() { // If the user "forced replace" of this instance of if there are any // "requires replace" paths left _after our filtering above_ then this // is a replace action. @@ -1274,6 +1281,16 @@ func (n *NodeAbstractResourceInstance) plan( case !reqRep.Empty(): actionReason = plans.ResourceInstanceReplaceBecauseCannotUpdate } + } + switch { + case priorVal.IsNull(): + action = plans.Create + case schema.PathSetContainsWriteOnly(unmarkedPlannedNewVal, reqRep): + replaceResAction() + case eq && !matchedForceReplace: + action = plans.NoOp + case matchedForceReplace || !reqRep.Empty(): + replaceResAction() default: action = plans.Update // "Delete" is never chosen here, because deletion plans are always