diff --git a/internal/engine/backup_image_test.go b/internal/engine/backup_image_test.go index 742879f..577e61d 100644 --- a/internal/engine/backup_image_test.go +++ b/internal/engine/backup_image_test.go @@ -71,6 +71,76 @@ func TestReEnableKeepsTheRecordedPinAndDoesNotReachTheRegistry(t *testing.T) { } } +func TestReEnableRejectsPinFromDifferentRepository(t *testing.T) { + const stalePin = "postgres@sha256:06cad38a5d9f5d24b4d83d86def30795d5e4b757fedbf5281172b576dedcd941" + const managedPin = "ghcr.io/labstack/onebox-postgres@sha256:16cad38a5d9f5d24b4d83d86def30795d5e4b757fedbf5281172b576dedcd942" + fake := &transport.Fake{Dynamic: func(cmd string) (transport.Result, bool) { + switch { + case strings.Contains(cmd, "docker image inspect") && strings.Contains(cmd, stalePin): + return transport.Result{Stdout: "present\n"}, true + case strings.Contains(cmd, "docker pull"): + return transport.Result{}, true + case strings.Contains(cmd, "RepoDigests"): + return transport.Result{Stdout: managedPin + "\n"}, true + } + return transport.Result{Stdout: "absent\n"}, true + }} + e := protectedImageTestEngine(fake) + bound, err := e.Spec.WithServiceRuntimeStates(map[string]app.ServiceRuntimeState{ + "database": { + BackupState: "disabled", ServiceImage: stalePin, + PublicationVerified: true, DigestAvailable: true, + }, + }) + if err != nil { + t.Fatalf("binding disabled service state: %v", err) + } + e.Spec = bound + + got, err := e.ResolveProtectedImage(context.Background(), "database", stalePin, "ghcr.io/labstack/onebox-postgres:18") + if err != nil { + t.Fatalf("resolving an inconsistent recorded pin: %v", err) + } + if got != managedPin { + t.Fatalf("resolved %q, want %q", got, managedPin) + } +} + +func TestReEnableDoesNotCreatePinFromDifferentRepository(t *testing.T) { + const stalePin = "postgres@sha256:06cad38a5d9f5d24b4d83d86def30795d5e4b757fedbf5281172b576dedcd941" + const managedPin = "ghcr.io/labstack/onebox-postgres@sha256:16cad38a5d9f5d24b4d83d86def30795d5e4b757fedbf5281172b576dedcd942" + fake := &transport.Fake{Dynamic: func(cmd string) (transport.Result, bool) { + switch { + case strings.Contains(cmd, "docker image inspect") && strings.Contains(cmd, stalePin): + return transport.Result{Stdout: "present\n"}, true + case strings.Contains(cmd, "docker pull"): + return transport.Result{}, true + case strings.Contains(cmd, "RepoDigests"): + return transport.Result{Stdout: managedPin + "\n"}, true + } + return transport.Result{Stdout: "absent\n"}, true + }} + e := protectedImageTestEngine(fake) + bound, err := e.Spec.WithServiceRuntimeStates(map[string]app.ServiceRuntimeState{ + "database": { + BackupState: "enabled", ServiceImage: stalePin, + PublicationVerified: true, DigestAvailable: true, + }, + }) + if err != nil { + t.Fatalf("binding enabled service state: %v", err) + } + e.Spec = bound + + got, err := e.ResolveProtectedImage(context.Background(), "database", stalePin, "postgres:18") + if err != nil { + t.Fatalf("resolving after a repository migration: %v", err) + } + if got != managedPin { + t.Fatalf("resolved %q, want %q", got, managedPin) + } +} + // The pin is only reusable while the project still declares the reference that // produced it. Changing the declared version is how an operator asks for // different bytes, and that has to reach the registry. diff --git a/internal/engine/backup_postgres.go b/internal/engine/backup_postgres.go index 688d7b5..7db2ca2 100644 --- a/internal/engine/backup_postgres.go +++ b/internal/engine/backup_postgres.go @@ -15,6 +15,7 @@ import ( "strings" "time" + distref "github.com/distribution/reference" "github.com/labstack/onebox/internal/app" ) @@ -469,11 +470,9 @@ func (e *Engine) RebindServiceRuntimeStates(states map[string]app.ServiceRuntime // ResolveProtectedImage pins the service image by the digest the host actually // has, after pulling it. // -// It is the stock PostgreSQL image — wal-g is mounted in beside it rather than -// baked into a derived one — but it is still pinned, because the reason -// protected image selection is durable state has nothing to do with which image -// it is: the bytes running over a live data directory must not change because a -// tag moved. +// WAL-G is mounted beside the PostgreSQL runtime image rather than baked into +// it, but the image is still pinned: the bytes running over a live data +// directory must not change because a tag moved. // recordedPin and recordedReference come from the service's lifecycle record: // the digest it was last bound with, and the reference that produced it. When // the project still declares that same reference and the host still holds those @@ -485,8 +484,11 @@ func (e *Engine) RebindServiceRuntimeStates(states map[string]app.ServiceRuntime // already had everything it needed, so a rate-limited Docker Hub failed a // command that had nothing to fetch. // -// Moving a protected service to a new image is a deliberate act with its own -// path; it is not a side effect of re-running enable. +// A declared repository migration is the exception: a digest from the old +// repository cannot be recorded as if the new reference produced it, so the +// new declaration is resolved before enablement can write the lifecycle pair. +// Moving between versions within one repository remains a deliberate act with +// its own path; it is not a side effect of re-running enable. func (e *Engine) ResolveProtectedImage(ctx context.Context, service, recordedPin, recordedReference string) (string, error) { reference, err := e.Spec.ServiceImageForRuntime(service) if err != nil { @@ -496,8 +498,13 @@ func (e *Engine) ResolveProtectedImage(ctx context.Context, service, recordedPin if err != nil { return "", err } - st := e.ui.Step("protected image "+reference.Image, false) - if recordedPin != "" && recordedReference == declared && containsDigest(recordedPin) { + resolutionReference := reference.Image + if !sameImageRepository(resolutionReference, declared) { + resolutionReference = declared + } + st := e.ui.Step("protected image "+resolutionReference, false) + if recordedPin != "" && recordedReference == declared && containsDigest(recordedPin) && + sameImageRepository(recordedPin, recordedReference) { held, err := e.imagePresentByDigest(ctx, recordedPin) if err != nil { st(err) @@ -513,26 +520,26 @@ func (e *Engine) ResolveProtectedImage(ctx context.Context, service, recordedPin // — it only spends registry quota and turns a re-enable into a failure on a // host that is offline or rate-limited while holding exactly what it needs. // A tag still resolves through the registry, because a tag can move. - present, err := e.imagePresentByDigest(ctx, reference.Image) + present, err := e.imagePresentByDigest(ctx, resolutionReference) if err != nil { st(err) return "", err } if present { st(nil) - return reference.Image, nil + return resolutionReference, nil } - res, err := e.T.Run(ctx, "docker pull "+q(reference.Image)) + res, err := e.T.Run(ctx, "docker pull "+q(resolutionReference)) if err != nil { st(err) return "", err } if res.ExitCode != 0 { - err := fmt.Errorf("cannot pull %s: %s", reference.Image, lastLines(res.Stderr, 3)) + err := fmt.Errorf("cannot pull %s: %s", resolutionReference, lastLines(res.Stderr, 3)) st(err) return "", err } - res, err = e.T.Run(ctx, "docker image inspect --format '{{index .RepoDigests 0}}' "+q(reference.Image)) + res, err = e.T.Run(ctx, "docker image inspect --format '{{index .RepoDigests 0}}' "+q(resolutionReference)) if err != nil { st(err) return "", err @@ -541,7 +548,12 @@ func (e *Engine) ResolveProtectedImage(ctx context.Context, service, recordedPin if res.ExitCode != 0 || !containsDigest(pinned) { err := fmt.Errorf( "%s has no registry digest on this host; a protected service runs an image pinned by digest, so it must come from a registry rather than a local build", - reference.Image) + resolutionReference) + st(err) + return "", err + } + if !sameImageRepository(pinned, resolutionReference) { + err := fmt.Errorf("resolved digest %s does not belong to image repository %s", pinned, resolutionReference) st(err) return "", err } @@ -730,3 +742,19 @@ func (e *Engine) imagePresentByDigest(ctx context.Context, reference string) (bo // tag. It is the one place that decides, so the pull-skipping guard and the // pinning check cannot disagree about what "pinned" means. func containsDigest(reference string) bool { return strings.Contains(reference, "@sha256:") } + +// sameImageRepository prevents a lifecycle record from pairing an authored +// reference with a digest from another repository. That mismatch can happen +// across a managed-image repository migration; retaining it would restart the +// service on bytes the current declaration can no longer produce. +func sameImageRepository(left, right string) bool { + leftNamed, err := distref.ParseNormalizedNamed(left) + if err != nil { + return false + } + rightNamed, err := distref.ParseNormalizedNamed(right) + if err != nil { + return false + } + return distref.TrimNamed(leftNamed).Name() == distref.TrimNamed(rightNamed).Name() +}