fix(gcp): harden Artifact Registry cleanup policies, and test the fix itself

docs/docs/reference/supported-resources.md

@@ -857,6 +857,38 @@ resources:
               password: "${env:REGISTRY_PASSWORD}"
 ```
 
+##### Image retention (`cleanupPolicies`)
+
+Artifact Registry keeps every image version forever unless a cleanup policy says otherwise, and storage is billed per GB. Retention can be declared here so it is reviewed as code.
+
+```yaml
+            cleanupPolicies:
+              - name: delete-untagged-older-30d
+                action: DELETE
+                condition:
+                  tagState: UNTAGGED
+                  olderThan: 30d                     # or "2592000s"; both accepted, integers only
+              - name: keep-most-recent-20
+                action: KEEP                         # KEEP wins over a matching DELETE
+                mostRecentVersions:
+                  keepCount: 20
+            cleanupPolicyDryRun: true                # report only; set false to delete
+```
+
+Three behaviours are worth knowing before you use this.
+
+**Omitting `cleanupPolicies` means Simple Container does not manage retention.** Any policy set outside SC — through `gcloud` or the console — is left alone. This is the default and it is deliberate: the field is authoritative in the provider, so a resource that declares nothing would otherwise *delete* whatever is configured.
+
+**Declaring it makes Simple Container authoritative.** Policies set outside SC are then replaced by the declared list on the next provision. An explicitly empty list (`cleanupPolicies: []`) means "managed, and I want none", which is how retention is removed.
+
+Note two asymmetries. Writing the key with no value (`cleanupPolicies:` alone) decodes as *absent*, i.e. unmanaged — visually almost identical to `[]`, which removes every policy. And once SC has managed the field, **deleting the block does not return the repository to out-of-band control**: Pulumi's `ignoreChanges` carries the previous value forward from state, so retention freezes at the last declared list and later console edits are reverted on each provision. Genuinely handing the field back requires removing the property from stack state.
+
+**`cleanupPolicyDryRun` defaults to `true`.** Nothing is deleted until it is explicitly set to `false`. Dry run evaluates the policies and reports what they would remove, so run it first and read the result: deleting an image that is still deployed makes the next node reschedule fail to pull, and no provision can restore a deleted layer.
+
+Policies are validated while the Pulumi program is evaluated, and one is rejected if it would match far more than it appears to: an empty `condition`, a `DELETE` that does not set `olderThan` or target `tagState: UNTAGGED`, a `KEEP` with no `keepCount`, an empty prefix, or a non-positive duration. Prefixes are deliberately **not** accepted as narrowing a `DELETE` — they select which packages a policy covers, not which ages, so `packageNamePrefixes` alone would delete the running version of that package.
+
+An unrecognised key in `server.yaml` is silently ignored rather than rejected, so a mistyped condition field would otherwise produce a policy matching every version, which is what these checks are for.
+
 ### **Database Resources**
 
 #### **Cloud SQL PostgreSQL** (`gcp-cloudsql-postgres`)

pkg/api/tests/refapp_gke_autopilot.go

@@ -91,12 +91,6 @@ var ResolvedRefappGkeAutopilotServerResources = map[string]api.ResourceDescripto
 			Config: &gcloud.ArtifactRegistryConfig{
 				Credentials: ResolvedCommonGcpCredentials,
 				Location:    "europe-west3",
-				// Empty rather than nil because this fixture is post-resolution:
-				// the placeholder resolver deep-copies by reflection and calls
-				// reflect.MakeSlice for every slice kind, so an omitted list
-				// arrives as an empty non-nil slice. Not managed either way,
-				// which ManagesCleanupPolicies asserts.
-				CleanupPolicies: []gcloud.ArtifactRegistryCleanupPolicy{},
 			},
 		},
 		Inherit: api.Inherit{},

pkg/clouds/gcloud/artifactregistry.go

@@ -26,7 +26,14 @@ type ArtifactRegistryConfig struct {
 	// next provision. Leaving this empty therefore means "SC does not manage
 	// retention", and SC preserves whatever is configured out of band rather
 	// than deleting it. See ManagesCleanupPolicies.
-	CleanupPolicies []ArtifactRegistryCleanupPolicy `json:"cleanupPolicies,omitempty" yaml:"cleanupPolicies,omitempty"`
+	// A POINTER so that "absent" and "declared empty" stay distinguishable.
+	// The placeholder resolver deep-copies configs by reflection and calls
+	// reflect.MakeSlice for every slice kind, so a nil SLICE arrives as an empty
+	// one and the difference is destroyed; its pointer branch returns early on
+	// nil, so a nil POINTER survives. Without that, `cleanupPolicies: []` could
+	// not mean "managed, and I want none", and removing policies from config
+	// would silently leave the live ones in place forever.
+	CleanupPolicies *[]ArtifactRegistryCleanupPolicy `json:"cleanupPolicies,omitempty" yaml:"cleanupPolicies,omitempty"`
 
 	// CleanupPolicyDryRun evaluates the policies and reports what they would
 	// delete without deleting it. Only meaningful alongside CleanupPolicies.
@@ -37,7 +44,17 @@ type ArtifactRegistryConfig struct {
 // not, the caller must tell Pulumi to ignore the field rather than send an empty
 // value, which is the difference between "not managed" and "delete the policy".
 func (c *ArtifactRegistryConfig) ManagesCleanupPolicies() bool {
-	return len(c.CleanupPolicies) > 0
+	return c.CleanupPolicies != nil
+}
+
+// DeclaredCleanupPolicies returns the declared retention, or nil when SC does
+// not manage it. An explicitly empty list is "managed, and empty", which is how
+// retention is removed.
+func (c *ArtifactRegistryConfig) DeclaredCleanupPolicies() []ArtifactRegistryCleanupPolicy {
+	if c.CleanupPolicies == nil {
+		return nil
+	}
+	return *c.CleanupPolicies
 }
 
 // ArtifactRegistryCleanupPolicy mirrors a single Artifact Registry cleanup

pkg/clouds/gcloud/resources_test.go

@@ -476,3 +476,86 @@ func TestReadGkeAutopilotResourceConfig(t *testing.T) {
 		Expect(err).To(HaveOccurred())
 	})
 }
+
+// The cleanup-policy fields are only ever reachable from YAML, and every other
+// test for them constructs Go structs directly. A wrong struct tag would ship
+// green while the declared retention silently became "not managed" — which is
+// the failure this whole feature exists to prevent.
+func TestArtifactRegistryConfigReadsCleanupPoliciesFromYAML(t *testing.T) {
+	RegisterTestingT(t)
+
+	cfg := &api.Config{Config: map[string]any{
+		"projectId": "p",
+		"location":  "europe-west3",
+		"cleanupPolicies": []any{
+			map[string]any{
+				"name":   "delete-old-feature-tags",
+				"action": "DELETE",
+				"condition": map[string]any{
+					"tagState": "TAGGED", "olderThan": "2592000s", "newerThan": "3600s",
+					"tagPrefixes":         []any{"feature-"},
+					"packageNamePrefixes": []any{"svc/"},
+					"versionNamePrefixes": []any{"sha256:"},
+				},
+			},
+			map[string]any{
+				"name":               "keep-most-recent-20",
+				"action":             "KEEP",
+				"mostRecentVersions": map[string]any{"keepCount": 20, "packageNamePrefixes": []any{"api/"}},
+			},
+		},
+		"cleanupPolicyDryRun": true,
+	}}
+	out, err := ArtifactRegistryConfigReadConfig(cfg)
+	Expect(err).To(BeNil())
+	ar, ok := out.Config.(*ArtifactRegistryConfig)
+	Expect(ok).To(BeTrue())
+
+	Expect(ar.ManagesCleanupPolicies()).To(BeTrue())
+	got := ar.DeclaredCleanupPolicies()
+	Expect(got).To(HaveLen(2))
+	Expect(got[0].Name).To(Equal("delete-old-feature-tags"))
+	Expect(got[0].Action).To(Equal("DELETE"))
+	Expect(got[0].Condition).NotTo(BeNil())
+	// Every key, because a dead struct tag on any prefix field silently drops
+	// the narrowing and widens the DELETE to the whole repository, with no error.
+	Expect(got[0].Condition.TagState).To(Equal("TAGGED"))
+	Expect(got[0].Condition.OlderThan).To(Equal("2592000s"))
+	Expect(got[0].Condition.NewerThan).To(Equal("3600s"))
+	Expect(got[0].Condition.TagPrefixes).To(ConsistOf("feature-"))
+	Expect(got[0].Condition.PackageNamePrefixes).To(ConsistOf("svc/"))
+	Expect(got[0].Condition.VersionNamePrefixes).To(ConsistOf("sha256:"))
+	Expect(got[1].MostRecentVersions).NotTo(BeNil())
+	Expect(*got[1].MostRecentVersions.KeepCount).To(Equal(20))
+	Expect(got[1].MostRecentVersions.PackageNamePrefixes).To(ConsistOf("api/"))
+	Expect(ar.CleanupPolicyDryRun).NotTo(BeNil())
+	Expect(*ar.CleanupPolicyDryRun).To(BeTrue())
+}
+
+// Absent must stay absent through the decode: it is what makes SC preserve
+// out-of-band retention instead of deleting it.
+func TestArtifactRegistryConfigAbsentCleanupPoliciesStaysUnmanaged(t *testing.T) {
+	RegisterTestingT(t)
+
+	out, err := ArtifactRegistryConfigReadConfig(&api.Config{Config: map[string]any{
+		"projectId": "p", "location": "europe-west3",
+	}})
+	Expect(err).To(BeNil())
+	ar := out.Config.(*ArtifactRegistryConfig)
+	Expect(ar.CleanupPolicies).To(BeNil())
+	Expect(ar.ManagesCleanupPolicies()).To(BeFalse())
+}
+
+// An explicitly empty YAML list must survive as managed-and-empty.
+func TestArtifactRegistryConfigEmptyListIsManaged(t *testing.T) {
+	RegisterTestingT(t)
+
+	out, err := ArtifactRegistryConfigReadConfig(&api.Config{Config: map[string]any{
+		"projectId": "p", "location": "europe-west3", "cleanupPolicies": []any{},
+	}})
+	Expect(err).To(BeNil())
+	ar := out.Config.(*ArtifactRegistryConfig)
+	Expect(ar.CleanupPolicies).NotTo(BeNil(), "an explicit empty list is how retention is removed")
+	Expect(ar.ManagesCleanupPolicies()).To(BeTrue())
+	Expect(ar.DeclaredCleanupPolicies()).To(BeEmpty())
+}