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 35 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
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
90 changes: 90 additions & 0 deletions internal/fwserver/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,11 @@ type Server struct {
// access from race conditions.
dataSourceTypesMutex sync.Mutex

// deferred indicates an automatic provider deferral. When this is set,
// the provider will automatically defer the PlanResourceChange, ReadResource,
// ImportResourceState, and ReadDataSource RPCs.
deferred *provider.Deferred

// functionDefinitions is the cached Function Definitions for RPCs that need to
// convert data from the protocol. If not found, it will be fetched from the
// Function.Definition() method.
Expand Down Expand Up @@ -137,6 +142,19 @@ type Server struct {
// resourceTypesMutex is a mutex to protect concurrent resourceTypes
// access from race conditions.
resourceTypesMutex sync.Mutex

// resourceBehaviors is the cached Resource behaviors for RPCs that need to
// control framework-specific logic when interacting with a resource.
resourceBehaviors map[string]resource.ResourceBehavior

// resourceBehaviorsDiags is the cached Diagnostics obtained while populating
// resourceBehaviors. This is to ensure any warnings or errors are also
// returned appropriately when fetching resourceBehaviors.
resourceBehaviorsDiags diag.Diagnostics

// resourceBehaviorsMutex is a mutex to protect concurrent resourceBehaviors
// access from race conditions.
resourceBehaviorsMutex sync.Mutex
SBGoods marked this conversation as resolved.
Show resolved Hide resolved
}

// DataSource returns the DataSource for a given type name.
Expand Down Expand Up @@ -414,6 +432,78 @@ func (s *Server) Resource(ctx context.Context, typeName string) (resource.Resour
return resourceFunc(), diags
}

// ResourceBehavior returns the ResourceBehavior for a given type name.
func (s *Server) ResourceBehavior(ctx context.Context, typeName string) (resource.ResourceBehavior, diag.Diagnostics) {
resourceBehaviors, diags := s.ResourceBehaviors(ctx)

resourceBehavior, ok := resourceBehaviors[typeName]

if !ok {
diags.AddError(
"Resource Type Not Found",
fmt.Sprintf("No resource type named %q was found in the provider.", typeName),
)

return resource.ResourceBehavior{}, diags
}

return resourceBehavior, diags
}

// ResourceBehaviors returns a map of ResourceBehavior. The results are cached
// on first use.
func (s *Server) ResourceBehaviors(ctx context.Context) (map[string]resource.ResourceBehavior, diag.Diagnostics) {
logging.FrameworkTrace(ctx, "Checking ResourceBehaviors lock")
s.resourceBehaviorsMutex.Lock()
defer s.resourceBehaviorsMutex.Unlock()

if s.resourceBehaviors != nil {
return s.resourceBehaviors, s.resourceBehaviorsDiags
}

providerTypeName := s.ProviderTypeName(ctx)
s.resourceBehaviors = make(map[string]resource.ResourceBehavior)

resourceFuncs, diags := s.ResourceFuncs(ctx)
s.resourceBehaviorsDiags.Append(diags...)

for _, resourceFunc := range resourceFuncs {
res := resourceFunc()

metadataRequest := resource.MetadataRequest{
ProviderTypeName: providerTypeName,
}
metadataResponse := resource.MetadataResponse{}

res.Metadata(ctx, metadataRequest, &metadataResponse)

if metadataResponse.TypeName == "" {
s.resourceBehaviorsDiags.AddError(
"Resource Type Name Missing",
fmt.Sprintf("The %T Resource returned an empty string from the Metadata method. ", res)+
"This is always an issue with the provider and should be reported to the provider developers.",
)
continue
}

logging.FrameworkTrace(ctx, "Found resource type", map[string]interface{}{logging.KeyResourceType: metadataResponse.TypeName})

if _, ok := s.resourceBehaviors[metadataResponse.TypeName]; ok {
s.resourceBehaviorsDiags.AddError(
"Duplicate Resource Type Defined",
fmt.Sprintf("The %s resource type name was returned for multiple resources. ", metadataResponse.TypeName)+
"Resource type names must be unique. "+
"This is always an issue with the provider and should be reported to the provider developers.",
)
continue
}

s.resourceBehaviors[metadataResponse.TypeName] = metadataResponse.ResourceBehavior
}

return s.resourceBehaviors, s.resourceBehaviorsDiags
}

// ResourceFuncs returns a map of Resource functions. The results are cached
// on first use.
func (s *Server) ResourceFuncs(ctx context.Context) (map[string]func() resource.Resource, diag.Diagnostics) {
Expand Down
12 changes: 12 additions & 0 deletions internal/fwserver/server_configureprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,18 @@ func (s *Server) ConfigureProvider(ctx context.Context, req *provider.ConfigureR

logging.FrameworkTrace(ctx, "Called provider defined Provider Configure")

if resp.Deferred != nil {
if !req.ClientCapabilities.DeferralAllowed {
resp.Diagnostics.AddError("Invalid Deferred Provider Response",
"Provider configured a deferred response for all resources and data sources but the Terraform request "+
"did not indicate support for deferred actions. This is an issue with the provider and should be reported to the provider developers.")
SBGoods marked this conversation as resolved.
Show resolved Hide resolved
} else {
logging.FrameworkDebug(ctx, "Provider has configured a deferred response, "+
"all associated resources and data sources will automatically return a deferred response.")
}
SBGoods marked this conversation as resolved.
Show resolved Hide resolved
}

s.deferred = resp.Deferred
SBGoods marked this conversation as resolved.
Show resolved Hide resolved
s.DataSourceConfigureData = resp.DataSourceData
s.ResourceConfigureData = resp.ResourceData
}
Loading
Loading