Skip to content

Fix provider configuration code to handle Unknown values correctly#8943

Merged
SarahFrench merged 15 commits into
mainfrom
make-pf-handle-unknown-values-like-nulls
Sep 19, 2023
Merged

Fix provider configuration code to handle Unknown values correctly#8943
SarahFrench merged 15 commits into
mainfrom
make-pf-handle-unknown-values-like-nulls

Conversation

@SarahFrench

@SarahFrench SarahFrench commented Sep 13, 2023

Copy link
Copy Markdown
Contributor

Fixes hashicorp/terraform-provider-google#14444

Related to #8798

Release Note Template for Downstream PRs (will be copied)

provider: addressed a bug where configuring the provider with unknown values did not behave as expected

@modular-magician

This comment was marked as outdated.

@SarahFrench SarahFrench force-pushed the make-pf-handle-unknown-values-like-nulls branch from 70f072d to 0c5b503 Compare September 13, 2023 20:21
@modular-magician

Copy link
Copy Markdown
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 3 files changed, 230 insertions(+), 183 deletions(-))
Terraform Beta: Diff ( 3 files changed, 230 insertions(+), 183 deletions(-))

@modular-magician

Copy link
Copy Markdown
Collaborator

Tests analytics

Total tests: 3037
Passed tests 2736
Skipped tests: 297
Affected tests: 4

Action taken

Found 4 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
TestAccMonitoringMonitoredProject_monitoringMonitoredProjectLongFormExample|TestAccMonitoringMonitoredProject_projectNumShortForm|TestAccMonitoringMonitoredProject_projectNumLongForm|TestAccMonitoringMonitoredProject_monitoringMonitoredProjectBasicExample

Get to know how VCR tests work

@modular-magician

Copy link
Copy Markdown
Collaborator

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccMonitoringMonitoredProject_monitoringMonitoredProjectLongFormExample[Debug log]
TestAccMonitoringMonitoredProject_projectNumShortForm[Debug log]
TestAccMonitoringMonitoredProject_projectNumLongForm[Debug log]
TestAccMonitoringMonitoredProject_monitoringMonitoredProjectBasicExample[Debug log]

Rerun these tests in REPLAYING mode to catch issues

$\textcolor{green}{\textsf{No issues found for passed tests after REPLAYING rerun.}}$


$\textcolor{green}{\textsf{All tests passed!}}$
View the build log or the debug log for each test

@SarahFrench SarahFrench force-pushed the make-pf-handle-unknown-values-like-nulls branch from 0c5b503 to cd9fde2 Compare September 14, 2023 18:07
@modular-magician

This comment was marked as outdated.

@SarahFrench SarahFrench marked this pull request as ready for review September 14, 2023 19:23
@SarahFrench

Copy link
Copy Markdown
Contributor Author

I've yet to manually test this change to see if this use case from the original issue works as expected again:

provider "google-beta" {
  project     = "foo"
  credentials = base64decode(google_service_account_key.terraform_service_account.private_key)
}

@modular-magician

Copy link
Copy Markdown
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 2 files changed, 227 insertions(+), 175 deletions(-))
Terraform Beta: Diff ( 2 files changed, 227 insertions(+), 175 deletions(-))

Comment thread mmv1/third_party/terraform/fwtransport/framework_config_test.go.erb
@modular-magician

Copy link
Copy Markdown
Collaborator

Tests analytics

Total tests: 3044
Passed tests 2745
Skipped tests: 298
Affected tests: 1

Action taken

Found 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
TestAccDataSourceGoogleServiceAccountIdToken_impersonation

Get to know how VCR tests work

@modular-magician

Copy link
Copy Markdown
Collaborator

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccDataSourceGoogleServiceAccountIdToken_impersonation[Debug log]

Rerun these tests in REPLAYING mode to catch issues

$\textcolor{green}{\textsf{No issues found for passed tests after REPLAYING rerun.}}$


$\textcolor{green}{\textsf{All tests passed!}}$
View the build log or the debug log for each test

…rovider config

Only testing `credentials` currently.
@SarahFrench

SarahFrench commented Sep 19, 2023

Copy link
Copy Markdown
Contributor Author

I manually tested the use case from the original issue (see comment) and I've added a non-VCR acceptance test that asserts the provider's behaviour.

I've tried to implement that test in a way so it uses the code in the repo (instead of downloading from the Terraform Registry) but kept hitting issues. The above is good enough for now I think.

@modular-magician

Copy link
Copy Markdown
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 3 files changed, 378 insertions(+), 175 deletions(-))
Terraform Beta: Diff ( 3 files changed, 378 insertions(+), 175 deletions(-))

@modular-magician

Copy link
Copy Markdown
Collaborator

Tests analytics

Total tests: 3070
Passed tests 2770
Skipped tests: 300
Affected tests: 0

$\textcolor{green}{\textsf{All tests passed in REPLAYING mode.}}$
View the build log

@SarahFrench SarahFrench merged commit b162a69 into main Sep 19, 2023
bobyu-google pushed a commit to bobyu-google/magic-modules that referenced this pull request Sep 20, 2023
…oogleCloudPlatform#8943)

* 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.
nevzheng pushed a commit to nevzheng/magic-modules that referenced this pull request Sep 22, 2023
…oogleCloudPlatform#8943)

* 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.
@SarahFrench SarahFrench deleted the make-pf-handle-unknown-values-like-nulls branch September 26, 2023 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Plugin framework version of provider configuration code doesn't handle unknown values the same way as the SDK

3 participants