diff --git a/.changes/v1.11/BUG FIXES-20250402-143931.yaml b/.changes/v1.11/BUG FIXES-20250402-143931.yaml new file mode 100644 index 0000000000..4971dd6277 --- /dev/null +++ b/.changes/v1.11/BUG FIXES-20250402-143931.yaml @@ -0,0 +1,5 @@ +kind: BUG FIXES +body: 'write-only attributes: internal providers should set write-only attributes to null' +time: 2025-04-02T14:39:31.672249+02:00 +custom: + Issue: "36824" diff --git a/internal/command/test_test.go b/internal/command/test_test.go index aa20ef51f0..aa819d1bb7 100644 --- a/internal/command/test_test.go +++ b/internal/command/test_test.go @@ -322,6 +322,18 @@ func TestTest_Runs(t *testing.T) { expectedErr: []string{"Cannot apply non-applyable plan"}, code: 1, }, + "write-only-attributes": { + expectedOut: []string{"1 passed, 0 failed."}, + code: 0, + }, + "write-only-attributes-mocked": { + expectedOut: []string{"1 passed, 0 failed."}, + code: 0, + }, + "write-only-attributes-overridden": { + expectedOut: []string{"1 passed, 0 failed."}, + code: 0, + }, } for name, tc := range tcs { t.Run(name, func(t *testing.T) { @@ -1618,6 +1630,7 @@ Terraform will perform the following actions: + destroy_fail = (known after apply) + id = "constant_value" + value = "bar" + + write_only = (write-only attribute) } Plan: 1 to add, 0 to change, 0 to destroy. @@ -1629,6 +1642,7 @@ resource "test_resource" "foo" { destroy_fail = false id = "constant_value" value = "bar" + write_only = (write-only attribute) } main.tftest.hcl... tearing down @@ -1951,6 +1965,7 @@ resource "test_resource" "module_resource" { destroy_fail = false id = "df6h8as9" value = "start" + write_only = (write-only attribute) } run "initial_apply"... pass @@ -1960,6 +1975,7 @@ resource "test_resource" "resource" { destroy_fail = false id = "598318e0" value = "start" + write_only = (write-only attribute) } run "plan_second_example"... pass @@ -1975,6 +1991,7 @@ Terraform will perform the following actions: + destroy_fail = (known after apply) + id = "b6a1d8cb" + value = "start" + + write_only = (write-only attribute) } Plan: 1 to add, 0 to change, 0 to destroy. @@ -1991,7 +2008,7 @@ Terraform will perform the following actions: ~ resource "test_resource" "resource" { id = "598318e0" ~ value = "start" -> "update" - # (1 unchanged attribute hidden) + # (2 unchanged attributes hidden) } Plan: 0 to add, 1 to change, 0 to destroy. @@ -2008,7 +2025,7 @@ Terraform will perform the following actions: ~ resource "test_resource" "module_resource" { id = "df6h8as9" ~ value = "start" -> "update" - # (1 unchanged attribute hidden) + # (2 unchanged attributes hidden) } Plan: 0 to add, 1 to change, 0 to destroy. @@ -2021,8 +2038,8 @@ Success! 5 passed, 0 failed. actual := output.All() - if !strings.Contains(actual, expected) { - t.Errorf("output didn't match expected:\nexpected:\n%s\nactual:\n%s", expected, actual) + if diff := cmp.Diff(expected, actual); diff != "" { + t.Errorf("output didn't match expected:\nexpected:\n%s\nactual:\n%s\ndiff:\n%s", expected, actual, diff) } if provider.ResourceCount() > 0 { @@ -2831,6 +2848,7 @@ resource "test_resource" "resource" { destroy_fail = false id = "9ddca5a9" value = (sensitive value) + write_only = (write-only attribute) } @@ -2845,6 +2863,7 @@ resource "test_resource" "resource" { destroy_fail = false id = "9ddca5a9" value = (sensitive value) + write_only = (write-only attribute) } diff --git a/internal/command/testdata/test/write-only-attributes-mocked/main.tf b/internal/command/testdata/test/write-only-attributes-mocked/main.tf new file mode 100644 index 0000000000..3f80d47a4a --- /dev/null +++ b/internal/command/testdata/test/write-only-attributes-mocked/main.tf @@ -0,0 +1,14 @@ + +variable "input" { + type = string +} + +data "test_data_source" "datasource" { + id = "resource" + write_only = var.input +} + +resource "test_resource" "resource" { + value = data.test_data_source.datasource.value + write_only = var.input +} diff --git a/internal/command/testdata/test/write-only-attributes-mocked/main.tftest.hcl b/internal/command/testdata/test/write-only-attributes-mocked/main.tftest.hcl new file mode 100644 index 0000000000..5efe8eabc1 --- /dev/null +++ b/internal/command/testdata/test/write-only-attributes-mocked/main.tftest.hcl @@ -0,0 +1,36 @@ + +mock_provider "test" { + mock_resource "test_resource" { + defaults = { + id = "resource" + } + } + + mock_data "test_data_source" { + defaults = { + value = "hello" + } + } +} + +run "test" { + variables { + input = "input" + } + + assert { + condition = data.test_data_source.datasource.value == "hello" + error_message = "wrong value" + } + + assert { + condition = test_resource.resource.value == "hello" + error_message = "wrong value" + } + + assert { + condition = test_resource.resource.id == "resource" + error_message = "wrong value" + } + +} diff --git a/internal/command/testdata/test/write-only-attributes-overridden/main.tf b/internal/command/testdata/test/write-only-attributes-overridden/main.tf new file mode 100644 index 0000000000..3f80d47a4a --- /dev/null +++ b/internal/command/testdata/test/write-only-attributes-overridden/main.tf @@ -0,0 +1,14 @@ + +variable "input" { + type = string +} + +data "test_data_source" "datasource" { + id = "resource" + write_only = var.input +} + +resource "test_resource" "resource" { + value = data.test_data_source.datasource.value + write_only = var.input +} diff --git a/internal/command/testdata/test/write-only-attributes-overridden/main.tftest.hcl b/internal/command/testdata/test/write-only-attributes-overridden/main.tftest.hcl new file mode 100644 index 0000000000..203f5fb7ef --- /dev/null +++ b/internal/command/testdata/test/write-only-attributes-overridden/main.tftest.hcl @@ -0,0 +1,38 @@ + +provider "test" {} + +override_resource { + target = test_resource.resource + values = { + id = "resource" + } +} + +override_data { + target = data.test_data_source.datasource + values = { + value = "hello" + } +} + +run "test" { + variables { + input = "input" + } + + assert { + condition = data.test_data_source.datasource.value == "hello" + error_message = "wrong value" + } + + assert { + condition = test_resource.resource.value == "hello" + error_message = "wrong value" + } + + assert { + condition = test_resource.resource.id == "resource" + error_message = "wrong value" + } + +} diff --git a/internal/command/testdata/test/write-only-attributes/main.tf b/internal/command/testdata/test/write-only-attributes/main.tf new file mode 100644 index 0000000000..82b012575a --- /dev/null +++ b/internal/command/testdata/test/write-only-attributes/main.tf @@ -0,0 +1,9 @@ + +variable "input" { + type = string +} + +resource "test_resource" "resource" { + id = "resource" + write_only = var.input +} diff --git a/internal/command/testdata/test/write-only-attributes/main.tftest.hcl b/internal/command/testdata/test/write-only-attributes/main.tftest.hcl new file mode 100644 index 0000000000..15764bae83 --- /dev/null +++ b/internal/command/testdata/test/write-only-attributes/main.tftest.hcl @@ -0,0 +1,8 @@ + +provider "test" {} + +run "test" { + variables { + input = "input" + } +} diff --git a/internal/command/testing/test_provider.go b/internal/command/testing/test_provider.go index 5e1acef246..f62322a0f9 100644 --- a/internal/command/testing/test_provider.go +++ b/internal/command/testing/test_provider.go @@ -39,6 +39,7 @@ var ( "destroy_fail": {Type: cty.Bool, Optional: true, Computed: true}, "create_wait_seconds": {Type: cty.Number, Optional: true}, "destroy_wait_seconds": {Type: cty.Number, Optional: true}, + "write_only": {Type: cty.String, Optional: true, WriteOnly: true}, }, }, }, @@ -47,8 +48,9 @@ var ( "test_data_source": { Body: &configschema.Block{ Attributes: map[string]*configschema.Attribute{ - "id": {Type: cty.String, Required: true}, - "value": {Type: cty.String, Computed: true}, + "id": {Type: cty.String, Required: true}, + "value": {Type: cty.String, Computed: true}, + "write_only": {Type: cty.String, Optional: true, WriteOnly: true}, // We never actually reference these values from a data // source, but we have tests that use the same cty.Value @@ -233,12 +235,18 @@ func (provider *TestProvider) PlanResourceChange(request providers.PlanResourceC resource = cty.ObjectVal(vals) } - if destryFail := resource.GetAttr("destroy_fail"); !destryFail.IsKnown() || destryFail.IsNull() { + if destroyFail := resource.GetAttr("destroy_fail"); !destroyFail.IsKnown() || destroyFail.IsNull() { vals := resource.AsValueMap() vals["destroy_fail"] = cty.UnknownVal(cty.Bool) resource = cty.ObjectVal(vals) } + if writeOnly := resource.GetAttr("write_only"); !writeOnly.IsNull() { + vals := resource.AsValueMap() + vals["write_only"] = cty.NullVal(cty.String) + resource = cty.ObjectVal(vals) + } + return providers.PlanResourceChangeResponse{ PlannedState: resource, } @@ -335,6 +343,12 @@ func (provider *TestProvider) ReadDataSource(request providers.ReadDataSourceReq diags = diags.Append(tfdiags.Sourceless(tfdiags.Error, "not found", fmt.Sprintf("%s does not exist", id))) } + if writeOnly := resource.GetAttr("write_only"); !writeOnly.IsNull() { + vals := resource.AsValueMap() + vals["write_only"] = cty.NullVal(cty.String) + resource = cty.ObjectVal(vals) + } + return providers.ReadDataSourceResponse{ State: resource, Diagnostics: diags, diff --git a/internal/lang/ephemeral/strip.go b/internal/lang/ephemeral/strip.go new file mode 100644 index 0000000000..cd6f41acb3 --- /dev/null +++ b/internal/lang/ephemeral/strip.go @@ -0,0 +1,45 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package ephemeral + +import ( + "github.com/zclconf/go-cty/cty" + + "github.com/hashicorp/terraform/internal/configs/configschema" +) + +// StripWriteOnlyAttributes converts all the write-only attributes in value to +// null values. +func StripWriteOnlyAttributes(value cty.Value, schema *configschema.Block) cty.Value { + // writeOnlyTransformer never returns errors, so we don't need to detect + // them here. + updated, _ := cty.TransformWithTransformer(value, &writeOnlyTransformer{ + schema: schema, + }) + return updated +} + +var _ cty.Transformer = (*writeOnlyTransformer)(nil) + +type writeOnlyTransformer struct { + schema *configschema.Block +} + +func (w *writeOnlyTransformer) Enter(path cty.Path, value cty.Value) (cty.Value, error) { + attr := w.schema.AttributeByPath(path) + if attr == nil { + return value, nil + } + + if attr.WriteOnly { + value, marks := value.Unmark() + return cty.NullVal(value.Type()).WithMarks(marks), nil + } + + return value, nil +} + +func (w *writeOnlyTransformer) Exit(_ cty.Path, value cty.Value) (cty.Value, error) { + return value, nil // no changes +} diff --git a/internal/lang/ephemeral/strip_test.go b/internal/lang/ephemeral/strip_test.go new file mode 100644 index 0000000000..c7562ed024 --- /dev/null +++ b/internal/lang/ephemeral/strip_test.go @@ -0,0 +1,193 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package ephemeral + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/zclconf/go-cty-debug/ctydebug" + "github.com/zclconf/go-cty/cty" + + "github.com/hashicorp/terraform/internal/configs/configschema" + "github.com/hashicorp/terraform/internal/lang/marks" +) + +func TestStripWriteOnlyAttributes(t *testing.T) { + tcs := map[string]struct { + val cty.Value + schema *configschema.Block + want cty.Value + }{ + "primitive": { + val: cty.ObjectVal(map[string]cty.Value{ + "value": cty.StringVal("value"), + }), + schema: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "value": { + Type: cty.String, + WriteOnly: true, + }, + }, + }, + want: cty.ObjectVal(map[string]cty.Value{ + "value": cty.NullVal(cty.String), + }), + }, + "complex": { + val: cty.ObjectVal(map[string]cty.Value{ + "value": cty.ObjectVal(map[string]cty.Value{ + "value": cty.StringVal("value"), + }), + }), + schema: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "value": { + NestedType: &configschema.Object{ + Attributes: map[string]*configschema.Attribute{ + "value": { + Type: cty.String, + }, + }, + Nesting: configschema.NestingSingle, + }, + WriteOnly: true, + }, + }, + }, + want: cty.ObjectVal(map[string]cty.Value{ + "value": cty.NullVal(cty.Object(map[string]cty.Type{ + "value": cty.String, + })), + }), + }, + "nested in object": { + val: cty.ObjectVal(map[string]cty.Value{ + "value": cty.ObjectVal(map[string]cty.Value{ + "value": cty.StringVal("value"), + }), + }), + schema: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "value": { + NestedType: &configschema.Object{ + Attributes: map[string]*configschema.Attribute{ + "value": { + Type: cty.String, + WriteOnly: true, + }, + }, + Nesting: configschema.NestingSingle, + }, + }, + }, + }, + want: cty.ObjectVal(map[string]cty.Value{ + "value": cty.ObjectVal(map[string]cty.Value{ + "value": cty.NullVal(cty.String), + }), + }), + }, + "nested in list": { + val: cty.ObjectVal(map[string]cty.Value{ + "value": cty.ListVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "value": cty.StringVal("value"), + }), + cty.ObjectVal(map[string]cty.Value{ + "value": cty.StringVal("value"), + }), + }), + }), + schema: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "value": { + NestedType: &configschema.Object{ + Attributes: map[string]*configschema.Attribute{ + "value": { + Type: cty.String, + WriteOnly: true, + }, + }, + Nesting: configschema.NestingList, + }, + }, + }, + }, + want: cty.ObjectVal(map[string]cty.Value{ + "value": cty.ListVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "value": cty.NullVal(cty.String), + }), + cty.ObjectVal(map[string]cty.Value{ + "value": cty.NullVal(cty.String), + }), + }), + }), + }, + "nested in map": { + val: cty.ObjectVal(map[string]cty.Value{ + "value": cty.MapVal(map[string]cty.Value{ + "one": cty.ObjectVal(map[string]cty.Value{ + "value": cty.StringVal("value"), + }), + "two": cty.ObjectVal(map[string]cty.Value{ + "value": cty.StringVal("value"), + }), + }), + }), + schema: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "value": { + NestedType: &configschema.Object{ + Attributes: map[string]*configschema.Attribute{ + "value": { + Type: cty.String, + WriteOnly: true, + }, + }, + Nesting: configschema.NestingMap, + }, + }, + }, + }, + want: cty.ObjectVal(map[string]cty.Value{ + "value": cty.MapVal(map[string]cty.Value{ + "one": cty.ObjectVal(map[string]cty.Value{ + "value": cty.NullVal(cty.String), + }), + "two": cty.ObjectVal(map[string]cty.Value{ + "value": cty.NullVal(cty.String), + }), + }), + }), + }, + "preserves marks": { + val: cty.ObjectVal(map[string]cty.Value{ + "value": cty.StringVal("value"), + }).Mark(marks.Sensitive), + schema: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "value": { + Type: cty.String, + WriteOnly: true, + }, + }, + }, + want: cty.ObjectVal(map[string]cty.Value{ + "value": cty.NullVal(cty.String), + }).Mark(marks.Sensitive), + }, + } + + for name, tc := range tcs { + t.Run(name, func(t *testing.T) { + got := StripWriteOnlyAttributes(tc.val, tc.schema) + if diff := cmp.Diff(got, tc.want, ctydebug.CmpOptions); len(diff) > 0 { + t.Errorf("got diff:\n%s", diff) + } + }) + } +} diff --git a/internal/lang/ephemeral/validate.go b/internal/lang/ephemeral/validate.go index 680e2a9ee2..3556651f94 100644 --- a/internal/lang/ephemeral/validate.go +++ b/internal/lang/ephemeral/validate.go @@ -6,15 +6,16 @@ package ephemeral import ( "fmt" + "github.com/zclconf/go-cty/cty" + "github.com/hashicorp/terraform/internal/configs/configschema" "github.com/hashicorp/terraform/internal/tfdiags" - "github.com/zclconf/go-cty/cty" ) // ValidateWriteOnlyAttributes identifies all instances of write-only paths that contain non-null values // and returns a diagnostic for each instance func ValidateWriteOnlyAttributes(summary string, detail func(cty.Path) string, newVal cty.Value, schema *configschema.Block) (diags tfdiags.Diagnostics) { - writeOnlyPaths, err := nonNullWriteOnlyPaths(newVal, schema, nil) + writeOnlyPaths, err := nonNullWriteOnlyPaths(newVal, schema) if err != nil { return diags.Append(tfdiags.Sourceless( tfdiags.Error, @@ -36,7 +37,7 @@ func ValidateWriteOnlyAttributes(summary string, detail func(cty.Path) string, n // nonNullWriteOnlyPaths returns a list of paths to attributes that are write-only // and non-null in the given value. -func nonNullWriteOnlyPaths(val cty.Value, schema *configschema.Block, p cty.Path) ([]cty.Path, error) { +func nonNullWriteOnlyPaths(val cty.Value, schema *configschema.Block) ([]cty.Path, error) { if schema == nil { panic("nonNullWriteOnlyPaths called wih nil schema") } diff --git a/internal/lang/ephemeral/validate_test.go b/internal/lang/ephemeral/validate_test.go index e82f172f5a..89bfdc6c59 100644 --- a/internal/lang/ephemeral/validate_test.go +++ b/internal/lang/ephemeral/validate_test.go @@ -6,8 +6,9 @@ package ephemeral import ( "testing" - "github.com/hashicorp/terraform/internal/configs/configschema" "github.com/zclconf/go-cty/cty" + + "github.com/hashicorp/terraform/internal/configs/configschema" ) func TestNonNullWriteOnlyPaths(t *testing.T) { @@ -145,7 +146,7 @@ func TestNonNullWriteOnlyPaths(t *testing.T) { }, } { t.Run(name, func(t *testing.T) { - paths, err := nonNullWriteOnlyPaths(tc.val, tc.schema, nil) + paths, err := nonNullWriteOnlyPaths(tc.val, tc.schema) if err != nil { t.Fatal(err) } diff --git a/internal/providers/mock.go b/internal/providers/mock.go index 9e560bf63c..f15e4e1ca4 100644 --- a/internal/providers/mock.go +++ b/internal/providers/mock.go @@ -11,6 +11,7 @@ import ( "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/configs/hcl2shim" + "github.com/hashicorp/terraform/internal/lang/ephemeral" "github.com/hashicorp/terraform/internal/moduletest/mocking" "github.com/hashicorp/terraform/internal/tfdiags" ) @@ -138,7 +139,7 @@ func (m *Mock) UpgradeResourceState(request UpgradeResourceStateRequest) (respon response.Diagnostics = response.Diagnostics.Append(err) return response } - response.UpgradedState = value + response.UpgradedState = ephemeral.StripWriteOnlyAttributes(value, resource.Body) return response } @@ -207,27 +208,26 @@ func (m *Mock) PlanResourceChange(request PlanResourceChangeRequest) PlanResourc } } + var response PlanResourceChangeResponse + schema := m.GetProviderSchema() + response.Diagnostics = response.Diagnostics.Append(schema.Diagnostics) + if schema.Diagnostics.HasErrors() { + // We couldn't retrieve the schema for some reason, so the mock + // provider can't really function. + return response + } + + resource, exists := schema.ResourceTypes[request.TypeName] + if !exists { + // This means something has gone wrong much earlier, we should have + // failed a validation somewhere if a resource type doesn't exist. + panic(fmt.Errorf("failed to retrieve schema for resource %s", request.TypeName)) + } + if request.PriorState.IsNull() { // Then we are creating this resource - we need to populate the computed // null fields with unknowns so Terraform will render them properly. - var response PlanResourceChangeResponse - - schema := m.GetProviderSchema() - response.Diagnostics = response.Diagnostics.Append(schema.Diagnostics) - if schema.Diagnostics.HasErrors() { - // We couldn't retrieve the schema for some reason, so the mock - // provider can't really function. - return response - } - - resource, exists := schema.ResourceTypes[request.TypeName] - if !exists { - // This means something has gone wrong much earlier, we should have - // failed a validation somewhere if a resource type doesn't exist. - panic(fmt.Errorf("failed to retrieve schema for resource %s", request.TypeName)) - } - replacement := &mocking.MockedData{ Value: cty.NilVal, // If we have no data then we use cty.NilVal. ComputedAsUnknown: true, @@ -241,17 +241,16 @@ func (m *Mock) PlanResourceChange(request PlanResourceChangeRequest) PlanResourc value, diags := mocking.PlanComputedValuesForResource(request.ProposedNewState, replacement, resource.Body) response.Diagnostics = response.Diagnostics.Append(diags) - response.PlannedState = value + response.PlannedState = ephemeral.StripWriteOnlyAttributes(value, resource.Body) response.PlannedPrivate = []byte("create") return response } // Otherwise, we're just doing a simple update and we don't need to populate // any values for that. - return PlanResourceChangeResponse{ - PlannedState: request.ProposedNewState, - PlannedPrivate: []byte("update"), - } + response.PlannedState = ephemeral.StripWriteOnlyAttributes(request.ProposedNewState, resource.Body) + response.PlannedPrivate = []byte("update") + return response } func (m *Mock) ApplyResourceChange(request ApplyResourceChangeRequest) ApplyResourceChangeResponse { @@ -344,7 +343,7 @@ func (m *Mock) ReadDataSource(request ReadDataSourceRequest) ReadDataSourceRespo value, diags := mocking.ComputedValuesForDataSource(request.Config, mockedData, datasource.Body) response.Diagnostics = response.Diagnostics.Append(diags) - response.State = value + response.State = ephemeral.StripWriteOnlyAttributes(value, datasource.Body) return response } diff --git a/internal/stacks/stackruntime/internal/stackeval/stubs/unknown.go b/internal/stacks/stackruntime/internal/stackeval/stubs/unknown.go index 78ea540fc0..c604e6c65e 100644 --- a/internal/stacks/stackruntime/internal/stackeval/stubs/unknown.go +++ b/internal/stacks/stackruntime/internal/stackeval/stubs/unknown.go @@ -8,6 +8,7 @@ import ( "github.com/zclconf/go-cty/cty" + "github.com/hashicorp/terraform/internal/lang/ephemeral" "github.com/hashicorp/terraform/internal/moduletest/mocking" "github.com/hashicorp/terraform/internal/providers" "github.com/hashicorp/terraform/internal/tfdiags" @@ -134,7 +135,7 @@ func (u *unknownProvider) PlanResourceChange(request providers.PlanResourceChang } return providers.PlanResourceChangeResponse{ - PlannedState: val, + PlannedState: ephemeral.StripWriteOnlyAttributes(val, schema.Body), Deferred: &providers.Deferred{ Reason: providers.DeferredReasonProviderConfigUnknown, }, @@ -232,7 +233,7 @@ func (u *unknownProvider) ReadDataSource(request providers.ReadDataSourceRequest } return providers.ReadDataSourceResponse{ - State: val, + State: ephemeral.StripWriteOnlyAttributes(val, schema.Body), Deferred: &providers.Deferred{ Reason: providers.DeferredReasonProviderConfigUnknown, }, diff --git a/internal/stacks/stackruntime/plan_test.go b/internal/stacks/stackruntime/plan_test.go index 63230d8132..a19b1678d3 100644 --- a/internal/stacks/stackruntime/plan_test.go +++ b/internal/stacks/stackruntime/plan_test.go @@ -242,6 +242,100 @@ func TestPlan(t *testing.T) { }, }, }, + "deferred-provider-with-write-only": { + path: "with-write-only-attribute", + cycle: TestCycle{ + planInputs: map[string]cty.Value{ + "providers": cty.UnknownVal(cty.Set(cty.String)), + }, + wantPlannedChanges: []stackplan.PlannedChange{ + &stackplan.PlannedChangeApplyable{ + Applyable: true, + }, + &stackplan.PlannedChangeComponentInstance{ + Addr: mustAbsComponentInstance("component.main"), + Action: plans.Create, + PlannedInputValues: map[string]plans.DynamicValue{ + "datasource_id": mustPlanDynamicValueDynamicType(cty.StringVal("datasource")), + "resource_id": mustPlanDynamicValueDynamicType(cty.StringVal("resource")), + "write_only_input": mustPlanDynamicValueDynamicType(cty.StringVal("secret")), + }, + PlannedInputValueMarks: map[string][]cty.PathValueMarks{ + "datasource_id": nil, + "resource_id": nil, + "write_only_input": nil, + }, + PlannedOutputValues: make(map[string]cty.Value), + PlannedCheckResults: &states.CheckResults{}, + PlanTimestamp: fakePlanTimestamp, + }, + &stackplan.PlannedChangeDeferredResourceInstancePlanned{ + ResourceInstancePlanned: stackplan.PlannedChangeResourceInstancePlanned{ + ResourceInstanceObjectAddr: mustAbsResourceInstanceObject("component.main.data.testing_write_only_data_source.data"), + ChangeSrc: &plans.ResourceInstanceChangeSrc{ + Addr: mustAbsResourceInstance("data.testing_write_only_data_source.data"), + PrevRunAddr: mustAbsResourceInstance("data.testing_write_only_data_source.data"), + ProviderAddr: mustDefaultRootProvider("testing"), + ChangeSrc: plans.ChangeSrc{ + Action: plans.Read, + Before: mustPlanDynamicValue(cty.NullVal(cty.DynamicPseudoType)), + After: mustPlanDynamicValue(cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("datasource"), + "value": cty.UnknownVal(cty.String), + "write_only": cty.NullVal(cty.String), + })), + AfterSensitivePaths: []cty.Path{ + cty.GetAttrPath("write_only"), + }, + }, + ActionReason: plans.ResourceInstanceReadBecauseDependencyPending, + }, + ProviderConfigAddr: mustDefaultRootProvider("testing"), + Schema: stacks_testing_provider.WriteOnlyDataSourceSchema, + }, + DeferredReason: providers.DeferredReasonProviderConfigUnknown, + }, + &stackplan.PlannedChangeDeferredResourceInstancePlanned{ + ResourceInstancePlanned: stackplan.PlannedChangeResourceInstancePlanned{ + ResourceInstanceObjectAddr: mustAbsResourceInstanceObject("component.main.testing_write_only_resource.data"), + ChangeSrc: &plans.ResourceInstanceChangeSrc{ + Addr: mustAbsResourceInstance("testing_write_only_resource.data"), + PrevRunAddr: mustAbsResourceInstance("testing_write_only_resource.data"), + ProviderAddr: mustDefaultRootProvider("testing"), + ChangeSrc: plans.ChangeSrc{ + Action: plans.Create, + Before: mustPlanDynamicValue(cty.NullVal(cty.DynamicPseudoType)), + After: mustPlanDynamicValue(cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("resource"), + "value": cty.UnknownVal(cty.String), + "write_only": cty.NullVal(cty.String), + })), + AfterSensitivePaths: []cty.Path{ + cty.GetAttrPath("write_only"), + }, + }, + }, + PriorStateSrc: nil, + ProviderConfigAddr: mustDefaultRootProvider("testing"), + Schema: stacks_testing_provider.WriteOnlyResourceSchema, + }, + DeferredReason: providers.DeferredReasonProviderConfigUnknown, + }, + &stackplan.PlannedChangeHeader{ + TerraformVersion: version.SemVer, + }, + &stackplan.PlannedChangePlannedTimestamp{ + PlannedTimestamp: fakePlanTimestamp, + }, + &stackplan.PlannedChangeRootInputValue{ + Addr: mustStackInputVariable("providers"), + Action: plans.Create, + Before: cty.NullVal(cty.DynamicPseudoType), + After: cty.UnknownVal(cty.Set(cty.String)), + }, + }, + }, + }, "deferred-provider-with-data-sources": { path: path.Join("with-data-source", "deferred-provider-for-each"), store: stacks_testing_provider.NewResourceStoreBuilder(). diff --git a/internal/stacks/stackruntime/testdata/mainbundle/test/with-write-only-attribute/with-write-only-attribute.tf b/internal/stacks/stackruntime/testdata/mainbundle/test/with-write-only-attribute/with-write-only-attribute.tf new file mode 100644 index 0000000000..3d05a71916 --- /dev/null +++ b/internal/stacks/stackruntime/testdata/mainbundle/test/with-write-only-attribute/with-write-only-attribute.tf @@ -0,0 +1,32 @@ +terraform { + required_providers { + testing = { + source = "hashicorp/testing" + version = "0.1.0" + } + } +} + +variable "datasource_id" { + type = string +} + +variable "resource_id" { + type = string +} + +variable "write_only_input" { + type = string + sensitive = true +} + +data "testing_write_only_data_source" "data" { + id = var.datasource_id + write_only = var.write_only_input +} + +resource "testing_write_only_resource" "data" { + id = var.resource_id + value = data.testing_write_only_data_source.data.value + write_only = var.write_only_input +} diff --git a/internal/stacks/stackruntime/testdata/mainbundle/test/with-write-only-attribute/with-write-only-attribute.tfstack.hcl b/internal/stacks/stackruntime/testdata/mainbundle/test/with-write-only-attribute/with-write-only-attribute.tfstack.hcl new file mode 100644 index 0000000000..39c88a5dbe --- /dev/null +++ b/internal/stacks/stackruntime/testdata/mainbundle/test/with-write-only-attribute/with-write-only-attribute.tfstack.hcl @@ -0,0 +1,28 @@ +required_providers { + testing = { + source = "hashicorp/testing" + version = "0.1.0" + } +} + +variable "providers" { + type = set(string) +} + +provider "testing" "main" { + for_each = var.providers +} + +component "main" { + source = "./" + + providers = { + testing = provider.testing.main["single"] + } + + inputs = { + datasource_id = "datasource" + resource_id = "resource" + write_only_input = "secret" + } +} diff --git a/internal/stacks/stackruntime/testing/provider.go b/internal/stacks/stackruntime/testing/provider.go index e70db5134f..210d50b143 100644 --- a/internal/stacks/stackruntime/testing/provider.go +++ b/internal/stacks/stackruntime/testing/provider.go @@ -59,6 +59,16 @@ var ( }, } + WriteOnlyResourceSchema = providers.Schema{ + Body: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "id": {Type: cty.String, Optional: true, Computed: true}, + "value": {Type: cty.String, Optional: true}, + "write_only": {Type: cty.String, WriteOnly: true, Optional: true}, + }, + }, + } + TestingDataSourceSchema = providers.Schema{ Body: &configschema.Block{ Attributes: map[string]*configschema.Attribute{ @@ -68,6 +78,16 @@ var ( }, } + WriteOnlyDataSourceSchema = providers.Schema{ + Body: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "id": {Type: cty.String, Required: true}, + "value": {Type: cty.String, Computed: true}, + "write_only": {Type: cty.String, WriteOnly: true, Optional: true}, + }, + }, + } + TestingResourceWithIdentitySchema = providers.Schema{ Body: &configschema.Block{ Attributes: map[string]*configschema.Attribute{ @@ -167,11 +187,17 @@ func NewProviderWithData(t *testing.T, store *ResourceStore) *MockProvider { Body: TestingResourceSchema.Body, Identity: TestingResourceWithIdentitySchema.Identity, }, + "testing_write_only_resource": { + Body: WriteOnlyResourceSchema.Body, + }, }, DataSources: map[string]providers.Schema{ "testing_data_source": { Body: TestingDataSourceSchema.Body, }, + "testing_write_only_data_source": { + Body: WriteOnlyDataSourceSchema.Body, + }, }, Functions: map[string]providers.FunctionDecl{ "echo": { diff --git a/internal/stacks/stackruntime/testing/resource.go b/internal/stacks/stackruntime/testing/resource.go index 2524191502..1f3aedb0f1 100644 --- a/internal/stacks/stackruntime/testing/resource.go +++ b/internal/stacks/stackruntime/testing/resource.go @@ -35,6 +35,8 @@ func getResource(name string) resource { return &failedResource{} case "testing_blocked_resource": return &blockedResource{} + case "testing_write_only_resource": + return &writeOnlyResource{} case "testing_resource_with_identity": return &testingResourceWithIdentity{} default: @@ -47,6 +49,7 @@ var ( _ resource = (*deferredResource)(nil) _ resource = (*failedResource)(nil) _ resource = (*blockedResource)(nil) + _ resource = (*writeOnlyResource)(nil) _ resource = (*testingResourceWithIdentity)(nil) ) @@ -319,6 +322,60 @@ func (b *blockedResource) Apply(request providers.ApplyResourceChangeRequest, st return } +// writeOnlyResource is the same as testingResource but it includes an extra +// write-only attribute. +type writeOnlyResource struct{} + +func (w *writeOnlyResource) Read(request providers.ReadResourceRequest, store *ResourceStore) (response providers.ReadResourceResponse) { + id := request.PriorState.GetAttr("id").AsString() + var exists bool + response.NewState, exists = store.Get(id) + if !exists { + response.NewState = cty.NullVal(WriteOnlyResourceSchema.Body.ImpliedType()) + } + return +} + +func (w *writeOnlyResource) Plan(request providers.PlanResourceChangeRequest, store *ResourceStore) (response providers.PlanResourceChangeResponse) { + if request.ProposedNewState.IsNull() { + response.PlannedState = request.ProposedNewState + return + } + + response.PlannedState = setNull(planEnsureId(request.ProposedNewState), "write_only") + replace, err := validateId(response.PlannedState, request.PriorState, store) + if err != nil { + response.Diagnostics = append(response.Diagnostics, tfdiags.Sourceless(tfdiags.Error, "testingResource error", err.Error())) + return + } + if replace { + response.RequiresReplace = []cty.Path{cty.GetAttrPath("id")} + } + return +} + +func (w *writeOnlyResource) Apply(request providers.ApplyResourceChangeRequest, store *ResourceStore) (response providers.ApplyResourceChangeResponse) { + if request.PlannedState.IsNull() { + store.Delete(request.PriorState.GetAttr("id").AsString()) + response.NewState = request.PlannedState + return + } + + value := applyEnsureId(request.PlannedState) + replace, err := validateId(value, request.PriorState, store) + if err != nil { + response.Diagnostics = append(response.Diagnostics, tfdiags.Sourceless(tfdiags.Error, "testingResource error", err.Error())) + return + } + response.NewState = value + + if replace { + store.Delete(request.PriorState.GetAttr("id").AsString()) + } + store.Set(response.NewState.GetAttr("id").AsString(), response.NewState) + return +} + // testingResourceWithIdentity is the same as testingResource but it returns an identity. type testingResourceWithIdentity struct{} @@ -433,3 +490,12 @@ func setKnown(value cty.Value, attr string, attrValue cty.Value) cty.Value { } return value } + +func setNull(value cty.Value, attr string) cty.Value { + if v := value.GetAttr(attr); !v.IsKnown() { + vals := value.AsValueMap() + vals[attr] = cty.NullVal(v.Type()) + return cty.ObjectVal(vals) + } + return value +} diff --git a/internal/terraform/node_resource_abstract_instance.go b/internal/terraform/node_resource_abstract_instance.go index 3ebd40eba8..92d2120e5c 100644 --- a/internal/terraform/node_resource_abstract_instance.go +++ b/internal/terraform/node_resource_abstract_instance.go @@ -943,14 +943,14 @@ func (n *NodeAbstractResourceInstance) plan( ComputedAsUnknown: !n.override.UseForPlan, }, schema.Body) resp = providers.PlanResourceChangeResponse{ - PlannedState: override, + PlannedState: ephemeral.StripWriteOnlyAttributes(override, schema.Body), Diagnostics: overrideDiags, } } else { // This is an update operation, and we don't actually have any // computed values that need to be applied. resp = providers.PlanResourceChangeResponse{ - PlannedState: proposedNewVal, + PlannedState: ephemeral.StripWriteOnlyAttributes(proposedNewVal, schema.Body), } } } else { @@ -1156,7 +1156,7 @@ func (n *NodeAbstractResourceInstance) plan( ComputedAsUnknown: !n.override.UseForPlan, }, schema.Body) resp = providers.PlanResourceChangeResponse{ - PlannedState: override, + PlannedState: ephemeral.StripWriteOnlyAttributes(override, schema.Body), Diagnostics: overrideDiags, } } else { @@ -1641,7 +1641,7 @@ func (n *NodeAbstractResourceInstance) readDataSource(ctx EvalContext, configVal ComputedAsUnknown: false, }, schema.Body) resp = providers.ReadDataSourceResponse{ - State: override, + State: ephemeral.StripWriteOnlyAttributes(override, schema.Body), Diagnostics: overrideDiags, } } else { diff --git a/internal/terraform/node_resource_plan_instance.go b/internal/terraform/node_resource_plan_instance.go index f0191fe028..178102f776 100644 --- a/internal/terraform/node_resource_plan_instance.go +++ b/internal/terraform/node_resource_plan_instance.go @@ -644,7 +644,7 @@ func (n *NodePlannableResourceInstance) importState(ctx EvalContext, addr addrs. ImportedResources: []providers.ImportedResource{ { TypeName: addr.Resource.Resource.Type, - State: override, + State: ephemeral.StripWriteOnlyAttributes(override, schema.Body), }, }, Diagnostics: overrideDiags.InConfigBody(n.Config.Config, absAddr.String()),