Deep Review: 20260416-144802-pr-320

	if err := r.deleteAllOfType(ctx, &containersv1alpha1.ContainerList{}); err != nil {
		errs = append(errs, fmt.Errorf("failed to delete Containers: %w", err))
	}
	if err := r.deleteAllOfType(ctx, &containersv1alpha1.VolumeList{}); err != nil {
		errs = append(errs, fmt.Errorf("failed to delete Volumes: %w", err))
	}
	if err := r.deleteAllOfType(ctx, &containersv1alpha1.ImageList{}); err != nil {
		errs = append(errs, fmt.Errorf("failed to delete Images: %w", err))
	}
	if err := r.deleteAllOfType(ctx, &containersv1alpha1.ContainerNamespaceList{}); err != nil {
		errs = append(errs, fmt.Errorf("failed to delete ContainerNamespaces: %w", err))
	}
	return errors.Join(errs...)
}

// deleteAllOfType lists and removes every resource of the given kind
// in r.apiNamespace. Engine is authoritative for Container, Image, and
// Volume in the App's namespace: this loop deletes every object, not
// just engine-owned mirrors. Coexistence with another writer to these
// kinds requires an engine-owned label and matching filters here and
// in the sync_*.go full-sync prune paths. Finalizer removal retries
// on conflict; per-item errors are collected so one stuck object does
// not block the rest.
func (r *EngineReconciler) deleteAllOfType(ctx context.Context, list client.ObjectList) error {
	if err := r.List(ctx, list, client.InNamespace(r.apiNamespace)); err != nil {
		return err
	}

	items, err := meta.ExtractList(list)
	if err != nil {
		return err
	}

	var errs []error
	for _, item := range items {
		obj := item.(client.Object)
		key := client.ObjectKeyFromObject(obj)
		// meta.ExtractList leaves each item's TypeMeta empty (GVK is
		// only on the list), so format the Go type with %T instead of
		// obj.GetObjectKind().
		retryErr := retry.RetryOnConflict(retry.DefaultRetry, func() error {
			latest := obj.DeepCopyObject().(client.Object)
			if err := r.Get(ctx, key, latest); err != nil {
				if apierrors.IsNotFound(err) {

When the engine stops, removeDockerContext (line 328) cancels probeCtx, clears currentContext if it points at us, and deletes the context dir — without waiting for the probe goroutine to finish. The probe interprets the cancellation in probeDockerContext's Ping as "context is unhealthy" and proceeds to write setCurrentDockerContext(contextName). That write can land after removeDockerContext has already deleted the meta dir, leaving currentContext pointing at a non-existent context. Every subsequent docker ps/docker build/docker context inspect then fails with "Current context … is not found" until the user manually runs docker context use default.

		// only on the list), so format the Go type with %T instead of
		// obj.GetObjectKind().
		retryErr := retry.RetryOnConflict(retry.DefaultRetry, func() error {
			latest := obj.DeepCopyObject().(client.Object)
			if err := r.Get(ctx, key, latest); err != nil {
				if apierrors.IsNotFound(err) {
					return nil
				}
				return err
			}
			if !controllerutil.RemoveFinalizer(latest, mirrorFinalizer) {

The generation counter (myGen) only protects the cancel-slot bookkeeping; the disk write on line 319 is not gated on probeCtx.Err(), generation match, or any wait on the cleanup path.

Fix: gate the promotion on probeCtx.Err(). A fully race-free fix also adds a sync.WaitGroup so removeDockerContext blocks until the probe returns:

 go func() {
+    r.contextProbeWg.Add(1)
+    defer r.contextProbeWg.Done()
     defer func() {
         ...
     }()
     ...
     if !healthy {
+        if probeCtx.Err() != nil {
+            return
+        }
         if err := setCurrentDockerContext(contextName); err != nil {
             ...
         }
     }
 }()

 func (r *EngineReconciler) removeDockerContext() {
     r.contextMu.Lock()
     ...
     r.contextMu.Unlock()
+    r.contextProbeWg.Wait()
     ...
 }

The wait is bounded by dockerContextProbeTimeout (3s) because probeCtx gets cancelled before the wait. The Err() check alone closes most of the window; the WaitGroup closes the remainder where the goroutine has read !healthy and is about to call setCurrentDockerContext when cancellation fires.

		return nil
	}
	return err
}

// getDockerContextSocket returns the Unix socket path that the named context
// points at. Returns an empty string if the context does not exist or has no
// docker endpoint.
func getDockerContextSocket(name string) (string, error) {
	path, err := contextMetaPath(name)
	if err != nil {
		return "", err
	}
	data, err := os.ReadFile(path)
	if errors.Is(err, os.ErrNotExist) {
		return "", nil
	}
	if err != nil {
		return "", err
	}
	var meta dockerContextMeta
	if err := json.Unmarshal(data, &meta); err != nil {
		return "", err
	}
	host := meta.Endpoints["docker"].Host
	return strings.TrimPrefix(host, "unix://"), nil
}

// getCurrentDockerContext reads the currentContext field from ~/.docker/config.json.
// Returns an empty string if the file does not exist or no context is set.
func getCurrentDockerContext() (string, error) {
	dir, err := dockerConfigDir()

If the user's current context has Host: "tcp://foo:2375" (or ssh://, npipe://), getDockerContextSocket returns the string unchanged because the "unix://" prefix doesn't match. probeDockerContext then constructs "unix://tcp://foo:2375", the moby client parses it as proto=unix, addr="tcp://foo:2375", attempts to dial a Unix socket at the literal path, and fails. The probe reports healthy=false, and the goroutine silently overrides the user's working remote/Windows-pipe context with ours.

This violates the PR's documented contract ("if the current context is healthy, leave it alone") for every non-Unix endpoint and is the same class of bug Windows users will hit in I4.

Fix: stop stripping the scheme — pass the full Host URL through to the client. Rename for clarity:

-func getDockerContextSocket(name string) (string, error) {
+// getDockerContextHost returns the Docker Host URL (e.g. "unix:///path",
+// "tcp://host:port") that the named context points at.
+func getDockerContextHost(name string) (string, error) {
     ...
-    host := meta.Endpoints["docker"].Host
-    return strings.TrimPrefix(host, "unix://"), nil
+    return meta.Endpoints["docker"].Host, nil
 }

-func probeDockerContext(ctx context.Context, socketPath string) bool {
+func probeDockerContext(ctx context.Context, host string) bool {
     ...
-    cli, err := dockerclient.New(dockerclient.WithHost("unix://" + socketPath))
+    cli, err := dockerclient.New(dockerclient.WithHost(host))
     ...
 }

}

// updateDockerConfig reads the Docker config file at path, applies mutate,
// and writes the result back. All other keys in the file are preserved.
// If the file does not exist and mutate leaves cfg empty, no file is written.
func updateDockerConfig(path string, mutate func(map[string]any)) error {
	cfg := map[string]any{}
	data, err := os.ReadFile(path)
	notExist := errors.Is(err, os.ErrNotExist)
	switch {
	case err == nil:
		if jsonErr := json.Unmarshal(data, &cfg); jsonErr != nil {
			return jsonErr
		}
	case notExist:
	// we should create the file below if needed
	default:
		return err
	}

	before := len(cfg)
	mutate(cfg)

	// file didn't exist and there is nothing left nothing to write.
	if notExist && len(cfg) == 0 && before == 0 {
		return nil
	}

	out, err := json.MarshalIndent(cfg, "", "\t")
	if err != nil {
		return err
	}
	if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil {
		return err
	}
	return os.WriteFile(path, append(out, '\n'), 0o600)
}

updateDockerConfig uses os.WriteFile, which opens with O_TRUNC and writes in place. Two failure modes:

1. Crash after truncate, before write completes: The user's ~/.docker/config.json is left empty or partial, losing every key it held (auths, credsStore, currentContext, plugin settings, etc.).
2. Race with concurrent docker CLI: A docker login or docker context use running concurrently performs its own read-modify-write. Whichever write lands second silently clobbers the other's changes — the user loses an auth entry or a context switch with no error.

Severity is amplified by the design: removeDockerContext is called on every reconcile when the engine is stopped (see S1), so the file is read, re-marshalled, and rewritten on a steady cadence in the background even when nothing has changed.

Fix: write to a sibling temp file and rename(2) — atomic on POSIX, and fixable on Windows by removing the destination first. Also short-circuit when mutate produces no change so steady state pays no I/O cost:

-func updateDockerConfig(path string, mutate func(map[string]any)) error {
+func updateDockerConfig(path string, mutate func(map[string]any) bool) error {
     cfg := map[string]any{}
     ...
-    before := len(cfg)
-    mutate(cfg)
-
-    if notExist && len(cfg) == 0 && before == 0 {
+    if changed := mutate(cfg); !changed {
         return nil
     }

     out, err := json.MarshalIndent(cfg, "", "\t")
     ...
-    return os.WriteFile(path, append(out, '\n'), 0o600)
+    tmp, err := os.CreateTemp(filepath.Dir(path), ".config.json.*")
+    if err != nil {
+        return err
+    }
+    if _, err := tmp.Write(append(out, '\n')); err != nil {
+        tmp.Close()
+        os.Remove(tmp.Name())
+        return err
+    }
+    if err := tmp.Chmod(0o600); err != nil {
+        tmp.Close()
+        os.Remove(tmp.Name())
+        return err
+    }
+    if err := tmp.Close(); err != nil {
+        os.Remove(tmp.Name())
+        return err
+    }
+    return os.Rename(tmp.Name(), path)
 }

The mutate callback returns bool so the caller can signal whether it actually changed anything — setCurrentDockerContext returns true only when the field changes, clearCurrentDockerContext only when the matching key existed. This collapses I3 with S1 (no rewrite-on-every-reconcile).

		return err
	}
	if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil {
		return err
	}
	meta := dockerContextMeta{
		Name:     name,
		Metadata: map[string]any{},
		Endpoints: map[string]dockerEndpointMeta{
			"docker": {Host: "unix://" + socketPath},
		},
	}
	data, err := json.MarshalIndent(meta, "", "\t")
	if err != nil {
		return err
	}

instance.DockerSocket() returns a Windows path like %USERPROFILE%\.rd<suffix>\docker.sock; the resulting meta.json contains Host: "unix://C:\\Users\\…\\docker.sock", which Docker CLI on Windows cannot use (it expects npipe://./pipe/...). The BATS file skips Windows at line 15, but manageDockerContext runs unconditionally in production, so the controller will write a broken context for every Windows user. Reviewer @mook-as raised the same concern at discussion_r3096484641, which is unresolved.

	"encoding/hex"
	"encoding/json"
	"errors"
	"os"
	"path/filepath"
	"strings"
	"time"

	dockerclient "github.com/moby/moby/client"
)

Fix (smallest): gate manageDockerContext and removeDockerContext on runtime.GOOS != "windows" until the named-pipe path is wired up. Long-term: switch to github.com/docker/cli/cli/context/store, which constructs the platform-appropriate Host (see Design Concerns).

 func (r *EngineReconciler) manageDockerContext(socketPath string) {
+    if runtime.GOOS == "windows" {
+        return
+    }
     contextName := instance.Name()
     ...
 }

Apply the same guard to removeDockerContext.

		if !changed {
			return nil
		}
		return r.Status().Update(ctx, latest)
	})
}

// cleanupMirrorResources removes every mirror resource the engine
// controller owns. Errors from all four kinds are joined so one stuck
// type does not block the rest.
func (r *EngineReconciler) cleanupMirrorResources(ctx context.Context) error {

Every other error site in this package uses log := logf.FromContext(ctx); log.Error(err, msg, kv...). The new code deviates on two axes:

1. .Info logs at info severity with an "error" field, so operators filtering on error level miss these and structured-log parsers do not promote err into a dedicated field.
2. logf.Log is the unscoped global logger. SetupWithManager primes r.watcherCtx with logf.IntoContext(..., mgr.GetLogger().WithName("engine-watcher")), but the new code ignores it — these lines have no controller name and no goroutine identity.

Fix: capture a scoped logger at goroutine launch and use .Error:

 func (r *EngineReconciler) manageDockerContext(socketPath string) {
     contextName := instance.Name()
+    log := logf.FromContext(r.watcherCtx).WithName("docker-context")

     if err := createReplaceDockerContext(contextName, socketPath); err != nil {
-        logf.Log.Info("Failed to create Docker context", "context", contextName, "error", err)
+        log.Error(err, "Failed to create Docker context", "context", contextName)
         return
     }
     ...
 }

Apply the same pattern to removeDockerContext.