Skip to content

Comments

Implement configurable default namespace for model references#3

Draft
Copilot wants to merge 2 commits intomainfrom
copilot/fix-541f5898-6205-49b8-9a4b-442a841cfdb0
Draft

Implement configurable default namespace for model references#3
Copilot wants to merge 2 commits intomainfrom
copilot/fix-541f5898-6205-49b8-9a4b-442a841cfdb0

Conversation

Copy link

Copilot AI commented Sep 4, 2025

This PR implements configurable default namespace support for container registry references in the model-distribution system, addressing the issue described in docker#126.

Problem

Currently, the system uses github.com/google/go-containerregistry/pkg/name package directly, which defaults to Docker Hub (index.docker.io) for references without explicit registry hostnames. The upstream library doesn't provide a configurable default namespace option, making it difficult to use alternative default registries for model distribution.

Solution

This PR introduces a new internal/naming package that wraps the upstream parsing functions with configurable default namespace support. The implementation:

  1. Preserves backward compatibility - existing code continues to work unchanged
  2. Provides flexible configuration - supports client-specific, global, and CLI configuration
  3. Uses intelligent registry detection - distinguishes between tags (mymodel:latest) and registry ports (localhost:5000/model)

Key Changes

New Naming Package

  • Created DefaultNamespace struct with configurable registry
  • Implemented ParseReference() and ParseTag() wrapper methods with smart registry detection
  • Added global configuration functions and comprehensive test coverage

Registry Client Enhancement

// New option for configurable default namespace
client := registry.NewClient(registry.WithDefaultNamespace("registry.example.com"))

Distribution Client Enhancement

// Unified configuration across registry and store operations
client, err := distribution.NewClient(
    distribution.WithStoreRootPath("/path/to/store"),
    distribution.WithDefaultRegistry("registry.example.com"),
)

CLI Tool Enhancement

# New --default-registry flag
mdltool --default-registry registry.example.com pull mymodel:latest
# Resolves to: registry.example.com/mymodel:latest instead of index.docker.io/library/mymodel:latest

Behavior Examples

Input Default Registry Output
mymodel:latest registry.example.com registry.example.com/mymodel:latest
user/mymodel:latest registry.example.com registry.example.com/user/mymodel:latest
other.registry.com/model:latest registry.example.com other.registry.com/model:latest (preserved)
mymodel:latest (none) index.docker.io/library/mymodel:latest (Docker Hub default)

Registry Detection Logic

The package uses heuristics to detect explicit registries:

  • mymodel:latest → No explicit registry (:latest is a tag)
  • registry.com/mymodel:latest → Has explicit registry (contains dot)
  • localhost:5000/mymodel:latest → Has explicit registry (port number)

Integration Points

All reference parsing throughout the codebase now consistently uses the configurable namespace:

  • Registry client operations (Model, BlobURL, BearerToken, NewTarget)
  • Store operations (Tag, UnTag, HasTag, MatchesReference)
  • CLI commands (pull, push, tag, etc.)

Testing

Added comprehensive test coverage including:

  • Unit tests for namespace parsing logic
  • Integration tests for client configuration
  • CLI tests for command-line flag functionality
  • Edge cases for registry detection heuristics

This change provides the exact functionality requested in docker#126 - a way to configure default namespaces without requiring upstream changes to go-containerregistry.


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

Co-authored-by: ericcurtin <1694275+ericcurtin@users.noreply.github.com>
Copilot AI changed the title [WIP] Implement as ekcasey described here: @docker/model-distribution/pull/126 "I think the easiest way to get the behavior we want without causing inconsistencies would be set the default namespace for the parsing library to use or supply a WithDefaultNam... Implement configurable default namespace for model references Sep 4, 2025
Copilot AI requested a review from ericcurtin September 4, 2025 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants