Skip to content

WSLC: Refactor ParseImage and ParseRepository (PR Feedback followup) - #41154

Merged
David Bennett (dkbennett) merged 1 commit into
masterfrom
user/dkbennett/imageparserefactor
Jul 24, 2026
Merged

David Bennett (dkbennett) merged 1 commit into
masterfrom
user/dkbennett/imageparserefactor

Conversation

@dkbennett

Copy link
Copy Markdown
Member

Summary of the Pull Request

This is a follow-up from some PR commentary that had a fairly large blast radius to do it cleanly so it is a separate PR.

Replaces the free functions ParseImage, GetCanonicalImageReference, and
NormalizeRepo in wslutil with two composable, immutable value types:
RepositoryReference and ImageReference. A reference is now parsed once
into a single source of truth, and all derived forms (normalized server/path,
canonical strings, tag-or-digest collapsing) are available on demand via
accessors — instead of being recomputed by callers or threaded through helper
functions as loose std::pair/std::string values.

Cleanup is entirely client-side:
no IDL, COM, or service-ABI changes.

PR Checklist

  • Closes: Link to issue #xxx
  • Communication: I've discussed this with core contributors already. If work hasn't been agreed, this work might be rejected
  • Tests: Added/updated if needed and all pass
  • Localization: All end user facing strings can be localized
  • Dev docs: Added/updated if needed
  • Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx

Detailed Description of the Pull Request / Additional comments

New types

RepositoryReference — an immutable container-repository reference.

  • const std::string Name — the verbatim repository token as written.
  • const std::string Server / const std::string Path — the normalized
    registry server and path (Docker client-side normalization,
    e.g. ubuntu → {docker.io, library/ubuntu}).
  • static Parse(repository) — splits and normalizes (folds in the old
    NormalizeRepo).
  • GetCanonical() — the fully-qualified server/path form.

ImageReference — an immutable image reference such as
ubuntu:22.04@sha256:....

  • const RepositoryReference Repository — composed, not a raw string.
  • const std::optional<std::string> Tag / Digest — kept as distinct fields.
  • const EnumReferenceFormat Format — None / Tag / Digest classification.
  • static Parse(input) — throws E_INVALIDARG (with a user-facing message)
    on a malformed reference.
  • TagOrDigest() — collapses to a single field, digest taking precedence.
  • GetCanonical() — the canonical string matching docker pull output
    (keeps both a tag and a digest when both are present).

Implementation details

  • Composition / single source of truth. ImageReference embeds a
    RepositoryReference; every part is const, so a reference is fixed once
    created. Consumers that need repository parts use accessors
    (.Repository.Server, .Repository.Name, .Repository.GetCanonical())
    rather than re-parsing.
  • Why Name is retained. Normalization is lossy — ubuntu,
    docker.io/ubuntu, and index.docker.io/library/ubuntu all normalize to the
    same server/path. Consumers that must echo the repository exactly as written
    (wslc image list display, wslc tag's C-API Repo) read Name.
  • Removed a redundant parse. EnforceRegistryAllowlist now takes a
    const RepositoryReference& instead of a raw string, eliminating a second
    normalization/regex pass on the pull/push paths.
  • Quiet-pull tidy-up. wslc image pull --quiet now only constructs the
    progress callback when it is actually needed
    (std::optional<ImageProgressCallback>).

Validation Steps Performed

  • Debug build clean (no errors).
  • Unit tests pass: WSLCTests::ImageParsing, RepoParsing,
    CanonicalImageReference (converted/expanded to cover the new types,
    including Name/Server/Path, TagOrDigest(), Format, and
    GetCanonical()).
  • E2E image tests pass: 87 passed, 0 failed, 3 skipped (the skips are
    intentional — stdin/terminal/build cases). Includes
    WSLCE2E_Image_Pull_QuietOption and WSLCE2E_Image_Pull_NameOnlyDefaultsTag,
    which exercise the quiet-callback and default-tag paths.
Copilot AI review requested due to automatic review settings July 23, 2026 21:33

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Refactors container image/repository parsing in the WSLC client-side code by replacing the legacy free functions with immutable parsed reference types, reducing repeated parsing and making derived forms (canonical, normalized server/path, tag vs digest) available via accessors.

Changes:

  • Introduces wslutil::RepositoryReference and wslutil::ImageReference (parse-once, immutable) and migrates callers off ParseImage/NormalizeRepo/GetCanonicalImageReference.
  • Updates allowlist enforcement to accept a parsed RepositoryReference to avoid redundant parsing on pull/push paths.
  • Tightens CLI pull behavior by only constructing the progress callback when not in --quiet, and uses the parsed reference for canonical output.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
test/windows/WSLCTests.cpp Updates/expands unit tests to validate the new parsed reference types (Name/Server/Path, Tag/Digest, TagOrDigest, Format, canonical).
src/windows/wslcsession/WSLCSession.cpp Switches pull/push/import/tag-related codepaths to ImageReference/RepositoryReference and updates allowlist enforcement signature.
src/windows/wslcsession/DockerHTTPClient.cpp Uses RepositoryReference::Parse(...).GetCanonical() when building Docker pull URL parameters.
src/windows/wslc/tasks/ImageTasks.cpp Parses once for default-tag messaging + canonical output; avoids constructing progress callback in --quiet.
src/windows/wslc/services/ImageService.cpp Uses ImageReference for server extraction, list parsing, and tag validation.
src/windows/common/wslutil.h Removes old free-function APIs and adds the new RepositoryReference / ImageReference public types.
src/windows/common/wslutil.cpp Implements the new Parse()/GetCanonical() methods and refactors the old logic into the new types.
Comment thread src/windows/common/wslutil.cpp
@dkbennett
David Bennett (dkbennett) marked this pull request as ready for review July 23, 2026 22:20
@dkbennett
David Bennett (dkbennett) merged commit acb5fdc into master Jul 24, 2026
12 checks passed
@dkbennett
David Bennett (dkbennett) deleted the user/dkbennett/imageparserefactor branch July 24, 2026 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants