update web app config datasource to plugin-framework #7501

megan07 · 2023-03-20T21:17:18Z

I have this based off #7266, updates the google_firebase_web_app_config to use plugin-framework

If this PR is for Terraform, I acknowledge that I have:

Searched through the issue tracker for an open issue that this either resolves or contributes to, commented on it to claim it, and written "fixes {url}" or "part of {url}" in this PR description. If there were no relevant open issues, I opened one and commented that I would like to work on it (not necessary for very small changes).
Ensured that all new fields I added that can be set by a user appear in at least one example (for generated resources) or third_party test (for handwritten resources or update tests).
Generated Terraform providers, and ran make test and make lint in the generated providers to ensure it passes unit and linter tests.
Ran relevant acceptance tests using my own Google Cloud project and credentials (If the acceptance tests do not yet pass or you are unable to run them, please let your reviewer know).
Read the Release Notes Guide before writing my release note below.

Release Note Template for Downstream PRs (will be copied)

modular-magician · 2023-03-24T15:32:02Z

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 ( 25 files changed, 4722 insertions(+), 668 deletions(-))
Terraform Beta: Diff ( 29 files changed, 5337 insertions(+), 853 deletions(-))
TF Validator: Diff ( 5 files changed, 571 insertions(+), 38 deletions(-))

modular-magician · 2023-03-24T16:23:30Z

Tests analytics

Total tests: 2526
Passed tests 2249
Skipped tests: 264
Affected tests: 13

Action taken

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

TestAccFrameworkProviderBasePath_setBasePath|TestAccFrameworkProviderBasePath_setInvalidBasePath|TestAccFrameworkProviderMeta_setModuleName|TestAccFirebaseWebApp_firebaseWebAppFull|TestAccFirebaseWebApp_firebaseWebAppBasicExample|TestAccDataSourceGoogleFirebaseAppleAppConfig|TestAccDataSourceGoogleCloudAssetResourcesSearchAll_basic|TestAccComposerEnvironment_withEncryptionConfigComposer1|TestAccDataSourceDnsRecordSet_basic|TestAccDataSourceDnsManagedZone_basic|TestAccDataSourceDNSKeys_noDnsSec|TestAccDataSourceDNSKeys_basic|TestAccNetworkServicesGateway_update

Get to know how VCR tests work

modular-magician · 2023-03-24T16:46:34Z

Tests passed during RECORDING mode:
TestAccFrameworkProviderBasePath_setBasePath[Debug log]
TestAccFrameworkProviderBasePath_setInvalidBasePath[Debug log]
TestAccFrameworkProviderMeta_setModuleName[Debug log]
TestAccFirebaseWebApp_firebaseWebAppFull[Debug log]
TestAccFirebaseWebApp_firebaseWebAppBasicExample[Debug log]
TestAccDataSourceGoogleFirebaseAppleAppConfig[Debug log]
TestAccComposerEnvironment_withEncryptionConfigComposer1[Debug log]
TestAccDataSourceDnsRecordSet_basic[Debug log]
TestAccDataSourceDnsManagedZone_basic[Debug log]
TestAccDataSourceDNSKeys_noDnsSec[Debug log]
TestAccDataSourceDNSKeys_basic[Debug log]
TestAccNetworkServicesGateway_update[Debug log]

Tests failed during RECORDING mode:
TestAccDataSourceGoogleCloudAssetResourcesSearchAll_basic[Error message] [Debug log]

Please fix these to complete your PR
View the build log or the debug log for each test

modular-magician · 2023-03-27T18:52:49Z

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 ( 25 files changed, 4725 insertions(+), 668 deletions(-))
Terraform Beta: Diff ( 29 files changed, 5340 insertions(+), 853 deletions(-))
TF Validator: Diff ( 5 files changed, 571 insertions(+), 38 deletions(-))

modular-magician · 2023-03-27T19:45:03Z

Tests analytics

Total tests: 2566
Passed tests 2288
Skipped tests: 267
Affected tests: 11

Action taken

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

TestAccNetworkServicesGateway_update|TestAccFirebaseWebApp_firebaseWebAppFull|TestAccFirebaseWebApp_firebaseWebAppBasicExample|TestAccComputeInstanceFromRegionTemplate_basic|TestAccFrameworkProviderBasePath_setBasePath|TestAccFrameworkProviderMeta_setModuleName|TestAccDataSourceGoogleFirebaseAppleAppConfig|TestAccDataSourceDnsManagedZone_basic|TestAccDataSourceDNSKeys_noDnsSec|TestAccDataSourceDnsRecordSet_basic|TestAccDataSourceDNSKeys_basic

Get to know how VCR tests work

modular-magician · 2023-03-27T19:49:05Z

Tests passed during RECORDING mode:
TestAccNetworkServicesGateway_update[Debug log]
TestAccFirebaseWebApp_firebaseWebAppFull[Debug log]
TestAccFirebaseWebApp_firebaseWebAppBasicExample[Debug log]
TestAccComputeInstanceFromRegionTemplate_basic[Debug log]
TestAccFrameworkProviderBasePath_setBasePath[Debug log]
TestAccFrameworkProviderMeta_setModuleName[Debug log]
TestAccDataSourceGoogleFirebaseAppleAppConfig[Debug log]
TestAccDataSourceDnsManagedZone_basic[Debug log]
TestAccDataSourceDNSKeys_noDnsSec[Debug log]
TestAccDataSourceDnsRecordSet_basic[Debug log]
TestAccDataSourceDNSKeys_basic[Debug log]

All tests passed
View the build log or the debug log for each test

modular-magician · 2023-03-28T15:30:38Z

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 ( 1 file changed, 5 insertions(+), 6 deletions(-))
Terraform Beta: Diff ( 8 files changed, 332 insertions(+), 151 deletions(-))
TF Validator: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

modular-magician · 2023-03-28T16:21:42Z

Tests analytics

Total tests: 2568
Passed tests 2287
Skipped tests: 267
Affected tests: 14

Action taken

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

TestAccFirebaserulesRelease_BasicRelease|TestAccFirebaseWebApp_firebaseWebAppFull|TestAccFirebaseWebApp_firebaseWebAppBasicExample|TestAccComposerEnvironment_withEncryptionConfigComposer2|TestAccComputeInstanceFromRegionTemplate_basic|TestAccComposerEnvironment_withEncryptionConfigComposer1|TestAccNetworkServicesGateway_update|TestAccFrameworkProviderMeta_setModuleName|TestAccFrameworkProviderBasePath_setBasePath|TestAccDataSourceDnsManagedZone_basic|TestAccDataSourceDnsRecordSet_basic|TestAccDataSourceDNSKeys_noDnsSec|TestAccDataSourceDNSKeys_basic|TestAccDataSourceGoogleFirebaseAppleAppConfig

Get to know how VCR tests work

modular-magician · 2023-03-28T16:53:55Z

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 ( 1 file changed, 3 insertions(+), 3 deletions(-))
Terraform Beta: Diff ( 6 files changed, 190 insertions(+), 99 deletions(-))
TF Validator: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

modular-magician · 2023-03-28T17:45:05Z

Tests analytics

Total tests: 2569
Passed tests 2288
Skipped tests: 267
Affected tests: 14

Action taken

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

TestAccFirebaseWebApp_firebaseWebAppFull|TestAccFirebaseWebApp_firebaseWebAppBasicExample|TestAccNetworkServicesGateway_update|TestAccComputeInstanceFromRegionTemplate_basic|TestAccComposerEnvironment_withEncryptionConfigComposer2|TestAccComposerEnvironment_withEncryptionConfigComposer1|TestAccComputeForwardingRule_update|TestAccFrameworkProviderBasePath_setBasePath|TestAccFrameworkProviderMeta_setModuleName|TestAccDataSourceGoogleFirebaseAppleAppConfig|TestAccDataSourceDnsManagedZone_basic|TestAccDataSourceDNSKeys_noDnsSec|TestAccDataSourceDnsRecordSet_basic|TestAccDataSourceDNSKeys_basic

Get to know how VCR tests work

modular-magician · 2023-03-28T18:16:35Z

Tests passed during RECORDING mode:
TestAccFirebaseWebApp_firebaseWebAppFull[Debug log]
TestAccFirebaseWebApp_firebaseWebAppBasicExample[Debug log]
TestAccNetworkServicesGateway_update[Debug log]
TestAccComputeInstanceFromRegionTemplate_basic[Debug log]
TestAccComputeForwardingRule_update[Debug log]
TestAccFrameworkProviderBasePath_setBasePath[Debug log]
TestAccFrameworkProviderMeta_setModuleName[Debug log]
TestAccDataSourceGoogleFirebaseAppleAppConfig[Debug log]
TestAccDataSourceDnsManagedZone_basic[Debug log]
TestAccDataSourceDNSKeys_noDnsSec[Debug log]
TestAccDataSourceDnsRecordSet_basic[Debug log]
TestAccDataSourceDNSKeys_basic[Debug log]

Tests failed during RECORDING mode:
TestAccComposerEnvironment_withEncryptionConfigComposer2[Error message] [Debug log]
TestAccComposerEnvironment_withEncryptionConfigComposer1[Error message] [Debug log]

Please fix these to complete your PR
View the build log or the debug log for each test

mmv1/third_party/terraform/data_sources/data_source_google_firebase_web_app_config.go.erb

rileykarson · 2023-03-29T16:41:27Z

mmv1/third_party/terraform/data_sources/data_source_google_firebase_web_app_config.go.erb

-	if err != nil {
-		return err
+	// Read Provider meta into the meta model
+	resp.Diagnostics.Append(req.ProviderMeta.Get(ctx, &metaData)...)


thinking out loud, not necessarily actionable: I've been finding the resp.Diagnostics.Append(doStuff()) pattern hard to read- I keep focusing on the part where we're writing to diagnostics, rather than the action we're taking . Did you find it grew on you? That was the case with errors in Go for me, initially, so it may just be that this looks different than I'm used to.

I've been wondering if a pattern like the following would be more clear to folks off the bat:

diag := req.ProviderMeta.Get(ctx, &metaData) resp.Diagnostics.Append(diag...) if resp.Diagnostics.HasError() { return }

Even better (imo) would be if there was a function in the framework like "IsEmpty()" or "HasAny()", allowing the control flow to be more clear:

diag := req.ProviderMeta.Get(ctx, &metaData) if !diag.IsEmpty() { resp.Diagnostics.Append(diag...) if resp.Diagnostics.HasError() { return } }

Interesting. Your first example is actually the pattern that I got used to, but I guess that was always in cases where diags was returned alongside another value. Often times cases like this particular one I mimicked/copied from the plugin-framework examples, but I think the only cases I ran into it were here, when reading the config, and when saving state (all which I got into the copy/paste habit of).

As for the second example, I don't believe they have any of those methods currently available. There are WarningsCount and ErrorsCount available that we could use.

I have no strong preference, so if you do I can update them and set that consistency.

I don't have strong enough feelings to say we need to make this change, I'm just overthinking this stuff now to try and preempt needing to change it later!

As for the second example, I don't believe they have any of those methods currently available

Yeah, they're not. They'd be trivial to PR in, I think!

This reverts commit 06f9b2e.

…ovider

…ters

megan07 · 2023-03-30T16:19:29Z

I've added in the suggested changes to the other plugin-framework data sources as well for consistency

modular-magician · 2023-03-30T16:34:17Z

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 ( 4 files changed, 36 insertions(+), 26 deletions(-))
Terraform Beta: Diff ( 9 files changed, 235 insertions(+), 128 deletions(-))
TF Validator: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

modular-magician · 2023-03-30T17:26:15Z

Tests analytics

Total tests: 2582
Passed tests 2296
Skipped tests: 267
Affected tests: 19

Action taken

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

TestAccFirebaseWebApp_firebaseWebAppFull|TestAccFirebaseWebApp_firebaseWebAppBasicExample|TestAccDNSResponsePolicy_update|TestAccContainerCluster_withFlexiblePodCIDR|TestAccFrameworkProviderMeta_setModuleName|TestAccContainerCluster_withPrivateClusterConfigBasic|TestAccFrameworkProviderBasePath_setBasePath|TestAccContainerNodePool_nodeLocations|TestAccContainerNodePool_maxPodsPerNode|TestAccDataSourceGoogleFirebaseAppleAppConfig|TestAccComposerEnvironment_withWebServerConfig|TestAccComposerEnvironment_withEncryptionConfigComposer1|TestAccComposerEnvironment_UpdateComposerV2WithTriggerer|TestAccApigeeKeystoresAliasesKeyCertFile_apigeeKeystoresAliasesKeyCertFileTestExample|TestAccDataSourceDnsRecordSet_basic|TestAccDataSourceDnsManagedZone_basic|TestAccDataSourceDNSKeys_basic|TestAccDataSourceDNSKeys_noDnsSec|TestAccWorkstationsWorkstationCluster_workstationClusterPrivateExample

Get to know how VCR tests work

modular-magician · 2023-03-30T18:20:59Z

Tests passed during RECORDING mode:
TestAccFirebaseWebApp_firebaseWebAppFull[Debug log]
TestAccFirebaseWebApp_firebaseWebAppBasicExample[Debug log]
TestAccDNSResponsePolicy_update[Debug log]
TestAccContainerCluster_withFlexiblePodCIDR[Debug log]
TestAccFrameworkProviderMeta_setModuleName[Debug log]
TestAccContainerCluster_withPrivateClusterConfigBasic[Debug log]
TestAccFrameworkProviderBasePath_setBasePath[Debug log]
TestAccContainerNodePool_maxPodsPerNode[Debug log]
TestAccDataSourceGoogleFirebaseAppleAppConfig[Debug log]
TestAccComposerEnvironment_withEncryptionConfigComposer1[Debug log]
TestAccApigeeKeystoresAliasesKeyCertFile_apigeeKeystoresAliasesKeyCertFileTestExample[Debug log]
TestAccDataSourceDnsRecordSet_basic[Debug log]
TestAccDataSourceDnsManagedZone_basic[Debug log]
TestAccDataSourceDNSKeys_basic[Debug log]
TestAccDataSourceDNSKeys_noDnsSec[Debug log]
TestAccWorkstationsWorkstationCluster_workstationClusterPrivateExample[Debug log]

Tests failed during RECORDING mode:
TestAccContainerNodePool_nodeLocations[Error message] [Debug log]
TestAccComposerEnvironment_withWebServerConfig[Error message] [Debug log]
TestAccComposerEnvironment_UpdateComposerV2WithTriggerer[Error message] [Debug log]

Please fix these to complete your PR
View the build log or the debug log for each test

…form#7501) * Revert "revert plugin framework code (GoogleCloudPlatform#7287)" This reverts commit 06f9b2e. * changes to plugin-framework work for parity * add some provider tests fix other inconsistent errors with the sdk provider * update provider alias name * other changes * fix up base path test * update tests to use muxer * use beta provider when we need to for some of the provider tests * remove all the specific providers from the config * attempt to fix vcr tests * refactor the vcr configuration * capitalize provider factories * some test failure fixes * remove skip step if vcr * review comments * fixed pointer errors * move providerversion450 to be dependent on resource, revers diag pointers * reverse another pointer * update firebase apple app config datasource to plugin-framework * update with latest plugin-framework changes * Revert "revert plugin framework code (GoogleCloudPlatform#7287)" This reverts commit 06f9b2e. * changes to plugin-framework work for parity * add some provider tests fix other inconsistent errors with the sdk provider * update tests to use muxer * use beta provider when we need to for some of the provider tests * remove all the specific providers from the config * changes after merge * attempt to fix vcr tests * refactor the vcr configuration * capitalize provider factories * some test failure fixes * remove skip step if vcr * review comments * fixed pointer errors * move providerversion450 to be dependent on resource, revers diag pointers * update firebase apple app config datasource to plugin-framework * update web app config * update external provider version tests * rm non erb, bad merge maybe * update after a bad merge * add comments * review comments, applied them to dns data sources as well

megan07 force-pushed the megan_fb_web_app_config branch 2 times, most recently from 2a21100 to b88fcd5 Compare March 24, 2023 15:15

megan07 force-pushed the megan_fb_web_app_config branch from b88fcd5 to 8565cf9 Compare March 27, 2023 18:36

megan07 force-pushed the megan_fb_web_app_config branch from 8565cf9 to e74a22c Compare March 28, 2023 15:14

megan07 force-pushed the megan_fb_web_app_config branch from e74a22c to 69c8ecd Compare March 28, 2023 16:26

megan07 marked this pull request as ready for review March 28, 2023 18:44

megan07 requested a review from a team as a code owner March 28, 2023 18:44

megan07 requested review from zli82016 and rileykarson and removed request for a team and zli82016 March 28, 2023 18:44

rileykarson approved these changes Mar 29, 2023

View reviewed changes

megan07 added 7 commits March 30, 2023 10:46

Revert "revert plugin framework code (GoogleCloudPlatform#7287)"

a09baab

This reverts commit 06f9b2e.

changes to plugin-framework work for parity

3854629

add some provider tests fix other inconsistent errors with the sdk pr…

2a80a8c

…ovider

update provider alias name

632b048

other changes

9316660

fix up base path test

33015fb

update tests to use muxer

1f3e5cf

megan07 added 18 commits March 30, 2023 10:46

use beta provider when we need to for some of the provider tests

4d76414

remove all the specific providers from the config

f74060a

changes after merge

8b825bb

attempt to fix vcr tests

cb2404e

refactor the vcr configuration

ff300ae

capitalize provider factories

b19e209

some test failure fixes

4386cd0

remove skip step if vcr

27d8116

review comments

4761c86

fixed pointer errors

4c1d8d3

move providerversion450 to be dependent on resource, revers diag poin…

a99edbd

…ters

update firebase apple app config datasource to plugin-framework

71a86c0

update web app config

86376c7

update external provider version tests

3181fd0

rm non erb, bad merge maybe

2f957e4

update after a bad merge

e3951c0

add comments

c7ae21b

review comments, applied them to dns data sources as well

bd04e46

megan07 force-pushed the megan_fb_web_app_config branch from efb9e78 to bd04e46 Compare March 30, 2023 16:18

megan07 merged commit 88f6847 into GoogleCloudPlatform:main Mar 30, 2023

This was referenced Mar 30, 2023

update web app config datasource to plugin-framework hashicorp/terraform-provider-google#14152

Merged

update web app config datasource to plugin-framework hashicorp/terraform-provider-google-beta#5403

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

update web app config datasource to plugin-framework #7501

update web app config datasource to plugin-framework #7501

megan07 commented Mar 20, 2023

modular-magician commented Mar 24, 2023

modular-magician commented Mar 24, 2023

modular-magician commented Mar 24, 2023

modular-magician commented Mar 27, 2023

modular-magician commented Mar 27, 2023

modular-magician commented Mar 27, 2023

modular-magician commented Mar 28, 2023

modular-magician commented Mar 28, 2023

modular-magician commented Mar 28, 2023

modular-magician commented Mar 28, 2023

modular-magician commented Mar 28, 2023

rileykarson Mar 29, 2023

megan07 Mar 29, 2023

rileykarson Mar 29, 2023

megan07 commented Mar 30, 2023

modular-magician commented Mar 30, 2023

modular-magician commented Mar 30, 2023

modular-magician commented Mar 30, 2023

update web app config datasource to plugin-framework #7501

update web app config datasource to plugin-framework #7501

Conversation

megan07 commented Mar 20, 2023

modular-magician commented Mar 24, 2023

Diff report

modular-magician commented Mar 24, 2023

Tests analytics

Action taken

modular-magician commented Mar 24, 2023

modular-magician commented Mar 27, 2023

Diff report

modular-magician commented Mar 27, 2023

Tests analytics

Action taken

modular-magician commented Mar 27, 2023

modular-magician commented Mar 28, 2023

Diff report

modular-magician commented Mar 28, 2023

Tests analytics

Action taken

modular-magician commented Mar 28, 2023

Diff report

modular-magician commented Mar 28, 2023

Tests analytics

Action taken

modular-magician commented Mar 28, 2023

rileykarson Mar 29, 2023

Choose a reason for hiding this comment

megan07 Mar 29, 2023

Choose a reason for hiding this comment

rileykarson Mar 29, 2023

Choose a reason for hiding this comment

megan07 commented Mar 30, 2023

modular-magician commented Mar 30, 2023

Diff report

modular-magician commented Mar 30, 2023

Tests analytics

Action taken

modular-magician commented Mar 30, 2023