-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Support IMDSv2 HttpTokens #63067
Support IMDSv2 HttpTokens #63067
Conversation
Hi there! Welcome to the Salt Community! Thank you for making your first contribution. We have a lengthy process for issues and PRs. Someone from the Core Team will follow up as soon as possible. In the meantime, here’s some information that may help as you continue your Salt journey.
There are lots of ways to get involved in our community. Every month, there are around a dozen opportunities to meet with other contributors and the Salt Core team and collaborate in real time. The best way to keep track is by subscribing to the Salt Community Events Calendar. |
@SteffeyDev can we get some tests and a change log? Thank your for contributing to salt. I see you kept backwards compatibility in mind, thanks for that! |
Sure. As I said in the initial comment, I don't see any existing tests for this file, so I'm not sure the best way to test just this change independently. Can you provide some pointers for that? |
@SteffeyDev you could write some mock tests for |
Ok @cmcmarrow I added some tests, can you please review? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just a clarifying question
I see two or three issues related to support for IMDSv2, and could possibly be closed by this patch. Can anyone testing confirm? |
@Ch3LL I did some testing last week, and believe IMDSv2 support is indeed working in salt 3006. It might be worth following up a bit on the three tickets I linked and seeing if they can be closed now. |
What does this PR do?
This PR allows
salt-cloud
to connect to IMDSv2 using tokens, while still supporting IMDSv1.I'm running salt-cloud on an EC2 instance that requires tokens for IMDS, which caused
salt-cloud
to fail to authenticate with EC2 when usinguse-instance-role-credentials
in the cloud provider. My changes allowsalt-cloud
to authenticate successfully by using IMDSv2 tokens to get thesecurity-credentials
.Merge requirements satisfied?
[NOTICE] Bug fixes or features added to Salt require tests.
Not sure if any documentation changes are needed, as I didn't see anything in the documentation about it not supporting IMDSv2 already. Let me know if any documentation changes are needed.
Commits signed with GPG?
No