From 3cd2fad15b6d6f8a330ee38a43f243fe31872fcb Mon Sep 17 00:00:00 2001 From: Daniel Schmidt Date: Thu, 21 Nov 2024 13:57:06 +0100 Subject: [PATCH] ephemeral: add support for write-only attributes --- internal/plans/changes.go | 8 +- internal/plans/changes_src.go | 6 + internal/states/instance_object.go | 5 + internal/states/instance_object_src.go | 2 + internal/states/remote/state_test.go | 10 +- internal/states/statefile/version4.go | 3 + .../terraform/context_apply_ephemeral_test.go | 292 +++++++++++++++++- .../terraform/context_plan_ephemeral_test.go | 1 - .../node_resource_abstract_instance.go | 15 +- .../terraform/node_resource_apply_instance.go | 5 + 10 files changed, 335 insertions(+), 12 deletions(-) diff --git a/internal/plans/changes.go b/internal/plans/changes.go index 9d069aa907c9..d4e648f4ad64 100644 --- a/internal/plans/changes.go +++ b/internal/plans/changes.go @@ -582,6 +582,10 @@ type Change struct { // collections/structures. Before, After cty.Value + // 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. // @@ -645,8 +649,8 @@ func (c *Change) Encode(ty cty.Type) (*ChangeSrc, error) { After: afterDV, BeforeSensitivePaths: sensitiveAttrsBefore, AfterSensitivePaths: sensitiveAttrsAfter, - BeforeWriteOnlyPaths: nil, // TODO: Add write-only paths - AfterWriteOnlyPaths: nil, // TODO: Add write-only paths + BeforeWriteOnlyPaths: c.BeforeWriteOnlyPaths, + AfterWriteOnlyPaths: c.AfterWriteOnlyPaths, Importing: c.Importing.Encode(), GeneratedConfig: c.GeneratedConfig, }, nil diff --git a/internal/plans/changes_src.go b/internal/plans/changes_src.go index d82ed4536947..1a03dbd0e22b 100644 --- a/internal/plans/changes_src.go +++ b/internal/plans/changes_src.go @@ -130,6 +130,12 @@ func (c *ChangesSrc) Decode(schemas *schemarepo.Schemas) (*Changes, error) { rc.Before = marks.MarkPaths(rc.Before, marks.Sensitive, rcs.BeforeSensitivePaths) rc.After = marks.MarkPaths(rc.After, marks.Sensitive, rcs.AfterSensitivePaths) + rc.Before = marks.MarkPaths(rc.Before, marks.Ephemeral, rcs.BeforeWriteOnlyPaths) + rc.After = marks.MarkPaths(rc.After, marks.Ephemeral, rcs.BeforeWriteOnlyPaths) + + rc.BeforeWriteOnlyPaths = rcs.BeforeWriteOnlyPaths + rc.AfterWriteOnlyPaths = rcs.AfterWriteOnlyPaths + changes.Resources = append(changes.Resources, rc) } diff --git a/internal/states/instance_object.go b/internal/states/instance_object.go index f398d01c6352..301d7ecc055b 100644 --- a/internal/states/instance_object.go +++ b/internal/states/instance_object.go @@ -50,6 +50,10 @@ type ResourceInstanceObject struct { // destroy operations, we need to record the status to ensure a resource // removed from the config will still be destroyed in the same manner. CreateBeforeDestroy bool + + // 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 } // ObjectStatus represents the status of a RemoteObject. @@ -135,6 +139,7 @@ func (o *ResourceInstanceObject) Encode(ty cty.Type, schemaVersion uint64) (*Res SchemaVersion: schemaVersion, AttrsJSON: src, AttrSensitivePaths: sensitivePaths, + AttrWriteOnlyPaths: o.AttrWriteOnlyPaths, Private: o.Private, Status: o.Status, Dependencies: dependencies, diff --git a/internal/states/instance_object_src.go b/internal/states/instance_object_src.go index 617e2eaf7530..5c44011e2783 100644 --- a/internal/states/instance_object_src.go +++ b/internal/states/instance_object_src.go @@ -100,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 } @@ -111,6 +112,7 @@ func (os *ResourceInstanceObjectSrc) Decode(ty cty.Type) (*ResourceInstanceObjec Dependencies: os.Dependencies, Private: os.Private, CreateBeforeDestroy: os.CreateBeforeDestroy, + AttrWriteOnlyPaths: os.AttrWriteOnlyPaths, }, nil } diff --git a/internal/states/remote/state_test.go b/internal/states/remote/state_test.go index cbb6a6219f07..43dd654fa687 100644 --- a/internal/states/remote/state_test.go +++ b/internal/states/remote/state_test.go @@ -129,8 +129,9 @@ func TestStatePersist(t *testing.T) { "attributes_flat": map[string]interface{}{ "filename": "file.txt", }, - "schema_version": 0.0, - "sensitive_attributes": []interface{}{}, + "schema_version": 0.0, + "sensitive_attributes": []interface{}{}, + "write_only_attributes": []interface{}{}, }, }, "mode": "managed", @@ -167,8 +168,9 @@ func TestStatePersist(t *testing.T) { "attributes_flat": map[string]interface{}{ "filename": "file.txt", }, - "schema_version": 0.0, - "sensitive_attributes": []interface{}{}, + "schema_version": 0.0, + "sensitive_attributes": []interface{}{}, + "write_only_attributes": []interface{}{}, }, }, "mode": "managed", diff --git a/internal/states/statefile/version4.go b/internal/states/statefile/version4.go index d38e0b0b284b..ba47079fbec4 100644 --- a/internal/states/statefile/version4.go +++ b/internal/states/statefile/version4.go @@ -492,6 +492,8 @@ func appendInstanceObjectStateV4(rs *states.Resource, is *states.ResourceInstanc // Marshal paths to JSON attributeSensitivePaths, pathsDiags := marshalPaths(obj.AttrSensitivePaths) diags = diags.Append(pathsDiags) + attributeWriteOnlyPaths, pathsDiags := marshalPaths(obj.AttrWriteOnlyPaths) + diags = diags.Append(pathsDiags) return append(isV4s, instanceObjectStateV4{ IndexKey: rawKey, @@ -501,6 +503,7 @@ func appendInstanceObjectStateV4(rs *states.Resource, is *states.ResourceInstanc AttributesFlat: obj.AttrsFlat, AttributesRaw: obj.AttrsJSON, AttributeSensitivePaths: attributeSensitivePaths, + AttributeWriteOnlyPaths: attributeWriteOnlyPaths, PrivateRaw: privateRaw, Dependencies: deps, CreateBeforeDestroy: obj.CreateBeforeDestroy, diff --git a/internal/terraform/context_apply_ephemeral_test.go b/internal/terraform/context_apply_ephemeral_test.go index 19109466403e..60d34d19c20c 100644 --- a/internal/terraform/context_apply_ephemeral_test.go +++ b/internal/terraform/context_apply_ephemeral_test.go @@ -14,6 +14,7 @@ import ( "github.com/hashicorp/terraform/internal/plans" "github.com/hashicorp/terraform/internal/providers" testing_provider "github.com/hashicorp/terraform/internal/providers/testing" + "github.com/hashicorp/terraform/internal/states" "github.com/zclconf/go-cty/cty" ) @@ -420,7 +421,7 @@ resource "ephem_write_only" "wo" { } if len(plan.Changes.Resources[0].AfterWriteOnlyPaths) != 1 { - t.Fatalf("Expected 1 write-only attribute, got %d", len(plan.Changes.Resources[0].AfterWriteOnlyPaths)) + t.Fatalf("Expected 1 write-only attribute in plan, got %d", len(plan.Changes.Resources[0].AfterWriteOnlyPaths)) } schemas, schemaDiags := ctx.Schemas(m, plan.PriorState) @@ -429,6 +430,288 @@ resource "ephem_write_only" "wo" { 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, + }, + }) + assertNoDiagnostics(t, diags) + + resource := state.Resource(addrs.AbsResource{ + Module: addrs.RootModuleInstance, + Resource: addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "ephem_write_only", + Name: "wo", + }, + }) + + if resource == nil { + t.Fatalf("Resource not found") + } + + resourceInstance := resource.Instances[addrs.NoKey] + if resourceInstance == nil { + t.Fatalf("Resource instance not found") + } + + attrs, err := resourceInstance.Current.Decode(cty.Object(map[string]cty.Type{ + "normal": cty.String, + "write_only": cty.String, + })) + if err != nil { + t.Fatalf("Failed to decode attributes: %v", err) + } + + if attrs.Value.GetAttr("normal").AsString() != "normal" { + t.Fatalf("normal attribute not as expected") + } + + if !attrs.Value.GetAttr("write_only").IsNull() { + t.Fatalf("write_only attribute should be null") + } + + if len(resourceInstance.Current.AttrWriteOnlyPaths) != 1 { + t.Fatalf("Expected 1 write only attribute in apply") + } + + if !resourceInstance.Current.AttrWriteOnlyPaths[0].Equals(cty.GetAttrPath("write_only")) { + t.Fatalf("Expected write_only to be a write only attribute") + } +} + +func TestContext2Apply_update_write_only_attribute_not_in_plan_and_state(t *testing.T) { + m := testModuleInline(t, map[string]string{ + "main.tf": ` +variable "ephem" { + type = string + ephemeral = true +} + +resource "ephem_write_only" "wo" { + normal = "normal" + write_only = var.ephem +} +`, + }) + + ephem := &testing_provider.MockProvider{ + GetProviderSchemaResponse: &providers.GetProviderSchemaResponse{ + ResourceTypes: map[string]providers.Schema{ + "ephem_write_only": { + Block: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "normal": { + Type: cty.String, + Required: true, + }, + "write_only": { + Type: cty.String, + WriteOnly: true, + Required: true, + }, + }, + }, + }, + }, + }, + } + + ctx := testContext2(t, &ContextOpts{ + Providers: map[addrs.Provider]providers.Factory{ + addrs.NewDefaultProvider("ephem"): testProviderFuncFixed(ephem), + }, + }) + + ephemVar := &InputValue{ + Value: cty.StringVal("ephemeral_value"), + SourceType: ValueFromCLIArg, + } + + priorState := states.BuildState(func(state *states.SyncState) { + state.SetResourceInstanceCurrent( + mustResourceInstanceAddr("ephem_write_only.wo"), + &states.ResourceInstanceObjectSrc{ + Status: states.ObjectReady, + AttrsJSON: mustParseJson(map[string]interface{}{ + "normal": "outdated", + }), + }, + addrs.AbsProviderConfig{ + Provider: addrs.NewDefaultProvider("ephem"), + Module: addrs.RootModule, + }) + }) + + plan, diags := ctx.Plan(m, priorState, &PlanOpts{ + Mode: plans.NormalMode, + SetVariables: InputValues{ + "ephem": ephemVar, + }, + }) + 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 in plan, 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, + }, + }) + assertNoDiagnostics(t, diags) + + resource := state.Resource(addrs.AbsResource{ + Module: addrs.RootModuleInstance, + Resource: addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "ephem_write_only", + Name: "wo", + }, + }) + + if resource == nil { + t.Fatalf("Resource not found") + } + + resourceInstance := resource.Instances[addrs.NoKey] + if resourceInstance == nil { + t.Fatalf("Resource instance not found") + } + + attrs, err := resourceInstance.Current.Decode(cty.Object(map[string]cty.Type{ + "normal": cty.String, + "write_only": cty.String, + })) + if err != nil { + t.Fatalf("Failed to decode attributes: %v", err) + } + + if attrs.Value.GetAttr("normal").AsString() != "normal" { + t.Fatalf("normal attribute not as expected") + } + + if !attrs.Value.GetAttr("write_only").IsNull() { + t.Fatalf("write_only attribute should be null") + } + + if len(resourceInstance.Current.AttrWriteOnlyPaths) != 1 { + t.Fatalf("Expected 1 write only attribute in apply") + } + + if !resourceInstance.Current.AttrWriteOnlyPaths[0].Equals(cty.GetAttrPath("write_only")) { + t.Fatalf("Expected write_only to be a write only attribute") + } +} + +func TestContext2Apply_normal_attributes_becomes_write_only_attribute(t *testing.T) { + m := testModuleInline(t, map[string]string{ + "main.tf": ` +variable "ephem" { + type = string + ephemeral = true +} + +resource "ephem_write_only" "wo" { + normal = "normal" + write_only = var.ephem +} +`, + }) + + ephem := &testing_provider.MockProvider{ + GetProviderSchemaResponse: &providers.GetProviderSchemaResponse{ + ResourceTypes: map[string]providers.Schema{ + "ephem_write_only": { + Block: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "normal": { + Type: cty.String, + Required: true, + }, + "write_only": { + Type: cty.String, + WriteOnly: true, + Required: true, + }, + }, + }, + }, + }, + }, + } + + ctx := testContext2(t, &ContextOpts{ + Providers: map[addrs.Provider]providers.Factory{ + addrs.NewDefaultProvider("ephem"): testProviderFuncFixed(ephem), + }, + }) + + ephemVar := &InputValue{ + Value: cty.StringVal("ephemeral_value"), + SourceType: ValueFromCLIArg, + } + + priorState := states.BuildState(func(state *states.SyncState) { + state.SetResourceInstanceCurrent( + mustResourceInstanceAddr("ephem_write_only.wo"), + &states.ResourceInstanceObjectSrc{ + Status: states.ObjectReady, + AttrsJSON: mustParseJson(map[string]interface{}{ + "normal": "normal", + "write_only": "this was not ephemeral but now is", + }), + }, + addrs.AbsProviderConfig{ + Provider: addrs.NewDefaultProvider("ephem"), + Module: addrs.RootModule, + }) + }) + + plan, diags := ctx.Plan(m, priorState, &PlanOpts{ + Mode: plans.NormalMode, + SetVariables: InputValues{ + "ephem": ephemVar, + }, + }) + 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 in plan, 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")) } @@ -465,12 +748,17 @@ resource "ephem_write_only" "wo" { if err != nil { t.Fatalf("Failed to decode attributes: %v", err) } + if attrs.Value.GetAttr("normal").AsString() != "normal" { t.Fatalf("normal attribute not as expected") } + if !attrs.Value.GetAttr("write_only").IsNull() { + t.Fatalf("write_only attribute should be null") + } + if len(resourceInstance.Current.AttrWriteOnlyPaths) != 1 { - t.Fatalf("Expected 1 write only attribute") + t.Fatalf("Expected 1 write only attribute in apply") } if !resourceInstance.Current.AttrWriteOnlyPaths[0].Equals(cty.GetAttrPath("write_only")) { diff --git a/internal/terraform/context_plan_ephemeral_test.go b/internal/terraform/context_plan_ephemeral_test.go index 890fbef742fa..f6b99a1ae24d 100644 --- a/internal/terraform/context_plan_ephemeral_test.go +++ b/internal/terraform/context_plan_ephemeral_test.go @@ -552,7 +552,6 @@ You can correct this by removing references to ephemeral values, or by carefully }, "write_only attribute": { - toBeImplemented: true, module: map[string]string{ "main.tf": ` ephemeral "ephem_resource" "data" { diff --git a/internal/terraform/node_resource_abstract_instance.go b/internal/terraform/node_resource_abstract_instance.go index a6dc801f4889..a8657f046c19 100644 --- a/internal/terraform/node_resource_abstract_instance.go +++ b/internal/terraform/node_resource_abstract_instance.go @@ -17,6 +17,7 @@ import ( "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/configs/configschema" "github.com/hashicorp/terraform/internal/instances" + "github.com/hashicorp/terraform/internal/lang/ephemeral" "github.com/hashicorp/terraform/internal/lang/marks" "github.com/hashicorp/terraform/internal/moduletest/mocking" "github.com/hashicorp/terraform/internal/plans" @@ -1180,6 +1181,10 @@ func (n *NodeAbstractResourceInstance) plan( } } + // We don't need ephemeral values anymore since the value has been planned + ephemeralValuePaths, _ := marks.PathsWithMark(unmarkedPaths, marks.Ephemeral) + plannedNewVal = ephemeral.RemoveEphemeralValues(plannedNewVal) + // Call post-refresh hook diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) { return h.PostDiff(n.HookResourceIdentity(), addrs.NotDeposed, action, priorVal, plannedNewVal) @@ -1202,6 +1207,9 @@ func (n *NodeAbstractResourceInstance) plan( // Marks will be removed when encoding. After: plannedNewVal, GeneratedConfig: n.generatedConfigHCL, + + BeforeWriteOnlyPaths: ephemeral.EphemeralValuePaths(priorVal), + AfterWriteOnlyPaths: ephemeralValuePaths, }, ActionReason: actionReason, RequiredReplace: reqRep, @@ -1215,9 +1223,10 @@ func (n *NodeAbstractResourceInstance) plan( // must _also_ record the returned change in the active plan, // which the expression evaluator will use in preference to this // incomplete value recorded in the state. - Status: states.ObjectPlanned, - Value: plannedNewVal, - Private: plannedPrivate, + Status: states.ObjectPlanned, + Value: plannedNewVal, + Private: plannedPrivate, + AttrWriteOnlyPaths: ephemeralValuePaths, } return plan, state, deferred, keyData, diags diff --git a/internal/terraform/node_resource_apply_instance.go b/internal/terraform/node_resource_apply_instance.go index 552807646c96..b3da83443d77 100644 --- a/internal/terraform/node_resource_apply_instance.go +++ b/internal/terraform/node_resource_apply_instance.go @@ -10,6 +10,7 @@ import ( "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/instances" + "github.com/hashicorp/terraform/internal/lang/ephemeral" "github.com/hashicorp/terraform/internal/plans" "github.com/hashicorp/terraform/internal/plans/objchange" "github.com/hashicorp/terraform/internal/providers" @@ -323,6 +324,10 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext) if state != nil { // dependencies are always updated to match the configuration during apply state.Dependencies = n.Dependencies + + // The value is updated to remove ephemeral values + state.Value = ephemeral.RemoveEphemeralValues(state.Value) + state.AttrWriteOnlyPaths = diffApply.AfterWriteOnlyPaths } err = n.writeResourceInstanceState(ctx, state, workingState) if err != nil {