phenixblue / imageswap-webhook

Image Swap Mutating Admission Webhook for Kubernetes
Apache License 2.0
154 stars 53 forks source link

Imageswap swaps out complete registry and repository path #27

Closed Martin-Idel-SI closed 3 years ago

Martin-Idel-SI commented 3 years ago

What happened:

Deploy an image with definition somepublic.io/repo/subrepo/image:4.0. Using IMAGE_PREFIX=someprivate.io to swap the registry, the resulting image was someprivate.io/image:4.0 (note the missing paths).

What you expected to happen:

I would have expected the resulting image to be someprivate.io/repo/subrepo/image:4.0.

How to reproduce it (as minimally and precisely as possible):

Take any kubernetes resource with an image with more than one slash in the path and look at the replacement.

Anything else we need to know?:

If this is indeed intended behaviour, I think it would be valuable to control it somehwo to allow partial replacement to keep repository structure. This is interesting when having multiple internal registries (e.g. dev + prod).

Environment:

phenixblue commented 3 years ago

I have a coming update that should resolve this, but it's bundled with some other large scale changes, so it may be a little bit before I release it. Keep an eye out for it. I'll leave this issue open until that release is cut.

phenixblue commented 3 years ago

@Martin-Idel-SI I just cut the v1.4.0-prerelease that includes the new swap logic I previously mentioned. Please give it a try and let me know if that solves this problem for you.

We do keep the main "QuickStart" install reference on the README pointing at the latest stable release, so here's the reference for the prerelease:

$ kubectl apply -f https://raw.githubusercontent.com/phenixblue/imageswap-webhook/v1.4.0-prerelease/deploy/install.yaml
Martin-Idel-SI commented 3 years ago

@phenixblue Thanks, I tested it and the problem appears to be solved! Thank you for your work!

One thing I noticed though: Since it takes some time for webhooks to be registered (that's normal, nothing one can do) I have a test-deployment with an image mybogusregistry/pause which should get swapped. It turns out that using a mapfile like default: mytrueregistry.azurecr.io will result in image mytrueregsitry.azurecr.io/mybogusregistry/pause. If I use mybogusregistry.azurecr.io/pause as image, the swapped image will be mytrueregsitry.azurecr.io/pause.

I have a feeling this is intended behaviour because mybogusregistry is simply not a valid registry name so probably gets interpreted as a subrepository, but maybe you could clarify?

phenixblue commented 3 years ago

Thanks for testing and getting back to me.

So, in the example you mentioned, you would expect the image to be swapped to mytrueregsitry.azurecr.io/mybogusregistry.azurecr.io/pause, correct?

Martin-Idel-SI commented 3 years ago

No, mybogusregistry.azurecr.io/pause should be swapped to mytrueregsitry.azurecr.io/pause. I would have expected mybogusregistry/pause to ALSO be swapped to mytrueregsitry.azurecr.io/pause since the first part is "the registry". However I realized that you probably mean to support cases where the registry is simply not added to the original string.

phenixblue commented 3 years ago

Ok, I think I understand what you're saying now.

The registry needs to follow the standard Docker image spec/DNS rules. If a DNS name format is not detected, it assumes the registry is docker.io and that mybogusregistry is the project name and not a registry name.

Since the project portion of the original image is now maintained, you get <registry_from_swap>/<project>/<image>:<tag> and that's where the mytrueregsitry.azurecr.io/mybogusregistry/pause comes from.

If you were to change your example up a bit to be mybogusregistry.com/pause you would get mytrueregistry.azurecr.io/pause as mybogusregistry.com is a proper DNS format and gets recognized as the registry portion of the image definition.

Does that make sense?