Skip to content

Commit 8d7851a

Browse files
JAORMXclaude
andcommitted
Report concrete runtime type as ownership identity
Client.Name() returned the static RuntimeName constant ("docker"), which the factory uses to register all Docker-API-compatible runtimes under one entry. Using it as the per-workload ownership identity collapsed Docker, Podman, and Colima into "docker", so a workload created under one could be claimed by another during status reconciliation -- the exact cross-runtime corruption this PR set out to prevent, still reachable between Docker and Podman. Return the concrete runtimeType detected at connection time so each runtime identifies itself distinctly. The stamp written at create time and the value compared at reconcile time both come from this method on the same runtime, so the ownership round-trip holds (a Podman workload stamps "podman" and is later recognized as Podman-owned). Released builds never persisted runtime_name, so all existing workloads take the empty/legacy "assume ours" path and are opportunistically migrated to their true runtime name on the next healthy reconcile -- no regression. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent d30f994 commit 8d7851a

2 files changed

Lines changed: 75 additions & 3 deletions

File tree

pkg/container/docker/client.go

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -651,9 +651,19 @@ func (c *Client) AttachToWorkload(ctx context.Context, workloadName string) (io.
651651
return resp.Conn, stdoutReader, nil
652652
}
653653

654-
// Name returns the registered name of this runtime.
655-
func (*Client) Name() string {
656-
return RuntimeName
654+
// Name returns the name of the concrete runtime this client is connected to
655+
// (e.g. "docker", "podman", "colima"), as detected at connection time.
656+
//
657+
// This intentionally returns the detected runtimeType rather than the static
658+
// RuntimeName constant used for factory registration. RuntimeName collapses all
659+
// Docker-API-compatible runtimes under a single "docker" registration, but for
660+
// workload-ownership tracking each concrete runtime must identify itself
661+
// distinctly: Docker and Podman are separate daemons with separate containers,
662+
// so a workload created under one must not be claimed by the other during
663+
// status reconciliation. Returning the constant would let Podman-owned and
664+
// Docker-owned workloads share the identity "docker" and corrupt each other.
665+
func (c *Client) Name() string {
666+
return string(c.runtimeType)
657667
}
658668

659669
// IsRunning checks the health of the container runtime.
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
package docker
5+
6+
import (
7+
"testing"
8+
9+
"github.com/stretchr/testify/assert"
10+
11+
rt "github.com/stacklok/toolhive/pkg/container/runtime"
12+
)
13+
14+
// TestClientName verifies that Name reports the concrete detected runtime type
15+
// rather than the static factory-registration constant. Distinct identities are
16+
// required so that workload-ownership tracking does not let Docker- and
17+
// Podman-owned workloads (separate daemons) corrupt each other's status.
18+
func TestClientName(t *testing.T) {
19+
t.Parallel()
20+
21+
tests := []struct {
22+
name string
23+
runtimeType rt.Type
24+
want string
25+
}{
26+
{name: "docker", runtimeType: rt.TypeDocker, want: "docker"},
27+
{name: "podman", runtimeType: rt.TypePodman, want: "podman"},
28+
{name: "colima", runtimeType: rt.TypeColima, want: "colima"},
29+
}
30+
31+
for _, tt := range tests {
32+
t.Run(tt.name, func(t *testing.T) {
33+
t.Parallel()
34+
35+
c := &Client{runtimeType: tt.runtimeType}
36+
assert.Equal(t, tt.want, c.Name())
37+
})
38+
}
39+
}
40+
41+
// TestClientNameOwnershipRoundTrip asserts the property the cross-runtime
42+
// protection relies on: the identity stamped at create time and the identity
43+
// compared at reconcile time come from the same Name call, so a workload created
44+
// under Podman is recognized as Podman-owned (and not as Docker-owned) later.
45+
func TestClientNameOwnershipRoundTrip(t *testing.T) {
46+
t.Parallel()
47+
48+
podman := &Client{runtimeType: rt.TypePodman}
49+
docker := &Client{runtimeType: rt.TypeDocker}
50+
51+
// Identity recorded on the RunConfig when the workload is created.
52+
stampedAtCreate := podman.Name()
53+
54+
// Same Podman runtime later reconciling: must recognize ownership.
55+
assert.Equal(t, stampedAtCreate, podman.Name(),
56+
"podman must recognize its own workloads across calls")
57+
58+
// A different runtime (Docker) reconciling the same workload: must NOT
59+
// claim ownership, otherwise it would corrupt the Podman workload's status.
60+
assert.NotEqual(t, stampedAtCreate, docker.Name(),
61+
"docker must not claim ownership of a podman-created workload")
62+
}

0 commit comments

Comments
 (0)