-
Notifications
You must be signed in to change notification settings - Fork 403
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
feat(auth): SigV4 Signing for calls that are taking IAM Auth #2435
base: v2
Are you sure you want to change the base?
Conversation
Work still progressing on this, but interested in early feedback. |
Hi @stephenbawks, thank you so much for your effort in creating this! I wanted to let you know that in order to add a new feature, we have a specific process that requires opening a feature request (possibly with a Request for Comments or RFC)[https://github.com/awslabs/aws-lambda-powertools-python/issues/new?assignees=&labels=RFC%2Ctriage&projects=&template=rfc.yml&title=RFC%3A+TITLE] before we consider implementing a new utility. Additionally, to ensure better visibility, it would be really helpful if you could answer the following questions: 1/ What are the differences or gaps between SigV4Auth and the well-tested utility available at https://github.com/davidmuller/aws-requests-auth? I kindly request you to open a Feature Request/RFC with these answers, so that we can gather feedback on the proposed utility! Once again, I sincerely appreciate your valuable time :) |
Folks - what's the latest here? |
Sorry for the delay but I have gone ahead and created the RFC. |
Thanks a lot @stephenbawks !! Gonna make comments in the RFC shortly |
Quick heads up it's missing details and CI checks still. I'll put this for August |
@heitorlessa / @stephenbawks Coming back to this PR, is there anything else needed before we review and merge? |
Leandro shared that docs and UX need a final pass -- I'm scheduling this for the next iteration cycle starting on Monday (Feb 12th-22nd) to make the cut for that release cycle. |
Quality Gate passedIssues Measures |
Let's do it. I have been waiting to complete my trifecta of PRs so that I can do more with VPC Lattice so I will do whatever we need here. I saw your comment about a RFC so I will get that going. I did create an RFC a while back but I am assuming it needs to be updated. |
@stephenbawks Thanks for commenting. Initially, I'll say this is missing tests for sure (unit/func/e2e) before we can review this properly. |
After a deeper look into the PR & RFC, we're missing a few things like docs and tests. I'm wondering if we care about the JWT side of this PR – perhaps this should be split into a different RFC so we can decide it's something we need. There's definitely a need for SigV4(a) signing util since that would be a nice convenience utility. |
Quality Gate passedIssues Measures |
I am getting back into the swing of things here. I would like to get this PR over the finish line, its been a while in the making. I think we could certainly split it into two different PRs. Just wondering if there is any more thought on that from anyone else. I feel like most of the work is done for adding JWT/client credentials but I am interested in feedback. |
Quality Gate passedIssues Measures |
@rubenfonseca @leandrodamascena |
Quality Gate passedIssues Measures |
Issue number: #2493
Summary
Changes
Adding the ability to create authentication using SigV4.
User experience
Checklist
If your change doesn't seem to apply, please leave them unchecked.
Is this a breaking change?
RFC issue number:
Checklist:
Acknowledgment
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.