-
Notifications
You must be signed in to change notification settings - Fork 40
fix: LaunchTemplates to set InstanceMetadataTags as "disabled" #503
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
fix: LaunchTemplates to set InstanceMetadataTags as "disabled" #503
Conversation
Signed-off-by: Kevin Downey <[email protected]>
Signed-off-by: Kevin Downey <[email protected]>
Signed-off-by: Kevin Downey <[email protected]>
Signed-off-by: Kevin Downey <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #503 +/- ##
==========================================
+ Coverage 46.57% 46.66% +0.08%
==========================================
Files 40 40
Lines 7094 7096 +2
==========================================
+ Hits 3304 3311 +7
+ Misses 3632 3627 -5
Partials 158 158 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| properties: | ||
| configuration: | ||
| properties: | ||
| nodeConfig: |
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.
why are we removing nodeConfig
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.
This is being removed by generate, make manifests. I asked @estela-ramirez and she confirmed we don't need this.
| HttpEndpoint: aws.String(input.HttpEndpoint), | ||
| HttpPutResponseHopLimit: aws.Int64(input.HttpPutHopLimit), | ||
| HttpTokens: aws.String(input.HttpTokens), | ||
| InstanceMetadataTags: aws.String("disabled"), // set disabled by default |
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.
do we need update CRD in instancemgr.keikoproj.io_instancegroups.yaml
metadataOptions:
properties:
httpEndpoint:
type: string
httpPutHopLimit:
format: int64
type: integer
httpTokens:
type: string
type: object
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.
No, we do not expose this for configuration in IG spec (input), it's hard-coded as "disabled" only
What type of PR is this?
Description
Fixes issue where IGs using LaunchTemplates may inherit
InstanceMetadataTags="enabled"property as an account setting but it won't work for Kubernetes Nodes due to a restriction of using "/" characters which conflicts withkubernetes.io/cluster/<clustername>tagging.This change will always set
InstanceMetadataTagsas "disabled"Related issue(s)
High-level overview of changes
Testing performed
Checklist
make testlocally and all tests passgit commit -sfor DCO verificationAdditional information