fix(master): seed the tag digest from running services and hide registry error text (#9)

internal/master/model/model.go

@@ -62,8 +62,9 @@ type Service struct {
 	AvailableAt     *time.Time
 	LastPollError   string // last registry error for the followed ref; "" when fine
 	// LastTagDigest is the newest digest of the followed tag that was deployed
-	// or started: set when the poller starts an auto job and when any plain tag
-	// job succeeds (the digest the agent pinned); "" if never. The poller does
+	// or started: set when the poller starts an auto job, when it sees the service
+	// already running that digest, and when any plain tag job succeeds (the
+	// digest the agent pinned); "" if never. The poller does
 	// not start another job for the same digest, so a rollback sticks until the
 	// registry shows a different one.
 	LastTagDigest string

internal/master/poller/poller.go

@@ -220,9 +220,18 @@ func (p *Poller) record(ctx context.Context, l lookup, svcs []model.Service, sta
 }
 
 // suggest marks l.digest as the available version of svc unless the service
-// already runs it or already has exactly that suggestion. It returns false
-// only when writing failed.
+// already runs it or already has exactly that suggestion. A service that
+// runs l.digest also gets it as last_tag_digest. It returns false only when
+// writing the suggestion failed.
 func (p *Poller) suggest(ctx context.Context, svc model.Service, l lookup, stats *cycleStats) bool {
+	if svc.ActualDigest == l.digest && svc.LastTagDigest != l.digest {
+		// Seed last_tag_digest for a service that was not updated by a tag job
+		// (it predates the column or was updated by hand), so that a later
+		// rollback is not undone.
+		if err := p.store.SetLastTagDigest(ctx, svc.ID, l.digest); err != nil {
+			p.log.Error("poll_set_last_tag_digest_failed", "service_id", svc.ID, "error", err)
+		}
+	}
 	if svc.ActualDigest == l.digest || (svc.AvailableRef == l.ref && svc.AvailableDigest == l.digest) {
 		return true
 	}

internal/master/poller/poller_test.go

@@ -30,7 +30,7 @@ type pollErrorCall struct {
 	msg string
 }
 
-type lastAutoCall struct {
+type lastTagCall struct {
 	id     int64
 	digest string
 }
@@ -39,7 +39,7 @@ type fakeStore struct {
 	mu         sync.Mutex
 	services   []model.Service
 	fresh      map[int64]model.Service // what GetService returns instead of the listed row
-	lastAuto   []lastAutoCall
+	lastTag    []lastTagCall
 	listErr    error
 	available  []availableCall
 	pollErrors []pollErrorCall
@@ -69,7 +69,7 @@ func (f *fakeStore) GetService(_ context.Context, id int64) (model.Service, erro
 func (f *fakeStore) SetLastTagDigest(_ context.Context, id int64, digest string) error {
 	f.mu.Lock()
 	defer f.mu.Unlock()
-	f.lastAuto = append(f.lastAuto, lastAutoCall{id, digest})
+	f.lastTag = append(f.lastTag, lastTagCall{id, digest})
 	for i := range f.services {
 		if f.services[i].ID == id {
 			f.services[i].LastTagDigest = digest
@@ -287,10 +287,10 @@ func TestNewDigest(t *testing.T) {
 				At: time.Date(2026, 10, 13, 12, 0, 0, 0, time.UTC)}, st.available[0].a)
 			if tt.wantStart {
 				assert.Equal(t, []startCall{{1, jobs.StartRequest{Trigger: model.TriggerPoll, Ref: ref}}}, j.starts)
-				assert.Equal(t, []lastAutoCall{{1, "sha256:new"}}, st.lastAuto)
+				assert.Equal(t, []lastTagCall{{1, "sha256:new"}}, st.lastTag)
 			} else {
 				assert.Empty(t, j.starts)
-				assert.Empty(t, st.lastAuto)
+				assert.Empty(t, st.lastTag)
 			}
 			assert.Equal(t, []events.Topic{events.Services}, pub.topics)
 		})
@@ -341,7 +341,7 @@ func TestStartErrorsDoNotAbortCycle(t *testing.T) {
 
 	assert.Len(t, st.available, 3)
 	assert.Len(t, j.starts, 3)
-	assert.Empty(t, st.lastAuto, "a job that was not created is retried next cycle")
+	assert.Empty(t, st.lastTag, "a job that was not created is retried next cycle")
 }
 
 func TestRollbackOfAutoServiceIsNotUndone(t *testing.T) {
@@ -366,7 +366,7 @@ func TestRollbackOfAutoServiceIsNotUndone(t *testing.T) {
 	p.Cycle(context.Background())
 
 	assert.Equal(t, []startCall{{1, jobs.StartRequest{Trigger: model.TriggerPoll, Ref: ref}}}, j.starts, "a new digest moves it again")
-	assert.Equal(t, []lastAutoCall{{1, "sha256:newer"}}, st.lastAuto)
+	assert.Equal(t, []lastTagCall{{1, "sha256:newer"}}, st.lastTag)
 }
 
 func TestActiveJobIsRetriedNextCycle(t *testing.T) {
@@ -380,13 +380,13 @@ func TestActiveJobIsRetriedNextCycle(t *testing.T) {
 
 	p.Cycle(context.Background())
 	require.Len(t, j.starts, 1)
-	assert.Empty(t, st.lastAuto)
+	assert.Empty(t, st.lastTag)
 
 	j.errFor = nil
 	p.Cycle(context.Background())
 
 	assert.Len(t, j.starts, 2)
-	assert.Equal(t, []lastAutoCall{{1, "sha256:new"}}, st.lastAuto)
+	assert.Equal(t, []lastTagCall{{1, "sha256:new"}}, st.lastTag)
 
 	p.Cycle(context.Background())
 	assert.Len(t, j.starts, 2, "started once the job exists")
@@ -423,7 +423,7 @@ func TestServiceSwitchedToManualDuringTheCycleIsNotStarted(t *testing.T) {
 
 	assert.Len(t, st.available, 1, "still suggested")
 	assert.Empty(t, j.starts)
-	assert.Empty(t, st.lastAuto)
+	assert.Empty(t, st.lastTag)
 }
 
 func TestErrorIsPerRef(t *testing.T) {
@@ -625,3 +625,27 @@ func TestRunReturnsDuringFirstDelay(t *testing.T) {
 }
 
 func ptr[T any](v T) *T { return &v }
+
+func TestSeedsTheTagDigestOfARunningService(t *testing.T) {
+	ref := repoA + ":main"
+	s := svc(1, ref+"@sha256:new")
+	s.UpdateMode = model.ModeAuto
+	s.ActualDigest = "sha256:new" // updated outside the updater: last_tag_digest is empty
+	st := &fakeStore{services: []model.Service{s}}
+	reg := &fakeRegistry{digests: map[string]string{ref: "sha256:new"}}
+	j := &fakeJobs{}
+	p, _ := newPoller(st, j, reg, Config{})
+
+	p.Cycle(context.Background())
+
+	assert.Equal(t, []lastTagCall{{1, "sha256:new"}}, st.lastTag)
+	assert.Empty(t, j.starts)
+
+	// Rolled back by hand to the older digest.
+	st.services[0].ActualRef, st.services[0].ActualDigest = ref+"@sha256:old", "sha256:old"
+	st.services[0].DesiredRef = ref + "@sha256:old"
+	p.Cycle(context.Background())
+
+	assert.Empty(t, j.starts, "the rollback stays")
+	assert.Len(t, st.lastTag, 1, "nothing more to seed")
+}

internal/master/registry/registry.go

@@ -6,7 +6,10 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"log/slog"
+	"net"
 	"net/http"
+	"net/url"
 	"regexp"
 	"strings"
 
@@ -24,14 +27,16 @@ import (
 type Client struct {
 	keychain  authn.Keychain
 	userAgent string
+	log       *slog.Logger
 }
 
 // New returns a Client that finds credentials the way the Docker CLI does:
 // in $DOCKER_CONFIG/config.json, else ~/.docker/config.json. Registries
 // without an entry are accessed anonymously. userAgent is added to the
-// User-Agent header of every request.
-func New(userAgent string) *Client {
-	return &Client{keychain: authn.DefaultKeychain, userAgent: userAgent}
+// User-Agent header of every request. log receives the full error of a
+// failed lookup, which Digest itself returns only in a sanitised form.
+func New(userAgent string, log *slog.Logger) *Client {
+	return &Client{keychain: authn.DefaultKeychain, userAgent: userAgent, log: log}
 }
 
 // Digest returns the digest that "repo:tag" points at, as the registry
@@ -49,20 +54,43 @@ func (c *Client) Digest(ctx context.Context, ref string) (string, error) {
 	if err != nil {
 		return "", fmt.Errorf("parse %s: %w", ref, err)
 	}
+	// Credentials are resolved here, not by ggcr, so that a broken credential
+	// helper keeps its own message: that text comes from the local config.
+	auth, err := c.keychain.Resolve(r.Context())
+	if err != nil {
+		return "", fmt.Errorf("lookup %s: %w", r, err)
+	}
 	desc, err := remote.Head(r,
 		remote.WithContext(ctx),
-		remote.WithAuthFromKeychain(c.keychain),
+		remote.WithAuth(auth),
 		remote.WithUserAgent(c.userAgent))
 	if err != nil {
 		var terr *transport.Error
 		if errors.As(err, &terr) {
 			return "", fmt.Errorf("lookup %s: %s", r, describe(terr))
 		}
-		return "", fmt.Errorf("lookup %s: %w", r, err)
+		c.log.Warn("registry_lookup_failed", "ref", ref, "error", err)
+		return "", fmt.Errorf("lookup %s: %s", r, classify(err))
 	}
 	return desc.Digest.String(), nil
 }
 
+// classify returns a fixed text for an error that is not a registry answer.
+// ggcr copies registry-controlled text (token responses, authentication
+// challenges, redirect URLs, certificate names) into such errors, so none of
+// it is passed on.
+func classify(err error) string {
+	var netErr net.Error
+	var urlErr *url.Error
+	switch {
+	case errors.Is(err, context.DeadlineExceeded), errors.Is(err, context.Canceled):
+		return "timeout"
+	case errors.As(err, &netErr), errors.As(err, &urlErr):
+		return "cannot reach registry"
+	}
+	return "lookup failed"
+}
+
 // errorCodePattern matches the registry error codes of the distribution
 // spec ("MANIFEST_UNKNOWN"). Anything else is dropped from error text.
 var errorCodePattern = regexp.MustCompile(`^[A-Z_]{1,40}$`)

internal/master/registry/registry_test.go

@@ -3,10 +3,15 @@ package registry
 import (
 	"context"
 	"encoding/base64"
+	"errors"
+	"fmt"
 	"io"
 	"log"
+	"log/slog"
+	"net"
 	"net/http"
 	"net/http/httptest"
+	"net/url"
 	"os"
 	"path/filepath"
 	"strings"
@@ -22,6 +27,10 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+var discardLog = slog.New(slog.NewTextHandler(io.Discard, nil))
+
+func newClient() *Client { return New("test", discardLog) }
+
 // request is one request the test registry received.
 type request struct{ method, path string }
 
@@ -108,7 +117,7 @@ func TestDigestOfIndexIsIndexDigest(t *testing.T) {
 	reg := newTestRegistry(t, "", "")
 	want := reg.pushIndex(t, "team/app", "1.2")
 
-	got, err := New("test").Digest(context.Background(), reg.host+"/team/app:1.2")
+	got, err := newClient().Digest(context.Background(), reg.host+"/team/app:1.2")
 
 	require.NoError(t, err)
 	assert.Equal(t, want, got)
@@ -119,7 +128,7 @@ func TestDigestIgnoresDigestSuffix(t *testing.T) {
 	reg := newTestRegistry(t, "", "")
 	want := reg.pushIndex(t, "team/app", "1.2")
 
-	got, err := New("test").Digest(context.Background(), reg.host+"/team/app:1.2@sha256:"+strings.Repeat("0", 64))
+	got, err := newClient().Digest(context.Background(), reg.host+"/team/app:1.2@sha256:"+strings.Repeat("0", 64))
 
 	require.NoError(t, err)
 	assert.Equal(t, want, got)
@@ -131,7 +140,7 @@ func TestDigestUsesHeadOnly(t *testing.T) {
 	reg.pushIndex(t, "team/app", "1.2")
 	reg.reset()
 
-	_, err := New("test").Digest(context.Background(), reg.host+"/team/app:1.2")
+	_, err := newClient().Digest(context.Background(), reg.host+"/team/app:1.2")
 
 	require.NoError(t, err)
 	assert.Equal(t, []request{{http.MethodHead, "/v2/team/app/manifests/1.2"}}, reg.manifestRequests())
@@ -151,7 +160,7 @@ func TestDigestSendsUserAgent(t *testing.T) {
 	}))
 	t.Cleanup(srv.Close)
 
-	_, _ = New("package-updater-test/1").Digest(context.Background(), strings.TrimPrefix(srv.URL, "http://")+"/a/b:1")
+	_, _ = New("package-updater-test/1", discardLog).Digest(context.Background(), strings.TrimPrefix(srv.URL, "http://")+"/a/b:1")
 
 	assert.Contains(t, ua, "package-updater-test/1")
 }
@@ -162,7 +171,7 @@ func TestCredentialsFromDockerConfig(t *testing.T) {
 	auth := base64.StdEncoding.EncodeToString([]byte("bot:s3cret"))
 	dockerConfig(t, `{"auths":{"`+reg.host+`":{"auth":"`+auth+`"}}}`)
 
-	got, err := New("test").Digest(context.Background(), reg.host+"/team/app:1.2")
+	got, err := newClient().Digest(context.Background(), reg.host+"/team/app:1.2")
 
 	require.NoError(t, err)
 	assert.Equal(t, want, got)
@@ -174,7 +183,7 @@ func TestWrongCredentialsFail(t *testing.T) {
 	auth := base64.StdEncoding.EncodeToString([]byte("bot:wrong"))
 	dockerConfig(t, `{"auths":{"`+reg.host+`":{"auth":"`+auth+`"}}}`)
 
-	_, err := New("test").Digest(context.Background(), reg.host+"/team/app:1.2")
+	_, err := newClient().Digest(context.Background(), reg.host+"/team/app:1.2")
 
 	require.Error(t, err)
 	assert.NotContains(t, err.Error(), "wrong", "credentials do not leak into errors")
@@ -187,11 +196,11 @@ func TestAnonymousWithoutConfig(t *testing.T) {
 	want := open.pushIndex(t, "team/app", "1.2")
 	protected := newTestRegistry(t, "bot", "s3cret")
 
-	got, err := New("test").Digest(context.Background(), open.host+"/team/app:1.2")
+	got, err := newClient().Digest(context.Background(), open.host+"/team/app:1.2")
 	require.NoError(t, err)
 	assert.Equal(t, want, got, "anonymous access works without a config file")
 
-	_, err = New("test").Digest(context.Background(), protected.host+"/team/app:1.2")
+	_, err = newClient().Digest(context.Background(), protected.host+"/team/app:1.2")
 	assert.Error(t, err, "a registry that needs credentials fails without them")
 }
 
@@ -200,13 +209,13 @@ func TestUnknownTagIsAnError(t *testing.T) {
 	reg := newTestRegistry(t, "", "")
 	reg.pushIndex(t, "team/app", "1.2")
 
-	_, err := New("test").Digest(context.Background(), reg.host+"/team/app:nope")
+	_, err := newClient().Digest(context.Background(), reg.host+"/team/app:nope")
 
 	assert.Error(t, err)
 }
 
 func TestRefWithoutTagIsAnError(t *testing.T) {
-	_, err := New("test").Digest(context.Background(), "team/app@sha256:"+strings.Repeat("a", 64))
+	_, err := newClient().Digest(context.Background(), "team/app@sha256:"+strings.Repeat("a", 64))
 
 	assert.ErrorContains(t, err, "has no tag")
 }
@@ -220,7 +229,7 @@ func TestCredsStoreWithoutHelperFailsTheLookup(t *testing.T) {
 	reg.pushIndex(t, "team/app", "1.2")
 	dockerConfig(t, `{"credsStore":"no-such-helper-for-package-updater-tests"}`)
 
-	_, err := New("test").Digest(context.Background(), reg.host+"/team/app:1.2")
+	_, err := newClient().Digest(context.Background(), reg.host+"/team/app:1.2")
 
 	assert.ErrorContains(t, err, "error getting credentials")
 	assert.ErrorContains(t, err, "docker-credential-no-such-helper-for-package-updater-tests")
@@ -244,7 +253,7 @@ func TestMissingDigestHeaderDoesNotFallBackToDownload(t *testing.T) {
 	}))
 	t.Cleanup(srv.Close)
 
-	_, err := New("test").Digest(context.Background(), strings.TrimPrefix(srv.URL, "http://")+"/a/b:1")
+	_, err := newClient().Digest(context.Background(), strings.TrimPrefix(srv.URL, "http://")+"/a/b:1")
 
 	assert.Error(t, err)
 	assert.Equal(t, []string{http.MethodHead}, methods)
@@ -274,7 +283,7 @@ func TestRegistryErrorBodyDoesNotLeak(t *testing.T) {
 			t.Cleanup(srv.Close)
 			host := strings.TrimPrefix(srv.URL, "http://")
 
-			_, err := New("test").Digest(context.Background(), host+"/a/b:1")
+			_, err := newClient().Digest(context.Background(), host+"/a/b:1")
 
 			require.Error(t, err)
 			assert.NotContains(t, err.Error(), "internal-secret-body")
@@ -282,3 +291,46 @@ func TestRegistryErrorBodyDoesNotLeak(t *testing.T) {
 		})
 	}
 }
+
+// ggcr puts the token server's answer into its error when the answer has no
+// token, and the token server is named by the registry's challenge.
+func TestNonRegistryErrorsAreFixedText(t *testing.T) {
+	dockerConfig(t, "")
+	tokenSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		_, _ = w.Write([]byte(`{"detail":"internal-secret-body"}`))
+	}))
+	t.Cleanup(tokenSrv.Close)
+	reg := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("WWW-Authenticate", `Bearer realm="`+tokenSrv.URL+`/internal-secret-realm",service="internal-secret-service"`)
+		w.WriteHeader(http.StatusUnauthorized)
+	}))
+	t.Cleanup(reg.Close)
+	host := strings.TrimPrefix(reg.URL, "http://")
+
+	_, err := newClient().Digest(context.Background(), host+"/a/b:1")
+
+	require.Error(t, err)
+	assert.NotContains(t, err.Error(), "internal-secret")
+	assert.Equal(t, "lookup "+host+"/a/b:1: lookup failed", err.Error())
+}
+
+func TestClassify(t *testing.T) {
+	refused := &url.Error{Op: "Get", URL: "http://internal-secret-host/v2/", Err: errors.New("connection refused")}
+	tests := []struct {
+		name string
+		err  error
+		want string
+	}{
+		{"deadline", fmt.Errorf("x: %w", context.DeadlineExceeded), "timeout"},
+		{"canceled", context.Canceled, "timeout"},
+		{"url error", refused, "cannot reach registry"},
+		{"net error", &net.DNSError{Name: "internal-secret-host"}, "cannot reach registry"},
+		{"deadline inside url error", &url.Error{Op: "Get", URL: "http://x", Err: context.DeadlineExceeded}, "timeout"},
+		{"anything else", errors.New("internal-secret"), "lookup failed"},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equal(t, tt.want, classify(tt.err))
+		})
+	}
+}

internal/master/store/services.go

@@ -261,12 +261,13 @@ func (s *Store) SetPollError(ctx context.Context, ids []int64, msg string) error
 	return nil
 }
 
-// SetLastTagDigest records the digest of the followed tag that the poller
-// started an update for; see model.Service.LastTagDigest. Inventory writes never change it.
+// SetLastTagDigest sets last_tag_digest; see model.Service.LastTagDigest. The
+// poller calls it, and FinishJob sets the same column after a tag job.
+// Inventory writes never change it.
 func (s *Store) SetLastTagDigest(ctx context.Context, id int64, digest string) error {
 	if _, err := s.pool.Exec(ctx,
 		`UPDATE services SET last_tag_digest = $2 WHERE id = $1`, id, digest); err != nil {
-		return fmt.Errorf("set last auto digest: %w", err)
+		return fmt.Errorf("set last tag digest: %w", err)
 	}
 	return nil
 }