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

oras-py doesn't work with unauthenticated pull #56

Closed
prabhu opened this issue Nov 25, 2022 · 16 comments · Fixed by #57
Closed

oras-py doesn't work with unauthenticated pull #56

prabhu opened this issue Nov 25, 2022 · 16 comments · Fixed by #57

Comments

@prabhu
Copy link

prabhu commented Nov 25, 2022

Works

oras pull ghcr.io/ngcloudsec/vdb:v1 -o /tmp/vdb

Performing the same action via the python library results in an authentication error with get_manifest.

oras-py pull ghcr.io/ngcloudsec/vdb:v1 --output /tmp/vdb1
This endpoint requires a token. Please set oras.provider.Registry.set_basic_auth(username, password) first or use oras-py login to do the same.
authentication required
Issue with https://ghcr.io/v2/ngcloudsec/vdb/manifests/v1:
Unauthorized
@vsoch
Copy link
Contributor

vsoch commented Nov 25, 2022

I am running the command to reproduce your error - and trying with and without my token for GitHub packages in my $HOME/.docker/config.json meaning I just have an "auths" section with ghcr.io

{
	"auths": {
		"ghcr.io": {
			"auth": "xxxxxxxxx........xxxxxxxxxxxxxxxx"
		},

By default with my config in place, it looks like it's working for me:

$ ls /tmp/vdb1/
data.index.vdb  data.vdb

And the full command:

$ oras-py pull ghcr.io/ngcloudsec/vdb:v1 --output /tmp/vdb1
Successfully pulled /tmp/vdb1/data.vdb.
Successfully pulled /tmp/vdb1/data.index.vdb.

I think GitHub packages has had an issue in the past where if you are authenticated, it actually doesn't work when it doesn't need auth (I had this error just using a standard pull).

When I rename the config, I reproduce the same error as you:

$ oras-py pull ghcr.io/ngcloudsec/vdb:v1 --output /tmp/vdb1
This endpoint requires a token. Please set oras.provider.Registry.set_basic_auth(username, password) first or use oras-py login to do the same.
authentication required
Issue with https://ghcr.io/v2/ngcloudsec/vdb/manifests/v1:
Unauthorized

Let me a take a look at how oras in Go does it. The message that we are getting above is correct - the server is upset that we aren't providing something and telling us we are unathorized. I'm just not sure how it works without providing something.

@prabhu
Copy link
Author

prabhu commented Nov 25, 2022

Thanks, @vsoch, for looking into this. Another problem I faced is the use of logger.exit which kills the process, so I'm unable to use this as a library. Any chance you could replace all use of such exit with errors and warnings?

@vsoch
Copy link
Contributor

vsoch commented Nov 25, 2022

Sure could you please open a separate issue for that?

@vsoch
Copy link
Contributor

vsoch commented Nov 25, 2022

Okay pinged oras-go devs to get their feedback here - let's hope we hear back soon! As soon as we talk about the strategy they use I'm happy to implement it here.

@vsoch
Copy link
Contributor

vsoch commented Nov 25, 2022

@prabhu here is a first PR to test adding exceptions - give that a whirl and let me know what you think! #57 We will need to wait for the oras-go maintainers to chime in for this issue.

And don't worry about opening an issue - I just did the PR straight away so we wouldn't need to remember :P The luxury of the Thanksgiving holiday is that you can do that! 😆

@prabhu
Copy link
Author

prabhu commented Nov 25, 2022

Thank you so much! Will give this a try!

@vsoch
Copy link
Contributor

vsoch commented Nov 28, 2022

Hey @prabhu! We found the bug I think - oras-go will generate essentially an empty token for the registry when this happens: https://github.com/oras-project/oras-go/blob/f09c8577c526c2e13172d6321b2bbb57cd7152e6/registry/remote/auth/client.go#L292-L336 and we don't do that here. I will try to make time to work on this soon, likely in the evening after work.

@vsoch
Copy link
Contributor

vsoch commented Nov 29, 2022

hey @prabhu ! Thank you for your patience! The pull request at #57 now addresses both issues:

  • we replace system exits with raising exceptions you can catch
  • the un-authenticated pull should now work given that the registry supports giving out anonymous tokens.

Take it for a test run and let me know if it solves the issues for you.

@vsoch
Copy link
Contributor

vsoch commented Dec 2, 2022

ping @prabhu ! Please test #57 thank you!

@prabhu
Copy link
Author

prabhu commented Dec 2, 2022

@vsoch I cannot test it by checking out this branch due to some issues with relative imports. Could you share some instructions?

@vsoch
Copy link
Contributor

vsoch commented Dec 2, 2022

What issues exactly? you should create a new environment, pull the branch directly, and install to it.

@vsoch
Copy link
Contributor

vsoch commented Dec 2, 2022

cd /tmp
git clone -b dont-sys-exit https://github.com/oras-project/oras-py
cd oras-py
python -m venv env
source env/bin/activate
pip install -e .
which oras-py

@prabhu
Copy link
Author

prabhu commented Dec 2, 2022

@vsoch Thank you. It is working, but the download is significantly slow. Will try later over the weekend.

@vsoch
Copy link
Contributor

vsoch commented Dec 2, 2022

okay gotcha! For the test case here, it was slow for me too. I'm not sure there is any way around that.

@prabhu
Copy link
Author

prabhu commented Dec 2, 2022

On the first test, the python version took 14min 20sec whereas go took 11min 7sec. I will repeat it several times, but this particular issue can be closed. Thank you for the quick fix!

@vsoch
Copy link
Contributor

vsoch commented Dec 2, 2022

Sure thing!

And I'm not surprised about the time difference - we can definitely discuss how to improve upon it, if that's possible. I suspect Go has some more efficient ways to do the requests that we aren't emulating here.

@vsoch vsoch closed this as completed in #57 Dec 2, 2022
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 a pull request may close this issue.

2 participants