Add authentication to utility registry #144
Conversation
|
@jeff-mccoy what's the best way to get Crane to log in now that the registry requires auth? I've been trying a few things out but nothing feels right. Would love to do some pairing on this if you have the time. |
|
Can you clarify the question? Under the hood crane uses the docker login info, with zarf you could do a login and pull without docker installed and it would work. |
|
I looked at running Crane's Is there a way to internally call a Cobra command from within the golang codebase? |
|
@jeff-mccoy The podinfo example's helm chart does not support Alternatively, we'll need to either find a different helm chart or submit a PR to add Thoughts? |
|
agreed, let's just drop it |
|
/test all |
|
/test all |
|
@jeff-mccoy ready for review on this. Went through and tested all of the examples, they all look good. |
…tials # Conflicts: # cli/internal/packager/deploy.go
|
/test all |
|
sorry still looking at this, just really hate how much K8s sucks for image things, including private registries. The mirror config with alternate domains will suck to maintain as well as all the image pull secrets will be a pain. The more I think about this, the more it might make sense to leverage the specified hostname/ip for the registry or use internal routes for appliance mode. This would allow us to not use mirror config at all (very important for when we move out of just k3s) and only have one imagepullsecret config that would could potentially just inject as well as a part of the deployment process without forcing the users to maintain that too. Thoughts? |
|
The more intuitive (and less "magical") we can make it the better. My worry would be that we make it harder to understand as we make it a bit less verbose. It made total sense to me that I'd have to add new secrets to each namespace that got created for each example, and I was totally fine with it. If we got rid of image mirroring we'd want to add a |
|
We're feeling pain because we have a bunch of these examples to deal with, but an end user won't experience as much churn as we are. IMO this way is fine, as long as we are fine with telling the user they have to do ImagePullSecrets when creating their zarf packages because the registry requires auth. |
|
Which I guess brings up a deficiency with this PR, it doesn't update the docs at all. I'd better fix that before @btlghrants notices and gives me a whoopin' |
|
With OCI Distribution you shouldn't need to change anything but the hostname of the image, there wouldn't be any funny transformation stuff happening as the registry would just respect the path anyway. We leverage that concept already when push to the registry. My concern is around how this takes some of the simplicity away just because K8s is dumb about private registries. We don't have to go along with it, just like for TLS we don't have people run a bunch of openssl commands to get a valid cert, we do the work for them. |
|
Gone back and forth on this (and went on a random code tangent to investigate), I do think we can do more later on down the road. But the most important thing right now is that we're not adding direct complexity to the deployer of a zarf package--so I think besides docs this all looks good and satisfies an important missing security piece. Thanks for finally getting this done, I put it off way too long. |
|
Actually I don't think there is anything to add yet documentation-wise. The changes would happen under "Roll Your Own" #132 which hasn't been written yet. @btlghrants do you agree? |
|
/test all |
|
@jeff-mccoy one more approval please so it's ready to merge if @btlghrants agrees with my assessment that there aren't any docs to update right now. |
|
Yeah, as far as I know, the game example is the only one we've done any sort of explicit user documentation for and that has the imagePullSecrets added as part of this PR. I think @RothAndrew right--it'll change how we write "Roll Your Own" #132 but we haven't gotten there yet. Seems fine to merge to me. |
RothAndrew
deleted the
feature/5-utility-registry-requires-credentials
branch
Signed-off-by: Jeff McCoy <[email protected]>
Signed-off-by: Jeff McCoy <[email protected]>
Fixes #5