From 36e584b2c5a716dc315d97ca9556408e488b8b14 Mon Sep 17 00:00:00 2001 From: Antoine Toussaint <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Sat, 24 Oct 2026 14:27:13 -0400 Subject: [PATCH 1/2] feat(registry): push to ghcr.io, pull through a cell's remote repository A mirror declared for the host of registry.url now splits push from pull: images are pushed (and probed, cached, checked) at registry.url and the rendered manifests and cell file name them under the mirror with the pushed digest. EnvironmentRegistry.PullRepository is url read through mirrors, with the one mirroredReference spelling now shared from pkg/environments. - registry.artifacts: where frontend bundles are published and read - auth: ghcr wired (GITHUB_TOKEN/GH_TOKEN/gh auth token, GITHUB_ACTOR/gh api user, docker login --password-stdin) and requires a ghcr.io url - a released version is only built when the registry answers not found; any other probe failure stops the render - ghcr.io pushes carry org.opencontainers.image.source, outside the identity - the legacy in-agent path tags and pushes the push reference No mirror for the registry.url host: unchanged naming, push and probe. Co-Authored-By: Claude Opus 5.5 --- docs/commands.md | 55 ++++++- pkg/builder/registry_auth.go | 73 ++++++++- pkg/builder/registry_auth_test.go | 87 ++++++++++- pkg/environments/environment.go | 106 ++++++++++++- pkg/environments/registry_pull_test.go | 86 +++++++++++ pkg/gitops/frontend_payload.go | 8 +- pkg/gitops/orchestrate.go | 6 +- pkg/gitops/registry_pull_test.go | 56 +++++++ pkg/orchestration/build_plan.go | 78 +++++++++- pkg/orchestration/builder.go | 36 ++++- pkg/orchestration/image_cache.go | 30 +++- pkg/orchestration/image_cache_test.go | 8 +- pkg/orchestration/registry_mirror.go | 64 +++----- pkg/orchestration/registry_mirror_test.go | 2 +- pkg/orchestration/registry_pull_test.go | 174 ++++++++++++++++++++++ 15 files changed, 795 insertions(+), 74 deletions(-) create mode 100644 pkg/environments/registry_pull_test.go create mode 100644 pkg/gitops/registry_pull_test.go create mode 100644 pkg/orchestration/registry_pull_test.go diff --git a/docs/commands.md b/docs/commands.md index ca23799eb..80b2e4c2a 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -1055,8 +1055,8 @@ it** — a mirror serves the same content, so the resolved digest is the same an it is the digest, never the registry it came from, that the reuse key binds. A mirror is **not** authenticated by `registry.auth`, which is the push side and -covers `registry.url` alone (and today is wired for `ecr` and `acr` only — `gar` -and `ghcr` are stubs). Both halves of a mirrored read use the host's Docker +covers `registry.url` alone (wired for `ecr`, `acr` and `ghcr`; `gar` is a +stub). Both halves of a mirrored read use the host's Docker credential store instead: `docker buildx imagetools inspect` for the resolution, and the container-driver builder for the pull. So log in to the mirror's registry on the machine that renders before it renders — for an Artifact @@ -1090,6 +1090,49 @@ premise — but it also means neither the build output nor the reuse key records *which* registry answered, so a mirror serving something else would be bound as if it were upstream. +**A cell pulls the environment's own images through a mirror too.** A mirror +declared for the host of `registry.url` itself makes the push and the pull two +registries: the build pushes to `registry.url`, and the rendered manifests (and +so the cell file) name the image under the mirror, with the pushed digest. This +is how one canonical private registry serves every cell: each cell reads it +through a same-cloud pull-through repository with its own workload identity, +and no registry credential is ever placed in a cell. + +```yaml +environments: + - name: staging + registry: + url: ghcr.io/example # pushed to, once, by whoever renders + auth: ghcr # requires a ghcr.io url + mirrors: + ghcr.io: us-east1-docker.pkg.dev/example/ghcr # a remote repository of ghcr.io + artifacts: us-east1-docker.pkg.dev/example/bundles # frontend bundles +``` + +Here `ghcr.io/example//:` is pushed and +`us-east1-docker.pkg.dev/example/ghcr/example//@sha256:…` is +deployed — the mirror's repository path is the upstream path, with the same +normalization a base image is mirrored with. Every registry read the build makes +(the released-version check, the reuse cache, the check that a kept image still +exists) uses the pushed reference. An image the recipe names under any other +repository is untouched. With no mirror for the `registry.url` host, nothing +changes: the image is pushed and pulled where `registry.url` names it. + +`auth: ghcr` logs Docker in to `ghcr.io` with a token from `GITHUB_TOKEN`, else +`GH_TOKEN`, else `gh auth token`, as `GITHUB_ACTOR`, else the `gh` login; the +token reaches `docker login` on stdin and is never logged. An image pushed to +`ghcr.io` is labelled (and, multi-platform, its index annotated) +`org.opencontainers.image.source` with the workspace's GitHub repository, which +is what links a private package to the repository whose access it inherits. The +label is added after the image's identity is computed, so it never causes a +rebuild. + +`registry.artifacts` is where frontend bundles are published and read instead +of `registry.url`. Bundles are OCI artifacts rather than images, and a +pull-through repository is not proven to serve them, so an environment whose +`registry.url` is mirrored names a registry its hosts read bundles from +directly. Absent, bundles go to `registry.url` as before. + `render` is a function of the workspace and needs no cluster: by default no service's manifests are sent to a Kubernetes API. Pass `--validate-cluster` to also dry-run each rendered service server-side (`kubectl apply --server-side @@ -3919,8 +3962,12 @@ nothing is built — when that label equals this build's inputs. A published image recording **other** inputs refuses the build: its version was not moved with its code, and the fix is a new release, never a rebuild under the old version. One recording none (pushed before builds recorded their inputs) is -built again. With a registry whose tags are immutable, each version then holds -exactly one build for good. +built again. Only the registry answering that the tag does not exist lets a +released version be built: any other failed read (an authorization refusal, a +timeout, an unreachable registry) stops the render, because building over it +would overwrite a released tag on any registry whose tags are mutable. With a +registry whose tags are immutable, each version then holds exactly one build +for good. A build also reuses the image this workspace already built from the same inputs instead of building it again. A configuration-only change — an edit to a value diff --git a/pkg/builder/registry_auth.go b/pkg/builder/registry_auth.go index 898bf35b4..5d376aa86 100644 --- a/pkg/builder/registry_auth.go +++ b/pkg/builder/registry_auth.go @@ -4,6 +4,8 @@ import ( "bytes" "context" "fmt" + "io" + "os" "os/exec" "regexp" "strings" @@ -30,7 +32,10 @@ import ( // (.azurecr.io). // "gar" — Google Artifact Registry. Stubbed (returns ErrUnimplemented) // until someone needs it. -// "ghcr" — GitHub Container Registry. Stubbed. +// "ghcr" — GitHub Container Registry. The token is GITHUB_TOKEN, else +// GH_TOKEN, else `gh auth token --hostname github.com`; the user +// is GITHUB_ACTOR, else `gh api user --jq .login`. The token is +// piped to `docker login ghcr.io --password-stdin`. // // Called from cli/cmd/build/service.go before flow.Build runs. Failures // here are loud — bad to discover at push time after a long build. @@ -48,7 +53,7 @@ func RegistryLogin(ctx context.Context, registryURL, authKind string) error { case "gar", "gcr": return fmt.Errorf("registry auth %q not yet wired — file an issue or add it in cli/pkg/builder/registry_auth.go", authKind) case "ghcr": - return fmt.Errorf("registry auth %q not yet wired — set GITHUB_TOKEN and run `echo $GITHUB_TOKEN | docker login ghcr.io -u --password-stdin` manually for now", authKind) + return ghcrLogin(ctx) default: return fmt.Errorf("unknown registry auth %q (supported: ecr, acr, gar, ghcr)", authKind) } @@ -173,3 +178,67 @@ func acrLogin(ctx context.Context, registryURL string) error { w.Info("ACR login succeeded", wool.Field("host", host), wool.Field("name", name)) return nil } + +// ghcrHost is the registry the ghcr auth logs in to. +const ghcrHost = "ghcr.io" + +// runRegistryCommand runs one credential command with stdin and returns its +// stdout; its stderr is folded into the error. A seam so a test needs neither +// gh nor docker. +var runRegistryCommand = func(ctx context.Context, stdin io.Reader, name string, args ...string) ([]byte, error) { + cmd := exec.CommandContext(ctx, name, args...) + cmd.Stdin = stdin + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + if err := cmd.Run(); err != nil { + return nil, fmt.Errorf("%v: %s", err, strings.TrimSpace(stderr.String())) + } + return stdout.Bytes(), nil +} + +// lookupRegistryEnv reads the environment; a seam for tests. +var lookupRegistryEnv = os.Getenv + +// ghcrLogin logs docker in to ghcr.io with the caller's GitHub identity: a CI +// token from the environment, or the GitHub CLI's login. The token only ever +// reaches docker on stdin and is never logged. +func ghcrLogin(ctx context.Context) error { + w := wool.Get(ctx).In("ghcrLogin") + token := firstNonEmpty(lookupRegistryEnv("GITHUB_TOKEN"), lookupRegistryEnv("GH_TOKEN")) + if token == "" { + out, err := runRegistryCommand(ctx, nil, "gh", "auth", "token", "--hostname", "github.com") + if err != nil { + return fmt.Errorf("registry auth ghcr: no GITHUB_TOKEN or GH_TOKEN, and `gh auth token` failed: %w", err) + } + token = strings.TrimSpace(string(out)) + } + if token == "" { + return fmt.Errorf("registry auth ghcr: no GitHub token (set GITHUB_TOKEN or run `gh auth login`)") + } + user := lookupRegistryEnv("GITHUB_ACTOR") + if user == "" { + out, err := runRegistryCommand(ctx, nil, "gh", "api", "user", "--jq", ".login") + if err != nil { + return fmt.Errorf("registry auth ghcr: no GITHUB_ACTOR, and `gh api user` failed: %w", err) + } + user = strings.TrimSpace(string(out)) + } + if user == "" { + return fmt.Errorf("registry auth ghcr: no GitHub user (set GITHUB_ACTOR or run `gh auth login`)") + } + if _, err := runRegistryCommand(ctx, strings.NewReader(token), "docker", "login", "--username", user, "--password-stdin", ghcrHost); err != nil { + return fmt.Errorf("docker login %s failed: %w", ghcrHost, err) + } + w.Info("GHCR login succeeded", wool.Field("host", ghcrHost), wool.Field("user", user)) + return nil +} + +func firstNonEmpty(values ...string) string { + for _, value := range values { + if value = strings.TrimSpace(value); value != "" { + return value + } + } + return "" +} diff --git a/pkg/builder/registry_auth_test.go b/pkg/builder/registry_auth_test.go index 4f236a9bb..c36e4fb78 100644 --- a/pkg/builder/registry_auth_test.go +++ b/pkg/builder/registry_auth_test.go @@ -1,6 +1,12 @@ package builder -import "testing" +import ( + "context" + "fmt" + "io" + "strings" + "testing" +) func TestACRName(t *testing.T) { cases := []struct { @@ -26,3 +32,82 @@ func TestACRName(t *testing.T) { } } } + +type registryCall struct { + argv string + stdin string +} + +// stubGHCR replaces the environment and every credential command: gh answers +// with ghToken and ghUser, docker login succeeds. +func stubGHCR(t *testing.T, env map[string]string, ghToken, ghUser string) *[]registryCall { + t.Helper() + previousRun, previousEnv := runRegistryCommand, lookupRegistryEnv + t.Cleanup(func() { runRegistryCommand, lookupRegistryEnv = previousRun, previousEnv }) + lookupRegistryEnv = func(key string) string { return env[key] } + calls := &[]registryCall{} + runRegistryCommand = func(_ context.Context, stdin io.Reader, name string, args ...string) ([]byte, error) { + call := registryCall{argv: name + " " + strings.Join(args, " ")} + if stdin != nil { + payload, _ := io.ReadAll(stdin) + call.stdin = string(payload) + } + *calls = append(*calls, call) + switch { + case strings.HasPrefix(call.argv, "gh auth token"): + return []byte(ghToken + "\n"), nil + case strings.HasPrefix(call.argv, "gh api user"): + return []byte(ghUser + "\n"), nil + case strings.HasPrefix(call.argv, "docker login"): + return nil, nil + } + return nil, fmt.Errorf("unexpected %s", call.argv) + } + return calls +} + +func TestGHCRLoginReadsTheTokenFromCIThenTheGitHubCLI(t *testing.T) { + for _, tc := range []struct { + name string + env map[string]string + calls []string + user string + token string + }{ + {"ci", map[string]string{"GITHUB_TOKEN": "env-token", "GH_TOKEN": "other", "GITHUB_ACTOR": "ci-bot"}, nil, "ci-bot", "env-token"}, + {"gh token", map[string]string{"GH_TOKEN": "gh-env-token"}, []string{"gh api user --jq .login"}, "octocat", "gh-env-token"}, + {"gh cli", map[string]string{}, []string{"gh auth token --hostname github.com", "gh api user --jq .login"}, "octocat", "cli-token"}, + } { + t.Run(tc.name, func(t *testing.T) { + calls := stubGHCR(t, tc.env, "cli-token", "octocat") + if err := RegistryLogin(t.Context(), "ghcr.io/obin-ai", "ghcr"); err != nil { + t.Fatal(err) + } + var argv []string + for _, call := range *calls { + argv = append(argv, call.argv) + } + want := append(append([]string(nil), tc.calls...), "docker login --username "+tc.user+" --password-stdin ghcr.io") + if strings.Join(argv, "\n") != strings.Join(want, "\n") { + t.Fatalf("commands = %q, want %q", argv, want) + } + login := (*calls)[len(*calls)-1] + if login.stdin != tc.token || strings.Contains(login.argv, tc.token) { + t.Fatalf("the token must reach docker on stdin only: %+v", login) + } + }) + } +} + +func TestGHCRLoginWithoutATokenFailsWithoutLeakingOne(t *testing.T) { + stubGHCR(t, map[string]string{}, "", "octocat") + err := RegistryLogin(t.Context(), "ghcr.io/obin-ai", "ghcr") + if err == nil || !strings.Contains(err.Error(), "no GitHub token") { + t.Fatalf("want a missing-token refusal, got %v", err) + } + stubGHCR(t, map[string]string{"GITHUB_TOKEN": "secret-token"}, "", "") + err = RegistryLogin(t.Context(), "ghcr.io/obin-ai", "ghcr") + if err == nil || !strings.Contains(err.Error(), "no GitHub user") || strings.Contains(err.Error(), "secret-token") { + t.Fatalf("want a missing-user refusal naming no token, got %v", err) + } +} diff --git a/pkg/environments/environment.go b/pkg/environments/environment.go index 8240a7f87..aa430bac5 100644 --- a/pkg/environments/environment.go +++ b/pkg/environments/environment.go @@ -61,18 +61,100 @@ type EnvironmentCluster struct { // "621829027644.dkr.ecr.us-east-1.amazonaws.com/myrepo". // Auth: how to authenticate before push — "" (anonymous / docker-creds), // "ecr" (run `aws ecr get-login-password`), "gcr" / "gar" (gcloud -// access token), "ghcr" (GITHUB_TOKEN env). The CLI handles auth +// access token), "ghcr" (a GitHub token; requires a ghcr.io URL). The CLI handles auth // side-effects based on this value. // Mirrors: where to READ an image from instead of the registry its // reference names, keyed by that registry's canonical host // ("docker.io": "us-east1-docker.pkg.dev//"). // It is a read-side substitution only: nothing is pushed to a mirror, // and the reference a recipe is written against never changes. See -// Validate for what a key and a value may be. +// Validate for what a key and a value may be. A mirror of the +// registry URL itself names where a cell pulls the images it pushes +// (PullRepository): the push goes to URL, the manifests name the mirror. +// Artifacts: where frontend bundles are published and read instead of URL. +// Images pull through a mirror; a non-image OCI artifact is not proven +// to, so an environment whose URL is mirrored names a registry its +// hosts read bundles from directly. type EnvironmentRegistry struct { - URL string `yaml:"url,omitempty"` - Auth string `yaml:"auth,omitempty"` - Mirrors map[string]string `yaml:"mirrors,omitempty"` + URL string `yaml:"url,omitempty"` + Auth string `yaml:"auth,omitempty"` + Mirrors map[string]string `yaml:"mirrors,omitempty"` + Artifacts string `yaml:"artifacts,omitempty"` +} + +// GHCRRegistry is GitHub Container Registry's host, the only registry the +// `ghcr` auth logs in to. +const GHCRRegistry = "ghcr.io" + +// PullRepository is the repository a cell reads this environment's images +// from: URL, read through the mirror declared for its registry when there is +// one, with the same normalization a base image is mirrored with +// (MirroredReference). Without a mirror for it, it is URL unchanged. +func (registry *EnvironmentRegistry) PullRepository() string { + if registry == nil { + return "" + } + base := strings.TrimSuffix(registry.URL, "/") + if base == "" { + return registry.URL + } + // A repository prefix is not a reference: "myorg" names docker.io/myorg, + // not docker.io/library/myorg. Parsing it with one more path component + // gives the parser the prefix a pushed image actually starts with. + probe := base + "/" + mirrorHostProbe + mirrored := MirroredReference(probe, registry.Mirrors) + if mirrored == probe { + return registry.URL + } + return strings.TrimSuffix(mirrored, "/"+mirrorHostProbe) +} + +// MirroredReference is the reference to read ref from: the same repository and +// tag under the mirror declared for its registry, or ref itself when no mirror +// is declared for it. +// +// A mirror of Docker Hub holds the upstream repository path verbatim, which for +// an unqualified reference is the `library` namespace Docker resolves it to — +// so `debian:trixie-slim` is read as `/library/debian:trixie-slim`, +// which is what an Artifact Registry remote repository (and a pull-through +// registry generally) serves it at. +func MirroredReference(ref string, mirrors map[string]string) string { + if len(mirrors) == 0 { + return ref + } + host, repository, ok := ReferenceRegistry(ref) + if !ok { + return ref + } + mirror := mirrors[host] + if mirror == "" { + return ref + } + return mirror + "/" + repository +} + +// ReferenceRegistry splits an image reference into the host of the registry it +// resolves against and the repository (with its tag or digest) within it, using +// the same parser the daemon, BuildKit and containerd all normalize references +// with — so the host this keys a mirror on is the host BuildKit will look that +// mirror up under, and the two cannot disagree about what a reference names. +// +// That parser folds exactly one alias, `index.docker.io`, into `docker.io`, and +// puts a reference with no path component in Hub's `library` namespace. A +// reference this cannot parse is reported unsplit. +func ReferenceRegistry(ref string) (string, string, bool) { + named, err := reference.ParseNormalizedNamed(ref) + if err != nil { + return "", "", false + } + repository := reference.Path(named) + if tagged, ok := named.(reference.Tagged); ok { + repository += ":" + tagged.Tag() + } + if digested, ok := named.(reference.Digested); ok { + repository += "@" + digested.Digest().String() + } + return reference.Domain(named), repository, true } // DockerHubRegistry is Docker Hub's canonical registry host: the one an @@ -114,6 +196,16 @@ func (registry *EnvironmentRegistry) Validate() error { if registry == nil { return nil } + if registry.Artifacts != "" { + if err := validateRegistryReference("registry artifacts", registry.Artifacts); err != nil { + return err + } + } + if strings.EqualFold(strings.TrimSpace(registry.Auth), "ghcr") { + if host, _, _ := strings.Cut(registry.URL, "/"); host != GHCRRegistry { + return fmt.Errorf("registry auth %q logs in to %s, but registry url %q is not on %s", registry.Auth, GHCRRegistry, registry.URL, GHCRRegistry) + } + } for host, mirror := range registry.Mirrors { if err := validateRegistryReference("registry mirror key", host); err != nil { return err @@ -147,7 +239,7 @@ func validateRegistryReference(kind, value string) error { return fmt.Errorf("%s %q carries a scheme; write the host alone", kind, value) } if strings.Contains(value, "@") { - return fmt.Errorf("%s %q carries a digest; a mirror is a registry, not an image", kind, value) + return fmt.Errorf("%s %q carries a digest; it names a registry, not an image", kind, value) } components := strings.Split(value, "/") for index, component := range components { @@ -157,7 +249,7 @@ func validateRegistryReference(kind, value string) error { // Only a registry host may carry a port; a colon anywhere else is the // tag separator, and a mirror names no image. if index > 0 && strings.Contains(component, ":") { - return fmt.Errorf("%s %q carries a tag; a mirror is a registry, not an image", kind, value) + return fmt.Errorf("%s %q carries a tag; it names a registry, not an image", kind, value) } } return nil diff --git a/pkg/environments/registry_pull_test.go b/pkg/environments/registry_pull_test.go new file mode 100644 index 000000000..bbb267170 --- /dev/null +++ b/pkg/environments/registry_pull_test.go @@ -0,0 +1,86 @@ +package environments + +import ( + "strings" + "testing" +) + +// A cell pulls the images an environment pushes through the mirror declared +// for the push registry; with no such mirror it pulls where it pushes. +func TestPullRepositoryReadsTheRegistryThroughItsMirror(t *testing.T) { + remote := "us-east1-docker.pkg.dev/cell/ghcr" + for _, tc := range []struct { + name string + registry *EnvironmentRegistry + want string + }{ + {"nil", nil, ""}, + {"no mirrors", &EnvironmentRegistry{URL: "ghcr.io/obin-ai"}, "ghcr.io/obin-ai"}, + {"mirror of another registry", &EnvironmentRegistry{URL: "ghcr.io/obin-ai", Mirrors: map[string]string{"docker.io": "mirror.example/hub"}}, "ghcr.io/obin-ai"}, + {"mirrored", &EnvironmentRegistry{URL: "ghcr.io/obin-ai", Mirrors: map[string]string{"ghcr.io": remote}}, remote + "/obin-ai"}, + {"trailing slash", &EnvironmentRegistry{URL: "ghcr.io/obin-ai/", Mirrors: map[string]string{"ghcr.io": remote}}, remote + "/obin-ai"}, + {"bare host", &EnvironmentRegistry{URL: "localhost:5001", Mirrors: map[string]string{"localhost:5001": remote}}, remote}, + // A Hub prefix is a namespace, not a library image. + {"hub namespace", &EnvironmentRegistry{URL: "myorg", Mirrors: map[string]string{"docker.io": remote}}, remote + "/myorg"}, + } { + t.Run(tc.name, func(t *testing.T) { + if got := tc.registry.PullRepository(); got != tc.want { + t.Fatalf("PullRepository() = %q, want %q", got, tc.want) + } + }) + } + // The same spelling a base image is mirrored with. + if got := MirroredReference("debian:trixie-slim", map[string]string{"docker.io": remote}); got != remote+"/library/debian:trixie-slim" { + t.Fatalf("MirroredReference = %q", got) + } +} + +func TestRegistryValidatesArtifactsAndGHCRAuth(t *testing.T) { + for _, tc := range []struct { + name string + registry EnvironmentRegistry + want string + }{ + {"artifacts", EnvironmentRegistry{URL: "ghcr.io/obin-ai", Artifacts: "us-east1-docker.pkg.dev/cell/bundles"}, ""}, + {"ghcr auth", EnvironmentRegistry{URL: "ghcr.io/obin-ai", Auth: "ghcr"}, ""}, + {"artifacts scheme", EnvironmentRegistry{URL: "ghcr.io/obin-ai", Artifacts: "https://bundles.example"}, "carries a scheme"}, + {"artifacts tag", EnvironmentRegistry{URL: "ghcr.io/obin-ai", Artifacts: "bundles.example/repo:latest"}, "carries a tag"}, + {"artifacts whitespace", EnvironmentRegistry{URL: "ghcr.io/obin-ai", Artifacts: " bundles.example"}, "whitespace"}, + {"ghcr auth elsewhere", EnvironmentRegistry{URL: "registry.example/team", Auth: "ghcr"}, "is not on ghcr.io"}, + {"ghcr auth lookalike", EnvironmentRegistry{URL: "ghcr.io.example/team", Auth: "ghcr"}, "is not on ghcr.io"}, + } { + t.Run(tc.name, func(t *testing.T) { + err := tc.registry.Validate() + if tc.want == "" { + if err != nil { + t.Fatal(err) + } + return + } + if err == nil || !strings.Contains(err.Error(), tc.want) { + t.Fatalf("want %q, got %v", tc.want, err) + } + }) + } +} + +// The coordinate decoder refuses unknown fields: auth, mirrors and artifacts +// are all carried by an imported contract. +func TestCoordinateContractCarriesRegistryAuthMirrorsAndArtifacts(t *testing.T) { + contract, err := ParseCoordinateContract([]byte(`{"schema":"codefly/coordinate/v1","environment":{"name":"staging","registry":{` + + `"url":"ghcr.io/obin-ai","auth":"ghcr","mirrors":{"ghcr.io":"us-east1-docker.pkg.dev/cell/ghcr"},"artifacts":"us-east1-docker.pkg.dev/cell/bundles"}}}`)) + if err != nil { + t.Fatal(err) + } + registry := contract.Environment.Registry + if registry.Auth != "ghcr" || registry.Artifacts != "us-east1-docker.pkg.dev/cell/bundles" { + t.Fatalf("registry = %+v", registry) + } + if got := registry.PullRepository(); got != "us-east1-docker.pkg.dev/cell/ghcr/obin-ai" { + t.Fatalf("PullRepository() = %q", got) + } + _, err = ParseCoordinateContract([]byte(`{"schema":"codefly/coordinate/v1","environment":{"name":"staging","registry":{"url":"registry.example/team","auth":"ghcr"}}}`)) + if err == nil || !strings.Contains(err.Error(), "is not on ghcr.io") { + t.Fatalf("ghcr auth on another registry: %v", err) + } +} diff --git a/pkg/gitops/frontend_payload.go b/pkg/gitops/frontend_payload.go index 003651af1..ba1c7eaf3 100644 --- a/pkg/gitops/frontend_payload.go +++ b/pkg/gitops/frontend_payload.go @@ -73,7 +73,13 @@ func frontendBundleRepositoryOf(env *environments.Environment, workspace string) if env.Registry == nil || strings.TrimSpace(env.Registry.URL) == "" { return "", fmt.Errorf("environment %s declares no registry.url, so a frontend bundle has nowhere to be published", env.Name) } - return strings.TrimSuffix(env.Registry.URL, "/") + "/" + workspace + "/" + FrontendBundleRepository, nil + // A bundle is not an image a mirror is proven to serve, so registry.artifacts, + // when declared, is where it is both published and read. + base := env.Registry.URL + if env.Registry.Artifacts != "" { + base = env.Registry.Artifacts + } + return strings.TrimSuffix(base, "/") + "/" + workspace + "/" + FrontendBundleRepository, nil } // artifactCacheInjection projects the artifact-cache group for a service that diff --git a/pkg/gitops/orchestrate.go b/pkg/gitops/orchestrate.go index 1e74149f2..7542998a5 100644 --- a/pkg/gitops/orchestrate.go +++ b/pkg/gitops/orchestrate.go @@ -739,11 +739,15 @@ func serviceRenderPath(module, service string) string { // declares an auth method, logs in so `docker push` can resolve the immutable // snapshot digest. Managed registries authenticate out-of-band and declare no // auth, so login is skipped. Runs once per render rather than per service. +// +// Images are named by the repository the cell pulls from (PullRepository, the +// mirror of the registry when one is declared), so the rendered manifests and +// the cell file name it; the build pushes to registry.url and logs in there. func prepareSnapshotRegistry(ctx context.Context, env *environments.Environment) error { if env.Registry == nil || strings.TrimSpace(env.Registry.URL) == "" { return fmt.Errorf("environment %s must declare registry.url for an immutable GitOps snapshot", env.Name) } - builder.SetRepository(env.Registry.URL) + builder.SetRepository(env.Registry.PullRepository()) if env.Registry.Auth != "" { if err := builder.RegistryLogin(ctx, env.Registry.URL, env.Registry.Auth); err != nil { return fmt.Errorf("authenticate snapshot registry: %w", err) diff --git a/pkg/gitops/registry_pull_test.go b/pkg/gitops/registry_pull_test.go new file mode 100644 index 000000000..e6fdd27ec --- /dev/null +++ b/pkg/gitops/registry_pull_test.go @@ -0,0 +1,56 @@ +package gitops + +import ( + "context" + "testing" + + "github.com/codefly-dev/cli/pkg/builder" + "github.com/codefly-dev/cli/pkg/environments" +) + +// A render names its images by the repository the cell pulls from — the +// remote repository mirroring ghcr.io — which is the DockerRepository every +// agent writes the manifest's image (and so the cell file's) under. +func TestSnapshotImagesAreNamedByThePullRepository(t *testing.T) { + previous, err := builder.DockerBuildContext(context.Background(), nil) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { builder.SetRepository(previous.DockerRepository) }) + for _, tc := range []struct { + registry *environments.EnvironmentRegistry + want string + }{ + {&environments.EnvironmentRegistry{URL: "ghcr.io/obin-ai", Mirrors: map[string]string{"ghcr.io": "us-east1-docker.pkg.dev/cell/ghcr"}}, "us-east1-docker.pkg.dev/cell/ghcr/obin-ai"}, + {&environments.EnvironmentRegistry{URL: "registry.example.com/team"}, "registry.example.com/team"}, + } { + if err := prepareSnapshotRegistry(context.Background(), &environments.Environment{Name: "snap", Registry: tc.registry}); err != nil { + t.Fatal(err) + } + built, err := builder.DockerBuildContext(context.Background(), nil) + if err != nil { + t.Fatal(err) + } + if built.DockerRepository != tc.want { + t.Fatalf("repository = %q, want %q", built.DockerRepository, tc.want) + } + } +} + +func TestFrontendBundlesUseTheArtifactsRegistryWhenDeclared(t *testing.T) { + for _, tc := range []struct { + registry *environments.EnvironmentRegistry + want string + }{ + {&environments.EnvironmentRegistry{URL: "ghcr.io/obin-ai", Artifacts: "us-east1-docker.pkg.dev/cell/bundles/"}, "us-east1-docker.pkg.dev/cell/bundles/platform/" + FrontendBundleRepository}, + {&environments.EnvironmentRegistry{URL: "ghcr.io/obin-ai"}, "ghcr.io/obin-ai/platform/" + FrontendBundleRepository}, + } { + got, err := frontendBundleRepositoryOf(&environments.Environment{Name: "staging", Registry: tc.registry}, "platform") + if err != nil { + t.Fatal(err) + } + if got != tc.want { + t.Fatalf("bundle repository = %q, want %q", got, tc.want) + } + } +} diff --git a/pkg/orchestration/build_plan.go b/pkg/orchestration/build_plan.go index ef0f21367..015276108 100644 --- a/pkg/orchestration/build_plan.go +++ b/pkg/orchestration/build_plan.go @@ -16,6 +16,7 @@ import ( "google.golang.org/protobuf/proto" + "github.com/codefly-dev/cli/pkg/environments" coreservices "github.com/codefly-dev/core/agents/services" builderv0 "github.com/codefly-dev/core/generated/go/codefly/services/builder/v0" "github.com/codefly-dev/core/wool" @@ -159,7 +160,14 @@ func (b *Builder) decideImageReuse( // built. An unversioned service (0.0.0, never published) is not a release and // is built as before. if shouldPush && releasedVersionTag(recipe.GetImage()) { - if digest, err := resolveImageManifestDigest(ctx, recipe.GetImage(), probeBuilder); err == nil && digest != "" { + digest, err := resolveImageManifestDigest(ctx, recipe.GetImage(), probeBuilder) + if err != nil && !isImageNotFound(err) { + // Only the registry saying the tag does not exist means it is not + // published: building over a probe that failed for any other reason + // would overwrite a released tag wherever tags are mutable. + return decision, w.Wrapf(err, "cannot tell whether %s is already published", recipe.GetImage()) + } + if err == nil && digest != "" { // The version alone is not proof: a service whose version was not moved // with its code would hand back another release's image. The published // image records the inputs it was built from, and only those inputs @@ -220,13 +228,24 @@ func placeholderProxies(recipe *builderv0.DockerBuildRecipe) map[string]string { } // recipeAsBuilt is the recipe as buildRecipe builds it: targeted to the -// environment's platforms when the image is pushed. The reuse decision is taken -// on this form, so it binds the platforms the build will actually run. +// environment's platforms when the image is pushed, and named by the +// reference it is pushed to (pushReference) when the cell pulls it through a +// mirror. The reuse decision is taken on this form, so it binds the platforms +// the build will actually run and probes the registry the image is pushed to; +// the manifests keep the name the cell pulls, with the pushed digest. func (b *Builder) recipeAsBuilt(recipe *builderv0.DockerBuildRecipe, shouldPush bool) (*builderv0.DockerBuildRecipe, error) { if !shouldPush { return recipe, nil } - return b.targetEnvironmentPlatforms(recipe) + targeted, err := b.targetEnvironmentPlatforms(recipe) + if err != nil { + return nil, err + } + if pushed := pushReference(b.world.Env, targeted.GetImage()); pushed != targeted.GetImage() { + targeted = proto.CloneOf(targeted) + targeted.Image = pushed + } + return targeted, nil } // recipeContextRoot is the directory a recipe's context is resolved against. @@ -390,6 +409,9 @@ func (b *Builder) buildRecipe( args, err := cachedBuildxArgs(recipe, dockerfile, contextDir, shouldPush, multiArch, metadataFile, builderName, cache, private) if err == nil { args = withBuildInputsLabel(args, identity.Key) + if shouldPush { + args = withSourceLabel(args, b.world.Env, workspaceSourceRepository(ctx, b.world.Workspace.Dir()), multiArch) + } } if err != nil { return err @@ -1014,16 +1036,62 @@ func withBuildInputsLabel(args []string, identityKey string) []string { if identityKey == "" { return args } + return withBuildFlags(args, "--label", buildInputsLabel+"="+identityKey) +} + +// withBuildFlags inserts flags right after the `build` subcommand of a buildx +// argv, leaving an argv with none unchanged. +func withBuildFlags(args []string, flags ...string) []string { for index, argument := range args { if argument == "build" { labelled := append([]string(nil), args[:index+1]...) - labelled = append(labelled, "--label", buildInputsLabel+"="+identityKey) + labelled = append(labelled, flags...) return append(labelled, args[index+1:]...) } } return args } +// imageSourceLabel is the OCI annotation GitHub Container Registry links a +// package to its repository by, which is how a private package inherits the +// repository's access. +const imageSourceLabel = "org.opencontainers.image.source" + +// withSourceLabel links an image pushed to ghcr.io to the workspace's GitHub +// repository. Like withBuildInputsLabel it is added after the identity was +// computed, so it is never an input to it; a multi-platform push also +// annotates the index, which is the manifest the registry reads it from. +func withSourceLabel(args []string, env *environments.Environment, source string, multiArch bool) []string { + if source == "" || env == nil || env.Registry == nil { + return args + } + if host, _, _ := strings.Cut(env.Registry.URL, "/"); host != environments.GHCRRegistry { + return args + } + flags := []string{"--label", imageSourceLabel + "=" + source} + if multiArch { + flags = append(flags, "--annotation", "index:"+imageSourceLabel+"="+source) + } + return withBuildFlags(args, flags...) +} + +// githubRemote matches the GitHub remotes a workspace is cloned from. +var githubRemote = regexp.MustCompile(`^(?:git@github\.com:|(?:https|ssh)://(?:git@)?github\.com/)([A-Za-z0-9_.-]+)/([A-Za-z0-9_.-]+?)(?:\.git)?/?$`) + +// workspaceSourceRepository is the https URL of the GitHub repository the +// workspace is checked out from, or "" when it is not one. A seam for tests. +var workspaceSourceRepository = func(ctx context.Context, dir string) string { + out, err := exec.CommandContext(ctx, "git", "-C", dir, "remote", "get-url", "origin").Output() + if err != nil { + return "" + } + match := githubRemote.FindStringSubmatch(strings.TrimSpace(string(out))) + if match == nil { + return "" + } + return "https://github.com/" + match[1] + "/" + match[2] +} + // resolveImageBuildInputs reads the build-inputs label every platform of a // published image carries, or "" when it carries none or its platforms disagree. // A seam so a test needs no registry. diff --git a/pkg/orchestration/builder.go b/pkg/orchestration/builder.go index e6c8d2322..b3acd4579 100644 --- a/pkg/orchestration/builder.go +++ b/pkg/orchestration/builder.go @@ -17,6 +17,7 @@ import ( "github.com/codefly-dev/core/resources" "github.com/codefly-dev/cli/pkg/builder" + "github.com/codefly-dev/cli/pkg/environments" dockerhelpers "github.com/codefly-dev/core/agents/helpers/docker" coreservices "github.com/codefly-dev/core/agents/services" coresbom "github.com/codefly-dev/core/agents/services/sbom" @@ -467,24 +468,32 @@ func (b *Builder) Build(ctx context.Context) (*OutputProperty, error) { if err = verifyImageArchitecture(ctx, im); err != nil { return nil, w.Wrapf(err, "refusing to push %s", im) } - cmd := exec.CommandContext(ctx, "docker", "push", im) - if pushErr := cmd.Run(); pushErr != nil { + if pushErr := pushLegacyImage(ctx, b.world.Env, im); pushErr != nil { return nil, w.Wrapf(pushErr, "cannot push docker image") } } } + // The digest is read from the reference that was pushed: only it has a + // repository digest. + var deployed string + if len(buildResult.Images) == 1 { + deployed = buildResult.Images[0] + if b.world.Push { + deployed = pushReference(b.world.Env, deployed) + } + } if b.world.Mode == SnapshotMode && len(buildResult.Images) > 0 { if len(buildResult.Images) != 1 { return nil, w.NewError("snapshot build returned %d images for %s; exactly one deployable image is required", len(buildResult.Images), b.instance.Unique()) } - b.imageDigest, err = inspectImageDigest(ctx, buildResult.Images[0]) + b.imageDigest, err = inspectImageDigest(ctx, deployed) if err != nil { return nil, w.Wrapf(err, "cannot resolve immutable image for %s", b.instance.Unique()) } } else if b.world.CaptureImageDigest && b.world.Push && len(buildResult.Images) == 1 { // The image is already pushed; resolving its digest is best-effort // for reporting, so a lookup failure must not fail the build. - if digest, digestErr := inspectImageDigest(ctx, buildResult.Images[0]); digestErr != nil { + if digest, digestErr := inspectImageDigest(ctx, deployed); digestErr != nil { w.Warn("built and pushed but could not resolve image digest", wool.ErrField(digestErr)) } else { b.imageDigest = digest @@ -512,6 +521,25 @@ func (b *Builder) Build(ctx context.Context) (*OutputProperty, error) { var sha256Digest = regexp.MustCompile(`^sha256:[0-9a-f]{64}$`) +// runDocker runs one docker command of the legacy push path. A seam so a test +// needs no daemon. +var runDocker = func(ctx context.Context, args ...string) error { + return exec.CommandContext(ctx, "docker", args...).Run() +} + +// pushLegacyImage pushes an image an agent built in-process. One named by the +// cell's pull repository is tagged and pushed to the registry that repository +// mirrors (pushReference, recipeAsBuilt's rule). +func pushLegacyImage(ctx context.Context, env *environments.Environment, image string) error { + pushed := pushReference(env, image) + if pushed != image { + if err := runDocker(ctx, "tag", image, pushed); err != nil { + return fmt.Errorf("tag %s as %s: %w", image, pushed, err) + } + } + return runDocker(ctx, "push", pushed) +} + func inspectImageDigest(ctx context.Context, image string) (string, error) { output, err := exec.CommandContext( ctx, diff --git a/pkg/orchestration/image_cache.go b/pkg/orchestration/image_cache.go index 21f86fdf7..2fa416040 100644 --- a/pkg/orchestration/image_cache.go +++ b/pkg/orchestration/image_cache.go @@ -488,7 +488,7 @@ var resolveImageManifestDigest = func(ctx context.Context, reference, builderNam args = append(args, "--raw", reference) manifest, err := exec.CommandContext(probe, "docker", args...).Output() if err != nil { - return "", err + return "", classifyImageProbeError(err) } if len(bytes.TrimSpace(manifest)) == 0 { return "", fmt.Errorf("registry served no manifest for %s", reference) @@ -497,6 +497,34 @@ var resolveImageManifestDigest = func(ctx context.Context, reference, builderNam return "sha256:" + hex.EncodeToString(sum[:]), nil } +// errImageNotFound is a registry's answer that a reference names nothing. +var errImageNotFound = errors.New("image not found") + +// imageNotFoundAnswers are how a registry, through buildx, says a reference +// does not exist: the distribution spec's error codes and buildx's own +// rendering of a 404. +var imageNotFoundAnswers = []string{"not found", "manifest unknown", "name unknown"} + +// classifyImageProbeError marks a probe the registry answered with "no such +// image" as errImageNotFound, keeping every other failure (an auth refusal, a +// timeout, an unreachable registry) what it was. +func classifyImageProbeError(err error) error { + var exit *exec.ExitError + if errors.As(err, &exit) { + answer := strings.ToLower(string(exit.Stderr)) + for _, notFound := range imageNotFoundAnswers { + if strings.Contains(answer, notFound) { + return fmt.Errorf("%w: %s", errImageNotFound, strings.TrimSpace(string(exit.Stderr))) + } + } + return fmt.Errorf("%w: %s", err, strings.TrimSpace(string(exit.Stderr))) + } + return err +} + +// isImageNotFound reports whether a probe found the reference absent. +func isImageNotFound(err error) bool { return errors.Is(err, errImageNotFound) } + // imageRegistryProbeTimeout bounds every registry read the reuse decision makes. // A registry that accepts a connection and then stalls would otherwise hang a // check whose whole purpose is to be cheaper than the build it replaces. diff --git a/pkg/orchestration/image_cache_test.go b/pkg/orchestration/image_cache_test.go index 73184b395..a399baa77 100644 --- a/pkg/orchestration/image_cache_test.go +++ b/pkg/orchestration/image_cache_test.go @@ -9,6 +9,7 @@ import ( "testing" "time" + "github.com/codefly-dev/cli/pkg/environments" coreservices "github.com/codefly-dev/core/agents/services" builderv0 "github.com/codefly-dev/core/generated/go/codefly/services/builder/v0" "github.com/codefly-dev/core/resources" @@ -168,6 +169,8 @@ type imageCacheHarness struct { client *coreservices.BuilderAgent push bool rebuild bool + // env is the environment the build targets, nil for none. + env *environments.Environment // goCalls records every go command the prefetch of each build runs. goCalls []goCall // gitCalls records every git command the git source prefetch runs. @@ -236,6 +239,7 @@ func (harness *imageCacheHarness) build(t *testing.T) string { instance.Builder = &services.BuilderInstance{Instance: instance, Builder: harness.client} world := &World{ Workspace: workspace, + Env: harness.env, Push: harness.push, CaptureImageDigest: harness.push, BuildxBuilder: "stub", @@ -867,7 +871,7 @@ func stubPublishedImage(t *testing.T, digest, inputs string) func() { resolveImageManifestDigest = func(ctx context.Context, reference, builder string) (string, error) { if strings.HasSuffix(reference, ":1.2.3") { if digest == "" { - return "", fmt.Errorf("not published") + return "", fmt.Errorf("%w: not published", errImageNotFound) } return digest, nil } @@ -905,7 +909,7 @@ func (harness *imageCacheHarness) tryBuild(ctx context.Context, t *testing.T) er service.WithDir(harness.serviceDir) instance := &services.Instance{Service: service, Identity: &resources.ServiceIdentity{Name: "api", Module: "saas"}} instance.Builder = &services.BuilderInstance{Instance: instance, Builder: harness.client} - world := &World{Workspace: workspace, Push: harness.push, CaptureImageDigest: harness.push, BuildxBuilder: "stub"} + world := &World{Workspace: workspace, Env: harness.env, Push: harness.push, CaptureImageDigest: harness.push, BuildxBuilder: "stub"} prefetch := newGoModulePrefetch() prefetch.run = recordingGo(&harness.goCalls, nil) world.goModulePrefetch = prefetch diff --git a/pkg/orchestration/registry_mirror.go b/pkg/orchestration/registry_mirror.go index 3a0c3d1f7..7dbc394b1 100644 --- a/pkg/orchestration/registry_mirror.go +++ b/pkg/orchestration/registry_mirror.go @@ -10,7 +10,6 @@ import ( "strings" "github.com/codefly-dev/cli/pkg/environments" - "github.com/distribution/reference" ) // REGISTRY MIRRORS: READING A BASE IMAGE FROM SOMEWHERE ELSE. @@ -49,56 +48,31 @@ func registryMirrors(env *environments.Environment) map[string]string { return env.Registry.Mirrors } -// mirroredReference is the reference to read `reference` from: the same -// repository and tag under the mirror declared for its registry, or the -// reference itself when no mirror is declared for it. -// -// A mirror of Docker Hub holds the upstream repository path verbatim, which for -// an unqualified reference is the `library` namespace Docker resolves it to — -// so `debian:trixie-slim` is read as `/library/debian:trixie-slim`, -// which is what an Artifact Registry remote repository (and a pull-through -// registry generally) serves it at. +// mirroredReference is the reference to read `reference` from. It is +// environments.MirroredReference, the one spelling a cell's pull repository +// (EnvironmentRegistry.PullRepository) is mirrored with too. func mirroredReference(ref string, mirrors map[string]string) string { - if len(mirrors) == 0 { - return ref - } - host, repository, ok := referenceRegistry(ref) - if !ok { - return ref - } - mirror := mirrors[host] - if mirror == "" { - return ref - } - return mirror + "/" + repository + return environments.MirroredReference(ref, mirrors) } -// referenceRegistry splits an image reference into the host of the registry it -// resolves against and the repository (with its tag or digest) within it, using -// the same parser the daemon, BuildKit and containerd all normalize references -// with — so the host this keys a mirror on is the host BuildKit will look that -// mirror up under, and the two cannot disagree about what a reference names. -// -// That parser folds exactly one alias, `index.docker.io`, into `docker.io`, and -// puts a reference with no path component in Hub's `library` namespace. -// `registry-1.docker.io` is NOT folded — it is a distinct registry to the -// parser and to BuildKit alike, so a reference spelling it reads through a -// mirror declared under that spelling and no other. A reference this cannot -// parse is reported unsplit: the build will fail on it with the registry's own -// error, which says more than a guess made here would. -func referenceRegistry(ref string) (string, string, bool) { - named, err := reference.ParseNormalizedNamed(ref) - if err != nil { - return "", "", false +// pushReference is the reference an image named under the environment's pull +// repository (EnvironmentRegistry.PullRepository) is pushed to: the same +// repository under registry.url. An image named under anything else, and every +// image of an environment whose registry is not mirrored, is pushed where it +// names. +func pushReference(env *environments.Environment, image string) string { + if env == nil || env.Registry == nil { + return image } - repository := reference.Path(named) - if tagged, ok := named.(reference.Tagged); ok { - repository += ":" + tagged.Tag() + pull := strings.TrimSuffix(env.Registry.PullRepository(), "/") + push := strings.TrimSuffix(env.Registry.URL, "/") + if pull == push || pull == "" { + return image } - if digested, ok := named.(reference.Digested); ok { - repository += "@" + digested.Digest().String() + if rest, ok := strings.CutPrefix(image, pull+"/"); ok { + return push + "/" + rest } - return reference.Domain(named), repository, true + return image } // buildkitRegistryConfig renders the mirrors as the buildkitd configuration a diff --git a/pkg/orchestration/registry_mirror_test.go b/pkg/orchestration/registry_mirror_test.go index 82fd94249..ff4827f4a 100644 --- a/pkg/orchestration/registry_mirror_test.go +++ b/pkg/orchestration/registry_mirror_test.go @@ -121,7 +121,7 @@ func TestNoBaseImageReadReachesTheMirroredRegistry(t *testing.T) { asked = append(asked, reference) // The blocked host: Docker Hub answers nothing at all here, so a read // that reached it would fail the recipe rather than pass unnoticed. - if host, _, _ := referenceRegistry(reference); host == environments.DockerHubRegistry { + if host, _, _ := environments.ReferenceRegistry(reference); host == environments.DockerHubRegistry { return "", fmt.Errorf("dial registry-1.docker.io: blocked") } return "sha256:" + strings.Repeat("a", 64), nil diff --git a/pkg/orchestration/registry_pull_test.go b/pkg/orchestration/registry_pull_test.go new file mode 100644 index 000000000..80b424a25 --- /dev/null +++ b/pkg/orchestration/registry_pull_test.go @@ -0,0 +1,174 @@ +package orchestration + +import ( + "context" + "errors" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/codefly-dev/cli/pkg/environments" + builderv0 "github.com/codefly-dev/core/generated/go/codefly/services/builder/v0" + "github.com/stretchr/testify/require" +) + +// remoteRepository is a cell's Artifact Registry remote repository of ghcr.io. +const remoteRepository = "us-east1-docker.pkg.dev/cell/ghcr" + +// pullThroughEnv pushes to ghcr.io and pulls through the cell's remote repository. +func pullThroughEnv() *environments.Environment { + return &environments.Environment{Name: "staging", Registry: &environments.EnvironmentRegistry{ + URL: "ghcr.io/obin-ai", Mirrors: map[string]string{"ghcr.io": remoteRepository}, + }} +} + +// recordBuildArgs makes the next `buildx build` write its argv to a file. +func (stub *dockerStub) recordBuildArgs(t *testing.T) func() string { + t.Helper() + path := filepath.Join(stub.dir, "args") + require.NoError(t, os.WriteFile(filepath.Join(stub.dir, "hook"), []byte(`printf '%s\n' "$*" > '`+path+"'\n"), 0o644)) + return func() string { return strings.TrimSpace(readFile(t, path)) } +} + +func readFile(t *testing.T, path string) string { + t.Helper() + payload, err := os.ReadFile(path) + require.NoError(t, err) + return string(payload) +} + +func TestPushReferenceMovesOnlyImagesUnderThePullRepository(t *testing.T) { + env := pullThroughEnv() + require.Equal(t, "ghcr.io/obin-ai/saas/api:1.2.3", pushReference(env, remoteRepository+"/obin-ai/saas/api:1.2.3")) + require.Equal(t, "docker.io/library/postgres:16", pushReference(env, "docker.io/library/postgres:16"), "a foreign image is untouched") + require.Equal(t, remoteRepository+"/obin-ai-other/api:1", pushReference(env, remoteRepository+"/obin-ai-other/api:1"), "a prefix is a whole path component") + unmirrored := &environments.Environment{Registry: &environments.EnvironmentRegistry{URL: "ghcr.io/obin-ai"}} + require.Equal(t, "ghcr.io/obin-ai/saas/api:1", pushReference(unmirrored, "ghcr.io/obin-ai/saas/api:1")) + require.Equal(t, "x/api:1", pushReference(nil, "x/api:1")) +} + +func TestRecipeAsBuiltNamesThePushedReferenceAndLeavesThePlanAlone(t *testing.T) { + recipe := &builderv0.DockerBuildRecipe{Name: "api", Image: remoteRepository + "/obin-ai/saas/api:1.2.3", Platforms: []string{"linux/amd64"}} + builder := &Builder{world: &World{Env: pullThroughEnv()}} + built, err := builder.recipeAsBuilt(recipe, true) + require.NoError(t, err) + require.Equal(t, "ghcr.io/obin-ai/saas/api:1.2.3", built.GetImage()) + require.Equal(t, remoteRepository+"/obin-ai/saas/api:1.2.3", recipe.GetImage(), "the plan keeps the name the cell pulls") + local, err := builder.recipeAsBuilt(recipe, false) + require.NoError(t, err) + require.Same(t, recipe, local, "a build that does not push is not renamed") +} + +// A released version is probed, tagged and pushed at ghcr.io; the digest the +// manifests pin is the pushed one. +func TestAReleasedVersionIsProbedAndPushedAtTheRegistryItPullsThrough(t *testing.T) { + harness := newImageCacheHarness(t, true, true) + harness.env = pullThroughEnv() + harness.peer.image = remoteRepository + "/obin-ai/saas/api:1.2.3" + var probed []string + previous := resolveImageManifestDigest + t.Cleanup(func() { resolveImageManifestDigest = previous }) + resolveImageManifestDigest = func(ctx context.Context, reference, builder string) (string, error) { + if strings.HasSuffix(reference, ":1.2.3") { + probed = append(probed, reference) + return "", fmt.Errorf("%w: not published", errImageNotFound) + } + return previous(ctx, reference, builder) + } + args := harness.docker.recordBuildArgs(t) + require.NotEmpty(t, harness.build(t)) + require.Equal(t, []string{"ghcr.io/obin-ai/saas/api:1.2.3"}, probed) + require.Contains(t, args(), "-t ghcr.io/obin-ai/saas/api:1.2.3") + require.NotContains(t, args(), remoteRepository) +} + +// Only the registry saying "no such tag" lets a released version be built: any +// other failed probe could overwrite a released tag. +func TestAReleasedVersionProbeThatFailsOtherwiseStopsTheRender(t *testing.T) { + harness := newImageCacheHarness(t, true, true) + harness.peer.image = "example.test/app:1.2.3" + previous := resolveImageManifestDigest + t.Cleanup(func() { resolveImageManifestDigest = previous }) + resolveImageManifestDigest = func(context.Context, string, string) (string, error) { + return "", errors.New("denied: unauthorized") + } + ctx, cancel := context.WithTimeout(t.Context(), time.Minute) + defer cancel() + require.ErrorContains(t, harness.tryBuild(ctx, t), "cannot tell whether example.test/app:1.2.3 is already published") + require.Equal(t, 0, harness.docker.calls(t, "build")) +} + +func TestImageProbeClassifiesOnlyARegistryNotFoundAsAbsent(t *testing.T) { + probe := func(stderr string) error { + _, err := exec.Command("sh", "-c", "printf '%s' \"$0\" >&2; exit 1", stderr).Output() + return classifyImageProbeError(err) + } + require.True(t, isImageNotFound(probe("ERROR: ghcr.io/obin-ai/api:1.2.3: not found"))) + require.True(t, isImageNotFound(probe("MANIFEST_UNKNOWN: manifest unknown"))) + unauthorized := probe("ERROR: failed to authorize: 401 Unauthorized") + require.False(t, isImageNotFound(unauthorized)) + require.ErrorContains(t, unauthorized, "401 Unauthorized") + require.False(t, isImageNotFound(classifyImageProbeError(context.DeadlineExceeded))) +} + +// The source label links a ghcr.io package to the workspace's repository; it +// is added after the identity, so a different remote reuses the same image. +func TestSourceLabelLinksAGHCRImageAndIsNotPartOfTheIdentity(t *testing.T) { + previous := workspaceSourceRepository + t.Cleanup(func() { workspaceSourceRepository = previous }) + source := "https://github.com/obin-ai/platform" + workspaceSourceRepository = func(context.Context, string) string { return source } + + harness := newImageCacheHarness(t, true, true) + harness.env = &environments.Environment{Registry: &environments.EnvironmentRegistry{URL: "ghcr.io/obin-ai"}} + args := harness.docker.recordBuildArgs(t) + first := harness.build(t) + require.Contains(t, args(), "--label "+imageSourceLabel+"="+source) + key := harness.key(t) + + source = "https://github.com/obin-ai/elsewhere" + require.Equal(t, first, harness.build(t)) + require.Equal(t, 1, harness.docker.calls(t, "build"), "the source label is not an image input") + require.Equal(t, key, harness.key(t)) + + base := []string{"buildx", "build", "--push", "."} + require.Equal(t, base, withSourceLabel(base, &environments.Environment{Registry: &environments.EnvironmentRegistry{URL: "registry.example/team"}}, source, false)) + require.Equal(t, []string{"buildx", "build", "--label", imageSourceLabel + "=" + source, "--annotation", "index:" + imageSourceLabel + "=" + source, "--push", "."}, + withSourceLabel(base, harness.env, source, true)) + require.Equal(t, base, withSourceLabel(base, harness.env, "", true)) + for remote, want := range map[string]string{ + "git@github.com:obin-ai/platform.git": "https://github.com/obin-ai/platform", + "https://github.com/obin-ai/platform.git": "https://github.com/obin-ai/platform", + "ssh://git@github.com/obin-ai/platform": "https://github.com/obin-ai/platform", + "https://gitlab.example/obin-ai/platform.git": "", + } { + match := githubRemote.FindStringSubmatch(remote) + got := "" + if match != nil { + got = "https://github.com/" + match[1] + "/" + match[2] + } + require.Equal(t, want, got, remote) + } +} + +// An image an agent built in-process is tagged and pushed at ghcr.io. +func TestLegacyPushTagsAndPushesThePushedReference(t *testing.T) { + previous := runDocker + t.Cleanup(func() { runDocker = previous }) + var calls []string + runDocker = func(_ context.Context, args ...string) error { + calls = append(calls, strings.Join(args, " ")) + return nil + } + pulled := remoteRepository + "/obin-ai/saas/api:1.2.3" + require.NoError(t, pushLegacyImage(t.Context(), pullThroughEnv(), pulled)) + require.Equal(t, []string{"tag " + pulled + " ghcr.io/obin-ai/saas/api:1.2.3", "push ghcr.io/obin-ai/saas/api:1.2.3"}, calls) + + calls = nil + require.NoError(t, pushLegacyImage(t.Context(), nil, "registry.example/team/api:1")) + require.Equal(t, []string{"push registry.example/team/api:1"}, calls, "an unmirrored registry pushes as before") +} From d602750f062f935608954cd2aeeacf99b28ea4bc Mon Sep 17 00:00:00 2001 From: Antoine Toussaint <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Sat, 24 Oct 2026 14:35:32 -0400 Subject: [PATCH 2/2] fix(environment): re-import withdraws registry.artifacts and registry.auth Like registry.mirrors (cli#1140), both are contract-owned: a contract that declares registry without them withdraws the value an earlier import wrote. Co-Authored-By: Claude Opus 5.5 --- cmd/environment/import.go | 10 ++++++---- cmd/environment/import_test.go | 18 ++++++++++++++++++ docs/commands.md | 9 +++++---- 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/cmd/environment/import.go b/cmd/environment/import.go index 3cb8bad78..872224d84 100644 --- a/cmd/environment/import.go +++ b/cmd/environment/import.go @@ -465,10 +465,12 @@ func applyContractFields(target, declaration *yaml.Node) { // contractOwnedOptional names the keys a coordinate contract owns whose ABSENCE is // itself a statement (docs/coordinate-contract-conventions.md in infra-base): an absent -// registry.mirrors means "pull the upstream as the Dockerfile names it". Merging alone -// would keep a mirror the contract withdrew, and the builder would keep mapping to a -// repository that no longer exists. -var contractOwnedOptional = map[string][]string{"registry": {"mirrors"}} +// registry.mirrors means "pull the upstream as the Dockerfile names it", an absent +// registry.artifacts "publish bundles to registry.url", an absent registry.auth "log +// in to nothing". Merging alone would keep a value the contract withdrew, and the +// render would keep pulling from, publishing to or logging in to a registry the +// environment no longer holds. +var contractOwnedOptional = map[string][]string{"registry": {"mirrors", "artifacts", "auth"}} // withdrawContractOwnedAbsences removes each contract-owned optional key the contract // no longer carries, under a parent the contract does declare. A parent the contract diff --git a/cmd/environment/import_test.go b/cmd/environment/import_test.go index 024a345f6..e7d1b6dc2 100644 --- a/cmd/environment/import_test.go +++ b/cmd/environment/import_test.go @@ -838,6 +838,24 @@ func TestImportRefusesANullRunnableScopeSelectionWithoutWriting(t *testing.T) { } } +func TestReimportWithdrawsArtifactsAndAuthTheContractNoLongerCarries(t *testing.T) { + dir := writeWorkspace(t, existingAzureWorkspace) + declared := strings.Replace(fixtureContract, `"registry": {"url": "x.azurecr.io"}`, + `"registry": {"url": "x.azurecr.io", "auth": "acr", "artifacts": "x.azurecr.io/bundles"}`, 1) + doImport(t, dir, importOptions{contractData: []byte(declared)}) + if got := loadWorkspace(t, dir).FindEnvironment("azure").Registry; got.Auth != "acr" || got.Artifacts != "x.azurecr.io/bundles" { + t.Fatalf("auth and artifacts not imported: %+v", got) + } + doImport(t, dir, importOptions{contractData: []byte(fixtureContract)}) + env := loadWorkspace(t, dir).FindEnvironment("azure") + if env.Registry.Auth != "" || env.Registry.Artifacts != "" || env.Registry.URL != "x.azurecr.io" { + t.Fatalf("a value the contract withdrew survived re-import: %+v", env.Registry) + } + if strings.Contains(readFile(t, filepath.Join(dir, resources.WorkspaceConfigurationName)), "bundles") { + t.Fatal("the withdrawn artifacts registry is still written in the workspace") + } +} + func TestReimportWithdrawsAMirrorTheContractNoLongerCarries(t *testing.T) { dir := writeWorkspace(t, existingAzureWorkspace) mirrored := strings.Replace(fixtureContract, `"registry": {"url": "x.azurecr.io"}`, diff --git a/docs/commands.md b/docs/commands.md index 80b2e4c2a..5374f5ea4 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -2318,10 +2318,11 @@ former spelling of `--coordinate-contract` for one release. Declared fields replace their exact named values. Maps merge by explicit key; omitted fields and unrelated entries remain intact. Explicit empty maps, empty sequences and nulls clear the corresponding field where its schema permits. -One key is the contract's alone and its absence is a statement: `registry.mirrors`. -A contract that declares `registry` without `mirrors` withdraws any mirror an earlier -import wrote, so the builder stops mapping to a repository the environment no longer -holds. +Three keys are the contract's alone and their absence is a statement: +`registry.mirrors`, `registry.artifacts` and `registry.auth`. A contract that declares +`registry` without one of them withdraws the value an earlier import wrote, so the +render stops pulling from, publishing bundles to or logging in to a registry the +environment no longer holds. There is no special `store` alias, single-database limit, default secret path or ignored producer extension. Unknown fields and capabilities fail validation.