Skip to content

Commit

Permalink
helper/schema: Ensure (ResourceData).GetRawConfig() is populated in p…
Browse files Browse the repository at this point in the history
…rovider Configure functions (#1271)

Reference: #1270

This change is intended to be as targeted as possible to prevent other unintended changes. In other RPCs, the protocol configuration data is able to be set upfront via `terraform.InstanceState`, however for provider configuration it is still using the legacy `terraform.ResourceConfig` value which previously did not have the same data field. This adds the data field while trying to be pragmatic about potentially breaking compatibility with the unfortunately exported APIs in this SDK.

New test failures prior to updating logic:

```
--- FAIL: TestProviderConfigure (0.00s)
    --- FAIL: TestProviderConfigure/ConfigureContextFunc-GetRawConfig (0.00s)
        /Users/bflad/src/github.com/hashicorp/terraform-plugin-sdk/helper/schema/provider_test.go:338: Unexpected diagnostics (-wanted +got):   diag.Diagnostics(
            - 	nil,
            + 	{{Summary: "unexpected GetRawConfig difference: expected: {{{{} map[test:{{{"...}},
              )
    --- FAIL: TestProviderConfigure/ConfigureFunc-GetRawConfig (0.00s)
        /Users/bflad/src/github.com/hashicorp/terraform-plugin-sdk/helper/schema/provider_test.go:338: Unexpected diagnostics (-wanted +got):   diag.Diagnostics(
            - 	nil,
            + 	{{Summary: "unexpected GetRawConfig difference: expected: {{{{} map[test:{{{"...}},
              )

--- FAIL: TestGRPCProviderServerConfigureProvider (0.00s)
    --- FAIL: TestGRPCProviderServerConfigureProvider/ConfigureContextFunc-GetRawConfig (0.00s)
        /Users/bflad/src/github.com/hashicorp/terraform-plugin-sdk/helper/schema/grpc_provider_test.go:343: unexpected difference:   &tfprotov5.ConfigureProviderResponse{
            - 	Diagnostics: []*tfprotov5.Diagnostic{
            - 		&{
            - 			Severity: s"ERROR",
            - 			Summary:  "unexpected difference: expected: {{{{} map[test:{{{} %!s(cty.pri"...,
            - 		},
            - 	},
            + 	Diagnostics: nil,
              }
    --- FAIL: TestGRPCProviderServerConfigureProvider/ConfigureFunc-GetRawConfig (0.00s)
        /Users/bflad/src/github.com/hashicorp/terraform-plugin-sdk/helper/schema/grpc_provider_test.go:343: unexpected difference:   &tfprotov5.ConfigureProviderResponse{
            - 	Diagnostics: []*tfprotov5.Diagnostic{
            - 		&{
            - 			Severity: s"ERROR",
            - 			Summary:  "unexpected difference: expected: {{{{} map[test:{{{} %!s(cty.pri"...,
            - 		},
            - 	},
            + 	Diagnostics: nil,
              }
```

This change also fixes `GetOkExists()` handling for provider configuration.

Updated tests prior to logic updates:

```
--- FAIL: TestProviderConfigure (0.00s)
    --- FAIL: TestProviderConfigure/ConfigureContextFunc-GetOkExists-zero-value (0.00s)
        /Users/bflad/src/github.com/hashicorp/terraform-plugin-sdk/helper/schema/provider_test.go:924: Unexpected diagnostics (-wanted +got):   diag.Diagnostics(
            - 	nil,
            + 	{{Summary: "unexpected GetOkExists difference: expected: false, got: true"}},
              )
    --- FAIL: TestProviderConfigure/ConfigureFunc-GetOkExists-zero-value (0.00s)
        /Users/bflad/src/github.com/hashicorp/terraform-plugin-sdk/helper/schema/provider_test.go:924: Unexpected diagnostics (-wanted +got):   diag.Diagnostics(
            - 	nil,
            + 	{{Summary: "unexpected GetOkExists difference: expected: false, got: true"}},
              )
```
  • Loading branch information
bflad committed Nov 9, 2023
1 parent 28e6317 commit 8130d59
Show file tree
Hide file tree
Showing 9 changed files with 3,566 additions and 23 deletions.
6 changes: 6 additions & 0 deletions .changes/unreleased/BUG FIXES-20231102-063650.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: BUG FIXES
body: 'helper/schema: Ensured `(ResourceData).GetRawConfig()` data is populated for
`Provider.ConfigureFunc` and `Provider.ConfigureContextFunc`'
time: 2023-11-02T06:36:50.137259-04:00
custom:
Issue: "1270"
7 changes: 7 additions & 0 deletions .changes/unreleased/BUG FIXES-20231106-085112.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
kind: BUG FIXES
body: 'helper/schema: Ensured `(ResourceData).GetOkExists()` second result is `true`
when configuration contains zero-value data in `Provider.ConfigureFunc` and
`Provider.ConfigureContextFunc`'
time: 2023-11-06T08:51:12.814228-05:00
custom:
Issue: "1270"
12 changes: 12 additions & 0 deletions helper/schema/grpc_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -582,6 +582,18 @@ func (s *GRPCProviderServer) ConfigureProvider(ctx context.Context, req *tfproto
}

config := terraform.NewResourceConfigShimmed(configVal, schemaBlock)

// CtyValue is the raw protocol configuration data from newer APIs.
//
// This field was only added as a targeted fix for passing raw protocol data
// through the existing (helper/schema.Provider).Configure() exported method
// and is only populated in that situation. The data could theoretically be
// set in the NewResourceConfigShimmed() function, however the consequences
// of doing this were not investigated at the time the fix was introduced.
//
// Reference: https://github.com/hashicorp/terraform-plugin-sdk/issues/1270
config.CtyValue = configVal

// TODO: remove global stop context hack
// This attaches a global stop synchro'd context onto the provider.Configure
// request scoped context. This provides a substitute for the removed provider.StopContext()
Expand Down
Loading

0 comments on commit 8130d59

Please sign in to comment.