From 8a22769441b23d5bf5d298076cef3f01d25ab221 Mon Sep 17 00:00:00 2001 From: Daniel Schmidt Date: Mon, 18 Nov 2024 17:08:10 +0100 Subject: [PATCH] ephemeral: support write-only attributes Only write-only attributes are allowed to be set to ephemeral values. Ephemeral values are set to null in both the plan and state. This commit only lays the basis, there is further work needed to support this both for stacks and the terminal UI interfaces. --- internal/command/jsonstate/state.go | 19 ++++--- internal/command/jsonstate/state_test.go | 34 ++++++++++++- internal/lang/ephemeral/marshal.go | 40 +++++++++++++++ internal/lang/ephemeral/marshal_test.go | 51 +++++++++++++++++++ internal/lang/funcs/conversion.go | 25 +-------- internal/plans/changes.go | 13 ++++- internal/plans/changes_src.go | 4 ++ internal/plans/objchange/compatible.go | 6 +++ internal/stacks/tfstackdata1/convert.go | 1 + internal/states/instance_object.go | 14 +++-- internal/states/instance_object_src.go | 5 ++ internal/states/state_deepcopy.go | 15 ++++-- internal/states/statefile/version4.go | 11 ++++ .../terraform/context_apply_ephemeral_test.go | 37 +++++++++----- internal/terraform/context_plan.go | 27 +++++----- .../terraform/context_plan_ephemeral_test.go | 23 +++++++++ internal/terraform/node_resource_validate.go | 28 +++++----- 17 files changed, 271 insertions(+), 82 deletions(-) create mode 100644 internal/lang/ephemeral/marshal.go create mode 100644 internal/lang/ephemeral/marshal_test.go diff --git a/internal/command/jsonstate/state.go b/internal/command/jsonstate/state.go index f77da0e036c9..aff4f7b929a2 100644 --- a/internal/command/jsonstate/state.go +++ b/internal/command/jsonstate/state.go @@ -13,6 +13,7 @@ import ( "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/command/jsonchecks" + "github.com/hashicorp/terraform/internal/lang/ephemeral" "github.com/hashicorp/terraform/internal/lang/marks" "github.com/hashicorp/terraform/internal/states" "github.com/hashicorp/terraform/internal/states/statefile" @@ -116,15 +117,21 @@ type Resource struct { // resource, whose structure depends on the resource type schema. type AttributeValues map[string]json.RawMessage -func marshalAttributeValues(value cty.Value) (unmarkedVal cty.Value, marshalledVals AttributeValues, sensitivePaths []cty.Path, err error) { +func marshalAttributeValues(value cty.Value) (unmarkedVal cty.Value, marshalledVals AttributeValues, sensitivePaths []cty.Path, ephemeralPaths []cty.Path, err error) { + // Every ephemeral value at this point must be set through a write_only attribute, otherwise + // validation would have failed. For this reason we can safely remove all ephemeral values from the value. + _, pvms := value.UnmarkDeepWithPaths() + ephemeralPaths, _ = marks.PathsWithMark(pvms, marks.Ephemeral) + value = ephemeral.RemoveEphemeralValuesForMarshaling(value) + // unmark our value to show all values value, sensitivePaths, err = unmarkValueForMarshaling(value) if err != nil { - return cty.NilVal, nil, nil, err + return cty.NilVal, nil, nil, nil, err } if value == cty.NilVal || value.IsNull() { - return value, nil, nil, nil + return value, nil, nil, nil, nil } ret := make(AttributeValues) @@ -135,7 +142,7 @@ func marshalAttributeValues(value cty.Value) (unmarkedVal cty.Value, marshalledV vJSON, _ := ctyjson.Marshal(v, v.Type()) ret[k.AsString()] = json.RawMessage(vJSON) } - return value, ret, sensitivePaths, nil + return value, ret, sensitivePaths, ephemeralPaths, nil } // newState() returns a minimally-initialized state @@ -403,7 +410,7 @@ func marshalResources(resources map[string]*states.Resource, module addrs.Module var value cty.Value var sensitivePaths []cty.Path - value, current.AttributeValues, sensitivePaths, err = marshalAttributeValues(riObj.Value) + value, current.AttributeValues, sensitivePaths, _, err = marshalAttributeValues(riObj.Value) if err != nil { return nil, fmt.Errorf("preparing attribute values for %s: %w", current.Address, err) } @@ -455,7 +462,7 @@ func marshalResources(resources map[string]*states.Resource, module addrs.Module var value cty.Value var sensitivePaths []cty.Path - value, deposed.AttributeValues, sensitivePaths, err = marshalAttributeValues(riObj.Value) + value, deposed.AttributeValues, sensitivePaths, _, err = marshalAttributeValues(riObj.Value) if err != nil { return nil, fmt.Errorf("preparing attribute values for %s: %w", current.Address, err) } diff --git a/internal/command/jsonstate/state_test.go b/internal/command/jsonstate/state_test.go index 2e7e8ed22b77..7cfdb71b6c3d 100644 --- a/internal/command/jsonstate/state_test.go +++ b/internal/command/jsonstate/state_test.go @@ -118,16 +118,19 @@ func TestMarshalAttributeValues(t *testing.T) { Attr cty.Value Want AttributeValues WantSensitivePaths []cty.Path + WantEphemeralPaths []cty.Path }{ { cty.NilVal, nil, nil, + nil, }, { cty.NullVal(cty.String), nil, nil, + nil, }, { cty.ObjectVal(map[string]cty.Value{ @@ -135,6 +138,7 @@ func TestMarshalAttributeValues(t *testing.T) { }), AttributeValues{"foo": json.RawMessage(`"bar"`)}, nil, + nil, }, { cty.ObjectVal(map[string]cty.Value{ @@ -142,6 +146,7 @@ func TestMarshalAttributeValues(t *testing.T) { }), AttributeValues{"foo": json.RawMessage(`null`)}, nil, + nil, }, { cty.ObjectVal(map[string]cty.Value{ @@ -158,6 +163,7 @@ func TestMarshalAttributeValues(t *testing.T) { "baz": json.RawMessage(`["goodnight","moon"]`), }, nil, + nil, }, // Sensitive values { @@ -177,12 +183,33 @@ func TestMarshalAttributeValues(t *testing.T) { []cty.Path{ cty.GetAttrPath("baz").IndexInt(1), }, + nil, + }, + // Ephemeral values + { + cty.ObjectVal(map[string]cty.Value{ + "bar": cty.MapVal(map[string]cty.Value{ + "hello": cty.StringVal("world"), + }), + "baz": cty.ListVal([]cty.Value{ + cty.StringVal("goodnight"), + cty.StringVal("moon").Mark(marks.Ephemeral), + }), + }), + AttributeValues{ + "bar": json.RawMessage(`{"hello":"world"}`), + "baz": json.RawMessage(`["goodnight",null]`), + }, + nil, + []cty.Path{ + cty.GetAttrPath("baz").IndexInt(1), + }, }, } for _, test := range tests { t.Run(fmt.Sprintf("%#v", test.Attr), func(t *testing.T) { - val, got, sensitivePaths, err := marshalAttributeValues(test.Attr) + val, got, sensitivePaths, ephemeralPaths, err := marshalAttributeValues(test.Attr) if err != nil { t.Fatalf("unexpected error: %s", err) } @@ -192,6 +219,9 @@ func TestMarshalAttributeValues(t *testing.T) { if !reflect.DeepEqual(sensitivePaths, test.WantSensitivePaths) { t.Errorf("wrong marks\ngot: %#v\nwant: %#v\n", sensitivePaths, test.WantSensitivePaths) } + if !reflect.DeepEqual(ephemeralPaths, test.WantEphemeralPaths) { + t.Errorf("wrong marks\ngot: %#v\nwant: %#v\n", ephemeralPaths, test.WantEphemeralPaths) + } if _, marks := val.Unmark(); len(marks) != 0 { t.Errorf("returned value still has marks; should have been unmarked\n%#v", marks) } @@ -199,7 +229,7 @@ func TestMarshalAttributeValues(t *testing.T) { } t.Run("reject unsupported marks", func(t *testing.T) { - _, _, _, err := marshalAttributeValues(cty.ObjectVal(map[string]cty.Value{ + _, _, _, _, err := marshalAttributeValues(cty.ObjectVal(map[string]cty.Value{ "disallowed": cty.StringVal("a").Mark("unsupported"), })) if err == nil { diff --git a/internal/lang/ephemeral/marshal.go b/internal/lang/ephemeral/marshal.go new file mode 100644 index 000000000000..d18467d5b03b --- /dev/null +++ b/internal/lang/ephemeral/marshal.go @@ -0,0 +1,40 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package ephemeral + +import ( + "github.com/hashicorp/terraform/internal/lang/marks" + "github.com/zclconf/go-cty/cty" +) + +// RemoveEphemeralValuesForMarshaling takes a value that possibly contains ephemeral +// values and returns an equal value without ephemeral values. If an attribute contains +// an ephemeral value it will be set to null. +func RemoveEphemeralValuesForMarshaling(value cty.Value) cty.Value { + // We currently have no error case, so we can ignore the error + val, _ := cty.Transform(value, func(p cty.Path, v cty.Value) (cty.Value, error) { + _, givenMarks := v.Unmark() + if _, isEphemeral := givenMarks[marks.Ephemeral]; isEphemeral { + // We'll strip the ephemeral mark but retain any other marks + // that might be present on the input. + delete(givenMarks, marks.Ephemeral) + if !v.IsKnown() { + // If the source value is unknown then we must leave it + // unknown because its final type might be more precise + // than the associated type constraint and returning a + // typed null could therefore over-promise on what the + // final result type will be. + // We're deliberately constructing a fresh unknown value + // here, rather than returning the one we were given, + // because we need to discard any refinements that the + // unknown value might be carrying that definitely won't + // be honored when we force the final result to be null. + return cty.UnknownVal(v.Type()).WithMarks(givenMarks), nil + } + return cty.NullVal(v.Type()).WithMarks(givenMarks), nil + } + return v, nil + }) + return val +} diff --git a/internal/lang/ephemeral/marshal_test.go b/internal/lang/ephemeral/marshal_test.go new file mode 100644 index 000000000000..56d9e515426f --- /dev/null +++ b/internal/lang/ephemeral/marshal_test.go @@ -0,0 +1,51 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package ephemeral + +import ( + "testing" + + "github.com/hashicorp/terraform/internal/lang/marks" + "github.com/zclconf/go-cty/cty" +) + +func TestEphemeral_removeEphemeralValuesForMarshaling(t *testing.T) { + for name, tc := range map[string]struct { + input cty.Value + want cty.Value + }{ + "empty case": { + input: cty.NullVal(cty.DynamicPseudoType), + want: cty.NullVal(cty.DynamicPseudoType), + }, + "ephemeral marks case": { + input: cty.ObjectVal(map[string]cty.Value{ + "ephemeral": cty.StringVal("ephemeral_value").Mark(marks.Ephemeral), + "normal": cty.StringVal("normal_value"), + }), + want: cty.ObjectVal(map[string]cty.Value{ + "ephemeral": cty.NullVal(cty.String), + "normal": cty.StringVal("normal_value"), + }), + }, + "sensitive marks case": { + input: cty.ObjectVal(map[string]cty.Value{ + "sensitive": cty.StringVal("sensitive_value").Mark(marks.Sensitive), + "normal": cty.StringVal("normal_value"), + }), + want: cty.ObjectVal(map[string]cty.Value{ + "sensitive": cty.StringVal("sensitive_value").Mark(marks.Sensitive), + "normal": cty.StringVal("normal_value"), + }), + }, + } { + t.Run(name, func(t *testing.T) { + got := RemoveEphemeralValuesForMarshaling(tc.input) + + if !got.RawEquals(tc.want) { + t.Errorf("got %#v, want %#v", got, tc.want) + } + }) + } +} diff --git a/internal/lang/funcs/conversion.go b/internal/lang/funcs/conversion.go index 25609ff02451..c81161d22f8e 100644 --- a/internal/lang/funcs/conversion.go +++ b/internal/lang/funcs/conversion.go @@ -6,6 +6,7 @@ package funcs import ( "strconv" + "github.com/hashicorp/terraform/internal/lang/ephemeral" "github.com/hashicorp/terraform/internal/lang/marks" "github.com/hashicorp/terraform/internal/lang/types" "github.com/zclconf/go-cty/cty" @@ -126,29 +127,7 @@ var EphemeralAsNullFunc = function.New(&function.Spec{ return args[0].Type(), nil }, Impl: func(args []cty.Value, retType cty.Type) (cty.Value, error) { - return cty.Transform(args[0], func(p cty.Path, v cty.Value) (cty.Value, error) { - _, givenMarks := v.Unmark() - if _, isEphemeral := givenMarks[marks.Ephemeral]; isEphemeral { - // We'll strip the ephemeral mark but retain any other marks - // that might be present on the input. - delete(givenMarks, marks.Ephemeral) - if !v.IsKnown() { - // If the source value is unknown then we must leave it - // unknown because its final type might be more precise - // than the associated type constraint and returning a - // typed null could therefore over-promise on what the - // final result type will be. - // We're deliberately constructing a fresh unknown value - // here, rather than returning the one we were given, - // because we need to discard any refinements that the - // unknown value might be carrying that definitely won't - // be honored when we force the final result to be null. - return cty.UnknownVal(v.Type()).WithMarks(givenMarks), nil - } - return cty.NullVal(v.Type()).WithMarks(givenMarks), nil - } - return v, nil - }) + return ephemeral.RemoveEphemeralValuesForMarshaling(args[0]), nil }, }) diff --git a/internal/plans/changes.go b/internal/plans/changes.go index 1ed49724b055..b002a64005bc 100644 --- a/internal/plans/changes.go +++ b/internal/plans/changes.go @@ -9,6 +9,7 @@ import ( "github.com/zclconf/go-cty/cty" "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/lang/ephemeral" "github.com/hashicorp/terraform/internal/lang/marks" "github.com/hashicorp/terraform/internal/providers" "github.com/hashicorp/terraform/internal/schemarepo" @@ -605,16 +606,22 @@ type Change struct { // to call the corresponding Encode method of that struct rather than working // directly with its embedded Change. func (c *Change) Encode(ty cty.Type) (*ChangeSrc, error) { + // When we encode the change, we need to serialize the Before and After + // values, but we want to set values with the ephemeral mark to null. + before := ephemeral.RemoveEphemeralValuesForMarshaling(c.Before) + after := ephemeral.RemoveEphemeralValuesForMarshaling(c.After) + // We can't serialize value marks directly so we'll need to extract the // sensitive marks and store them in a separate field. // // We don't accept any other marks here. The caller should have dealt // with those somehow and replaced them with unmarked placeholders before // writing the value into the state. - unmarkedBefore, marksesBefore := c.Before.UnmarkDeepWithPaths() - unmarkedAfter, marksesAfter := c.After.UnmarkDeepWithPaths() + unmarkedBefore, marksesBefore := before.UnmarkDeepWithPaths() + unmarkedAfter, marksesAfter := after.UnmarkDeepWithPaths() sensitiveAttrsBefore, unsupportedMarksesBefore := marks.PathsWithMark(marksesBefore, marks.Sensitive) sensitiveAttrsAfter, unsupportedMarksesAfter := marks.PathsWithMark(marksesAfter, marks.Sensitive) + if len(unsupportedMarksesBefore) != 0 { return nil, fmt.Errorf( "prior value %s: can't serialize value marked with %#v (this is a bug in Terraform)", @@ -645,6 +652,8 @@ func (c *Change) Encode(ty cty.Type) (*ChangeSrc, error) { After: afterDV, BeforeSensitivePaths: sensitiveAttrsBefore, AfterSensitivePaths: sensitiveAttrsAfter, + BeforeWriteOnlyPaths: ephemeral.EphemeralValuePaths(c.Before), + AfterWriteOnlyPaths: ephemeral.EphemeralValuePaths(c.After), Importing: c.Importing.Encode(), GeneratedConfig: c.GeneratedConfig, }, nil diff --git a/internal/plans/changes_src.go b/internal/plans/changes_src.go index ff851ea83e62..d82ed4536947 100644 --- a/internal/plans/changes_src.go +++ b/internal/plans/changes_src.go @@ -388,6 +388,10 @@ type ChangeSrc struct { // the serialized change. BeforeSensitivePaths, AfterSensitivePaths []cty.Path + // BeforeWriteOnlyPaths and AfterWriteOnlyPaths are paths for any values + // in Before or After (respectively) that are considered to be write-only. + BeforeWriteOnlyPaths, AfterWriteOnlyPaths []cty.Path + // Importing is present if the resource is being imported as part of this // change. // diff --git a/internal/plans/objchange/compatible.go b/internal/plans/objchange/compatible.go index e6afa83e3e1b..b5a2bf4302de 100644 --- a/internal/plans/objchange/compatible.go +++ b/internal/plans/objchange/compatible.go @@ -51,6 +51,12 @@ func assertObjectCompatible(schema *configschema.Block, planned, actual cty.Valu } for name, attrS := range schema.Attributes { + // We need to ignore write_only attributes, they are not recorded in the plan + // and are ephemeral, therefore allowed to differ between plan and apply. + if attrS.WriteOnly { + continue + } + plannedV := planned.GetAttr(name) actualV := actual.GetAttr(name) diff --git a/internal/stacks/tfstackdata1/convert.go b/internal/stacks/tfstackdata1/convert.go index efb9afeced28..8e26b7b56b10 100644 --- a/internal/stacks/tfstackdata1/convert.go +++ b/internal/stacks/tfstackdata1/convert.go @@ -182,6 +182,7 @@ func DecodeProtoResourceInstanceObject(protoObj *StateResourceInstanceObjectV1) paths = append(paths, path) } objSrc.AttrSensitivePaths = paths + // TODO: Handle write only paths if len(protoObj.Dependencies) != 0 { objSrc.Dependencies = make([]addrs.ConfigResource, len(protoObj.Dependencies)) diff --git a/internal/states/instance_object.go b/internal/states/instance_object.go index f398d01c6352..597bc2561848 100644 --- a/internal/states/instance_object.go +++ b/internal/states/instance_object.go @@ -11,6 +11,7 @@ import ( ctyjson "github.com/zclconf/go-cty/cty/json" "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/lang/ephemeral" "github.com/hashicorp/terraform/internal/lang/marks" "github.com/hashicorp/terraform/internal/tfdiags" ) @@ -98,7 +99,7 @@ func (o *ResourceInstanceObject) Encode(ty cty.Type, schemaVersion uint64) (*Res // If it contains marks, remove these marks before traversing the // structure with UnknownAsNull, and save the PathValueMarks // so we can save them in state. - val, sensitivePaths, err := unmarkValueForStorage(o.Value) + val, sensitivePaths, writeOnlyPaths, err := unmarkValueForStorage(o.Value) if err != nil { return nil, err } @@ -135,6 +136,7 @@ func (o *ResourceInstanceObject) Encode(ty cty.Type, schemaVersion uint64) (*Res SchemaVersion: schemaVersion, AttrsJSON: src, AttrSensitivePaths: sensitivePaths, + AttrWriteOnlyPaths: writeOnlyPaths, Private: o.Private, Status: o.Status, Dependencies: dependencies, @@ -167,14 +169,16 @@ func (o *ResourceInstanceObject) AsTainted() *ResourceInstanceObject { // know how to store must be dealt with somehow by a caller -- presumably by // replacing each marked value with some sort of storage placeholder -- before // writing a value into the state. -func unmarkValueForStorage(v cty.Value) (unmarkedV cty.Value, sensitivePaths []cty.Path, err error) { - val, pvms := v.UnmarkDeepWithPaths() +func unmarkValueForStorage(v cty.Value) (cty.Value, []cty.Path, []cty.Path, error) { + val := ephemeral.RemoveEphemeralValuesForMarshaling(v) + ephemeralPaths := ephemeral.EphemeralValuePaths(v) + val, pvms := val.UnmarkDeepWithPaths() sensitivePaths, withOtherMarks := marks.PathsWithMark(pvms, marks.Sensitive) if len(withOtherMarks) != 0 { - return cty.NilVal, nil, fmt.Errorf( + return cty.NilVal, nil, nil, fmt.Errorf( "%s: cannot serialize value marked as %#v for inclusion in a state snapshot (this is a bug in Terraform)", tfdiags.FormatCtyPath(withOtherMarks[0].Path), withOtherMarks[0].Marks, ) } - return val, sensitivePaths, nil + return val, sensitivePaths, ephemeralPaths, nil } diff --git a/internal/states/instance_object_src.go b/internal/states/instance_object_src.go index 7960524b66a8..1c51992cf580 100644 --- a/internal/states/instance_object_src.go +++ b/internal/states/instance_object_src.go @@ -57,6 +57,10 @@ type ResourceInstanceObjectSrc struct { // state, or to save as sensitive paths when saving state AttrSensitivePaths []cty.Path + // AttrWriteOnlyPaths is an array of paths to mark as ephemeral coming out of + // state, or to save as write_only paths when saving state + AttrWriteOnlyPaths []cty.Path + // These fields all correspond to the fields of the same name on // ResourceInstanceObject. Private []byte @@ -96,6 +100,7 @@ func (os *ResourceInstanceObjectSrc) Decode(ty cty.Type) (*ResourceInstanceObjec default: val, err = ctyjson.Unmarshal(os.AttrsJSON, ty) val = marks.MarkPaths(val, marks.Sensitive, os.AttrSensitivePaths) + val = marks.MarkPaths(val, marks.Ephemeral, os.AttrWriteOnlyPaths) if err != nil { return nil, err } diff --git a/internal/states/state_deepcopy.go b/internal/states/state_deepcopy.go index 8486e9548565..cf830512b2c7 100644 --- a/internal/states/state_deepcopy.go +++ b/internal/states/state_deepcopy.go @@ -142,10 +142,16 @@ func (os *ResourceInstanceObjectSrc) DeepCopy() *ResourceInstanceObjectSrc { copy(attrsJSON, os.AttrsJSON) } - var attrPaths []cty.Path + var sensitiveAttrPaths []cty.Path if os.AttrSensitivePaths != nil { - attrPaths = make([]cty.Path, len(os.AttrSensitivePaths)) - copy(attrPaths, os.AttrSensitivePaths) + sensitiveAttrPaths = make([]cty.Path, len(os.AttrSensitivePaths)) + copy(sensitiveAttrPaths, os.AttrSensitivePaths) + } + + var writeOnlyAttrPaths []cty.Path + if os.AttrWriteOnlyPaths != nil { + writeOnlyAttrPaths = make([]cty.Path, len(os.AttrWriteOnlyPaths)) + copy(writeOnlyAttrPaths, os.AttrWriteOnlyPaths) } var private []byte @@ -168,7 +174,8 @@ func (os *ResourceInstanceObjectSrc) DeepCopy() *ResourceInstanceObjectSrc { Private: private, AttrsFlat: attrsFlat, AttrsJSON: attrsJSON, - AttrSensitivePaths: attrPaths, + AttrSensitivePaths: sensitiveAttrPaths, + AttrWriteOnlyPaths: writeOnlyAttrPaths, Dependencies: dependencies, CreateBeforeDestroy: os.CreateBeforeDestroy, decodeValueCache: os.decodeValueCache, diff --git a/internal/states/statefile/version4.go b/internal/states/statefile/version4.go index 6477e66cf24c..d38e0b0b284b 100644 --- a/internal/states/statefile/version4.go +++ b/internal/states/statefile/version4.go @@ -166,6 +166,16 @@ func prepareStateV4(sV4 *stateV4) (*File, tfdiags.Diagnostics) { obj.AttrSensitivePaths = paths } + // write-only paths + if isV4.AttributeWriteOnlyPaths != nil { + paths, pathsDiags := unmarshalPaths([]byte(isV4.AttributeWriteOnlyPaths)) + diags = diags.Append(pathsDiags) + if pathsDiags.HasErrors() { + continue + } + obj.AttrWriteOnlyPaths = paths + } + { // Status raw := isV4.Status @@ -701,6 +711,7 @@ type instanceObjectStateV4 struct { AttributesRaw json.RawMessage `json:"attributes,omitempty"` AttributesFlat map[string]string `json:"attributes_flat,omitempty"` AttributeSensitivePaths json.RawMessage `json:"sensitive_attributes,omitempty"` + AttributeWriteOnlyPaths json.RawMessage `json:"write_only_attributes,omitempty"` PrivateRaw []byte `json:"private,omitempty"` diff --git a/internal/terraform/context_apply_ephemeral_test.go b/internal/terraform/context_apply_ephemeral_test.go index d73d60a41cda..40caecbcdd01 100644 --- a/internal/terraform/context_apply_ephemeral_test.go +++ b/internal/terraform/context_apply_ephemeral_test.go @@ -360,7 +360,7 @@ resource "test_object" "test" { assertNoDiagnostics(t, diags) } -func TestContext2Apply_write_only_attribute_not_in_plan(t *testing.T) { +func TestContext2Apply_write_only_attribute_not_in_plan_and_state(t *testing.T) { m := testModuleInline(t, map[string]string{ "main.tf": ` variable "ephem" { @@ -403,15 +403,6 @@ resource "ephem_write_only" "wo" { }, }) - _, diags := ctx.Plan(m, nil, &PlanOpts{ - Mode: plans.NormalMode, - SetVariables: InputValues{ - "ephem": &InputValue{ - Value: cty.StringVal("ephemeral_value"), - SourceType: ValueFromCLIArg, - }, - }, - }) ephemVar := &InputValue{ Value: cty.StringVal("ephemeral_value"), SourceType: ValueFromCLIArg, @@ -424,6 +415,23 @@ resource "ephem_write_only" "wo" { }) assertNoDiagnostics(t, diags) + if len(plan.Changes.Resources) != 1 { + t.Fatalf("Expected 1 resource change, got %d", len(plan.Changes.Resources)) + } + + if len(plan.Changes.Resources[0].AfterWriteOnlyPaths) != 1 { + t.Fatalf("Expected 1 write-only attribute, got %d", len(plan.Changes.Resources[0].AfterWriteOnlyPaths)) + } + schemas, schemaDiags := ctx.Schemas(m, plan.PriorState) + assertNoDiagnostics(t, schemaDiags) + planChanges, err := plan.Changes.Decode(schemas) + if err != nil { + t.Fatalf("Failed to decode plan changes: %v.", err) + } + if !planChanges.Resources[0].After.GetAttr("write_only").IsNull() { + t.Fatalf("Expected write_only to be null, got %v", planChanges.Resources[0].After.GetAttr("write_only")) + } + state, diags := ctx.Apply(plan, m, &ApplyOpts{ SetVariables: InputValues{ "ephem": ephemVar, @@ -460,8 +468,11 @@ resource "ephem_write_only" "wo" { t.Fatalf("normal attribute not as expected") } - // TODO: Or should this be omitted completely and therefore explode on decode? - if attrs.Value.GetAttr("write_only").IsKnown() { - t.Fatalf("write_only attribute should not be known") + if len(resourceInstance.Current.AttrWriteOnlyPaths) != 1 { + t.Fatalf("Expected 1 write only attribute") + } + + if !resourceInstance.Current.AttrWriteOnlyPaths[0].Equals(cty.GetAttrPath("write_only")) { + t.Fatalf("Expected write_only to be a write only attribute") } } diff --git a/internal/terraform/context_plan.go b/internal/terraform/context_plan.go index 7b7c06b7fdbd..c459d2dab521 100644 --- a/internal/terraform/context_plan.go +++ b/internal/terraform/context_plan.go @@ -1157,24 +1157,25 @@ func (c *Context) relevantResourceAttrsForPlan(config *configs.Config, plan *pla } var refs []globalref.Reference - for _, change := range plan.Changes.Resources { - if change.Action == plans.NoOp { - continue + if plan.Changes != nil { + for _, change := range plan.Changes.Resources { + if change.Action == plans.NoOp { + continue + } + + moreRefs := azr.ReferencesFromResourceInstance(change.Addr) + refs = append(refs, moreRefs...) } - moreRefs := azr.ReferencesFromResourceInstance(change.Addr) - refs = append(refs, moreRefs...) - } + for _, change := range plan.Changes.Outputs { + if change.Action == plans.NoOp { + continue + } - for _, change := range plan.Changes.Outputs { - if change.Action == plans.NoOp { - continue + moreRefs := azr.ReferencesFromOutputValue(change.Addr) + refs = append(refs, moreRefs...) } - - moreRefs := azr.ReferencesFromOutputValue(change.Addr) - refs = append(refs, moreRefs...) } - var contributors []globalref.ResourceAttr for _, ref := range azr.ContributingResourceReferences(refs...) { diff --git a/internal/terraform/context_plan_ephemeral_test.go b/internal/terraform/context_plan_ephemeral_test.go index 470eb92942de..f3041a340a44 100644 --- a/internal/terraform/context_plan_ephemeral_test.go +++ b/internal/terraform/context_plan_ephemeral_test.go @@ -21,6 +21,7 @@ import ( func TestContext2Plan_ephemeralValues(t *testing.T) { for name, tc := range map[string]struct { + toBeImplemented bool module map[string]string expectValidateDiagnostics func(m *configs.Config) tfdiags.Diagnostics expectPlanDiagnostics func(m *configs.Config) tfdiags.Diagnostics @@ -550,7 +551,24 @@ You can correct this by removing references to ephemeral values, or by carefully }, }, + "write_only attribute": { + module: map[string]string{ + "main.tf": ` +ephemeral "ephem_resource" "data" { +} + +resource "ephem_write_only" "test" { + write_only = ephemeral.ephem_resource.data.value +} +`, + }, + expectOpenEphemeralResourceCalled: true, + expectValidateEphemeralResourceConfigCalled: true, + expectCloseEphemeralResourceCalled: true, + }, + "write_only with hardcoded value": { + toBeImplemented: true, module: map[string]string{ "main.tf": ` variable "ephem" { @@ -582,6 +600,7 @@ output "out" { }, "write_only attribute references": { + toBeImplemented: true, module: map[string]string{ "main.tf": ` variable "ephem" { @@ -617,6 +636,10 @@ output "out" { }, } { t.Run(name, func(t *testing.T) { + if tc.toBeImplemented { + t.Skip("To be implemented") + } + m := testModuleInline(t, tc.module) ephem := &testing_provider.MockProvider{ diff --git a/internal/terraform/node_resource_validate.go b/internal/terraform/node_resource_validate.go index 9725f3437999..649e96324572 100644 --- a/internal/terraform/node_resource_validate.go +++ b/internal/terraform/node_resource_validate.go @@ -762,21 +762,21 @@ func validateDependsOn(ctx EvalContext, dependsOn []hcl.Traversal) (diags tfdiag // by calling [tfdiags.Diagnostics.InConfigBody] before returning them to // any caller that expects fully-resolved diagnostics. func validateResourceForbiddenEphemeralValues(ctx EvalContext, value cty.Value, schema *configschema.Block) (diags tfdiags.Diagnostics) { - // NOTE: We take a schema argument in anticipation of a future feature - // that might allow managed resources to declare certain attributes as - // being "write-only", which would create a little nested island where - // ephemeral values are permitted in return for providers accepting that - // those values will not be preserved between plan and apply or between - // sequential plan/apply rounds. But we aren't doing that yet, so we - // just ignore that argument for now. - for _, path := range ephemeral.EphemeralValuePaths(value) { - diags = diags.Append(tfdiags.AttributeValue( - tfdiags.Error, - "Invalid use of ephemeral value", - "Ephemeral values are not valid in resource arguments, because resource instances must persist between Terraform phases.", - path, - )) + attr := schema.AttributeByPath(path) + if attr == nil { + // This should not happen, add a diag + continue + } + + if !attr.WriteOnly { + diags = diags.Append(tfdiags.AttributeValue( + tfdiags.Error, + "Invalid use of ephemeral value", + "Ephemeral values are not valid in resource arguments, because resource instances must persist between Terraform phases.", + path, + )) + } } return diags }