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

Fixes #37661 - block users properly from pushing content #40

Merged
merged 1 commit into from
Jul 26, 2024

Conversation

ianballou
Copy link
Member

@ianballou ianballou commented Jul 18, 2024

Stops users from pushing content:

[root@centos9-proxy-devel smart_proxy_container_gateway-3.0.0]# podman  push bef568b8a706 `hostname`/default_organization-precipitation-container_push_view-buttermilk_biscuits-arianna3
Getting image source signatures
Copying blob cd55268b264d [--------------------------------------] 8.0b / 15.6MiB | 1.9 MiB/s
Copying blob d4fc045c9e3a [--------------------------------------] 8.0b / 7.3MiB | 1.8 MiB/s
Copying blob 5f70bf18a086 [--------------------------------------] 8.0b / 1.0KiB | 606.3 KiB/s
WARN[0001] Failed, retrying in 1s ... (1/3). Error: writing blob: initiating layer upload to /v2/default_organization-precipitation-container_push_view-buttermilk_biscuits-arianna3/blobs/uploads/ in centos9-proxy-devel.manicotto.example.com: unsupported: Pushing content is unsupported 
...

Currently blocked by pulp/pulp_container#1703, so to test in the meantime either the content pushing needs to already exist in Pulp (not trigger the 500) or the container gateway needs a hack to throw a 404 always on blob checking.

Also, ensure the repo exists in Pulp first. We must throw 401 errors for trying to push to nonexistent repos. Muddies the experience a bit, but that's just how podman works.

Ensure that the latest pulp-container 2.20 is being used.
Test that all of the endpoints still work properly since general content access was changed.
Test that container push is blocked with a proper message in all cases.

TODO:

  • Write tests

@ianballou ianballou force-pushed the 37661-block-push branch 2 times, most recently from cb5f801 to 0b11baf Compare July 18, 2024 18:08
@ianballou ianballou marked this pull request as ready for review July 18, 2024 19:36
@ianballou ianballou force-pushed the 37661-block-push branch 4 times, most recently from 071c9e9 to d565704 Compare July 24, 2024 15:01
@ianballou
Copy link
Member Author

Also, ensure the repo exists in Pulp first. We must throw 401 errors for trying to push to nonexistent repos. Muddies the experience a bit, but that's just how podman works.

After chatting with @qcjames53 , we realized that this is wrong. A 404 should be thrown instead of a 401, both for a better user experience and to follow the OCI registry spec. If that's the case, a user pushing content to a repo that doesn't exist should show an error saying the content doesn't exist rather than it saying .

Onc the 404 is hit, podman should try pushing content, which will trigger the unsupported error.

Copy link
Contributor

@qcjames53 qcjames53 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the write-up, Ian. I can confirm that while using podman, unauthenticated users, users pushing to a missing container address, and users pushing to an address they are not authorized to read all 404 on blob lookup. These 404s are ignored by podman and the subsequent blob upload returns the new 'Pushing content is unsupported' error.

These 404s now follow OCI spec as well, which is a nice upgrade.

The source and test cases all look great to me. Thanks for humoring all the back-and-forth needed with this one. ACK'ing :)

@ianballou ianballou merged commit d74a50f into Katello:main Jul 26, 2024
5 checks passed
@ianballou ianballou deleted the 37661-block-push branch July 26, 2024 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants