diff --git a/internal/command/jsonformat/structured/change.go b/internal/command/jsonformat/structured/change.go index dfe08fd313b2..7c97efdaf842 100644 --- a/internal/command/jsonformat/structured/change.go +++ b/internal/command/jsonformat/structured/change.go @@ -187,9 +187,11 @@ func FromJsonViewsOutput(output viewsjson.Output) Change { func FromJsonActionInvocation(actionInvocation jsonplan.ActionInvocation) Change { return Change{ - Before: unwrapAttributeValues(actionInvocation.ConfigValues), - After: unwrapAttributeValues(actionInvocation.ConfigValues), - Unknown: false, + Before: unwrapAttributeValues(actionInvocation.ConfigValues), + After: unwrapAttributeValues(actionInvocation.ConfigValues), + Unknown: false, + BeforeSensitive: unmarshalGeneric(actionInvocation.ConfigSensitive), + AfterSensitive: unmarshalGeneric(actionInvocation.ConfigSensitive), ReplacePaths: attribute_path.Empty(false), RelevantAttributes: attribute_path.AlwaysMatcher(), diff --git a/internal/command/jsonplan/action_invocations.go b/internal/command/jsonplan/action_invocations.go index cfd797594991..5c2fd1466b3e 100644 --- a/internal/command/jsonplan/action_invocations.go +++ b/internal/command/jsonplan/action_invocations.go @@ -8,6 +8,7 @@ import ( "fmt" "sort" + "github.com/hashicorp/terraform/internal/command/jsonstate" "github.com/hashicorp/terraform/internal/lang/marks" "github.com/hashicorp/terraform/internal/plans" "github.com/hashicorp/terraform/internal/providers" @@ -25,7 +26,8 @@ type ActionInvocation struct { Name string `json:"name,omitempty"` // ConfigValues is the JSON representation of the values in the config block of the action - ConfigValues map[string]json.RawMessage `json:"config_values,omitempty"` + ConfigValues map[string]json.RawMessage `json:"config_values,omitempty"` + ConfigSensitive json.RawMessage `json:"config_sensitive,omitempty"` // ProviderName allows the property "type" to be interpreted unambiguously // in the unusual situation where a provider offers a type whose @@ -85,7 +87,7 @@ func marshalConfigValues(value cty.Value) map[string]json.RawMessage { } ret := make(map[string]json.RawMessage) - it := value.ElementIterator() + it := v.ElementIterator() for it.Next() { k, v := it.Element() vJSON, _ := ctyjson.Marshal(v, v.Type()) @@ -99,7 +101,7 @@ func MarshalActionInvocations(actions []*plans.ActionInvocationInstanceSrc, sche for _, action := range actions { ai, err := MarshalActionInvocation(action, schemas) if err != nil { - return nil, err + return ret, fmt.Errorf("failed to decode action %s: %w", action.Addr, err) } ret = append(ret, ai) } @@ -113,7 +115,6 @@ func MarshalActionInvocation(action *plans.ActionInvocationInstanceSrc, schemas Name: action.Addr.Action.Action.Name, ProviderName: action.ProviderAddr.Provider.String(), } - schema := schemas.ActionTypeConfig( action.ProviderAddr.Provider, action.Addr.Action.Action.Type, @@ -140,12 +141,8 @@ func MarshalActionInvocation(action *plans.ActionInvocationInstanceSrc, schemas } if actionDec.ConfigValue != cty.NilVal { - // TODO: Support sensitive and ephemeral values in action invocations. _, pvms := actionDec.ConfigValue.UnmarkDeepWithPaths() sensitivePaths, otherMarks := marks.PathsWithMark(pvms, marks.Sensitive) - if len(sensitivePaths) > 0 { - return ai, fmt.Errorf("action %s has sensitive config values, which are not supported in action invocations", action.Addr) - } ephemeralPaths, otherMarks := marks.PathsWithMark(otherMarks, marks.Ephemeral) if len(ephemeralPaths) > 0 { return ai, fmt.Errorf("action %s has ephemeral config values, which are not supported in action invocations", action.Addr) @@ -154,12 +151,18 @@ func MarshalActionInvocation(action *plans.ActionInvocationInstanceSrc, schemas return ai, fmt.Errorf("action %s has config values with unsupported marks: %v", action.Addr, otherMarks) } - if actionDec.ConfigValue.IsWhollyKnown() { - ai.ConfigValues = marshalConfigValues(actionDec.ConfigValue) - } else { - knowns := omitUnknowns(actionDec.ConfigValue) - ai.ConfigValues = marshalConfigValues(knowns) + configValue := actionDec.ConfigValue + if !configValue.IsWhollyKnown() { + configValue = omitUnknowns(actionDec.ConfigValue) } + cs := jsonstate.SensitiveAsBool(marks.MarkPaths(configValue, marks.Sensitive, sensitivePaths)) + configSensitive, err := ctyjson.Marshal(cs, cs.Type()) + if err != nil { + return ai, err + } + + ai.ConfigValues = marshalConfigValues(configValue) + ai.ConfigSensitive = configSensitive } return ai, nil } diff --git a/internal/plans/action_invocation.go b/internal/plans/action_invocation.go index cacf6b68f7c7..a5e2d194f122 100644 --- a/internal/plans/action_invocation.go +++ b/internal/plans/action_invocation.go @@ -4,9 +4,13 @@ package plans import ( + "fmt" + "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/configs" + "github.com/hashicorp/terraform/internal/lang/marks" "github.com/hashicorp/terraform/internal/providers" + "github.com/hashicorp/terraform/internal/tfdiags" "github.com/zclconf/go-cty/cty" ) @@ -109,8 +113,15 @@ func (ai *ActionInvocationInstance) Encode(schema *providers.ActionSchema) (*Act ty = schema.ConfigSchema.ImpliedType() } + unmarkedConfigValue, pvms := ai.ConfigValue.UnmarkDeepWithPaths() + sensitivePaths, otherMarks := marks.PathsWithMark(pvms, marks.Sensitive) + if len(otherMarks) > 0 { + return nil, fmt.Errorf("%s: error serializing action invocation with unexpected marks on config value: %#v. This is a bug in Terraform.", tfdiags.FormatCtyPath(otherMarks[0].Path), otherMarks[0].Marks) + } + var err error - ret.ConfigValue, err = NewDynamicValue(ai.ConfigValue, ty) + ret.ConfigValue, err = NewDynamicValue(unmarkedConfigValue, ty) + ret.SensitiveConfigPaths = sensitivePaths if err != nil { return nil, err } diff --git a/internal/plans/changes_src.go b/internal/plans/changes_src.go index 2c7bd1f2b507..e50d037826c3 100644 --- a/internal/plans/changes_src.go +++ b/internal/plans/changes_src.go @@ -568,7 +568,8 @@ type ActionInvocationInstanceSrc struct { Addr addrs.AbsActionInstance ActionTrigger ActionTrigger - ConfigValue DynamicValue + ConfigValue DynamicValue + SensitiveConfigPaths []cty.Path ProviderAddr addrs.AbsProviderConfig } @@ -584,12 +585,13 @@ func (acs *ActionInvocationInstanceSrc) Decode(schema *providers.ActionSchema) ( if err != nil { return nil, fmt.Errorf("error decoding 'config' value: %s", err) } + markedConfigValue := marks.MarkPaths(config, marks.Sensitive, acs.SensitiveConfigPaths) ai := &ActionInvocationInstance{ Addr: acs.Addr, ActionTrigger: acs.ActionTrigger, ProviderAddr: acs.ProviderAddr, - ConfigValue: config, + ConfigValue: markedConfigValue, } return ai, nil } diff --git a/internal/plans/planfile/tfplan.go b/internal/plans/planfile/tfplan.go index e3a138ce2626..ac0a40420d84 100644 --- a/internal/plans/planfile/tfplan.go +++ b/internal/plans/planfile/tfplan.go @@ -1365,6 +1365,11 @@ func actionInvocationFromTfplan(rawAction *planproto.ActionInvocationInstance) ( return nil, fmt.Errorf("invalid config value: %s", err) } ret.ConfigValue = configVal + sensitiveConfigPaths, err := pathsFromTfplan(rawAction.SensitiveConfigPaths) + if err != nil { + return nil, err + } + ret.SensitiveConfigPaths = sensitiveConfigPaths } return ret, nil @@ -1412,6 +1417,11 @@ func actionInvocationToTfPlan(action *plans.ActionInvocationInstanceSrc) (*planp if action.ConfigValue != nil { ret.ConfigValue = valueToTfplan(action.ConfigValue) + sensitiveConfigPaths, err := pathsToTfplan(action.SensitiveConfigPaths) + if err != nil { + return nil, err + } + ret.SensitiveConfigPaths = sensitiveConfigPaths } return ret, nil diff --git a/internal/plans/planfile/tfplan_test.go b/internal/plans/planfile/tfplan_test.go index 4b17b33e433e..cd8a3dcfdc0d 100644 --- a/internal/plans/planfile/tfplan_test.go +++ b/internal/plans/planfile/tfplan_test.go @@ -335,6 +335,24 @@ func examplePlanForTest(t *testing.T) *plans.Plan { "id": cty.StringVal("testing"), }), objTy), }, + { + Addr: addrs.Action{Type: "example", Name: "baz"}.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), + ProviderAddr: provider, + ActionTrigger: plans.LifecycleActionTrigger{ + ActionTriggerEvent: configs.BeforeCreate, + ActionTriggerBlockIndex: 2, + ActionsListIndex: 1, + TriggeringResourceAddr: addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "test_thing", + Name: "woot", + }.Instance(addrs.IntKey(0)).Absolute(addrs.RootModuleInstance), + }, + ConfigValue: mustNewDynamicValue(cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("secret"), + }), objTy), + SensitiveConfigPaths: []cty.Path{cty.GetAttrPath("id")}, + }, }, }, DriftedResources: []*plans.ResourceInstanceChangeSrc{ diff --git a/internal/plans/planproto/planfile.pb.go b/internal/plans/planproto/planfile.pb.go index 2cdbc1f49cc0..cb0b3026d460 100644 --- a/internal/plans/planproto/planfile.pb.go +++ b/internal/plans/planproto/planfile.pb.go @@ -1725,6 +1725,10 @@ type ActionInvocationInstance struct { Provider string `protobuf:"bytes,2,opt,name=provider,proto3" json:"provider,omitempty"` LinkedResources []*ResourceInstanceActionChange `protobuf:"bytes,3,rep,name=linked_resources,json=linkedResources,proto3" json:"linked_resources,omitempty"` ConfigValue *DynamicValue `protobuf:"bytes,4,opt,name=config_value,json=configValue,proto3" json:"config_value,omitempty"` + // An unordered set of paths into config_value which are marked as + // sensitive. Values at these paths should be obscured in human-readable + // output. + SensitiveConfigPaths []*Path `protobuf:"bytes,5,rep,name=sensitive_config_paths,json=sensitiveConfigPaths,proto3" json:"sensitive_config_paths,omitempty"` // Types that are valid to be assigned to ActionTrigger: // // *ActionInvocationInstance_LifecycleActionTrigger @@ -1791,6 +1795,13 @@ func (x *ActionInvocationInstance) GetConfigValue() *DynamicValue { return nil } +func (x *ActionInvocationInstance) GetSensitiveConfigPaths() []*Path { + if x != nil { + return x.SensitiveConfigPaths + } + return nil +} + func (x *ActionInvocationInstance) GetActionTrigger() isActionInvocationInstance_ActionTrigger { if x != nil { return x.ActionTrigger @@ -1812,7 +1823,7 @@ type isActionInvocationInstance_ActionTrigger interface { } type ActionInvocationInstance_LifecycleActionTrigger struct { - LifecycleActionTrigger *LifecycleActionTrigger `protobuf:"bytes,5,opt,name=lifecycle_action_trigger,json=lifecycleActionTrigger,proto3,oneof"` + LifecycleActionTrigger *LifecycleActionTrigger `protobuf:"bytes,6,opt,name=lifecycle_action_trigger,json=lifecycleActionTrigger,proto3,oneof"` } func (*ActionInvocationInstance_LifecycleActionTrigger) isActionInvocationInstance_ActionTrigger() {} @@ -2272,13 +2283,14 @@ const file_planfile_proto_rawDesc = "" + "\aunknown\x18\x02 \x01(\bR\aunknown\x120\n" + "\bidentity\x18\x03 \x01(\v2\x14.tfplan.DynamicValueR\bidentity\":\n" + "\bDeferred\x12.\n" + - "\x06reason\x18\x01 \x01(\x0e2\x16.tfplan.DeferredReasonR\x06reason\"\xc2\x02\n" + + "\x06reason\x18\x01 \x01(\x0e2\x16.tfplan.DeferredReasonR\x06reason\"\x86\x03\n" + "\x18ActionInvocationInstance\x12\x12\n" + "\x04addr\x18\x01 \x01(\tR\x04addr\x12\x1a\n" + "\bprovider\x18\x02 \x01(\tR\bprovider\x12O\n" + "\x10linked_resources\x18\x03 \x03(\v2$.tfplan.ResourceInstanceActionChangeR\x0flinkedResources\x127\n" + - "\fconfig_value\x18\x04 \x01(\v2\x14.tfplan.DynamicValueR\vconfigValue\x12Z\n" + - "\x18lifecycle_action_trigger\x18\x05 \x01(\v2\x1e.tfplan.LifecycleActionTriggerH\x00R\x16lifecycleActionTriggerB\x10\n" + + "\fconfig_value\x18\x04 \x01(\v2\x14.tfplan.DynamicValueR\vconfigValue\x12B\n" + + "\x16sensitive_config_paths\x18\x05 \x03(\v2\f.tfplan.PathR\x14sensitiveConfigPaths\x12Z\n" + + "\x18lifecycle_action_trigger\x18\x06 \x01(\v2\x1e.tfplan.LifecycleActionTriggerH\x00R\x16lifecycleActionTriggerB\x10\n" + "\x0eaction_trigger\"\xfe\x01\n" + "\x16LifecycleActionTrigger\x128\n" + "\x18triggering_resource_addr\x18\x01 \x01(\tR\x16triggeringResourceAddr\x12?\n" + @@ -2427,18 +2439,19 @@ var file_planfile_proto_depIdxs = []int32{ 3, // 36: tfplan.Deferred.reason:type_name -> tfplan.DeferredReason 24, // 37: tfplan.ActionInvocationInstance.linked_resources:type_name -> tfplan.ResourceInstanceActionChange 18, // 38: tfplan.ActionInvocationInstance.config_value:type_name -> tfplan.DynamicValue - 23, // 39: tfplan.ActionInvocationInstance.lifecycle_action_trigger:type_name -> tfplan.LifecycleActionTrigger - 4, // 40: tfplan.LifecycleActionTrigger.trigger_event:type_name -> tfplan.ActionTriggerEvent - 11, // 41: tfplan.ResourceInstanceActionChange.change:type_name -> tfplan.Change - 18, // 42: tfplan.Plan.VariablesEntry.value:type_name -> tfplan.DynamicValue - 19, // 43: tfplan.Plan.resource_attr.attr:type_name -> tfplan.Path - 5, // 44: tfplan.CheckResults.ObjectResult.status:type_name -> tfplan.CheckResults.Status - 18, // 45: tfplan.Path.Step.element_key:type_name -> tfplan.DynamicValue - 46, // [46:46] is the sub-list for method output_type - 46, // [46:46] is the sub-list for method input_type - 46, // [46:46] is the sub-list for extension type_name - 46, // [46:46] is the sub-list for extension extendee - 0, // [0:46] is the sub-list for field type_name + 19, // 39: tfplan.ActionInvocationInstance.sensitive_config_paths:type_name -> tfplan.Path + 23, // 40: tfplan.ActionInvocationInstance.lifecycle_action_trigger:type_name -> tfplan.LifecycleActionTrigger + 4, // 41: tfplan.LifecycleActionTrigger.trigger_event:type_name -> tfplan.ActionTriggerEvent + 11, // 42: tfplan.ResourceInstanceActionChange.change:type_name -> tfplan.Change + 18, // 43: tfplan.Plan.VariablesEntry.value:type_name -> tfplan.DynamicValue + 19, // 44: tfplan.Plan.resource_attr.attr:type_name -> tfplan.Path + 5, // 45: tfplan.CheckResults.ObjectResult.status:type_name -> tfplan.CheckResults.Status + 18, // 46: tfplan.Path.Step.element_key:type_name -> tfplan.DynamicValue + 47, // [47:47] is the sub-list for method output_type + 47, // [47:47] is the sub-list for method input_type + 47, // [47:47] is the sub-list for extension type_name + 47, // [47:47] is the sub-list for extension extendee + 0, // [0:47] is the sub-list for field type_name } func init() { file_planfile_proto_init() } diff --git a/internal/plans/planproto/planfile.proto b/internal/plans/planproto/planfile.proto index 05db4d4bc8d6..6480c9fc25c2 100644 --- a/internal/plans/planproto/planfile.proto +++ b/internal/plans/planproto/planfile.proto @@ -468,9 +468,13 @@ message ActionInvocationInstance { repeated ResourceInstanceActionChange linked_resources = 3; DynamicValue config_value = 4; + // An unordered set of paths into config_value which are marked as + // sensitive. Values at these paths should be obscured in human-readable + // output. + repeated Path sensitive_config_paths = 5; oneof action_trigger { - LifecycleActionTrigger lifecycle_action_trigger = 5; + LifecycleActionTrigger lifecycle_action_trigger = 6; } } diff --git a/internal/terraform/context_apply_action_test.go b/internal/terraform/context_apply_action_test.go index 90f66e87c24c..73168ea615bf 100644 --- a/internal/terraform/context_apply_action_test.go +++ b/internal/terraform/context_apply_action_test.go @@ -556,7 +556,6 @@ resource "test_object" "b" { }, "action with secrets in configuration": { - toBeImplemented: true, // We currently don't suppport sensitive values in the plan module: map[string]string{ "main.tf": ` variable "secret_value" { diff --git a/internal/terraform/context_plan_actions_test.go b/internal/terraform/context_plan_actions_test.go index 1431cd546fc7..8dfd62e8e48b 100644 --- a/internal/terraform/context_plan_actions_test.go +++ b/internal/terraform/context_plan_actions_test.go @@ -14,6 +14,7 @@ import ( "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/configs/configschema" + "github.com/hashicorp/terraform/internal/lang/marks" "github.com/hashicorp/terraform/internal/plans" "github.com/hashicorp/terraform/internal/providers" testing_provider "github.com/hashicorp/terraform/internal/providers/testing" @@ -22,6 +23,19 @@ import ( ) func TestContextPlan_actions(t *testing.T) { + unlinkedActionSchema := providers.ActionSchema{ + ConfigSchema: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "attr": { + Type: cty.String, + Optional: true, + }, + }, + }, + + Unlinked: &providers.UnlinkedAction{}, + } + for name, tc := range map[string]struct { toBeImplemented bool module map[string]string @@ -1315,6 +1329,56 @@ resource "test_object" "a" { } }, }, + + "secret values": { + module: map[string]string{ + "main.tf": ` +variable "secret" { + type = string + sensitive = true +} +action "test_unlinked" "hello" { + config { + attr = var.secret + } +} +resource "test_object" "a" { + lifecycle { + action_trigger { + events = [before_create] + actions = [action.test_unlinked.hello] + } + } +} +`, + }, + planOpts: &PlanOpts{ + Mode: plans.NormalMode, + SetVariables: InputValues{ + "secret": &InputValue{ + Value: cty.StringVal("secret"), + SourceType: ValueFromCLIArg, + }}, + }, + expectPlanActionCalled: true, + + assertPlan: func(t *testing.T, p *plans.Plan) { + if len(p.Changes.ActionInvocations) != 1 { + t.Fatalf("expected 1 action in plan, got %d", len(p.Changes.ActionInvocations)) + } + + action := p.Changes.ActionInvocations[0] + ac, err := action.Decode(&unlinkedActionSchema) + if err != nil { + t.Fatalf("expected action to decode successfully, but got error: %v", err) + } + + if !marks.Has(ac.ConfigValue.GetAttr("attr"), marks.Sensitive) { + t.Fatalf("expected attribute 'attr' to be marked as sensitive") + } + }, + }, + "provider deferring action while not allowed": { module: map[string]string{ "main.tf": ` @@ -1656,18 +1720,7 @@ resource "test_object" "a" { p := &testing_provider.MockProvider{ GetProviderSchemaResponse: &providers.GetProviderSchemaResponse{ Actions: map[string]providers.ActionSchema{ - "test_unlinked": { - ConfigSchema: &configschema.Block{ - Attributes: map[string]*configschema.Attribute{ - "attr": { - Type: cty.String, - Optional: true, - }, - }, - }, - - Unlinked: &providers.UnlinkedAction{}, - }, + "test_unlinked": unlinkedActionSchema, "test_lifecycle": { ConfigSchema: &configschema.Block{ diff --git a/internal/terraform/node_action_trigger_instance_plan.go b/internal/terraform/node_action_trigger_instance_plan.go index 6b665204ad91..5ca4f468d3f7 100644 --- a/internal/terraform/node_action_trigger_instance_plan.go +++ b/internal/terraform/node_action_trigger_instance_plan.go @@ -9,6 +9,7 @@ import ( "github.com/hashicorp/hcl/v2" "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/configs" + "github.com/hashicorp/terraform/internal/lang/marks" "github.com/hashicorp/terraform/internal/plans" "github.com/hashicorp/terraform/internal/plans/deferring" "github.com/hashicorp/terraform/internal/providers" @@ -120,9 +121,23 @@ func (n *nodeActionTriggerPlanInstance) Execute(ctx EvalContext, operation walkO return diags } + // We remove the marks for planning, we will record the sensitive values in the plans.ActionInvocationInstance + unmarkedConfig, pvms := actionInstance.ConfigValue.UnmarkDeepWithPaths() + // We only support sensitive marks, all other marks cause an error + _, otherMarks := marks.PathsWithMark(pvms, marks.Sensitive) + if len(otherMarks) > 0 { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Unsupported marks", + Detail: "Only sensitive marks are supported in action configuration", + Subject: &n.actionConfig.DeclRange, + }) + return diags + } + resp := provider.PlanAction(providers.PlanActionRequest{ ActionType: n.actionAddress.Action.Action.Type, - ProposedActionData: actionInstance.ConfigValue, + ProposedActionData: unmarkedConfig, ClientCapabilities: ctx.ClientCapabilities(), })