Merge branch 'GoogleCloudPlatform:main' into gcip_config

.ci/containers/gcb-terraform-vcr-tester/test_terraform_vcr.sh

@@ -98,7 +98,7 @@ while [[ -n $TESTS_TERMINATED ]]; do
   gsutil -h "Content-Type:text/plain" -q cp replaying_test$test_suffix.log gs://ci-vcr-logs/beta/refs/heads/auto-pr-$pr_number/artifacts/$build_id/build-log/
 
   if [[ $counter -gt 3 ]]; then
-    comment="Failed to run VCR tests in REPLAYING mode${NEWLINE}"
+    comment="$\textcolor{red}{\textsf{Failed to run VCR tests in REPLAYING mode}}$ ${NEWLINE}"
     comment+="View the [build log](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-$pr_number/artifacts/$build_id/build-log/replaying_test$test_suffix.log)${NEWLINE}"
     comment+="If you believe the error is unrelated to your PR, please rerun the tests"
     add_comment "${comment}"
@@ -128,8 +128,8 @@ gsutil -h "Content-Type:text/plain" -m -q cp testlog/replaying/* gs://ci-vcr-log
 TESTS_PANIC=$(grep "^panic: " replaying_test$test_suffix.log)
 
 if [[ -n $TESTS_PANIC ]]; then
-  comment="The provider crashed while running the VCR tests in REPLAYING mode${NEWLINE}"
-  comment+="Please fix it to complete your PR${NEWLINE}"
+  comment="$\textcolor{red}{\textsf{The provider crashed while running the VCR tests in REPLAYING mode}}$ ${NEWLINE}"
+  comment+="$\textcolor{red}{\textsf{Please fix it to complete your PR}}$ ${NEWLINE}"
   comment+="View the [build log](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-$pr_number/artifacts/$build_id/build-log/replaying_test$test_suffix.log)"
   add_comment "${comment}"
   update_status "failure"
@@ -181,7 +181,7 @@ if [[ -n $FAILED_TESTS_PATTERN ]]; then
   export VCR_MODE=RECORDING
   FAILED_TESTS=$(grep "^--- FAIL: TestAcc" replaying_test$test_suffix.log | awk '{print $3}')
   # test_exit_code=0
-  parallel --jobs 16 TF_LOG=DEBUG TF_LOG_PATH_MASK=$local_path/testlog/recording/%s.log TF_ACC=1 TF_SCHEMA_PANIC_ON_ERROR=1 go test $GOOGLE_TEST_DIRECTORY -parallel 1 -v -run="{}$" -timeout 240m -ldflags="-X=github.com/hashicorp/terraform-provider-google-beta/version.ProviderVersion=acc" ">" testlog/recording_build/{}_recording_test.log ::: $FAILED_TESTS
+  parallel --jobs 16 TF_LOG=DEBUG TF_LOG_PATH_MASK=$local_path/testlog/recording/%s.log TF_ACC=1 TF_SCHEMA_PANIC_ON_ERROR=1 go test {1} -parallel 1 -v -run="{2}$" -timeout 240m -ldflags="-X=github.com/hashicorp/terraform-provider-google-beta/version.ProviderVersion=acc" ">>" testlog/recording_build/{2}_recording_test.log ::: $GOOGLE_TEST_DIRECTORY ::: $FAILED_TESTS
 
   test_exit_code=$?
 
@@ -208,8 +208,9 @@ if [[ -n $FAILED_TESTS_PATTERN ]]; then
   RECORDING_TESTS_PANIC=$(grep "^panic: " recording_test.log)
 
   if [[ -n $RECORDING_TESTS_PANIC ]]; then
-    comment="The provider crashed while running the VCR tests in RECORDING mode${NEWLINE}"
-    comment+="Please fix it to complete your PR${NEWLINE}"
+
+    comment="$\textcolor{red}{\textsf{The provider crashed while running the VCR tests in RECORDING mode}}$ ${NEWLINE}"
+    comment+="$\textcolor{red}{\textsf{Please fix it to complete your PR}}$ ${NEWLINE}"
     comment+="View the [build log](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-$pr_number/artifacts/$build_id/build-log/recording_test.log)"
     add_comment "${comment}"
     update_status "failure"
@@ -225,16 +226,16 @@ if [[ -n $FAILED_TESTS_PATTERN ]]; then
   RECORDING_PASSED_TESTS_COUNT=0
   RECORDING_FAILED_TESTS_COUNT=0
   if [[ -n $RECORDING_PASSED_TESTS ]]; then
-    comment+="Tests passed during RECORDING mode:${NEWLINE} $RECORDING_PASSED_TESTS ${NEWLINE}${NEWLINE}"
+    comment+="$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$ ${NEWLINE} $RECORDING_PASSED_TESTS ${NEWLINE}${NEWLINE}"
     RECORDING_PASSED_TESTS_COUNT=$(echo "$RECORDING_PASSED_TESTS" | wc -l)
     comment+="##### Rerun these tests in REPLAYING mode to catch issues ${NEWLINE}${NEWLINE}"
 
     # Rerun passed tests in REPLAYING mode 3 times to catch issues
     export VCR_MODE=REPLAYING
     count=3
-    parallel --jobs 16 TF_LOG=DEBUG TF_LOG_PATH_MASK=$local_path/testlog/replaying_after_recording/%s.log TF_ACC=1 TF_SCHEMA_PANIC_ON_ERROR=1 go test $GOOGLE_TEST_DIRECTORY -parallel 1 -count=$count -v -run="{}$" -timeout 120m -ldflags="-X=github.com/hashicorp/terraform-provider-google-beta/version.ProviderVersion=acc" ">" testlog/replaying_build_after_recording/{}_replaying_test.log ::: $RECORDING_PASSED_TEST_LIST
+    parallel --jobs 16 TF_LOG=DEBUG TF_LOG_PATH_MASK=$local_path/testlog/replaying_after_recording/%s.log TF_ACC=1 TF_SCHEMA_PANIC_ON_ERROR=1 go test {1} -parallel 1 -count=$count -v -run="{2}$" -timeout 120m -ldflags="-X=github.com/hashicorp/terraform-provider-google-beta/version.ProviderVersion=acc" ">>" testlog/replaying_build_after_recording/{2}_replaying_test.log ::: $GOOGLE_TEST_DIRECTORY ::: $RECORDING_PASSED_TEST_LIST
 
-    test_exit_code=$?
+    test_exit_code=$(($test_exit_code || $?))
 
     # Concatenate recording build logs to one file
     for test in $RECORDING_PASSED_TEST_LIST
@@ -250,11 +251,11 @@ if [[ -n $FAILED_TESTS_PATTERN ]]; then
 
     REPLAYING_FAILED_TESTS=$(grep "^--- FAIL: TestAcc" replaying_build_after_recording.log | sort -u -t' ' -k3,3 | awk -v pr_number=$pr_number -v build_id=$build_id '{print "`"$3"`[[Error message](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-"pr_number"/artifacts/"build_id"/build-log/replaying_build_after_recording/"$3"_replaying_test.log)] [[Debug log](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-"pr_number"/artifacts/"build_id"/replaying_after_recording/"$3".log)]"}')
     if [[ -n $REPLAYING_FAILED_TESTS ]]; then
-      comment+="Tests failed when rerunning REPLAYING mode:${NEWLINE} $REPLAYING_FAILED_TESTS ${NEWLINE}${NEWLINE}"
+      comment+="$\textcolor{red}{\textsf{Tests failed when rerunning REPLAYING mode:}}$ ${NEWLINE} $REPLAYING_FAILED_TESTS ${NEWLINE}${NEWLINE}"
       comment+="Tests failed due to non-determinism or randomness when the VCR replayed the response after the HTTP request was made.${NEWLINE}${NEWLINE}"
-      comment+="Please fix these to complete your PR. If you do not know how VCR tests work, please work with the code reviewer to figure out the reason why the tests failed and fix the tests.${NEWLINE}"
+      comment+="Please fix these to complete your PR. If you believe these test failures to be incorrect or unrelated to your change, or if you have any questions, please raise the concern with your reviewer.${NEWLINE}"
     else
-      comment+="All tests passed after rerunning REPLAYING mode.${NEWLINE}"
+      comment+="$\textcolor{green}{\textsf{No issues found for passed tests after REPLAYING rerun.}}$ ${NEWLINE}"
     fi
     comment+="${NEWLINE}---${NEWLINE}"
 
@@ -264,18 +265,20 @@ if [[ -n $FAILED_TESTS_PATTERN ]]; then
   fi
 
   if [[ -n $RECORDING_FAILED_TESTS ]]; then
-    comment+="Tests failed during RECORDING mode:${NEWLINE} $RECORDING_FAILED_TESTS ${NEWLINE}${NEWLINE}"
+    comment+="$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$ ${NEWLINE} $RECORDING_FAILED_TESTS ${NEWLINE}${NEWLINE}"
     RECORDING_FAILED_TESTS_COUNT=$(echo "$RECORDING_FAILED_TESTS" | wc -l)
     if [[ $RECORDING_PASSED_TESTS_COUNT+$RECORDING_FAILED_TESTS_COUNT -lt $FAILED_TESTS_COUNT ]]; then
-      comment+="Several tests got terminated during RECORDING mode.${NEWLINE}"
+      comment+="$\textcolor{red}{\textsf{Several tests got terminated during RECORDING mode.}}$ ${NEWLINE}"
     fi
-    comment+="Please fix these to complete your PR${NEWLINE}"
+    comment+="$\textcolor{red}{\textsf{Please fix these to complete your PR.}}$ ${NEWLINE}"
   else
     if [[ $RECORDING_PASSED_TESTS_COUNT+$RECORDING_FAILED_TESTS_COUNT -lt $FAILED_TESTS_COUNT ]]; then
-      comment+="Several tests got terminated during RECORDING mode.${NEWLINE}"
+      comment+="$\textcolor{red}{\textsf{Several tests got terminated during RECORDING mode.}}$ ${NEWLINE}"
     elif [[ $test_exit_code -ne 0 ]]; then
       # check for any uncaught errors in RECORDING mode
-      comment+="Errors occurred during RECORDING mode. Please fix them to complete your PR${NEWLINE}"
+      comment+="$\textcolor{red}{\textsf{Errors occurred during RECORDING mode. Please fix them to complete your PR.}}$ ${NEWLINE}"
+    else
+      comment+="$\textcolor{green}{\textsf{All tests passed!}} ${NEWLINE}"
     fi
   fi
 
@@ -285,9 +288,9 @@ if [[ -n $FAILED_TESTS_PATTERN ]]; then
 else
   if [[ $test_exit_code -ne 0 ]]; then
     # check for any uncaught errors errors in REPLAYING mode
-    comment+="Errors occurred during REPLAYING mode. Please fix them to complete your PR${NEWLINE}"
+    comment+="$\textcolor{red}{\textsf{Errors occurred during REPLAYING mode. Please fix them to complete your PR}}$ ${NEWLINE}"
   else
-    comment+="All tests passed in REPLAYING mode${NEWLINE}"
+    comment+="$\textcolor{green}{\textsf{All tests passed in REPLAYING mode.}}$ ${NEWLINE}"
   fi
   comment+="View the [build log](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-$pr_number/artifacts/$build_id/build-log/replaying_test$test_suffix.log)"
   add_comment "${comment}"

docs/content/contribute/review-pr.md

@@ -0,0 +1,45 @@
+---
+title: "Review a PR"
+weight: 11
+---
+
+# Review a PR
+
+This page provides guidelines for reviewing Magic Modules pull requests
+
+1. Read the PR description to understand the context and ensure the PR either
+   * is linked to a GitHub issue or an internal bug
+      * if not, check the [issue tracker](https://github.com/hashicorp/terraform-provider-google/issues) to see whether the feature has already been requested and add the issues in the description, if any.
+   * establishes clear context itself via title or description.
+2. If the PR adds any new resource, ensure that the resource does not already exists in the [GA provider](https://github.com/hashicorp/terraform-provider-google) or [beta provider](https://github.com/hashicorp/terraform-provider-google-beta)
+1. Read through all the changes in the PR, generated code in the downstreams and the API documentation to ensure that:
+   1. the resource schema added in the PR matches the API structure.
+   1. the features are added in the correct version
+      * features only available in beta are not included in the GA google provider.
+      * features added to the GA provider are also included in the beta provider -- beta should be a strict superset of GA.
+   1. no [breaking changes]({{< ref "/develop/breaking-changes" >}}) are introduced without a valid justification.
+   1. verify the change actually resolves the linked issues, if any.
+1. Check the tests added/modified to ensure that:
+   1. all fields added/updated in the PR appear in at least one test.
+      * It is advisable to test updating from a non-zero value to a zero value if feasible.
+   1. all mutable fields are tested in at least one update test.
+   1. all related tests pass in GA for features promoted from beta to GA.
+      {{< hint info >}}Note:
+      Presubmit VCR tests do not run in GA. Manual testing is required for promoted GA features.
+      {{< /hint >}}
+   1. newly added or modified diff suppress functions are tested in at least one unit test.
+   1. the linked issue (if any) is covered by at least one test that reproduces the issue
+      * for example - a bugfix should test the bug (or explain why it's not feasible to do so in the description) and an enhancement should test the new behaviour(s).
+   1. all related PR presubmit tests have been completed successfully, including:
+      * terraform-provider-breaking-change-test
+      * presubmit-rake-tests
+      * terraform-provider-google-build-and-unit-tests
+      * terraform-provider-google-beta-build-and-unit-tests
+      * VCR-test
+      {{< hint info >}}Note:
+      Some acceptance tests may be skipped in VCR and manual testing is required.
+      {{< /hint >}}
+1. Check documentation to ensure
+   1. resouce-level and field-level documentation are generated correctly for MMv1-based resource
+   1. documentation is added manually for handwritten resources.   
+1. Check if release notes capture all changes in the PR, and are correctly formatted following the guidance in [write release notes]({{< ref "release-notes" >}}) before merge the PR.

docs/content/get-started/generate-providers.md

@@ -128,7 +128,7 @@ If you are familiar with Docker or Podman, you may want to use the experimental
 1. Set up application default credentials for Terraform
    ```bash
    gcloud auth application-default login
-   export GOOGLE_USE_DEFAULT_CREDENTIALS=TRUE
+   export GOOGLE_USE_DEFAULT_CREDENTIALS=true
    ```
 1. Set required environment variables
    ```bash

mmv1/api/type.rb

@@ -655,21 +655,18 @@ def validate
         # TODO: (camthornton) product reference may not exist yet
         return if @__resource.__product.nil?
 
-        check_resource_ref_exists
         check_resource_ref_property_exists
       end
 
       def property
         props = resource_ref.all_user_properties
                             .select { |prop| prop.name == @imports }
         return props.first unless props.empty?
-        raise "#{@imports} does not exist on #{@resource}" if props.empty?
       end
 
       def resource_ref
         product = @__resource.__product
         resources = product.objects.select { |obj| obj.name == @resource }
-        raise "Unknown item type '#{@resource}'" if resources.empty?
 
         resources[0]
       end
@@ -683,13 +680,9 @@ def property_class
 
       private
 
-      def check_resource_ref_exists
-        product = @__resource.__product
-        resources = product.objects.select { |obj| obj.name == @resource }
-        raise "Missing '#{@resource}'" if resources.empty?
-      end
-
       def check_resource_ref_property_exists
+        return unless defined?(resource_ref.all_user_properties)
+
         exported_props = resource_ref.all_user_properties
         exported_props << Api::Type::String.new('selfLink') \
           if resource_ref.has_self_link

mmv1/products/bigquery/Dataset.yaml

@@ -356,3 +356,12 @@ properties:
       - 'und:ci': undetermined locale, case insensitive.
       - '': empty string. Default to case-sensitive behavior.
     default_from_api: true
+  - !ruby/object:Api::Type::String
+    name: 'storageBillingModel'
+    description: |
+      Specifies the storage billing model for the dataset.
+      Set this flag value to LOGICAL to use logical bytes for storage billing,
+      or to PHYSICAL to use physical bytes instead.
+
+      LOGICAL is the default if this flag isn't specified.
+    default_from_api: true

mmv1/products/compute/Address.yaml

@@ -106,6 +106,7 @@ parameters:
     required: false
     default_from_api: true
     custom_flatten: 'templates/terraform/custom_flatten/name_from_self_link.erb'
+    custom_expand: 'templates/terraform/custom_expand/resourceref_with_validation.go.erb'
 properties:
   - !ruby/object:Api::Type::String
     name: 'address'
@@ -188,6 +189,7 @@ properties:
       This field can only be used with INTERNAL type with
       GCE_ENDPOINT/DNS_RESOLVER purposes.
     default_from_api: true
+    custom_expand: 'templates/terraform/custom_expand/resourceref_with_validation.go.erb'
   - !ruby/object:Api::Type::Array
     name: 'users'
     description: 'The URLs of the resources that are using this address.'
@@ -216,6 +218,7 @@ properties:
       The URL of the network in which to reserve the address. This field
       can only be used with INTERNAL type with the VPC_PEERING and
       IPSEC_INTERCONNECT purposes.
+    custom_expand: 'templates/terraform/custom_expand/resourceref_with_validation.go.erb'
   - !ruby/object:Api::Type::Integer
     name: prefixLength
     description: |

mmv1/products/compute/Autoscaler.yaml

@@ -85,6 +85,7 @@ parameters:
     required: false
     immutable: true
     default_from_api: true
+    custom_expand: 'templates/terraform/custom_expand/resourceref_with_validation.go.erb'
 properties:
   - !ruby/object:Api::Type::Time
     name: 'creationTimestamp'

mmv1/products/compute/BackendBucketSignedUrlKey.yaml

@@ -75,6 +75,7 @@ parameters:
     required: true
     immutable: true
     ignore_read: true
+    custom_expand: 'templates/terraform/custom_expand/resourceref_with_validation.go.erb'
 properties:
   - !ruby/object:Api::Type::String
     name: 'name'

mmv1/products/compute/BackendService.yaml

@@ -1247,6 +1247,7 @@ properties:
           connections to backends of a service. This resource itself does not affect
           configuration unless it is attached to a backend service resource.
         required: true
+        custom_expand: 'templates/terraform/custom_expand/resourceref_with_validation.go.erb'
       - !ruby/object:Api::Type::Array
         name: 'subjectAltNames'
         description: |

mmv1/products/compute/BackendServiceSignedUrlKey.yaml

@@ -75,6 +75,7 @@ parameters:
     required: true
     immutable: true
     ignore_read: true
+    custom_expand: 'templates/terraform/custom_expand/resourceref_with_validation.go.erb'
 properties:
   - !ruby/object:Api::Type::String
     name: 'name'

mmv1/products/compute/Disk.yaml

@@ -108,6 +108,7 @@ parameters:
     required: false
     default_from_api: true
     custom_flatten: 'templates/terraform/custom_flatten/name_from_self_link.erb'
+    custom_expand: 'templates/terraform/custom_expand/resourceref_with_validation.go.erb'
   - !ruby/object:Api::Type::NestedObject
     name: 'sourceImageEncryptionKey'
     description: |
@@ -218,6 +219,7 @@ parameters:
       * `projects/project/global/snapshots/snapshot`
       * `global/snapshots/snapshot`
       * `snapshot`
+    custom_expand: 'templates/terraform/custom_expand/resourceref_with_validation.go.erb'
   - !ruby/object:Api::Type::NestedObject
     name: 'sourceSnapshotEncryptionKey'
     description: |
@@ -336,6 +338,7 @@ properties:
       resource: 'Instance'
       imports: 'selfLink'
       description: 'A reference to a user of this disk'
+    custom_expand: 'templates/terraform/custom_expand/array_resourceref_with_validation.go.erb'
     output: true
   - !ruby/object:Api::Type::Integer
     name: 'physicalBlockSizeBytes'
@@ -389,6 +392,7 @@ properties:
       create the disk. Provide this when creating the disk.
     custom_flatten: 'templates/terraform/custom_flatten/name_from_self_link.erb'
     default_value: 'pd-standard'
+    custom_expand: 'templates/terraform/custom_expand/resourceref_with_validation.go.erb'
   - !ruby/object:Api::Type::String
     name: 'image'
     api_name: 'sourceImage'
@@ -423,6 +427,7 @@ properties:
       description:
         'A resource policy applied to this disk for automatic snapshot
         creations.'
+    custom_expand: 'templates/terraform/custom_expand/array_resourceref_with_validation.go.erb'
   - !ruby/object:Api::Type::Boolean
     name: 'multiWriter'
     description: |
@@ -433,7 +438,7 @@ properties:
     description: |
       Indicates how many IOPS must be provisioned for the disk.
       Note: Updating currently is only supported by hyperdisk skus without the need to delete and recreate the disk, hyperdisk
-      allows for an update of IOPS every 4 hours. To update your hyperdisk more frequently, you'll need to manually delete and recreate it 
+      allows for an update of IOPS every 4 hours. To update your hyperdisk more frequently, you'll need to manually delete and recreate it
     required: false
     default_from_api: true
     update_verb: :PATCH
@@ -443,7 +448,7 @@ properties:
     description: |
       Indicates how much Throughput must be provisioned for the disk.
       Note: Updating currently is only supported by hyperdisk skus without the need to delete and recreate the disk, hyperdisk
-      allows for an update of Throughput every 4 hours. To update your hyperdisk more frequently, you'll need to manually delete and recreate it 
+      allows for an update of Throughput every 4 hours. To update your hyperdisk more frequently, you'll need to manually delete and recreate it
     default_from_api: true
     update_verb: :PATCH
     update_url: 'projects/{{project}}/zones/{{zone}}/disks/{{name}}?paths=provisionedThroughput'
@@ -492,3 +497,4 @@ properties:
       description: 'An applicable license URI'
       resource: 'License'
       imports: 'selfLink'
+    custom_expand: 'templates/terraform/custom_expand/array_resourceref_with_validation.go.erb'

mmv1/products/compute/DiskType.yaml

@@ -37,6 +37,7 @@ parameters:
     imports: 'name'
     description: 'A reference to the zone where the disk type resides.'
     required: true
+    custom_expand: 'templates/terraform/custom_expand/resourceref_with_validation.go.erb'
 properties:
   - !ruby/object:Api::Type::Time
     name: 'creationTimestamp'