Auth registry for authenticated pull - #112
Conversation
79f9d79 to
93f3137
Compare
93f3137 to
0fc3c05
Compare
Johan-Liebert1
left a comment
Johan-Liebert1
left a comment
There was a problem hiding this comment.
Overall looks sane
0fc3c05 to
e72f8a4
Compare
|
@HarshwardhanPatil07 thanks for the work, would you mind to add an integration test. Like push an image and try to pull it using the secret registry and authentication. Thanks! |
e72f8a4 to
9ab9335
Compare
|
Thank you very much for the amazing reviews @alicefr @Johan-Liebert1. If you have more questions please do let me know |
|
Yes @alicefr, I will create test in follow up PR to keep it clean |
Please, include it in this PR so we can check if the code is correct |
|
@alicefr Yes |
|
cc @ptalgulk01 |
55f0cd6 to
5cca48c
Compare
|
cc @Johan-Liebert1 @alicefr PTAL |
5cca48c to
748347c
Compare
|
cc @alicefr |
|
@HarshwardhanPatil07 can you also please add some documentation in the README how to use the registry with authentication, and the limits? Like we can only pull from it but not pushing. Additionally, the user and password are still hardcoded, do you plan to address this comment: #112 (comment) |
|
@alicefr hardcoded credentials are now removed. |
fa8d6fc to
a50a9e4
Compare
|
rebase |
|
@HarshwardhanPatil07 I would rather simplify this PR. I think the user should simply create the registry with their credentials. I wouldn't store them. If they forgot the password, they can always restart the registry. This PR adds too much logic for less gain imo. |
a50a9e4 to
3e52a46
Compare
|
Simplified as suggested. Thank you! |
|
@HarshwardhanPatil07 please squash the commits that remove the logic of setting the password with the label and similarly. Otherwise, it becomes quite hard to review the code that is removed in the following up commits and we need to try to preserve a clean git history |
|
@HarshwardhanPatil07 we overall code looks good, some minor stuff, but the git history needs to be fix as I highlighted in the previous comment. Once done, it looks good and it is ready to be merged |
7fad038 to
d0bf8cc
Compare
|
@alicefr sorry for having all the commits and not cleaning the history |
|
One minor comment and this PR requires a rebase, but then it can be merged. Thanks for this work! |
3dfe271 to
0e367fc
Compare
Add a second registry instance, bink-auth-registry, protected by htpasswd basic auth. It shares the storage volume with the existing unauthenticated registry, so images pushed to localhost:5000 are immediately pullable from localhost:5001 with credentials, without duplicating any data. Sharing storage between two registry processes requires both to agree on REGISTRY_HTTP_SECRET, so the existing registry is given the same shared secret. Credentials are supplied by the caller and are never persisted anywhere inspectable: the bcrypt htpasswd entry is generated at start time and no username or password hash is recorded as a container label. Because there is nothing to compare against, an existing authenticated registry is only started, not recreated; to change credentials, stop the registry and start it again. Assisted-by: AI Signed-off-by: HarshwardhanPatil07 <harshpat@redhat.com>
Add a cluster-hosts entry for the authenticated registry and register it as an insecure registry in the VM container runtime configuration, so that pods can pull from it over plain HTTP using imagePullSecrets. Assisted-by: AI Signed-off-by: HarshwardhanPatil07 <harshpat@redhat.com>
Start and stop the authenticated registry alongside the unauthenticated one during the cluster lifecycle, and report both in "bink registry info". Add --registry-user and --registry-password flags to "bink cluster start" and "bink registry start". Credentials must be provided explicitly; there is no default password, and neither the username nor the password is ever printed back to the user. Assisted-by: AI Signed-off-by: HarshwardhanPatil07 <harshpat@redhat.com>
Start an authenticated registry with test-specific credentials, push an image through the shared storage via the unauthenticated registry, verify anonymous access is rejected, and confirm a pod can pull the image using imagePullSecrets. Assisted-by: AI Signed-off-by: HarshwardhanPatil07 <harshpat@redhat.com>
Cached or pre-pulled images currently replace the cluster and DNS images built from the pull request under the same tags. Build checkout-owned images last so integration tests consistently exercise the submitted source on both cache hits and misses. Assisted-by: AI Signed-off-by: HarshwardhanPatil07 <harshpat@redhat.com>
0e367fc to
9d321aa
Compare
|
Thank you! |
Include the auth registry for bink introduced by bootc-dev/bink#112 Signed-off-by: Alice Frosi <afrosi@redhat.com>
Include the auth registry for bink introduced by bootc-dev/bink#112 Signed-off-by: Alice Frosi <afrosi@redhat.com>
3 participants
Adds a second registry instance (
bink-auth-registry) configured with htpasswd authentication. This registry shares the same storage volume as the existing unauthenticated registry, ensuring that images pushed tolocalhost:5000are immediately available onlocalhost:5001with credentials, without any data duplication.Verification steps:
1. Build and start
make build-bink
./bink cluster start --cluster-name test --api-port 0
2. Push an image to the unauthenticated registry
podman pull quay.io/libpod/busybox:latest
podman tag quay.io/libpod/busybox:latest localhost:5000/busybox:test
podman push --tls-verify=false localhost:5000/busybox:test
3. Verify auth works
curl -s http://localhost:5001/v2/_catalog # → 401 (blocked)
curl -s -u testuser:testpassword http://localhost:5001/v2/_catalog # → {"repositories":["busybox"]}
4. Clean up
./bink cluster stop --cluster-name test --remove-data
closes #101