From 425c6bead2bf6c9c12fd828264523b8de3c07d4b Mon Sep 17 00:00:00 2001 From: James Bardin Date: Mon, 20 Mar 2023 13:27:53 -0400 Subject: [PATCH 1/3] store non-root sensitive outputs in state Module outputs are evaluated from state, so in order to have detailed information about sensitivity from non-root module outputs, we need to store the value along with all sensitive marks. This aligns with the usage of state being the in-memory store for other temporary values like locals and variables. --- internal/terraform/node_output.go | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/internal/terraform/node_output.go b/internal/terraform/node_output.go index af60c95691..82fe534135 100644 --- a/internal/terraform/node_output.go +++ b/internal/terraform/node_output.go @@ -605,18 +605,13 @@ func (n *NodeApplyableOutput) setValue(state *states.SyncState, changes *plans.C // out here and then we'll save the real unknown value in the planned // changeset, if we have one on this graph walk. log.Printf("[TRACE] setValue: Saving value for %s in state", n.Addr) - sensitive := n.Config.Sensitive - unmarkedVal, valueMarks := val.UnmarkDeep() - // If the evaluated value contains sensitive marks, the output has no - // choice but to declare itself as "sensitive". - for mark := range valueMarks { - if mark == marks.Sensitive { - sensitive = true - break - } + // non-root outputs need to keep sensitive marks for evaluation, but are + // not serialized. + if n.Addr.Module.IsRoot() { + val, _ = val.UnmarkDeep() + val = cty.UnknownAsNull(val) } - stateVal := cty.UnknownAsNull(unmarkedVal) - state.SetOutputValue(n.Addr, stateVal, sensitive) + state.SetOutputValue(n.Addr, val, n.Config.Sensitive) } From d33e62751411863c6b85c6fa5eb99fa101bbc2c5 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Mon, 20 Mar 2023 14:07:39 -0400 Subject: [PATCH 2/3] remove old comments --- internal/terraform/node_output.go | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/internal/terraform/node_output.go b/internal/terraform/node_output.go index 82fe534135..5b3a707d24 100644 --- a/internal/terraform/node_output.go +++ b/internal/terraform/node_output.go @@ -513,10 +513,6 @@ func (n *NodeDestroyableOutput) DotNode(name string, opts *dag.DotOpts) *dag.Dot } func (n *NodeApplyableOutput) setValue(state *states.SyncState, changes *plans.ChangesSync, val cty.Value) { - // If we have an active changeset then we'll first replicate the value in - // there and lookup the prior value in the state. This is used in - // preference to the state where present, since it *is* able to represent - // unknowns, while the state cannot. if changes != nil && n.Planning { // if this is a root module, try to get a before value from the state for // the diff @@ -538,8 +534,8 @@ func (n *NodeApplyableOutput) setValue(state *states.SyncState, changes *plans.C } } - // We will not show the value is either the before or after are marked - // as sensitivity. We can show the value again once sensitivity is + // We will not show the value if either the before or after are marked + // as sensitive. We can show the value again once sensitivity is // removed from both the config and the state. sensitiveChange := sensitiveBefore || n.Config.Sensitive @@ -601,9 +597,6 @@ func (n *NodeApplyableOutput) setValue(state *states.SyncState, changes *plans.C return } - // The state itself doesn't represent unknown values, so we null them - // out here and then we'll save the real unknown value in the planned - // changeset, if we have one on this graph walk. log.Printf("[TRACE] setValue: Saving value for %s in state", n.Addr) // non-root outputs need to keep sensitive marks for evaluation, but are From defd7f0cde8965a75e42c9e4eb7b94d78331819b Mon Sep 17 00:00:00 2001 From: James Bardin Date: Mon, 20 Mar 2023 13:46:15 -0400 Subject: [PATCH 3/3] test that module outputs maintain sensitive marks --- internal/terraform/context_apply2_test.go | 67 +++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/internal/terraform/context_apply2_test.go b/internal/terraform/context_apply2_test.go index 4ce61ff2a5..97769176dd 100644 --- a/internal/terraform/context_apply2_test.go +++ b/internal/terraform/context_apply2_test.go @@ -1887,3 +1887,70 @@ output "null_module_test" { _, diags = ctx.Apply(plan, m) assertNoErrors(t, diags) } + +func TestContext2Apply_moduleOutputWithSensitiveAttrs(t *testing.T) { + // Ensure that nested sensitive marks are stored when accessing non-root + // module outputs, and that they do not cause the entire output value to + // become sensitive. + m := testModuleInline(t, map[string]string{ + "main.tf": ` +module "mod" { + source = "./mod" +} + +resource "test_resource" "b" { + // if the module output were wholly sensitive it would not be valid to use in + // for_each + for_each = module.mod.resources + value = each.value.output +} + +output "root_output" { + // The root output cannot contain any sensitive marks at all. + // Applying nonsensitive would fail here if the nested sensitive mark were + // not maintained through the output. + value = [ for k, v in module.mod.resources : nonsensitive(v.output) ] +} +`, + "./mod/main.tf": ` +resource "test_resource" "a" { + for_each = {"key": "value"} + value = each.key +} + +output "resources" { + value = test_resource.a +} +`, + }) + + p := testProvider("test") + p.GetProviderSchemaResponse = getProviderSchemaResponseFromProviderSchema(&ProviderSchema{ + ResourceTypes: map[string]*configschema.Block{ + "test_resource": { + Attributes: map[string]*configschema.Attribute{ + "value": { + Type: cty.String, + Required: true, + }, + "output": { + Type: cty.String, + Sensitive: true, + Computed: true, + }, + }, + }, + }, + }) + ctx := testContext2(t, &ContextOpts{ + Providers: map[addrs.Provider]providers.Factory{ + addrs.NewDefaultProvider("test"): testProviderFuncFixed(p), + }, + }) + plan, diags := ctx.Plan(m, states.NewState(), &PlanOpts{ + Mode: plans.NormalMode, + }) + assertNoErrors(t, diags) + _, diags = ctx.Apply(plan, m) + assertNoErrors(t, diags) +}