Skip to content

Commit

Permalink
Unignore empty values in the provider configuration block in 5.0.0 …
Browse files Browse the repository at this point in the history
…(#9014)

* Stop validating `""` as valid `credentials` value in the PF provider config

* Remove code that makes PF ignore empty values

* Update PF config code tests to match change in PF config code behaviour

* Update remaining PF config code tests

* Update test for the PF version of credentials validator

* Make not on SDK version of credentials validator that it cannot distinguish between `""` in config vs zero values

Signed-off-by: Modular Magician <magic-modules@google.com>
  • Loading branch information
modular-magician committed Sep 22, 2023
1 parent 3f338dc commit 33fc047
Show file tree
Hide file tree
Showing 7 changed files with 63 additions and 140 deletions.
3 changes: 3 additions & 0 deletions .changelog/9014.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:breaking-change
provider: Empty strings in the provider configuration block will no longer be ignored when configuring the provider
```
3 changes: 2 additions & 1 deletion google-beta/fwprovider/framework_provider_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,11 @@ func TestFrameworkProvider_CredentialsValidator(t *testing.T) {
return types.StringValue(stringContents)
},
},
"configuring credentials as an empty string is valid": {
"configuring credentials as an empty string is not valid": {
ConfigValue: func(t *testing.T) types.String {
return types.StringValue("")
},
ExpectedErrorCount: 1,
},
"leaving credentials unconfigured is valid": {
ConfigValue: func(t *testing.T) types.String {
Expand Down
3 changes: 1 addition & 2 deletions google-beta/fwprovider/framework_validators.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"time"

"github.com/hashicorp/terraform-plugin-framework/schema/validator"
"github.com/hashicorp/terraform-plugin-framework/types"

googleoauth "golang.org/x/oauth2/google"
)
Expand All @@ -33,7 +32,7 @@ func (v credentialsValidator) MarkdownDescription(ctx context.Context) string {

// ValidateString performs the validation.
func (v credentialsValidator) ValidateString(ctx context.Context, request validator.StringRequest, response *validator.StringResponse) {
if request.ConfigValue.IsNull() || request.ConfigValue.IsUnknown() || request.ConfigValue.Equal(types.StringValue("")) {
if request.ConfigValue.IsNull() || request.ConfigValue.IsUnknown() {
return
}

Expand Down
80 changes: 0 additions & 80 deletions google-beta/fwtransport/framework_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"google.golang.org/grpc"

"github.com/hashicorp/go-cleanhttp"
"github.com/hashicorp/terraform-plugin-framework/attr"
"github.com/hashicorp/terraform-plugin-framework/diag"
"github.com/hashicorp/terraform-plugin-framework/types"
"github.com/hashicorp/terraform-plugin-log/tflog"
Expand Down Expand Up @@ -168,14 +167,6 @@ type FrameworkProviderConfig struct {
// it is pulled out so that we can manually call this from our testing provider as well
func (p *FrameworkProviderConfig) LoadAndValidateFramework(ctx context.Context, data *fwmodels.ProviderModel, tfVersion string, diags *diag.Diagnostics, providerversion string) {

// Make the plugin framwork code behave like the SDK by ignoring zero values. This means re-setting zero values to null.
// This is added to fix https://github.com/hashicorp/terraform-provider-google/issues/14255 in a v4.x.x release
// TODO(SarahFrench) remove as part of https://github.com/hashicorp/terraform-provider-google/issues/14447 in 5.0.0
p.HandleZeroValues(ctx, data, diags)
if diags.HasError() {
return
}

// Set defaults if needed
p.HandleDefaults(ctx, data, diags)
if diags.HasError() {
Expand Down Expand Up @@ -335,77 +326,6 @@ func (p *FrameworkProviderConfig) LoadAndValidateFramework(ctx context.Context,
p.RequestBatcherIam = transport_tpg.NewRequestBatcher("IAM", ctx, batchingConfig)
}

// HandleZeroValues will make the plugin framework act like the SDK; zero value, particularly empty strings, are converted to null.
// This causes the plugin framework to treat the field as unset, just like how the SDK ignores empty strings.
func (p *FrameworkProviderConfig) HandleZeroValues(ctx context.Context, data *fwmodels.ProviderModel, diags *diag.Diagnostics) {

// Change empty strings to null values
if data.AccessToken.Equal(types.StringValue("")) {
data.AccessToken = types.StringNull()
}
if data.BillingProject.Equal(types.StringValue("")) {
data.BillingProject = types.StringNull()
}
if data.Credentials.Equal(types.StringValue("")) {
data.Credentials = types.StringNull()
}
if data.ImpersonateServiceAccount.Equal(types.StringValue("")) {
data.ImpersonateServiceAccount = types.StringNull()
}
if data.Project.Equal(types.StringValue("")) {
data.Project = types.StringNull()
}
if data.Region.Equal(types.StringValue("")) {
data.Region = types.StringNull()
}
if data.RequestReason.Equal(types.StringValue("")) {
data.RequestReason = types.StringNull()
}
if data.RequestTimeout.Equal(types.StringValue("")) {
data.RequestTimeout = types.StringNull()
}
if data.Zone.Equal(types.StringValue("")) {
data.Zone = types.StringNull()
}

// Change lists that aren't null or unknown with length of zero to null lists
if !data.Scopes.IsNull() && !data.Scopes.IsUnknown() && (len(data.Scopes.Elements()) == 0) {
data.Scopes = types.ListNull(types.StringType)
}
if !data.ImpersonateServiceAccountDelegates.IsNull() && !data.ImpersonateServiceAccountDelegates.IsUnknown() && (len(data.ImpersonateServiceAccountDelegates.Elements()) == 0) {
data.ImpersonateServiceAccountDelegates = types.ListNull(types.StringType)
}

// Batching implementation will change in future, but this code will be removed in 5.0.0 so may be unaffected
if !data.Batching.IsNull() && !data.Batching.IsUnknown() && (len(data.Batching.Elements()) > 0) {
var pbConfigs []fwmodels.ProviderBatching
d := data.Batching.ElementsAs(ctx, &pbConfigs, true)
diags.Append(d...)
if diags.HasError() {
return
}
if pbConfigs[0].SendAfter.Equal(types.StringValue("")) {
pbConfigs[0].SendAfter = types.StringNull() // Convert empty string to null
}
b, _ := types.ObjectValue(
map[string]attr.Type{
"enable_batching": types.BoolType,
"send_after": types.StringType,
},
map[string]attr.Value{
"enable_batching": pbConfigs[0].EnableBatching,
"send_after": pbConfigs[0].SendAfter,
},
)
newBatching, d := types.ListValue(types.ObjectType{}.WithAttributeTypes(fwmodels.ProviderBatchingAttributes), []attr.Value{b})
diags.Append(d...)
if diags.HasError() {
return
}
data.Batching = newBatching
}
}

// HandleDefaults will handle all the defaults necessary in the provider
func (p *FrameworkProviderConfig) HandleDefaults(ctx context.Context, data *fwmodels.ProviderModel, diags *diag.Diagnostics) {
if (data.AccessToken.IsNull() || data.AccessToken.IsUnknown()) && (data.Credentials.IsNull() || data.Credentials.IsUnknown()) {
Expand Down
Loading

0 comments on commit 33fc047

Please sign in to comment.