-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
feat: Add support for specifiying NTP address to use private Amazon Time Sync Service #2125
feat: Add support for specifiying NTP address to use private Amazon Time Sync Service #2125
Conversation
default = ["0.0.0.0/0"] | ||
} | ||
|
||
# TODO - at next breaking change, make fd00:ec2::123/128 the 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.
Good idea to mark such todos
## [18.26.0](v18.25.0...v18.26.0) (2022-06-28) ### Features * Add support for specifiying NTP address to use private Amazon Time Sync Service ([#2125](#2125)) ([4543ab4](4543ab4))
This PR is included in version 18.26.0 🎉 |
@@ -92,6 +92,7 @@ module "eks" { | |||
} | |||
|
|||
# Extend node-to-node security group rules | |||
node_security_group_ntp_ipv4_cidr_block = ["fd00:ec2::123/128"] |
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.
Sorry for my ignorance, but I was following the eks_managed_node_group
example and noticed that this is a configuration for ipv4
, but it seems that this is an ipv6 value. Am I missing something here where this is supposed to be using ["169.254.169.123/32"]
instead of the default ["0.0.0.0/0"]
?
I'm also curious to know if this is necessary, especially since node_security_group_additional_rules.egress_all
is specifying cidr_blocks
to ["0.0.0.0/0"]
and ipv6_cidr_blocks
to ["::/0"]
just a few lines below.
I'm fairly new to terraform and I'm still trying to understand the AWS EKS configuration, so I apologize for any confusion on my part.
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 a typo - my mistake. The variable here should be node_security_group_ntp_ipv6_cidr_block
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.
@bryantbiggs I wasn't sure if the issue here was with the variable name or the value. Thanks for the quick response!
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.
PRs are welcome (to fix my mistakes 😅 ) as well!
The example extends the Security Group NTP IPV4 CIDR block, however, it's meant to update the IPV6 CIDR block
## [18.26.0](terraform-aws-modules/terraform-aws-eks@v18.25.0...v18.26.0) (2022-06-28) ### Features * Add support for specifiying NTP address to use private Amazon Time Sync Service ([terraform-aws-modules#2125](terraform-aws-modules#2125)) ([4543ab4](terraform-aws-modules@4543ab4))
## [18.26.0](terraform-aws-modules/terraform-aws-eks@v18.25.0...v18.26.0) (2022-06-28) ### Features * Add support for specifiying NTP address to use private Amazon Time Sync Service ([#2125](terraform-aws-modules/terraform-aws-eks#2125)) ([2a2e1e8](terraform-aws-modules/terraform-aws-eks@2a2e1e8))
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
Description
Motivation and Context
Breaking Changes
How Has This Been Tested?
examples/*
to demonstrate and validate my change(s)examples/*
projectspre-commit run -a
on my pull request