-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
chore: Case insensitive (ami_type
)
#3532
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
chore: Case insensitive (ami_type
)
#3532
Conversation
ami_type
)ami_type
)
ami_type
)ami_type
)
ami_type
)ami_type
)
|
||
# Map the AMI type to the respective SSM param path | ||
ssm_ami_type_to_ssm_param = { | ||
AL2_x86_64 = "/aws/service/eks/optimized-ami/${local.ssm_kubernetes_version}/amazon-linux-2/recommended/release_version" |
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.
these corrections are valid - but I don't think the change from var.ami_type
to local.ami_type
is valid. could you elaborate on this change?
also, we'll (unfortunately) need to update here as well
terraform-aws-eks/modules/self-managed-node-group/main.tf
Lines 22 to 42 in de2aa10
ami_type_to_ssm_param = { | |
AL2_x86_64 = "/aws/service/eks/optimized-ami/${local.ssm_kubernetes_version}/amazon-linux-2/recommended/image_id" | |
AL2_x86_64_GPU = "/aws/service/eks/optimized-ami/${local.ssm_kubernetes_version}/amazon-linux-2-gpu/recommended/image_id" | |
AL2_ARM_64 = "/aws/service/eks/optimized-ami/${local.ssm_kubernetes_version}/amazon-linux-2-arm64/recommended/image_id" | |
BOTTLEROCKET_ARM_64 = "/aws/service/bottlerocket/aws-k8s-${local.ssm_kubernetes_version}/arm64/latest/image_id" | |
BOTTLEROCKET_x86_64 = "/aws/service/bottlerocket/aws-k8s-${local.ssm_kubernetes_version}/x86_64/latest/image_id" | |
BOTTLEROCKET_ARM_64_FIPS = "/aws/service/bottlerocket/aws-k8s-${local.ssm_kubernetes_version}-fips/arm64/latest/image_id" | |
BOTTLEROCKET_x86_64_FIPS = "/aws/service/bottlerocket/aws-k8s-${local.ssm_kubernetes_version}-fips/x86_64/latest/image_id" | |
BOTTLEROCKET_ARM_64_NVIDIA = "/aws/service/bottlerocket/aws-k8s-${local.ssm_kubernetes_version}-nvidia/arm64/latest/image_id" | |
BOTTLEROCKET_x86_64_NVIDIA = "/aws/service/bottlerocket/aws-k8s-${local.ssm_kubernetes_version}-nvidia/x86_64/latest/image_id" | |
WINDOWS_CORE_2019_x86_64 = "/aws/service/ami-windows-latest/Windows_Server-2019-English-Full-EKS_Optimized-${local.ssm_kubernetes_version}/image_id" | |
WINDOWS_FULL_2019_x86_64 = "/aws/service/ami-windows-latest/Windows_Server-2019-English-Core-EKS_Optimized-${local.ssm_kubernetes_version}/image_id" | |
WINDOWS_CORE_2022_x86_64 = "/aws/service/ami-windows-latest/Windows_Server-2022-English-Full-EKS_Optimized-${local.ssm_kubernetes_version}/image_id" | |
WINDOWS_FULL_2022_x86_64 = "/aws/service/ami-windows-latest/Windows_Server-2022-English-Core-EKS_Optimized-${local.ssm_kubernetes_version}/image_id" | |
AL2023_x86_64_STANDARD = "/aws/service/eks/optimized-ami/${local.ssm_kubernetes_version}/amazon-linux-2023/x86_64/standard/recommended/image_id" | |
AL2023_ARM_64_STANDARD = "/aws/service/eks/optimized-ami/${local.ssm_kubernetes_version}/amazon-linux-2023/arm64/standard/recommended/image_id" | |
AL2023_x86_64_NEURON = "/aws/service/eks/optimized-ami/${local.ssm_kubernetes_version}/amazon-linux-2023/x86_64/neuron/recommended/image_id" | |
AL2023_x86_64_NVIDIA = "/aws/service/eks/optimized-ami/${local.ssm_kubernetes_version}/amazon-linux-2023/x86_64/nvidia/recommended/image_id" | |
AL2023_ARM_64_NVIDIA = "/aws/service/eks/optimized-ami/${local.ssm_kubernetes_version}/amazon-linux-2023/arm64/nvidia/recommended/image_id" | |
} | |
} |
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.
rather surprised this wasn't caught earlier - oy vey
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.
local.ami_type
changed the value of ami_type
variable to uppercase
actually I take all that back - the way it is currently written is correct per https://docs.aws.amazon.com/AWSCloudFormation/latest/TemplateReference/aws-resource-eks-nodegroup.html#cfn-eks-nodegroup-amitype and more specifically https://pkg.go.dev/github.com/aws/aws-sdk-go-v2/service/[email protected]/types#AMITypes which is what the AWS Terraform provider uses So no changes required here |
Description
Make
ami_type
case-insensitive.Motivation and Context
I copied the
ami_type
from the official AWS documentation, https://docs.aws.amazon.com/cdk/api/v2/python/aws_cdk.aws_eks/NodegroupAmiType.html. I didn't realize the X was capitalized, so the index wasn't found.Breaking Changes
No.
How Has This Been Tested?
examples/*
to demonstrate and validate my change(s)examples/*
projectspre-commit run -a
on my pull request