From a77d351ea25cd4087d5a8c38305f1e2ecf113cac Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Tue, 13 Feb 2024 16:28:49 -0800 Subject: [PATCH] plugin+plugin6: Don't panic if ProviderMeta not populated "ProviderMeta" is a niche feature that's used by only two providers, but this code was written under the assumption that the caller would always provide it for any provider that has a schema for it, and unfortunately the Terraform SDK seems to always provide a meta schema even for providers that don't use it, and thus it's empty. The current phase of the "unknown_instances" language experiment is not fully wired in to the main logic as a way to reduce the risk of it impacting behavior for those not participating in the experiment, but that means that right now it doesn't actually have a resolved ProviderMeta value to present, and so was just omitting it on the assumption that it is always optional to provide anyway. As a pragmatic stopgap to resolve that conflict, this makes the gRPC provider dispatching logic tolerate an absent ProviderMeta value and synthesize a null value to use in that case. This makes that logic slightly more robust and also means we can defer doing all of the work to weave ProviderMeta into this experimental codepath. The new TODO comment in that codepath is intended to remind us to consider this again should we decide to stablize the experiment later; when we do that, we'll hopefully do it by refactoring to share more code between the two codepaths, at which point ProviderMeta will be easier to support. --- internal/plugin/grpc_provider.go | 14 ++++++++++++-- internal/plugin6/grpc_provider.go | 14 ++++++++++++-- .../terraform/node_resource_plan_partialexp.go | 4 ++++ 3 files changed, 28 insertions(+), 4 deletions(-) diff --git a/internal/plugin/grpc_provider.go b/internal/plugin/grpc_provider.go index ed32abc0a0..fdd203665b 100644 --- a/internal/plugin/grpc_provider.go +++ b/internal/plugin/grpc_provider.go @@ -483,7 +483,12 @@ func (p *GRPCProvider) PlanResourceChange(r providers.PlanResourceChangeRequest) } if metaSchema.Block != nil { - metaMP, err := msgpack.Marshal(r.ProviderMeta, metaSchema.Block.ImpliedType()) + metaTy := metaSchema.Block.ImpliedType() + metaVal := r.ProviderMeta + if metaVal == cty.NilVal { + metaVal = cty.NullVal(metaTy) + } + metaMP, err := msgpack.Marshal(metaVal, metaTy) if err != nil { resp.Diagnostics = resp.Diagnostics.Append(err) return resp @@ -558,7 +563,12 @@ func (p *GRPCProvider) ApplyResourceChange(r providers.ApplyResourceChangeReques } if metaSchema.Block != nil { - metaMP, err := msgpack.Marshal(r.ProviderMeta, metaSchema.Block.ImpliedType()) + metaTy := metaSchema.Block.ImpliedType() + metaVal := r.ProviderMeta + if metaVal == cty.NilVal { + metaVal = cty.NullVal(metaTy) + } + metaMP, err := msgpack.Marshal(metaVal, metaTy) if err != nil { resp.Diagnostics = resp.Diagnostics.Append(err) return resp diff --git a/internal/plugin6/grpc_provider.go b/internal/plugin6/grpc_provider.go index 5fbdd8d4f9..02a48b694f 100644 --- a/internal/plugin6/grpc_provider.go +++ b/internal/plugin6/grpc_provider.go @@ -472,7 +472,12 @@ func (p *GRPCProvider) PlanResourceChange(r providers.PlanResourceChangeRequest) } if metaSchema.Block != nil { - metaMP, err := msgpack.Marshal(r.ProviderMeta, metaSchema.Block.ImpliedType()) + metaTy := metaSchema.Block.ImpliedType() + metaVal := r.ProviderMeta + if metaVal == cty.NilVal { + metaVal = cty.NullVal(metaTy) + } + metaMP, err := msgpack.Marshal(metaVal, metaTy) if err != nil { resp.Diagnostics = resp.Diagnostics.Append(err) return resp @@ -547,7 +552,12 @@ func (p *GRPCProvider) ApplyResourceChange(r providers.ApplyResourceChangeReques } if metaSchema.Block != nil { - metaMP, err := msgpack.Marshal(r.ProviderMeta, metaSchema.Block.ImpliedType()) + metaTy := metaSchema.Block.ImpliedType() + metaVal := r.ProviderMeta + if metaVal == cty.NilVal { + metaVal = cty.NullVal(metaTy) + } + metaMP, err := msgpack.Marshal(metaVal, metaTy) if err != nil { resp.Diagnostics = resp.Diagnostics.Append(err) return resp diff --git a/internal/terraform/node_resource_plan_partialexp.go b/internal/terraform/node_resource_plan_partialexp.go index 3851f14757..73b0d3f54e 100644 --- a/internal/terraform/node_resource_plan_partialexp.go +++ b/internal/terraform/node_resource_plan_partialexp.go @@ -210,6 +210,10 @@ func (n *nodePlannablePartialExpandedResource) managedResourceExecute(ctx EvalCo Config: unmarkedConfigVal, PriorState: priorVal, ProposedNewState: proposedNewVal, + // TODO: Should we send "ProviderMeta" here? We don't have the + // necessary data for that wired through here right now, but + // we might need to do that before stabilizing support for unknown + // resource instance expansion. }) diags = diags.Append(resp.Diagnostics.InConfigBody(n.config.Config, n.addr.String())) if diags.HasErrors() {