Merge pull request #30029 from hashicorp/alisdair/add-sensitive-marks-for-nested-attributes

configs: Add sensitive marks for nested attributes

internal/configs/configschema/implied_type.go

@@ -44,6 +44,9 @@ func (b *Block) ContainsSensitive() bool {
 		if attrS.Sensitive {
 			return true
 		}
+		if attrS.NestedType != nil && attrS.NestedType.ContainsSensitive() {
+			return true
+		}
 	}
 	for _, blockS := range b.BlockTypes {
 		if blockS.ContainsSensitive() {
@@ -108,8 +111,8 @@ func (o *Object) ContainsSensitive() bool {
 		if attrS.Sensitive {
 			return true
 		}
-		if attrS.NestedType != nil {
+		if attrS.NestedType != nil && attrS.NestedType.ContainsSensitive() {
-			return attrS.NestedType.ContainsSensitive()
+			return true
 		}
 	}
 	return false

internal/configs/configschema/implied_type_test.go

@@ -154,6 +154,70 @@ func TestBlockImpliedType(t *testing.T) {
 	}
 }
 
+func TestBlockContainsSensitive(t *testing.T) {
+	tests := map[string]struct {
+		Schema *Block
+		Want   bool
+	}{
+		"object contains sensitive": {
+			&Block{
+				Attributes: map[string]*Attribute{
+					"sensitive": {Sensitive: true},
+				},
+			},
+			true,
+		},
+		"no sensitive attrs": {
+			&Block{
+				Attributes: map[string]*Attribute{
+					"insensitive": {},
+				},
+			},
+			false,
+		},
+		"nested object contains sensitive": {
+			&Block{
+				Attributes: map[string]*Attribute{
+					"nested": {
+						NestedType: &Object{
+							Nesting: NestingSingle,
+							Attributes: map[string]*Attribute{
+								"sensitive": {Sensitive: true},
+							},
+						},
+					},
+				},
+			},
+			true,
+		},
+		"nested obj, no sensitive attrs": {
+			&Block{
+				Attributes: map[string]*Attribute{
+					"nested": {
+						NestedType: &Object{
+							Nesting: NestingSingle,
+							Attributes: map[string]*Attribute{
+								"public": {},
+							},
+						},
+					},
+				},
+			},
+			false,
+		},
+	}
+
+	for name, test := range tests {
+		t.Run(name, func(t *testing.T) {
+			got := test.Schema.ContainsSensitive()
+			if got != test.Want {
+				t.Errorf("wrong result\ngot:  %#v\nwant: %#v", got, test.Want)
+			}
+		})
+	}
+
+}
+
 func TestObjectImpliedType(t *testing.T) {
 	tests := map[string]struct {
 		Schema *Object
@@ -353,6 +417,37 @@ func TestObjectContainsSensitive(t *testing.T) {
 			},
 			false,
 		},
+		"several nested objects, one contains sensitive": {
+			&Object{
+				Attributes: map[string]*Attribute{
+					"alpha": {
+						NestedType: &Object{
+							Nesting: NestingSingle,
+							Attributes: map[string]*Attribute{
+								"nonsensitive": {},
+							},
+						},
+					},
+					"beta": {
+						NestedType: &Object{
+							Nesting: NestingSingle,
+							Attributes: map[string]*Attribute{
+								"sensitive": {Sensitive: true},
+							},
+						},
+					},
+					"gamma": {
+						NestedType: &Object{
+							Nesting: NestingSingle,
+							Attributes: map[string]*Attribute{
+								"nonsensitive": {},
+							},
+						},
+					},
+				},
+			},
+			true,
+		},
 	}
 
 	for name, test := range tests {

internal/configs/configschema/marks.go

@@ -12,6 +12,8 @@ import (
 // blocks are descended (if present in the given value).
 func (b *Block) ValueMarks(val cty.Value, path cty.Path) []cty.PathValueMarks {
 	var pvm []cty.PathValueMarks
+
+	// We can mark attributes as sensitive even if the value is null
 	for name, attrS := range b.Attributes {
 		if attrS.Sensitive {
 			// Create a copy of the path, with this step added, to add to our PathValueMarks slice
@@ -25,9 +27,28 @@ func (b *Block) ValueMarks(val cty.Value, path cty.Path) []cty.PathValueMarks {
 		}
 	}
 
+	// If the value is null, no other marks are possible
 	if val.IsNull() {
 		return pvm
 	}
+
+	// Extract marks for nested attribute type values
+	for name, attrS := range b.Attributes {
+		// If the attribute has no nested type, or the nested type doesn't
+		// contain any sensitive attributes, skip inspecting it
+		if attrS.NestedType == nil || !attrS.NestedType.ContainsSensitive() {
+			continue
+		}
+
+		// Create a copy of the path, with this step added, to add to our PathValueMarks slice
+		attrPath := make(cty.Path, len(path), len(path)+1)
+		copy(attrPath, path)
+		attrPath = append(path, cty.GetAttrStep{Name: name})
+
+		pvm = append(pvm, attrS.NestedType.ValueMarks(val.GetAttr(name), attrPath)...)
+	}
+
+	// Extract marks for nested blocks
 	for name, blockS := range b.BlockTypes {
 		// If our block doesn't contain any sensitive attributes, skip inspecting it
 		if !blockS.Block.ContainsSensitive() {
@@ -59,3 +80,72 @@ func (b *Block) ValueMarks(val cty.Value, path cty.Path) []cty.PathValueMarks {
 	}
 	return pvm
 }
+
+// ValueMarks returns a set of path value marks for a given value and path,
+// based on the sensitive flag for each attribute within the nested attribute.
+// Attributes with nested types are descended (if present in the given value).
+func (o *Object) ValueMarks(val cty.Value, path cty.Path) []cty.PathValueMarks {
+	var pvm []cty.PathValueMarks
+
+	if val.IsNull() || !val.IsKnown() {
+		return pvm
+	}
+
+	for name, attrS := range o.Attributes {
+		// Skip attributes which can never produce sensitive path value marks
+		if !attrS.Sensitive && (attrS.NestedType == nil || !attrS.NestedType.ContainsSensitive()) {
+			continue
+		}
+
+		switch o.Nesting {
+		case NestingSingle, NestingGroup:
+			// Create a path to this attribute
+			attrPath := make(cty.Path, len(path), len(path)+1)
+			copy(attrPath, path)
+			attrPath = append(path, cty.GetAttrStep{Name: name})
+
+			if attrS.Sensitive {
+				// If the entire attribute is sensitive, mark it so
+				pvm = append(pvm, cty.PathValueMarks{
+					Path:  attrPath,
+					Marks: cty.NewValueMarks(marks.Sensitive),
+				})
+			} else {
+				// The attribute has a nested type which contains sensitive
+				// attributes, so recurse
+				pvm = append(pvm, attrS.NestedType.ValueMarks(val.GetAttr(name), attrPath)...)
+			}
+		case NestingList, NestingMap, NestingSet:
+			// For nested attribute types which have a non-single nesting mode,
+			// we add path value marks for each element of the collection
+			for it := val.ElementIterator(); it.Next(); {
+				idx, attrEV := it.Element()
+				attrV := attrEV.GetAttr(name)
+
+				// Create a path to this element of the attribute's collection. Note
+				// that the path is extended in opposite order to the iteration order
+				// of the loops: index into the collection, then the contained
+				// attribute name. This is because we have one type
+				// representing multiple collection elements.
+				attrPath := make(cty.Path, len(path), len(path)+2)
+				copy(attrPath, path)
+				attrPath = append(path, cty.IndexStep{Key: idx}, cty.GetAttrStep{Name: name})
+
+				if attrS.Sensitive {
+					// If the entire attribute is sensitive, mark it so
+					pvm = append(pvm, cty.PathValueMarks{
+						Path:  attrPath,
+						Marks: cty.NewValueMarks(marks.Sensitive),
+					})
+				} else {
+					// The attribute has a nested type which contains sensitive
+					// attributes, so recurse
+					pvm = append(pvm, attrS.NestedType.ValueMarks(attrV, attrPath)...)
+				}
+			}
+		default:
+			panic(fmt.Sprintf("unsupported nesting mode %s", attrS.NestedType.Nesting))
+		}
+	}
+	return pvm
+}

internal/configs/configschema/marks_test.go

@@ -1,7 +1,6 @@
 package configschema
 
 import (
-	"fmt"
 	"testing"
 
 	"github.com/hashicorp/terraform/internal/lang/marks"
@@ -19,6 +18,20 @@ func TestBlockValueMarks(t *testing.T) {
 				Type:      cty.String,
 				Sensitive: true,
 			},
+			"nested": {
+				NestedType: &Object{
+					Attributes: map[string]*Attribute{
+						"boop": {
+							Type: cty.String,
+						},
+						"honk": {
+							Type:      cty.String,
+							Sensitive: true,
+						},
+					},
+					Nesting: NestingList,
+				},
+			},
 		},
 
 		BlockTypes: map[string]*NestedBlock{
@@ -40,34 +53,46 @@ func TestBlockValueMarks(t *testing.T) {
 		},
 	}
 
-	for _, tc := range []struct {
+	testCases := map[string]struct {
 		given  cty.Value
 		expect cty.Value
 	}{
-		{
+		"unknown object": {
 			cty.UnknownVal(schema.ImpliedType()),
 			cty.UnknownVal(schema.ImpliedType()),
 		},
-		{
+		"null object": {
 			cty.NullVal(schema.ImpliedType()),
 			cty.NullVal(schema.ImpliedType()),
 		},
-		{
+		"object with unknown attributes and blocks": {
 			cty.ObjectVal(map[string]cty.Value{
 				"sensitive":   cty.UnknownVal(cty.String),
 				"unsensitive": cty.UnknownVal(cty.String),
-				"list":        cty.UnknownVal(schema.BlockTypes["list"].ImpliedType()),
+				"nested": cty.NullVal(cty.List(cty.Object(map[string]cty.Type{
+					"boop": cty.String,
+					"honk": cty.String,
+				}))),
+				"list": cty.UnknownVal(schema.BlockTypes["list"].ImpliedType()),
 			}),
 			cty.ObjectVal(map[string]cty.Value{
 				"sensitive":   cty.UnknownVal(cty.String).Mark(marks.Sensitive),
 				"unsensitive": cty.UnknownVal(cty.String),
-				"list":        cty.UnknownVal(schema.BlockTypes["list"].ImpliedType()),
+				"nested": cty.NullVal(cty.List(cty.Object(map[string]cty.Type{
+					"boop": cty.String,
+					"honk": cty.String,
+				}))),
+				"list": cty.UnknownVal(schema.BlockTypes["list"].ImpliedType()),
 			}),
 		},
-		{
+		"object with block value": {
 			cty.ObjectVal(map[string]cty.Value{
 				"sensitive":   cty.NullVal(cty.String),
 				"unsensitive": cty.UnknownVal(cty.String),
+				"nested": cty.NullVal(cty.List(cty.Object(map[string]cty.Type{
+					"boop": cty.String,
+					"honk": cty.String,
+				}))),
 				"list": cty.ListVal([]cty.Value{
 					cty.ObjectVal(map[string]cty.Value{
 						"sensitive":   cty.UnknownVal(cty.String),
@@ -82,6 +107,10 @@ func TestBlockValueMarks(t *testing.T) {
 			cty.ObjectVal(map[string]cty.Value{
 				"sensitive":   cty.NullVal(cty.String).Mark(marks.Sensitive),
 				"unsensitive": cty.UnknownVal(cty.String),
+				"nested": cty.NullVal(cty.List(cty.Object(map[string]cty.Type{
+					"boop": cty.String,
+					"honk": cty.String,
+				}))),
 				"list": cty.ListVal([]cty.Value{
 					cty.ObjectVal(map[string]cty.Value{
 						"sensitive":   cty.UnknownVal(cty.String).Mark(marks.Sensitive),
@@ -94,8 +123,56 @@ func TestBlockValueMarks(t *testing.T) {
 				}),
 			}),
 		},
-	} {
+		"object with known values and nested attribute": {
-		t.Run(fmt.Sprintf("%#v", tc.given), func(t *testing.T) {
+			cty.ObjectVal(map[string]cty.Value{
+				"sensitive":   cty.StringVal("foo"),
+				"unsensitive": cty.StringVal("bar"),
+				"nested": cty.ListVal([]cty.Value{
+					cty.ObjectVal(map[string]cty.Value{
+						"boop": cty.StringVal("foo"),
+						"honk": cty.StringVal("bar"),
+					}),
+					cty.ObjectVal(map[string]cty.Value{
+						"boop": cty.NullVal(cty.String),
+						"honk": cty.NullVal(cty.String),
+					}),
+					cty.ObjectVal(map[string]cty.Value{
+						"boop": cty.UnknownVal(cty.String),
+						"honk": cty.UnknownVal(cty.String),
+					}),
+				}),
+				"list": cty.NullVal(cty.List(cty.Object(map[string]cty.Type{
+					"sensitive":   cty.String,
+					"unsensitive": cty.String,
+				}))),
+			}),
+			cty.ObjectVal(map[string]cty.Value{
+				"sensitive":   cty.StringVal("foo").Mark(marks.Sensitive),
+				"unsensitive": cty.StringVal("bar"),
+				"nested": cty.ListVal([]cty.Value{
+					cty.ObjectVal(map[string]cty.Value{
+						"boop": cty.StringVal("foo"),
+						"honk": cty.StringVal("bar").Mark(marks.Sensitive),
+					}),
+					cty.ObjectVal(map[string]cty.Value{
+						"boop": cty.NullVal(cty.String),
+						"honk": cty.NullVal(cty.String).Mark(marks.Sensitive),
+					}),
+					cty.ObjectVal(map[string]cty.Value{
+						"boop": cty.UnknownVal(cty.String),
+						"honk": cty.UnknownVal(cty.String).Mark(marks.Sensitive),
+					}),
+				}),
+				"list": cty.NullVal(cty.List(cty.Object(map[string]cty.Type{
+					"sensitive":   cty.String,
+					"unsensitive": cty.String,
+				}))),
+			}),
+		},
+	}
+
+	for name, tc := range testCases {
+		t.Run(name, func(t *testing.T) {
 			got := tc.given.MarkWithPaths(schema.ValueMarks(tc.given, nil))
 			if !got.RawEquals(tc.expect) {
 				t.Fatalf("\nexpected: %#v\ngot:      %#v\n", tc.expect, got)