Merge branch 'main' of https://github.com/GoogleCloudPlatform/magic-m…

…odules into pgh

.ci/containers/go-plus/Dockerfile

@@ -1,5 +1,25 @@
-from golang:1.19-bullseye as resource
+# Stage 1: Download go module cache for builds
+FROM golang:1.19-bullseye AS builder
+ENV GOCACHE=/go/cache
+
+RUN apt-get update && apt-get install -y unzip
+WORKDIR /app1
+# Add the source code and build
+ADD "https://github.com/GoogleCloudPlatform/magic-modules/archive/refs/heads/main.zip" source.zip
+RUN unzip source.zip && rm source.zip
+WORKDIR /app1/magic-modules-main/.ci/magician
+# Build the binary (we won't use it in the final image, but it's cached)
+RUN go build -o /dev/null .
+
+# Stage 2: Creating the final image
+FROM golang:1.19-bullseye
 SHELL ["/bin/bash", "-c"]
+ENV GOCACHE=/go/cache
+
+# Copy Go dependencies and Go build cache
+COPY --from=builder /go/pkg/mod /go/pkg/mod
+COPY --from=builder /go/cache /go/cache
+
 # Set up Github SSH cloning.
 RUN ssh-keyscan github.com >> /known_hosts
 RUN echo "UserKnownHostsFile /known_hosts" >> /etc/ssh/ssh_config

.ci/magician/github/membership.go

@@ -27,10 +27,14 @@ var (
 	}
 
 	// This is for new team members who are onboarding
-	trustedContributors = []string{}
+	trustedContributors = []string{
+		"BBBmau",
+	}
 
 	// This is for reviewers who are "on vacation": will not receive new review assignments but will still receive re-requests for assigned PRs.
-	onVacationReviewers = []string{}
+	onVacationReviewers = []string{
+		"hao-nan-li",
+	}
 )
 
 type UserType int64

.ci/scripts/go-plus/magician/exec.sh

@@ -1,20 +1,28 @@
 #!/bin/bash
 
-# Check if there's at least one argument
-if [ "$#" -eq 0 ]; then
-    echo "No arguments provided"
-    exit 1
-fi
-
 # Get the directory of the current script
 DIR="$(dirname $(realpath $0))"
 
-# Construct the path to the Go program
-GO_PROGRAM="$DIR/../../../magician/"
+# Construct the path to the Go program directory and binary
+GO_PROGRAM_DIR="$DIR/../../../magician/"
+GO_BINARY="$GO_PROGRAM_DIR/magician_binary"
 
-pushd $GO_PROGRAM
+pushd $GO_PROGRAM_DIR
 
 set -x
-# Pass all arguments to the child command
-go run . "$@"
+# Check if the binary exists
+if [ ! -f "$GO_BINARY" ]; then
+    # If it doesn't exist, compile the binary
+    echo "Building the magician binary at $GO_BINARY"
+    go build -o "$GO_BINARY"
+fi
+
+# If there are no arguments only compile the binary
+if [ "$#" -eq 0 ]; then
+    echo "No arguments provided"
+    exit 0
+fi
+
+# Run the binary and pass all arguments
+$GO_BINARY "$@"
 set +x

.github/workflows/test-tgc.yml

@@ -25,8 +25,6 @@ on:
       sha:
         description: "The commit SHA in magic-modules repository where the status result will be posted"
         required: true
-      caller_id:
-        description: "Identity of the workflow dispatch caller"
 
 concurrency:
   group: test-tgc-${{ github.event.inputs.owner }}-${{ github.event.inputs.repo }}-${{ github.event.inputs.branch }}
@@ -42,12 +40,19 @@ jobs:
       with:
         repository: ${{ github.event.inputs.owner }}/${{ github.event.inputs.repo }}
         ref: ${{ github.event.inputs.branch }}
-        path: tgc
         fetch-depth: 2
+    - name: Cache Go modules and build cache
+      uses: actions/cache@v3
+      with:
+        path: |
+          ~/go/pkg/mod
+        key: ${{ runner.os }}-test-${{ github.event.inputs.repo }}-${{ hashFiles('go.sum') }}
+        restore-keys: |
+          ${{ runner.os }}-test-${{ github.event.inputs.repo }}-${{ hashFiles('go.sum') }}
+          ${{ runner.os }}-test-${{ github.event.inputs.repo }}-
     - name: Check for Code Changes
       id: pull_request
       run: |
-        cd tgc
         gofiles=$(git diff --name-only HEAD~1 | { grep -e "\.go$" -e "go.mod$" -e "go.sum$" || test $? = 1; })
         if [ -z "$gofiles" ]; then
           echo "has_changes=false" >> $GITHUB_OUTPUT
@@ -80,14 +85,12 @@ jobs:
     - name: Build Terraform Google Conversion
       if: ${{ !failure() && steps.pull_request.outputs.has_changes == 'true' }}
       run: |
-        cd tgc
         go mod edit -replace github.com/hashicorp/terraform-provider-google-beta=github.com/${{ github.event.inputs.owner }}/terraform-provider-google-beta@${{ github.event.inputs.branch }}
         go mod tidy
         make build
     - name: Run Unit Tests
       if: ${{ !failure() && steps.pull_request.outputs.has_changes == 'true' }}
       run: |
-        cd tgc
         make test
     - name: Post Result Status to Pull Request
       if: ${{ !cancelled() }}

.github/workflows/test-tpg.yml

@@ -25,8 +25,6 @@ on:
       sha:
         description: "The commit SHA in magic-modules repository where the status result will be posted"
         required: true
-      caller_id:
-        description: "Identity of the workflow dispatch caller"
 
 concurrency:
   group: test-tpg-${{ github.event.inputs.owner }}-${{ github.event.inputs.repo }}-${{ github.event.inputs.branch }}
@@ -42,12 +40,20 @@ jobs:
       with:
         repository: ${{ github.event.inputs.owner }}/${{ github.event.inputs.repo }}
         ref: ${{ github.event.inputs.branch }}
-        path: provider
         fetch-depth: 2
+    - name: Cache Go modules and build cache
+      uses: actions/cache@v3
+      with:
+        path: |
+          ~/go/pkg/mod
+          ~/.cache/go-build
+        key: ${{ runner.os }}-test-${{ github.event.inputs.repo }}-${{hashFiles('go.sum','google-beta/transport/**','google-beta/tpgresource/**','google-beta/acctest/**','google-beta/envvar/**','google-beta/sweeper/**','google-beta/verify/**') }}
+        restore-keys: |
+          ${{ runner.os }}-test-${{ github.event.inputs.repo }}-${{ hashFiles('go.sum') }}
+          ${{ runner.os }}-test-${{ github.event.inputs.repo }}-
     - name: Check for Code Changes
       id: pull_request
       run: |
-        cd provider
         gofiles=$(git diff --name-only HEAD~1 | { grep -e "\.go$" -e "go.mod$" -e "go.sum$" || test $? = 1; })
         if [ -z "$gofiles" ]; then
           echo "has_changes=false" >> $GITHUB_OUTPUT
@@ -80,22 +86,18 @@ jobs:
     - name: Build Provider
       if: ${{ !failure() && steps.pull_request.outputs.has_changes == 'true' }}
       run: |
-        cd provider
         go build
     - name: Run Unit Tests
       if: ${{ !failure() && steps.pull_request.outputs.has_changes == 'true' }}
       run: |
-        cd provider
-        make testnolint
+        make testnolint TESTARGS="-p 4"
     - name: Lint Check
       if: ${{ !cancelled() && steps.pull_request.outputs.has_changes == 'true' }}
       run: |
-        cd provider
         make lint
     - name: Documentation Check
       if: ${{ !cancelled() && steps.pull_request.outputs.has_changes == 'true' }}
       run: |
-        cd provider
         make docscheck
     - name: Post Result Status to Pull Request
       if: ${{ !cancelled() }}

docs/content/contribute/create-pr.md

@@ -18,14 +18,16 @@ weight: 10
 
 1. A reviewer will automatically be assigned to your PR.
 1. Creating a new pull request or pushing a new commit automatically triggers our CI pipelines and workflows. After CI starts, downstream diff generation takes about 10 minutes; VCR tests can take up to 2 hours. If you are a community contributor, some tests will only run after approval from a reviewer.
+   - While convenient, relying on CI to test iterative changes to PRs often adds extreme latency to reviews if there are errors in test configurations or at runtime. We **strongly** recommend you [test your changes locally before pushing]({{< ref "/develop/run-tests" >}}) even after the initial change.
 1. If your assigned reviewer does not respond to changes on a pull request within two US business days, ping them on the pull request.
 
 {{< hint info >}}
 **TIP:** Speeding up review:
+1. [Test your changes locally before pushing]({{< ref "/develop/run-tests" >}}) to iterate faster.
+   - You can push them and test in parallel as well. New CI runs will preempt old ones where possible.
 1. Resolve failed [status checks](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/collaborating-on-repositories-with-code-quality-features/about-status-checks) quickly
-   - [Run provider tests locally]({{< ref "/develop/run-tests" >}}) to iterate faster
-   - Ask your reviewer for help if you get stuck.
-1. [Self-review your PR]({{< ref "/contribute/review-pr" >}}) or ask someone you know to review
+   - Directly ask your reviewer for help if you don't know how to proceed. If there are failed checks they may only check in if there's no progress after a couple days.
+1. [Self-review your PR]({{< ref "/contribute/review-pr" >}}) or ask someone else familiar with Terraform to review
 {{< /hint >}}
 
 

mmv1/products/alloydb/Cluster.yaml

@@ -99,6 +99,8 @@ examples:
     skip_docs: true
 custom_code: !ruby/object:Provider::Terraform::CustomCode
   pre_create: templates/terraform/pre_create/alloydb_cluster.go.erb
+  pre_update: templates/terraform/pre_update/alloydb_cluster.go.erb
+  pre_delete: templates/terraform/pre_delete/alloydb_cluster.go.erb
 parameters:
   - !ruby/object:Api::Type::String
     name: 'clusterId'
@@ -477,7 +479,6 @@ properties:
       - :PRIMARY
       - :SECONDARY
     default_value: :PRIMARY
-    immutable: true
     description: |
       The type of cluster. If not set, defaults to PRIMARY.
   - !ruby/object:Api::Type::NestedObject
@@ -487,8 +488,18 @@ properties:
     properties:
       - !ruby/object:Api::Type::String
         name: "primaryClusterName"
-        immutable: true
         required: true
         description: |
           Name of the primary cluster must be in the format
           'projects/{project}/locations/{location}/clusters/{cluster_id}'
+virtual_fields:
+  - !ruby/object:Api::Type::Enum
+    name: 'deletion_policy'
+    description: |
+      Policy to determine if the cluster should be deleted forcefully.
+      Deleting a cluster forcefully, deletes the cluster and all its associated instances within the cluster.
+      Deleting a Secondary cluster with a secondary instance REQUIRES setting deletion_policy = "FORCE" otherwise an error is returned. This is needed as there is no support to delete just the secondary instance, and the only way to delete secondary instance is to delete the associated secondary cluster forcefully which also deletes the secondary instance.
+    values:
+      - :DEFAULT
+      - :FORCE
+    default_value: :DEFAULT

mmv1/products/alloydb/Instance.yaml

@@ -13,6 +13,11 @@
 
 --- !ruby/object:Api::Resource
 name: 'Instance'
+docs: !ruby/object:Provider::Terraform::Docs
+  warning: |
+    Deleting an instance with instanceType = SECONDARY does not delete the secondary instance, and abandons it instead.
+    Use deletion_policy = "FORCE" in the associated secondary cluster and delete the cluster forcefully to delete the secondary cluster as well its associated secondary instance.
+    Users can undo the delete secondary instance action by importing the deleted secondary instance by calling terraform import.
 self_link: '{{cluster}}/instances/{{instance_id}}'
 base_url: '{{cluster}}/instances?instanceId={{instance_id}}'
 update_verb: :PATCH
@@ -28,9 +33,9 @@ async: !ruby/object:Api::OpAsync
     base_url: '{{op_id}}'
     wait_ms: 1000
     timeouts: !ruby/object:Api::Timeouts
-      insert_minutes: 40
-      update_minutes: 40
-      delete_minutes: 40
+      insert_minutes: 120
+      update_minutes: 120
+      delete_minutes: 120
   result: !ruby/object:Api::OpAsync::Result
     path: 'response'
   status: !ruby/object:Api::OpAsync::Status
@@ -49,6 +54,8 @@ skip_sweeper: true
 autogen_async: true
 custom_code: !ruby/object:Provider::Terraform::CustomCode
   custom_import: templates/terraform/custom_import/alloydb_instance.go.erb
+  pre_create: templates/terraform/pre_create/alloydb_instance.go.erb
+  pre_delete: templates/terraform/pre_delete/alloydb_instance.go.erb
 examples:
   - !ruby/object:Provider::Terraform::Examples
     name: 'alloydb_instance_basic'
@@ -61,6 +68,19 @@ examples:
       - 'reconciling'
       - 'update_time'
     skip_test: true
+  - !ruby/object:Provider::Terraform::Examples
+    name: 'alloydb_secondary_instance_basic'
+    primary_resource_id: 'secondary'
+    vars:
+      alloydb_primary_cluster_name: 'alloydb-primary-cluster'
+      alloydb_primary_instance_name: 'alloydb-primary-instance'
+      alloydb_secondary_cluster_name: 'alloydb-secondary-cluster'
+      alloydb_secondary_instance_name: 'alloydb-secondary-instance'
+      network_name: 'alloydb-secondary-network'
+    ignore_read_extra:
+      - 'reconciling'
+      - 'update_time'
+    skip_test: true
   - !ruby/object:Provider::Terraform::Examples
     name: 'alloydb_instance_basic_test'
     primary_resource_id: 'default'
@@ -74,6 +94,21 @@ examples:
       - 'reconciling'
       - 'update_time'
     skip_docs: true
+  - !ruby/object:Provider::Terraform::Examples
+    name: 'alloydb_secondary_instance_basic_test'
+    primary_resource_id: 'secondary'
+    vars:
+      alloydb_primary_cluster_name: 'alloydb-primary-cluster'
+      alloydb_primary_instance_name: 'alloydb-primary-instance'
+      alloydb_secondary_cluster_name: 'alloydb-secondary-cluster'
+      alloydb_secondary_instance_name: 'alloydb-secondary-instance'
+      network_name: 'alloydb-secondary-network'
+    test_vars_overrides:
+      network_name: 'acctest.BootstrapSharedServiceNetworkingConnection(t, "alloydbinstance-network-config-1")'
+    ignore_read_extra:
+      - 'reconciling'
+      - 'update_time'
+    skip_docs: true
 parameters:
   - !ruby/object:Api::Type::ResourceRef
     name: 'cluster'
@@ -167,10 +202,17 @@ properties:
     required: true
     immutable: true
     description: |
-      The type of the instance. If the instance type is READ_POOL, provide the associated PRIMARY instance in the `depends_on` meta-data attribute.
+      The type of the instance.
+      If the instance type is READ_POOL, provide the associated PRIMARY/SECONDARY instance in the `depends_on` meta-data attribute.
+      If the instance type is SECONDARY, point to the cluster_type of the associated secondary cluster instead of mentioning SECONDARY.
+      Example: {instance_type = google_alloydb_cluster.<secondary_cluster_name>.cluster_type} instead of {instance_type = SECONDARY}
+      If the instance type is SECONDARY, the terraform delete instance operation does not delete the secondary instance but abandons it instead.
+      Use deletion_policy = "FORCE" in the associated secondary cluster and delete the cluster forcefully to delete the secondary cluster as well its associated secondary instance.
+      Users can undo the delete secondary instance action by importing the deleted secondary instance by calling terraform import.
     values:
       - :PRIMARY
       - :READ_POOL
+      - :SECONDARY
   - !ruby/object:Api::Type::String
     name: 'ipAddress'
     output: true

mmv1/products/apigee/Organization.yaml

@@ -60,6 +60,8 @@ examples:
     test_env_vars:
       org_id: :ORG_ID
       billing_account: :BILLING_ACCT
+    ignore_read_extra:
+      - properties
     skip_docs:
       true
       # Resource creation race
@@ -76,6 +78,8 @@ examples:
     test_env_vars:
       org_id: :ORG_ID
       billing_account: :BILLING_ACCT
+    ignore_read_extra:
+      - properties
     skip_docs:
       true
       # Resource creation race

mmv1/products/bigtable/AppProfile.yaml

@@ -43,6 +43,8 @@ examples:
       deletion_protection: 'false'
     ignore_read_extra:
       - 'ignore_warnings'
+    # bigtable instance does not use the shared HTTP client, this test creates an instance
+    skip_vcr: true
   - !ruby/object:Provider::Terraform::Examples
     name: 'bigtable_app_profile_singlecluster'
     primary_resource_id: 'ap'
@@ -56,6 +58,8 @@ examples:
       deletion_protection: 'false'
     ignore_read_extra:
       - 'ignore_warnings'
+    # bigtable instance does not use the shared HTTP client, this test creates an instance
+    skip_vcr: true
   - !ruby/object:Provider::Terraform::Examples
     name: 'bigtable_app_profile_multicluster'
     primary_resource_id: 'ap'
@@ -69,6 +73,8 @@ examples:
       deletion_protection: 'false'
     ignore_read_extra:
       - 'ignore_warnings'
+    # bigtable instance does not use the shared HTTP client, this test creates an instance
+    skip_vcr: true
 custom_code: !ruby/object:Provider::Terraform::CustomCode
   encoder: templates/terraform/encoders/bigtable_app_profile.go.erb
   extra_schema_entry: templates/terraform/extra_schema_entry/bigtable_app_profile.go.erb