diff --git a/CHANGELOG.md b/CHANGELOG.md index 6da4337d59..502885f844 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,8 +2,9 @@ ENHANCEMENTS: -* `tofu show` now supports a `-config` option, to be used in conjunction with `-json` to produce a machine-readable summary of the configuration without first creating a plan. ([#2820](https://github.com/opentofu/opentofu/pull/2820)) +* OpenTofu will now suggest using `-defer` if a provider reports that it cannot create a plan for a particular resource instance due to values that won't be known until the apply phase. ([#2643](https://github.com/opentofu/opentofu/pull/2643)) * `tofu validate` now supports running in a module that contains provider configuration_aliases. ([#2905](https://github.com/opentofu/opentofu/pull/2905)) +* `tofu show` now supports a `-config` option, to be used in conjunction with `-json` to produce a machine-readable summary of the configuration without first creating a plan. ([#2820](https://github.com/opentofu/opentofu/pull/2820)) ## Previous Releases diff --git a/internal/plugin/convert/deferral.go b/internal/plugin/convert/deferral.go new file mode 100644 index 0000000000..7eeab22b6d --- /dev/null +++ b/internal/plugin/convert/deferral.go @@ -0,0 +1,24 @@ +// Copyright (c) The OpenTofu Authors +// SPDX-License-Identifier: MPL-2.0 +// Copyright (c) 2023 HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package convert + +import ( + "github.com/opentofu/opentofu/internal/providers" + "github.com/opentofu/opentofu/internal/tfplugin5" +) + +func DeferralReasonFromProto(reason tfplugin5.Deferred_Reason) providers.DeferralReason { + switch reason { + case tfplugin5.Deferred_RESOURCE_CONFIG_UNKNOWN: + return providers.DeferredBecauseResourceConfigUnknown + case tfplugin5.Deferred_PROVIDER_CONFIG_UNKNOWN: + return providers.DeferredBecauseProviderConfigUnknown + case tfplugin5.Deferred_ABSENT_PREREQ: + return providers.DeferredBecausePrereqAbsent + default: + return providers.DeferredReasonUnknown + } +} diff --git a/internal/plugin/grpc_provider.go b/internal/plugin/grpc_provider.go index 7459031f47..36e4c6f382 100644 --- a/internal/plugin/grpc_provider.go +++ b/internal/plugin/grpc_provider.go @@ -11,9 +11,8 @@ import ( "fmt" "sync" - "github.com/zclconf/go-cty/cty" - plugin "github.com/hashicorp/go-plugin" + "github.com/zclconf/go-cty/cty" ctyjson "github.com/zclconf/go-cty/cty/json" "github.com/zclconf/go-cty/cty/msgpack" "google.golang.org/grpc" @@ -33,6 +32,17 @@ type GRPCProviderPlugin struct { GRPCProvider func() proto.ProviderServer } +var clientCapabilities = &proto.ClientCapabilities{ + // DeferralAllowed tells the provider that it is allowed to respond to + // all of the various post-configuration requests (as described by the + // [providers.Configured] interface) by reporting that the request + // must be "deferred" because there isn't yet enough information to + // satisfy the request. Setting this means that we need to be prepared + // for there to be a "deferred" object in the response from various + // other provider RPC functions. + DeferralAllowed: true, +} + func (p *GRPCProviderPlugin) GRPCClient(ctx context.Context, broker *plugin.GRPCBroker, c *grpc.ClientConn) (interface{}, error) { return &GRPCProvider{ client: proto.NewProviderClient(c), @@ -362,6 +372,7 @@ func (p *GRPCProvider) ConfigureProvider(ctx context.Context, r providers.Config Config: &proto.DynamicValue{ Msgpack: mp, }, + ClientCapabilities: clientCapabilities, } protoResp, err := p.client.Configure(ctx, protoReq) @@ -417,9 +428,10 @@ func (p *GRPCProvider) ReadResource(ctx context.Context, r providers.ReadResourc } protoReq := &proto.ReadResource_Request{ - TypeName: r.TypeName, - CurrentState: &proto.DynamicValue{Msgpack: mp}, - Private: r.Private, + TypeName: r.TypeName, + CurrentState: &proto.DynamicValue{Msgpack: mp}, + Private: r.Private, + ClientCapabilities: clientCapabilities, } if metaSchema.Block != nil { @@ -437,6 +449,11 @@ func (p *GRPCProvider) ReadResource(ctx context.Context, r providers.ReadResourc return resp } resp.Diagnostics = resp.Diagnostics.Append(convert.ProtoToDiagnostics(protoResp.Diagnostics)) + if protoDeferred := protoResp.Deferred; protoDeferred != nil { + reason := convert.DeferralReasonFromProto(protoDeferred.Reason) + resp.Diagnostics = resp.Diagnostics.Append(providers.NewDeferralDiagnostic(reason)) + return resp + } state, err := decodeDynamicValue(protoResp.NewState, resSchema.Block.ImpliedType()) if err != nil { @@ -494,11 +511,12 @@ func (p *GRPCProvider) PlanResourceChange(ctx context.Context, r providers.PlanR } protoReq := &proto.PlanResourceChange_Request{ - TypeName: r.TypeName, - PriorState: &proto.DynamicValue{Msgpack: priorMP}, - Config: &proto.DynamicValue{Msgpack: configMP}, - ProposedNewState: &proto.DynamicValue{Msgpack: propMP}, - PriorPrivate: r.PriorPrivate, + TypeName: r.TypeName, + PriorState: &proto.DynamicValue{Msgpack: priorMP}, + Config: &proto.DynamicValue{Msgpack: configMP}, + ProposedNewState: &proto.DynamicValue{Msgpack: propMP}, + PriorPrivate: r.PriorPrivate, + ClientCapabilities: clientCapabilities, } if metaSchema.Block != nil { @@ -516,6 +534,11 @@ func (p *GRPCProvider) PlanResourceChange(ctx context.Context, r providers.PlanR return resp } resp.Diagnostics = resp.Diagnostics.Append(convert.ProtoToDiagnostics(protoResp.Diagnostics)) + if protoDeferred := protoResp.Deferred; protoDeferred != nil { + reason := convert.DeferralReasonFromProto(protoDeferred.Reason) + resp.Diagnostics = resp.Diagnostics.Append(providers.NewDeferralDiagnostic(reason)) + return resp + } state, err := decodeDynamicValue(protoResp.PlannedState, resSchema.Block.ImpliedType()) if err != nil { @@ -623,8 +646,9 @@ func (p *GRPCProvider) ImportResourceState(ctx context.Context, r providers.Impo } protoReq := &proto.ImportResourceState_Request{ - TypeName: r.TypeName, - Id: r.ID, + TypeName: r.TypeName, + Id: r.ID, + ClientCapabilities: clientCapabilities, } protoResp, err := p.client.ImportResourceState(ctx, protoReq) @@ -633,6 +657,11 @@ func (p *GRPCProvider) ImportResourceState(ctx context.Context, r providers.Impo return resp } resp.Diagnostics = resp.Diagnostics.Append(convert.ProtoToDiagnostics(protoResp.Diagnostics)) + if protoDeferred := protoResp.Deferred; protoDeferred != nil { + reason := convert.DeferralReasonFromProto(protoDeferred.Reason) + resp.Diagnostics = resp.Diagnostics.Append(providers.NewDeferralDiagnostic(reason)) + return resp + } for _, imported := range protoResp.ImportedResources { resource := providers.ImportedResource{ @@ -731,6 +760,7 @@ func (p *GRPCProvider) ReadDataSource(ctx context.Context, r providers.ReadDataS Config: &proto.DynamicValue{ Msgpack: config, }, + ClientCapabilities: clientCapabilities, } if metaSchema.Block != nil { @@ -748,6 +778,11 @@ func (p *GRPCProvider) ReadDataSource(ctx context.Context, r providers.ReadDataS return resp } resp.Diagnostics = resp.Diagnostics.Append(convert.ProtoToDiagnostics(protoResp.Diagnostics)) + if protoDeferred := protoResp.Deferred; protoDeferred != nil { + reason := convert.DeferralReasonFromProto(protoDeferred.Reason) + resp.Diagnostics = resp.Diagnostics.Append(providers.NewDeferralDiagnostic(reason)) + return resp + } state, err := decodeDynamicValue(protoResp.State, dataSchema.Block.ImpliedType()) if err != nil { diff --git a/internal/plugin/grpc_provider_test.go b/internal/plugin/grpc_provider_test.go index a52a48aeba..e39c8adc83 100644 --- a/internal/plugin/grpc_provider_test.go +++ b/internal/plugin/grpc_provider_test.go @@ -10,6 +10,7 @@ import ( "fmt" "testing" + "github.com/davecgh/go-spew/spew" "github.com/google/go-cmp/cmp" "github.com/zclconf/go-cty/cty" "go.uber.org/mock/gomock" @@ -622,6 +623,52 @@ func TestGRPCProvider_PlanResourceChange(t *testing.T) { } } +func TestGRPCProvider_PlanResourceChange_deferred(t *testing.T) { + client := mockProviderClient(t) + p := &GRPCProvider{ + client: client, + } + + client.EXPECT().PlanResourceChange( + gomock.Any(), + gomock.Any(), + ).Return(&proto.PlanResourceChange_Response{ + PlannedState: &proto.DynamicValue{ + Msgpack: []byte("\x81\xa4attr\xa3bar"), + }, + Deferred: &proto.Deferred{ + Reason: proto.Deferred_PROVIDER_CONFIG_UNKNOWN, + }, + }, nil) + + resp := p.PlanResourceChange(t.Context(), providers.PlanResourceChangeRequest{ + TypeName: "resource", + PriorState: cty.ObjectVal(map[string]cty.Value{ + "attr": cty.StringVal("foo"), + }), + ProposedNewState: cty.ObjectVal(map[string]cty.Value{ + "attr": cty.StringVal("bar"), + }), + Config: cty.ObjectVal(map[string]cty.Value{ + "attr": cty.StringVal("bar"), + }), + }) + + if len(resp.Diagnostics) != 1 { + t.Fatal("wrong number of diagnostics; want one\n" + spew.Sdump(resp.Diagnostics)) + } + desc := resp.Diagnostics[0].Description() + if got, want := desc.Summary, `Provider configuration is incomplete`; got != want { + t.Errorf("wrong error summary\ngot: %s\nwant: %s", got, want) + } + if got, want := desc.Detail, `The provider was unable to work with this resource because the associated provider configuration makes use of values from other resources that will not be known until after apply.`; got != want { + t.Errorf("wrong error detail\ngot: %s\nwant: %s", got, want) + } + if !providers.IsDeferralDiagnostic(resp.Diagnostics[0]) { + t.Errorf("diagnostic is not marked as being a \"deferral diagnostic\"") + } +} + func TestGRPCProvider_PlanResourceChangeJSON(t *testing.T) { client := mockProviderClient(t) p := &GRPCProvider{ diff --git a/internal/plugin6/convert/deferral.go b/internal/plugin6/convert/deferral.go new file mode 100644 index 0000000000..d47420a394 --- /dev/null +++ b/internal/plugin6/convert/deferral.go @@ -0,0 +1,24 @@ +// Copyright (c) The OpenTofu Authors +// SPDX-License-Identifier: MPL-2.0 +// Copyright (c) 2023 HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package convert + +import ( + "github.com/opentofu/opentofu/internal/providers" + "github.com/opentofu/opentofu/internal/tfplugin6" +) + +func DeferralReasonFromProto(reason tfplugin6.Deferred_Reason) providers.DeferralReason { + switch reason { + case tfplugin6.Deferred_RESOURCE_CONFIG_UNKNOWN: + return providers.DeferredBecauseResourceConfigUnknown + case tfplugin6.Deferred_PROVIDER_CONFIG_UNKNOWN: + return providers.DeferredBecauseProviderConfigUnknown + case tfplugin6.Deferred_ABSENT_PREREQ: + return providers.DeferredBecausePrereqAbsent + default: + return providers.DeferredReasonUnknown + } +} diff --git a/internal/plugin6/grpc_provider.go b/internal/plugin6/grpc_provider.go index 49acb3c089..c417c2ba5b 100644 --- a/internal/plugin6/grpc_provider.go +++ b/internal/plugin6/grpc_provider.go @@ -11,9 +11,8 @@ import ( "fmt" "sync" - "github.com/zclconf/go-cty/cty" - plugin "github.com/hashicorp/go-plugin" + "github.com/zclconf/go-cty/cty" ctyjson "github.com/zclconf/go-cty/cty/json" "github.com/zclconf/go-cty/cty/msgpack" "google.golang.org/grpc" @@ -33,6 +32,17 @@ type GRPCProviderPlugin struct { GRPCProvider func() proto6.ProviderServer } +var clientCapabilities = &proto6.ClientCapabilities{ + // DeferralAllowed tells the provider that it is allowed to respond to + // all of the various post-configuration requests (as described by the + // [providers.Configured] interface) by reporting that the request + // must be "deferred" because there isn't yet enough information to + // satisfy the request. Setting this means that we need to be prepared + // for there to be a "deferred" object in the response from various + // other provider RPC functions. + DeferralAllowed: true, +} + func (p *GRPCProviderPlugin) GRPCClient(ctx context.Context, broker *plugin.GRPCBroker, c *grpc.ClientConn) (interface{}, error) { return &GRPCProvider{ client: proto6.NewProviderClient(c), @@ -351,6 +361,7 @@ func (p *GRPCProvider) ConfigureProvider(ctx context.Context, r providers.Config Config: &proto6.DynamicValue{ Msgpack: mp, }, + ClientCapabilities: clientCapabilities, } protoResp, err := p.client.ConfigureProvider(ctx, protoReq) @@ -406,9 +417,10 @@ func (p *GRPCProvider) ReadResource(ctx context.Context, r providers.ReadResourc } protoReq := &proto6.ReadResource_Request{ - TypeName: r.TypeName, - CurrentState: &proto6.DynamicValue{Msgpack: mp}, - Private: r.Private, + TypeName: r.TypeName, + CurrentState: &proto6.DynamicValue{Msgpack: mp}, + Private: r.Private, + ClientCapabilities: clientCapabilities, } if metaSchema.Block != nil { @@ -426,6 +438,11 @@ func (p *GRPCProvider) ReadResource(ctx context.Context, r providers.ReadResourc return resp } resp.Diagnostics = resp.Diagnostics.Append(convert.ProtoToDiagnostics(protoResp.Diagnostics)) + if protoDeferred := protoResp.Deferred; protoDeferred != nil { + reason := convert.DeferralReasonFromProto(protoDeferred.Reason) + resp.Diagnostics = resp.Diagnostics.Append(providers.NewDeferralDiagnostic(reason)) + return resp + } state, err := decodeDynamicValue(protoResp.NewState, resSchema.Block.ImpliedType()) if err != nil { @@ -483,11 +500,12 @@ func (p *GRPCProvider) PlanResourceChange(ctx context.Context, r providers.PlanR } protoReq := &proto6.PlanResourceChange_Request{ - TypeName: r.TypeName, - PriorState: &proto6.DynamicValue{Msgpack: priorMP}, - Config: &proto6.DynamicValue{Msgpack: configMP}, - ProposedNewState: &proto6.DynamicValue{Msgpack: propMP}, - PriorPrivate: r.PriorPrivate, + TypeName: r.TypeName, + PriorState: &proto6.DynamicValue{Msgpack: priorMP}, + Config: &proto6.DynamicValue{Msgpack: configMP}, + ProposedNewState: &proto6.DynamicValue{Msgpack: propMP}, + PriorPrivate: r.PriorPrivate, + ClientCapabilities: clientCapabilities, } if metaSchema.Block != nil { @@ -505,6 +523,11 @@ func (p *GRPCProvider) PlanResourceChange(ctx context.Context, r providers.PlanR return resp } resp.Diagnostics = resp.Diagnostics.Append(convert.ProtoToDiagnostics(protoResp.Diagnostics)) + if protoDeferred := protoResp.Deferred; protoDeferred != nil { + reason := convert.DeferralReasonFromProto(protoDeferred.Reason) + resp.Diagnostics = resp.Diagnostics.Append(providers.NewDeferralDiagnostic(reason)) + return resp + } state, err := decodeDynamicValue(protoResp.PlannedState, resSchema.Block.ImpliedType()) if err != nil { @@ -612,8 +635,9 @@ func (p *GRPCProvider) ImportResourceState(ctx context.Context, r providers.Impo } protoReq := &proto6.ImportResourceState_Request{ - TypeName: r.TypeName, - Id: r.ID, + TypeName: r.TypeName, + Id: r.ID, + ClientCapabilities: clientCapabilities, } protoResp, err := p.client.ImportResourceState(ctx, protoReq) @@ -622,6 +646,11 @@ func (p *GRPCProvider) ImportResourceState(ctx context.Context, r providers.Impo return resp } resp.Diagnostics = resp.Diagnostics.Append(convert.ProtoToDiagnostics(protoResp.Diagnostics)) + if protoDeferred := protoResp.Deferred; protoDeferred != nil { + reason := convert.DeferralReasonFromProto(protoDeferred.Reason) + resp.Diagnostics = resp.Diagnostics.Append(providers.NewDeferralDiagnostic(reason)) + return resp + } for _, imported := range protoResp.ImportedResources { resource := providers.ImportedResource{ @@ -720,6 +749,7 @@ func (p *GRPCProvider) ReadDataSource(ctx context.Context, r providers.ReadDataS Config: &proto6.DynamicValue{ Msgpack: config, }, + ClientCapabilities: clientCapabilities, } if metaSchema.Block != nil { @@ -737,6 +767,11 @@ func (p *GRPCProvider) ReadDataSource(ctx context.Context, r providers.ReadDataS return resp } resp.Diagnostics = resp.Diagnostics.Append(convert.ProtoToDiagnostics(protoResp.Diagnostics)) + if protoDeferred := protoResp.Deferred; protoDeferred != nil { + reason := convert.DeferralReasonFromProto(protoDeferred.Reason) + resp.Diagnostics = resp.Diagnostics.Append(providers.NewDeferralDiagnostic(reason)) + return resp + } state, err := decodeDynamicValue(protoResp.State, dataSchema.Block.ImpliedType()) if err != nil { diff --git a/internal/plugin6/grpc_provider_test.go b/internal/plugin6/grpc_provider_test.go index aa6d86009b..86a6f32fc4 100644 --- a/internal/plugin6/grpc_provider_test.go +++ b/internal/plugin6/grpc_provider_test.go @@ -10,6 +10,7 @@ import ( "fmt" "testing" + "github.com/davecgh/go-spew/spew" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" "github.com/zclconf/go-cty/cty" @@ -628,6 +629,52 @@ func TestGRPCProvider_PlanResourceChange(t *testing.T) { } } +func TestGRPCProvider_PlanResourceChange_deferred(t *testing.T) { + client := mockProviderClient(t) + p := &GRPCProvider{ + client: client, + } + + client.EXPECT().PlanResourceChange( + gomock.Any(), + gomock.Any(), + ).Return(&proto.PlanResourceChange_Response{ + PlannedState: &proto.DynamicValue{ + Msgpack: []byte("\x81\xa4attr\xa3bar"), + }, + Deferred: &proto.Deferred{ + Reason: proto.Deferred_PROVIDER_CONFIG_UNKNOWN, + }, + }, nil) + + resp := p.PlanResourceChange(t.Context(), providers.PlanResourceChangeRequest{ + TypeName: "resource", + PriorState: cty.ObjectVal(map[string]cty.Value{ + "attr": cty.StringVal("foo"), + }), + ProposedNewState: cty.ObjectVal(map[string]cty.Value{ + "attr": cty.StringVal("bar"), + }), + Config: cty.ObjectVal(map[string]cty.Value{ + "attr": cty.StringVal("bar"), + }), + }) + + if len(resp.Diagnostics) != 1 { + t.Fatal("wrong number of diagnostics; want one\n" + spew.Sdump(resp.Diagnostics)) + } + desc := resp.Diagnostics[0].Description() + if got, want := desc.Summary, `Provider configuration is incomplete`; got != want { + t.Errorf("wrong error summary\ngot: %s\nwant: %s", got, want) + } + if got, want := desc.Detail, `The provider was unable to work with this resource because the associated provider configuration makes use of values from other resources that will not be known until after apply.`; got != want { + t.Errorf("wrong error detail\ngot: %s\nwant: %s", got, want) + } + if !providers.IsDeferralDiagnostic(resp.Diagnostics[0]) { + t.Errorf("diagnostic is not marked as being a \"deferral diagnostic\"") + } +} + func TestGRPCProvider_PlanResourceChangeJSON(t *testing.T) { client := mockProviderClient(t) p := &GRPCProvider{ diff --git a/internal/providers/deferral.go b/internal/providers/deferral.go new file mode 100644 index 0000000000..b3c41157be --- /dev/null +++ b/internal/providers/deferral.go @@ -0,0 +1,128 @@ +// Copyright (c) The OpenTofu Authors +// SPDX-License-Identifier: MPL-2.0 +// Copyright (c) 2023 HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package providers + +import ( + "github.com/opentofu/opentofu/internal/tfdiags" +) + +// DeferralReason is an enumeration of the different reasons a provider might +// return when reporting why it is unable to perform a requested action with +// the currently-available context. +type DeferralReason int + +//go:generate go run golang.org/x/tools/cmd/stringer -type=DeferralReason + +const ( + // DeferredReasonUnknown is the zero value of DeferralReason, used when + // a provider returns an unsupported deferral reason. + DeferredReasonUnknown DeferralReason = 0 + + // DeferredBecauseResourceConfigUnknown indicates that the request cannot + // be completed because of unknown values in the resource configuration. + DeferredBecauseResourceConfigUnknown DeferralReason = 1 + + // DeferredBecauseProviderConfigUnknown indicates that the request cannot + // be completed because of unknown values in the provider configuration. + DeferredBecauseProviderConfigUnknown DeferralReason = 2 + + // DeferredBecausePrereqAbsent indicates that the request cannot be + // completed because a hard dependency has not been satisfied. + DeferredBecausePrereqAbsent DeferralReason = 3 +) + +// NewDeferralDiagnostic returns a contextual error diagnostic reporting that +// an operation cannot be completed for the given deferral reason. +// +// The returned diagnostic will cause [IsDeferralDiagnostic] to return true, +// so that callers can optionally annotate the diagnostic with suggestions +// for how to skip the affected request so that other unaffected requests can +// still be completed. +func NewDeferralDiagnostic(reason DeferralReason) tfdiags.Diagnostic { + var summary, detail string + switch reason { + case DeferredBecauseResourceConfigUnknown: + summary = "Resource configuration is incomplete" + detail = "The provider was unable to act on this resource configuration because it makes use of values from other resources that will not be known until after apply." + case DeferredBecauseProviderConfigUnknown: + summary = "Provider configuration is incomplete" + detail = "The provider was unable to work with this resource because the associated provider configuration makes use of values from other resources that will not be known until after apply." + default: + // This is the most general (and therefore least helpful) message, which + // we'll return if the provider produces a reason that we don't know about + // yet, such as one added in a later protocol version. This fallback + // is very much a last resort. + // (Note that this also currently handles DeferredBecausePrereqAbsent, + // because there are not yet any known examples of providers using that + // reason and so we don't yet know what it will turn out to mean in + // practice. If it becomes used in more providers in future then we can + // hopefully devise a better message that describes what those providers + // use it to mean.) + summary = "Operation cannot be completed yet" + detail = "The provider reported that it is not able to perform the requested operation until more information is available." + } + + // We start with a "whole-body" contextual diagnostic, so that the caller + // can elaborate this with any configuration body whose presence caused + // the request to be made and thus add useful source location information + // that we cannot determine at the provider layer. + contextual := tfdiags.WholeContainingBody(tfdiags.Error, summary, detail) + + // We then further wrap that contextual diagnostic in an override so that + // we can annotate it with the "extra info" that IsDeferralDiagnostic + // will use to recognize diagnostics returned from this function. + return tfdiags.Override(contextual, tfdiags.Error, func() tfdiags.DiagnosticExtraWrapper { + return &deferralDiagnosticExtraImpl{reason: reason} + }) +} + +// IsDeferralDiagnostic returns true if the given diagnostic was constructed +// with NewDeferralDiagnostic, meaning that it describes a situation where a +// provider reported that it is not yet able to complete an operation with the +// currently-available context. +func IsDeferralDiagnostic(diag tfdiags.Diagnostic) bool { + // NOTE: We're intentionally not actually exposing the deferral reason + // in the first incarnation of this functionality because the associated + // plugin protocol feature is not yet well-deployed and so we're trying + // to keep our use of it as confined to the provider-related packages + // as possible in case ongoing protocol evolution causes us to need to + // revise this in a later version. However, once the underlying protocol + // has settled it would be reasonable to actually expose the reason + // so that callers can substitute more contextually-relevant versions of + // the error diagnostics when appropriate, or indeed choose to handle + // this situation in a way that doesn't return an error to the user at all. + extra := tfdiags.ExtraInfo[deferralDiagnosticExtra](diag) + return extra != nil +} + +type deferralDiagnosticExtra interface { + deferralReason() DeferralReason +} + +// deferralDiagnosticExtraImpl is an indirection used to combine +// deferralDiagnosticExtra with tfdiags.DiagnosticExtraWrapper because the +// design of tfdiags.Override unfortunately exposes its implementation +// detail of possibly needing to preserve an existing "extra info" +// as part of its public API. +// +// (Maybe we can improve tfdiags.Override to encapsulate this better in +// future, so that the type representing the union of two "extra info" +// values can be an unexported struct within package tfdiags, but +// this API is already being used elsewhere and so is risky to change.) +type deferralDiagnosticExtraImpl struct { + reason DeferralReason + wrapped any +} + +// WrapDiagnosticExtra implements deferralDiagnosticExtra. +func (d *deferralDiagnosticExtraImpl) deferralReason() DeferralReason { + return d.reason +} + +// WrapDiagnosticExtra implements tfdiags.DiagnosticExtraWrapper. +func (d *deferralDiagnosticExtraImpl) WrapDiagnosticExtra(inner any) { + d.wrapped = inner +} diff --git a/internal/providers/deferralreason_string.go b/internal/providers/deferralreason_string.go new file mode 100644 index 0000000000..502fbdc891 --- /dev/null +++ b/internal/providers/deferralreason_string.go @@ -0,0 +1,26 @@ +// Code generated by "stringer -type=DeferralReason"; DO NOT EDIT. + +package providers + +import "strconv" + +func _() { + // An "invalid array index" compiler error signifies that the constant values have changed. + // Re-run the stringer command to generate them again. + var x [1]struct{} + _ = x[DeferredReasonUnknown-0] + _ = x[DeferredBecauseResourceConfigUnknown-1] + _ = x[DeferredBecauseProviderConfigUnknown-2] + _ = x[DeferredBecausePrereqAbsent-3] +} + +const _DeferralReason_name = "DeferredReasonUnknownDeferredBecauseResourceConfigUnknownDeferredBecauseProviderConfigUnknownDeferredBecausePrereqAbsent" + +var _DeferralReason_index = [...]uint8{0, 21, 57, 93, 120} + +func (i DeferralReason) String() string { + if i < 0 || i >= DeferralReason(len(_DeferralReason_index)-1) { + return "DeferralReason(" + strconv.FormatInt(int64(i), 10) + ")" + } + return _DeferralReason_name[_DeferralReason_index[i]:_DeferralReason_index[i+1]] +} diff --git a/internal/tofu/context_plan2_test.go b/internal/tofu/context_plan2_test.go index 5e7aadc231..ed644f9733 100644 --- a/internal/tofu/context_plan2_test.go +++ b/internal/tofu/context_plan2_test.go @@ -17,11 +17,10 @@ import ( "github.com/davecgh/go-spew/spew" "github.com/google/go-cmp/cmp" - "github.com/zclconf/go-cty/cty" - "github.com/hashicorp/hcl/v2" "github.com/opentofu/opentofu/internal/addrs" "github.com/opentofu/opentofu/internal/checks" + "github.com/zclconf/go-cty/cty" // "github.com/opentofu/opentofu/internal/configs" "github.com/opentofu/opentofu/internal/configs/configschema" @@ -4911,6 +4910,97 @@ func TestContext2Plan_dataSourceReadPlanError(t *testing.T) { } } +func TestContext2Plan_providerDefersPlanning(t *testing.T) { + m := testModuleInline(t, map[string]string{ + "main.tf": ` + resource "test" "test" { + } + `, + }) + + tests := []struct { + DeferralReason providers.DeferralReason + WantDiagSummary, WantDiagDetail string + }{ + { + DeferralReason: providers.DeferredBecauseProviderConfigUnknown, + WantDiagSummary: `Provider configuration is incomplete`, + WantDiagDetail: `The provider was unable to work with this resource because the associated provider configuration makes use of values from other resources that will not be known until after apply. + +To work around this, use the planning option -exclude="test.test" to first apply without this object, and then apply normally to converge.`, + }, + { + DeferralReason: providers.DeferredBecauseResourceConfigUnknown, + WantDiagSummary: `Resource configuration is incomplete`, + WantDiagDetail: `The provider was unable to act on this resource configuration because it makes use of values from other resources that will not be known until after apply. + +To work around this, use the planning option -exclude="test.test" to first apply without this object, and then apply normally to converge.`, + }, + { + // This one is currently a generic fallback message because it's + // unclear what this reason is intended to mean and no providers + // are using it yet at the time of writing. + DeferralReason: providers.DeferredBecausePrereqAbsent, + WantDiagSummary: `Operation cannot be completed yet`, + WantDiagDetail: `The provider reported that it is not able to perform the requested operation until more information is available. + +To work around this, use the planning option -exclude="test.test" to first apply without this object, and then apply normally to converge.`, + }, + { + // This special reason is the one we use if a provider returns + // a later-added reason that the current OpenTofu version doesn't + // know about. + DeferralReason: providers.DeferredReasonUnknown, + WantDiagSummary: `Operation cannot be completed yet`, + WantDiagDetail: `The provider reported that it is not able to perform the requested operation until more information is available. + +To work around this, use the planning option -exclude="test.test" to first apply without this object, and then apply normally to converge.`, + }, + } + + for _, test := range tests { + t.Run(test.DeferralReason.String(), func(t *testing.T) { + provider := &MockProvider{ + GetProviderSchemaResponse: &providers.GetProviderSchemaResponse{ + ResourceTypes: map[string]providers.Schema{ + "test": { + Block: &configschema.Block{}, + }, + }, + }, + PlanResourceChangeFn: func(req providers.PlanResourceChangeRequest) providers.PlanResourceChangeResponse { + var diags tfdiags.Diagnostics + diags = diags.Append(providers.NewDeferralDiagnostic( + test.DeferralReason, + )) + return providers.PlanResourceChangeResponse{ + Diagnostics: diags, + } + }, + } + tofuCtx := testContext2(t, &ContextOpts{ + Providers: map[addrs.Provider]providers.Factory{ + addrs.NewDefaultProvider("test"): testProviderFuncFixed(provider), + }, + }) + _, diags := tofuCtx.Plan(t.Context(), m, states.NewState(), DefaultPlanOpts) + if !diags.HasErrors() { + t.Fatal("plan succeeded; want an error") + } + if len(diags) != 1 { + t.Fatal("wrong number of diagnostics; want one\n" + spew.Sdump(diags.ForRPC())) + } + desc := diags[0].Description() + if got, want := test.WantDiagSummary, desc.Summary; got != want { + t.Errorf("wrong error summary\ngot: %s\nwant: %s", got, want) + } + if got, want := test.WantDiagDetail, desc.Detail; got != want { + t.Errorf("wrong error detail\ngot: %s\nwant: %s", got, want) + } + }) + } +} + func TestContext2Plan_ignoredMarkedValue(t *testing.T) { m := testModuleInline(t, map[string]string{ "main.tf": ` diff --git a/internal/tofu/node_resource_abstract_instance.go b/internal/tofu/node_resource_abstract_instance.go index cfb8b99b3d..097c08165c 100644 --- a/internal/tofu/node_resource_abstract_instance.go +++ b/internal/tofu/node_resource_abstract_instance.go @@ -2821,3 +2821,60 @@ func (n *NodeAbstractResourceInstance) getProvider(ctx context.Context, evalCtx return provider, schema, nil } + +func maybeImproveResourceInstanceDiagnostics(diags tfdiags.Diagnostics, excludeAddr addrs.Targetable) tfdiags.Diagnostics { + // We defer allocating a new diagnostics array until we know we need to + // change something, because most of the time we'll just be returning + // the given diagnostics verbatim. + var ret tfdiags.Diagnostics + for i, diag := range diags { + if excludeAddr != nil && providers.IsDeferralDiagnostic(diag) { + // We've found a diagnostic we want to change, so we'll allocate + // a new diagnostics array if we didn't already. + if ret == nil { + ret = make(tfdiags.Diagnostics, len(diags)) + copy(ret, diags) + } + // FIXME: The following is a hack to slightly modify the diagnostic + // with an extra paragraph of detail content. If this becomes a + // more common need elsewhere then we should find a less clunky way + // to do this, probably with a new feature in tfdiags. + desc := diag.Description() + src := diag.Source() + extraDetail := fmt.Sprintf( + // FIXME: This should use a technique similar to evalchecks.commandLineArgumentsSuggestion + // to generate appropriate quoting/escaping of the address for the current platform. + "\n\nTo work around this, use the planning option -exclude=%q to first apply without this object, and then apply normally to converge.", + excludeAddr.String(), + ) + newDiag := &hcl.Diagnostic{ + Severity: diag.Severity().ToHCL(), + Summary: desc.Summary, + Detail: desc.Detail + extraDetail, + } + if src.Subject != nil { + newDiag.Subject = src.Subject.ToHCL().Ptr() + } + if src.Context != nil { + newDiag.Context = src.Context.ToHCL().Ptr() + } + // The following is a little awkward because of how tfdiags is + // designed: we need to "append" a new diagnostic over the + // one we're trying to replace so that tfdiags has an opportunity + // to transform it, so we'll make a zero-length slice whose + // capacity covers the one element we're trying to replace. + appendTo := ret[i : i : i+1] + appendTo = appendTo.Append(newDiag) + // appendTo.Append isn't _actually_ required to use the + // capacity we gave it (that's an implementation detail) + // so just to make sure we'll copy from what was returned + // into the final slot. This is likely to be a no-op in most + // cases. + ret[i] = appendTo[0] + } + } + if ret == nil { // We didn't change anything + return diags + } + return ret +} diff --git a/internal/tofu/node_resource_abstract_instance_test.go b/internal/tofu/node_resource_abstract_instance_test.go index ed7b12be7a..57aa19861f 100644 --- a/internal/tofu/node_resource_abstract_instance_test.go +++ b/internal/tofu/node_resource_abstract_instance_test.go @@ -15,7 +15,9 @@ import ( "github.com/opentofu/opentofu/internal/addrs" "github.com/opentofu/opentofu/internal/configs" "github.com/opentofu/opentofu/internal/configs/configschema" + "github.com/opentofu/opentofu/internal/providers" "github.com/opentofu/opentofu/internal/states" + "github.com/opentofu/opentofu/internal/tfdiags" ) func TestNodeAbstractResourceInstanceProvider(t *testing.T) { @@ -266,3 +268,72 @@ func TestFilterResourceProvisioners(t *testing.T) { }) } } + +func TestMaybeImproveResourceInstanceDiagnostics(t *testing.T) { + // This test is focused mainly on whether + // maybeImproveResourceInstanceDiagnostics is able to correctly identify + // deferral-related diagnostics and transform them, while keeping + // other unrelated diagnostics intact and unmodified. + // TestContext2Plan_providerDefersPlanning tests that the effect of + // this function is exposed externally when a provider's PlanResourceChange + // method returns a suitable diagnostic. + + var input tfdiags.Diagnostics + input = input.Append(tfdiags.Sourceless( + tfdiags.Warning, + "This is not a deferral-related diagnostic", + "This one should not be modified at all.", + )) + input = input.Append(providers.NewDeferralDiagnostic(providers.DeferredBecauseProviderConfigUnknown)) + input = input.Append(tfdiags.Sourceless( + tfdiags.Error, + "This is not a deferral-related diagnostic", + "This one should not be modified at all.", + )) + input = input.Append(providers.NewDeferralDiagnostic(providers.DeferredBecauseResourceConfigUnknown)) + input = input.Append(tfdiags.Sourceless( + tfdiags.Error, + "This is not a deferral-related diagnostic either", + "Leave this one alone too.", + )) + + // We'll use ForRPC here just to make the diagnostics easier to compare, + // since we care primarily about their description test here. + got := maybeImproveResourceInstanceDiagnostics(input, mustAbsResourceAddr("foo.bar").Instance(addrs.IntKey(1))).ForRPC() + var want tfdiags.Diagnostics + want = want.Append(tfdiags.Sourceless( + tfdiags.Warning, + "This is not a deferral-related diagnostic", + "This one should not be modified at all.", + )) + want = want.Append(tfdiags.Sourceless( + tfdiags.Error, + "Provider configuration is incomplete", + `The provider was unable to work with this resource because the associated provider configuration makes use of values from other resources that will not be known until after apply. + +To work around this, use the planning option -exclude="foo.bar[1]" to first apply without this object, and then apply normally to converge.`, + )) + want = want.Append(tfdiags.Sourceless( + tfdiags.Error, + "This is not a deferral-related diagnostic", + "This one should not be modified at all.", + )) + want = want.Append(tfdiags.Sourceless( + tfdiags.Error, + "Resource configuration is incomplete", + `The provider was unable to act on this resource configuration because it makes use of values from other resources that will not be known until after apply. + +To work around this, use the planning option -exclude="foo.bar[1]" to first apply without this object, and then apply normally to converge.`, + )) + want = want.Append(tfdiags.Sourceless( + tfdiags.Error, + "This is not a deferral-related diagnostic either", + "Leave this one alone too.", + )) + want = want.ForRPC() + + if diff := cmp.Diff(want, got); diff != "" { + t.Error("wrong result\n" + diff) + } + +} diff --git a/internal/tofu/node_resource_import.go b/internal/tofu/node_resource_import.go index 8d47e4d845..3b97b213ac 100644 --- a/internal/tofu/node_resource_import.go +++ b/internal/tofu/node_resource_import.go @@ -121,7 +121,7 @@ func (n *graphNodeImportState) Execute(ctx context.Context, evalCtx EvalContext, TypeName: n.Addr.Resource.Resource.Type, ID: n.ID, }) - diags = diags.Append(resp.Diagnostics) + diags = diags.Append(maybeImproveResourceInstanceDiagnostics(resp.Diagnostics, n.Addr)) if diags.HasErrors() { return diags } diff --git a/internal/tofu/node_resource_plan_instance.go b/internal/tofu/node_resource_plan_instance.go index ca23ff4009..6175727c2b 100644 --- a/internal/tofu/node_resource_plan_instance.go +++ b/internal/tofu/node_resource_plan_instance.go @@ -283,7 +283,7 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx context.Conte // The import process handles its own refresh if !n.skipRefresh && !importing { s, refreshDiags := n.refresh(ctx, evalCtx, states.NotDeposed, instanceRefreshState) - diags = diags.Append(refreshDiags) + diags = diags.Append(maybeImproveResourceInstanceDiagnostics(refreshDiags, addr)) if diags.HasErrors() { return diags } @@ -327,7 +327,7 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx context.Conte change, instancePlanState, repeatData, planDiags := n.plan( ctx, evalCtx, nil, instanceRefreshState, n.ForceCreateBeforeDestroy, n.forceReplace, ) - diags = diags.Append(planDiags) + diags = diags.Append(maybeImproveResourceInstanceDiagnostics(planDiags, addr)) if diags.HasErrors() { // If we are importing and generating a configuration, we need to // ensure the change is written out so the configuration can be diff --git a/internal/tofu/upgrade_resource_state.go b/internal/tofu/upgrade_resource_state.go index 9b18731113..3c7c002e33 100644 --- a/internal/tofu/upgrade_resource_state.go +++ b/internal/tofu/upgrade_resource_state.go @@ -111,7 +111,7 @@ func upgradeResourceStateTransform(args stateTransformArgs) (cty.Value, []byte, } resp := args.provider.UpgradeResourceState(context.TODO(), req) - diags := resp.Diagnostics + diags := maybeImproveResourceInstanceDiagnostics(resp.Diagnostics, args.currentAddr) if diags.HasErrors() { log.Printf("[TRACE] upgradeResourceStateTransform: failed - address: %s", args.currentAddr) return cty.NilVal, nil, diags