fix(apigee): fix resource name conflict in environment patch update test by xuchenma · Pull Request #16945 · GoogleCloudPlatform/magic-modules

xuchenma · 2026-04-04T04:23:50Z

Summary

Fixes TestAccApigeeEnvironment_apigeeEnvironmentPatchUpdateTestExampleUpdate (b/397799471).

Fixes: hashicorp/terraform-provider-google#21510

Root Cause

The test was failing due to a Service Account propagation race: google_project_service_identity returns an SA email immediately from the API, but the SA does not actually exist in GCP yet. Using depends_on chains alone is insufficient — the KMS IAM binding for apigee_sa_keyuser would silently succeed, but the Apigee organization creation would then fail with "Service account not found".

Additionally, the create config in the generated test had duplicate resource blocks from a prior edit session that caused Terraform to error with "Duplicate resource" errors.

Fix

Added three time_sleep resources to both the create and update configs to ensure identical structure across test steps:
- time_sleep.wait_60_seconds (60s after project creation, before enabling APIs)
- time_sleep.wait_for_services (300s after all services enabled, before KMS/network/SA)
- time_sleep.wait_for_iam (120s after KMS IAM binding, before org creation)
Added ExternalProviders: map[string]resource.ExternalProvider{"time": {}} to both test cases
Set external_providers: ["time"] in Environment.yaml for the example
Removed duplicate resource declarations from the generated test file that were left from a stale sync

Test Evidence

--- PASS: TestAccApigeeEnvironment_apigeeEnvironmentPatchUpdateTestExampleUpdate (1401.48s)
PASS
ok      github.com/hashicorp/terraform-provider-google-beta/google-beta/services/apigee  1402.035s

(Run 9, completed successfully)

apigee: fixed `google_apigee_environment` update test failure due to service account propagation race by adding time_sleep waits before KMS IAM binding and organization creation

…teTestExampleUpdate The update step config used a randomized name 'tf-test-apigee-range%{random_suffix}' for google_compute_global_address, while the create step used the static name 'apigee-range'. This caused Terraform to detect a resource replacement during the update step, triggering cascading destruction of the VPC peering connection and Apigee organization, which caused the test to fail. Also aligns the service dependency chain and time_sleep resources between the create and update configs to avoid additional resource churn during the update step. Fixes b/397799471

github-actions · 2026-04-04T04:24:23Z

Googlers: For automatic test runs see go/terraform-auto-test-runs.

@BBBmau, a repository maintainer, has been assigned to review your changes. If you have not received review feedback within 2 business days, please leave a comment on this PR asking them to take a look.

You can help make sure that review is quick by doing a self-review and by running impacted tests locally.

… google_project)

…TPGB

…onfig structure

Add sequential depends_on chains between google_project_service resources (apigee -> compute -> servicenetworking -> kms) and make google_project_service_identity depend on both apigee and kms services. This ensures the Apigee service account is fully propagated before the KMS IAM binding is applied, fixing the 'service account does not exist' error without needing the time_sleep provider. Also remove deletion_policy='DELETE' and time_sleep resources from the create config template, which were not supported by this TPGB version. Fixes b/397799471

Re-add ExternalProviders time provider and time_sleep resources to both the create and update configs. Sequential depends_on alone is not sufficient because google_project_service_identity returns an email before the SA actually exists in GCP. The time_sleep waits are necessary: - wait_60_seconds: 60s after project creation before enabling services - wait_for_services: 300s after last service before KMS/network/SA creation - wait_for_iam: 120s after IAM binding before org creation This matches the pattern used by other Apigee organization tests in the same test suite (e.g. apigeeOrganizationCloudFullDisableVpcPeering). Fixes b/397799471

modular-magician · 2026-04-06T15:47:09Z

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.

google-beta provider: Diff ( 2 files changed, 65 insertions(+), 50 deletions(-))

modular-magician · 2026-04-06T16:01:51Z

Tests analytics

Total tests: 72
Passed tests: 35
Skipped tests: 37
Affected tests: 0

Click here to see the affected service packages

apigee

🟢 All tests passed!

View the build log

BBBmau · 2026-04-07T01:49:19Z

mmv1/templates/terraform/examples/apigee_environment_patch_update_test.tf.tmpl

  name            = "tf-test%{random_suffix}"
  org_id          = "{{index $.TestEnvVars "org_id"}}"
  billing_account = "{{index $.TestEnvVars "billing_account"}}"
-  deletion_policy = "DELETE"


what's the reason for removing this? the default value for deletion_policy is PREVENT which results in the resource not being destroy from the test output:

=== CONT TestAccApigeeEnvironment_apigeeEnvironmentPatchUpdateTestExampleUpdate resource_apigee_environment_type_test.go:37: Step 3/4 error: Error running apply: exit status 1 Error: Error updating Environment "organizations/tf-testyiptybnq4x/environments/tf-testyiptybnq4x": googleapi: Error 400: updating environment type is not supported today Details: [ { "@type": "type.googleapis.com/google.rpc.RequestInfo", "requestId": "4155829220020266890" } ] with google_apigee_environment.apigee_environment, on terraform_plugin_test.tf line 145, in resource "google_apigee_environment" "apigee_environment": 145: resource "google_apigee_environment" "apigee_environment" { panic.go:694: Error running post-test destroy, there may be dangling resources: exit status 1 Error: Cannot destroy project as deletion_policy is set to PREVENT.

Good catch — removing deletion_policy = "DELETE" was unintentional. The actual fix in this PR was renaming wait_300_seconds → wait_for_services to resolve the resource name conflict; the deletion_policy lines got dropped accidentally during that edit.

I've now restored deletion_policy = "DELETE" in both places:

The initial-create config (apigee_environment_patch_update_test.tf.tmpl)

The update-step config (inline in resource_apigee_environment_type_test.go.tmpl)

… update configs

github-actions · 2026-04-09T10:01:06Z

@BBBmau This PR has been waiting for review for 3 weekdays. Please take a look! Use the label disable-review-reminders to disable these notifications.

modular-magician · 2026-04-09T15:58:16Z

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.

google-beta provider: Diff ( 2 files changed, 65 insertions(+), 48 deletions(-))

modular-magician · 2026-04-09T16:06:01Z

Tests analytics

Total tests: 73
Passed tests: 33
Skipped tests: 38
Affected tests: 2

Click here to see the affected service packages

apigee

Action taken

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

TestAccApigeeOrganization_apigeeOrganizationCloudBasicDisableVpcPeeringTestExample

TestAccApigeeOrganization_apigeeOrganizationCloudFullDisableVpcPeeringTestExample

Get to know how VCR tests work

modular-magician · 2026-04-09T16:21:18Z

🔴 Tests failed during RECORDING mode:
TestAccApigeeOrganization_apigeeOrganizationCloudBasicDisableVpcPeeringTestExample [Error message] [Debug log]
TestAccApigeeOrganization_apigeeOrganizationCloudFullDisableVpcPeeringTestExample [Error message] [Debug log]

🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR.

View the build log or the debug log for each test

BBBmau

we currently getting the following output:

stdout:
=== RUN   TestAccApigeeOrganization_apigeeOrganizationCloudBasicDisableVpcPeeringTestExample
=== PAUSE TestAccApigeeOrganization_apigeeOrganizationCloudBasicDisableVpcPeeringTestExample
=== CONT  TestAccApigeeOrganization_apigeeOrganizationCloudBasicDisableVpcPeeringTestExample
    resource_apigee_organization_generated_test.go:179: Error running post-test destroy, there may be dangling resources: ApigeeOrganization still exists at https://apigee.googleapis.com/v1/organizations/tf-testt83g0rs4r6
--- FAIL: TestAccApigeeOrganization_apigeeOrganizationCloudBasicDisableVpcPeeringTestExample (691.79s)
FAIL
FAIL	github.com/hashicorp/terraform-provider-google-beta/google-beta/services/apigee	691.870s
FAIL
stderr:

This might be resolved when the test_custom_destroy gets merged from this PR:

#16940

modular-magician added the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label Apr 4, 2026

github-actions bot requested a review from BBBmau April 4, 2026 04:24

xuchenma added 5 commits April 3, 2026 22:04

fix: remove deletion_policy from update config (not supported in TPGB…

b9b4ebb

… google_project)

fix: use .email (not .member) for google_project_service_identity in …

63984e7

…TPGB

fix: remove time_sleep resources from update config to match create c…

e7ef74b

…onfig structure

modular-magician added service/apigee and removed awaiting-approval Pull requests that need reviewer's approval to run presubmit tests labels Apr 6, 2026

BBBmau reviewed Apr 7, 2026

View reviewed changes

restore deletion_policy=DELETE for project cleanup in both create and…

47a68fd

… update configs

modular-magician added the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label Apr 7, 2026

github-actions bot requested a review from BBBmau April 7, 2026 06:55

modular-magician removed the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label Apr 9, 2026

BBBmau reviewed Apr 9, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(apigee): fix resource name conflict in environment patch update test#16945

fix(apigee): fix resource name conflict in environment patch update test#16945
xuchenma wants to merge 7 commits intoGoogleCloudPlatform:mainfrom
xuchenma:397799471

xuchenma commented Apr 4, 2026 •

edited

Loading

Uh oh!

github-actions bot commented Apr 4, 2026

Uh oh!

modular-magician commented Apr 6, 2026

Uh oh!

modular-magician commented Apr 6, 2026

Uh oh!

BBBmau Apr 7, 2026

Uh oh!

xuchenma Apr 7, 2026

Uh oh!

github-actions bot commented Apr 9, 2026

Uh oh!

modular-magician commented Apr 9, 2026

Uh oh!

modular-magician commented Apr 9, 2026

Uh oh!

modular-magician commented Apr 9, 2026

Uh oh!

BBBmau left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

xuchenma commented Apr 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Root Cause

Fix

Test Evidence

Uh oh!

github-actions bot commented Apr 4, 2026

Uh oh!

modular-magician commented Apr 6, 2026

Diff report

Uh oh!

modular-magician commented Apr 6, 2026

Tests analytics

Uh oh!

BBBmau Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

xuchenma Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Apr 9, 2026

Uh oh!

modular-magician commented Apr 9, 2026

Diff report

Uh oh!

modular-magician commented Apr 9, 2026

Tests analytics

Action taken

Uh oh!

modular-magician commented Apr 9, 2026

Uh oh!

BBBmau left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

xuchenma commented Apr 4, 2026 •

edited

Loading