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

Allow users to create signed urls with write permissions #294

Merged
merged 6 commits into from
Dec 3, 2021

Conversation

FabioRosado
Copy link
Contributor

@FabioRosado FabioRosado commented Nov 29, 2021

Fixes #293

This is a draft PR to allow users to generate signed URLs with write permissions. I was a bit unsure on what to name the parameter, I was torn between:

  • permissions
  • is_write
  • allow_write

Initially, I was implementing an ENUM so users could specify their permissions, would that be preferable? I decided against it after giving it some thought since perhaps accepting a bool would make it easier to work with the generate_url method, but I'm happy with either as long as we have some docs for it.

One thing that is missing from this PR is testing, I'm planning to reuse the test_generate_url test that we have and push the content with the URL then check that it's there.

So far I've tested this with GCP and seems to work fine, but I want to do a bit more manual testing as well

@nuwang
Copy link
Contributor

nuwang commented Nov 30, 2021

I was going to suggest writable as an alternative, but this all looks great to me.

@FabioRosado
Copy link
Contributor Author

I like writable! I’ll change it thank you!

@FabioRosado FabioRosado marked this pull request as ready for review December 2, 2021 19:17
@FabioRosado
Copy link
Contributor Author

Finally got time to keep working on this and finished off the implementation of #293, I've manually tested this in GCP and AWS and works as expected.

I don't have an OpenStack account so I can't test it fully and need to get new creds for Azure to test it with Azure but from the three Azure might work fine since its just a permissions thing that we need to pass when generating the URL 😄 )

nuwang
nuwang previously approved these changes Dec 3, 2021
cloudbridge/providers/aws/resources.py Outdated Show resolved Hide resolved
cloudbridge/providers/azure/resources.py Outdated Show resolved Hide resolved
cloudbridge/providers/gcp/resources.py Outdated Show resolved Hide resolved
cloudbridge/providers/openstack/resources.py Outdated Show resolved Hide resolved
tests/test_object_store_service.py Outdated Show resolved Hide resolved
tests/test_object_store_service.py Outdated Show resolved Hide resolved
@nuwang
Copy link
Contributor

nuwang commented Dec 3, 2021

Thanks for making these changes so quickly @FabioRosado. I'll merge this in and prep for release.

@nuwang nuwang merged commit 3edd2c5 into CloudVE:master Dec 3, 2021
@FabioRosado FabioRosado deleted the fr/signed-url branch December 3, 2021 15:15
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.

Should we allow users to specify read/write signed urls?
2 participants