Skip to content
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

Allow host+subpath as the source registry for registry-override #2135

Closed
lsoica opened this issue Nov 10, 2023 · 2 comments · Fixed by #2306
Closed

Allow host+subpath as the source registry for registry-override #2135

lsoica opened this issue Nov 10, 2023 · 2 comments · Fixed by #2306
Labels
enhancement ✨ New feature or request good first issue 🥇 Good for newcomers
Milestone

Comments

@lsoica
Copy link

lsoica commented Nov 10, 2023

Currently, the --registry-override will respect the subpath when going from a host to a host+subpath but not the other way around.
(so --registry-override docker.io=mydomain.com/subpath works but --registry-override docker.io/subpath=mydomain.com doesn't)

The ask here is to extend the functionality with support for:

  • --registry-override docker.io/subpath=mydomain.com
  • --registry-override docker.io/subpath=mydomain.com/anothersubpat

Additional context

Slack thread https://kubernetes.slack.com/archives/C03B6BJAUJ3/p1699559557724489?thread_ts=1687158762.361229&cid=C03B6BJAUJ3

@lsoica lsoica added the enhancement ✨ New feature or request label Nov 10, 2023
@Racer159 Racer159 added the good first issue 🥇 Good for newcomers label Nov 10, 2023
@Racer159 Racer159 added this to the The Bucket milestone Nov 10, 2023
@waveywaves
Copy link
Contributor

Hi team, I tried reproducing this issue. Was unsuccessful in doing so.
I took the following steps to reproduce this issue.

change pwd to one of the examples

cd examples/docs-games

update the zarf.yaml images spec image to contain docker.io
so that the configuration looks like the following

images:
- docker.io/defenseunicorns/zarf-game:multi-tile-dark

pull, re-tag and push this image to personal docker registry at docker.io/waveywaves/zarf-game:multi-tile-dark

override registry

zarf create package --registry-override docker.io/defenseunicorns=docker.io/waveywaves

the above command runs successfully and so do the ones below

zarf create package --registry-override whatever.io=docker.io/defenseunicorns
zarf package create --registry-override whatever.io/whatevs=docker.io

Would appreciate any direction understanding how to reproduce this issue
As per the slack thread I am expecting to see the following message in the CLI

Falling back to docker for whatever.io/whatevs/repository/image:tag

@Racer159
Copy link
Contributor

@waveywaves the way @lsoica ran it was a bit different, mostly showing how a replacement happened or didn't happen; with just a host the replacement will happen with whatever you put in the = so:

zarf package create examples/dos-games/ --registry-override docker.io=localhost:555/hello

Works as expected, but:

zarf package create examples/dos-games/ --registry-override docker.io/defenseunicorns=localhost:555

Does not work and will proceed as normal (in @lsoica 's case keeping dummy.remap and not using fqdn.ext at all, and in yours ignoring the replacement and continuing to success).

The problem comes down to this part in the code:

if overrideHost, present := i.RegistryOverrides[refInfo.Host]; present {

We only look for refInfo.Host in the override map when we should instead be looking for a string prefix to the entire refInfo.Reference. That function should also not use ImageTransformHostWithoutChecksum since we already have the parsed ref and all the info we need to do the replacement (and need to perform the replacement differently than that function expects.

Instead of doing that if statement it should loop through the keys and values in i.RegistryOverrides, check if the refInfo.Reference begins with an override key and, if it does, replace that override text with the override value and set it back to actualSrc.

waveywaves added a commit to waveywaves/zarf that referenced this issue Feb 18, 2024
…efenseunicorns#2135)

Instead of looking for refInfo.Host in the override map loop through the keys and values in i.RegistryOverrides, check if the refInfo.Reference begins with an override key and, if it does, replace that override text with the override value and set it back to actualSrc.

Do not use ImageTransformHostWithoutChecksum since we already have the parsed ref and all the info we need to do the replacement.

default actualSrc to refInfo.Reference

Signed-off-by: Vibhav Bobade <vibhav.bobde@gmail.com>
waveywaves added a commit to waveywaves/zarf that referenced this issue Feb 20, 2024
…efenseunicorns#2135)

Instead of looking for refInfo.Host in the override map loop through the keys and values in i.RegistryOverrides, check if the refInfo.Reference begins with an override key and, if it does, replace that override text with the override value and set it back to actualSrc.

Do not use ImageTransformHostWithoutChecksum since we already have the parsed ref and all the info we need to do the replacement.

default actualSrc to refInfo.Reference

Signed-off-by: Vibhav Bobade <vibhav.bobde@gmail.com>
waveywaves added a commit to waveywaves/zarf that referenced this issue Feb 20, 2024
…efenseunicorns#2135)

Instead of looking for refInfo.Host in the override map loop through the keys and values in i.RegistryOverrides, check if the refInfo.Reference begins with an override key and, if it does, replace that override text with the override value and set it back to actualSrc.

Do not use ImageTransformHostWithoutChecksum since we already have the parsed ref and all the info we need to do the replacement.

default actualSrc to refInfo.Reference

Signed-off-by: Vibhav Bobade <vibhav.bobde@gmail.com>
waveywaves added a commit to waveywaves/zarf that referenced this issue Feb 21, 2024
…efenseunicorns#2135)

Instead of looking for refInfo.Host in the override map loop through the keys and values in i.RegistryOverrides, check if the refInfo.Reference begins with an override key and, if it does, replace that override text with the override value and set it back to actualSrc.

Do not use ImageTransformHostWithoutChecksum since we already have the parsed ref and all the info we need to do the replacement.

default actualSrc to refInfo.Reference

Signed-off-by: Vibhav Bobade <vibhav.bobde@gmail.com>
waveywaves added a commit to waveywaves/zarf that referenced this issue Feb 21, 2024
…efenseunicorns#2135)

Instead of looking for refInfo.Host in the override map loop through the keys and values in i.RegistryOverrides, check if the refInfo.Reference begins with an override key and, if it does, replace that override text with the override value and set it back to actualSrc.

Do not use ImageTransformHostWithoutChecksum since we already have the parsed ref and all the info we need to do the replacement.

default actualSrc to refInfo.Reference

Signed-off-by: Vibhav Bobade <vibhav.bobde@gmail.com>
waveywaves added a commit to waveywaves/zarf that referenced this issue Feb 22, 2024
…efenseunicorns#2135)

Instead of looking for refInfo.Host in the override map loop through the keys and values in i.RegistryOverrides, check if the refInfo.Reference begins with an override key and, if it does, replace that override text with the override value and set it back to actualSrc.

Do not use ImageTransformHostWithoutChecksum since we already have the parsed ref and all the info we need to do the replacement.

default actualSrc to refInfo.Reference

Signed-off-by: Vibhav Bobade <vibhav.bobde@gmail.com>
waveywaves added a commit to waveywaves/zarf that referenced this issue Feb 22, 2024
…efenseunicorns#2135)

Instead of looking for refInfo.Host in the override map loop through the keys and values in i.RegistryOverrides, check if the refInfo.Reference begins with an override key and, if it does, replace that override text with the override value and set it back to actualSrc.

Do not use ImageTransformHostWithoutChecksum since we already have the parsed ref and all the info we need to do the replacement.

default actualSrc to refInfo.Reference

Signed-off-by: Vibhav Bobade <vibhav.bobde@gmail.com>
Racer159 added a commit that referenced this issue Feb 26, 2024
…2306)

## Description

Instead of looking for refInfo.Host in the override map loop through the
keys and values in i.RegistryOverrides, check if the refInfo.Reference
begins with an override key and, if it does, replace that override text
with the override value and set it back to actualSrc.

Do not use ImageTransformHostWithoutChecksum since we already have the
parsed ref and all the info we need to do the replacement.

## Related Issue

Fixes #2135 

## Type of change

- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Other (security config, docs update, etc)

## Checklist before merging

- [x] Test, docs, adr added or updated as needed
- [x] [Contributor Guide
Steps](https://github.com/defenseunicorns/zarf/blob/main/CONTRIBUTING.md#developer-workflow)
followed

---------

Signed-off-by: Vibhav Bobade <vibhav.bobde@gmail.com>
Co-authored-by: Wayne Starr <Racer159@users.noreply.github.com>
Co-authored-by: Austin Abro <37223396+AustinAbro321@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement ✨ New feature or request good first issue 🥇 Good for newcomers
Projects
No open projects
Status: Closed
Development

Successfully merging a pull request may close this issue.

3 participants