Add support for creating Apigee Organization without VPC peering

[baskaran-md]

This pull request fixes hashicorp/terraform-provider-google#15135

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.
  • make test is failing due to known issue. Please check below comment for more info.
• 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).
  • Acceptance tests for Beta has a known failure. Failing test(s): TestAccApigeeOrganization_apigeeOrganization* hashicorp/terraform-provider-google#13274
• Read Write release notes before writing my release note below.

Release Note Template for Downstream PRs (will be copied)

apigee: added `disable_vpc_peering` field to `google_apigee_organization` resource

[modular-magician]

Hello! I am a robot who works on Magic Modules PRs.

I've detected that you're a community contributor. @ScottSuarez, a repository maintainer, has been assigned to assist you and help review your changes.

❓ First time contributing? Click here for more details

---

Your assigned reviewer will help review your code by:

• Ensuring it's backwards compatible, covers common error cases, etc.
• Summarizing the change into a user-facing changelog note.
• Passes tests, either our "VCR" suite, a set of presubmit tests, or with manual test runs.

You can help make sure that review is quick by running local tests and ensuring they're passing in between each push you make to your PR's branch. Also, try to leave a comment with each push you make, as pushes generally don't generate emails.

If your reviewer doesn't get back to you within a week after your most recent change, please feel free to leave a comment on the issue asking them to take a look! In the absence of a dedicated review dashboard most maintainers manage their pending reviews through email, and those will sometimes get lost in their inbox.

---

[baskaran-md]

Unit tests are failing in my environment, even without any changes. However, the acceptance test for my changes are running successfully. Is it some known issue?

Any help to resolve the failures would be appreciated. Thanks.

https://gist.github.com/baskaran-md/5c1815980162e01a8a162741025aefc4

[ScottSuarez]

Unit tests are failing in my environment, even without any changes. However, the acceptance test for my changes are running successfully. Is it some known issue?

Any help to resolve the failures would be appreciated. Thanks.

gist.github.com/baskaran-md/5c1815980162e01a8a162741025aefc4_ (too large to embed)_

Ah, looks like this is an issue on our end! Seems we are not unsetting the env variables in our unit tests. I'll put out a patch soon. Don't worry about the unit tests locally for now. As long as our CI for them is green.

[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 ( 3 files changed, 155 insertions(+))
Terraform Beta: Diff ( 3 files changed, 277 insertions(+))
TF Conversion: Diff ( 3 files changed, 13 insertions(+), 3 deletions(-))

[baskaran-md]

Acceptance Test for GA:

Updated property [core/project].
sh -c "'/usr/local/google/home/deivasigamani/go/src/github.com/hashicorp/terraform-provider-google/scripts/gofmtcheck.sh'"
==> Checking that code complies with gofmt requirements...
go vet
==> Checking that code complies with gofmt requirements...
go vet
TF_ACC=1 TF_SCHEMA_PANIC_ON_ERROR=1 go test ./google -v -run=TestAccApigeeOrganization -timeout 240m -ldflags="-X=github.com/hashicorp/terraform-provider-google/version.ProviderVersion=acc"
=== RUN   TestAccApigeeOrganization_apigeeOrganizationCloudBasicTestExample
=== PAUSE TestAccApigeeOrganization_apigeeOrganizationCloudBasicTestExample
=== RUN   TestAccApigeeOrganization_apigeeOrganizationCloudBasicDisableVpcPeeringTestExample
=== PAUSE TestAccApigeeOrganization_apigeeOrganizationCloudBasicDisableVpcPeeringTestExample
=== CONT  TestAccApigeeOrganization_apigeeOrganizationCloudBasicTestExample
=== CONT  TestAccApigeeOrganization_apigeeOrganizationCloudBasicDisableVpcPeeringTestExample
--- PASS: TestAccApigeeOrganization_apigeeOrganizationCloudBasicDisableVpcPeeringTestExample (335.77s)
--- PASS: TestAccApigeeOrganization_apigeeOrganizationCloudBasicTestExample (699.58s)
PASS
ok  	github.com/hashicorp/terraform-provider-google/google	699.930s

[modular-magician]

Tests analytics

Total tests: 2854
Passed tests 2554
Skipped tests: 298
Affected tests: 2

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

TestAccContainerAwsNodePool_BetaBasicHandWritten|TestAccComputeFirewallPolicyRule_multipleRules

Get to know how VCR tests work

[modular-magician]

$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccContainerAwsNodePool_BetaBasicHandWritten[Error message] [Debug log]
TestAccComputeFirewallPolicyRule_multipleRules[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

[baskaran-md]

Acceptance Test for BETA:

Known issue: hashicorp/terraform-provider-google#13274

Updated property [core/project].
sh -c "'/usr/local/google/home/deivasigamani/go/src/github.com/hashicorp/terraform-provider-google/scripts/gofmtcheck.sh'"
==> Checking that code complies with gofmt requirements...
go vet
==> Checking that code complies with gofmt requirements...
go vet
==> Checking that code complies with gofmt requirements...
go vet
TF_ACC=1 TF_SCHEMA_PANIC_ON_ERROR=1 go test ./google-beta -v -run=TestAccApigeeOrganization -timeout 240m -ldflags="-X=github.com/hashicorp/terraform-provider-google-beta/version.ProviderVersion=acc"
=== RUN   TestAccApigeeOrganization_apigeeOrganizationCloudBasicTestExample
=== PAUSE TestAccApigeeOrganization_apigeeOrganizationCloudBasicTestExample
=== RUN   TestAccApigeeOrganization_apigeeOrganizationCloudBasicDisableVpcPeeringTestExample
=== PAUSE TestAccApigeeOrganization_apigeeOrganizationCloudBasicDisableVpcPeeringTestExample
=== RUN   TestAccApigeeOrganization_apigeeOrganizationCloudFullTestExample
=== PAUSE TestAccApigeeOrganization_apigeeOrganizationCloudFullTestExample
=== RUN   TestAccApigeeOrganization_apigeeOrganizationCloudFullDisableVpcPeeringTestExample
=== PAUSE TestAccApigeeOrganization_apigeeOrganizationCloudFullDisableVpcPeeringTestExample
=== RUN   TestAccApigeeOrganization_apigeeOrganizationRetentionTestExample
=== PAUSE TestAccApigeeOrganization_apigeeOrganizationRetentionTestExample
=== CONT  TestAccApigeeOrganization_apigeeOrganizationCloudBasicTestExample
=== CONT  TestAccApigeeOrganization_apigeeOrganizationCloudFullDisableVpcPeeringTestExample
=== CONT  TestAccApigeeOrganization_apigeeOrganizationCloudFullTestExample
=== CONT  TestAccApigeeOrganization_apigeeOrganizationCloudBasicDisableVpcPeeringTestExample
=== CONT  TestAccApigeeOrganization_apigeeOrganizationRetentionTestExample
--- PASS: TestAccApigeeOrganization_apigeeOrganizationCloudBasicDisableVpcPeeringTestExample (338.44s)
=== NAME  TestAccApigeeOrganization_apigeeOrganizationCloudFullDisableVpcPeeringTestExample
    vcr_utils.go:152: Step 1/2 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_apigee_organization.org will be updated in-place
          ~ resource "google_apigee_organization" "org" {
                id                                   = "organizations/tf-testi2depofvps"
                name                                 = "tf-testi2depofvps"
                # (12 unchanged attributes hidden)

              ~ properties {
                  ~ property {
                      ~ name  = "features.hybrid.enabled" -> "features.mart.connect.enabled"
                        # (1 unchanged attribute hidden)
                    }
                  ~ property {
                      ~ name  = "features.mart.connect.enabled" -> "features.hybrid.enabled"
                        # (1 unchanged attribute hidden)
                    }
                }
            }

        Plan: 0 to add, 1 to change, 0 to destroy.
--- FAIL: TestAccApigeeOrganization_apigeeOrganizationCloudFullDisableVpcPeeringTestExample (411.30s)
--- PASS: TestAccApigeeOrganization_apigeeOrganizationCloudBasicTestExample (701.57s)
--- PASS: TestAccApigeeOrganization_apigeeOrganizationRetentionTestExample (721.58s)
--- PASS: TestAccApigeeOrganization_apigeeOrganizationCloudFullTestExample (721.63s)
FAIL
FAIL	github.com/hashicorp/terraform-provider-google-beta/google-beta	722.016s
FAIL
make: *** [GNUmakefile:15: testacc] Error 1

[ScottSuarez]

We can add a diff DiffSuppressFunc for name field so that the api assigned values would be tolerated. Would you willing to try and make these change? It would be a DiffSuppressFunc and a accompanying unit test on the name field.

[ScottSuarez]

Ah never mind, apologies, this fix would be pretty complex to do since the api is reordering them, not revaluing them.

[ScottSuarez]

Could you try adding unordered_list: true to the properties array?

[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 ( 3 files changed, 155 insertions(+))
Terraform Beta: Diff ( 3 files changed, 277 insertions(+))
TF Conversion: Diff ( 3 files changed, 13 insertions(+), 3 deletions(-))

[modular-magician]

Tests analytics

Total tests: 2855
Passed tests 2550
Skipped tests: 299
Affected tests: 6

Action taken

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

TestAccServiceAccountIamPolicy|TestAccVertexAIIndex_updated|TestAccVertexAIIndex_vertexAiIndexStreamingExample|TestAccVertexAIIndex_vertexAiIndexExample|TestAccContainerAwsNodePool_BetaBasicHandWritten|TestAccComputeFirewallPolicyRule_multipleRules

Get to know how VCR tests work

[modular-magician]

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccServiceAccountIamPolicy[Debug log]
TestAccVertexAIIndex_updated[Debug log]
TestAccVertexAIIndex_vertexAiIndexStreamingExample[Debug log]
TestAccVertexAIIndex_vertexAiIndexExample[Debug log]

Rerun these tests in REPLAYING mode to catch issues

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

---

$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccContainerAwsNodePool_BetaBasicHandWritten[Error message] [Debug log]
TestAccComputeFirewallPolicyRule_multipleRules[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

[baskaran-md]

Thank you @ScottSuarez for the pointer. Adding unordered_list to the Apigee Organization properties is still not fixing the test failures.

Changes made to test:

diff --git a/mmv1/products/apigee/Organization.yaml b/mmv1/products/apigee/Organization.yaml
index e6c0d808b..052ff5c7f 100644
--- a/mmv1/products/apigee/Organization.yaml
+++ b/mmv1/products/apigee/Organization.yaml
@@ -225,10 +225,12 @@ properties:
     name: 'properties'
     description: Properties defined in the Apigee organization profile.
     default_from_api: true
+    unordered_list: true
     properties:
       - !ruby/object:Api::Type::Array
         name: 'property'
         description: List of all properties in the object.
+        unordered_list: true
         item_type: !ruby/object:Api::Type::NestedObject
           properties:
             - !ruby/object:Api::Type::String

[ScottSuarez]

Ah, well thanks for checking !! We can still get this through considering it's a known issue with other tests.

[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 ( 3 files changed, 155 insertions(+))
Terraform Beta: Diff ( 3 files changed, 277 insertions(+))
TF Conversion: Diff ( 3 files changed, 13 insertions(+), 3 deletions(-))

[baskaran-md]

@ScottSuarez Thanks for reviewing. Is there anything needed from my side? If not, when can we expect this to be merged & roll out to production? Any ETA would be great to communicate internally & externally with customers.

[modular-magician]

Tests analytics

Total tests: 2856
Passed tests 2549
Skipped tests: 299
Affected tests: 8

Action taken

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

TestAccContainerAwsNodePool_BetaBasicHandWritten|TestAccComputeFirewallPolicyRule_multipleRules|TestAccClouddeployDeliveryPipeline_VerifyDeliveryPipeline|TestAccClouddeployDeliveryPipeline_DeliveryPipeline|TestAccClouddeployDeliveryPipeline_CanaryDeliveryPipeline|TestAccClouddeployDeliveryPipeline_CanaryServiceNetworkingDeliveryPipeline|TestAccClouddeployDeliveryPipeline_CanaryrunDeliveryPipeline|TestAccClouddeployTarget_Target

Get to know how VCR tests work

[modular-magician]

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccClouddeployDeliveryPipeline_VerifyDeliveryPipeline[Debug log]
TestAccClouddeployDeliveryPipeline_DeliveryPipeline[Debug log]
TestAccClouddeployDeliveryPipeline_CanaryDeliveryPipeline[Debug log]
TestAccClouddeployDeliveryPipeline_CanaryServiceNetworkingDeliveryPipeline[Debug log]
TestAccClouddeployDeliveryPipeline_CanaryrunDeliveryPipeline[Debug log]
TestAccClouddeployTarget_Target[Debug log]

Rerun these tests in REPLAYING mode to catch issues

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

---

$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccContainerAwsNodePool_BetaBasicHandWritten[Error message] [Debug log]
TestAccComputeFirewallPolicyRule_multipleRules[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

[ScottSuarez]

@ScottSuarez Thanks for reviewing. Is there anything needed from my side? If not, when can we expect this to be merged & roll out to production? Any ETA would be great to communicate internally & externally with customers.

Merging now. You can expect it to be rolled out to production in about 1.5w. Likely the 24th release pending no issue.