Repository navigation
Conversation
When builds.publish is configured, the host now copies the completed local image to <remote-registry>/<prefix>/<build-id>@<digest> and verifies the remote manifest by digest before the build reports ready. Build.ImageRef records the immutable remote reference; a publication failure fails the build. Local image conversion is unchanged. The copy runs entirely on the host using the existing go-containerregistry client: the local pull uses a short-lived token generated in-process, and remote credentials come from a host-side Docker client config. Neither enters the builder VM. Blob and manifest uploads are content-addressed and idempotent, and transient registry errors are retried. Tests use in-memory registries to cover destination derivation, successful publication and verification, digest mismatch and not-found verification failures, idempotent re-publish, retry of transient failures, permanent-failure short-circuiting, credential-file auth, credential confinement from the builder VM, and build-level error propagation.
rgarcia
marked this pull request as ready for review
August 2, 2026 03:29
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Local registry TLS settings ignored
- The publisher now receives local registry insecure/CA settings, parses local references with
name.Insecurewhen configured, and uses a CA-augmented HTTP transport when fetching the local image.
- The publisher now receives local registry insecure/CA settings, parses local references with
- ✅ Fixed: Credential host key mismatch
- Docker auth keys are now normalized to strip scheme and path suffixes before storage and lookup so
/v1and/v2entries resolve correctly.
- Docker auth keys are now normalized to strip scheme and path suffixes before storage and lookup so
Or push these changes by commenting:
@cursor push 3a2b61815e
Preview (3a2b61815e)
diff --git a/lib/builds/manager.go b/lib/builds/manager.go
--- a/lib/builds/manager.go
+++ b/lib/builds/manager.go
@@ -152,7 +152,13 @@
}
tokenGen := NewRegistryTokenGenerator(config.RegistrySecret)
- publisher, err := newBuildPublisher(config.Publish, stripRegistryScheme(config.RegistryURL), tokenGen)
+ publisher, err := newBuildPublisher(
+ config.Publish,
+ stripRegistryScheme(config.RegistryURL),
+ config.RegistryInsecure,
+ config.RegistryCACert,
+ tokenGen,
+ )
if err != nil {
return nil, fmt.Errorf("publish config: %w", err)
}
diff --git a/lib/builds/publish_remote.go b/lib/builds/publish_remote.go
--- a/lib/builds/publish_remote.go
+++ b/lib/builds/publish_remote.go
@@ -2,10 +2,13 @@
import (
"context"
+ "crypto/tls"
+ "crypto/x509"
"encoding/base64"
"encoding/json"
"errors"
"fmt"
+ "net/http"
"os"
"strings"
"time"
@@ -23,22 +26,38 @@
// are read from a host-side Docker client config. Neither is sent to builder
// VMs.
type remotePublisher struct {
- config PublishConfig
- localRegistry string
- tokenGen *RegistryTokenGenerator
- keychain authn.Keychain
+ config PublishConfig
+ localRegistry string
+ localRegistryInsecure bool
+ localTransport http.RoundTripper
+ tokenGen *RegistryTokenGenerator
+ keychain authn.Keychain
}
-func newRemotePublisher(cfg PublishConfig, localRegistry string, tokenGen *RegistryTokenGenerator) (BuildPublisher, error) {
+func newRemotePublisher(
+ cfg PublishConfig,
+ localRegistry string,
+ localRegistryInsecure bool,
+ localRegistryCACert string,
+ tokenGen *RegistryTokenGenerator,
+) (BuildPublisher, error) {
keychain, err := dockerFileKeychain(cfg.CredentialsFile)
if err != nil {
return nil, err
}
+
+ localTransport, err := newLocalRegistryTransport(localRegistryCACert)
+ if err != nil {
+ return nil, err
+ }
+
return &remotePublisher{
- config: cfg,
- localRegistry: localRegistry,
- tokenGen: tokenGen,
- keychain: keychain,
+ config: cfg,
+ localRegistry: localRegistry,
+ localRegistryInsecure: localRegistryInsecure,
+ localTransport: localTransport,
+ tokenGen: tokenGen,
+ keychain: keychain,
}, nil
}
@@ -51,7 +70,7 @@
return "", fmt.Errorf("publish: digest is required")
}
- srcRef, err := name.ParseReference(fmt.Sprintf("%s/%s@%s", p.localRegistry, localRef, digest))
+ srcRef, err := parseLocalSourceRef(p.localRegistry, localRef, digest, p.localRegistryInsecure)
if err != nil {
return "", fmt.Errorf("publish: parse local reference: %w", err)
}
@@ -67,7 +86,15 @@
}
localAuth := authn.FromConfig(authn.AuthConfig{Username: token, Password: "x"})
- img, err := remote.Image(srcRef, remote.WithContext(ctx), remote.WithAuth(localAuth))
+ localOpts := []remote.Option{
+ remote.WithContext(ctx),
+ remote.WithAuth(localAuth),
+ }
+ if p.localTransport != nil {
+ localOpts = append(localOpts, remote.WithTransport(p.localTransport))
+ }
+
+ img, err := remote.Image(srcRef, localOpts...)
if err != nil {
return "", fmt.Errorf("publish: fetch local image: %w", err)
}
@@ -122,6 +149,43 @@
return nil
}
+func parseLocalSourceRef(localRegistry, localRef, digest string, insecure bool) (name.Reference, error) {
+ options := []name.Option{}
+ if insecure {
+ options = append(options, name.Insecure)
+ }
+ return name.ParseReference(fmt.Sprintf("%s/%s@%s", localRegistry, localRef, digest), options...)
+}
+
+func newLocalRegistryTransport(caCert string) (http.RoundTripper, error) {
+ if caCert == "" {
+ return nil, nil
+ }
+
+ pool, err := x509.SystemCertPool()
+ if err != nil || pool == nil {
+ pool = x509.NewCertPool()
+ }
+ if ok := pool.AppendCertsFromPEM([]byte(caCert)); !ok {
+ return nil, fmt.Errorf("publish: parse registry_ca_cert: no certificates found")
+ }
+
+ base, ok := http.DefaultTransport.(*http.Transport)
+ if !ok {
+ return nil, fmt.Errorf("publish: create local registry transport")
+ }
+ transport := base.Clone()
+
+ if transport.TLSClientConfig == nil {
+ transport.TLSClientConfig = &tls.Config{}
+ } else {
+ transport.TLSClientConfig = transport.TLSClientConfig.Clone()
+ }
+ transport.TLSClientConfig.RootCAs = pool
+
+ return transport, nil
+}
+
// publishedRepository derives the remote repository for a completed local
// build reference: the last path segment of the local repository (the build
// ID for the canonical "builds/<id>" reference) under the configured prefix.
@@ -157,7 +221,7 @@
}
func (k dockerConfigKeychain) Resolve(r authn.Resource) (authn.Authenticator, error) {
- if cfg, ok := k.auths[r.RegistryStr()]; ok {
+ if cfg, ok := k.auths[normalizeDockerAuthHost(r.RegistryStr())]; ok {
return authn.FromConfig(cfg), nil
}
return authn.Anonymous, nil
@@ -200,11 +264,21 @@
}
ac.Username, ac.Password = user, pass
}
- host = strings.TrimPrefix(host, "https://")
- host = strings.TrimPrefix(host, "http://")
- host = strings.TrimSuffix(host, "/")
- auths[host] = ac
+ normalizedHost := normalizeDockerAuthHost(host)
+ if normalizedHost == "" {
+ continue
+ }
+ auths[normalizedHost] = ac
}
return dockerConfigKeychain{auths: auths}, nil
}
+
+func normalizeDockerAuthHost(host string) string {
+ host = strings.TrimSpace(stripRegistryScheme(host))
+ host = strings.TrimSuffix(host, "/")
+ if idx := strings.Index(host, "/"); idx != -1 {
+ host = host[:idx]
+ }
+ return host
+}
diff --git a/lib/builds/publish_remote_test.go b/lib/builds/publish_remote_test.go
--- a/lib/builds/publish_remote_test.go
+++ b/lib/builds/publish_remote_test.go
@@ -3,6 +3,7 @@
import (
"context"
"encoding/base64"
+ "encoding/pem"
"errors"
"fmt"
"net/http"
@@ -74,7 +75,7 @@
func newTestRemotePublisher(t *testing.T, cfg PublishConfig, localRegistry string) BuildPublisher {
t.Helper()
- p, err := newRemotePublisher(cfg, localRegistry, NewRegistryTokenGenerator("test-secret"))
+ p, err := newRemotePublisher(cfg, localRegistry, false, "", NewRegistryTokenGenerator("test-secret"))
require.NoError(t, err)
return p
}
@@ -253,7 +254,7 @@
Registry: dstHost,
RepositoryPrefix: "team/builds",
CredentialsFile: credFile,
- }, srcHost, NewRegistryTokenGenerator("test-secret"))
+ }, srcHost, false, "", NewRegistryTokenGenerator("test-secret"))
require.NoError(t, err)
ref, err := p.Publish(context.Background(), "builds/build-7", digest)
@@ -304,8 +305,69 @@
require.NoError(t, err)
assert.Equal(t, authn.Anonymous, other)
})
+
+ t.Run("resolves credentials with v1/v2 path suffixes", func(t *testing.T) {
+ f := filepath.Join(t.TempDir(), "config.json")
+ registryAuth := base64.StdEncoding.EncodeToString([]byte("path-user:path-pass"))
+ dockerHubAuth := base64.StdEncoding.EncodeToString([]byte("hub-user:hub-pass"))
+ require.NoError(t, os.WriteFile(f, []byte(fmt.Sprintf(
+ `{"auths":{"https://registry.example.com/v2/":{"auth":%q},"https://index.docker.io/v1/":{"auth":%q}}}`,
+ registryAuth, dockerHubAuth,
+ )), 0600))
+
+ k, err := dockerFileKeychain(f)
+ require.NoError(t, err)
+
+ registryAuthenticator, err := k.Resolve(mustRegistry(t, "registry.example.com"))
+ require.NoError(t, err)
+ registryCfg, err := registryAuthenticator.Authorization()
+ require.NoError(t, err)
+ assert.Equal(t, "path-user", registryCfg.Username)
+ assert.Equal(t, "path-pass", registryCfg.Password)
+
+ hubAuthenticator, err := k.Resolve(mustRegistry(t, "index.docker.io"))
+ require.NoError(t, err)
+ hubCfg, err := hubAuthenticator.Authorization()
+ require.NoError(t, err)
+ assert.Equal(t, "hub-user", hubCfg.Username)
+ assert.Equal(t, "hub-pass", hubCfg.Password)
+ })
}
+func TestParseLocalSourceRef_RespectsInsecureSetting(t *testing.T) {
+ digest := "sha256:" + strings.Repeat("ab", 32)
+
+ secureRef, err := parseLocalSourceRef("registry.example.com:5000", "builds/build-8", digest, false)
+ require.NoError(t, err)
+ assert.Equal(t, "https", secureRef.Context().Registry.Scheme())
+
+ insecureRef, err := parseLocalSourceRef("registry.example.com:5000", "builds/build-8", digest, true)
+ require.NoError(t, err)
+ assert.Equal(t, "http", insecureRef.Context().Registry.Scheme())
+}
+
+func TestNewLocalRegistryTransport_UsesProvidedCACert(t *testing.T) {
+ server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+ w.WriteHeader(http.StatusOK)
+ }))
+ t.Cleanup(server.Close)
+
+ cert := server.Certificate()
+ require.NotNil(t, cert)
+ certPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: cert.Raw})
+ require.NotEmpty(t, certPEM)
+
+ transport, err := newLocalRegistryTransport(string(certPEM))
+ require.NoError(t, err)
+ require.NotNil(t, transport)
+
+ client := &http.Client{Transport: transport}
+ resp, err := client.Get(server.URL)
+ require.NoError(t, err)
+ assert.Equal(t, http.StatusOK, resp.StatusCode)
+ require.NoError(t, resp.Body.Close())
+}
+
func mustRegistry(t *testing.T, host string) name.Registry {
t.Helper()
r, err := name.NewRegistry(host)
diff --git a/lib/builds/publisher.go b/lib/builds/publisher.go
--- a/lib/builds/publisher.go
+++ b/lib/builds/publisher.go
@@ -58,12 +58,18 @@
// newBuildPublisher returns the publisher for the given publication config.
// Publication is a no-op unless a remote registry is configured.
-func newBuildPublisher(cfg PublishConfig, localRegistry string, tokenGen *RegistryTokenGenerator) (BuildPublisher, error) {
+func newBuildPublisher(
+ cfg PublishConfig,
+ localRegistry string,
+ localRegistryInsecure bool,
+ localRegistryCACert string,
+ tokenGen *RegistryTokenGenerator,
+) (BuildPublisher, error) {
if err := cfg.validate(); err != nil {
return nil, err
}
if !cfg.Enabled() {
return noopPublisher{}, nil
}
- return newRemotePublisher(cfg, localRegistry, tokenGen)
+ return newRemotePublisher(cfg, localRegistry, localRegistryInsecure, localRegistryCACert, tokenGen)
}
diff --git a/lib/builds/publisher_test.go b/lib/builds/publisher_test.go
--- a/lib/builds/publisher_test.go
+++ b/lib/builds/publisher_test.go
@@ -22,7 +22,7 @@
}
func TestNewBuildPublisher_DisabledByDefault(t *testing.T) {
- p, err := newBuildPublisher(PublishConfig{}, "localhost:5000", NewRegistryTokenGenerator("test"))
+ p, err := newBuildPublisher(PublishConfig{}, "localhost:5000", false, "", NewRegistryTokenGenerator("test"))
require.NoError(t, err)
assert.IsType(t, noopPublisher{}, p)
}
@@ -30,13 +30,13 @@
func TestNewBuildPublisher_ValidatesConfig(t *testing.T) {
tokenGen := NewRegistryTokenGenerator("test")
- _, err := newBuildPublisher(PublishConfig{RepositoryPrefix: "team/builds"}, "localhost:5000", tokenGen)
+ _, err := newBuildPublisher(PublishConfig{RepositoryPrefix: "team/builds"}, "localhost:5000", false, "", tokenGen)
require.ErrorContains(t, err, "registry")
- _, err = newBuildPublisher(PublishConfig{CredentialsFile: "/creds.json"}, "localhost:5000", tokenGen)
+ _, err = newBuildPublisher(PublishConfig{CredentialsFile: "/creds.json"}, "localhost:5000", false, "", tokenGen)
require.ErrorContains(t, err, "registry")
- _, err = newBuildPublisher(PublishConfig{Registry: "registry.example.com"}, "localhost:5000", tokenGen)
+ _, err = newBuildPublisher(PublishConfig{Registry: "registry.example.com"}, "localhost:5000", false, "", tokenGen)
require.ErrorContains(t, err, "repository_prefix")
}You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit dc2f51b. Configure here.
Pull completed images from the local registry using the configured RegistryInsecure (plain HTTP) and RegistryCACert (custom CA) settings, and normalize Docker config credential keys by stripping path suffixes (e.g. /v1/, /v2/) so Resolve matches the registry host.
Contributor
Author
|
Closing in favor of a fresh first-class Builder resource stack. Useful implementation and test work from this PR will be selectively reapplied in smaller reviewable layers. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Builds on #329. When
builds.publishis configured, the host now publishes each completed build image to a remote OCI registry.<remote-registry>/<prefix>/<build-id>@<digest>, using the existing go-containerregistry client — no new dependency.Build.ImageRefrecords the immutable remote reference. Local image conversion (waitForImageReady, re-tag viaimage_name) is exactly as before.build.json.Tests
All against in-memory OCI registries (no live external registry):
Build.ImageRef.go build ./...,go vet, andgo test ./lib/builds/ ./cmd/api/config/ ./lib/providers/all pass.Note
Medium Risk
Changes post-build image handling and registry I/O with host-stored credentials; misconfiguration or registry outages can fail otherwise successful builds, but builder VMs do not receive publish secrets.
Overview
When
builds.publishis set, the build manager no longer stops at a local image: it copies each finished build from the local registry to<remote>/<prefix>/<build-id>@<digest>, then HEAD-verifies the remote manifest matches the build digest before marking the build ready.newBuildPublishernow selects aremotePublisher(via go-containerregistry) instead of always using a no-op. The host pulls locally with a short-lived registry token and pushes remotely using Docker config credentials read only on the host—publish settings and secrets are not written to the builder VMbuild.json. Local registry insecure/CA settings apply to the pull; pushes retry transient registry errors and fail the build on publish failure, withBuild.ImageRefset to the immutable remote digest ref when publication succeeds.Reviewed by Cursor Bugbot for commit 564c4e2. Bugbot is set up for automated code reviews on this repo. Configure here.