Address review comments

Co-authored-by: James Humphries <James@james-humphries.co.uk>
Co-authored-by: Ilia Gogotchuri <ilia.gogotchuri0@gmail.com>
Signed-off-by: Christian Mesh <christianmesh1@gmail.com>

internal/backend/local/backend_local.go

@@ -429,17 +429,17 @@ func generateEphemeralPlanValues(vv map[string]backend.UnparsedVariableValue, vc
 	if diags.HasErrors() {
 		return nil, diags
 	}
-	ephemeralVars, ephemeralDiags := ephemeralValuesForPlanDuringFromUserInput(parsedVars, vcfgs)
+	ephemeralVars, ephemeralDiags := ephemeralValuesForPlanFromVariables(parsedVars, vcfgs)
 	diags = diags.Append(ephemeralDiags)
 	return ephemeralVars, diags
 }
 
-// ephemeralValuesForPlanDuringFromUserInput is creating a map ready to be given to the plan to merge these together
+// ephemeralValuesForPlanFromVariables is creating a map ready to be given to the plan to merge these together
 // with the variables that are already in the plan.
 // This function is handling only the ephemeral variables since those are the only ones that are not
 // stored in the plan, so the only way to pass then into the apply phase is to provide them again
 // in the -var/-var-file.
-func ephemeralValuesForPlanDuringFromUserInput(parsedVars tofu.InputValues, variables map[string]*configs.Variable) (map[string]cty.Value, tfdiags.Diagnostics) {
+func ephemeralValuesForPlanFromVariables(parsedVars tofu.InputValues, variables map[string]*configs.Variable) (map[string]cty.Value, tfdiags.Diagnostics) {
 	var diags tfdiags.Diagnostics
 	out := make(map[string]cty.Value)
 	for vn, vc := range variables {

internal/command/format/object_id.go

@@ -10,6 +10,10 @@ import (
 	"github.com/zclconf/go-cty/cty"
 )
 
+func isValueMarkedUnusable(v cty.Value) bool {
+	return v.HasMark(marks.Sensitive) || v.HasMark(marks.Ephemeral)
+}
+
 // ObjectValueID takes a value that is assumed to be an object representation
 // of some resource instance object and attempts to heuristically find an
 // attribute of it that is likely to be a unique identifier in the remote
@@ -36,7 +40,7 @@ func ObjectValueID(obj cty.Value) (k, v string) {
 
 	case atys["id"] == cty.String:
 		v := obj.GetAttr("id")
-		if v.HasMark(marks.Sensitive) || v.HasMark(marks.Ephemeral) {
+		if isValueMarkedUnusable(v) {
 			break
 		}
 		v, _ = v.Unmark()
@@ -49,7 +53,7 @@ func ObjectValueID(obj cty.Value) (k, v string) {
 		// "name" isn't always globally unique, but if there isn't also an
 		// "id" then it _often_ is, in practice.
 		v := obj.GetAttr("name")
-		if v.HasMark(marks.Sensitive) || v.HasMark(marks.Ephemeral) {
+		if isValueMarkedUnusable(v) {
 			break
 		}
 		v, _ = v.Unmark()
@@ -93,7 +97,7 @@ func ObjectValueName(obj cty.Value) (k, v string) {
 
 	case atys["name"] == cty.String:
 		v := obj.GetAttr("name")
-		if v.HasMark(marks.Sensitive) || v.HasMark(marks.Ephemeral) {
+		if isValueMarkedUnusable(v) {
 			break
 		}
 		v, _ = v.Unmark()
@@ -104,7 +108,7 @@ func ObjectValueName(obj cty.Value) (k, v string) {
 
 	case atys["tags"].IsMapType() && atys["tags"].ElementType() == cty.String:
 		tags := obj.GetAttr("tags")
-		if tags.IsNull() || !tags.IsWhollyKnown() || tags.HasMark(marks.Sensitive) || tags.HasMark(marks.Ephemeral) {
+		if tags.IsNull() || !tags.IsWhollyKnown() || isValueMarkedUnusable(tags) {
 			break
 		}
 		tags, _ = tags.Unmark()
@@ -112,7 +116,7 @@ func ObjectValueName(obj cty.Value) (k, v string) {
 		switch {
 		case tags.HasIndex(cty.StringVal("name")).RawEquals(cty.True):
 			v := tags.Index(cty.StringVal("name"))
-			if v.HasMark(marks.Sensitive) || v.HasMark(marks.Ephemeral) {
+			if isValueMarkedUnusable(v) {
 				break
 			}
 			v, _ = v.Unmark()
@@ -123,7 +127,7 @@ func ObjectValueName(obj cty.Value) (k, v string) {
 		case tags.HasIndex(cty.StringVal("Name")).RawEquals(cty.True):
 			// AWS-style naming convention
 			v := tags.Index(cty.StringVal("Name"))
-			if v.HasMark(marks.Sensitive) || v.HasMark(marks.Ephemeral) {
+			if isValueMarkedUnusable(v) {
 				break
 			}
 			v, _ = v.Unmark()

internal/genconfig/generate_config.go

@@ -95,7 +95,7 @@ func writeConfigAttributes(addr addrs.AbsResourceInstance, buf *strings.Builder,
 			if !hclsyntax.ValidIdentifier(name) {
 				name = string(hclwrite.TokensForValue(cty.StringVal(name)).Bytes())
 			}
-			_, _ = fmt.Fprintf(buf, "%s = ", name)
+			fmt.Fprintf(buf, "%s = ", name)
 			tok := hclwrite.TokensForValue(attrS.EmptyValue())
 			if _, err := tok.WriteTo(buf); err != nil {
 				diags = diags.Append(&hcl.Diagnostic{
@@ -113,7 +113,7 @@ func writeConfigAttributes(addr addrs.AbsResourceInstance, buf *strings.Builder,
 			if !hclsyntax.ValidIdentifier(name) {
 				name = string(hclwrite.TokensForValue(cty.StringVal(name)).Bytes())
 			}
-			_, _ = fmt.Fprintf(buf, "%s = ", name)
+			fmt.Fprintf(buf, "%s = ", name)
 			tok := hclwrite.TokensForValue(attrS.EmptyValue())
 			if _, err := tok.WriteTo(buf); err != nil {
 				diags = diags.Append(&hcl.Diagnostic{
@@ -154,7 +154,7 @@ func writeConfigAttributesFromExisting(addr addrs.AbsResourceInstance, buf *stri
 		// Exclude computed-only attributes
 		if attrS.Required || attrS.Optional {
 			buf.WriteString(strings.Repeat(" ", indent))
-			_, _ = fmt.Fprintf(buf, "%s = ", name)
+			fmt.Fprintf(buf, "%s = ", name)
 
 			var val cty.Value
 			if !stateVal.IsNull() && stateVal.Type().HasAttribute(name) {
@@ -163,7 +163,7 @@ func writeConfigAttributesFromExisting(addr addrs.AbsResourceInstance, buf *stri
 				val = attrS.EmptyValue()
 			}
 			if attrS.Sensitive || val.HasMark(marks.Sensitive) {
-				_, _ = fmt.Fprintf(buf, "null # sensitive%s", writeOnlyComment(attrS, false))
+				fmt.Fprintf(buf, "null # sensitive%s", writeOnlyComment(attrS, false))
 			} else {
 				if val.Type() == cty.String {
 					unmarked, valMarks := val.Unmark()
@@ -191,7 +191,7 @@ func writeConfigAttributesFromExisting(addr addrs.AbsResourceInstance, buf *stri
 					})
 					continue
 				}
-				_, _ = fmt.Fprintf(buf, "%s", writeOnlyComment(attrS, true))
+				fmt.Fprintf(buf, "%s", writeOnlyComment(attrS, true))
 			}
 
 			buf.WriteString("\n")
@@ -228,7 +228,7 @@ func writeConfigNestedBlock(addr addrs.AbsResourceInstance, buf *strings.Builder
 	switch schema.Nesting {
 	case configschema.NestingSingle, configschema.NestingGroup:
 		buf.WriteString(strings.Repeat(" ", indent))
-		_, _ = fmt.Fprintf(buf, "%s {", name)
+		fmt.Fprintf(buf, "%s {", name)
 		writeBlockTypeConstraint(buf, schema)
 		diags = diags.Append(writeConfigAttributes(addr, buf, schema.Attributes, indent+2))
 		diags = diags.Append(writeConfigBlocks(addr, buf, schema.BlockTypes, indent+2))
@@ -236,7 +236,7 @@ func writeConfigNestedBlock(addr addrs.AbsResourceInstance, buf *strings.Builder
 		return diags
 	case configschema.NestingList, configschema.NestingSet:
 		buf.WriteString(strings.Repeat(" ", indent))
-		_, _ = fmt.Fprintf(buf, "%s {", name)
+		fmt.Fprintf(buf, "%s {", name)
 		writeBlockTypeConstraint(buf, schema)
 		diags = diags.Append(writeConfigAttributes(addr, buf, schema.Attributes, indent+2))
 		diags = diags.Append(writeConfigBlocks(addr, buf, schema.BlockTypes, indent+2))
@@ -245,7 +245,7 @@ func writeConfigNestedBlock(addr addrs.AbsResourceInstance, buf *strings.Builder
 	case configschema.NestingMap:
 		buf.WriteString(strings.Repeat(" ", indent))
 		// we use an arbitrary placeholder key (block label) "key"
-		_, _ = fmt.Fprintf(buf, "%s \"key\" {", name)
+		fmt.Fprintf(buf, "%s \"key\" {", name)
 		writeBlockTypeConstraint(buf, schema)
 		diags = diags.Append(writeConfigAttributes(addr, buf, schema.Attributes, indent+2))
 		diags = diags.Append(writeConfigBlocks(addr, buf, schema.BlockTypes, indent+2))
@@ -262,7 +262,7 @@ func writeConfigNestedTypeAttribute(addr addrs.AbsResourceInstance, buf *strings
 	var diags tfdiags.Diagnostics
 
 	buf.WriteString(strings.Repeat(" ", indent))
-	_, _ = fmt.Fprintf(buf, "%s = ", name)
+	fmt.Fprintf(buf, "%s = ", name)
 
 	switch schema.NestedType.Nesting {
 	case configschema.NestingSingle:
@@ -333,7 +333,7 @@ func writeConfigNestedTypeAttributeFromExisting(addr addrs.AbsResourceInstance,
 	case configschema.NestingSingle:
 		if schema.Sensitive || stateVal.HasMark(marks.Sensitive) {
 			buf.WriteString(strings.Repeat(" ", indent))
-			_, _ = fmt.Fprintf(buf, "%s = {} # sensitive%s\n", name, writeOnlyComment(schema, false))
+			fmt.Fprintf(buf, "%s = {} # sensitive%s\n", name, writeOnlyComment(schema, false))
 			return diags
 		}
 
@@ -349,12 +349,12 @@ func writeConfigNestedTypeAttributeFromExisting(addr addrs.AbsResourceInstance,
 			// There is a difference between a null object, and an object with
 			// no attributes.
 			buf.WriteString(strings.Repeat(" ", indent))
-			_, _ = fmt.Fprintf(buf, "%s = null%s\n", name, writeOnlyComment(schema, true))
+			fmt.Fprintf(buf, "%s = null%s\n", name, writeOnlyComment(schema, true))
 			return diags
 		}
 
 		buf.WriteString(strings.Repeat(" ", indent))
-		_, _ = fmt.Fprintf(buf, "%s = {\n", name)
+		fmt.Fprintf(buf, "%s = {\n", name)
 		diags = diags.Append(writeConfigAttributesFromExisting(addr, buf, nestedVal, schema.NestedType.Attributes, indent+2))
 		buf.WriteString("}\n")
 		return diags
@@ -363,7 +363,7 @@ func writeConfigNestedTypeAttributeFromExisting(addr addrs.AbsResourceInstance,
 
 		if schema.Sensitive || stateVal.HasMark(marks.Sensitive) {
 			buf.WriteString(strings.Repeat(" ", indent))
-			_, _ = fmt.Fprintf(buf, "%s = [] # sensitive%s\n", name, writeOnlyComment(schema, false))
+			fmt.Fprintf(buf, "%s = [] # sensitive%s\n", name, writeOnlyComment(schema, false))
 			return diags
 		}
 
@@ -371,12 +371,12 @@ func writeConfigNestedTypeAttributeFromExisting(addr addrs.AbsResourceInstance,
 		if listVals == nil {
 			// There is a difference between an empty list and a null list
 			buf.WriteString(strings.Repeat(" ", indent))
-			_, _ = fmt.Fprintf(buf, "%s = null%s\n", name, writeOnlyComment(schema, true))
+			fmt.Fprintf(buf, "%s = null%s\n", name, writeOnlyComment(schema, true))
 			return diags
 		}
 
 		buf.WriteString(strings.Repeat(" ", indent))
-		_, _ = fmt.Fprintf(buf, "%s = [\n", name)
+		fmt.Fprintf(buf, "%s = [\n", name)
 		for i := range listVals {
 			buf.WriteString(strings.Repeat(" ", indent+2))
 			// The entire element is marked.
@@ -397,7 +397,7 @@ func writeConfigNestedTypeAttributeFromExisting(addr addrs.AbsResourceInstance,
 	case configschema.NestingMap:
 		if schema.Sensitive || stateVal.HasMark(marks.Sensitive) {
 			buf.WriteString(strings.Repeat(" ", indent))
-			_, _ = fmt.Fprintf(buf, "%s = {} # sensitive%s\n", name, writeOnlyComment(schema, false))
+			fmt.Fprintf(buf, "%s = {} # sensitive%s\n", name, writeOnlyComment(schema, false))
 			return diags
 		}
 
@@ -405,7 +405,7 @@ func writeConfigNestedTypeAttributeFromExisting(addr addrs.AbsResourceInstance,
 		if attr.IsNull() {
 			// There is a difference between an empty map and a null map.
 			buf.WriteString(strings.Repeat(" ", indent))
-			_, _ = fmt.Fprintf(buf, "%s = null%s\n", name, writeOnlyComment(schema, true))
+			fmt.Fprintf(buf, "%s = null%s\n", name, writeOnlyComment(schema, true))
 			return diags
 		}
 
@@ -418,10 +418,10 @@ func writeConfigNestedTypeAttributeFromExisting(addr addrs.AbsResourceInstance,
 		sort.Strings(keys)
 
 		buf.WriteString(strings.Repeat(" ", indent))
-		_, _ = fmt.Fprintf(buf, "%s = {\n", name)
+		fmt.Fprintf(buf, "%s = {\n", name)
 		for _, key := range keys {
 			buf.WriteString(strings.Repeat(" ", indent+2))
-			_, _ = fmt.Fprintf(buf, "%s = {", key)
+			fmt.Fprintf(buf, "%s = {", key)
 
 			// This entire value is marked
 			if vals[key].HasMark(marks.Sensitive) {
@@ -453,7 +453,7 @@ func writeConfigNestedBlockFromExisting(addr addrs.AbsResourceInstance, buf *str
 			return diags
 		}
 		buf.WriteString(strings.Repeat(" ", indent))
-		_, _ = fmt.Fprintf(buf, "%s {", name)
+		fmt.Fprintf(buf, "%s {", name)
 
 		// If the entire value is marked, don't print any nested attributes
 		if stateVal.HasMark(marks.Sensitive) {
@@ -468,13 +468,13 @@ func writeConfigNestedBlockFromExisting(addr addrs.AbsResourceInstance, buf *str
 	case configschema.NestingList, configschema.NestingSet:
 		if stateVal.HasMark(marks.Sensitive) {
 			buf.WriteString(strings.Repeat(" ", indent))
-			_, _ = fmt.Fprintf(buf, "%s {} # sensitive\n", name)
+			fmt.Fprintf(buf, "%s {} # sensitive\n", name)
 			return diags
 		}
 		listVals := ctyCollectionValues(stateVal)
 		for i := range listVals {
 			buf.WriteString(strings.Repeat(" ", indent))
-			_, _ = fmt.Fprintf(buf, "%s {\n", name)
+			fmt.Fprintf(buf, "%s {\n", name)
 			diags = diags.Append(writeConfigAttributesFromExisting(addr, buf, listVals[i], schema.Attributes, indent+2))
 			diags = diags.Append(writeConfigBlocksFromExisting(addr, buf, listVals[i], schema.BlockTypes, indent+2))
 			buf.WriteString("}\n")
@@ -483,7 +483,7 @@ func writeConfigNestedBlockFromExisting(addr addrs.AbsResourceInstance, buf *str
 	case configschema.NestingMap:
 		// If the entire value is marked, don't print any nested attributes
 		if stateVal.HasMark(marks.Sensitive) {
-			_, _ = fmt.Fprintf(buf, "%s {} # sensitive\n", name)
+			fmt.Fprintf(buf, "%s {} # sensitive\n", name)
 			return diags
 		}
 
@@ -496,7 +496,7 @@ func writeConfigNestedBlockFromExisting(addr addrs.AbsResourceInstance, buf *str
 		sort.Strings(keys)
 		for _, key := range keys {
 			buf.WriteString(strings.Repeat(" ", indent))
-			_, _ = fmt.Fprintf(buf, "%s %q {", name, key)
+			fmt.Fprintf(buf, "%s %q {", name, key)
 			// This entire map element is marked
 			if vals[key].HasMark(marks.Sensitive) {
 				buf.WriteString("} # sensitive\n")
@@ -523,9 +523,9 @@ func writeAttrTypeConstraint(buf *strings.Builder, schema *configschema.Attribut
 	}
 
 	if schema.NestedType != nil {
-		_, _ = fmt.Fprintf(buf, "%s\n", schema.NestedType.ImpliedType().FriendlyName())
+		fmt.Fprintf(buf, "%s\n", schema.NestedType.ImpliedType().FriendlyName())
 	} else {
-		_, _ = fmt.Fprintf(buf, "%s\n", schema.Type.FriendlyName())
+		fmt.Fprintf(buf, "%s\n", schema.Type.FriendlyName())
 	}
 }
 

internal/lang/funcs/ephemeral_test.go

@@ -124,5 +124,4 @@ func TestEphemeralAsNullFunc(t *testing.T) {
 			}
 		})
 	}
-
 }

internal/lang/funcs/redact.go

@@ -13,10 +13,15 @@ import (
 )
 
 func redactIfSensitiveOrEphemeral(value interface{}, valueMarks ...cty.ValueMarks) string {
-	if marks.Has(cty.DynamicVal.WithMarks(valueMarks...), marks.Ephemeral) {
+	isEphemeral := marks.Has(cty.DynamicVal.WithMarks(valueMarks...), marks.Ephemeral)
+	isSensitive := marks.Has(cty.DynamicVal.WithMarks(valueMarks...), marks.Sensitive)
+	if isEphemeral && isSensitive {
+		return "(ephemeral sensitive value)"
+	}
+	if isEphemeral {
 		return "(ephemeral value)"
 	}
-	if marks.Has(cty.DynamicVal.WithMarks(valueMarks...), marks.Sensitive) {
+	if isSensitive {
 		return "(sensitive value)"
 	}
 	switch v := value.(type) {

internal/lang/funcs/redact_test.go

@@ -51,7 +51,7 @@ func TestRedactIfSensitive(t *testing.T) {
 		"ephemeral and sensitive string": {
 			value: "foo",
 			marks: []cty.ValueMarks{cty.NewValueMarks(marks.Ephemeral, marks.Sensitive)},
-			want:  "(ephemeral value)",
+			want:  "(ephemeral sensitive value)",
 		},
 	}
 

internal/plugin/validation/write_only.go

@@ -22,6 +22,7 @@ import (
 // and there is no issue in the provider SDK when it comes to the write-only attributes.
 // Returning those with actual values can create unknown behavior leading to possible confidential
 // information exposure.
+// NOTE: Keep this in sync with the equivalent in internal/plugin6/validation/write_only.go
 func WriteOnlyAttributes(schema *configschema.Block, v cty.Value, resAddr string) (diags tfdiags.Diagnostics) {
 	if !schema.ContainsWriteOnly() {
 		return diags
@@ -32,7 +33,7 @@ func WriteOnlyAttributes(schema *configschema.Block, v cty.Value, resAddr string
 
 		pathVal, err := path.Apply(v)
 		if err != nil {
-			log.Printf("[WARN] Error when tried to get the path (%s) value from the given object: %s", pathAsString, err)
+			log.Printf("[WARN] Error when trying to get the path (%s) value from the given object: %s", pathAsString, err)
 			continue
 		}
 		if pathVal.IsNull() {

internal/plugin6/validation/write_only.go

@@ -22,6 +22,7 @@ import (
 // and there is no issue in the provider SDK when it comes to the write-only attributes.
 // Returning those with actual values can create unknown behavior leading to possible confidential
 // information exposure.
+// NOTE: Keep this in sync with the equivalent in internal/plugin/validation/write_only.go
 func WriteOnlyAttributes(schema *configschema.Block, v cty.Value, resAddr string) (diags tfdiags.Diagnostics) {
 	if !schema.ContainsWriteOnly() {
 		return diags

website/docs/cli/commands/apply.mdx

@@ -39,7 +39,7 @@ decisions.
 #### Ephemeral variables
 Since ephemeral variables can't be stored in a planfile, any ephemeral variables set during the generation of a planfile from `tofu plan` must also be set when running tofu apply.
 
-For example, if `-var` has been used to create the planfile (`tofu plan -var "token=$MY_TOKEN"`), the same argument _must_
+For example, if `-var` has been used to provide an ephemeral variable `token` to create the planfile (`tofu plan -var "token=$MY_TOKEN"`), the same argument _must_
 be set for the apply command (`tofu apply -var "token=$MY_TOKEN" planfile`).
 If required ephemeral variables are not provided to the apply command, OpenTofu will exit with a clear message of additional variables that must be specified.
 

website/docs/language/ephemerality/index.mdx

@@ -36,7 +36,7 @@ As part of this concept, the following constructs can be used:
 ## Compatibility
 :::warning
 This concept is inspired by and aims for compatibility with the equivalent "Ephemeral" concept in Terraform v1.12.
-If incompatibilities are discovered, we will consider accepting breaking changes in subsequent versions
+If incompatibilities are discovered, the OpenTofu team will consider accepting breaking changes in subsequent versions
 of OpenTofu to ensure compatibility.
 :::
 
@@ -59,7 +59,7 @@ terraform {
   required_providers {
     aws = {
       source  = "hashicorp/aws"
-      version = "6.0.0-beta1"
+      version = ">=6.0.0"
     }
   }
 }

website/docs/language/ephemerality/write-only-attributes.mdx

@@ -11,8 +11,7 @@ description: >-
 Write-only attributes can be used only with OpenTofu v1.11 onwards.
 :::
 
-This attribute is only found in [`managed resources`](../resources/index.mdx)
+This attribute is only found in [`managed resources`](../resources/index.mdx) that are designed to accept transient values that will never be stored in the state or plan.
-designed to accept transient values that will never be stored in the state or plan.
 For example, a secret can be read by using an ephemeral resource and then passed into the write-only
 attribute `password_wo` of a managed resource.