Skip to content

Commit

Permalink
ephemeral: handle write-only attributes in stackdata
Browse files Browse the repository at this point in the history
  • Loading branch information
DanielMSchmidt committed Nov 19, 2024
1 parent 8a22769 commit 90a9119
Show file tree
Hide file tree
Showing 8 changed files with 1,057 additions and 825 deletions.
19 changes: 16 additions & 3 deletions internal/rpcapi/terraform1/stacks/conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,13 @@ func ToDynamicValue(from cty.Value, ty cty.Type) (*DynamicValue, error) {
// Separate out sensitive marks from the decoded value so we can re-serialize it
// with MessagePack. Sensitive paths get encoded separately in the final message.
unmarkedValue, markses := from.UnmarkDeepWithPaths()
sensitivePaths, otherMarkses := marks.PathsWithMark(markses, marks.Sensitive)

// TODO: I used this pattern a bunch of times but only now noticed that values which are marked
// two ways (which is explicitly allowed by the spec) are in both lists. I will need to handle that somehow.

sensitivePaths, nonSensitiveMarks := marks.PathsWithMark(markses, marks.Sensitive)
ephemeralPaths, otherMarkses := marks.PathsWithMark(nonSensitiveMarks, marks.Ephemeral)

if len(otherMarkses) != 0 {
// Any other marks should've been dealt with by our caller before
// getting here, since we only know how to preserve the sensitive
Expand All @@ -67,7 +73,7 @@ func ToDynamicValue(from cty.Value, ty cty.Type) (*DynamicValue, error) {
if err != nil {
return nil, err
}
return NewDynamicValue(encValue, sensitivePaths), nil
return NewDynamicValue(encValue, sensitivePaths, ephemeralPaths), nil
}

// NewDynamicValue constructs a [DynamicValue] message object from a
Expand All @@ -77,7 +83,7 @@ func ToDynamicValue(from cty.Value, ty cty.Type) (*DynamicValue, error) {
// The plans package represents the sensitive value mark as a separate field
// in [plans.ChangeSrc] rather than as part of the value itself, so callers must
// also provide a separate set of paths that are marked as sensitive.
func NewDynamicValue(from plans.DynamicValue, sensitivePaths []cty.Path) *DynamicValue {
func NewDynamicValue(from plans.DynamicValue, sensitivePaths []cty.Path, writeOnlyPaths []cty.Path) *DynamicValue {
// plans.DynamicValue is always MessagePack-serialized today, so we'll
// just write its bytes into the field for msgpack serialization
// unconditionally. If plans.DynamicValue grows to support different
Expand All @@ -93,6 +99,13 @@ func NewDynamicValue(from plans.DynamicValue, sensitivePaths []cty.Path) *Dynami
}
}

if len(writeOnlyPaths) != 0 {
ret.WriteOnly = make([]*AttributePath, 0, len(writeOnlyPaths))
for _, path := range writeOnlyPaths {
ret.WriteOnly = append(ret.WriteOnly, NewAttributePath(path))
}
}

return ret
}

Expand Down
1,558 changes: 786 additions & 772 deletions internal/rpcapi/terraform1/stacks/stacks.pb.go

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions internal/rpcapi/terraform1/stacks/stacks.proto
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,7 @@ message InspectExpressionResult {
message DynamicValue {
bytes msgpack = 1; // The default serialization format
repeated AttributePath sensitive = 2; // Paths to any sensitive-marked values.
repeated AttributePath write_only = 3; // Paths to any write-only-marked values.
}

// Represents a change of some object from one dynamic value to another.
Expand Down
2 changes: 2 additions & 0 deletions internal/stacks/stackplan/planned_change.go
Original file line number Diff line number Diff line change
Expand Up @@ -481,10 +481,12 @@ func (pc *PlannedChangeResourceInstancePlanned) ChangeDescription() (*stacks.Pla
Old: stacks.NewDynamicValue(
pc.ChangeSrc.Before,
pc.ChangeSrc.BeforeSensitivePaths,
pc.ChangeSrc.BeforeWriteOnlyPaths,
),
New: stacks.NewDynamicValue(
pc.ChangeSrc.After,
pc.ChangeSrc.AfterSensitivePaths,
pc.ChangeSrc.AfterWriteOnlyPaths,
),
},
ReplacePaths: replacePaths,
Expand Down
35 changes: 30 additions & 5 deletions internal/stacks/tfstackdata1/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,12 @@ func ResourceInstanceObjectStateToTFStackData1(objSrc *states.ResourceInstanceOb
// attribute paths here just so we don't need to reimplement the
// slice-of-paths conversion in yet another place. We don't
// actually do anything with the value part of this.
protoValue := stacks.NewDynamicValue(plans.DynamicValue(nil), objSrc.AttrSensitivePaths)
protoValue := stacks.NewDynamicValue(plans.DynamicValue(nil), objSrc.AttrSensitivePaths, objSrc.AttrWriteOnlyPaths)
rawMsg := &StateResourceInstanceObjectV1{
SchemaVersion: objSrc.SchemaVersion,
ValueJson: objSrc.AttrsJSON,
SensitivePaths: Terraform1ToPlanProtoAttributePaths(protoValue.Sensitive),
WriteOnlyPaths: Terraform1ToPlanProtoAttributePaths(protoValue.WriteOnly),
CreateBeforeDestroy: objSrc.CreateBeforeDestroy,
ProviderConfigAddr: providerConfigAddr.String(),
ProviderSpecificData: objSrc.Private,
Expand Down Expand Up @@ -61,6 +62,7 @@ func Terraform1ToStackDataDynamicValue(value *stacks.DynamicValue) *DynamicValue
Msgpack: value.Msgpack,
},
SensitivePaths: Terraform1ToPlanProtoAttributePaths(value.Sensitive),
WriteOnlyPaths: Terraform1ToPlanProtoAttributePaths(value.WriteOnly),
}
}

Expand All @@ -87,6 +89,20 @@ func DynamicValueFromTFStackData1(protoVal *DynamicValue, ty cty.Type) (cty.Valu
})
}
}

if len(protoVal.WriteOnlyPaths) != 0 {
marks := cty.NewValueMarks(marks.Ephemeral)
for _, protoPath := range protoVal.WriteOnlyPaths {
path, err := planfile.PathFromProto(protoPath)
if err != nil {
return cty.NilVal, fmt.Errorf("invalid ephemeral value path: %w", err)
}
markses = append(markses, cty.PathValueMarks{
Path: path,
Marks: marks,
})
}
}
return unmarkedV.MarkWithPaths(markses), nil
}

Expand Down Expand Up @@ -173,16 +189,25 @@ func DecodeProtoResourceInstanceObject(protoObj *StateResourceInstanceObjectV1)
return nil, fmt.Errorf("unsupported status %s", protoObj.Status.String())
}

paths := make([]cty.Path, 0, len(protoObj.SensitivePaths))
sensitivePaths := make([]cty.Path, 0, len(protoObj.SensitivePaths))
for _, p := range protoObj.SensitivePaths {
path, err := planfile.PathFromProto(p)
if err != nil {
return nil, err
}
paths = append(paths, path)
sensitivePaths = append(sensitivePaths, path)
}
objSrc.AttrSensitivePaths = sensitivePaths

writeOnlyPaths := make([]cty.Path, 0, len(protoObj.WriteOnlyPaths))
for _, p := range protoObj.WriteOnlyPaths {
path, err := planfile.PathFromProto(p)
if err != nil {
return nil, err
}
writeOnlyPaths = append(writeOnlyPaths, path)
}
objSrc.AttrSensitivePaths = paths
// TODO: Handle write only paths
objSrc.AttrWriteOnlyPaths = writeOnlyPaths

if len(protoObj.Dependencies) != 0 {
objSrc.Dependencies = make([]addrs.ConfigResource, len(protoObj.Dependencies))
Expand Down
150 changes: 150 additions & 0 deletions internal/stacks/tfstackdata1/convert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,3 +148,153 @@ func TestDynamicValueFromTFStackData1(t *testing.T) {
t.Errorf("wrong result\ngot: %#v\nwant: %#v", got, want)
}
}

func TestDynamicValueWithEphemeralMarks(t *testing.T) {
startVal := cty.ObjectVal(map[string]cty.Value{
"x": cty.StringVal("x").Mark(marks.Ephemeral),
"y": cty.StringVal("y"),
"z": cty.ListVal([]cty.Value{
cty.StringVal("z[0]"),
cty.StringVal("z[1]").Mark(marks.Ephemeral),
}),
})
ty := startVal.Type()

partial, err := stacks.ToDynamicValue(startVal, ty)
if err != nil {
t.Fatalf("unexpected error: %s", err)
}
got := Terraform1ToStackDataDynamicValue(partial)
want := &DynamicValue{
Value: &planproto.DynamicValue{
Msgpack: []byte("\x83\xa1x\xa1x\xa1y\xa1y\xa1z\x92\xa4z[0]\xa4z[1]"),
},

WriteOnlyPaths: []*planproto.Path{
{
Steps: []*planproto.Path_Step{
{
Selector: &planproto.Path_Step_AttributeName{
AttributeName: "x",
},
},
},
},
{
Steps: []*planproto.Path_Step{
{
Selector: &planproto.Path_Step_AttributeName{
AttributeName: "z",
},
},
{
Selector: &planproto.Path_Step_ElementKey{
ElementKey: &planproto.DynamicValue{
Msgpack: []byte{0b00000001}, // MessagePack-encoded fixint 1
},
},
},
},
},
},
}

if len(got.WriteOnlyPaths) == 2 && len(got.WriteOnlyPaths[0].Steps) == 2 {
got.WriteOnlyPaths[0], got.WriteOnlyPaths[1] = got.WriteOnlyPaths[1], got.WriteOnlyPaths[0]
}

if diff := cmp.Diff(want, got, protocmp.Transform()); diff != "" {
t.Errorf("wrong result\n%s", diff)
}
}

func TestDynamicValueWithEphemeralAndSensitiveMarks(t *testing.T) {
startVal := cty.ObjectVal(map[string]cty.Value{
"m": cty.StringVal("m").Mark(marks.Ephemeral).Mark(marks.Sensitive),
"n": cty.StringVal("n"),
"o": cty.ListVal([]cty.Value{
cty.StringVal("o[0]"),
cty.StringVal("o[1]").Mark(marks.Ephemeral).Mark(marks.Sensitive),
}),
})
ty := startVal.Type()

partial, err := stacks.ToDynamicValue(startVal, ty)
if err != nil {
t.Fatalf("unexpected error: %s", err)
}
got := Terraform1ToStackDataDynamicValue(partial)
want := &DynamicValue{
Value: &planproto.DynamicValue{
Msgpack: []byte("\x83\xa1m\xa1m\xa1n\xa1n\xa1o\x92\xa4o[0]\xa4o[1]"),
},

SensitivePaths: []*planproto.Path{
{
Steps: []*planproto.Path_Step{
{
Selector: &planproto.Path_Step_AttributeName{
AttributeName: "m",
},
},
},
},
{
Steps: []*planproto.Path_Step{
{
Selector: &planproto.Path_Step_AttributeName{
AttributeName: "o",
},
},
{
Selector: &planproto.Path_Step_ElementKey{
ElementKey: &planproto.DynamicValue{
Msgpack: []byte{0b00000001}, // MessagePack-encoded fixint 1
},
},
},
},
},
},

WriteOnlyPaths: []*planproto.Path{
{
Steps: []*planproto.Path_Step{
{
Selector: &planproto.Path_Step_AttributeName{
AttributeName: "m",
},
},
},
},
{
Steps: []*planproto.Path_Step{
{
Selector: &planproto.Path_Step_AttributeName{
AttributeName: "o",
},
},
{
Selector: &planproto.Path_Step_ElementKey{
ElementKey: &planproto.DynamicValue{
Msgpack: []byte{0b00000001}, // MessagePack-encoded fixint 1
},
},
},
},
},
},
}

if len(got.SensitivePaths) == 2 && len(got.SensitivePaths[0].Steps) == 2 {
got.SensitivePaths[0], got.SensitivePaths[1] = got.SensitivePaths[1], got.SensitivePaths[0]
}

if len(got.WriteOnlyPaths) == 2 && len(got.WriteOnlyPaths[0].Steps) == 2 {
got.WriteOnlyPaths[0], got.WriteOnlyPaths[1] = got.WriteOnlyPaths[1], got.WriteOnlyPaths[0]
}

if diff := cmp.Diff(want, got, protocmp.Transform()); diff != "" {
t.Errorf("wrong result\n%s", diff)
}
}
Loading

0 comments on commit 90a9119

Please sign in to comment.