Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

all: Add automatic deferred action support for unknown provider configuration #1002

Merged
merged 51 commits into from
May 30, 2024
Merged
Show file tree
Hide file tree
Changes from 45 commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
df3f0cf
Implement manual deferred action support for `resource.importResource…
SBGoods May 1, 2024
0a27076
Implement manual deferred action support for `resource.readResource`
SBGoods May 3, 2024
bbf6c8d
Implement manual deferred action support for `resource.modifyPlan`
SBGoods May 7, 2024
29cfc74
Rename `DeferralReason` and `DeferralResponse` to `DeferredReason` an…
SBGoods May 7, 2024
67ff590
Update `terraform-plugin-go` dependency to `v0.23.0`
SBGoods May 7, 2024
4a11408
Rename `deferral.go` to `deferred.go`
SBGoods May 7, 2024
ce1d14b
Implement manual deferred action support for data sources
SBGoods May 7, 2024
39c4bb5
Update documentation and diagnostic messages
SBGoods May 7, 2024
e6a3700
Merge branch 'main' into SBGoods/deferred-action-support
SBGoods May 7, 2024
3a5a8c3
Add copyright headers
SBGoods May 7, 2024
04042c2
Add changelog entries
SBGoods May 8, 2024
ea64005
Apply suggestions from code review
SBGoods May 10, 2024
ac952fc
Add comment and changelog notes to indicate experimental nature of de…
SBGoods May 10, 2024
bcadaee
Rename constant to be specific to data sources
SBGoods May 13, 2024
3777d5d
Remove TODO comment
SBGoods May 13, 2024
0c48915
Remove unnecessary nil check
SBGoods May 13, 2024
95ef72e
Add default values for `ClientCapabilities` request fields
SBGoods May 13, 2024
c5156c5
Rename `DeferredResponse` to `Deferred`
SBGoods May 13, 2024
96166fb
Remove error handling for deferral response without deferral capability
SBGoods May 13, 2024
0a6b8a5
Remove variable indirection in tests
SBGoods May 13, 2024
f1112d5
Add copyright headers
SBGoods May 13, 2024
2433dca
Apply suggestions from code review
SBGoods May 14, 2024
50aaca9
Add unit tests for client capabilities
SBGoods May 14, 2024
f5d659d
Merge branch 'main' into SBGoods/deferred-action-support
SBGoods May 14, 2024
a705a12
Implement deferred action support for `provider.configureProvider`
SBGoods May 14, 2024
5ab0c24
Move client capabilities defaulting behavior to `fromproto5/6` package
SBGoods May 14, 2024
038d486
Move `toproto5/6` `Deferred` conversion handling to its own files
SBGoods May 14, 2024
8f0b6aa
Use `ResourceDeferred()` and `DataSourceDeferred()` functions in `top…
SBGoods May 15, 2024
720016f
Merge branch 'refs/heads/SBGoods/deferred-action-support' into SBGood…
SBGoods May 15, 2024
b92fc38
move configure provider client capabilities unset test to `fromproto5/6`
SBGoods May 15, 2024
c89dcdc
Add throw an error diagnostic in server_configureprovider.go if a def…
SBGoods May 15, 2024
188522e
Add `ResourceBehavior` field to `MetadataResponse`
SBGoods May 16, 2024
693e708
Initial implementation of automatic deferrals for resource/datasource…
SBGoods May 16, 2024
b29ca83
Merge branch 'refs/heads/main' into SBGoods/automatic-deferred-action…
SBGoods May 16, 2024
a5089d6
Implement provider automatic deferral for `PlanResourceChange` RPC
SBGoods May 17, 2024
9fd5971
Add default values for automatic deferrals
SBGoods May 20, 2024
a1e8983
Update resource/metadata.go
SBGoods May 20, 2024
299ac68
Add experimental note
SBGoods May 20, 2024
bc60806
Merge remote-tracking branch 'origin/SBGoods/automatic-deferred-actio…
SBGoods May 20, 2024
4669aaa
Implement resource behavior in `proto6server`
SBGoods May 20, 2024
fe78dbb
Add changelog entries
SBGoods May 20, 2024
1cb1aa4
Merge branch 'main' into SBGoods/automatic-deferred-action-support
SBGoods May 20, 2024
277ce82
Apply suggestions from code review
SBGoods May 21, 2024
8593412
Log deferred reason in debug logging
SBGoods May 21, 2024
904a87f
Add error diagnostics to automatic deferral tests
SBGoods May 21, 2024
31f33f8
Refactor `PlanResourceChange` automatic deferred action implementatio…
SBGoods May 23, 2024
a05621a
Add a comment calling out intentional design of replacing configured …
SBGoods May 23, 2024
a3dbd79
Return early in `PlanResourceChange` if `ProviderDeferredBehavior.Ena…
SBGoods May 29, 2024
b3f4d24
Update internal/fwserver/server_configureprovider.go
SBGoods May 30, 2024
a7d7292
Add separate unit test for overriding provider deferred reason with r…
SBGoods May 30, 2024
7a578b1
Merge branch 'main' into SBGoods/automatic-deferred-action-support
SBGoods May 30, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changes/unreleased/FEATURES-20240520-180458.yaml
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to terraform-plugin-sdk, we should include a changelog note talking about deferral being experimental 👍

https://github.com/hashicorp/terraform-plugin-sdk/releases/tag/v2.34.0 for inspiration

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we are planning on releasing this with the manual deferred action work, this should be covered by: https://github.com/hashicorp/terraform-plugin-framework/blob/main/.changes/unreleased/NOTES-20240510-143136.yaml

Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: FEATURES
body: 'provider: Add `Deferred` field to `ConfigureResponse`
which indicates a provider deferred action to the Terraform client'
time: 2024-05-20T18:04:58.852448-04:00
custom:
Issue: "1002"
6 changes: 6 additions & 0 deletions .changes/unreleased/FEATURES-20240520-180735.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: FEATURES
body: 'provider: Add `ClientCapabilities` field to `ConfigureRequest` which specifies
optionally supported protocol features for the Terraform client'
time: 2024-05-20T18:07:35.862641-04:00
custom:
Issue: "1002"
14 changes: 14 additions & 0 deletions internal/fromproto5/client_capabilities.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,23 @@ import (
"github.com/hashicorp/terraform-plugin-go/tfprotov5"

"github.com/hashicorp/terraform-plugin-framework/datasource"
"github.com/hashicorp/terraform-plugin-framework/provider"
"github.com/hashicorp/terraform-plugin-framework/resource"
)

func ConfigureProviderClientCapabilities(in *tfprotov5.ConfigureProviderClientCapabilities) provider.ConfigureProviderClientCapabilities {
if in == nil {
// Client did not indicate any supported capabilities
return provider.ConfigureProviderClientCapabilities{
DeferralAllowed: false,
}
}

return provider.ConfigureProviderClientCapabilities{
DeferralAllowed: in.DeferralAllowed,
}
}

func ReadDataSourceClientCapabilities(in *tfprotov5.ReadDataSourceClientCapabilities) datasource.ReadClientCapabilities {
if in == nil {
// Client did not indicate any supported capabilities
Expand Down
6 changes: 4 additions & 2 deletions internal/fromproto5/configureprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@ package fromproto5
import (
"context"

"github.com/hashicorp/terraform-plugin-go/tfprotov5"

"github.com/hashicorp/terraform-plugin-framework/diag"
"github.com/hashicorp/terraform-plugin-framework/internal/fwschema"
"github.com/hashicorp/terraform-plugin-framework/provider"
"github.com/hashicorp/terraform-plugin-go/tfprotov5"
)

// ConfigureProviderRequest returns the *fwserver.ConfigureProviderRequest
Expand All @@ -20,7 +21,8 @@ func ConfigureProviderRequest(ctx context.Context, proto5 *tfprotov5.ConfigurePr
}

fw := &provider.ConfigureRequest{
TerraformVersion: proto5.TerraformVersion,
TerraformVersion: proto5.TerraformVersion,
ClientCapabilities: ConfigureProviderClientCapabilities(proto5.ClientCapabilities),
}

config, diags := Config(ctx, proto5.Config, providerSchema)
Expand Down
25 changes: 23 additions & 2 deletions internal/fromproto5/configureprovider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,15 @@ import (
"testing"

"github.com/google/go-cmp/cmp"
"github.com/hashicorp/terraform-plugin-go/tfprotov5"
"github.com/hashicorp/terraform-plugin-go/tftypes"

"github.com/hashicorp/terraform-plugin-framework/diag"
"github.com/hashicorp/terraform-plugin-framework/internal/fromproto5"
"github.com/hashicorp/terraform-plugin-framework/internal/fwschema"
"github.com/hashicorp/terraform-plugin-framework/provider"
"github.com/hashicorp/terraform-plugin-framework/provider/schema"
"github.com/hashicorp/terraform-plugin-framework/tfsdk"
"github.com/hashicorp/terraform-plugin-go/tfprotov5"
"github.com/hashicorp/terraform-plugin-go/tftypes"
)

func TestConfigureProviderRequest(t *testing.T) {
Expand Down Expand Up @@ -94,6 +95,26 @@ func TestConfigureProviderRequest(t *testing.T) {
TerraformVersion: "99.99.99",
},
},
"client-capabilities": {
input: &tfprotov5.ConfigureProviderRequest{
ClientCapabilities: &tfprotov5.ConfigureProviderClientCapabilities{
DeferralAllowed: true,
},
},
expected: &provider.ConfigureRequest{
ClientCapabilities: provider.ConfigureProviderClientCapabilities{
DeferralAllowed: true,
},
},
},
"client-capabilities-unset": {
input: &tfprotov5.ConfigureProviderRequest{},
expected: &provider.ConfigureRequest{
ClientCapabilities: provider.ConfigureProviderClientCapabilities{
DeferralAllowed: false,
},
},
},
}

for name, testCase := range testCases {
Expand Down
3 changes: 2 additions & 1 deletion internal/fromproto5/planresourcechange.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (

// PlanResourceChangeRequest returns the *fwserver.PlanResourceChangeRequest
// equivalent of a *tfprotov5.PlanResourceChangeRequest.
func PlanResourceChangeRequest(ctx context.Context, proto5 *tfprotov5.PlanResourceChangeRequest, reqResource resource.Resource, resourceSchema fwschema.Schema, providerMetaSchema fwschema.Schema) (*fwserver.PlanResourceChangeRequest, diag.Diagnostics) {
func PlanResourceChangeRequest(ctx context.Context, proto5 *tfprotov5.PlanResourceChangeRequest, reqResource resource.Resource, resourceSchema fwschema.Schema, providerMetaSchema fwschema.Schema, resourceBehavior resource.ResourceBehavior) (*fwserver.PlanResourceChangeRequest, diag.Diagnostics) {
if proto5 == nil {
return nil, nil
}
Expand All @@ -39,6 +39,7 @@ func PlanResourceChangeRequest(ctx context.Context, proto5 *tfprotov5.PlanResour
}

fw := &fwserver.PlanResourceChangeRequest{
ResourceBehavior: resourceBehavior,
ResourceSchema: resourceSchema,
Resource: reqResource,
ClientCapabilities: ModifyPlanClientCapabilities(proto5.ClientCapabilities),
Expand Down
20 changes: 19 additions & 1 deletion internal/fromproto5/planresourcechange_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ func TestPlanResourceChangeRequest(t *testing.T) {

testCases := map[string]struct {
input *tfprotov5.PlanResourceChangeRequest
resourceBehavior resource.ResourceBehavior
resourceSchema fwschema.Schema
resource resource.Resource
providerMetaSchema fwschema.Schema
Expand Down Expand Up @@ -241,6 +242,23 @@ func TestPlanResourceChangeRequest(t *testing.T) {
ResourceSchema: testFwSchema,
},
},
"resource-behavior": {
input: &tfprotov5.PlanResourceChangeRequest{},
resourceSchema: testFwSchema,
resourceBehavior: resource.ResourceBehavior{
ProviderDeferred: resource.ProviderDeferredBehavior{
EnablePlanModification: true,
},
},
expected: &fwserver.PlanResourceChangeRequest{
ResourceBehavior: resource.ResourceBehavior{
ProviderDeferred: resource.ProviderDeferredBehavior{
EnablePlanModification: true,
},
},
ResourceSchema: testFwSchema,
},
},
}

for name, testCase := range testCases {
Expand All @@ -249,7 +267,7 @@ func TestPlanResourceChangeRequest(t *testing.T) {
t.Run(name, func(t *testing.T) {
t.Parallel()

got, diags := fromproto5.PlanResourceChangeRequest(context.Background(), testCase.input, testCase.resource, testCase.resourceSchema, testCase.providerMetaSchema)
got, diags := fromproto5.PlanResourceChangeRequest(context.Background(), testCase.input, testCase.resource, testCase.resourceSchema, testCase.providerMetaSchema, testCase.resourceBehavior)

if diff := cmp.Diff(got, testCase.expected, cmp.AllowUnexported(privatestate.ProviderData{})); diff != "" {
t.Errorf("unexpected difference: %s", diff)
Expand Down
14 changes: 14 additions & 0 deletions internal/fromproto6/client_capabilities.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,23 @@ import (
"github.com/hashicorp/terraform-plugin-go/tfprotov6"

"github.com/hashicorp/terraform-plugin-framework/datasource"
"github.com/hashicorp/terraform-plugin-framework/provider"
"github.com/hashicorp/terraform-plugin-framework/resource"
)

func ConfigureProviderClientCapabilities(in *tfprotov6.ConfigureProviderClientCapabilities) provider.ConfigureProviderClientCapabilities {
if in == nil {
// Client did not indicate any supported capabilities
return provider.ConfigureProviderClientCapabilities{
DeferralAllowed: false,
}
}

return provider.ConfigureProviderClientCapabilities{
DeferralAllowed: in.DeferralAllowed,
}
}

func ReadDataSourceClientCapabilities(in *tfprotov6.ReadDataSourceClientCapabilities) datasource.ReadClientCapabilities {
if in == nil {
// Client did not indicate any supported capabilities
Expand Down
6 changes: 4 additions & 2 deletions internal/fromproto6/configureprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@ package fromproto6
import (
"context"

"github.com/hashicorp/terraform-plugin-go/tfprotov6"

"github.com/hashicorp/terraform-plugin-framework/diag"
"github.com/hashicorp/terraform-plugin-framework/internal/fwschema"
"github.com/hashicorp/terraform-plugin-framework/provider"
"github.com/hashicorp/terraform-plugin-go/tfprotov6"
)

// ConfigureProviderRequest returns the *fwserver.ConfigureProviderRequest
Expand All @@ -20,7 +21,8 @@ func ConfigureProviderRequest(ctx context.Context, proto6 *tfprotov6.ConfigurePr
}

fw := &provider.ConfigureRequest{
TerraformVersion: proto6.TerraformVersion,
TerraformVersion: proto6.TerraformVersion,
ClientCapabilities: ConfigureProviderClientCapabilities(proto6.ClientCapabilities),
}

config, diags := Config(ctx, proto6.Config, providerSchema)
Expand Down
25 changes: 23 additions & 2 deletions internal/fromproto6/configureprovider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,15 @@ import (
"testing"

"github.com/google/go-cmp/cmp"
"github.com/hashicorp/terraform-plugin-go/tfprotov6"
"github.com/hashicorp/terraform-plugin-go/tftypes"

"github.com/hashicorp/terraform-plugin-framework/diag"
"github.com/hashicorp/terraform-plugin-framework/internal/fromproto6"
"github.com/hashicorp/terraform-plugin-framework/internal/fwschema"
"github.com/hashicorp/terraform-plugin-framework/provider"
"github.com/hashicorp/terraform-plugin-framework/provider/schema"
"github.com/hashicorp/terraform-plugin-framework/tfsdk"
"github.com/hashicorp/terraform-plugin-go/tfprotov6"
"github.com/hashicorp/terraform-plugin-go/tftypes"
)

func TestConfigureProviderRequest(t *testing.T) {
Expand Down Expand Up @@ -94,6 +95,26 @@ func TestConfigureProviderRequest(t *testing.T) {
TerraformVersion: "99.99.99",
},
},
"client-capabilities": {
input: &tfprotov6.ConfigureProviderRequest{
ClientCapabilities: &tfprotov6.ConfigureProviderClientCapabilities{
DeferralAllowed: true,
},
},
expected: &provider.ConfigureRequest{
ClientCapabilities: provider.ConfigureProviderClientCapabilities{
DeferralAllowed: true,
},
},
},
"client-capabilities-unset": {
input: &tfprotov6.ConfigureProviderRequest{},
expected: &provider.ConfigureRequest{
ClientCapabilities: provider.ConfigureProviderClientCapabilities{
DeferralAllowed: false,
},
},
},
}

for name, testCase := range testCases {
Expand Down
3 changes: 2 additions & 1 deletion internal/fromproto6/planresourcechange.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (

// PlanResourceChangeRequest returns the *fwserver.PlanResourceChangeRequest
// equivalent of a *tfprotov6.PlanResourceChangeRequest.
func PlanResourceChangeRequest(ctx context.Context, proto6 *tfprotov6.PlanResourceChangeRequest, reqResource resource.Resource, resourceSchema fwschema.Schema, providerMetaSchema fwschema.Schema) (*fwserver.PlanResourceChangeRequest, diag.Diagnostics) {
func PlanResourceChangeRequest(ctx context.Context, proto6 *tfprotov6.PlanResourceChangeRequest, reqResource resource.Resource, resourceSchema fwschema.Schema, providerMetaSchema fwschema.Schema, resourceBehavior resource.ResourceBehavior) (*fwserver.PlanResourceChangeRequest, diag.Diagnostics) {
if proto6 == nil {
return nil, nil
}
Expand All @@ -39,6 +39,7 @@ func PlanResourceChangeRequest(ctx context.Context, proto6 *tfprotov6.PlanResour
}

fw := &fwserver.PlanResourceChangeRequest{
ResourceBehavior: resourceBehavior,
ResourceSchema: resourceSchema,
Resource: reqResource,
ClientCapabilities: ModifyPlanClientCapabilities(proto6.ClientCapabilities),
Expand Down
20 changes: 19 additions & 1 deletion internal/fromproto6/planresourcechange_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ func TestPlanResourceChangeRequest(t *testing.T) {

testCases := map[string]struct {
input *tfprotov6.PlanResourceChangeRequest
resourceBehavior resource.ResourceBehavior
resourceSchema fwschema.Schema
resource resource.Resource
providerMetaSchema fwschema.Schema
Expand Down Expand Up @@ -241,6 +242,23 @@ func TestPlanResourceChangeRequest(t *testing.T) {
ResourceSchema: testFwSchema,
},
},
"resource-behavior": {
input: &tfprotov6.PlanResourceChangeRequest{},
resourceSchema: testFwSchema,
resourceBehavior: resource.ResourceBehavior{
ProviderDeferred: resource.ProviderDeferredBehavior{
EnablePlanModification: true,
},
},
expected: &fwserver.PlanResourceChangeRequest{
ResourceBehavior: resource.ResourceBehavior{
ProviderDeferred: resource.ProviderDeferredBehavior{
EnablePlanModification: true,
},
},
ResourceSchema: testFwSchema,
},
},
}

for name, testCase := range testCases {
Expand All @@ -249,7 +267,7 @@ func TestPlanResourceChangeRequest(t *testing.T) {
t.Run(name, func(t *testing.T) {
t.Parallel()

got, diags := fromproto6.PlanResourceChangeRequest(context.Background(), testCase.input, testCase.resource, testCase.resourceSchema, testCase.providerMetaSchema)
got, diags := fromproto6.PlanResourceChangeRequest(context.Background(), testCase.input, testCase.resource, testCase.resourceSchema, testCase.providerMetaSchema, testCase.resourceBehavior)

if diff := cmp.Diff(got, testCase.expected, cmp.AllowUnexported(privatestate.ProviderData{})); diff != "" {
t.Errorf("unexpected difference: %s", diff)
Expand Down
Loading