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

feat!: Replace the use of aws-auth configmap with EKS cluster access entry #2858

Conversation

bryantbiggs
Copy link
Member

@bryantbiggs bryantbiggs commented Dec 19, 2023

Description

List of backwards incompatible changes

  • Minium supported AWS provider version increased to v5.34
  • Minimum supported Terraform version increased to v1.3 to support Terraform state moved blocks as well as other advanced features
  • The resolve_conflicts argument within the cluster_addons configuration has been replaced with resolve_conflicts_on_create and resolve_conflicts_on_delete now that resolve_conflicts is deprecated
  • The default/fallback value for the preserve argument of cluster_addonsis now set to true. This has shown to be useful for users deprovisioning clusters while avoiding the situation where the CNI is deleted too early and causes resources to be left orphaned resulting in conflicts.
  • The Karpenter sub-module's use of the irsa naming convention has been removed, along with an update to the Karpenter controller IAM policy to align with Karpenter's v1beta1/v0.32 changes. Instead of referring to the role as irsa or pod_identity, its simply just an IAM role used by the Karpenter controller and there is support for use with either IRSA and/or Pod Identity (default) at this time
  • The aws-auth ConfigMap resources have been moved to a standalone sub-module. This removes the Kubernetes provider requirement from the main module and allows for the aws-auth ConfigMap to be managed independently of the main module. This sub-module will be removed entirely in the next major release.
  • Support for cluster access management has been added with the default authentication mode set as API_AND_CONFIG_MAP. This is a one way change if applied; if you wish to use CONFIG_MAP, you will need to set authentication_mode = "CONFIG_MAP" explicitly when upgrading.
  • Karpenter EventBridge rule key spot_interrupt updated to correct mis-spelling (was spot_interupt). This will cause the rule to be replaced

Additional changes

Added

  • A module tag has been added to the cluster control plane
  • Support for cluster access entries. The bootstrap_cluster_creator_admin_permissions setting on the control plane has been hardcoded to false since this operation is a one time operation only at cluster creation per the EKS API. Instead, users can enable/disable enable_cluster_creator_admin_permissions at any time to achieve the same functionality. This takes the identity that Terraform is using to make API calls and maps it into a cluster admin via an access entry. For users on existing clusters, you will need to remove the default cluster administrator that was created by EKS prior to the cluster access entry APIs - see the section Removing the default cluster administrator for more details.
  • Support for specifying the CloudWatch log group class (standard or infrequent access)
  • Native support for Windows based managed nodegroups similar to AL2 and Bottlerocket
  • Self-managed nodegroups now support instance_maintenance_policy and have added max_healthy_percentage, scale_in_protected_instances, and standby_instances arguments to the instance_refresh.preferences block

Modified

  • For sts:AssumeRole permissions by services, the use of dynamically looking up the DNS suffix has been replaced with the static value of amazonaws.com. This does not appear to change by partition and instead requires users to set this manually for non-commercial regions.
  • The default value for kms_key_enable_default_policy has changed from false to true to align with the default behavior of the aws_kms_key resource
  • The Karpenter default value for create_instance_profile has changed from true to false to align with the changes in Karpenter v0.32
  • The Karpenter variable create_instance_profile default value has changed from true to false. Starting with Karpenter v0.32.0, Karpenter accepts an IAM role and creates the EC2 instance profile used by the nodes

Removed

  • The complete example has been removed due to its redundancy with the other examples
  • References to the IRSA sub-module in the IAM repository have been removed. Once https://github.com/clowdhaus/terraform-aws-eks-pod-identity has been updated and moved into the organization, the documentation here will be updated to mention the new module.

Motivation and Context

Breaking Changes

  • Absolutely

How Has This Been Tested?

  • I have updated at least one of the examples/* to demonstrate and validate my change(s)
  • I have tested and validated these changes using one or more of the provided examples/* projects
  • I have executed pre-commit run -a on my pull request

main.tf Show resolved Hide resolved
main.tf Outdated Show resolved Hide resolved
modules/karpenter/variables.tf Outdated Show resolved Hide resolved
@jbronn
Copy link

jbronn commented Jan 2, 2024

Thank you @bryantbiggs for the upgrade work! With it, I successfully used the karpenter module here with the latest Karpenter 0.33.1 release. However, I needed to apply policy updates to be in sync with the new v1beta controller policy. In particular, I added region statements, added spot instance support, and fixed typos.

@bryantbiggs
Copy link
Member Author

awesome, thanks @jbronn - theres a few parts we're waiting on for this release, and one of those is aws/karpenter-provider-aws#5195. I'd prefer to follow the policy set by the upstream project instead of crafting something custom here so we'll see what the Karpenter team comes back with

@jbronn
Copy link

jbronn commented Jan 5, 2024

@bryantbiggs I think we're on the same page, as my changes were an attempt to sync with latest 0.33.1 policy rather than some custom one I made myself. In particular, there are significant issues with the current policy here that prevent Karpenter working at all:

  • Typos in AllowScopedResourceTagging and AllowScopedDeletion policy statements: they should be using ResourceTag instead of RequestTag in conditions.
  • The AllowScopedEC2InstanceActionsWithTags has wrong conditions specified.
  • The AllowInterruptionQueueActions and AllowPassingInstanceRole statements should be referring to the SQS queue and role ARNs instead of hard-coding strings that don't match the managed terraform resources.

variables.tf Outdated Show resolved Hide resolved
@bryantbiggs bryantbiggs force-pushed the refactor/cluster-access-mgmt-replace-aws-auth branch from 92579a5 to df833f2 Compare January 24, 2024 22:53
@bryantbiggs bryantbiggs force-pushed the refactor/cluster-access-mgmt-replace-aws-auth branch from df833f2 to 4f3be36 Compare January 26, 2024 18:24
@bryantbiggs bryantbiggs mentioned this pull request Jan 26, 2024
3 tasks
main.tf Show resolved Hide resolved
…esource names to support migrating for users
@bryantbiggs bryantbiggs force-pushed the refactor/cluster-access-mgmt-replace-aws-auth branch 6 times, most recently from 5a8d51c to aa5bbfd Compare January 31, 2024 20:08
@bryantbiggs bryantbiggs changed the title feat: Replace resolve_conflicts with resolve_conflicts_on_create/… feat!: Replace the use of aws-auth configmap with EKS cluster access entry Jan 31, 2024
@bryantbiggs bryantbiggs force-pushed the refactor/cluster-access-mgmt-replace-aws-auth branch from aa5bbfd to 15d8235 Compare January 31, 2024 20:13
README.md Show resolved Hide resolved
Copy link
Member

@antonbabenko antonbabenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, small minor concerns about resource renaming and we are good to go!

docs/UPGRADE-20.0.md Show resolved Hide resolved
docs/UPGRADE-20.0.md Show resolved Hide resolved
examples/karpenter/README.md Outdated Show resolved Hide resolved
examples/karpenter/README.md Show resolved Hide resolved
examples/outposts/README.md Outdated Show resolved Hide resolved
modules/karpenter/main.tf Show resolved Hide resolved
modules/karpenter/migrations.tf Show resolved Hide resolved
modules/karpenter/README.md Outdated Show resolved Hide resolved
bryantbiggs and others added 3 commits February 2, 2024 08:26
Co-authored-by: Anton Babenko <anton@antonbabenko.com>
Co-authored-by: Anton Babenko <anton@antonbabenko.com>
Co-authored-by: Anton Babenko <anton@antonbabenko.com>
@bryantbiggs bryantbiggs merged commit 6b40bdb into terraform-aws-modules:master Feb 2, 2024
18 checks passed
@bryantbiggs bryantbiggs deleted the refactor/cluster-access-mgmt-replace-aws-auth branch February 2, 2024 14:36
antonbabenko pushed a commit that referenced this pull request Feb 2, 2024
## [20.0.0](v19.21.0...v20.0.0) (2024-02-02)

### ⚠ BREAKING CHANGES

* Replace the use of `aws-auth` configmap with EKS cluster access entry (#2858)

### Features

* Replace the use of `aws-auth` configmap with EKS cluster access entry ([#2858](#2858)) ([6b40bdb](6b40bdb))
@antonbabenko
Copy link
Member

This PR is included in version 20.0.0 🎉

@morganrowse
Copy link

A module tag has been added to the cluster control plane

Hi @bryantbiggs any particular reason for this change? I don't see any way to modify the behavior as its a hard set merge. We like to enforce our own tagging standards and this breaks that. PRs welcome?

Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.