Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 24 additions & 9 deletions pkg/config/settings_v1.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,15 +188,36 @@ func RequireImageRegistry(projectPath, profileName string) error {
" Example: scion config set --global image_registry ghcr.io/myorg")
}

// RewriteImageRegistry replaces the registry prefix of a container image reference
// with newRegistry. Only images whose basename starts with "scion-" are rewritten.
// RewriteImageRegistry rewrites an image reference to use newRegistry.
//
// Detection uses the Docker/OCI convention: if the part before the first "/"
// contains a "." or ":" it is treated as a registry domain and the image is
// considered fully qualified — it is returned unchanged.
//
// Bare names (e.g. "scion-claude:latest", "my-agent:v2") and relative paths
// (e.g. "library/scion-claude:latest") are rewritten so that the basename
// (last path component) is placed under newRegistry.
//
// If newRegistry is empty, the original image is returned unchanged.
func RewriteImageRegistry(fullImage, newRegistry string) string {
if newRegistry == "" || fullImage == "" {
return fullImage
}

// Extract the basename (last path component, e.g. "scion-claude:latest")
// Check whether the image reference is fully qualified (has a registry domain).
// Per Docker/OCI convention, the first path component is a domain if it
// contains a "." (e.g. "ghcr.io", "us-docker.pkg.dev") or a ":"
// (e.g. "localhost:5000").
if firstSlash := strings.Index(fullImage, "/"); firstSlash >= 0 {
firstComponent := fullImage[:firstSlash]
if strings.ContainsAny(firstComponent, ".:") {
// Fully qualified image — keep as-is.
return fullImage
}
}
Comment on lines +211 to +217

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

According to the Docker/OCI reference parsing convention, localhost is explicitly recognized as a registry domain even if it does not contain a . or :. Currently, an image reference like localhost/myimage:latest would not be recognized as fully qualified and would be incorrectly rewritten. Adding an explicit check for localhost ensures full compliance with the Docker/OCI convention.

Suggested change
if firstSlash := strings.Index(fullImage, "/"); firstSlash >= 0 {
firstComponent := fullImage[:firstSlash]
if strings.ContainsAny(firstComponent, ".:") {
// Fully qualified image — keep as-is.
return fullImage
}
}
if firstSlash := strings.Index(fullImage, "/"); firstSlash >= 0 {
firstComponent := fullImage[:firstSlash]
if strings.ContainsAny(firstComponent, ".:") || firstComponent == "localhost" {
// Fully qualified image — keep as-is.
return fullImage
}
}


// Bare name or relative path — rewrite to newRegistry.
// Extract the basename (last path component, e.g. "scion-claude:latest").
lastSlash := strings.LastIndex(fullImage, "/")
var basename string
if lastSlash >= 0 {
Expand All @@ -205,12 +226,6 @@ func RewriteImageRegistry(fullImage, newRegistry string) string {
basename = fullImage
}

// Only rewrite images following the scion naming convention
if !strings.HasPrefix(basename, "scion-") {
return fullImage
}

// Strip trailing slash from registry
registry := strings.TrimRight(newRegistry, "/")
return registry + "/" + basename
}
Expand Down
123 changes: 98 additions & 25 deletions pkg/config/settings_v1_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3230,6 +3230,10 @@ telemetry:
}

// --- RewriteImageRegistry tests ---
//
// Format-based detection: if the first path component contains a "." or ":"
// it is treated as a registry domain → image kept as-is.
// Otherwise (bare name or relative path) → rewrite to newRegistry.

func TestRewriteImageRegistry(t *testing.T) {
tests := []struct {
Expand All @@ -3238,35 +3242,104 @@ func TestRewriteImageRegistry(t *testing.T) {
newRegistry string
want string
}{
// --- Bare scion-* names → rewritten ---
{
name: "rewrite scion image",
fullImage: "us-central1-docker.pkg.dev/example-project/scion-images/scion-claude:latest",
name: "bare scion image with tag",
fullImage: "scion-claude:latest",
newRegistry: "ghcr.io/myorg",
want: "ghcr.io/myorg/scion-claude:latest",
},
{
name: "rewrite with trailing slash",
fullImage: "us-central1-docker.pkg.dev/example-project/scion-images/scion-gemini:latest",
newRegistry: "ghcr.io/myorg/",
want: "ghcr.io/myorg/scion-gemini:latest",
name: "bare scion image no tag",
fullImage: "scion-claude",
newRegistry: "ghcr.io/myorg",
want: "ghcr.io/myorg/scion-claude",
},
{
name: "bare scion image with custom tag",
fullImage: "scion-base:v2",
newRegistry: "docker.io/myuser",
want: "docker.io/myuser/scion-base:v2",
},

// --- Bare non-scion names → rewritten (new behavior) ---
{
name: "bare non-scion image with tag",
fullImage: "my-custom-agent:v2",
newRegistry: "ghcr.io/myorg",
want: "ghcr.io/myorg/my-custom-agent:v2",
},
{
name: "do not rewrite non-scion image",
name: "bare ubuntu image with tag",
fullImage: "ubuntu:22.04",
newRegistry: "ghcr.io/myorg",
want: "ubuntu:22.04",
want: "ghcr.io/myorg/ubuntu:22.04",
},
{
name: "bare image no tag",
fullImage: "my-agent",
newRegistry: "ghcr.io/myorg",
want: "ghcr.io/myorg/my-agent",
},

// --- Relative path (no domain) → rewritten ---
{
name: "relative path library/scion-claude",
fullImage: "library/scion-claude:latest",
newRegistry: "ghcr.io/myorg",
want: "ghcr.io/myorg/scion-claude:latest",
},
{
name: "relative path myorg/myimage",
fullImage: "myorg/myimage:v1",
newRegistry: "ghcr.io/other",
want: "ghcr.io/other/myimage:v1",
},

// --- Fully qualified with registry domain → NOT rewritten ---
{
name: "fully qualified ghcr.io",
fullImage: "ghcr.io/myorg/scion-elixir:latest",
newRegistry: "us-docker.pkg.dev/proj/repo",
want: "ghcr.io/myorg/scion-elixir:latest",
},
{
name: "fully qualified us-docker.pkg.dev",
fullImage: "us-docker.pkg.dev/ptone-misc/scion-alt/scion-elixir:latest",
newRegistry: "ghcr.io/myorg",
want: "us-docker.pkg.dev/ptone-misc/scion-alt/scion-elixir:latest",
},
{
name: "fully qualified docker.io",
fullImage: "docker.io/library/ubuntu:22.04",
newRegistry: "ghcr.io/myorg",
want: "docker.io/library/ubuntu:22.04",
},
{
name: "fully qualified custom registry with port",
fullImage: "localhost:5000/myimage:dev",
newRegistry: "ghcr.io/myorg",
want: "localhost:5000/myimage:dev",
},
Comment on lines +3318 to +3323

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Add a test case to verify that localhost without a port is correctly recognized as a fully qualified registry domain and kept as-is.

Suggested change
{
name: "fully qualified custom registry with port",
fullImage: "localhost:5000/myimage:dev",
newRegistry: "ghcr.io/myorg",
want: "localhost:5000/myimage:dev",
},
{
name: "fully qualified custom registry with port",
fullImage: "localhost:5000/myimage:dev",
newRegistry: "ghcr.io/myorg",
want: "localhost:5000/myimage:dev",
},
{
name: "fully qualified localhost without port",
fullImage: "localhost/myimage:dev",
newRegistry: "ghcr.io/myorg",
want: "localhost/myimage:dev",
},

{
name: "fully qualified custom.registry:5000",
fullImage: "custom.registry:5000/scion-claude:latest",
newRegistry: "ghcr.io/myorg",
want: "custom.registry:5000/scion-claude:latest",
},
{
name: "do not rewrite custom registry image",
name: "fully qualified myregistry.io",
fullImage: "myregistry.io/custom-agent:v1",
newRegistry: "ghcr.io/myorg",
want: "myregistry.io/custom-agent:v1",
},

// --- Edge cases ---
{
name: "empty registry returns original",
fullImage: "us-central1-docker.pkg.dev/example-project/scion-images/scion-claude:latest",
fullImage: "scion-claude:latest",
newRegistry: "",
want: "us-central1-docker.pkg.dev/example-project/scion-images/scion-claude:latest",
want: "scion-claude:latest",
},
{
name: "empty image returns empty",
Expand All @@ -3275,28 +3348,28 @@ func TestRewriteImageRegistry(t *testing.T) {
want: "",
},
{
name: "scion-base image is rewritten",
fullImage: "us-central1-docker.pkg.dev/example-project/scion-images/scion-base:v2",
newRegistry: "docker.io/myuser",
want: "docker.io/myuser/scion-base:v2",
name: "trailing slash on newRegistry is stripped",
fullImage: "scion-gemini:latest",
newRegistry: "ghcr.io/myorg/",
want: "ghcr.io/myorg/scion-gemini:latest",
},
{
name: "preserves tag",
fullImage: "us-central1-docker.pkg.dev/example-project/scion-images/scion-opencode:sha-abc123",
name: "sha256 digest bare name",
fullImage: "scion-claude@sha256:abc123def456",
newRegistry: "ghcr.io/myorg",
want: "ghcr.io/myorg/scion-opencode:sha-abc123",
want: "ghcr.io/myorg/scion-claude@sha256:abc123def456",
},
{
name: "bare image name (no registry prefix)",
fullImage: "scion-claude:latest",
newRegistry: "ghcr.io/myorg",
want: "ghcr.io/myorg/scion-claude:latest",
name: "sha256 digest fully qualified",
fullImage: "ghcr.io/myorg/scion-claude@sha256:abc123def456",
newRegistry: "docker.io/other",
want: "ghcr.io/myorg/scion-claude@sha256:abc123def456",
},
{
name: "bare image name empty registry",
fullImage: "scion-gemini:latest",
name: "fully qualified empty registry returns original",
fullImage: "ghcr.io/myorg/scion-claude:latest",
newRegistry: "",
want: "scion-gemini:latest",
want: "ghcr.io/myorg/scion-claude:latest",
},
}

Expand Down