From 667004e603878a9eef096ec8db2b37db6310873f Mon Sep 17 00:00:00 2001 From: Daniel Schmidt Date: Thu, 26 Sep 2024 17:28:44 +0200 Subject: [PATCH] ephemeral: implement deferred actions --- internal/plans/deferring/deferred.go | 28 ++++++------------- .../terraform/context_apply_deferred_test.go | 25 ++++++++--------- internal/terraform/evaluate.go | 17 +++++++++-- internal/terraform/node_resource_ephemeral.go | 25 ++++++++--------- .../terraform/node_resource_partial_plan.go | 9 ++++++ internal/terraform/node_resource_plan.go | 6 ---- .../terraform/node_resource_plan_instance.go | 16 ++++++++++- .../node_resource_plan_partialexp.go | 2 ++ 8 files changed, 72 insertions(+), 56 deletions(-) diff --git a/internal/plans/deferring/deferred.go b/internal/plans/deferring/deferred.go index db13df6674..d6f9e82eb2 100644 --- a/internal/plans/deferring/deferred.go +++ b/internal/plans/deferring/deferred.go @@ -10,6 +10,7 @@ import ( "github.com/zclconf/go-cty/cty" "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/lang/marks" "github.com/hashicorp/terraform/internal/plans" "github.com/hashicorp/terraform/internal/providers" "github.com/hashicorp/terraform/internal/tfdiags" @@ -162,11 +163,6 @@ func (d *Deferred) GetDeferredChanges() []*plans.DeferredResourceInstanceChange changes = append(changes, changeElem.Value) } } - for _, configMapElem := range d.ephemeralResourceInstancesDeferred.Elems { - for _, changeElem := range configMapElem.Value.Elems { - changes = append(changes, changeElem.Value) - } - } for _, configMapElem := range d.partialExpandedResourcesDeferred.Elems { for _, changeElem := range configMapElem.Value.Elems { changes = append(changes, changeElem.Value) @@ -177,11 +173,6 @@ func (d *Deferred) GetDeferredChanges() []*plans.DeferredResourceInstanceChange changes = append(changes, changeElem.Value) } } - for _, configMapElem := range d.partialExpandedEphemeralResourceDeferred.Elems { - for _, changeElem := range configMapElem.Value.Elems { - changes = append(changes, changeElem.Value) - } - } return changes } @@ -294,6 +285,11 @@ func (d *Deferred) GetDeferredResourceInstances(addr addrs.AbsResource) map[addr instanceAddr := elem.Key change := elem.Value + if addr.Resource.Mode == addrs.EphemeralResourceMode { + // Deferred ephemeral resources always have an unknown value. + result[instanceAddr.Resource.Key] = cty.UnknownVal(cty.DynamicPseudoType).Mark(marks.Ephemeral) + continue + } // instances contains all the resources identified by the config address // regardless of the instances of the module they might be in. We need // to filter out the instances that are not part of the module we are @@ -506,15 +502,7 @@ func (d *Deferred) ReportDataSourceExpansionDeferred(addr addrs.PartialExpandedR }) } -func (d *Deferred) ReportEphemeralResourceExpansionDeferred(addr addrs.PartialExpandedResource, change *plans.ResourceInstanceChange) { - if change == nil { - // This indicates a bug in Terraform, we shouldn't ever be setting a - // null change. Note, if we don't make this check here, then we'll - // just crash later anyway. This way the stack trace points to the - // source of the problem. - panic("change must not be nil") - } - +func (d *Deferred) ReportEphemeralResourceExpansionDeferred(addr addrs.PartialExpandedResource) { d.mu.Lock() defer d.mu.Unlock() @@ -537,7 +525,7 @@ func (d *Deferred) ReportEphemeralResourceExpansionDeferred(addr addrs.PartialEx } configMap.Put(addr, &plans.DeferredResourceInstanceChange{ DeferredReason: providers.DeferredReasonInstanceCountUnknown, - Change: change, + Change: nil, // since we don't serialize this we can get away with no change, we store the addr, that should be enough }) } diff --git a/internal/terraform/context_apply_deferred_test.go b/internal/terraform/context_apply_deferred_test.go index 746446db19..c8f7901c17 100644 --- a/internal/terraform/context_apply_deferred_test.go +++ b/internal/terraform/context_apply_deferred_test.go @@ -3308,11 +3308,11 @@ ephemeral "test" "data" { }, stages: []deferredActionsTestStage{ { - complete: false, - wantActions: map[string]plans.Action{}, - wantPlanned: map[string]cty.Value{}, + complete: false, + wantActions: map[string]plans.Action{}, + wantPlanned: map[string]cty.Value{}, wantDeferred: map[string]ExpectedDeferred{ - "ephemeral.test.data": {Reason: providers.DeferredReasonProviderConfigUnknown, Action: plans.Read}, + // We don't record the ephemeral deferrals }, }, }, @@ -3332,12 +3332,11 @@ ephemeral "test" "dep" { }, stages: []deferredActionsTestStage{ { - complete: false, - wantActions: map[string]plans.Action{}, - wantPlanned: map[string]cty.Value{}, + complete: false, + wantActions: map[string]plans.Action{}, + wantPlanned: map[string]cty.Value{}, wantDeferred: map[string]ExpectedDeferred{ - "ephemeral.test.data": {Reason: providers.DeferredReasonProviderConfigUnknown, Action: plans.Read}, - "ephemeral.test.dep": {Reason: providers.DeferredReasonResourceConfigUnknown, Action: plans.Read}, + // We don't record the ephemeral deferrals }, }, }, @@ -3363,11 +3362,11 @@ ephemeral "test" "data" { inputs: map[string]cty.Value{ "each": cty.DynamicVal, }, - complete: false, - wantActions: map[string]plans.Action{}, - wantPlanned: map[string]cty.Value{}, + complete: false, + wantActions: map[string]plans.Action{}, + wantPlanned: map[string]cty.Value{}, wantDeferred: map[string]ExpectedDeferred{ - "ephemeral.test.*.data": {Reason: providers.DeferredReasonInstanceCountUnknown, Action: plans.Read}, + // We don't record the ephemeral deferrals }, }, }, diff --git a/internal/terraform/evaluate.go b/internal/terraform/evaluate.go index 82779e7c20..ad90b40489 100644 --- a/internal/terraform/evaluate.go +++ b/internal/terraform/evaluate.go @@ -541,9 +541,13 @@ func (d *evaluationStateData) GetResource(addr addrs.Resource, rng tfdiags.Sourc // (We can't get in here for a single-instance resource because in that // case we would know that there's only one key and it's addrs.NoKey, // so we'll fall through to the other logic below.) - // - // FIXME: this could catch ephemeral values too - return cty.DynamicVal, diags + unknownVal := cty.DynamicVal + + // If an ephemeral resource is deferred we need to mark the returned unknown value as ephemeral + if addr.Mode == addrs.EphemeralResourceMode { + unknownVal = unknownVal.Mark(marks.Ephemeral) + } + return unknownVal, diags } } @@ -818,6 +822,13 @@ func (d *evaluationStateData) getEphemeralResource(addr addrs.Resource, rng tfdi ephems := d.Evaluator.EphemeralResources getInstValue := func(addr addrs.AbsResourceInstance) (cty.Value, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics + + // If we have a deferred instance with this key we don't need to check if it is live or not, + // it has not been created so we can just return the deferred value. + if v, ok := instances[addr.Resource.Key]; ok { + return v, diags + } + val, isLive := ephems.InstanceValue(addr) if !isLive { // If the instance is no longer "live" by the time we're accessing diff --git a/internal/terraform/node_resource_ephemeral.go b/internal/terraform/node_resource_ephemeral.go index 6ce81acce5..011717eef4 100644 --- a/internal/terraform/node_resource_ephemeral.go +++ b/internal/terraform/node_resource_ephemeral.go @@ -28,14 +28,14 @@ type ephemeralResourceInput struct { // ephemeralResourceOpen implements the "open" step of the ephemeral resource // instance lifecycle, which behaves the same way in both the plan and apply // walks. -func ephemeralResourceOpen(ctx EvalContext, inp ephemeralResourceInput) tfdiags.Diagnostics { +func ephemeralResourceOpen(ctx EvalContext, inp ephemeralResourceInput) (*providers.Deferred, tfdiags.Diagnostics) { log.Printf("[TRACE] ephemeralResourceOpen: opening %s", inp.addr) var diags tfdiags.Diagnostics provider, providerSchema, err := getProvider(ctx, inp.providerConfig) if err != nil { diags = diags.Append(err) - return diags + return nil, diags } config := inp.config @@ -47,7 +47,7 @@ func ephemeralResourceOpen(ctx EvalContext, inp ephemeralResourceInput) tfdiags. inp.providerConfig, inp.addr.ContainingResource().Resource.Type, ), ) - return diags + return nil, diags } ephemerals := ctx.EphemeralResources() @@ -62,13 +62,13 @@ func ephemeralResourceOpen(ctx EvalContext, inp ephemeralResourceInput) tfdiags. ) diags = diags.Append(checkDiags) if diags.HasErrors() { - return diags // failed preconditions prevent further evaluation + return nil, diags // failed preconditions prevent further evaluation } configVal, _, configDiags := ctx.EvaluateBlock(config.Config, schema, nil, keyData) diags = diags.Append(configDiags) if diags.HasErrors() { - return diags + return nil, diags } unmarkedConfigVal, configMarks := configVal.UnmarkDeepWithPaths() @@ -79,20 +79,19 @@ func ephemeralResourceOpen(ctx EvalContext, inp ephemeralResourceInput) tfdiags. diags = diags.Append(validateResp.Diagnostics) if diags.HasErrors() { - return diags + return nil, diags } resp := provider.OpenEphemeralResource(providers.OpenEphemeralResourceRequest{ TypeName: inp.addr.ContainingResource().Resource.Type, Config: unmarkedConfigVal, }) - if resp.Deferred != nil { - // FIXME: Actually implement this. - diags = diags.Append(fmt.Errorf("we don't support deferral of ephemeral resource instances yet")) - } diags = diags.Append(resp.Diagnostics.InConfigBody(config.Config, inp.addr.String())) if diags.HasErrors() { - return diags + return nil, diags + } + if resp.Deferred != nil { + return resp.Deferred, diags } resultVal := resp.Result.MarkWithPaths(configMarks) @@ -110,7 +109,7 @@ func ephemeralResourceOpen(ctx EvalContext, inp ephemeralResourceInput) tfdiags. )).InConfigBody(config.Config, inp.addr.String()) } if diags.HasErrors() { - return diags + return nil, diags } // We are going to wholesale mark the entire resource as ephemeral. This @@ -133,7 +132,7 @@ func ephemeralResourceOpen(ctx EvalContext, inp ephemeralResourceInput) tfdiags. Private: resp.Private, }) - return diags + return nil, diags } // nodeEphemeralResourceClose is the node type for closing the previously-opened diff --git a/internal/terraform/node_resource_partial_plan.go b/internal/terraform/node_resource_partial_plan.go index c701885623..7ad41ff438 100644 --- a/internal/terraform/node_resource_partial_plan.go +++ b/internal/terraform/node_resource_partial_plan.go @@ -62,6 +62,9 @@ func (n *nodeExpandPlannableResource) dynamicExpandPartial(ctx EvalContext, know func() { ss := ctx.PrevRunState() + if ss == nil { + return // No previous state, so nothing to do here. + } state := ss.Lock() defer ss.Unlock() @@ -299,6 +302,12 @@ func (n *nodeExpandPlannableResource) knownModuleSubgraph(ctx EvalContext, addr }), DynamicTransformer(func(graph *Graph) error { + // Ephemeral resources don't need to be accounted for in this transform, + // since they are not in the state. + if addr.Resource.Mode == addrs.EphemeralResourceMode { + return nil + } + // We'll add nodes for any orphaned resources. rs := state.Resource(addr) if rs == nil { diff --git a/internal/terraform/node_resource_plan.go b/internal/terraform/node_resource_plan.go index ffd28c7154..8714990c20 100644 --- a/internal/terraform/node_resource_plan.go +++ b/internal/terraform/node_resource_plan.go @@ -114,12 +114,6 @@ func (n *nodeExpandPlannableResource) DynamicExpand(ctx EvalContext) (*Graph, tf expander := ctx.InstanceExpander() moduleInstances := expander.ExpandModule(n.Addr.Module, false) - if n.Addr.Resource.Mode == addrs.EphemeralResourceMode { - g, expandDiags := n.dynamicExpand(ctx, moduleInstances, addrs.Map[addrs.AbsResourceInstance, cty.Value]{}) - diags = diags.Append(expandDiags) - return g, diags - } - // The possibility of partial-expanded modules and resources is guarded by a // top-level option for the whole plan, so that we can preserve mainline // behavior for the modules runtime. So, we currently branch off into an diff --git a/internal/terraform/node_resource_plan_instance.go b/internal/terraform/node_resource_plan_instance.go index f07be0f340..f242f8b47d 100644 --- a/internal/terraform/node_resource_plan_instance.go +++ b/internal/terraform/node_resource_plan_instance.go @@ -155,11 +155,25 @@ func (n *NodePlannableResourceInstance) dataResourceExecute(ctx EvalContext) (di } func (n *NodePlannableResourceInstance) ephemeralResourceExecute(ctx EvalContext) (diags tfdiags.Diagnostics) { - return ephemeralResourceOpen(ctx, ephemeralResourceInput{ + deferrals := ctx.Deferrals() + // For deferred ephemeral resources, we don't need to do anything here. + if deferrals.ShouldDeferResourceInstanceChanges(n.Addr, n.Dependencies) { + deferrals.ReportEphemeralResourceInstanceDeferred(n.Addr, providers.DeferredReasonDeferredPrereq) + return nil + } + + deferred, diags := ephemeralResourceOpen(ctx, ephemeralResourceInput{ addr: n.Addr, config: n.Config, providerConfig: n.ResolvedProvider, }) + + if deferred != nil { + // Then this ephemeral resource has been deferred while opening. + deferrals.ReportEphemeralResourceInstanceDeferred(n.Addr, deferred.Reason) + } + + return diags } func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext) (diags tfdiags.Diagnostics) { diff --git a/internal/terraform/node_resource_plan_partialexp.go b/internal/terraform/node_resource_plan_partialexp.go index 15559e3c72..e65bae045c 100644 --- a/internal/terraform/node_resource_plan_partialexp.go +++ b/internal/terraform/node_resource_plan_partialexp.go @@ -97,6 +97,8 @@ func (n *nodePlannablePartialExpandedResource) Execute(ctx EvalContext, op walkO change, changeDiags := n.dataResourceExecute(ctx) diags = diags.Append(changeDiags) ctx.Deferrals().ReportDataSourceExpansionDeferred(n.addr, change) + case addrs.EphemeralResourceMode: + ctx.Deferrals().ReportEphemeralResourceExpansionDeferred(n.addr) default: panic(fmt.Errorf("unsupported resource mode %s", n.config.Mode)) }