fix TestAccFirebaseWebApp_firebaseWebAppFull

[shuyama1]

fixes hashicorp/terraform-provider-google#16164

Release Note Template for Downstream PRs (will be copied)

[modular-magician]

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 Beta: Diff ( 1 file changed, 1 insertion(+), 1 deletion(-))

[modular-magician]

Tests analytics

Total tests: 3144
Passed tests 2827
Skipped tests: 316
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

TestAccCloudRunService_cloudRunServiceSqlExample

Get to know how VCR tests work

[modular-magician]

$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccCloudRunService_cloudRunServiceSqlExample[Error message] [Debug log]

$\textcolor{red}{\textsf{Please fix these to complete your PR.}}$
View the build log or the debug log for each test

[shuyama1]

/gcbrun

[modular-magician]

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 Beta: Diff ( 1 file changed, 1 insertion(+), 1 deletion(-))

[shuyama1]

test skipped in VCR. Running tests on TC: https://hashicorp.teamcity.com/buildConfiguration/TerraformProviders_GoogleBeta_MmUpstreamTesting_GOOGLEBETA_PACKAGE_FIREBASE/55335?hideTestsFromDependencies=false&hideProblemsFromDependencies=false

[modular-magician]

Tests analytics

Total tests: 3225
Passed tests 2896
Skipped tests: 329
Affected tests: 0

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

[rainshen49]

There is a bug in the API. Just to unblock this test, could you change the two instances ofbrowser_key_restrictions to allowed_referrers = []?

[modular-magician]

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 Beta: Diff ( 1 file changed, 3 insertions(+), 3 deletions(-))

[shuyama1]

new TC run: https://hashicorp.teamcity.com/buildConfiguration/TerraformProviders_GoogleBeta_MmUpstreamTesting_GOOGLEBETA_PACKAGE_FIREBASE/55340

[shuyama1]

new failure is

vcr_utils.go:152: Step 1/4 error: After applying this test step, the plan was not empty.
        stdout:
        Terraform used the selected providers to generate the following execution
        plan. Resource actions are indicated with the following symbols:
          ~ update in-place
        Terraform will perform the following actions:
          # google_apikeys_key.key1 will be updated in-place
          ~ resource "google_apikeys_key" "key1" {
                id           = "projects/ci-test-project-188019/locations/global/keys/tf-test-api-key1csojhfm2gn"
                name         = "tf-test-api-key1csojhfm2gn"
                # (4 unchanged attributes hidden)
              ~ restrictions {
                  + browser_key_restrictions {
                      + allowed_referrers = [
                          + "",
                        ]
                    }
                }
            }
          # google_apikeys_key.key2 will be updated in-place
          ~ resource "google_apikeys_key" "key2" {
                id           = "projects/ci-test-project-188019/locations/global/keys/tf-test-api-key2csojhfm2gn"
                name         = "tf-test-api-key2csojhfm2gn"
                # (4 unchanged attributes hidden)
              ~ restrictions {
                  + browser_key_restrictions {
                      + allowed_referrers = [
                          + "",
                        ]
                    }
                }
            }
        Plan: 0 to add, 2 to change, 0 to destroy.

Looks like the API (google_apikeys_key) doesn't return back the empty list the list with empty strings, nor empty list (tested it locally)

[modular-magician]

Tests analytics

Total tests: 3225
Passed tests 2896
Skipped tests: 329
Affected tests: 0

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

[rainshen49]

That is weird. Since the purpose of this test is just to update the apiKeyId on the WebApp, let's remove the restrictions blocks altogether then. I'll look into restoring this test as part of b/310012020

[modular-magician]

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(+))
Terraform Beta: Diff ( 1 file changed, 1 insertion(+), 13 deletions(-))

[shuyama1]

TC: https://hashicorp.teamcity.com/buildConfiguration/TerraformProviders_GoogleBeta_MmUpstreamTesting_GOOGLEBETA_PACKAGE_FIREBASE/55808?buildTab=tests

update: test passed on TC

[modular-magician]

Tests analytics

Total tests: 3225
Passed tests 2896
Skipped tests: 329
Affected tests: 0

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

[NickElliot]

TestAccFirebaseWebApp_firebaseWebAppCustomApiKeyExample appears to be passing while still having the removed block in this test using the wildcard, do we know why the scenarios are different? I assume it's the b/310012020 issue, but the failure above suggests the error happening before an update step was hit, and if the issue is still present in the provider should the test highlighting it still be temporarily changed?

[rainshen49]

The API only checks the the allowed referrers of the API key when trying to update the WebApp entity. On creation, the API key is accepted as-is, which is why TestAccFirebaseWebApp_firebaseWebAppCustomApiKeyExample passed. Therefore, if the user just follows the example code in TestAccFirebaseWebApp_firebaseWebAppCustomApiKeyExample, terraform apply will succeed.

[NickElliot]

Sorry, I kept editing my above comment so I think the specific thing I'm asking got missed -- the failure posted above seemed to suggest it was a permadiff being hit before the update case was attempted (i.e. step 1/4 plan was not empty), so all that happened was the initial creation, and the only other difference is the presence of the data.google_firebase_web_app_config source, and a second (but currently unreferenced) apikey.

[rainshen49]

There are two separate failures.

1.TestAccFirebaseWebApp_firebaseWebAppFull updates the api_key_id field of google_firebase_web_app with a mismatched allowed_referrers. TestAccFirebaseWebApp_firebaseWebAppCustomApiKeyExample only creates the google_firebase_web_app with a custom API key, but doesn't update, so it doesn't hit the issue.
2. To fix TestAccFirebaseWebApp_firebaseWebAppFull, I suggested using an empty list for allowed_referrers, but it turns out the google_apikeys_key resource doesn't distinguish between empty list and unset browser_key_restrictions, thus the permadiff in #9251 (comment). You're right that the creation of both google_apikeys_keys happens before the creation of the google_firebase_web_app.

Therefore, the only option at the moment is to make an unrestricted API key for testing updating the api_key_id field of google_firebase_web_app.

[NickElliot]

Ah ok I'm seeing now, the perma-diff happened with an empty field as opposed to a wildcard. Since as you said the point of the test here is to verify the ability to update the API key, LGTM