Skip to content

Commit 569a266

Browse files
authored
fix(isolation): apply the configured registry to namespaced images (#1759)
## Bug `docker_isolation.registry` is documented as the custom registry for isolation images, and air-gapped installs depend on it (see `isolation_git_image_registry_test.go`). `buildFullImageName` treats any image containing a `/` as already qualified and returns it unchanged. A Docker Hub namespaced image such as `myorg/node-mcp:1` therefore bypasses the configured registry and is pulled from docker.io. The final `registry + "/" + image` branch is unreachable. Docker's own rule is that the first path component is a registry host only if it contains `.` or `:`, or is `localhost`. Otherwise it is a Docker Hub namespace. ## Trigger ```json "docker_isolation": { "enabled": true, "registry": "mirror.corp", "default_images": {"node": "myorg/node-mcp:1"} } ``` A per-server `isolation.image: "myorg/node-mcp:1"` triggers it too. - Expected image: `mirror.corp/myorg/node-mcp:1`. - Actual image: `myorg/node-mcp:1`, which a host without internet access cannot pull. ## Fix Use Docker's host rule (`imageHasRegistryHost`) to decide whether a reference is already qualified. With a custom registry configured, namespaced images get the registry prefix like official images already do. With no registry configured, `org/image` is still returned unchanged, so ordinary installs see no difference. Fully qualified references (`ghcr.io/...`, `localhost:5000/...`, `registry.local/...@sha256:...`) are untouched. ## Test `TestBuildFullImageName_ConfiguredRegistryPrefixesNamespacedImages` is a table test covering the configured-registry and no-registry cases above. ## Commands run - `CGO_ENABLED=0 go test -count=1 ./internal/upstream/core`: ok (includes the existing git-image registry tests) - With the fix reverted (`git stash` of `isolation.go`), the new test fails with `buildFullImageName("myorg/node-mcp:1") = "myorg/node-mcp:1", want "mirror.corp/myorg/node-mcp:1"`. - `gofmt -l` on the edited files: no output
1 parent a5c2b1a commit 569a266

2 files changed

Lines changed: 49 additions & 7 deletions

File tree

‎internal/upstream/core/isolation.go‎

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -472,8 +472,13 @@ func (im *IsolationManager) ResolveDefaults(serverConfig *config.ServerConfig) *
472472

473473
// buildFullImageName constructs the full image name with registry if needed
474474
func (im *IsolationManager) buildFullImageName(image string) string {
475-
// If image already contains a registry (has a slash before the first colon), use as-is
476-
if strings.Contains(image, "/") && strings.Index(image, "/") < strings.Index(image, ":") {
475+
if imageHasRegistryHost(image) {
476+
return image
477+
}
478+
479+
// Without a configured registry, a namespaced Docker Hub reference
480+
// (org/image) is already pullable as written.
481+
if im.globalConfig.Registry == "" && strings.Contains(image, "/") {
477482
return image
478483
}
479484

@@ -483,11 +488,6 @@ func (im *IsolationManager) buildFullImageName(image string) string {
483488
registry = "docker.io"
484489
}
485490

486-
// Don't prepend registry if image already contains one
487-
if strings.Contains(image, "/") {
488-
return image
489-
}
490-
491491
// For official images (no slash), prepend library/
492492
if !strings.Contains(image, "/") {
493493
return fmt.Sprintf("%s/library/%s", registry, image)
@@ -496,6 +496,18 @@ func (im *IsolationManager) buildFullImageName(image string) string {
496496
return fmt.Sprintf("%s/%s", registry, image)
497497
}
498498

499+
// imageHasRegistryHost applies Docker's reference rule: the first path
500+
// component names a registry only if it contains '.' or ':' or is
501+
// "localhost"; otherwise it is a Docker Hub namespace (org/image).
502+
func imageHasRegistryHost(image string) bool {
503+
i := strings.Index(image, "/")
504+
if i < 0 {
505+
return false
506+
}
507+
host := image[:i]
508+
return strings.ContainsAny(host, ".:") || host == "localhost"
509+
}
510+
499511
// BuildDockerArgs constructs Docker run arguments for isolation
500512
func (im *IsolationManager) BuildDockerArgs(serverConfig *config.ServerConfig, runtimeType string) ([]string, error) {
501513
image, err := im.GetDockerImage(serverConfig, runtimeType)
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
package core
2+
3+
import "testing"
4+
5+
func TestBuildFullImageName_ConfiguredRegistryPrefixesNamespacedImages(t *testing.T) {
6+
mirror := registryOnlyManager("mirror.corp", nil)
7+
cases := map[string]string{
8+
"myorg/node-mcp:1": "mirror.corp/myorg/node-mcp:1",
9+
"python:3.11": "mirror.corp/library/python:3.11",
10+
"ghcr.io/org/img:2": "ghcr.io/org/img:2",
11+
"localhost:5000/team/img": "localhost:5000/team/img",
12+
"localhost/team/img": "localhost/team/img",
13+
"registry.local/img@sha256:abc": "registry.local/img@sha256:abc",
14+
}
15+
for in, want := range cases {
16+
if got := mirror.buildFullImageName(in); got != want {
17+
t.Errorf("registry=mirror.corp: buildFullImageName(%q) = %q, want %q", in, got, want)
18+
}
19+
}
20+
21+
plain := registryOnlyManager("", nil)
22+
for in, want := range map[string]string{
23+
"myorg/node-mcp:1": "myorg/node-mcp:1",
24+
"python:3.11": "docker.io/library/python:3.11",
25+
} {
26+
if got := plain.buildFullImageName(in); got != want {
27+
t.Errorf("no registry: buildFullImageName(%q) = %q, want %q", in, got, want)
28+
}
29+
}
30+
}

0 commit comments

Comments
 (0)