From 0d0b4796de3658b7fc57c0985f72047f9c959a15 Mon Sep 17 00:00:00 2001 From: gaetan bogaert Date: Thu, 3 Sep 2026 09:31:13 +0200 Subject: [PATCH] artifactregistry: fix create/update of virtual repo with empty upstream_policies --- .../products/artifactregistry/Repository.yaml | 1 + ...pository_virtual_repository_config.go.tmpl | 74 ++++++++++++++++++ ..._artifact_registry_repository_test.go.tmpl | 77 +++++++++++++++++++ 3 files changed, 152 insertions(+) create mode 100644 mmv1/templates/terraform/custom_expand/artifact_registry_repository_virtual_repository_config.go.tmpl diff --git a/mmv1/products/artifactregistry/Repository.yaml b/mmv1/products/artifactregistry/Repository.yaml index c30cafc473a9..3dd58c2bac7e 100644 --- a/mmv1/products/artifactregistry/Repository.yaml +++ b/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 diff --git a/mmv1/templates/terraform/custom_expand/artifact_registry_repository_virtual_repository_config.go.tmpl b/mmv1/templates/terraform/custom_expand/artifact_registry_repository_virtual_repository_config.go.tmpl new file mode 100644 index 000000000000..7fe576b9e578 --- /dev/null +++ b/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 +} diff --git a/mmv1/third_party/terraform/services/artifactregistry/resource_artifact_registry_repository_test.go.tmpl b/mmv1/third_party/terraform/services/artifactregistry/resource_artifact_registry_repository_test.go.tmpl index 62131d229421..21b7bf4c1e91 100644 --- a/mmv1/third_party/terraform/services/artifactregistry/resource_artifact_registry_repository_test.go.tmpl +++ b/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" {