cmd-buildupload: Add flag for s3 server selection

[wranders]

Added --endpoint-url as an s3 sub-command flag under the buildupload command.

$ cosa buildupload --artifact=ostree s3 --endpointurl=http://s3host.internal builds/

This allows buildupload to use non-AWS storage servers like Minio. Useful for local development or environments where AWS S3 doesn't fit.

Tested with local external Minio server and credentials loaded automatically by boto3 client.

[openshift-ci]

Hi @wranders. Thanks for your PR.

I'm waiting for a coreos member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[dustymabe]

It's a bit ridiculous to me that boto/boto3#2746 hasn't merged yet.

[dustymabe]

/ok-to-test

[dustymabe]

LGTM, but also tagging in @jlebon for a lookover.

It seems like in a future iteration of this we could just create the boto client (s3 = boto3.client()..) at a higher level and pass down that object rather than all of the options.

[wranders]

It's a bit ridiculous to me that boto/boto3#2746 hasn't merged yet.

My thoughts exactly. But after all this time, this seems to be the route AWS wants to go.

It seems like in a future iteration of this we could just create the boto client (s3 = boto3.client()..) at a higher level and pass down that object rather than all of the options.

I could get on that if you'd like.

[dustymabe]

It's a bit ridiculous to me that boto/boto3#2746 hasn't merged yet.

My thoughts exactly. But after all this time, this seems to be the route AWS wants to go.

☹️

It seems like in a future iteration of this we could just create the boto client (s3 = boto3.client()..) at a higher level and pass down that object rather than all of the options.

I could get on that if you'd like.

Let's see what @jlebon's opinion is before you do any work. Thanks for offering.

[jlebon]

Thanks for the patch! Some minor comments but makes sense to me overall. Also good with the client rework mentioned above, though maybe let's do that in a separate PR instead?

Could you squash your indentation fix commits into the first commit?

For the commit title, s/Added/add/. And can you add what you wrote in the first PR comment as part of the commit body?

[wranders]

Changes made and squashed. Should be good to go now.

[dustymabe]

LGTM

[jlebon]

Looks like the commit title lost the cmd-buildupload: prefix. Maybe cmd-buildupload: add --endpoint-url S3 flag ?

Otherwise, LGTM!

[wranders]

Looks like the commit title lost the cmd-buildupload: prefix. Maybe cmd-buildupload: add --endpoint-url S3 flag ?

🤦‍♂️ Let's try this again... lol

[jlebon]

Thanks!