From 8fe75b2cbf6f4be0cd8bf02c2f382b48bef69a01 Mon Sep 17 00:00:00 2001 From: Samuel Enocsson Date: Tue, 13 Oct 2026 08:38:17 +0200 Subject: [PATCH] fix(master): seed the tag digest from running services and hide registry error text (#9) --- CLAUDE.md | 13 ++-- README.md | 12 ++- cmd/master/main.go | 2 +- ...2026-10-13-master-agent-phase-4-polling.md | 2 +- internal/e2e/agent_master_test.go | 6 +- internal/e2e/jobs_test.go | 12 ++- internal/e2e/versions_test.go | 4 +- internal/e2e/webhook_test.go | 2 +- internal/master/model/model.go | 5 +- internal/master/poller/poller.go | 13 +++- internal/master/poller/poller_test.go | 44 ++++++++--- internal/master/registry/registry.go | 38 +++++++-- internal/master/registry/registry_test.go | 78 +++++++++++++++---- internal/master/store/services.go | 7 +- 14 files changed, 185 insertions(+), 53 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index b9e9075..b930f83 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -81,12 +81,13 @@ DATABASE_URL=postgres://... go test -tags integration ./internal/master/store/ protocol 3 (`protocol.MinPinnedRefVersion`); `Hub.Route` takes the minimum version. - **Picking a tagless version (`repo@digest`) leaves the service following no tag:** it is no longer polled and the plain Update button is refused. -- **`services.last_tag_digest`** is set by the poller when it starts an auto job and by - `FinishJob` after any successful plain tag job (not pinned/tagless picks; the digest is the - one the agent pinned). The poller starts no job for that digest, nor for one `desired_ref` - (`ref@digest`) already names, and re-reads the service before `Start`; so a rollback of an - auto service holds until the registry shows a different digest. `ErrJobActive` and - `NotUpdatableError` leave it unset so the next cycle retries. +- **`services.last_tag_digest`** is set by the poller when it starts an auto job and when it + sees the service already running the tag's digest, and by `FinishJob` after any successful + plain tag job (not pinned/tagless picks; the digest is the one the agent pinned). The poller + starts no job for that digest, nor for one `desired_ref` (`ref@digest`) already names, and + re-reads the service before `Start`; so a rollback of an auto service holds until the + registry shows a different digest. `ErrJobActive` and `NotUpdatableError` leave it unset so + the next cycle retries. - **A bare image's running version is recorded with tag `latest`** (like `jobs.FollowedRef`); `repo@digest` is recorded with tag `''`. Only `sha256:<64 hex>` digests are recorded. - **Image matching is case-sensitive.** The tricky case is `localhost:5000/foo:v1` where diff --git a/README.md b/README.md index 1110d1e..fe1cc94 100644 --- a/README.md +++ b/README.md @@ -132,8 +132,9 @@ multi-arch image the digest is the index digest, the same one `docker inspect` s already suggested is not suggested again. A service in **auto** mode also gets an update job to `repo:tag` (the agent resolves the digest itself, so agents on protocol version 2 keep working). The master remembers the newest digest it started or deployed for the tag - in `services.last_tag_digest` (set when polling starts a job and when any plain tag job - succeeds, whoever started it) and starts no job for that digest again, whether the job + in `services.last_tag_digest` (set when polling starts a job, when polling sees the + service already running that digest, and when any plain tag job succeeds, whoever started + it) and starts no job for that digest again, whether the job failed or the user rolled back afterwards. If the service already had a job when the digest was found, the next cycle tries again. - **What is not polled.** Services that follow no tag (`repo@digest`), `build:` services, and @@ -154,7 +155,9 @@ multi-arch image the digest is the index digest, the same one `docker inspect` s registry host in the image name, including a `GET /v2/` ping that follows redirects. Anyone who can start containers on a managed host can therefore make the master send blind requests to internal hosts. The registry's answer is not shown: an error keeps only - the HTTP status and the registry's error codes. + the HTTP status and the registry's error codes; any other failure shows only "timeout", + "cannot reach registry" or "lookup failed". The full error goes to the master's log + (`registry_lookup_failed`). ### Version history and rollback @@ -174,6 +177,9 @@ digest-pinned or tagless versions need an agent speaking protocol version 3 (thi release); on an older agent the job fails with "agent too old to deploy a pinned version (needs protocol 3)". +A rollback made before the first poll cycle after the upgrade has seen the service is not +protected, because the master has not yet recorded the digest it was running. + A service in **auto** mode stays auto after a rollback. Polling does not undo it: the service stays on the older version until the registry shows a digest of the followed tag that differs from the newest one the master deployed. The newer version it was rolled back diff --git a/cmd/master/main.go b/cmd/master/main.go index 83ae845..76cc04a 100644 --- a/cmd/master/main.go +++ b/cmd/master/main.go @@ -158,7 +158,7 @@ func run() error { logger.Info("registry_credentials", "docker_config", credDir, "config_json_found", credFound) pollCtx, stopPolling := context.WithCancel(ctx) defer stopPolling() - registryPoller := poller.New(st, jobService, registry.New("package-updater-master/"+version), broker, poller.Config{ + registryPoller := poller.New(st, jobService, registry.New("package-updater-master/"+version, logger), broker, poller.Config{ Interval: cfg.PollInterval, FirstDelay: pollFirstDelay, Workers: pollWorkers, }, logger) pollDone := make(chan struct{}) diff --git a/docs/superpowers/plans/2026-10-13-master-agent-phase-4-polling.md b/docs/superpowers/plans/2026-10-13-master-agent-phase-4-polling.md index d50497d..91d1933 100644 --- a/docs/superpowers/plans/2026-10-13-master-agent-phase-4-polling.md +++ b/docs/superpowers/plans/2026-10-13-master-agent-phase-4-polling.md @@ -32,7 +32,7 @@ 2. **Empty string instead of NULL** for missing tag or digest in `image_versions`, so `UNIQUE (repo, tag, digest)` works. A CHECK forbids both being empty. 3. **`first_seen_at` is never bumped.** Re-observation is `ON CONFLICT DO NOTHING`; `source` is the first source that saw it. "10 most recent" = "10 most recently first seen". A new digest of a tag is a new row at the top; old digest stays below (enables rollback of moving tags). 4. **Not polled:** services that follow no tag, `build:` services, services with empty `actual_digest`. UI shows the reason. Self and container services ARE polled (badge informative; jobs still refused by `jobs`). -5. **Poller does not re-suggest the same version.** If `available_ref == ref && available_digest == D`, the badge is not written again (no `available_at` bump). The auto job is decided separately: it starts only if the service is auto, `actual_digest != D`, `D != last_tag_digest` and `desired_ref != ref@D`, on a fresh read of the row. `last_tag_digest` is set when a job row was created (not on `ErrJobActive`/`NotUpdatableError`, which retry next cycle). A failed auto job is not retried every interval; a rollback of an auto service is not undone by polling until a new digest appears. `FinishJob` also sets `last_tag_digest` to the pinned digest after any successful job whose `to_ref` as created has no digest (manual, webhook or poll); pinned and tagless picks leave it, so a rollback holds whoever started the update. +5. **Poller does not re-suggest the same version.** If `available_ref == ref && available_digest == D`, the badge is not written again (no `available_at` bump). The auto job is decided separately: it starts only if the service is auto, `actual_digest != D`, `D != last_tag_digest` and `desired_ref != ref@D`, on a fresh read of the row. `last_tag_digest` is set when a job row was created (not on `ErrJobActive`/`NotUpdatableError`, which retry next cycle). A failed auto job is not retried every interval; a rollback of an auto service is not undone by polling until a new digest appears. The poller also sets `last_tag_digest` to D when `actual_digest == D` (seeding services that predate the column). `FinishJob` also sets `last_tag_digest` to the pinned digest after any successful job whose `to_ref` as created has no digest (manual, webhook or poll); pinned and tagless picks leave it, so a rollback holds whoever started the update. 6. **FinishJob clearing rule gets a digest condition** (C3). Rollback to an older digest of the same tag no longer clears the newer-version badge. 7. **Poll auto jobs use `repo:tag`, not `repo:tag@D`.** v2 agents keep working for poll and webhook jobs. Only UI picks of digest-pinned or tagless versions need a v3 agent. 8. **First cycle runs 1 minute after start**, then every `POLL_INTERVAL` measured from the end of the previous cycle. diff --git a/internal/e2e/agent_master_test.go b/internal/e2e/agent_master_test.go index bb3a5c9..44c65e1 100644 --- a/internal/e2e/agent_master_test.go +++ b/internal/e2e/agent_master_test.go @@ -11,6 +11,7 @@ import ( "net" "net/http" "net/http/httptest" + "regexp" "sort" "strings" "sync" @@ -31,6 +32,9 @@ import ( "github.com/shcizo/package-updater/internal/protocol" ) +// digestPattern is the digest format the real store records. +var digestPattern = regexp.MustCompile(`^sha256:[0-9a-f]{64}$`) + type memStore struct { mu sync.Mutex agents map[string]model.Agent @@ -241,7 +245,7 @@ func (m *memStore) FinishJob(_ context.Context, agentID, jobID int64, f model.Jo if f.Status == model.JobSucceeded { svc := m.svcRows[j.ServiceID] svc.DesiredRef = f.Ref - if _, d, _ := strings.Cut(f.Ref, "@"); !strings.Contains(requestedRef, "@") && d != "" { + if _, d, _ := strings.Cut(f.Ref, "@"); !strings.Contains(requestedRef, "@") && digestPattern.MatchString(d) { svc.LastTagDigest = d } // Same rule as the SQL: the job was created for the suggested ref after it was reported. diff --git a/internal/e2e/jobs_test.go b/internal/e2e/jobs_test.go index a4d1669..7154eeb 100644 --- a/internal/e2e/jobs_test.go +++ b/internal/e2e/jobs_test.go @@ -32,6 +32,9 @@ const ( lostMessage = "lost: the agent did not know this job when it reconnected" ) +// e2eDigest is the digest the fake executor pins a tag to. +const e2eDigest = "sha256:" + "e2e0e2e0e2e0e2e0e2e0e2e0e2e0e2e0e2e0e2e0e2e0e2e0e2e0e2e0e2e0e2e0" + // gateExecutor writes a line of output, then blocks until release is closed // or the job is cancelled. type gateExecutor struct { @@ -52,7 +55,10 @@ func (g *gateExecutor) Execute(ctx context.Context, job protocol.JobStart, out i return "", ctx.Err() } _, _ = io.WriteString(out, "recreated\n") - return job.Ref + "@sha256:e2e", nil + if strings.Contains(job.Ref, "@") { + return job.Ref, nil + } + return job.Ref + "@" + e2eDigest, nil } // jobAgent is a connected agent with a real runner and its two lifetimes: the @@ -159,12 +165,12 @@ func TestManualJobFlowsToAgentAndBack(t *testing.T) { j := e.trigger(t) done := e.waitStatus(t, j.ID, model.JobSucceeded) - assert.Equal(t, slusk+":1.4.2@sha256:e2e", done.ToRef) + assert.Equal(t, slusk+":1.4.2@"+e2eDigest, done.ToRef) assert.Contains(t, done.Output, "pulling "+slusk+":1.4.2") assert.Contains(t, done.Output, "recreated") svc, err := e.store.GetService(context.Background(), 1) require.NoError(t, err) - assert.Equal(t, slusk+":1.4.2@sha256:e2e", svc.DesiredRef) + assert.Equal(t, slusk+":1.4.2@"+e2eDigest, svc.DesiredRef) require.Eventually(t, func() bool { return len(e.agent.runner.Known()) == 0 }, waitFor, pollEvery, "the result must be acked") } diff --git a/internal/e2e/versions_test.go b/internal/e2e/versions_test.go index 0350f34..ad94645 100644 --- a/internal/e2e/versions_test.go +++ b/internal/e2e/versions_test.go @@ -117,14 +117,14 @@ func TestPollKeepsARollbackAfterAWebhookUpdate(t *testing.T) { env.waitStatus(t, hook.ID, model.JobSucceeded) svc, err = env.store.GetService(ctx, 1) require.NoError(t, err) - assert.Equal(t, "sha256:e2e", svc.LastTagDigest, "the deployed digest of the tag is remembered") + assert.Equal(t, e2eDigest, svc.LastTagDigest, "the deployed digest of the tag is remembered") pick, err := env.jobs.TriggerVersion(ctx, 1, 5, "alice") require.NoError(t, err) env.waitStatus(t, pick.ID, model.JobSucceeded) jobsBefore := env.store.jobCount() - reg := &fixedRegistry{digests: map[string]string{slusk + ":main": "sha256:e2e"}} + reg := &fixedRegistry{digests: map[string]string{slusk + ":main": e2eDigest}} p := poller.New(env.store, env.jobs, reg, env.m.broker, poller.Config{Workers: 1}, slog.New(slog.NewTextHandler(io.Discard, nil))) p.Cycle(ctx) diff --git a/internal/e2e/webhook_test.go b/internal/e2e/webhook_test.go index ed9ef10..787aec4 100644 --- a/internal/e2e/webhook_test.go +++ b/internal/e2e/webhook_test.go @@ -99,7 +99,7 @@ func TestWebhookWaitsForAutoJob(t *testing.T) { job := e.job(t, r.body.Results[0].JobID) assert.Equal(t, model.TriggerWebhook, job.Trigger) assert.Equal(t, model.JobSucceeded, job.Status) - assert.Equal(t, slusk+":2.0.0@sha256:e2e", job.ToRef) + assert.Equal(t, slusk+":2.0.0@"+e2eDigest, job.ToRef) svc, err := e.store.GetService(context.Background(), 1) require.NoError(t, err) assert.Empty(t, svc.AvailableRef, "the job deployed the suggestion, so it is cleared") diff --git a/internal/master/model/model.go b/internal/master/model/model.go index 7226daf..7ca11b7 100644 --- a/internal/master/model/model.go +++ b/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 diff --git a/internal/master/poller/poller.go b/internal/master/poller/poller.go index 2c33d3d..eb1b17b 100644 --- a/internal/master/poller/poller.go +++ b/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 } diff --git a/internal/master/poller/poller_test.go b/internal/master/poller/poller_test.go index 29944d0..2c5fd1b 100644 --- a/internal/master/poller/poller_test.go +++ b/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") +} diff --git a/internal/master/registry/registry.go b/internal/master/registry/registry.go index 3017a47..3674c3e 100644 --- a/internal/master/registry/registry.go +++ b/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}$`) diff --git a/internal/master/registry/registry_test.go b/internal/master/registry/registry_test.go index 2f6859f..e9565c0 100644 --- a/internal/master/registry/registry_test.go +++ b/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)) + }) + } +} diff --git a/internal/master/store/services.go b/internal/master/store/services.go index 5a5fd60..ea94419 100644 --- a/internal/master/store/services.go +++ b/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 }