diff --git a/docs/schemas/gcp/artifactregistryconfig.json b/docs/schemas/gcp/artifactregistryconfig.json index 87b95e1c..da00ddcd 100644 --- a/docs/schemas/gcp/artifactregistryconfig.json +++ b/docs/schemas/gcp/artifactregistryconfig.json @@ -47,6 +47,78 @@ ], "type": "object" }, + "cleanupPolicies": { + "items": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "properties": { + "action": { + "type": "string" + }, + "condition": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "properties": { + "newerThan": { + "type": "string" + }, + "olderThan": { + "type": "string" + }, + "packageNamePrefixes": { + "items": { + "type": "string" + }, + "type": "array" + }, + "tagPrefixes": { + "items": { + "type": "string" + }, + "type": "array" + }, + "tagState": { + "type": "string" + }, + "versionNamePrefixes": { + "items": { + "type": "string" + }, + "type": "array" + } + }, + "required": [], + "type": "object" + }, + "mostRecentVersions": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "properties": { + "keepCount": { + "type": "integer" + }, + "packageNamePrefixes": { + "items": { + "type": "string" + }, + "type": "array" + } + }, + "required": [], + "type": "object" + }, + "name": { + "type": "string" + } + }, + "required": [ + "action", + "name" + ], + "type": "object" + }, + "type": "array" + }, + "cleanupPolicyDryRun": { + "type": "boolean" + }, "docker": { "$schema": "https://json-schema.org/draft/2020-12/schema", "properties": { diff --git a/pkg/api/tests/refapp_gke_autopilot.go b/pkg/api/tests/refapp_gke_autopilot.go index 696883bc..c8050001 100644 --- a/pkg/api/tests/refapp_gke_autopilot.go +++ b/pkg/api/tests/refapp_gke_autopilot.go @@ -91,6 +91,12 @@ 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{}, diff --git a/pkg/clouds/gcloud/artifactregistry.go b/pkg/clouds/gcloud/artifactregistry.go index 57bbc4c2..ca9395a9 100644 --- a/pkg/clouds/gcloud/artifactregistry.go +++ b/pkg/clouds/gcloud/artifactregistry.go @@ -17,6 +17,55 @@ type ArtifactRegistryConfig struct { Docker *DockerConfig `json:"docker,omitempty" yaml:"docker,omitempty"` Domain *string `json:"domain" yaml:"domain"` BasicAuth *RegistryBasicAuth `json:"basicAuth,omitempty" yaml:"basicAuth,omitempty"` + + // CleanupPolicies declares image retention for the repository. + // + // The provider treats cleanupPolicies as authoritative (unlike labels, whose + // schema documents itself as non-authoritative), so a repository resource + // that does not declare it will CLEAR any policy set by another tool on the + // 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"` + + // CleanupPolicyDryRun evaluates the policies and reports what they would + // delete without deleting it. Only meaningful alongside CleanupPolicies. + CleanupPolicyDryRun *bool `json:"cleanupPolicyDryRun,omitempty" yaml:"cleanupPolicyDryRun,omitempty"` +} + +// ManagesCleanupPolicies reports whether retention is declared here. When it is +// 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 +} + +// ArtifactRegistryCleanupPolicy mirrors a single Artifact Registry cleanup +// policy. Durations are the API's second-suffixed form, e.g. "2592000s". +type ArtifactRegistryCleanupPolicy struct { + // Name identifies the policy within the repository. + Name string `json:"name" yaml:"name"` + // Action is DELETE or KEEP (case-insensitive). + Action string `json:"action" yaml:"action"` + Condition *ArtifactRegistryCleanupPolicyCondition `json:"condition,omitempty" yaml:"condition,omitempty"` + // MostRecentVersions retains a minimum number of versions. KEEP only. + MostRecentVersions *ArtifactRegistryCleanupMostRecentVersions `json:"mostRecentVersions,omitempty" yaml:"mostRecentVersions,omitempty"` +} + +type ArtifactRegistryCleanupPolicyCondition struct { + // TagState is TAGGED, UNTAGGED or ANY. + TagState string `json:"tagState,omitempty" yaml:"tagState,omitempty"` + // OlderThan / NewerThan are durations, e.g. "2592000s" for 30 days. + OlderThan string `json:"olderThan,omitempty" yaml:"olderThan,omitempty"` + NewerThan string `json:"newerThan,omitempty" yaml:"newerThan,omitempty"` + TagPrefixes []string `json:"tagPrefixes,omitempty" yaml:"tagPrefixes,omitempty"` + PackageNamePrefixes []string `json:"packageNamePrefixes,omitempty" yaml:"packageNamePrefixes,omitempty"` + VersionNamePrefixes []string `json:"versionNamePrefixes,omitempty" yaml:"versionNamePrefixes,omitempty"` +} + +type ArtifactRegistryCleanupMostRecentVersions struct { + KeepCount *int `json:"keepCount,omitempty" yaml:"keepCount,omitempty"` + PackageNamePrefixes []string `json:"packageNamePrefixes,omitempty" yaml:"packageNamePrefixes,omitempty"` } type RegistryBasicAuth struct { diff --git a/pkg/clouds/pulumi/gcp/artifactregistry.go b/pkg/clouds/pulumi/gcp/artifactregistry.go index 5e250e8f..4c72e981 100644 --- a/pkg/clouds/pulumi/gcp/artifactregistry.go +++ b/pkg/clouds/pulumi/gcp/artifactregistry.go @@ -79,6 +79,21 @@ func ArtifactRegistry(ctx *sdk.Context, stack api.Stack, input api.ResourceInput return nil, errors.Errorf("registry format is not supported") } + // cleanupPolicies is authoritative in the provider: a Repository resource + // that omits it sends an update clearing whatever is there. So either + // declare it, or tell the engine to leave it alone. Sending nothing is the + // one option that silently deletes another tool's retention policy. + if arCfg.ManagesCleanupPolicies() { + policies, err := cleanupPolicyArgs(arCfg.CleanupPolicies) + if err != nil { + return nil, err + } + repoArgs.CleanupPolicies = policies + repoArgs.CleanupPolicyDryRun = sdk.Bool(lo.FromPtr(arCfg.CleanupPolicyDryRun)) + } else { + opts = append(opts, sdk.IgnoreChanges(cleanupPolicyFields)) + } + params.Log.Info(ctx.Context(), "configure artifact registry repository %q", artifactRegistryName) repo, err := artifactregistry.NewRepository(ctx, artifactRegistryName, &repoArgs, opts...) if err != nil { @@ -252,3 +267,85 @@ func toRegistryServiceAccountKeyExport(input api.ResourceInput, saType string, r func toRegistryServiceAccountEmailExport(input api.ResourceInput, saType string, registryName string) string { return input.ToResName(fmt.Sprintf("%s-%s-sa", saType, registryName)) } + +// cleanupPolicyFields are the property paths SC declines to manage when no +// retention is configured. Both are needed: leaving cleanupPolicyDryRun out +// would let a provision flip an out-of-band dry-run repository into enforcing. +var cleanupPolicyFields = []string{"cleanupPolicies", "cleanupPolicyDryRun"} + +// cleanupPolicyArgs converts the declared retention into provider inputs. +// +// Validation is deliberate rather than passing strings through: the provider +// rejects an unknown action or tagState at APPLY, and an Artifact Registry +// misconfiguration is measured in deleted images. +func cleanupPolicyArgs(policies []gcloud.ArtifactRegistryCleanupPolicy) (artifactregistry.RepositoryCleanupPolicyArray, error) { + out := make(artifactregistry.RepositoryCleanupPolicyArray, 0, len(policies)) + seen := make(map[string]bool, len(policies)) + for _, p := range policies { + if p.Name == "" { + return nil, errors.Errorf("cleanup policy is missing a name") + } + if seen[p.Name] { + return nil, errors.Errorf("duplicate cleanup policy name %q", p.Name) + } + seen[p.Name] = true + + action := strings.ToUpper(p.Action) + if action != "DELETE" && action != "KEEP" { + return nil, errors.Errorf("cleanup policy %q: action must be DELETE or KEEP, got %q", p.Name, p.Action) + } + if p.MostRecentVersions != nil && action != "KEEP" { + return nil, errors.Errorf("cleanup policy %q: mostRecentVersions is only valid with a KEEP action", p.Name) + } + if p.Condition == nil && p.MostRecentVersions == nil { + return nil, errors.Errorf("cleanup policy %q: needs a condition or mostRecentVersions", p.Name) + } + + args := &artifactregistry.RepositoryCleanupPolicyArgs{ + Id: sdk.String(p.Name), + Action: sdk.String(action), + } + if c := p.Condition; c != nil { + tagState := strings.ToUpper(c.TagState) + switch tagState { + case "", "TAGGED", "UNTAGGED", "ANY": + default: + return nil, errors.Errorf("cleanup policy %q: tagState must be TAGGED, UNTAGGED or ANY, got %q", p.Name, c.TagState) + } + for field, v := range map[string]string{"olderThan": c.OlderThan, "newerThan": c.NewerThan} { + if v != "" && !strings.HasSuffix(v, "s") { + return nil, errors.Errorf("cleanup policy %q: %s must be a duration in seconds with an 's' suffix, e.g. \"2592000s\", got %q", p.Name, field, v) + } + } + cond := &artifactregistry.RepositoryCleanupPolicyConditionArgs{ + TagPrefixes: sdk.ToStringArray(c.TagPrefixes), + PackageNamePrefixes: sdk.ToStringArray(c.PackageNamePrefixes), + VersionNamePrefixes: sdk.ToStringArray(c.VersionNamePrefixes), + } + if tagState != "" { + cond.TagState = sdk.StringPtr(tagState) + } + if c.OlderThan != "" { + cond.OlderThan = sdk.StringPtr(c.OlderThan) + } + if c.NewerThan != "" { + cond.NewerThan = sdk.StringPtr(c.NewerThan) + } + args.Condition = cond + } + if m := p.MostRecentVersions; m != nil { + if lo.FromPtr(m.KeepCount) < 0 { + return nil, errors.Errorf("cleanup policy %q: keepCount cannot be negative", p.Name) + } + mrv := &artifactregistry.RepositoryCleanupPolicyMostRecentVersionsArgs{ + PackageNamePrefixes: sdk.ToStringArray(m.PackageNamePrefixes), + } + if m.KeepCount != nil { + mrv.KeepCount = sdk.IntPtr(*m.KeepCount) + } + args.MostRecentVersions = mrv + } + out = append(out, args) + } + return out, nil +} diff --git a/pkg/clouds/pulumi/gcp/artifactregistry_cleanup_test.go b/pkg/clouds/pulumi/gcp/artifactregistry_cleanup_test.go new file mode 100644 index 00000000..fded1323 --- /dev/null +++ b/pkg/clouds/pulumi/gcp/artifactregistry_cleanup_test.go @@ -0,0 +1,165 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) Simple Container + +package gcp + +import ( + "testing" + + . "github.com/onsi/gomega" + "github.com/samber/lo" + + "github.com/simple-container-com/api/pkg/clouds/gcloud" +) + +// The provider treats cleanupPolicies as authoritative. A Repository resource +// that omits it sends an update CLEARING whatever is configured, which is how a +// separately-managed retention policy gets deleted on the next provision. +// Observed on payspace-475408 2026-08-12: a policy set at 13:35:10 was cleared +// by pulumi-gcp/v8.41.1 at 13:37:31, in a request whose cleanupPolicies was +// empty. Refresh had loaded the live policy into outputs, the program declared +// no input, and the diff resolved as "delete it". +// +// So the contract is: declaring nothing must mean "not managed", never +// "delete". That is what ManagesCleanupPolicies gates. +func TestManagesCleanupPolicies(t *testing.T) { + RegisterTestingT(t) + + Expect((&gcloud.ArtifactRegistryConfig{}).ManagesCleanupPolicies()).To(BeFalse(), + "no declared policies must mean 'SC does not manage retention', so the field is ignored rather than emptied") + + Expect((&gcloud.ArtifactRegistryConfig{ + CleanupPolicies: []gcloud.ArtifactRegistryCleanupPolicy{{Name: "x", Action: "DELETE"}}, + }).ManagesCleanupPolicies()).To(BeTrue()) + + // An explicitly empty slice is still "not managed". Treating it as "delete + // everything declared" would make an empty YAML list destructive. + Expect((&gcloud.ArtifactRegistryConfig{ + CleanupPolicies: []gcloud.ArtifactRegistryCleanupPolicy{}, + }).ManagesCleanupPolicies()).To(BeFalse()) +} + +// Both fields must be ignored together. Ignoring only cleanupPolicies would let +// a provision flip a repository that another tool put in dry-run into +// enforcing, which turns a reporting run into real deletions. +func TestCleanupPolicyFieldsCoverDryRun(t *testing.T) { + RegisterTestingT(t) + + Expect(cleanupPolicyFields).To(ConsistOf("cleanupPolicies", "cleanupPolicyDryRun")) +} + +func TestCleanupPolicyArgsAcceptsRealPolicies(t *testing.T) { + RegisterTestingT(t) + + got, err := cleanupPolicyArgs([]gcloud.ArtifactRegistryCleanupPolicy{ + { + Name: "delete-untagged-older-30d", + Action: "DELETE", + Condition: &gcloud.ArtifactRegistryCleanupPolicyCondition{TagState: "UNTAGGED", OlderThan: "2592000s"}, + }, + { + Name: "keep-most-recent-20", + Action: "KEEP", + MostRecentVersions: &gcloud.ArtifactRegistryCleanupMostRecentVersions{KeepCount: lo.ToPtr(20)}, + }, + }) + Expect(err).To(BeNil()) + Expect(got).To(HaveLen(2)) +} + +func TestCleanupPolicyArgsRejectsBadInput(t *testing.T) { + RegisterTestingT(t) + + for _, tc := range []struct { + name string + policy gcloud.ArtifactRegistryCleanupPolicy + wantErr string + why string + }{ + { + name: "unknown action", + policy: gcloud.ArtifactRegistryCleanupPolicy{Name: "p", Action: "PURGE"}, + wantErr: "action must be DELETE or KEEP", + why: "the provider rejects this at apply, by which point the config is already merged", + }, + { + name: "mostRecentVersions with DELETE", + policy: gcloud.ArtifactRegistryCleanupPolicy{ + Name: "p", Action: "DELETE", + MostRecentVersions: &gcloud.ArtifactRegistryCleanupMostRecentVersions{KeepCount: lo.ToPtr(5)}, + }, + wantErr: "only valid with a KEEP action", + why: "a keep-count under a DELETE action reads as protective while deleting", + }, + { + name: "no condition and no mostRecentVersions", + policy: gcloud.ArtifactRegistryCleanupPolicy{Name: "p", Action: "DELETE"}, + wantErr: "needs a condition or mostRecentVersions", + why: "an unconditional DELETE policy matches every version in the repository", + }, + { + name: "duration without a seconds suffix", + policy: gcloud.ArtifactRegistryCleanupPolicy{ + Name: "p", Action: "DELETE", + Condition: &gcloud.ArtifactRegistryCleanupPolicyCondition{OlderThan: "30d"}, + }, + wantErr: "'s' suffix", + why: "gcloud docs use 30d style durations but the API wants seconds; silently wrong retention otherwise", + }, + { + name: "unknown tagState", + policy: gcloud.ArtifactRegistryCleanupPolicy{ + Name: "p", Action: "DELETE", + Condition: &gcloud.ArtifactRegistryCleanupPolicyCondition{TagState: "RELEASED", OlderThan: "1s"}, + }, + wantErr: "tagState must be", + why: "a typo'd tagState defaults to ANY server-side, widening the delete set", + }, + { + name: "missing name", + policy: gcloud.ArtifactRegistryCleanupPolicy{Action: "KEEP", MostRecentVersions: &gcloud.ArtifactRegistryCleanupMostRecentVersions{KeepCount: lo.ToPtr(1)}}, + wantErr: "missing a name", + }, + { + name: "negative keepCount", + policy: gcloud.ArtifactRegistryCleanupPolicy{ + Name: "p", Action: "KEEP", + MostRecentVersions: &gcloud.ArtifactRegistryCleanupMostRecentVersions{KeepCount: lo.ToPtr(-1)}, + }, + wantErr: "cannot be negative", + }, + } { + t.Run(tc.name, func(t *testing.T) { + RegisterTestingT(t) + _, err := cleanupPolicyArgs([]gcloud.ArtifactRegistryCleanupPolicy{tc.policy}) + Expect(err).NotTo(BeNil(), tc.why) + Expect(err.Error()).To(ContainSubstring(tc.wantErr)) + }) + } +} + +func TestCleanupPolicyArgsRejectsDuplicateNames(t *testing.T) { + RegisterTestingT(t) + + // Artifact Registry keys policies by name, so a duplicate silently drops one + // of them and the repository ends up with retention nobody reviewed. + _, err := cleanupPolicyArgs([]gcloud.ArtifactRegistryCleanupPolicy{ + {Name: "dup", Action: "DELETE", Condition: &gcloud.ArtifactRegistryCleanupPolicyCondition{OlderThan: "1s"}}, + {Name: "dup", Action: "KEEP", MostRecentVersions: &gcloud.ArtifactRegistryCleanupMostRecentVersions{KeepCount: lo.ToPtr(1)}}, + }) + Expect(err).NotTo(BeNil()) + Expect(err.Error()).To(ContainSubstring("duplicate cleanup policy name")) +} + +// Case-insensitivity matters because the committed JSON in at least one fleet +// uses "Delete"/"Keep" while the API enum is upper-case. +func TestCleanupPolicyArgsNormalisesCase(t *testing.T) { + RegisterTestingT(t) + + got, err := cleanupPolicyArgs([]gcloud.ArtifactRegistryCleanupPolicy{{ + Name: "p", Action: "Delete", + Condition: &gcloud.ArtifactRegistryCleanupPolicyCondition{TagState: "untagged", OlderThan: "1s"}, + }}) + Expect(err).To(BeNil()) + Expect(got).To(HaveLen(1)) +}