diff --git a/internal/plugin/convert/deferral.go b/internal/plugin/convert/deferral.go deleted file mode 100644 index 7eeab22b6d..0000000000 --- a/internal/plugin/convert/deferral.go +++ /dev/null @@ -1,24 +0,0 @@ -// 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 f3ca086a23..263ebc7939 100644 --- a/internal/plugin/grpc_provider.go +++ b/internal/plugin/grpc_provider.go @@ -34,14 +34,6 @@ type GRPCProviderPlugin struct { } 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, // WriteOnlyAttributesAllowed indicates that the current system version // supports write-only attributes. // This enables the SDK to run specific validations and enable the @@ -502,11 +494,6 @@ 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 { @@ -589,11 +576,6 @@ 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 { @@ -716,11 +698,6 @@ 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{ @@ -839,11 +816,6 @@ 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 { @@ -902,9 +874,6 @@ func (p *GRPCProvider) OpenEphemeralResource(ctx context.Context, r providers.Op renewAt := protoResp.RenewAt.AsTime() resp.RenewAt = &renewAt } - if protoDeferred := protoResp.Deferred; protoDeferred != nil { - resp.Deferred = &providers.EphemeralResourceDeferred{DeferralReason: convert.DeferralReasonFromProto(protoDeferred.Reason)} - } return resp } diff --git a/internal/plugin/grpc_provider_test.go b/internal/plugin/grpc_provider_test.go index eb47ec6656..513eac872a 100644 --- a/internal/plugin/grpc_provider_test.go +++ b/internal/plugin/grpc_provider_test.go @@ -14,7 +14,6 @@ import ( "testing" "time" - "github.com/davecgh/go-spew/spew" "github.com/google/go-cmp/cmp" "github.com/zclconf/go-cty/cty" "github.com/zclconf/go-cty/cty/msgpack" @@ -826,52 +825,6 @@ 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{ @@ -1301,9 +1254,6 @@ func TestGRPCProvider_OpenEphemeralResource(t *testing.T) { }, Private: []byte("private data"), RenewAt: timestamppb.New(future), - Deferred: &proto.Deferred{ - Reason: proto.Deferred_RESOURCE_CONFIG_UNKNOWN, - }, }, nil) resp := p.OpenEphemeralResource(t.Context(), providers.OpenEphemeralResourceRequest{ @@ -1327,14 +1277,6 @@ func TestGRPCProvider_OpenEphemeralResource(t *testing.T) { if got, want := resp.Private, []byte("private data"); !slices.Equal(got, want) { t.Fatalf("unexpected private data. got: %q, want %q", got, want) } - { - if resp.Deferred == nil { - t.Fatal("expected to have a deferred object but got none") - } - if got, want := resp.Deferred.DeferralReason, providers.DeferredBecauseResourceConfigUnknown; got != want { - t.Fatalf("unexpected deferred reason. got: %d, want %d", got, want) - } - } }) t.Run("requested type is not in schema", func(t *testing.T) { client := mockProviderClient(t) diff --git a/internal/plugin6/convert/deferral.go b/internal/plugin6/convert/deferral.go deleted file mode 100644 index d47420a394..0000000000 --- a/internal/plugin6/convert/deferral.go +++ /dev/null @@ -1,24 +0,0 @@ -// 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 8477df79ed..aadf96710c 100644 --- a/internal/plugin6/grpc_provider.go +++ b/internal/plugin6/grpc_provider.go @@ -491,11 +491,6 @@ 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 { @@ -578,11 +573,6 @@ 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 { @@ -705,11 +695,6 @@ 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{ @@ -828,11 +813,6 @@ 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 { @@ -891,9 +871,6 @@ func (p *GRPCProvider) OpenEphemeralResource(ctx context.Context, r providers.Op renewAt := protoResp.RenewAt.AsTime() resp.RenewAt = &renewAt } - if protoDeferred := protoResp.Deferred; protoDeferred != nil { - resp.Deferred = &providers.EphemeralResourceDeferred{DeferralReason: convert.DeferralReasonFromProto(protoDeferred.Reason)} - } return resp } diff --git a/internal/plugin6/grpc_provider_test.go b/internal/plugin6/grpc_provider_test.go index e5e4ef8427..2fdc411def 100644 --- a/internal/plugin6/grpc_provider_test.go +++ b/internal/plugin6/grpc_provider_test.go @@ -14,7 +14,6 @@ import ( "testing" "time" - "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" @@ -833,52 +832,6 @@ 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{ @@ -1308,9 +1261,6 @@ func TestGRPCProvider_OpenEphemeralResource(t *testing.T) { }, Private: []byte("private data"), RenewAt: timestamppb.New(future), - Deferred: &proto.Deferred{ - Reason: proto.Deferred_RESOURCE_CONFIG_UNKNOWN, - }, }, nil) resp := p.OpenEphemeralResource(t.Context(), providers.OpenEphemeralResourceRequest{ @@ -1334,14 +1284,6 @@ func TestGRPCProvider_OpenEphemeralResource(t *testing.T) { if got, want := resp.Private, []byte("private data"); !slices.Equal(got, want) { t.Fatalf("unexpected private data. got: %q, want %q", got, want) } - { - if resp.Deferred == nil { - t.Fatal("expected to have a deferred object but got none") - } - if got, want := resp.Deferred.DeferralReason, providers.DeferredBecauseResourceConfigUnknown; got != want { - t.Fatalf("unexpected deferred reason. got: %d, want %d", got, want) - } - } }) t.Run("requested type is not in schema", func(t *testing.T) { client := mockProviderClient(t) diff --git a/internal/providers/deferral.go b/internal/providers/deferral.go deleted file mode 100644 index cf0190400d..0000000000 --- a/internal/providers/deferral.go +++ /dev/null @@ -1,141 +0,0 @@ -// 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 tool 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 { - summary := DeferralReasonSummary(reason) - - var detail string - switch reason { - case DeferredBecauseResourceConfigUnknown: - 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: - 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.) - 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} - }) -} - -// DeferralReasonSummary returns a more informative string representation of the given DeferralReason to be used -// it other places too. -// For more details, check the comments from NewDeferralDiagnostic. -func DeferralReasonSummary(reason DeferralReason) string { - switch reason { - case DeferredBecauseResourceConfigUnknown: - return "Resource configuration is incomplete" - case DeferredBecauseProviderConfigUnknown: - return "Provider configuration is incomplete" - default: - return "Operation cannot be completed yet" - } -} - -// 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 deleted file mode 100644 index ad9a9efedb..0000000000 --- a/internal/providers/deferralreason_string.go +++ /dev/null @@ -1,27 +0,0 @@ -// 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 { - idx := int(i) - 0 - if i < 0 || idx >= len(_DeferralReason_index)-1 { - return "DeferralReason(" + strconv.FormatInt(int64(i), 10) + ")" - } - return _DeferralReason_name[_DeferralReason_index[idx]:_DeferralReason_index[idx+1]] -} diff --git a/internal/providers/provider.go b/internal/providers/provider.go index d8a472fca0..038ef48570 100644 --- a/internal/providers/provider.go +++ b/internal/providers/provider.go @@ -608,8 +608,6 @@ type OpenEphemeralResourceResponse struct { Result cty.Value // Private is the provider information that needs to be used later on Renew/Close call. Private []byte - // Deferred returns only a reason of why the provider is asking deferring the opening. - Deferred *EphemeralResourceDeferred // RenewAt indicates if(!=nil) and when(<=time.Now()) the Renew call needs to be performed. RenewAt *time.Time @@ -617,10 +615,6 @@ type OpenEphemeralResourceResponse struct { Diagnostics tfdiags.Diagnostics } -type EphemeralResourceDeferred struct { - DeferralReason DeferralReason -} - type RenewEphemeralResourceRequest struct { // TypeName is the name of the ephemeral resource to Renew. TypeName string diff --git a/internal/tofu/context_plan2_test.go b/internal/tofu/context_plan2_test.go index ed23245694..54ee8b117d 100644 --- a/internal/tofu/context_plan2_test.go +++ b/internal/tofu/context_plan2_test.go @@ -5064,97 +5064,6 @@ 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": ` @@ -8584,111 +8493,6 @@ func TestContext2Plan_removedModuleButModuleBlockStillExists(t *testing.T) { } } -// TestContext2Plan_ephemeralResourceDeferred is testing that an ephemeral resource gets deferred -// correctly: -// * gets deferred when a dependency is having planned changes, so OpenEphemeralResource is not called. -// * gets deferred when the response from OpenEphemeralResource is indicating so. -func TestContext2Plan_ephemeralResourceDeferred(t *testing.T) { - // Ephemeral resource is deferred by opentofu itself, before calling OpenEphemeralResource. This is - // due to pending changes in the ephemeral's dependencies. - t.Run("before open", func(t *testing.T) { - m := testModuleInline(t, map[string]string{ - "main.tf": ` - resource "test_object" "testres" { - } - ephemeral "test_object" "testeph" { - depends_on = [ - test_object.testres - ] - } - `, - }) - - state := states.BuildState(func(s *states.SyncState) {}) - - p := simpleMockProvider() - ctx := testContext2(t, &ContextOpts{ - Providers: map[addrs.Provider]providers.Factory{ - addrs.NewDefaultProvider("test"): testProviderFuncFixed(p), - }, - }) - hook := &testHook{} - ctx.hooks = append(ctx.hooks, hook) - - _, diags := ctx.Plan(context.Background(), m, state, &PlanOpts{ - Mode: plans.NormalMode, - }) - - if diags.HasErrors() { - t.Fatalf("unexpected errors: %s", diags.Err()) - } - - // last call should have been on the ephemeral defer - deferCall := hook.Calls[len(hook.Calls)-1] - if wantAction, wantInstID := "Deferred", "ephemeral_test_object.testeph"; deferCall.Action != wantAction && deferCall.InstanceID != wantInstID { - t.Fatalf("expected the last call to be a %q for %q. got action %q for %q", wantAction, wantInstID, deferCall.Action, deferCall.InstanceID) - } - }) - // Ephemeral is deferred because of the defer reason returned from OpenEphemeralResource. - t.Run("from open", func(t *testing.T) { - m := testModuleInline(t, map[string]string{ - "main.tf": ` - resource "test_object" "testres" { - test_string = "test value" - } - ephemeral "test_object" "testeph" { - depends_on = [ - test_object.testres - ] - } - `, - }) - - addr := mustAbsResourceAddr("test_object.testres") - state := states.BuildState(func(s *states.SyncState) { - s.SetResourceInstanceCurrent(addr.Instance(addrs.NoKey), &states.ResourceInstanceObjectSrc{ - AttrsJSON: []byte(`{"test_string": "test value"}`), - Status: states.ObjectReady, - }, mustProviderConfig(`provider["registry.opentofu.org/hashicorp/test"]`), addrs.NoKey) - }) - - p := simpleMockProvider() - p.OpenEphemeralResourceResponse = &providers.OpenEphemeralResourceResponse{ - Result: cty.Value{}, - Deferred: &providers.EphemeralResourceDeferred{DeferralReason: providers.DeferredBecauseResourceConfigUnknown}, - } - p.PlanResourceChangeFn = func(req providers.PlanResourceChangeRequest) (resp providers.PlanResourceChangeResponse) { - cfg := req.Config.AsValueMap() - resp.PlannedState = cty.ObjectVal(cfg) - return resp - } - ctx := testContext2(t, &ContextOpts{ - Providers: map[addrs.Provider]providers.Factory{ - addrs.NewDefaultProvider("test"): testProviderFuncFixed(p), - }, - }) - hook := &testHook{} - ctx.hooks = append(ctx.hooks, hook) - - _, diags := ctx.Plan(context.Background(), m, state, &PlanOpts{ - Mode: plans.NormalMode, - }) - - if diags.HasErrors() { - t.Fatalf("unexpected errors: %s", diags.Err()) - } - - if !p.OpenEphemeralResourceCalled { - t.Fatal("expected OpenEphemeralResource to be called but it was not") - } - // last call should have been on the ephemeral defer - deferCall := hook.Calls[len(hook.Calls)-1] - if wantAction, wantInstID := "Deferred", "ephemeral_test_object.testeph"; deferCall.Action != wantAction && deferCall.InstanceID != wantInstID { - t.Fatalf("expected the last call to be a %q for %q. got action %q for %q", wantAction, wantInstID, deferCall.Action, deferCall.InstanceID) - } - }) -} - func TestContext2Plan_importResourceWithSensitiveDataSource(t *testing.T) { addr := mustResourceInstanceAddr("test_object.b") m := testModuleInline(t, map[string]string{ diff --git a/internal/tofu/node_resource_abstract_instance.go b/internal/tofu/node_resource_abstract_instance.go index a7acd83fd9..e721bc0abd 100644 --- a/internal/tofu/node_resource_abstract_instance.go +++ b/internal/tofu/node_resource_abstract_instance.go @@ -2054,7 +2054,7 @@ func (n *NodeAbstractResourceInstance) providerMetas(ctx context.Context, evalCt return metaConfigVal, diags } -func (n *NodeAbstractResourceInstance) openEphemeralResource(ctx context.Context, evalCtx EvalContext, configVal cty.Value) (cty.Value, providers.DeferralReason, tfdiags.Diagnostics) { +func (n *NodeAbstractResourceInstance) openEphemeralResource(ctx context.Context, evalCtx EvalContext, configVal cty.Value) (cty.Value, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics var newVal cty.Value @@ -2063,13 +2063,13 @@ func (n *NodeAbstractResourceInstance) openEphemeralResource(ctx context.Context provider, providerSchema, err := n.getProvider(ctx, evalCtx) diags = diags.Append(err) if diags.HasErrors() { - return newVal, providers.DeferredReasonUnknown, diags + return newVal, diags } schema, _ := providerSchema.SchemaForResourceAddr(n.Addr.ContainingResource().Resource) if schema == nil { // Should be caught during validation, so we don't bother with a pretty error here diags = diags.Append(fmt.Errorf("provider %q does not support ephemeral resource %q", n.ResolvedProvider.ProviderConfig, n.Addr.ContainingResource().Resource.Type)) - return newVal, providers.DeferredReasonUnknown, diags + return newVal, diags } // Unmark before sending to provider, will re-mark before returning @@ -2086,7 +2086,7 @@ func (n *NodeAbstractResourceInstance) openEphemeralResource(ctx context.Context ) diags = diags.Append(validateResp.Diagnostics.InConfigBody(config.Config, n.Addr.String())) if diags.HasErrors() { - return newVal, providers.DeferredReasonUnknown, diags + return newVal, diags } // If we get down here then our configuration is complete and we're ready @@ -2097,7 +2097,7 @@ func (n *NodeAbstractResourceInstance) openEphemeralResource(ctx context.Context return h.PreOpen(n.Addr) })) if diags.HasErrors() { - return newVal, providers.DeferredReasonUnknown, diags + return newVal, diags } req := providers.OpenEphemeralResourceRequest{ @@ -2107,12 +2107,9 @@ func (n *NodeAbstractResourceInstance) openEphemeralResource(ctx context.Context resp := provider.OpenEphemeralResource(ctx, req) diags = diags.Append(resp.Diagnostics.InConfigBody(config.Config, n.Addr.String())) if diags.HasErrors() { - return newVal, providers.DeferredReasonUnknown, diags + return newVal, diags } - if resp.Deferred != nil { - return newVal, resp.Deferred.DeferralReason, diags - } newVal = resp.Result for _, err := range newVal.Type().TestConformance(schema.ImpliedType()) { @@ -2126,7 +2123,7 @@ func (n *NodeAbstractResourceInstance) openEphemeralResource(ctx context.Context )) } if diags.HasErrors() { - return newVal, providers.DeferredReasonUnknown, diags + return newVal, diags } if newVal.IsNull() { @@ -2138,7 +2135,7 @@ func (n *NodeAbstractResourceInstance) openEphemeralResource(ctx context.Context n.ResolvedProvider.ProviderConfig.InstanceString(n.ResolvedProviderKey), n.Addr, ), )) - return newVal, providers.DeferredReasonUnknown, diags + return newVal, diags } if !newVal.IsNull() && !newVal.IsWhollyKnown() { @@ -2150,7 +2147,7 @@ func (n *NodeAbstractResourceInstance) openEphemeralResource(ctx context.Context n.ResolvedProvider.ProviderConfig.InstanceString(n.ResolvedProviderKey), n.Addr, ), )) - return newVal, providers.DeferredReasonUnknown, diags + return newVal, diags } if len(pvm) > 0 { @@ -2175,7 +2172,7 @@ func (n *NodeAbstractResourceInstance) openEphemeralResource(ctx context.Context // when calling provider.CloseEphemeralResource. go n.startEphemeralRenew(ctx, evalCtx, provider, resp.RenewAt, resp.Private) - return newVal, providers.DeferredReasonUnknown, diags + return newVal, diags } // planDataSource deals with the main part of the data resource lifecycle: @@ -3257,63 +3254,6 @@ func (n *NodeAbstractResourceInstance) getProvider(ctx context.Context, evalCtx return underlyingProvider, schema, err } -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 -} - func (n *NodeAbstractResourceInstance) applyEphemeralResource(ctx context.Context, evalCtx EvalContext) (*states.ResourceInstanceObject, instances.RepetitionData, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics var keyData instances.RepetitionData @@ -3358,17 +3298,7 @@ func (n *NodeAbstractResourceInstance) applyEphemeralResource(ctx context.Contex // We have a complete configuration with no dependencies to wait on, so we // can open the ephemeral resource and store its value in the state. - newVal, deferralReason, readDiags := n.openEphemeralResource(ctx, evalCtx, configVal) - if deferralReason != providers.DeferredReasonUnknown { - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Ephemeral resource deferred during apply", - Detail: fmt.Sprintf("Ephemeral resource %q asked for being deferred. This is a provider error.", n.Addr.String()), - Subject: n.Config.TypeRange.Ptr(), - Context: n.Config.DeclRange.Ptr(), - }) - return nil, instances.RepetitionData{}, diags - } + newVal, readDiags := n.openEphemeralResource(ctx, evalCtx, configVal) diags = diags.Append(readDiags) if diags.HasErrors() { return nil, keyData, diags @@ -3462,14 +3392,7 @@ func (n *NodeAbstractResourceInstance) planEphemeralResource(ctx context.Context // We have a complete configuration with no dependencies to wait on, so we // can open the ephemeral resource and store its value in the state. - newVal, deferralReason, readDiags := n.openEphemeralResource(ctx, evalCtx, configVal) - if deferralReason != providers.DeferredReasonUnknown { - reason := providers.DeferralReasonSummary(deferralReason) - - plannedChange, plannedNewState, deferDiags := n.deferEphemeralResource(evalCtx, schema, priorVal, configVal, reason) - diags = diags.Append(deferDiags) - return plannedChange, plannedNewState, keyData, diags - } + newVal, readDiags := n.openEphemeralResource(ctx, evalCtx, configVal) diags = diags.Append(readDiags) if diags.HasErrors() { return nil, nil, instances.RepetitionData{}, diags diff --git a/internal/tofu/node_resource_abstract_instance_test.go b/internal/tofu/node_resource_abstract_instance_test.go index 57aa19861f..ed7b12be7a 100644 --- a/internal/tofu/node_resource_abstract_instance_test.go +++ b/internal/tofu/node_resource_abstract_instance_test.go @@ -15,9 +15,7 @@ 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) { @@ -268,72 +266,3 @@ 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 244ad18e3b..2c7fbca6eb 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(maybeImproveResourceInstanceDiagnostics(resp.Diagnostics, n.Addr)) + diags = diags.Append(resp.Diagnostics) if diags.HasErrors() { return diags } diff --git a/internal/tofu/node_resource_plan_instance.go b/internal/tofu/node_resource_plan_instance.go index be2115d0a3..bbc10e22fd 100644 --- a/internal/tofu/node_resource_plan_instance.go +++ b/internal/tofu/node_resource_plan_instance.go @@ -351,7 +351,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(maybeImproveResourceInstanceDiagnostics(refreshDiags, addr)) + diags = diags.Append(refreshDiags) if diags.HasErrors() { return diags } @@ -389,7 +389,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(maybeImproveResourceInstanceDiagnostics(planDiags, addr)) + diags = diags.Append(planDiags) 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 3c7c002e33..9b18731113 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 := maybeImproveResourceInstanceDiagnostics(resp.Diagnostics, args.currentAddr) + diags := resp.Diagnostics if diags.HasErrors() { log.Printf("[TRACE] upgradeResourceStateTransform: failed - address: %s", args.currentAddr) return cty.NilVal, nil, diags