Skip to content

Commit

Permalink
ephemeral: support write-only attributes
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
DanielMSchmidt committed Nov 19, 2024
1 parent 30b9379 commit 8a22769
Show file tree
Hide file tree
Showing 17 changed files with 271 additions and 82 deletions.
19 changes: 13 additions & 6 deletions internal/command/jsonstate/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down
34 changes: 32 additions & 2 deletions internal/command/jsonstate/state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,30 +118,35 @@ 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{
"foo": cty.StringVal("bar"),
}),
AttributeValues{"foo": json.RawMessage(`"bar"`)},
nil,
nil,
},
{
cty.ObjectVal(map[string]cty.Value{
"foo": cty.NullVal(cty.String),
}),
AttributeValues{"foo": json.RawMessage(`null`)},
nil,
nil,
},
{
cty.ObjectVal(map[string]cty.Value{
Expand All @@ -158,6 +163,7 @@ func TestMarshalAttributeValues(t *testing.T) {
"baz": json.RawMessage(`["goodnight","moon"]`),
},
nil,
nil,
},
// Sensitive values
{
Expand All @@ -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)
}
Expand All @@ -192,14 +219,17 @@ 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)
}
})
}

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 {
Expand Down
40 changes: 40 additions & 0 deletions internal/lang/ephemeral/marshal.go
Original file line number Diff line number Diff line change
@@ -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
}
51 changes: 51 additions & 0 deletions internal/lang/ephemeral/marshal_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
})
}
}
25 changes: 2 additions & 23 deletions internal/lang/funcs/conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
},
})

Expand Down
13 changes: 11 additions & 2 deletions internal/plans/changes.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)",
Expand Down Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions internal/plans/changes_src.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
//
Expand Down
6 changes: 6 additions & 0 deletions internal/plans/objchange/compatible.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
1 change: 1 addition & 0 deletions internal/stacks/tfstackdata1/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
Loading

0 comments on commit 8a22769

Please sign in to comment.