update: adding session tokens for temporary credentials for AWS S3 #6
Conversation
shinitiandrei
changed the title
|
|
entrypoint.sh
Outdated
| # Execute mc mirror | ||
| mc mirror $* "$MIRROR_SOURCE" "$STORAGE_SERVICE_ALIAS/$MIRROR_TARGET" |
There was a problem hiding this comment.
These two lines are duplicated in both paths of the if block. I think you can just extract this out after the if block.
There was a problem hiding this comment.
Sure I'll remove the duplicate
I did this in a bit of a rush and that's why I opened the PR to check on my code.
entrypoint.sh
Outdated
| # Execute mc mirror | ||
| mc mirror $* "$MIRROR_SOURCE" "$STORAGE_SERVICE_ALIAS/$MIRROR_TARGET" | ||
| else | ||
| export MC_HOST_${STORAGE_SERVICE_ALIAS}=https://${ACCESS_KEY_ID}:${SECRET_ACCESS_KEY}:${SESSION_TOKEN}@s3.${REGION}.amazonaws.com |
There was a problem hiding this comment.
This hard codes the SESSION_TOKEN functionality to AWS S3 only. It looks like MinIO server also supports session tokens. Ideally it would be nice to support any S3 compatible backend.
I think there are two options here:
- Support any S3 backend that has session token functionality.
- Only support AWS S3 and make that clear in the action interface.
For option 1, we should use STORAGE_SERVICE_URL instead of hard coding to @s3.${REGION}.amazonaws.com. i.e. Parse the URL and split at https://. Add the key, secret, session and @ symbol.
For option 2, I think we should change the name of SESSION_TOKEN and REGION to AWS_SESSION_TOKEN and AWS_REGION to make it clear they only work for AWS S3.
I'm happy with either option. We can always improve it to cover option 1 in the future if you don't want to try and make it work now.
There was a problem hiding this comment.
I'm happy with option 2 for now, at least we make it visible that it's AWS exclusive.
Option 1 would require a little bit more code and testing, but happy to do it when I get a bit more time.
|
Changes pushed, please let me know what you think. Thank you for the review! :D |
|
Released as Thank you! |
Hi there,
Sorry for opening a PR like this, but it would be nice to get a review on this :).
A bit of a background: my current scenario doesn't allow me to use user keys in AWS. We are using AWS roles (OIDC) and without the session token the authentication doesn't work, so I added it to this PR.
I've built a new image, and it's working like a charm.
Adding Session tokens when using temporary credentials in AWS. README updated and minio/mc image was updated to latest.