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

Make storage deletion work on more S3 providers #1600

Merged
merged 1 commit into from Mar 16, 2024

Conversation

wvengen
Copy link
Contributor

@wvengen wvengen commented Mar 15, 2024

I came across this problem and noticed that the access URL is used when deleting files, causing my file deletions to fail on OpenStack SWIFT S3 (relates to #1090). This trivial change makes it work there.

@Shrinks99 Shrinks99 requested a review from ikreymer March 16, 2024 02:02
@ikreymer
Copy link
Member

@wvengen Are you setting an access_endpoint_url in your storages config? This should only make a difference if there is a different access url that is being used. If access_endpoint_url is not set, then it will use the endpoint_url for signing. This may need a closer look.
Can you share what your storage config looks like if possible, mostly the endpoint and access endpoint values?

@ikreymer
Copy link
Member

Ah, i see I guess the _delete_file operation doesn't need to have anything to do with presigned URLs, so makes sense to remove it there. We actually haven't been testing with custom access_endpoint_url paths at all at the moment - perhaps this can all be simplified in the future.

@ikreymer ikreymer merged commit 6278157 into webrecorder:main Mar 16, 2024
2 checks passed
@ikreymer
Copy link
Member

Since this is such a small change, also porting to #1603 so you can use this in the v1.9.4 release.

ikreymer added a commit that referenced this pull request Mar 16, 2024
@wvengen
Copy link
Contributor Author

wvengen commented Mar 16, 2024

Thank you so much!

@wvengen
Copy link
Contributor Author

wvengen commented Mar 16, 2024

In our situation, there is an endpoint_url to use, which only works with authentication (token or signature, for signed URLs). If any public URL would be used without a signature, then the access_endpoint_url would be important, because each user has its own prefix for accessing files (our provider supports the same bucket names for different users). I think URLs without authentication and without a signature are not used, currently, so indeed this distinction would not be needed. But for public archives, I could imagine that one may want to omit a signature? In that case, this distinction would still be relevant (and I'd want a proxy in that case, for cleaner URLs (and some filtering for security), or maybe a CDN).
Just some thoughts on our perspective for the use of a separate access_endpoint_url. Thanks for considering!

@ikreymer
Copy link
Member

Thanks for the quick fix - that slipped through somehow, since we weren't using the access point URLs, perhaps we should add more tests there. This is now in the 1.9.4 release so you should be able to use that.

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.

None yet

2 participants