search

singularityhub / sregistry-cli

Singularity Global Client for container management
https://singularityhub.github.io/sregistry-cli/
Mozilla Public License 2.0
14 stars 18 forks source link

sregistry.main.s3: allow connecting anonymously to bucket #229

Closed tschoonj closed 5 years ago

tschoonj commented 5 years ago

Closes #204

vsoch commented 5 years ago

Don't forget to update the CHANGELOG.md and bump the version @tschoonj !

tschoonj commented 5 years ago

Done!

vsoch commented 5 years ago

@fenz does this LGTY (look good to you, just made that up, not sure if it's a thing :) )

tschoonj commented 5 years ago

This is the output you get now when trying to access a bucket anonymously when this is not allowed:

WARNING Accessing the bucket anonymously. Consider defining AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY if access fails.
ERROR Could not create bucket my-new-bucket: An error occurred (AccessDenied) when calling the CreateBucket operation: Anonymous access is forbidden for this operation

This is normal as whenever we don't have access to a bucket or it doesn't exist, we try to create it:

https://github.com/singularityhub/sregistry-cli/blob/ba606abad0454ebb011f394d537b0488cf98267c/sregistry/main/s3/__init__.py#L72-L95

fenz commented 5 years ago

I'm not really linking the "a priori" warning. It would be enough for me to get the error explaining you must set the keys to get access to a private bucket. But I'll try to clarify my idea for the whole init function.

The init looks for the bucket:

if (bucket exists):
    if (bucket is not public )
        you get an error (missing keys)
    else
        that's fine
else (bucket doesn't exist):
    ask if you want to create one (you may have misspelled the name):
        case (yes):
            if (keys exists):
                proceed
            else
                you get an error (missing keys)
        case (no):
            close (name misspelled)

What do you think? If this workflow more or less fine?

vsoch commented 5 years ago

It's different than the other workflows that use a bucket (for example, Google Storage will try to create the bucket if it doesn't exist) but thinking about it, it is intended to be run somewhat interactively and it's reasonable to ask the user if the bucket exists first. I'm good with this workflow :)

tschoonj commented 5 years ago

Apologies for my inactivity lately... I still plan on fixing this, but it may be later this week as things are rather busy at work.

vsoch commented 5 years ago

No worries! Thanks for the update.

tschoonj commented 5 years ago

Done! Tested everything as well...

vsoch commented 5 years ago

If you are all set, I'm good to merge. I think you've well met @fenz example client flow so he would probably approve as well.

tschoonj commented 5 years ago

Yep, I think this is ready for a merge as well

vsoch commented 5 years ago

https://pypi.org/project/sregistry/0.2.24/