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

default http-put-response-hop-limit to 1 for al2023 #1614

Closed
wants to merge 2 commits into from

Conversation

Issacwww
Copy link
Member

@Issacwww Issacwww commented Feb 2, 2024

Issue #, if available:

Description of changes:
this option is default to 2, per this doc.
And suggested by AppSec, since we supported pod identity, we should default it to 1

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Testing Done
Build 1.28 AMI and tested with kubetest2 and conformance

See this guide for recommended testing for PRs. Some tests may not apply. Completing tests and providing additional validation steps are not required, but it is recommended and may reduce review time and time to merge.

@Issacwww Issacwww self-assigned this Feb 2, 2024
@cartermckinnon
Copy link
Member

cartermckinnon commented Feb 2, 2024

Do the metadata_options apply to Packer's builder instance, to the resulting AMI, or both?

My understanding is the http_tokens bit only applied to the builder instance. What does this hop limit apply to?

@Issacwww
Copy link
Member Author

Issacwww commented Feb 2, 2024

Do the metadata_options apply to Packer's builder instance, to the resulting AMI, or both?
Ah, you are right, this is for packer builder instance, we still need config the resulting AMI

@Issacwww
Copy link
Member Author

Issacwww commented Feb 3, 2024

Discard this change as the IMDS Hop should be set when launch ec2 instance, thus need change this config in our launch template. This should be set for MNG, for self MNG cx should set it to 1 in their own LT

@Issacwww Issacwww closed this Feb 3, 2024
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.

2 participants