diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d0f398..ce43218 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed +- **`catalog.Validate()` no longer flags a private image bound only in a local + overlay.** It checked `List()` (the overlay-merged catalog) for the + shipped-catalog-must-be-public rule (#392), so any machine with a + `~/.spawn/catalog.yaml` rebinding an app to a private ECR image — the + overlay's whole purpose — failed `Validate()` as if the SHIPPED catalog were + broken, when only that machine's own (correctly private) local binding was. + This broke `go test`/`make check` for any contributor with such an overlay, + and spawn's `TestCatalogValid` calls `Validate()` directly, so it broke + spawn's CI-equivalent too (spawn#489). The public-image check now runs + against the embedded `catalog.yaml` only. + ### Changed - CI moved off the self-hosted orion runner fleet onto `ubuntu-latest`. The fleet (colima/Docker on orion.local) is being decommissioned org-wide; no diff --git a/catalog/catalog.go b/catalog/catalog.go index 85a1dc4..adb4efc 100644 --- a/catalog/catalog.go +++ b/catalog/catalog.go @@ -266,3 +266,16 @@ func List() []AppEntry { load() return allSorted } + +// embeddedApps parses catalog.yaml directly, with NO overlay merge — the +// shipped/global baseline as it ships, before any local ~/.spawn/catalog.yaml +// rebind is layered on. Used by Validate's public-image check, which is a +// statement about the shipped artifact and must not be affected by what a +// given machine happens to have configured locally. +func embeddedApps() []AppEntry { + var f catalogFile + if err := yaml.Unmarshal(catalogData, &f); err != nil { + panic("catalog: failed to parse catalog.yaml: " + err.Error()) + } + return f.Apps +} diff --git a/catalog/validate.go b/catalog/validate.go index 8bd2ec1..e1b0b6e 100644 --- a/catalog/validate.go +++ b/catalog/validate.go @@ -27,10 +27,16 @@ func Validate() []error { // The shipped/global catalog must contain only PUBLIC images (#392): a private // image here is unlaunchable for everyone but its owner, so it has no place in // the artifact shipped to all consumers. Private images belong in a user's - // local overlay. (This is the offline half; online resolvability is a separate - // authenticated CI gate, libs#18.) validateApps stays overlay-safe — it does - // NOT enforce this, since overlays legitimately carry private images. - for _, app := range apps { + // local overlay — so this check runs against the EMBEDDED catalog only, not + // List()'s overlay-merged result. Checking the merged list made Validate() (and + // TestCatalogValid, which spawn's CI runs too) fail on any machine with a local + // ~/.spawn/catalog.yaml binding a private image, exactly the case the overlay + // exists for — the shipped catalog was never actually invalid, only the local + // developer machine running the check was. (This is the offline half; online + // resolvability is a separate authenticated CI gate, libs#18.) validateApps + // stays overlay-safe — it does NOT enforce this, since overlays legitimately + // carry private images. + for _, app := range embeddedApps() { if app.Containerized() && app.ImageVisibility() != VisibilityPublic { errs = append(errs, fmt.Errorf("%s: image %q is %s — the shipped catalog must be public; put private images in a local overlay (#392)", app.Name, app.Image, app.ImageVisibility())) diff --git a/catalog/validate_test.go b/catalog/validate_test.go index f612b25..6fffa79 100644 --- a/catalog/validate_test.go +++ b/catalog/validate_test.go @@ -16,6 +16,25 @@ func TestValidate_EmbeddedCatalogClean(t *testing.T) { } } +// TestValidate_IgnoresOverlayPrivateImage is the regression guard for the bug +// that broke Validate() (and spawn's TestCatalogValid, which calls it) on any +// machine with a local ~/.spawn/catalog.yaml binding a private image — exactly +// the overlay's intended use (#392). Validate() used to check List()'s +// overlay-merged result, so a perfectly valid local rebind made the check fail +// as if the SHIPPED catalog were broken, when only the developer's own overlay +// was (correctly) private. +func TestValidate_IgnoresOverlayPrivateImage(t *testing.T) { + withOverlay(t, ` +apps: + - name: paraview + image: 123456789012.dkr.ecr.us-east-1.amazonaws.com/paraview + tag_default: "5.13.2" +`) + for _, err := range Validate() { + t.Errorf("Validate() must not flag an overlay-bound private image: %v", err) + } +} + func TestValidateApps_CatchesDefects(t *testing.T) { base := map[string]string{"us-east-1": "ami-123"} tests := []struct {