internal/command: Ensure ephemeral variables are validated accordingly

internal/command/apply_test.go

@@ -292,6 +292,7 @@ func TestApply_parallelism(t *testing.T) {
 	// called once we reach the desired concurrency, allowing all apply calls
 	// to proceed in unison.
 	beginCtx, begin := context.WithCancel(context.Background())
+	t.Cleanup(begin)
 
 	// Since our mock provider has its own mutex preventing concurrent calls
 	// to ApplyResourceChange, we need to use a number of separate providers

internal/command/plan_test.go

@@ -22,11 +22,13 @@ import (
 	"github.com/hashicorp/terraform/internal/addrs"
 	backendinit "github.com/hashicorp/terraform/internal/backend/init"
 	"github.com/hashicorp/terraform/internal/checks"
+	"github.com/hashicorp/terraform/internal/command/views"
 	"github.com/hashicorp/terraform/internal/configs/configschema"
 	"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/hashicorp/terraform/internal/terminal"
 	"github.com/hashicorp/terraform/internal/tfdiags"
 )
 
@@ -1428,7 +1430,7 @@ func TestPlan_parallelism(t *testing.T) {
 	// called once we reach the desired concurrency, allowing all apply calls
 	// to proceed in unison.
 	beginCtx, begin := context.WithCancel(context.Background())
-
+	t.Cleanup(begin)
 	// Since our mock provider has its own mutex preventing concurrent calls
 	// to ApplyResourceChange, we need to use a number of separate providers
 	// here. They will all have the same mock implementation function assigned
@@ -1583,6 +1585,95 @@ func TestPlan_jsonGoldenReference(t *testing.T) {
 	checkGoldenReference(t, output, "plan")
 }
 
+func TestPlan_ephemeralValues(t *testing.T) {
+	testCases := []struct {
+		fixturePath    string
+		expectedCode   int
+		expectedOutput string
+	}{
+		{
+			"plan-ephemeral-values-invalid-eph-var-in-non-eph-output",
+			1,
+			`Error: Ephemeral value not allowed`,
+		},
+		{
+			"plan-ephemeral-values-invalid-module-eph-output",
+			1,
+			`Error: Value not allowed in ephemeral output`,
+		},
+		{
+			"plan-ephemeral-values-invalid-module-input",
+			1,
+			"Error: Ephemeral value not allowed",
+		},
+		{
+			"plan-ephemeral-values-invalid-module-non-eph-output",
+			1,
+			`Error: Ephemeral value not allowed`,
+		},
+		{
+			"plan-ephemeral-values-invalid-noneph-var-in-eph-output",
+			1,
+			`Error: Value not allowed in ephemeral output`,
+		},
+		{
+			"plan-ephemeral-values-valid",
+			0,
+			"",
+		},
+		{
+			"plan-ephemeral-values-valid-module-input",
+			0,
+			"",
+		},
+		{
+			"plan-ephemeral-values-variable-validation",
+			1,
+			"Error: Invalid value for variable",
+		},
+	}
+
+	for i, tc := range testCases {
+		t.Run(fmt.Sprintf("%2d-%s", i, tc.fixturePath), func(t *testing.T) {
+			td := t.TempDir()
+			testCopyDir(t, testFixturePath(tc.fixturePath), td)
+			defer testChdir(t, td)()
+
+			p := planFixtureProvider()
+			p.GetProviderSchemaResponse.Provider = providers.Schema{
+				Version: 0,
+				Block: &configschema.Block{
+					Attributes: map[string]*configschema.Attribute{
+						"token": {
+							Type:      cty.String,
+							Optional:  true,
+							Sensitive: true,
+						},
+					},
+				},
+			}
+
+			streams, done := terminal.StreamsForTesting(t)
+			c := &PlanCommand{
+				Meta: Meta{
+					testingOverrides: metaOverridesForProvider(p),
+					View:             views.NewView(streams),
+				},
+			}
+			code := c.Run([]string{"-json"})
+			out := done(t)
+			if code != tc.expectedCode {
+				t.Fatalf("expected exit code %d, given %d\noutput: %q", tc.expectedCode, code, out.All())
+			}
+
+			stdout := out.Stdout()
+			if tc.expectedOutput != "" && !strings.Contains(stdout, tc.expectedOutput) {
+				t.Fatalf("unexpected output: %s", stdout)
+			}
+		})
+	}
+}
+
 // planFixtureSchema returns a schema suitable for processing the
 // configuration in testdata/plan . This schema should be
 // assigned to a mock provider named "test".

internal/command/testdata/plan-ephemeral-values-invalid-eph-var-in-non-eph-output/invalid-variable-in-output.tf

@@ -0,0 +1,8 @@
+variable "eph" {
+  ephemeral = true
+  default   = "foo"
+}
+
+output "not-eph" {
+  value = var.eph
+}

internal/command/testdata/plan-ephemeral-values-invalid-module-eph-output/.terraform/modules/modules.json

@@ -0,0 +1 @@
+{"Modules":[{"Key":"","Source":"","Dir":"."},{"Key":"test","Source":"./eph-module","Dir":"eph-module"}]}

internal/command/testdata/plan-ephemeral-values-invalid-module-eph-output/eph-module/outputs.tf

@@ -0,0 +1,8 @@
+output "eph" {
+  ephemeral = true
+  value     = var.eph
+}
+
+output "not-eph" {
+  value = "foo"
+}

internal/command/testdata/plan-ephemeral-values-invalid-module-eph-output/eph-module/variables.tf

@@ -0,0 +1,4 @@
+variable "eph" {
+  type      = string
+  ephemeral = true
+}

internal/command/testdata/plan-ephemeral-values-invalid-module-eph-output/invalid-module-output.tf

@@ -0,0 +1,9 @@
+module "test" {
+  source = "./eph-module"
+  eph    = "foo"
+}
+
+output "eph" {
+  ephemeral = true
+  value     = module.test.not-eph
+}

internal/command/testdata/plan-ephemeral-values-invalid-module-input/.terraform/modules/modules.json

@@ -0,0 +1 @@
+{"Modules":[{"Key":"","Source":"","Dir":"."},{"Key":"test","Source":"./eph-module","Dir":"eph-module"}]}

internal/command/testdata/plan-ephemeral-values-invalid-module-input/eph-module/variables.tf

@@ -0,0 +1,3 @@
+variable "eph" {
+  type = string
+}

internal/command/testdata/plan-ephemeral-values-invalid-module-input/variable-in-module-input.tf

@@ -0,0 +1,10 @@
+variable "test-eph" {
+  type      = string
+  default   = "foo"
+  ephemeral = true
+}
+
+module "test" {
+  source = "./eph-module"
+  eph    = var.test-eph
+}

internal/command/testdata/plan-ephemeral-values-invalid-module-non-eph-output/.terraform/modules/modules.json

@@ -0,0 +1 @@
+{"Modules":[{"Key":"","Source":"","Dir":"."},{"Key":"test","Source":"./eph-module","Dir":"eph-module"}]}

internal/command/testdata/plan-ephemeral-values-invalid-module-non-eph-output/eph-module/outputs.tf

@@ -0,0 +1,8 @@
+output "eph" {
+  ephemeral = true
+  value     = var.eph
+}
+
+output "not-eph" {
+  value = "foo"
+}

internal/command/testdata/plan-ephemeral-values-invalid-module-non-eph-output/eph-module/variables.tf

@@ -0,0 +1,4 @@
+variable "eph" {
+  type      = string
+  ephemeral = true
+}

internal/command/testdata/plan-ephemeral-values-invalid-module-non-eph-output/invalid-module-output.tf

@@ -0,0 +1,8 @@
+module "test" {
+  source = "./eph-module"
+  eph    = "foo"
+}
+
+output "eph" {
+  value = module.test.eph
+}

internal/command/testdata/plan-ephemeral-values-invalid-noneph-var-in-eph-output/invalid-variable-in-output.tf

@@ -0,0 +1,8 @@
+variable "not-eph" {
+  default = "foo"
+}
+
+output "eph" {
+  ephemeral = true
+  value     = var.not-eph
+}

internal/command/testdata/plan-ephemeral-values-valid-module-input/.terraform/modules/modules.json

@@ -0,0 +1 @@
+{"Modules":[{"Key":"","Source":"","Dir":"."},{"Key":"test","Source":"./eph-module","Dir":"eph-module"}]}

internal/command/testdata/plan-ephemeral-values-valid-module-input/eph-module/variables.tf

@@ -0,0 +1,4 @@
+variable "eph" {
+  ephemeral = true
+  type      = string
+}

internal/command/testdata/plan-ephemeral-values-valid-module-input/variable-in-module-input.tf

@@ -0,0 +1,10 @@
+variable "test-eph" {
+  type      = string
+  default   = "foo"
+  ephemeral = true
+}
+
+module "test" {
+  source = "./eph-module"
+  eph    = var.test-eph
+}

internal/command/testdata/plan-ephemeral-values-valid/variable-in-output.tf

@@ -0,0 +1,9 @@
+variable "valid-eph" {
+  ephemeral = true
+  default   = "foo"
+}
+
+output "valid-eph" {
+  ephemeral = true
+  value     = var.valid-eph
+}

internal/command/testdata/plan-ephemeral-values-valid/variable-in-provider.tf

@@ -0,0 +1,8 @@
+variable "token" {
+  ephemeral = true
+  default   = "insecure"
+}
+
+provider "test" {
+  token = var.token
+}

internal/command/testdata/plan-ephemeral-values-variable-validation/variable-validation.tf

@@ -0,0 +1,9 @@
+variable "test" {
+  ephemeral = true
+  default   = "foo"
+
+  validation {
+    condition     = var.test != "foo"
+    error_message = "value"
+  }
+}

internal/terraform/node_output.go

@@ -487,25 +487,25 @@ If you do intend to export this data, annotate the output value as sensitive by
 	// more recently than the historical change to treat invalid output values
 	// as errors rather than warnings.
 
-	if n.Config.Ephemeral {
-		// An ephemeral output value always produces an ephemeral result,
-		// even if the value assigned to it internally is not. This is
-		// a useful simplification so that module authors can be
-		// explicit about what guarantees they are intending to make
-		// (regardless of current implementation details). Marking an
-		// output value as ephemeral when it wasn't before is always a
-		// breaking change to a module's API.
-		val = val.Mark(marks.Ephemeral)
-	} else {
-		if marks.Contains(val, marks.Ephemeral) {
-			diags = diags.Append(&hcl.Diagnostic{
-				Severity: hcl.DiagError,
-				Summary:  "Ephemeral value not allowed",
-				Detail:   "This output value is not declared as returning an ephemeral value, so it cannot be set to a result derived from an ephemeral value.",
-				Subject:  n.Config.Expr.Range().Ptr(),
-			})
-			return diags
-		}
+	if n.Config.Ephemeral && !marks.Has(val, marks.Ephemeral) {
+		// An ephemeral output value must always be ephemeral
+		// This is to prevent accidental persistence upstream
+		// from here.
+		diags = diags.Append(&hcl.Diagnostic{
+			Severity: hcl.DiagError,
+			Summary:  "Value not allowed in ephemeral output",
+			Detail:   "This output value is declared as returning an ephemeral value, so it can only be set to an ephemeral value.",
+			Subject:  n.Config.Expr.Range().Ptr(),
+		})
+		return diags
+	} else if !n.Config.Ephemeral && marks.Contains(val, marks.Ephemeral) {
+		diags = diags.Append(&hcl.Diagnostic{
+			Severity: hcl.DiagError,
+			Summary:  "Ephemeral value not allowed",
+			Detail:   "This output value is not declared as returning an ephemeral value, so it cannot be set to a result derived from an ephemeral value.",
+			Subject:  n.Config.Expr.Range().Ptr(),
+		})
+		return diags
 	}
 
 	n.setValue(ctx.NamedValues(), state, changes, ctx.Deferrals(), val)