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.

Sign up for GitHub

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

Jump to bottom

schema.ResourceData.GetRaw<whatever>() not populated during provider configuration #1270

Closed
gdavison opened this issue Nov 1, 2023 · 3 comments · Fixed by #1271
Closed

schema.ResourceData.GetRaw<whatever>() not populated during provider configuration #1270

gdavison opened this issue Nov 1, 2023 · 3 comments · Fixed by #1271
Assignees
@bflad
Labels
bug Something isn't working
Milestone
v2.30.0

Comments

@gdavison
Copy link
Contributor

gdavison commented Nov 1, 2023

SDK version

github.com/hashicorp/terraform-plugin-sdk/v2 v2.29.0

Relevant provider source code

provider.ConfigureContextFunc = func(ctx context.Context, d *schema.ResourceData) (interface{}, diag.Diagnostics) {
  	// ...
  	rawConfig := d.GetRawConfig()
	if rawConfig.IsKnown() {
		if rawConfig.IsNull() {
			fmt.Println("null config")
		}
	}

	rawPlan := d.GetRawPlan()
	if rawPlan.IsKnown() {
		if rawPlan.IsNull() {
			fmt.Println("null plan")
		}
	}

	rawState := d.GetRawState()
	if rawState.IsKnown() {
		if rawState.IsNull() {
			fmt.Println("null state")
		}
	}

}

Expected Behavior

The use case for using the Raw functions is to allow an empty string parameter in provider configs. To allow fallback behaviour to prevent a breaking change.

The traditional d.Get, d.GetOk, and d.GetOkExists functions do not make a distinction between an empty string and the value not being set.

Actual Behavior

The cty.Values returned by the GetRaw<whatever> functions are all Null
@gdavison gdavison added the bug Something isn't working label Nov 1, 2023
@bflad bflad self-assigned this Nov 1, 2023
@bflad
Copy link
Member

bflad commented Nov 1, 2023

Looks like this RPC was never setup for those GetRaw* methods when that was originally added. Taking a quick peek it looks like it should be feasible, but it might be a little awkward implementation-wise in the SDK code.

bflad added a commit that referenced this issue Nov 2, 2023
@bflad
helper/schema: Ensure (ResourceData).GetRawConfig() is populated in p…
68a4c85 
…rovider Configure functions

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,
              }
```
@bflad bflad mentioned this issue Nov 2, 2023
Merged
@bflad
Copy link
Member

bflad commented Nov 2, 2023

Proposed fix: #1271 -- please give that a try and let us know if it resolves this issue. If so, we'll cut a bug fix release with it. Thanks!

@bflad bflad added this to the v2.30.0 milestone Nov 2, 2023
bflad added a commit that referenced this issue Nov 9, 2023
@bflad
helper/schema: Ensure (ResourceData).GetRawConfig() is populated in p…
8130d59 
…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"}},
              )
```
@github-actions GitHub Actions
Copy link

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@bflad bflad

Labels
bug Something isn't working
Projects
None yet
Milestone
v2.30.0
2 participants
@bflad @gdavison