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

Enable support for AWS temporary credentials - N8N-3209 #2587

Merged
merged 15 commits into from
Apr 22, 2022

Conversation

BasitAli
Copy link
Contributor

We recently ran into a blocker where we were fetching temporary AWS credentials for uploading a file to S3 but the AWS S3 node in n8n didn't have proper support for it.

This pull request adds the third credential parameter sessionToken which is then passed on to aws4.

@CLAassistant
Copy link

CLAassistant commented Dec 21, 2021

CLA assistant check
All committers have signed the CLA.

@ivov ivov added node/improvement New feature or request community Authored by a community member labels Dec 22, 2021
@BasitAli
Copy link
Contributor Author

Bump

@BasitAli
Copy link
Contributor Author

@ivov Can you help guide what the next step needs to be to get this reviewed?

@ivov
Copy link
Contributor

ivov commented Feb 23, 2022

Thank you very much for your contribution. This is up to @csuermann and @sirdavidoff.

@BasitAli
Copy link
Contributor Author

BasitAli commented Mar 8, 2022

@csuermann, @sirdavidoff. Any feedback for the above PR?

@BasitAli
Copy link
Contributor Author

@janober @mutdmour @Joffcom we've been using n8n through a fork to get this enhancement and it's limiting our ability to keep n8n updated. Any update regarding this would be very helpful. Thanks!

@mutdmour
Copy link
Contributor

Hello. Thanks for highlighting this issue. Will take a look and try to get this into this week's release.

@BasitAli
Copy link
Contributor Author

@mutdmour Thank you!

@mutdmour
Copy link
Contributor

mutdmour commented Apr 1, 2022

update - sorry have not been able to test this yet. @nivb06 is leading initiative to merge in community PRs and I have asked him to prioritise this.

@nivbe06
Copy link
Contributor

nivbe06 commented Apr 1, 2022

@BasitAli, this PR is being worked on as we speak, if there are no issues (🤞), might be released on this or the next week's update.
We are working on moving faster with PRs, so hopefully, you won't be waiting for such a long time again

@BasitAli
Copy link
Contributor Author

BasitAli commented Apr 1, 2022

Thank you for the update @nivb06 @mutdmour.

@michael-radency
Copy link
Contributor

Hello @BasitAli I made some fixes for this PR but can not commit them because access is denied
image

@michael-radency michael-radency self-requested a review April 1, 2022 14:27
@BasitAli
Copy link
Contributor Author

BasitAli commented Apr 1, 2022

It's a private fork. Can you create a separate fork and share the changes with me?

@michael-radency
Copy link
Contributor

michael-radency commented Apr 1, 2022

It's a private fork. Can you create a separate fork and share the changes with me?

sure, please take a look here https://github.com/michael-radency/n8n/tree/aws-credentials-fix

@BasitAli
Copy link
Contributor Author

BasitAli commented Apr 2, 2022

@michael-radency Applied the changes from your fork.

@BasitAli BasitAli requested a review from michael-radency April 6, 2022 04:31
@michael-radency
Copy link
Contributor

Hello @BasitAli , thank you, pass this PR to a final stage review, hopefully it would be merged this week

@janober
Copy link
Member

janober commented Apr 25, 2022

Got released with n8n@0.174.0

@janober janober removed the Upcoming Release Will be part of the upcoming release label Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Authored by a community member node/improvement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants