Merge pull request #11 from LINUXexpert-org/container-recreate-fidelity

Tell what a container inherits from what it overrides

ARCHITECTURE.md

@@ -300,7 +300,21 @@ against an already-migrated store.
   it carries across, naming what it found. The list is conservative and
   deliberately not exhaustive; docker's HostConfig has far more fields than
   it checks, and one it does not know about is a reason not to be
-  recreating that container at all.
+  recreating that container at all. That question is asked in preflight,
+  while the server is still running, and again at cutover: the answer does
+  not change between them, and only one of the two points can refuse
+  without having already cost an outage.
+- What a container *inherits* from its image is not what it *overrides*,
+  and only the override is the operator's. `docker inspect` reports
+  `User`, `Cmd` and `Entrypoint` either way - a container off the official
+  image reports user `stalwart` and command
+  `--config /etc/stalwart/config.json` having been given neither - so each
+  is compared against `docker image inspect` of the image the container is
+  actually on. An inherited value is left to the new image, whose own
+  defaults are the ones that go with it; an override is carried onto the
+  recreate. Reading an inherited value as an override is not a harmless
+  over-refusal: before this distinction existed, every ordinary container
+  off the official image was refused at cutover, after the stop.
 - The old container is renamed rather than removed, and the old image is
   never pruned. Together they are the container's manual restore path, the
   nearest equivalent to the preserved binary of §4.2: one command starts
@@ -852,8 +866,20 @@ happens to need them. `preflight.DeploymentKind` is a type alias for
   `run` writes it under the host side of whichever mount covers
   `--data-dir` and names it on the container side, because cutover recreates
   a container with the mounts it had and cannot invent a new one for a
-  config file. That is an inference from how the mounts must line up rather
-  than something a real deployment has confirmed.
+  config file. Cutover then starts the new container with `--config` at
+  that path, because the image's own default command points at
+  `/etc/stalwart/config.json` - a different volume, holding whatever the
+  old version left there. An overridden command and that `--config` are
+  the same argv and cannot be merged honestly, so a container with one is
+  refused rather than guessed at.
+  The config is also chowned to whatever owns the data directory before
+  anything reads it: the official image runs as uid 2000, and a
+  root-owned 0640 config is one the server cannot open - a failure that
+  arrives as a recovery boot that never comes up rather than as a
+  permission error anyone would recognise. §4.8 is why that is not left to
+  chance.
+  The path itself remains an inference from how the mounts must line up
+  rather than something a real deployment has confirmed.
 - **Cutover ignores systemd drop-ins.** It rewrites only the main unit
   file, so an `ExecStart` or `Environment` override in
   `/etc/systemd/system/stalwart.service.d/*.conf` is invisible to it -

cmd/stalwart-migrate/ownership.go

@@ -0,0 +1,49 @@
+// SPDX-FileCopyrightText: 2026 LINUXexpert-org
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package main
+
+import (
+	"fmt"
+	"os"
+	"syscall"
+)
+
+// matchOwnership gives paths the uid and gid of reference.
+//
+// It exists for the container path, where this tool writes the migrated
+// config onto the host side of a volume and something inside the container
+// has to read it. That something is not root: the official Stalwart image
+// runs as uid 2000, so a config written root-owned and 0640 is a config
+// the server cannot open, and the failure arrives as a recovery boot that
+// will not come up rather than as a permissions error anyone would
+// recognise.
+//
+// The reference is the data directory itself, whose ownership is already
+// whatever the container writes as. Copying it is more reliable than
+// resolving a user out of the image, and it is the same move
+// cutover.installConfig makes for the systemd path with
+// ConfigOwnerReference.
+//
+// ARCHITECTURE.md §4.8 is the reason this is not left to chance: the
+// rollback that was removed from this tool restored every byte correctly
+// and lost ownership, and reported success.
+func matchOwnership(reference string, paths ...string) (string, error) {
+	info, err := os.Stat(reference)
+	if err != nil {
+		return "", fmt.Errorf("stat %s to copy its ownership: %w", reference, err)
+	}
+	sys, ok := info.Sys().(*syscall.Stat_t)
+	if !ok {
+		return "ownership unchanged (this platform does not report it)", nil
+	}
+	uid, gid := int(sys.Uid), int(sys.Gid)
+	for _, p := range paths {
+		if err := os.Chown(p, uid, gid); err != nil {
+			return "", fmt.Errorf(
+				"set ownership on %s to %d:%d, which is what owns %s - the server in the container runs as that user and "+
+					"cannot read a config it does not own: %w", p, uid, gid, reference, err)
+		}
+	}
+	return fmt.Sprintf("uid %d, gid %d, copied from %s", uid, gid, reference), nil
+}

cmd/stalwart-migrate/run.go

@@ -240,6 +240,7 @@ func runRun(args []string) (err error) {
 	// side.
 	convertedConfig := filepath.Join(runWorkDir, "config.json")
 	containerConfigPath := ""
+	containerConfigDir, containerConfigOwner := "", ""
 	if isContainer {
 		mount, ok := containerFacts.MountFor(*dataDir)
 		if !ok {
@@ -254,6 +255,8 @@ func runRun(args []string) (err error) {
 		}
 		convertedConfig = filepath.Join(hostDir, "config.json")
 		containerConfigPath = path.Join(mount.Destination, "stalwart-migrate", "config.json")
+		containerConfigOwner = mount.Source
+		containerConfigDir = hostDir
 	}
 	convertedExport := filepath.Join(runWorkDir, "export.json")
 	unmigratedPath := filepath.Join(runWorkDir, "unmigrated.txt")
@@ -378,6 +381,17 @@ func runRun(args []string) (err error) {
 			if len(tenantFix.Adoptions) > 0 {
 				detail += " - " + tenantFix.String()
 			}
+			// The container reads this config, and it does not run as
+			// root. Done here rather than at cutover because the recovery
+			// cycle is the first thing to open it, and a config it cannot
+			// read surfaces there as a boot that never comes up.
+			if containerConfigOwner != "" {
+				owner, err := matchOwnership(containerConfigOwner, containerConfigDir, convertedConfig)
+				if err != nil {
+					return checkpoint.StepOutcome{}, err
+				}
+				detail += fmt.Sprintf("; %s is owned %s", convertedConfig, owner)
+			}
 			return checkpoint.StepOutcome{Detail: detail}, nil
 		}); err != nil {
 			return fmt.Errorf("convert settings: %w", err)
@@ -446,7 +460,7 @@ func runRun(args []string) (err error) {
 		ServiceUnitPath: *serviceUnitPath, ConfigPath: *newConfigPath,
 		ConfigSource: configSource, ConfigOwnerReference: *configPath,
 		Deployment:             service.Options{Kind: service.Kind(rs.Topology.DeploymentKind), UnitName: *unitName, ContainerName: *containerName},
-		Container:              containerCutover(isContainer, *containerName, stagedImage, runStateDir),
+		Container:              containerCutover(isContainer, *containerName, stagedImage, runStateDir, containerConfigOf(p, containerConfigPath)),
 		RecoveryPointConfirmed: *recoveryConfirmed,
 		AdminURL:               *adminURL, AdminUser: *adminUser, AdminPassword: *adminPassword,
 		HTTPClient: httpClient, RecalculateQuotas: *recalcQuotas && p.CrossesMajorBoundary,
@@ -524,9 +538,26 @@ func buildSupplement(settingsPath, principalsPath, unmigratedPath, outPath strin
 // containerCutover is the cutover options for a container deployment, or
 // nil for a binary one. Nil is what keeps cutover refusing a container it
 // was given no image to recreate from.
-func containerCutover(isContainer bool, name, image, preserveDir string) *cutover.ContainerOptions {
+// containerConfigOf is the container-side config path cutover should start
+// the new container with, which is none at all for a patch bump: nothing
+// was converted, so the container keeps the command its image gives it and
+// the configuration it was already running under.
+func containerConfigOf(p *plan.Plan, configPath string) string {
+	if !p.CrossesMajorBoundary {
+		return ""
+	}
+	return configPath
+}
+
+func containerCutover(isContainer bool, name, image, preserveDir, configPath string) *cutover.ContainerOptions {
 	if !isContainer {
 		return nil
 	}
-	return &cutover.ContainerOptions{ContainerName: name, StagedImage: image, PreserveDir: preserveDir}
+	// configPath is the container-side path to the migrated config, and is
+	// empty for a patch bump that converted nothing - there the container
+	// keeps whatever command its image gives it, which is what it was
+	// already running under.
+	return &cutover.ContainerOptions{
+		ContainerName: name, StagedImage: image, PreserveDir: preserveDir, ConfigPath: configPath,
+	}
 }

internal/cutover/container.go

@@ -36,6 +36,18 @@ type ContainerOptions struct {
 	// PreserveDir is where the inspected definition is written.
 	PreserveDir string
 
+	// ConfigPath is the migrated v0.16 config, named as the *container*
+	// sees it - the path inside a mount the container already has, since a
+	// recreate carries the mounts it had and cannot invent a new one.
+	//
+	// Without this the recreated container falls back to its image's own
+	// default command, which on the official image is
+	// `--config /etc/stalwart/config.json`. That path is a different
+	// volume from the data directory and holds whatever the old version
+	// left there, so the new container would come up on a config that has
+	// nothing to do with the migration that just happened.
+	ConfigPath string
+
 	DockerBinary string
 }
 
@@ -96,6 +108,11 @@ func runContainerCutover(ctx context.Context, rs *checkpoint.RunState, step step
 		return err
 	}
 
+	// Preflight asked this too, before anything stopped (§4.1). It is
+	// asked again here because the two are separated by the whole
+	// migration, and a container can be reconfigured in between - but by
+	// the time this refuses, the answer has cost an outage, which is why
+	// preflight is where it is meant to be caught.
 	if err := step("container-is-recreatable", func() (checkpoint.StepOutcome, error) {
 		if len(facts.Unsupported) > 0 {
 			return checkpoint.StepOutcome{}, fmt.Errorf(
@@ -104,6 +121,19 @@ func runContainerCutover(ctx context.Context, rs *checkpoint.RunState, step step
 					"the definition is preserved as %s, and the staged image is %s",
 				strings.Join(facts.Unsupported, "; "), rs.Artifacts[ArtifactContainerDefinition].Path, opts.StagedImage)
 		}
+		// A command of the operator's own and a config this tool has to
+		// hand over are the same argv, and there is no honest way to merge
+		// them: their command may point at another config, or at something
+		// that is not the server at all. Refusing names both rather than
+		// picking one and being quietly wrong about which server came up.
+		if opts.ConfigPath != "" && len(facts.Cmd) > 0 {
+			return checkpoint.StepOutcome{}, fmt.Errorf(
+				"this container overrides its image's command (%s), and cutting over has to start the new one with "+
+					"`--config %s` - the migrated configuration. Both are the container's argv and this tool will not guess at "+
+					"a merge. Recreate it by hand from the preserved definition at %s, on image %s, with your command adjusted "+
+					"to that config",
+				strings.Join(facts.Cmd, " "), opts.ConfigPath, rs.Artifacts[ArtifactContainerDefinition].Path, opts.StagedImage)
+		}
 		return checkpoint.StepOutcome{Detail: "the container's definition is entirely within what a recreate carries across"}, nil
 	}); err != nil {
 		return err
@@ -130,6 +160,17 @@ func runContainerCutover(ctx context.Context, rs *checkpoint.RunState, step step
 		if facts.RestartPolicy != "" && facts.RestartPolicy != "no" {
 			args = append(args, "--restart", facts.RestartPolicy)
 		}
+		// Only what the container overrode on its old image is carried.
+		// An inherited value belongs to the image, and the new image's own
+		// default is the one that goes with the new image - pinning the
+		// old image's USER or ENTRYPOINT onto it would be carrying across
+		// a decision nobody made.
+		if facts.User != "" {
+			args = append(args, "--user", facts.User)
+		}
+		if len(facts.Entrypoint) > 0 {
+			args = append(args, "--entrypoint", facts.Entrypoint[0])
+		}
 		for _, e := range facts.Env {
 			// Recovery-mode variables must never survive into a normal
 			// start: leaving STALWART_RECOVERY_MODE set would recovery-boot
@@ -164,12 +205,29 @@ func runContainerCutover(ctx context.Context, rs *checkpoint.RunState, step step
 		}
 		args = append(args, opts.StagedImage)
 
+		// Everything after the image is the container's argv. `docker run`
+		// takes only the first word of an entrypoint as --entrypoint, so
+		// the rest of it leads here.
+		if len(facts.Entrypoint) > 1 {
+			args = append(args, facts.Entrypoint[1:]...)
+		}
+		switch {
+		case opts.ConfigPath != "":
+			args = append(args, "--config", opts.ConfigPath)
+		case len(facts.Cmd) > 0:
+			args = append(args, facts.Cmd...)
+		}
+
 		if out, err := dockerOut(ctx, opts.docker(), args...); err != nil {
 			return checkpoint.StepOutcome{}, fmt.Errorf(
 				"create %s from %s: %w (%s). The previous container is still here as %s",
 				opts.ContainerName, opts.StagedImage, err, out, retired)
 		}
-		return checkpoint.StepOutcome{Detail: fmt.Sprintf("recreated %s on image %s", opts.ContainerName, opts.StagedImage)}, nil
+		detail := fmt.Sprintf("recreated %s on image %s", opts.ContainerName, opts.StagedImage)
+		if opts.ConfigPath != "" {
+			detail += ", started with --config " + opts.ConfigPath
+		}
+		return checkpoint.StepOutcome{Detail: detail}, nil
 	})
 }
 

internal/cutover/container_test.go

@@ -37,6 +37,12 @@ func inspectJSON(t *testing.T, extraHost map[string]any, networks map[string]any
 		"Config": map[string]any{
 			"Image": "stalwartlabs/stalwart:v0.15.5",
 			"Env":   []string{"TZ=UTC", "STALWART_RECOVERY_MODE=1"},
+			// Matching imageJSON's defaults, so nothing here reads as an
+			// override. A container off the official image reports all
+			// three having been given none of them.
+			"User":       imageUser,
+			"Entrypoint": imageEntrypoint,
+			"Cmd":        imageCmd,
 		},
 		"State":           map[string]any{"Running": false},
 		"Mounts":          []map[string]any{{"Type": "volume", "Name": "stalwart-data", "Destination": "/opt/stalwart", "RW": true}},
@@ -50,6 +56,28 @@ func inspectJSON(t *testing.T, extraHost map[string]any, networks map[string]any
 	return string(b)
 }
 
+// The defaults the official Stalwart image gives every container made
+// from it. They are here rather than inline because the whole point of
+// the image comparison is that a container reporting exactly these has
+// overridden nothing.
+var (
+	imageUser       = "stalwart"
+	imageEntrypoint = []string{"/usr/local/bin/stalwart"}
+	imageCmd        = []string{"--config", "/etc/stalwart/config.json"}
+)
+
+// imageJSON is `docker image inspect` for the image a container is on.
+func imageJSON(t *testing.T) string {
+	t.Helper()
+	b, err := json.Marshal([]map[string]any{{
+		"Config": map[string]any{"User": imageUser, "Entrypoint": imageEntrypoint, "Cmd": imageCmd},
+	}})
+	if err != nil {
+		t.Fatal(err)
+	}
+	return string(b)
+}
+
 // fakeDockerCutover installs a docker that records arguments and serves the
 // given inspect document.
 func fakeDockerCutover(t *testing.T, doc string) (log string) {
@@ -60,14 +88,21 @@ func fakeDockerCutover(t *testing.T, doc string) (log string) {
 	if err := os.WriteFile(inspectFile, []byte(doc), 0o644); err != nil {
 		t.Fatal(err)
 	}
+	imageFile := filepath.Join(dir, "image.json")
+	if err := os.WriteFile(imageFile, []byte(imageJSON(t)), 0o644); err != nil {
+		t.Fatal(err)
+	}
 	script := fmt.Sprintf(`#!/bin/sh
 echo "$@" >> %q
+case "$1 $2" in
+  "image inspect") cat %q ; exit 0 ;;
+esac
 case "$1" in
   inspect) cat %q ;;
   rename) exit 0 ;;
   run) echo newcontainerid ;;
 esac
-`, log, inspectFile)
+`, log, imageFile, inspectFile)
 	if err := os.WriteFile(filepath.Join(dir, "docker"), []byte(script), 0o755); err != nil {
 		t.Fatal(err)
 	}
@@ -95,10 +130,15 @@ func runContainerFor(t *testing.T, doc string) (*checkpoint.RunState, Report, er
 	}
 	err = runContainerCutover(context.Background(), rs, step, ContainerOptions{
 		ContainerName: "stalwart", StagedImage: "sha256:new", PreserveDir: t.TempDir(),
+		ConfigPath: containerConfig,
 	})
 	return rs, report, err
 }
 
+// containerConfig is the container-side path to the migrated config that
+// run passes to cutover, mirroring run.go's <data mount>/stalwart-migrate.
+const containerConfig = "/opt/stalwart/stalwart-migrate/config.json"
+
 func TestContainerCutoverPreservesTheDefinitionFirst(t *testing.T) {
 	rs, _, err := runContainerFor(t, inspectJSON(t, nil, nil))
 	if err != nil {
@@ -252,3 +292,140 @@ func readLog(t *testing.T, path string) string {
 	}
 	return string(b)
 }
+
+// The recreated container has to be started on the config the migration
+// just produced. Left to the image's own default command it would come up
+// on /etc/stalwart/config.json - a different volume from the data
+// directory, holding whatever the old version left there - and be a
+// server with nothing to do with the migration that preceded it.
+// @kaya-eu did this step by hand on three real migrations.
+func TestContainerCutoverStartsOnTheMigratedConfig(t *testing.T) {
+	log := fakeDockerCutover(t, inspectJSON(t, nil, nil))
+	store := checkpoint.NewStore(t.TempDir())
+	rs, err := store.Create("0.15.5", "0.16.14")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := runContainerCutover(context.Background(), rs, noopStep(store, rs), ContainerOptions{
+		ContainerName: "stalwart", StagedImage: "sha256:new", PreserveDir: t.TempDir(),
+		ConfigPath: containerConfig,
+	}); err != nil {
+		t.Fatalf("runContainerCutover: %v", err)
+	}
+	run := runLine(t, log)
+	if !strings.Contains(run, "--config "+containerConfig) {
+		t.Errorf("run should start the container on the migrated config, got: %s", run)
+	}
+	// After the image, not before: everything past it is the argv.
+	if strings.Index(run, "sha256:new") > strings.Index(run, "--config "+containerConfig) {
+		t.Errorf("--config must come after the image, got: %s", run)
+	}
+}
+
+// An inherited USER belongs to the image. Passing --user stalwart to the
+// new image would be carrying across a decision nobody made, and would
+// break outright on an image that named its user differently.
+func TestContainerCutoverDoesNotCarryInheritedDefaults(t *testing.T) {
+	log := fakeDockerCutover(t, inspectJSON(t, nil, nil))
+	store := checkpoint.NewStore(t.TempDir())
+	rs, _ := store.Create("0.15.5", "0.16.14")
+	if err := runContainerCutover(context.Background(), rs, noopStep(store, rs), ContainerOptions{
+		ContainerName: "stalwart", StagedImage: "sha256:new", PreserveDir: t.TempDir(),
+		ConfigPath: containerConfig,
+	}); err != nil {
+		t.Fatalf("runContainerCutover: %v", err)
+	}
+	run := runLine(t, log)
+	for _, unwanted := range []string{"--user", "--entrypoint"} {
+		if strings.Contains(run, unwanted) {
+			t.Errorf("run carried %s across from the old image's defaults: %s", unwanted, run)
+		}
+	}
+}
+
+// What the operator did override is theirs, and a recreate that drops it
+// starts cleanly as a different server - the failure Unsupported exists to
+// prevent, for two settings that were not being read at all.
+func TestContainerCutoverCarriesTheOperatorsOverrides(t *testing.T) {
+	doc := inspectJSON(t, nil, nil)
+	doc = strings.Replace(doc, `"User":"stalwart"`, `"User":"1500:1500"`, 1)
+	doc = strings.Replace(doc, `"Entrypoint":["/usr/local/bin/stalwart"]`,
+		`"Entrypoint":["/usr/local/bin/wrapper","--trace"]`, 1)
+	log := fakeDockerCutover(t, doc)
+	store := checkpoint.NewStore(t.TempDir())
+	rs, _ := store.Create("0.15.5", "0.16.14")
+	if err := runContainerCutover(context.Background(), rs, noopStep(store, rs), ContainerOptions{
+		ContainerName: "stalwart", StagedImage: "sha256:new", PreserveDir: t.TempDir(),
+		ConfigPath: containerConfig,
+	}); err != nil {
+		t.Fatalf("runContainerCutover: %v", err)
+	}
+	run := runLine(t, log)
+	if !strings.Contains(run, "--user 1500:1500") {
+		t.Errorf("run should carry the overridden user: %s", run)
+	}
+	// docker run takes one word as --entrypoint; the rest is argv.
+	if !strings.Contains(run, "--entrypoint /usr/local/bin/wrapper") {
+		t.Errorf("run should carry the overridden entrypoint: %s", run)
+	}
+	if !strings.Contains(run, "sha256:new --trace") {
+		t.Errorf("the rest of the entrypoint should lead the argv: %s", run)
+	}
+}
+
+// A command of the operator's own and the config this tool has to hand
+// over are the same argv. Their command may point at another config, or
+// at something that is not the server - so this refuses rather than
+// merging and being quietly wrong about which server came up.
+func TestContainerCutoverRefusesToMergeACommandWithTheConfig(t *testing.T) {
+	doc := strings.Replace(inspectJSON(t, nil, nil),
+		`"Cmd":["--config","/etc/stalwart/config.json"]`, `"Cmd":["--config","/srv/mine.toml"]`, 1)
+	_, _, err := runContainerFor(t, doc)
+	if err == nil {
+		t.Fatal("want a refusal when an overridden command collides with the migrated config")
+	}
+	for _, want := range []string{"/srv/mine.toml", containerConfig, "by hand"} {
+		if !strings.Contains(err.Error(), want) {
+			t.Errorf("refusal should name %q, got: %v", want, err)
+		}
+	}
+}
+
+// A patch bump converts nothing, so there is no config to point at and
+// the container keeps the command its image gives it.
+func TestContainerCutoverKeepsTheImageCommandWithoutAConfig(t *testing.T) {
+	log := fakeDockerCutover(t, inspectJSON(t, nil, nil))
+	store := checkpoint.NewStore(t.TempDir())
+	rs, _ := store.Create("0.16.14", "0.16.19")
+	if err := runContainerCutover(context.Background(), rs, noopStep(store, rs), ContainerOptions{
+		ContainerName: "stalwart", StagedImage: "sha256:new", PreserveDir: t.TempDir(),
+	}); err != nil {
+		t.Fatalf("runContainerCutover: %v", err)
+	}
+	if run := runLine(t, log); !strings.HasSuffix(strings.TrimSpace(run), "sha256:new") {
+		t.Errorf("run should end at the image, with no argv of its own: %s", run)
+	}
+}
+
+func noopStep(store *checkpoint.Store, rs *checkpoint.RunState) stepFunc {
+	return func(name string, fn func() (checkpoint.StepOutcome, error)) error {
+		_, err := store.RunStep(rs, checkpoint.PhaseCutover, name, fn)
+		return err
+	}
+}
+
+// runLine is the `docker run` the fake recorded.
+func runLine(t *testing.T, log string) string {
+	t.Helper()
+	data, err := os.ReadFile(log)
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, line := range strings.Split(string(data), "\n") {
+		if strings.HasPrefix(line, "run ") {
+			return line
+		}
+	}
+	t.Fatalf("no `docker run` in the recorded arguments:\n%s", data)
+	return ""
+}

internal/preflight/checks_test.go

@@ -474,6 +474,14 @@ func withFakeDocker(t *testing.T) {
 // dockerPreflight runs a minimal but real preflight against a fake 0.15.5
 // install with a fake docker on PATH.
 func dockerPreflight(t *testing.T, advisory bool) Report {
+	t.Helper()
+	return dockerPreflightOn(t, advisory, nil)
+}
+
+// dockerPreflightOn runs preflight against a container whose inspect
+// document has been rewritten by edit, for the cases that need it to be
+// something other than an ordinary one.
+func dockerPreflightOn(t *testing.T, advisory bool, edit func(string) string) Report {
 	t.Helper()
 	// disk-space stats DataDir on this host, and container-data-volume
 	// wants it covered by a mount, so it has to be both: a real directory,
@@ -484,7 +492,11 @@ func dockerPreflight(t *testing.T, advisory bool) Report {
 			t.Skipf("host has %s, which detection prefers over docker", p)
 		}
 	}
-	fakeInspect(t, inspectDoc(t, nil, []Mount{dataVolume(dataDir)}))
+	doc := inspectDoc(t, nil, []Mount{dataVolume(dataDir)})
+	if edit != nil {
+		doc = edit(doc)
+	}
+	fakeInspect(t, doc)
 
 	counterPath := filepath.Join(t.TempDir(), "invocations")
 	binaryPath := writeFakeBinary(t, "0.15.5", counterPath)
@@ -553,3 +565,45 @@ func TestRehearseStillRunsAgainstADockerDeployment(t *testing.T) {
 		t.Fatalf("advisory mode should not block on docker, got:\n%s", report.String())
 	}
 }
+
+// Cutover already refused a container it could not recreate, but cutover
+// is downstream of the stop, the settings conversion and the store
+// migration - so that refusal arrived with the mail down and the data
+// already moved, which is the shape of failure issue #1 was filed for.
+// The answer never changes between the two points, so it is asked here,
+// while the server is still running.
+func TestPreflightRefusesAContainerItCouldNotRecreate(t *testing.T) {
+	report := dockerPreflightOn(t, false, func(doc string) string {
+		return strings.Replace(doc, `"State":`, `"HostConfig":{"Privileged":true},"State":`, 1)
+	})
+	if !report.Blocking() {
+		t.Fatalf("a container preflight cannot recreate should block before anything stops:\n%s", report.String())
+	}
+	var found bool
+	for _, res := range report.Results {
+		if res.Name == "container-recreatable" {
+			found = true
+			if res.Status != StatusFail {
+				t.Errorf("container-recreatable status = %q, want %q", res.Status, StatusFail)
+			}
+			if !strings.Contains(res.Detail, "privileged") {
+				t.Errorf("detail should name what would be dropped, got %q", res.Detail)
+			}
+		}
+	}
+	if !found {
+		t.Fatalf("no container-recreatable result in report:\n%s", report.String())
+	}
+}
+
+// rehearse never stops or recreates anything, so the same finding is a
+// warning there: an operator migrating by hand needs to know it more than
+// an automated run does.
+func TestRehearseWarnsRatherThanBlocksOnAnUnrecreatableContainer(t *testing.T) {
+	report := dockerPreflightOn(t, true, func(doc string) string {
+		return strings.Replace(doc, `"State":`, `"HostConfig":{"Privileged":true},"State":`, 1)
+	})
+	if report.Blocking() {
+		t.Fatalf("advisory mode should not block, got:\n%s", report.String())
+	}
+}

internal/preflight/container.go

@@ -55,7 +55,23 @@ type ContainerFacts struct {
 	Ports         map[string][]PortBinding
 	RestartPolicy string
 	NetworkMode   string
-	Unsupported   []string // populated by UnsupportedForRecreate
+	Unsupported   []string // populated by unsupportedForRecreate
+
+	// User, Entrypoint and Cmd are set only when the container overrides
+	// what its image already says.
+	//
+	// The distinction is the whole point. `docker inspect` reports these
+	// three whether the operator set them or the image did - a container
+	// off the official image reports User "stalwart" and Cmd
+	// ["--config", "/etc/stalwart/config.json"] having been given
+	// neither. Treating an inherited value as the operator's would either
+	// refuse every ordinary container or pin the new image to the old
+	// image's defaults, and the new image's defaults are the ones that go
+	// with the new image. Only a genuine override is the operator's
+	// decision, and only that has to survive a recreate.
+	User       string
+	Entrypoint []string
+	Cmd        []string
 }
 
 // PortBinding is one published port.
@@ -109,12 +125,7 @@ func (f ContainerFacts) MountFor(path string) (Mount, bool) {
 type inspectOutput struct {
 	Name   string          `json:"Name"`
 	Image  string          `json:"Image"`
-	Config struct {
-		Image  string            `json:"Image"`
-		Labels map[string]string `json:"Labels"`
-		Env    []string          `json:"Env"`
-		User   string            `json:"User"`
-	} `json:"Config"`
+	Config containerConfig `json:"Config"`
 	State  struct {
 		Running bool `json:"Running"`
 	} `json:"State"`
@@ -145,6 +156,24 @@ type inspectOutput struct {
 	} `json:"NetworkSettings"`
 }
 
+// containerConfig is the part of a container's or an image's Config this
+// reasons about. Both docker objects carry the same shape here, which is
+// what makes comparing them possible.
+type containerConfig struct {
+	Image      string            `json:"Image"`
+	Labels     map[string]string `json:"Labels"`
+	Env        []string          `json:"Env"`
+	User       string            `json:"User"`
+	Entrypoint []string          `json:"Entrypoint"`
+	Cmd        []string          `json:"Cmd"`
+}
+
+// imageInspectOutput is `docker image inspect`, which reports the defaults
+// a container inherits when it was given none of its own.
+type imageInspectOutput struct {
+	Config containerConfig `json:"Config"`
+}
+
 // InspectContainer reads the facts about containerName. An error here is
 // an error, not an absent container: callers reach this only after
 // DetectDeploymentKind has already established that a container answers to
@@ -178,10 +207,66 @@ func InspectContainer(ctx context.Context, containerName string) (ContainerFacts
 		RestartPolicy: c.HostConfig.RestartPolicy.Name,
 		NetworkMode:   c.HostConfig.NetworkMode,
 	}
+
+	// The image the container is actually on, by ID rather than by the tag
+	// it was started from: a tag can have moved since, and then this would
+	// be comparing the container against something it never inherited
+	// from.
+	base, err := inspectImage(ctx, c.Image)
+	if err != nil {
+		return ContainerFacts{}, err
+	}
+	if c.Config.User != base.User {
+		f.User = c.Config.User
+	}
+	if !sameArgs(c.Config.Entrypoint, base.Entrypoint) {
+		f.Entrypoint = c.Config.Entrypoint
+	}
+	if !sameArgs(c.Config.Cmd, base.Cmd) {
+		f.Cmd = c.Config.Cmd
+	}
+
 	f.Unsupported = unsupportedForRecreate(c)
 	return f, nil
 }
 
+// inspectImage reads the defaults an image gives the containers made from
+// it. A failure here is an error for the same reason a failed container
+// inspect is: without it there is no way to tell an operator's --user from
+// the image's own USER, and the difference decides what a recreate has to
+// carry.
+func inspectImage(ctx context.Context, imageID string) (containerConfig, error) {
+	if imageID == "" {
+		return containerConfig{}, fmt.Errorf("preflight: container reports no image to compare its configuration against")
+	}
+	out, err := exec.CommandContext(ctx, "docker", "image", "inspect", imageID).Output()
+	if err != nil {
+		return containerConfig{}, fmt.Errorf("preflight: docker image inspect %s: %w", imageID, err)
+	}
+	var got []imageInspectOutput
+	if err := json.Unmarshal(out, &got); err != nil {
+		return containerConfig{}, fmt.Errorf("preflight: parsing docker image inspect %s: %w", imageID, err)
+	}
+	if len(got) == 0 {
+		return containerConfig{}, fmt.Errorf("preflight: docker image inspect %s returned no image", imageID)
+	}
+	return got[0].Config, nil
+}
+
+// sameArgs compares two argv slices, treating nil and empty as the same
+// thing - docker reports an absent Cmd either way depending on version.
+func sameArgs(a, b []string) bool {
+	if len(a) != len(b) {
+		return false
+	}
+	for i := range a {
+		if a[i] != b[i] {
+			return false
+		}
+	}
+	return true
+}
+
 // unsupportedForRecreate names every piece of this container's
 // configuration that recreating it would not carry across.
 //
@@ -217,7 +302,6 @@ func unsupportedForRecreate(c inspectOutput) []string {
 	add(len(h.SecurityOpt) > 0, "security options (--security-opt)")
 	add(len(h.Tmpfs) > 0, "tmpfs mounts (--tmpfs)")
 	add(h.LogConfig.Type != "" && h.LogConfig.Type != "json-file", "a non-default log driver (--log-driver "+h.LogConfig.Type+")")
-	add(c.Config.User != "", "a container user (--user "+c.Config.User+")")
 
 	// A user-defined network is a name in NetworkSettings.Networks that is
 	// not one of docker's built-ins. Recreating without it puts the server
@@ -274,6 +358,31 @@ func (c *Checker) runContainerChecks(ctx context.Context, runCheck checkFunc) er
 		return err
 	}
 
+	if _, err := runCheck("container-recreatable", func() (CheckResult, string) {
+		// Asked here, while the server is still running, rather than at
+		// cutover where the answer was first needed. Cutover is downstream
+		// of the stop, the settings conversion and the store migration, so
+		// a refusal there is a refusal with the mail already down and the
+		// data already moved - the shape of failure issue #1 was filed for.
+		// Nothing about this answer changes between the two points.
+		if len(facts.Unsupported) > 0 {
+			status := StatusFail
+			if c.opts.DeploymentCheckAdvisory {
+				status = StatusWarn
+			}
+			return CheckResult{Status: status, Detail: fmt.Sprintf(
+				"this container uses configuration that recreating it would not carry across: %s. A container is replaced "+
+					"rather than edited, so those would be silently dropped and the result would start cleanly without being "+
+					"the server it was. Migrate this one by hand",
+				strings.Join(facts.Unsupported, "; "))}, strings.Join(facts.Unsupported, "; ")
+		}
+		carried := describeOverrides(facts)
+		return CheckResult{Status: StatusOK, Detail: "the container's definition is entirely within what a recreate " +
+			"carries across" + carried}, ""
+	}); err != nil {
+		return err
+	}
+
 	_, err := runCheck("container-data-volume", func() (CheckResult, string) {
 		writable := facts.WritableMounts()
 		if len(writable) == 0 {
@@ -307,6 +416,26 @@ func (c *Checker) runContainerChecks(ctx context.Context, runCheck checkFunc) er
 	return err
 }
 
+// describeOverrides names the settings a container holds that its image
+// does not, so an operator reading a green check can see what a recreate
+// is being trusted to carry rather than taking "entirely within" on faith.
+func describeOverrides(f ContainerFacts) string {
+	var parts []string
+	if f.User != "" {
+		parts = append(parts, "--user "+f.User)
+	}
+	if len(f.Entrypoint) > 0 {
+		parts = append(parts, "--entrypoint "+strings.Join(f.Entrypoint, " "))
+	}
+	if len(f.Cmd) > 0 {
+		parts = append(parts, "a command ("+strings.Join(f.Cmd, " ")+")")
+	}
+	if len(parts) == 0 {
+		return ""
+	}
+	return ", including what it overrides on its image: " + strings.Join(parts, ", ")
+}
+
 func shortID(id string) string {
 	id = strings.TrimPrefix(id, "sha256:")
 	if len(id) > 12 {

internal/preflight/container_test.go

@@ -10,33 +10,72 @@ import (
 	"net/http"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 
 	"github.com/LINUXexpert-org/stalwart-migrator/internal/checkpoint"
 )
 
+// The defaults the official Stalwart image gives every container made from
+// it. A container reporting exactly these has overridden nothing, which is
+// the case the image comparison exists to recognise - `docker inspect`
+// reports all three either way.
+var (
+	imageUser       = "stalwart"
+	imageEntrypoint = []string{"/usr/local/bin/stalwart"}
+	imageCmd        = []string{"--config", "/etc/stalwart/config.json"}
+)
+
 // fakeInspect writes a `docker` that answers `inspect` with the given JSON
 // document, so the container checks can be exercised without a container.
+// `image inspect` answers with the official image's own defaults.
 func fakeInspect(t *testing.T, doc string) {
+	t.Helper()
+	fakeInspectOn(t, doc, imageDoc(t, imageUser, imageEntrypoint, imageCmd))
+}
+
+// fakeInspectOn is fakeInspect with the image's defaults named, for the
+// tests that need the container and its image to disagree.
+func fakeInspectOn(t *testing.T, containerDoc, imgDoc string) {
 	t.Helper()
 	dir := t.TempDir()
 	out := filepath.Join(dir, "inspect.json")
-	if err := os.WriteFile(out, []byte(doc), 0o644); err != nil {
+	if err := os.WriteFile(out, []byte(containerDoc), 0o644); err != nil {
 		t.Fatal(err)
 	}
-	script := fmt.Sprintf("#!/bin/sh\ncase \"$1\" in inspect) cat %q ;; *) exit 1 ;; esac\n", out)
+	img := filepath.Join(dir, "image.json")
+	if err := os.WriteFile(img, []byte(imgDoc), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	script := fmt.Sprintf("#!/bin/sh\n"+
+		"case \"$1 $2\" in \"image inspect\") cat %q ; exit 0 ;; esac\n"+
+		"case \"$1\" in inspect) cat %q ;; *) exit 1 ;; esac\n", img, out)
 	if err := os.WriteFile(filepath.Join(dir, "docker"), []byte(script), 0o755); err != nil {
 		t.Fatal(err)
 	}
 	t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH"))
 }
 
+func imageDoc(t *testing.T, user string, entrypoint, cmd []string) string {
+	t.Helper()
+	b, err := json.Marshal([]map[string]any{{
+		"Config": map[string]any{"User": user, "Entrypoint": entrypoint, "Cmd": cmd},
+	}})
+	if err != nil {
+		t.Fatal(err)
+	}
+	return string(b)
+}
+
 func inspectDoc(t *testing.T, labels map[string]string, mounts []Mount) string {
 	t.Helper()
 	doc := []map[string]any{{
 		"Name":  "/stalwart",
 		"Image": "sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef",
-		"Config": map[string]any{"Image": "stalwartlabs/stalwart:v0.15.5", "Labels": labels},
+		"Config": map[string]any{
+			"Image": "stalwartlabs/stalwart:v0.15.5", "Labels": labels,
+			"User": imageUser, "Entrypoint": imageEntrypoint, "Cmd": imageCmd,
+		},
 		"State":  map[string]any{"Running": true},
 		"Mounts": mounts,
 	}}
@@ -218,3 +257,69 @@ func TestRehearseReportsContainerProblemsWithoutBlocking(t *testing.T) {
 		}
 	}
 }
+
+// docker reports Config.User, Cmd and Entrypoint whether the operator set
+// them or the image did. A container off the official image reports user
+// "stalwart" having been given no --user, and reading that as an operator
+// override made this tool refuse to recreate every ordinary Stalwart
+// container - at cutover, with the mail already down. Found while checking
+// @kaya-eu's field report against a real image.
+func TestInspectContainerIgnoresWhatItInheritedFromItsImage(t *testing.T) {
+	fakeInspect(t, inspectDoc(t, nil, []Mount{dataVolume("/var/lib/stalwart")}))
+
+	facts, err := InspectContainer(context.Background(), "stalwart")
+	if err != nil {
+		t.Fatalf("InspectContainer: %v", err)
+	}
+	if facts.User != "" {
+		t.Errorf("User = %q, want empty: it is the image's own USER, not an override", facts.User)
+	}
+	if len(facts.Cmd) != 0 {
+		t.Errorf("Cmd = %v, want none: it is the image's own CMD", facts.Cmd)
+	}
+	if len(facts.Entrypoint) != 0 {
+		t.Errorf("Entrypoint = %v, want none: it is the image's own ENTRYPOINT", facts.Entrypoint)
+	}
+	if len(facts.Unsupported) != 0 {
+		t.Errorf("Unsupported = %v, want none for a plain container off the official image", facts.Unsupported)
+	}
+}
+
+// The other half of the same distinction: what the operator really did
+// override has to be visible, because a recreate that drops it starts
+// cleanly as a different server.
+func TestInspectContainerReportsWhatTheOperatorOverrode(t *testing.T) {
+	doc := inspectDoc(t, nil, []Mount{dataVolume("/var/lib/stalwart")})
+	doc = strings.Replace(doc, `"User":"stalwart"`, `"User":"1500:1500"`, 1)
+	doc = strings.Replace(doc, `"Cmd":["--config","/etc/stalwart/config.json"]`, `"Cmd":["--config","/srv/mine.toml"]`, 1)
+	if strings.Contains(doc, `"User":"stalwart"`) || strings.Contains(doc, "/etc/stalwart/config.json") {
+		t.Fatal("the fixture did not take the overrides; the inspect document shape changed")
+	}
+	fakeInspect(t, doc)
+
+	facts, err := InspectContainer(context.Background(), "stalwart")
+	if err != nil {
+		t.Fatalf("InspectContainer: %v", err)
+	}
+	if facts.User != "1500:1500" {
+		t.Errorf("User = %q, want the overridden 1500:1500", facts.User)
+	}
+	if strings.Join(facts.Cmd, " ") != "--config /srv/mine.toml" {
+		t.Errorf("Cmd = %v, want the overridden command", facts.Cmd)
+	}
+	// An entrypoint it did not override still reads as inherited.
+	if len(facts.Entrypoint) != 0 {
+		t.Errorf("Entrypoint = %v, want none", facts.Entrypoint)
+	}
+}
+
+// Without the image's defaults there is no way to tell an override from an
+// inheritance, and guessing decides what a recreate carries. Same rule as
+// a failed container inspect: an error, not an assumption.
+func TestInspectContainerRefusesWhenTheImageCannotBeRead(t *testing.T) {
+	fakeInspectOn(t, inspectDoc(t, nil, []Mount{dataVolume("/var/lib/stalwart")}), "")
+
+	if _, err := InspectContainer(context.Background(), "stalwart"); err == nil {
+		t.Fatal("want an error when the image's defaults cannot be read")
+	}
+}