Skip to content

ContainerRegistry: Reject invalid repository names #138

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
May 29, 2025

Conversation

euanh
Copy link
Collaborator

@euanh euanh commented May 29, 2025

Motivation

ImageReference does not check for illegal characters in parsed image references. This means that containertool will send illegal image names to the registry. The registry will reject them, but the error message might not explain why, so a generic error message will be printed. Runtimes reject illegal image references immediately, without sending them to the registry.

Modifications

  • Introduce a Repository wrapper type
  • Check validity when constructing the Repository
  • Change the low-level API functions to accept Repository instead of String.

Result

It is impossible to create a Repository object containing a malformed name, because the constructor checks the string value. It is impossible to send a malformed name to the registry because the API wrappers only accept Repository objects.

Fixes #135

Test Plan

Existing tests continue to pass.
New tests exercise additional checks which were previously missing.

@euanh euanh marked this pull request as draft May 29, 2025 10:50
@euanh euanh force-pushed the 135-registry-name-checks branch from 00338d0 to a685f2e Compare May 29, 2025 11:09
@euanh euanh added kind/bug Something isn't working semver/patch No public API change. labels May 29, 2025
@euanh euanh force-pushed the 135-registry-name-checks branch 2 times, most recently from 989fd23 to 8f19b3b Compare May 29, 2025 11:25
@euanh euanh changed the title ContainerRegistry: Introduce Repository type ContainerRegistry: Reject invalid repository names May 29, 2025
@euanh euanh force-pushed the 135-registry-name-checks branch 3 times, most recently from c8ef624 to 1e30ef9 Compare May 29, 2025 11:40
@euanh euanh marked this pull request as ready for review May 29, 2025 11:54
@euanh euanh force-pushed the 135-registry-name-checks branch 3 times, most recently from a894e29 to e90b537 Compare May 29, 2025 13:52
@euanh euanh force-pushed the 135-registry-name-checks branch from e90b537 to 91e9988 Compare May 29, 2025 13:59
@euanh euanh force-pushed the 135-registry-name-checks branch from 5c067e0 to 4f4b735 Compare May 29, 2025 14:15
@euanh euanh merged commit bf5e256 into apple:main May 29, 2025
23 checks passed
@euanh euanh deleted the 135-registry-name-checks branch May 29, 2025 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

containertool should reject capital letters in repository names
1 participant