-
Notifications
You must be signed in to change notification settings - Fork 319
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 #325
Support IMDSv2 #325
Conversation
I forked @msiuts repo and built own image as a workaround. Would be fine to have this PR merged :) |
@jtblin @ccarlfjord @walkafwalka @mariusv Any chance this can get reviewed and merged? |
If there is an interest I could also update everything once more on this PR, since I am now also running a fork of this. Just let me know. |
Hello! I was wondering what is necessary to get this PR merged, this fix is relevant to my organization's work. I have forked @msiuts repository, and am running this fix in my organization successfully - however we only use it within AWS. |
I second @paranoidd's question. If kube2iam can be made to work with IMDSv2, that is better. Security scanners will flag clusters that have instances allowing IMDSv1, and we need special code to tell AWS to allow IMDSv1 when creating a cluster. I am discussing some of these issues in a blogpost which you can find here |
Please, I'd like to have this PR merged. |
I just removed the merge conflicts and added the latest version of the aws-sdk. |
Any reason why this PR is not merged yet? |
What is the suggested workaround for this problem, since the fix is not merged yet? |
@msiuts that would be great! |
@xmengkinaxis you could build the fork and push a copy to ghcr while waiting for the merge we all need. Forking and using ghcr looks like this.
|
@msiuts thanks for creating this! I am slightly confused by why the "official fix" in 0.10.11 isn't working, perhaps you can shine some light please? I noticed PRs #270 and #279 (merged to 0.10.11) were supposed to fix these issues, but we are still getting 401 errors when we enable IMDSv2. Do you know why these stopped working? Was something changed on the AWS side in the meantime? |
I wouldn't be surprised if those prior changes didn't actually work. |
@vgrudenic As said in the description, this PR here does upgrade to an [AWS SDK which supports and includes another PR which fixes the health check. As you can see in the link v1.8.7 of the Go SDK does not yet support it. I can not remember the exact behaviour at this time without this PR (which is becoming 2 years old in September), maybe just try out an official version and document it here in the thread if this important to you. |
Hi. Can this be merged? |
Would be great to see this get merged. I've been waiting to delete my package for over half a year now: https://github.com/users/protosam/packages/container/package/kube2iam-pr325 |
Can this be merged - hopefully in 2023 :D @riadhnamely |
What this PR does / why we need it:
This PRs makes Kube2IAM compatible with IMDSv2 .
It updates all the dependencies, mainly to upgrade to an aws-sdk which support IMDSv2 and also merges
#304 already, which fixes the health check.
Since alle dependencies are updated, this includes the K8S api.
Which issue this PR fixes
(optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close that issue when PR gets merged)Special notes:
Checklist chart
[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]