Increase IMDSv2 hop limit on control plane nodes #10702
Conversation
Non-hostNetworking fails to talk to the instance metadata otherwise. Breaking e.g CSI controller
k8s-ci-robot
added
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: olemarkus The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
| g.Spec.InstanceMetadata = &api.InstanceMetadataOptions{ | ||
| HTTPPutResponseHopLimit: fi.Int64(2), | ||
| } |
There was a problem hiding this comment.
Should check if HTTPPutResponseHopLimit is already set by user. Probably all this logic has to be updated a bit.
There was a problem hiding this comment.
Also, should be IMDSv2, if I'm not mistaken.
There was a problem hiding this comment.
I only set hop limit if InstanceMetadata is nil, so user should not have set anything. IMDSv2 is used when httpTokens: required and this is set ... somewhere else.
There was a problem hiding this comment.
IMDSv2 meant in title.
Here there is a case when HTTPTokens can be set, but HTTPPutResponseHopLimit won't be set. Would not happen in practice but code should be consistent.
There was a problem hiding this comment.
The goal of this one was just to see if we can get the tests passing. We need to add more logic here soon to get CNI (well cilium anyway) working.
olemarkus
changed the title
|
/lgtm |
k8s-ci-robot
added
the
lgtm
Non-hostNetworking fails to talk to the instance metadata otherwise. Breaking e.g CSI controller
See kubernetes/cloud-provider-aws#169
Note: This should work for kubenet, but other CNIs may need other hop limits. E.g with Cilium, I had to use 3 to get this to work. Will have to investigate a bit more first. Meanwhile this should fix the broken test.