Add test for updating persistent disk size and disk type in cloud workstations

.ci/infra/terraform/main.tf

@@ -298,6 +298,7 @@ module "project-services" {
     "logging.googleapis.com",
     "looker.googleapis.com",
     "managedidentities.googleapis.com",
+    "managedkafka.googleapis.com",
     "memcache.googleapis.com",
     "memorystore.googleapis.com",
     "metastore.googleapis.com",

.ci/magician/cmd/generate_comment.go

@@ -73,6 +73,10 @@ type diffCommentData struct {
 	Errors          []Errors
 }
 
+type simpleSchemaDiff struct {
+	AddedResources, ModifiedResources, RemovedResources []string
+}
+
 const allowBreakingChangesLabel = "override-breaking-change"
 
 var gcEnvironmentVariables = [...]string{
@@ -204,24 +208,26 @@ func execGenerateComment(prNumber int, ghTokenMagicModules, buildId, buildStep,
 		repo.Branch = newBranch
 		repo.Cloned = true
 		if err := ctlr.Clone(repo); err != nil {
-			fmt.Println("Failed to clone repo at new branch: ", err)
+			fmt.Printf("Failed to clone repo %q at branch %q: %s\n", repo.Name, newBranch, err)
 			errors[repo.Title] = append(errors[repo.Title], "Failed to clone repo at new branch")
 			repo.Cloned = false
 		}
 		if err := ctlr.Fetch(repo, oldBranch); err != nil {
-			fmt.Println("Failed to fetch old branch: ", err)
+			fmt.Printf("Failed to fetch branch %q for repo %q: %s\n", oldBranch, repo.Name, err)
 			errors[repo.Title] = append(errors[repo.Title], "Failed to clone repo at old branch")
 			repo.Cloned = false
 			continue
 		}
 		if repo.Name == "terraform-provider-google-beta" || repo.Name == "terraform-provider-google" {
 			if err := ctlr.Checkout(repo, oldBranch); err != nil {
+				fmt.Printf("Failed to checkout branch %q for repo %q: %s\n", oldBranch, repo.Name, err)
 				errors[repo.Title] = append(errors[repo.Title], fmt.Sprintf("Failed to checkout branch %s", oldBranch))
 				repo.Cloned = false
 				continue
 			}
 			rnr.PushDir(repo.Path)
 			if _, err := rnr.Run("make", []string{"build"}, nil); err != nil {
+				fmt.Printf("Failed to build branch %q for repo %q: %s\n", oldBranch, repo.Name, err)
 				errors[repo.Title] = append(errors[repo.Title], fmt.Sprintf("Failed to build branch %s", oldBranch))
 				repo.Cloned = false
 			}
@@ -303,7 +309,7 @@ func execGenerateComment(prNumber int, ghTokenMagicModules, buildId, buildStep,
 			data.MissingTests = missingTests
 		}
 
-		affectedResources, err := changedSchemaResources(diffProcessorPath, rnr)
+		affectedResources, err := computeAffectedResources(diffProcessorPath, rnr, repo)
 		if err != nil {
 			fmt.Println("computing changed resource schemas: ", err)
 			errors[repo.Title] = append(errors[repo.Title], "The diff processor crashed while computing changed resource schemas.")
@@ -460,27 +466,31 @@ func computeBreakingChanges(diffProcessorPath string, rnr ExecRunner) ([]Breakin
 	return changes, rnr.PopDir()
 }
 
-func changedSchemaResources(diffProcessorPath string, rnr ExecRunner) ([]string, error) {
+func computeAffectedResources(diffProcessorPath string, rnr ExecRunner, repo source.Repo) ([]string, error) {
 	if err := rnr.PushDir(diffProcessorPath); err != nil {
 		return nil, err
 	}
 
-	output, err := rnr.Run("bin/diff-processor", []string{"changed-schema-resources"}, nil)
+	output, err := rnr.Run("bin/diff-processor", []string{"schema-diff"}, nil)
 	if err != nil {
 		return nil, err
 	}
 
-	fmt.Println("Resources with changed schemas: " + output)
+	fmt.Printf("Schema diff for %q: %s\n", repo.Name, output)
 
-	var labels []string
-	if err = json.Unmarshal([]byte(output), &labels); err != nil {
+	var simpleDiff simpleSchemaDiff
+	if err = json.Unmarshal([]byte(output), &simpleDiff); err != nil {
 		return nil, err
 	}
 
 	if err = rnr.PopDir(); err != nil {
 		return nil, err
 	}
-	return labels, nil
+	var resources []string
+	resources = append(resources, simpleDiff.AddedResources...)
+	resources = append(resources, simpleDiff.ModifiedResources...)
+	resources = append(resources, simpleDiff.RemovedResources...)
+	return resources, nil
 }
 
 // Run the missing test detector and return the results.

.ci/magician/cmd/generate_comment_test.go

@@ -89,11 +89,11 @@ func TestExecGenerateComment(t *testing.T) {
 			{"/mock/dir/tfoics", "git", []string{"diff", "origin/auto-pr-123456-old", "origin/auto-pr-123456", "--shortstat"}, map[string]string(nil)},
 			{"/mock/dir/magic-modules/tools/diff-processor", "make", []string{"build"}, diffProcessorEnv},
 			{"/mock/dir/magic-modules/tools/diff-processor", "bin/diff-processor", []string{"breaking-changes"}, map[string]string(nil)},
-			{"/mock/dir/magic-modules/tools/diff-processor", "bin/diff-processor", []string{"changed-schema-resources"}, map[string]string(nil)},
+			{"/mock/dir/magic-modules/tools/diff-processor", "bin/diff-processor", []string{"schema-diff"}, map[string]string(nil)},
 			{"/mock/dir/magic-modules/tools/diff-processor", "make", []string{"build"}, diffProcessorEnv},
 			{"/mock/dir/magic-modules/tools/diff-processor", "bin/diff-processor", []string{"breaking-changes"}, map[string]string(nil)},
 			{"/mock/dir/magic-modules/tools/diff-processor", "bin/diff-processor", []string{"detect-missing-tests", "/mock/dir/tpgb/google-beta/services"}, map[string]string(nil)},
-			{"/mock/dir/magic-modules/tools/diff-processor", "bin/diff-processor", []string{"changed-schema-resources"}, map[string]string(nil)},
+			{"/mock/dir/magic-modules/tools/diff-processor", "bin/diff-processor", []string{"schema-diff"}, map[string]string(nil)},
 		},
 	} {
 		if actualCalls, ok := mr.Calls(method); !ok {

.ci/magician/cmd/mock_runner_test.go

@@ -70,7 +70,7 @@ func NewMockRunner() MockRunner {
 			"/mock/dir/magic-modules/.ci/magician git [clone -b auto-pr-123456 https://modular-magician:*******@github.com/modular-magician/terraform-provider-google-beta /mock/dir/tpgb] map[]": "",
 			"/mock/dir/magic-modules/tools/diff-processor bin/diff-processor [breaking-changes] map[]":                                                                                            "",
 			"/mock/dir/magic-modules/tools/diff-processor make [build] " + sortedEnvString(diffProcessorEnv):                                                                                      "",
-			"/mock/dir/magic-modules/tools/diff-processor bin/diff-processor [changed-schema-resources] map[]":                                                                                    "[\"google_alloydb_instance\"]",
+			"/mock/dir/magic-modules/tools/diff-processor bin/diff-processor [schema-diff] map[]":                                                                                                 "{\"AddedResources\": [\"google_alloydb_instance\"]}",
 			"/mock/dir/magic-modules/tools/diff-processor bin/diff-processor [detect-missing-tests /mock/dir/tpgb/google-beta/services] map[]":                                                    `{"google_folder_access_approval_settings":{"SuggestedTest":"resource \"google_folder_access_approval_settings\" \"primary\" {\n  uncovered_field = # value needed\n}","Tests":["a","b","c"]}}`,
 			"/mock/dir/tgc git [diff origin/auto-pr-123456-old origin/auto-pr-123456 --shortstat] map[]":                                                                                          " 1 file changed, 10 insertions(+)\n",
 			"/mock/dir/tgc git [fetch origin auto-pr-123456-old] map[]":                                                                                                                           "",

.ci/magician/github/membership_data.go

@@ -118,5 +118,7 @@ var (
 	}
 
 	// This is for new team members who are onboarding
-	trustedContributors = map[string]struct{}{}
+	trustedContributors = map[string]struct{}{
+		"bbasata": struct{}{},
+	}
 )

.github/workflows/build-downstream.yml

@@ -9,15 +9,11 @@ on:
         required: true
         type: string
 
-concurrency:
-  group: ${{ inputs.repo  }}-${{ github.event_name == 'pull_request' && format('pr-{0}-', github.event.pull_request.number) || format('commit-{0}', github.sha) }}
-  cancel-in-progress: true
-
 jobs:
   generate-repository:
     runs-on: ubuntu-22.04
     env:
-      BASE_BRANCH: ${{ github.event.pull_request.base.ref || github.ref_name }}
+      BASE_BRANCH: ${{ github.event_name == 'merge_group' && github.event.merge_group.base_ref || github.event.pull_request.base.ref || github.ref_name }}
       GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
     steps:
       - name: Checkout Repository
@@ -32,7 +28,7 @@ jobs:
 
       # Cache Go modules
       - name: Cache Go modules
-        uses: actions/cache@0c45773b623bea8c8e75f6c82b208c3cf94ea4f9 # v4.0.2
+        uses: actions/cache@1bd1e32a3bdc45362d1e726936510720a7c30a57 # v4.2.0
         with:
           path: ~/go/pkg/mod
           key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }}

.github/workflows/downstreams.yml

@@ -6,9 +6,12 @@ on:
     branches:
       - main
       - 'FEATURE-BRANCH-*'
+  merge_group:
+    types: [checks_requested]
+  pull_request:
 
 concurrency:
-  group: ${{ github.event_name == 'pull_request' && format('pr-{0}', github.event.pull_request.number) || format('commit-{0}', github.sha) }}
+  group: ${{ github.event_name == 'merge_group' && format('merge-group-{0}', github.event.merge_group.head_sha) || github.event_name == 'pull_request' && format('pr-{0}', github.event.pull_request.number) || format('commit-{0}', github.sha) }}
   cancel-in-progress: true
 
 jobs:

docs/content/code-review/create-pr.md

@@ -52,7 +52,7 @@ VCR test failures that do not immediately seem related to your PR are most likel
    git checkout modular-magician/auto-pr-PR_NUMBER
    make test
    make lint
-   make testacc TEST=./google/services/container TESTARGS='-run=TestAccContainerNodePool'
+   make testacc TEST=./google/services/container TESTARGS='-run=TestAccContainerNodePool_basic$$'
    ```
    Replace PR_NUMBER with your PR's ID.
    {{< /tab >}}
@@ -65,7 +65,7 @@ VCR test failures that do not immediately seem related to your PR are most likel
    git checkout modular-magician/auto-pr-PR_NUMBER
    make test
    make lint
-   make testacc TEST=./google/services/container TESTARGS='-run=TestAccContainerNodePool'
+   make testacc TEST=./google/services/container TESTARGS='-run=TestAccContainerNodePool_basic$$'
    ```
    Replace PR_NUMBER with your PR's ID.
    {{< /tab >}}

docs/content/code-review/release-notes.md

@@ -98,10 +98,10 @@ For each release note block, choose an appropriate type from the following list:
 Do | Don't
 -- | -----
 Use past tense to describe the end state after the change is released. Start with a verb. For example, "added...", "fixed...", or "resolved...". You can use future tense to describe future changes, such as saying that a deprecated field will be removed in a future version. | Don't use present or future tense to describe changes that are included in the pull request.
-Write user-focused release notes. For example, reference specific impacted terraform resource and field names, and discuss changes in behavior users will experience. | Avoid API field/resource/feature names. Avoid implementation details. Avoid language that requires understanding of provider internals.
+Write user-focused release notes. For example, reference specific impacted terraform resource and field names, and discuss changes in behavior users will experience. | Avoid API field/resource/feature names. Avoid implementation details. Avoid language that requires understanding of provider internals. However, in case of substantial refactorings like API version changes or engine changes (tpgtools/DCL -> MMv1, handwritten <> MMv1) **do** cover the change so users can quickly identify the release if they are affected by the change.
 Surround resource or field names with backticks. | Don't use resource or field names without punctuation or with other punctuation like quotation marks.
 Use impersonal third person. | Don't use "I", "you", etc.
-If the pull request impacts any specific, begin your release note with that product name followed by a colon. Use lower case for the first letter after the colon. For example, `cloudrun: added...` For MMv1 resources, use the folder name that contains the yaml files as the product name; for handwritten or tpgtools resources, use the API subdomain; for broad cross-product changes, use `provider`. | Don't begin your release note with the full resource name. Don't add backticks around the product name. Don't capitalize the first letter after the colon.
+If the pull request impacts a specific product, begin your release note with that product name followed by a colon. Use lower case for the first letter after the colon. For example, `cloudrun: added...` For MMv1 resources, use the folder name that contains the yaml files as the product name; for handwritten or tpgtools resources, use the API subdomain; for broad cross-product changes, use `provider`. | Don't begin your release note with the full resource name. Don't add backticks around the product name. Don't capitalize the first letter after the colon.
 
 ### Examples
 

docs/content/test/run-tests.md

@@ -59,16 +59,22 @@ aliases:
 
 1. Run acceptance tests for only modified resources. (Full test runs can take over 9 hours.) See [Go's documentation](https://pkg.go.dev/cmd/go#hdr-Testing_flags) for more information about `-run` and other flags.
 
+    ```bash
+    make testacc TEST=./google/services/container TESTARGS='-run=TestAccContainerNodePool_basic$$'
+    ```
+
+    To run all tests matching, e.g., `TestAccContainerNodePool*`, omit the trailing `$$`:
+
     ```bash
     make testacc TEST=./google/services/container TESTARGS='-run=TestAccContainerNodePool'
     ```
 
-> **Note:** Acceptance tests create actual infrastructure which can incur costs. Acceptance tests may not clean up after themselves if interrupted, so you may want to check for stray resources and / or billing charges.
+    > **Note:** Acceptance tests create actual infrastructure which can incur costs. Acceptance tests may not clean up after themselves if interrupted, so you may want to check for stray resources and / or billing charges.
 
 1. Optional: Save verbose test output (including API requests and responses) to a file for analysis.
 
     ```bash
-    TF_LOG=DEBUG make testacc TEST=./google/services/container TESTARGS='-run=TestAccContainerNodePool_basic' > output.log
+    TF_LOG=DEBUG make testacc TEST=./google/services/container TESTARGS='-run=TestAccContainerNodePool_basic$$' > output.log
     ```
 
 1. Optional: Debug tests with [Delve](https://github.com/go-delve/delve). See [`dlv test` documentation](https://github.com/go-delve/delve/blob/master/Documentation/usage/dlv_test.md) for information about available flags.
@@ -95,12 +101,19 @@ aliases:
     ```bash
     make testacc TEST=./google-beta/services/container TESTARGS='-run=TestAccContainerNodePool'
     ```
-> **Note:** Acceptance tests create actual infrastructure which can incur costs. Acceptance tests may not clean up after themselves if interrupted, so you may want to check for stray resources and / or billing charges.
+
+    To run all tests matching, e.g., `TestAccContainerNodePool*`, omit the trailing `$$`:
+
+    ```bash
+    make testacc TEST=./google-beta/services/container TESTARGS='-run=TestAccContainerNodePool'
+    ```
+
+    > **Note:** Acceptance tests create actual infrastructure which can incur costs. Acceptance tests may not clean up after themselves if interrupted, so you may want to check for stray resources and / or billing charges.
 
 1. Optional: Save verbose test output to a file for analysis.
 
     ```bash
-    TF_LOG=DEBUG make testacc TEST=./google-beta/services/container TESTARGS='-run=TestAccContainerNodePool_basic' > output.log
+    TF_LOG=DEBUG make testacc TEST=./google-beta/services/container TESTARGS='-run=TestAccContainerNodePool_basic$$' > output.log
     ```
 
 1. Optional: Debug tests with [Delve](https://github.com/go-delve/delve). See [`dlv test` documentation](https://github.com/go-delve/delve/blob/master/Documentation/usage/dlv_test.md) for information about available flags.
@@ -290,6 +303,32 @@ Configure Terraform to use locally-built binaries for `google` and `google-beta`
     TF_LOG=DEBUG TF_LOG_PATH=output.log TF_CLI_CONFIG_FILE="$HOME/tf-dev-override.tfrc" terraform apply
     ```
 
+### Run Tests with VCR Locally
+
+VCR tests record HTTP request/response interactions in cassettes and replay them in future runs without calling the real API.
+
+Running tests in `REPLAYING` mode locally can sometimes be useful. In particular, it can allow you to test more quickly, cheaply, and without spinning up real infrastructure, once you've got an initial recording.
+
+It can also be helpful for debugging tests that seem to work locally, but fail in CI in replaying mode.
+
+VCR is controlled via two variables:
+- `VCR_MODE`: `REPLAYING` or `RECORDING` mode 
+- `VCR_PATH`: Path where recorded cassettes are stored. 
+
+Ensure both variables are configured to properly trigger VCR tests locally.
+
+If you don't already have an existing cassette that's up to date, first do a run in `RECORDING` mode:
+
+```bash
+VCR_PATH=$HOME/.vcr/ VCR_MODE=RECORDING  make testacc TEST=./google/services/alloydb TESTARGS='-run=TestAccContainerNodePool_basic$$'
+```
+
+Now run the same test again in `REPLAYING` mode:
+
+```bash
+VCR_PATH=$HOME/.vcr/ VCR_MODE=REPLAYING make testacc TEST=./google/services/alloydb TESTARGS='-run=TestAccContainerNodePool_basic$$'
+```
+
 ### Cleanup
 
 To stop using developer overrides, stop setting `TF_CLI_CONFIG_FILE` in the commands you are executing.

mmv1/api/resource.go

@@ -337,7 +337,8 @@ type Resource struct {
 	// fine-grained resources and legacy resources.
 	ApiResourceTypeKind string `yaml:"api_resource_type_kind,omitempty"`
 
-	ImportPath string `yaml:"-"`
+	ImportPath     string `yaml:"-"`
+	SourceYamlFile string `yaml:"-"`
 }
 
 func (r *Resource) UnmarshalYAML(unmarshal func(any) error) error {
@@ -1787,3 +1788,45 @@ func (r Resource) CaiIamAssetNameTemplate(productBackendName string) string {
 	}
 	return fmt.Sprintf("//%s.googleapis.com/%s/{{%s}}", productBackendName, caiBaseUrl, r.IamParentResourceName())
 }
+
+func urlContainsOnlyAllowedKeys(templateURL string, allowedKeys []string) bool {
+	// Create regex to match anything between {{ and }}
+	re := regexp.MustCompile(`{{\s*([^}]+)\s*}}`)
+
+	// Find all matches in the template URL
+	matches := re.FindAllStringSubmatch(templateURL, -1)
+
+	// Create a map of allowed keys for O(1) lookup
+	allowedKeysMap := make(map[string]bool)
+	for _, key := range allowedKeys {
+		allowedKeysMap[key] = true
+	}
+
+	// Check each found key against the allowed keys
+	for _, match := range matches {
+		if len(match) < 2 {
+			continue
+		}
+
+		// Trim spaces from the key
+		key := strings.TrimSpace(match[1])
+
+		// If the key isn't in our allowed list, return false
+		if !allowedKeysMap[key] {
+			return false
+		}
+	}
+
+	return true
+}
+
+func (r Resource) ShouldGenerateSweepers() bool {
+	allowedKeys := []string{"project", "region", "location", "zone", "billing_account"}
+	if !urlContainsOnlyAllowedKeys(r.ListUrlTemplate(), allowedKeys) {
+		return false
+	}
+	if r.ExcludeSweeper || r.CustomCode.CustomDelete != "" || r.CustomCode.PreDelete != "" || r.CustomCode.PostDelete != "" || r.ExcludeDelete {
+		return false
+	}
+	return true
+}

mmv1/api/resource/sweeper.go

@@ -16,7 +16,7 @@ package resource
 type Sweeper struct {
 	// The field checked by sweeper to determine
 	// eligibility for deletion for generated resources
-	SweepableIdentifierField string   `yaml:"sweepable_identifier_field"`
-	Regions                  []string `yaml:"regions,omitempty"`
-	Prefixes                 []string `yaml:"prefixes,omitempty"`
+	IdentifierField string   `yaml:"identifier_field"`
+	Regions         []string `yaml:"regions,omitempty"`
+	Prefixes        []string `yaml:"prefixes,omitempty"`
 }

mmv1/main.go

@@ -256,6 +256,7 @@ func GenerateProduct(productChannel chan string, providerToGenerate provider.Pro
 
 		resource := &api.Resource{}
 		api.Compile(resourceYamlPath, resource, overrideDirectory)
+		resource.SourceYamlFile = resourceYamlPath
 
 		resource.TargetVersionName = *version
 		resource.Properties = resource.AddLabelsRelatedFields(resource.PropertiesWithExcluded(), nil)