diff --git a/cmd/environment/import.go b/cmd/environment/import.go index 3cb8bad7..872224d8 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 024a345f..e7d1b6dc 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 ca23799e..5374f5ea 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 @@ -2275,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. @@ -3919,8 +3963,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 898bf35b..5d376aa8 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 4f236a9b..c36e4fb7 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 8240a7f8..aa430bac 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 00000000..bbb26717 --- /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 003651af..ba1c7eaf 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 1e74149f..7542998a 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 00000000..e6fdd27e --- /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 ef0f2136..01527610 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 e6c8d232..b3acd457 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 21f86fdf..2fa41604 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 73184b39..a399baa7 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 3a0c3d1f..7dbc394b 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 82fd9424..ff4827f4 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 00000000..80b424a2 --- /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") +}