artifactregistry: fix create/update of virtual repo with empty upstream_policies

mmv1/products/artifactregistry/Repository.yaml

@@ -445,6 +445,7 @@ properties:
     description: Configuration specific for a Virtual Repository.
     conflicts:
       - remote_repository_config
+    custom_expand: templates/terraform/custom_expand/artifact_registry_repository_virtual_repository_config.go.tmpl
     properties:
       - name: upstreamPolicies
         type: Array

mmv1/templates/terraform/custom_expand/artifact_registry_repository_virtual_repository_config.go.tmpl

@@ -0,0 +1,74 @@
+/*
+ * Artifact Registry requires `virtual_repository_config` to be present on a
+ * VIRTUAL_REPOSITORY-mode repository, including one with zero
+ * `upstream_policies` configured yet (the Artifact Registry REST API accepts
+ * `virtualRepositoryConfig: {upstreamPolicies: []}` and creates the repo
+ * fine).
+ *
+ * The default generated expand uses reflect.ValueOf(...)+IsEmptyValue to
+ * decide whether to include the "upstreamPolicies" key, which drops it
+ * whenever the list is empty. That in turn makes the returned
+ * `virtualRepositoryConfig` map itself empty, so the surrounding
+ * create/update logic (which also gates on IsEmptyValue) drops the whole
+ * `virtualRepositoryConfig` key from the request. The API then rejects the
+ * request with "Virtual repository config is not specified", even though an
+ * explicitly empty upstream_policies list is valid input.
+ *
+ * Fix: include "upstreamPolicies" whenever it was actually expanded (i.e.
+ * the block was set), regardless of whether the resulting list is empty.
+ *
+ * NB: a custom_expand on this property replaces generation for its whole
+ * subtree, so the (otherwise-identical) upstream_policies expand is inlined
+ * below rather than calling a separately-generated helper.
+ */
+func expandArtifactRegistryRepositoryVirtualRepositoryConfig(v interface{}, d tpgresource.TerraformResourceData, config *transport_tpg.Config) (interface{}, error) {
+	if v == nil {
+		return nil, nil
+	}
+	l := v.([]interface{})
+	if len(l) == 0 || l[0] == nil {
+		return nil, nil
+	}
+	raw := l[0]
+	original := raw.(map[string]interface{})
+	transformed := make(map[string]interface{})
+
+	transformedUpstreamPolicies, err := expandArtifactRegistryRepositoryVirtualRepositoryConfigUpstreamPolicies(original["upstream_policies"], d, config)
+	if err != nil {
+		return nil, err
+	} else if transformedUpstreamPolicies != nil {
+		transformed["upstreamPolicies"] = transformedUpstreamPolicies
+	}
+
+	return transformed, nil
+}
+
+func expandArtifactRegistryRepositoryVirtualRepositoryConfigUpstreamPolicies(v interface{}, d tpgresource.TerraformResourceData, config *transport_tpg.Config) (interface{}, error) {
+	if v == nil {
+		return nil, nil
+	}
+	l := v.([]interface{})
+	req := make([]interface{}, 0, len(l))
+	for _, raw := range l {
+		if raw == nil {
+			continue
+		}
+		original := raw.(map[string]interface{})
+		transformed := make(map[string]interface{})
+
+		if val := reflect.ValueOf(original["id"]); val.IsValid() && !tpgresource.IsEmptyValue(val) {
+			transformed["id"] = original["id"]
+		}
+
+		if val := reflect.ValueOf(original["repository"]); val.IsValid() && !tpgresource.IsEmptyValue(val) {
+			transformed["repository"] = original["repository"]
+		}
+
+		if val := reflect.ValueOf(original["priority"]); val.IsValid() && !tpgresource.IsEmptyValue(val) {
+			transformed["priority"] = original["priority"]
+		}
+
+		req = append(req, transformed)
+	}
+	return req, nil
+}

mmv1/third_party/terraform/services/artifactregistry/resource_artifact_registry_repository_test.go.tmpl

@@ -502,6 +502,50 @@ func TestAccArtifactRegistryRepository_virtual(t *testing.T) {
 	})
 }
 
+func TestAccArtifactRegistryRepository_virtualEmpty(t *testing.T) {
+	t.Parallel()
+
+	repositoryID := fmt.Sprintf("tf-test-%d-virtual-empty", acctest.RandInt(t))
+	upstreamRepositoryID := fmt.Sprintf("tf-test-%d-upstream", acctest.RandInt(t))
+
+	acctest.VcrTest(t, resource.TestCase{
+		PreCheck:                 func() { acctest.AccTestPreCheck(t) },
+		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
+		CheckDestroy:             testAccCheckArtifactRegistryRepositoryDestroyProducer(t),
+		Steps: []resource.TestStep{
+			{
+				// Regression test: creating a VIRTUAL_REPOSITORY with an
+				// empty upstream_policies list used to fail with
+				// "Error 400: Virtual repository config is not specified",
+				// because the generated expand dropped the whole
+				// virtual_repository_config object whenever
+				// upstream_policies was empty - even though the Artifact
+				// Registry API itself accepts an explicit empty list fine.
+				Config: testAccArtifactRegistryRepository_virtualEmpty(repositoryID, upstreamRepositoryID, false),
+			},
+			{
+				ResourceName:      "google_artifact_registry_repository.vr-empty-test",
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+			{
+				// Zero -> one upstream policy must also work (same expand
+				// function is used on the update path).
+				Config: testAccArtifactRegistryRepository_virtualEmpty(repositoryID, upstreamRepositoryID, true),
+			},
+			{
+				ResourceName:      "google_artifact_registry_repository.vr-empty-test",
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+			{
+				// And back down to zero again.
+				Config: testAccArtifactRegistryRepository_virtualEmpty(repositoryID, upstreamRepositoryID, false),
+			},
+		},
+	})
+}
+
 func TestAccArtifactRegistryRepository_remote(t *testing.T) {
 	t.Parallel()
 
@@ -615,6 +659,39 @@ resource "google_artifact_registry_repository" "vr-test" {
 `,upstreamRepositoryID, upstreamRepositoryID, repositoryID, policy_a, policy_b)
 }
 
+func testAccArtifactRegistryRepository_virtualEmpty(repositoryID string, upstreamRepositoryID string, withPolicy bool) string {
+	policy := ""
+	if withPolicy {
+		policy = `
+    upstream_policies {
+      id = "upstream-a"
+      repository = google_artifact_registry_repository.upstream-empty-test.id
+      priority = 1
+    }
+`
+	}
+	return fmt.Sprintf(`
+resource "google_artifact_registry_repository" "upstream-empty-test" {
+  repository_id = "%s"
+  location = "us-central1"
+  description = "upstream repo"
+  format = "DOCKER"
+}
+
+resource "google_artifact_registry_repository" "vr-empty-test" {
+  repository_id = "%s"
+  location = "us-central1"
+  description = "virtual repo with no upstream policies at creation"
+  format = "DOCKER"
+  mode = "VIRTUAL_REPOSITORY"
+
+  virtual_repository_config {
+%s
+  }
+}
+`, upstreamRepositoryID, repositoryID, policy)
+}
+
 func testAccArtifactRegistryRepository_remote(repositoryID string, remoteDescription string) string {
 	return fmt.Sprintf(`
 resource "google_artifact_registry_repository" "rr-test" {