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

Use IMDSv2 token when fetching node ip in entrypoint #1727

Closed
wants to merge 1 commit into from

Conversation

chlunde
Copy link

@chlunde chlunde commented Nov 5, 2021

What type of PR is this?
bug

Which issue does this PR fix:
What does this PR do / Why do we need it:

Use IMDSv2/tokens.

This is required in hardened setups (as recommended by the AWS
documentation)

Without the change, the CNI driver did not become ready on the master
branch.

If an issue # is not available please add repro steps and logs from IPAMD/CNI showing the issue:
Testing done on this change:

This has been tested on nodes with the following launch template
options:

MetadataOptions.HttpTokens required
MetadataOptions.HttpPutResponseHopLimit 1

Automation added to e2e:

N/A, I can create an issue for this (switch e2e nodes to IMDSv2)

Will this break upgrades or downgrades. Has updating a running cluster been tested?:

Does this change require updates to the CNI daemonset config files to work?:

Does this PR introduce any user-facing change?:
No, it worked with 1.9.3

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

This is required in hardened setups (as recommended by the AWS
documentation)

This has been tested on nodes with the following launch template
options:

  MetadataOptions.HttpTokens required
  MetadataOptions.HttpPutResponseHopLimit 1

Without the change, the CNI driver did not become ready on the master
branch.
@chlunde chlunde force-pushed the imdsv2-token-entrypoint-nodeip branch from b970048 to 06e9ea4 Compare November 9, 2021 13:01
@jayanthvn
Copy link
Contributor

jayanthvn commented Nov 9, 2021

#1726 will replace the entrypoint script and also fix this issue.

@chlunde
Copy link
Author

chlunde commented Nov 9, 2021

@jayanthvn cool, for 1.10.0 or .1? I can close this issue then. This works with 1.9.3 at the moment.

@jayanthvn
Copy link
Contributor

This will make it to 1.10.1, but do you need it based of 1.9 branch?

@chlunde
Copy link
Author

chlunde commented Nov 9, 2021

@jayanthvn no, I don't need it, but I think it works on 1.9 already? I had not noticed any issue with this on 1.9.3 but when trying out the bottlerocket+SG+stateful set issue I noticed the issue.

Looking at git blame, it seems like this was introduced with #1587

So 1.9.3 works for IMDSv2 customers, release 1.10.0 will not unless fixed.

For us this is not an issue because we're waiting for 1.10.1 anyway, but you might want some kind of fix for 1.10.0 to avoid regressions for customers using IMDSv2 only.

If you want another kind of fix I can close this.

@johngmyers
Copy link

I confirm this fix works cherrypicked to the release-1.10 branch with kOps IPv4.

@jayanthvn
Copy link
Contributor

@chlunde - Yes will look into it. Thanks for bringing it up.

@johngmyers - Thanks for verifying.

@chlunde
Copy link
Author

chlunde commented Nov 10, 2021

@jayanthvn I see 1.10 was already released so I'll close this

@chlunde chlunde closed this Nov 10, 2021
@chlunde chlunde deleted the imdsv2-token-entrypoint-nodeip branch November 10, 2021 20:01
@jayanthvn jayanthvn mentioned this pull request Nov 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants