Skip to content

Commit

Permalink
Fix provider configuration code to handle Unknown values correctly (#…
Browse files Browse the repository at this point in the history 
…8943) (#6312)

* Uncomment test cases for unknown values

* Update test case names

* Update unknown value test for `project`, make it pass

* Update unknown value test for `access_token`, make `access_token` and `credentials` tests pass

* Update unknown value tests for `region` and `zone`, make those tests pass

* Update unknown value tests for `user_project_override`, make that test pass

* Update unknown value test for `impersonate_service_account`, make that test pass

* Update unknown value tests for `request_reason` and `request_timeout`, make those tests pass

* Make unknown batching.send_after and batching.enable_batching values be set to same defaults as if they were null, update test

* Update code to handle when the whole batching block is Unknown

* Update the test function for `batching` unit tests to navigate how `GetBatchingConfig` is used by the code

* Update code to handle null/unknown Scopes and ImpersonateServiceAccountDelegates values

* Improve `impersonate_service_account_delegates` tests for unknown values

* Add missing null test case for `batching` field

* Add non-VCR acceptance test to assert handling of unknown values in provider config

Only testing `credentials` currently.

Signed-off-by: Modular Magician <[email protected]>
  • Loading branch information
@modular-magician
modular-magician authored Sep 19, 2023
1 parent 8773d87 commit 44003a2
Show file tree
Hide file tree
Showing 4 changed files with 381 additions and 175 deletions.
3 changes: 3 additions & 0 deletions .changelog/8943.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
provider: addressed a bug where configuring the provider with unknown values did not behave as expected
```
51 changes: 28 additions & 23 deletions google-beta/fwtransport/framework_config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@ func (p *FrameworkProviderConfig) HandleZeroValues(ctx context.Context, data *fw

// 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.Credentials.IsNull() {
if (data.AccessToken.IsNull() || data.AccessToken.IsUnknown()) && (data.Credentials.IsNull() || data.Credentials.IsUnknown()) {
credentials := transport_tpg.MultiEnvDefault([]string{
"GOOGLE_CREDENTIALS",
"GOOGLE_CLOUD_KEYFILE_JSON",
Expand All @@ -432,11 +432,11 @@ func (p *FrameworkProviderConfig) HandleDefaults(ctx context.Context, data *fwmo
}
}

if data.ImpersonateServiceAccount.IsNull() && os.Getenv("GOOGLE_IMPERSONATE_SERVICE_ACCOUNT") != "" {
if (data.ImpersonateServiceAccount.IsNull() || data.ImpersonateServiceAccount.IsUnknown()) && os.Getenv("GOOGLE_IMPERSONATE_SERVICE_ACCOUNT") != "" {
data.ImpersonateServiceAccount = types.StringValue(os.Getenv("GOOGLE_IMPERSONATE_SERVICE_ACCOUNT"))
}

if data.Project.IsNull() {
if data.Project.IsNull() || data.Project.IsUnknown() {
project := transport_tpg.MultiEnvDefault([]string{
"GOOGLE_PROJECT",
"GOOGLE_CLOUD_PROJECT",
Expand All @@ -452,7 +452,7 @@ func (p *FrameworkProviderConfig) HandleDefaults(ctx context.Context, data *fwmo
data.BillingProject = types.StringValue(os.Getenv("GOOGLE_BILLING_PROJECT"))
}

if data.Region.IsNull() {
if data.Region.IsNull() || data.Region.IsUnknown() {
region := transport_tpg.MultiEnvDefault([]string{
"GOOGLE_REGION",
"GCLOUD_REGION",
Expand All @@ -464,7 +464,7 @@ func (p *FrameworkProviderConfig) HandleDefaults(ctx context.Context, data *fwmo
}
}

if data.Zone.IsNull() {
if data.Zone.IsNull() || data.Zone.IsUnknown() {
zone := transport_tpg.MultiEnvDefault([]string{
"GOOGLE_ZONE",
"GCLOUD_ZONE",
Expand All @@ -485,26 +485,26 @@ func (p *FrameworkProviderConfig) HandleDefaults(ctx context.Context, data *fwmo
}
}

if !data.Batching.IsNull() {
if !data.Batching.IsNull() && !data.Batching.IsUnknown() {
var pbConfigs []fwmodels.ProviderBatching
d := data.Batching.ElementsAs(ctx, &pbConfigs, true)
diags.Append(d...)
if diags.HasError() {
return
}

if pbConfigs[0].SendAfter.IsNull() {
if pbConfigs[0].SendAfter.IsNull() || pbConfigs[0].SendAfter.IsUnknown() {
pbConfigs[0].SendAfter = types.StringValue("10s")
}

if pbConfigs[0].EnableBatching.IsNull() {
if pbConfigs[0].EnableBatching.IsNull() || pbConfigs[0].EnableBatching.IsUnknown() {
pbConfigs[0].EnableBatching = types.BoolValue(true)
}

data.Batching, d = types.ListValueFrom(ctx, types.ObjectType{}.WithAttributeTypes(fwmodels.ProviderBatchingAttributes), pbConfigs)
}

if data.UserProjectOverride.IsNull() && os.Getenv("USER_PROJECT_OVERRIDE") != "" {
if (data.UserProjectOverride.IsNull() || data.UserProjectOverride.IsUnknown()) && os.Getenv("USER_PROJECT_OVERRIDE") != "" {
override, err := strconv.ParseBool(os.Getenv("USER_PROJECT_OVERRIDE"))
if err != nil {
diags.AddError(
Expand All @@ -513,11 +513,11 @@ func (p *FrameworkProviderConfig) HandleDefaults(ctx context.Context, data *fwmo
data.UserProjectOverride = types.BoolValue(override)
}

if data.RequestReason.IsNull() && os.Getenv("CLOUDSDK_CORE_REQUEST_REASON") != "" {
if (data.RequestReason.IsNull() || data.RequestReason.IsUnknown()) && os.Getenv("CLOUDSDK_CORE_REQUEST_REASON") != "" {
data.RequestReason = types.StringValue(os.Getenv("CLOUDSDK_CORE_REQUEST_REASON"))
}

if data.RequestTimeout.IsNull() {
if data.RequestTimeout.IsNull() || data.RequestTimeout.IsUnknown() {
data.RequestTimeout = types.StringValue("120s")
}

Expand Down Expand Up @@ -1727,7 +1727,7 @@ func (p *FrameworkProviderConfig) logGoogleIdentities(ctx context.Context, data
// a separate diagnostics here
var d diag.Diagnostics

if data.ImpersonateServiceAccount.IsNull() {
if data.ImpersonateServiceAccount.IsNull() || data.ImpersonateServiceAccount.IsUnknown() {

tokenSource := GetTokenSource(ctx, data, true, diags)
if diags.HasError() {
Expand Down Expand Up @@ -1787,19 +1787,23 @@ func GetCredentials(ctx context.Context, data fwmodels.ProviderModel, initialCre
var clientScopes []string
var delegates []string

d := data.Scopes.ElementsAs(ctx, &clientScopes, false)
diags.Append(d...)
if diags.HasError() {
return googleoauth.Credentials{}
if !data.Scopes.IsNull() && !data.Scopes.IsUnknown() {
d := data.Scopes.ElementsAs(ctx, &clientScopes, false)
diags.Append(d...)
if diags.HasError() {
return googleoauth.Credentials{}
}
}

d = data.ImpersonateServiceAccountDelegates.ElementsAs(ctx, &delegates, false)
diags.Append(d...)
if diags.HasError() {
return googleoauth.Credentials{}
if !data.ImpersonateServiceAccountDelegates.IsNull() && !data.ImpersonateServiceAccountDelegates.IsUnknown() {
d := data.ImpersonateServiceAccountDelegates.ElementsAs(ctx, &delegates, false)
diags.Append(d...)
if diags.HasError() {
return googleoauth.Credentials{}
}
}

if !data.AccessToken.IsNull() {
if !data.AccessToken.IsNull() && !data.AccessToken.IsUnknown() {
contents, _, err := verify.PathOrContents(data.AccessToken.ValueString())
if err != nil {
diags.AddError("error loading access token", err.Error())
Expand All @@ -1824,7 +1828,7 @@ func GetCredentials(ctx context.Context, data fwmodels.ProviderModel, initialCre
}
}

if !data.Credentials.IsNull() {
if !data.Credentials.IsNull() && !data.Credentials.IsUnknown() {
contents, _, err := verify.PathOrContents(data.Credentials.ValueString())
if err != nil {
diags.AddError(fmt.Sprintf("error loading credentials: %s", err), err.Error())
Expand Down Expand Up @@ -1883,7 +1887,8 @@ func GetBatchingConfig(ctx context.Context, data types.List, diags *diag.Diagnos
EnableBatching: true,
}

if data.IsNull() {
// Handle if entire batching block is null/unknown
if data.IsNull() || data.IsUnknown() {
return bc
}

Expand Down
Loading

0 comments on commit 44003a2

Please sign in to comment.