Feat/add node pool common

[g-iannelli]

Summary 💡

Currently, there is no built-in support for configuring Instance Metadata Service Version 2 (IMDSv2) for self-managed EKS node pools. IMDSv2 enhances security by enforcing session-based authentication for metadata requests, reducing the risk of metadata theft.

Self-managed EKS node pools should support the configuration of IMDSv2 settings, including:

• Enforcing the use of IMDSv2 (metadata_http_tokens = "required")
• Enabling or disabling the metadata service (metadata_http_endpoint)
• Setting the hop limit for metadata requests (metadata_http_put_response_hop_limit)

Expected Outcome:

• Users should be able to define IMDSv2 settings for self-managed node pools.

Closes:
#352

Relates:

• feat(module,eks): Add workers_group_defaults Variable to Manage AWS IDMS Defaults fury-eks-installer#83

Description 📝

• Added spec.kubernetes.nodePoolCommon with properties for managing IDMS parameters in self-managed EKS node pools.
• Updated the Terraform template to handle the new variables from spec.kubernetes.nodePoolCommon.

Breaking Changes 💔

No breaking changes should be in place

Tests performed 🧪

• Create a cluster without spec.kubernetes.nodePoolsCommon
• Create a cluster setting IDMSv2 with required token

[ralgozino]

Some minor changes before approving.

I'm not sure nodePoolsCommon is a good name for this parameter though. Maybe nodePoolsDefaults is better? but then it could cause some confusion with nodePoolsLaunchKind and nodePoolGlobalAmiType 🤔

Also, I'm not a fan that we are starting to have:

.spec.kubernetes.nodePools
.spec.kubernetes.nodePoolsLaunchKind
.spec.kubernetes.nodePoolGlobalAmiType (notice that is nodePool and not nodePool**s**)
.spec.kubernetes.nodePoolsCommon (this is being added)

This smells like all should be inside .spec.kubernetes.nodePools and maybe launchKind inside common too, but then what is currently under .spec.kubernetes.nodePools should be inside another key that I can't think of right now.

[g-iannelli]

Some minor changes before approving.

I'm not sure nodePoolsCommon is a good name for this parameter though. Maybe nodePoolsDefaults is better? but then it could cause some confusion with nodePoolsLaunchKind and nodePoolGlobalAmiType 🤔

Also, I'm not a fan that we are starting to have:

.spec.kubernetes.nodePools
.spec.kubernetes.nodePoolsLaunchKind
.spec.kubernetes.nodePoolGlobalAmiType (notice that is nodePool and not nodePool**s**)
.spec.kubernetes.nodePoolsCommon (this is being added)

This smells like all should be inside .spec.kubernetes.nodePools and maybe launchKind inside common too, but then what is currently under .spec.kubernetes.nodePools should be inside another key that I can't think of right now.

@ralgozino I don't have a strong preference for the name nodePoolsCommon, so it could be changed to nodePoolsDefaults, even though it only applies to self-managed node pools. In my opinion, nodePoolsLaunchKind could be deprecated since the only valid value is launch_templates, as launch_configurations has been deprecated since 2022. Refactoring the spec.kubernetes.nodePools structure might be necessary, but it would introduce a breaking change in the interface (are we ready to move from ekscluster-kfd-v1alpha2 to ekscluster-kfd-v2alpha1?)

[ralgozino]

@ralgozino I don't have a strong preference for the name nodePoolsCommon, so it could be changed to nodePoolsDefaults, even though it only applies to self-managed node pools.

Mm.. I did pick up that this applied only to self-managed node pools, in that case we should be very clear in the descriptions that this fields apply only to that case.

Are there any other properties that could be added in the future to nodePoolsCommon that apply to eks-managed nodepools or will it always be only for self-managed ones?

In my opinion, nodePoolsLaunchKind could be deprecated since the only valid value is launch_templates, as launch_configurations has been deprecated since 2022. Refactoring the spec.kubernetes.nodePools structure might be necessary, but it would introduce a breaking change in the interface (are we ready to move from ekscluster-kfd-v1alpha2 to ekscluster-kfd-v2alpha1?)

Yes, all of that would be breaking changes that ideally would require bumping the schema version. Let's leave that out of the scope of this PR.

[g-iannelli]

@ralgozino I don't have a strong preference for the name nodePoolsCommon, so it could be changed to nodePoolsDefaults, even though it only applies to self-managed node pools.

Mm.. I did pick up that this applied only to self-managed node pools, in that case we should be very clear in the descriptions that this fields apply only to that case.

Are there any other properties that could be added in the future to nodePoolsCommon that apply to eks-managed nodepools or will it always be only for self-managed ones?

Any keys that are common between https://github.com/terraform-aws-modules/terraform-aws-eks/blob/v17.24.0/locals.tf#L36 and https://github.com/terraform-aws-modules/terraform-aws-eks/blob/v17.24.0/modules/node_groups/locals.tf#L3

[stefanoghinelli]

Hi @g-iannelli, Ci is failing, could you please check it?