Conversation
Add compute_config, storage_config, and elastic_load_balancing object variables with enabled flags. Adds dynamic blocks to aws_eks_cluster, Auto Mode IAM policies (Compute, BlockStorage, LoadBalancing, Networking), sts:TagSession trust policy, and auto_mode_enabled output. Bumps AWS provider to >= 5.79.0. All defaults preserve current behavior. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
coalesce treats false as empty, causing an error when both Auto Mode is disabled and the user hasn't set bootstrap_self_managed_addons_enabled. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When EKS Auto Mode is enabled, AWS automatically creates an access entry for the node role specified in compute_config. Attempting to create it again via aws_eks_access_entry.linux causes a 409 ResourceInUseException. Filter out the compute_config.node_role_arn from the linux access entries when auto mode is enabled. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The ARN-based filtering caused "count depends on resource attributes" errors because the node_role_arn isn't known at plan time when the IAM role is being created in the same apply. The fix is handled at the component level instead — the component simply does not pass the auto mode node role to access_entries_for_nodes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR adds comprehensive EKS Auto Mode support to the module by introducing new IAM policy attachments, cluster-level configuration blocks, validation checks, and input variables to control compute, storage, and networking capabilities under Auto Mode, along with an output to expose Auto Mode enablement status. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
main.tf (1)
34-39: Consider usingpreconditionfor strict enforcement instead ofcheck.The
checkblock produces a warning but does not prevent the apply. If the intention is to strictly enforce that all three Auto Mode capabilities must be toggled together, apreconditionon theaws_eks_clusterresource would block invalid configurations at plan time.💡 Alternative using precondition for strict enforcement
resource "aws_eks_cluster" "default" { count = local.enabled ? 1 : 0 # ... existing config ... + lifecycle { + precondition { + condition = local.auto_mode_all_enabled || local.auto_mode_all_disabled + error_message = "compute_config.enabled, storage_config.block_storage.enabled, and elastic_load_balancing.enabled must all be true or all be false." + } + # existing ignore_changes ... + }Keep the
checkblock if you prefer advisory warnings, or replace with apreconditionif strict enforcement is required.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@main.tf` around lines 34 - 39, The current check block using local.auto_mode_all_enabled / local.auto_mode_all_disabled only emits a warning; replace or add a precondition on the aws_eks_cluster resource to strictly enforce the same boolean condition at plan time: add a lifecycle precondition that asserts (local.auto_mode_all_enabled || local.auto_mode_all_disabled) with an explanatory error_message referencing the three toggles, and remove or keep the advisory check as desired; locate the aws_eks_cluster resource and add the precondition block there using the same condition and message.variables.tf (1)
209-221: Consider adding validation fornode_role_arnwhen compute is enabled.When
compute_config.enabled = true, AWS requires anode_role_arn. If a user forgets to provide it, the error will come from the AWS API at apply time rather than during validation. Adding a validation block would provide clearer feedback.💡 Suggested validation
variable "compute_config" { description = <<-EOT EKS Auto Mode compute configuration. When enabled, AWS manages node provisioning via managed Karpenter. EOT type = object({ enabled = optional(bool, false) node_pools = optional(set(string), ["general-purpose", "system"]) node_role_arn = optional(string, null) }) default = {} nullable = false + + validation { + condition = !var.compute_config.enabled || var.compute_config.node_role_arn != null + error_message = "node_role_arn is required when compute_config.enabled is true." + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@variables.tf` around lines 209 - 221, The compute_config variable lacks a validation rule to ensure node_role_arn is provided when compute is enabled; add a validation block inside the variable "compute_config" that checks if var.compute_config.enabled is true then var.compute_config.node_role_arn is not null and not an empty string (or use length(...) > 0), and supply a clear error_message like "compute_config.node_role_arn must be set when compute_config.enabled is true"; reference the variable name compute_config and its attributes enabled and node_role_arn when implementing the validation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@main.tf`:
- Around line 34-39: The current check block using local.auto_mode_all_enabled /
local.auto_mode_all_disabled only emits a warning; replace or add a precondition
on the aws_eks_cluster resource to strictly enforce the same boolean condition
at plan time: add a lifecycle precondition that asserts
(local.auto_mode_all_enabled || local.auto_mode_all_disabled) with an
explanatory error_message referencing the three toggles, and remove or keep the
advisory check as desired; locate the aws_eks_cluster resource and add the
precondition block there using the same condition and message.
In `@variables.tf`:
- Around line 209-221: The compute_config variable lacks a validation rule to
ensure node_role_arn is provided when compute is enabled; add a validation block
inside the variable "compute_config" that checks if var.compute_config.enabled
is true then var.compute_config.node_role_arn is not null and not an empty
string (or use length(...) > 0), and supply a clear error_message like
"compute_config.node_role_arn must be set when compute_config.enabled is true";
reference the variable name compute_config and its attributes enabled and
node_role_arn when implementing the validation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fee7cc3d-7bbf-4332-a42a-23185990d86c
📒 Files selected for processing (5)
iam.tfmain.tfoutputs.tfvariables.tfversions.tf
|
/terratest |
Rename compute_config -> auto_mode_compute_config, storage_config -> auto_mode_storage_config, elastic_load_balancing -> auto_mode_elastic_load_balancing for clarity. Also add EKS Auto Mode section to README. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add `capabilities` map variable for independently-enableable managed platform features - Create capabilities.tf with aws_eks_capability resources and auto-created IAM roles per capability - Add capabilities and capability_role_arns outputs - Bump AWS provider to >= 6.25.0 for aws_eks_capability resource - Support ARGOCD configuration (IDC, RBAC, network access) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
/terratest |
Add auto_mode_enabled variable, Auto Mode node IAM role, and pass auto_mode_compute_config/storage_config/elastic_load_balancing to the module. Disable node group when Auto Mode is enabled. Incorporates example patterns from PR #253 using our variable naming. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
/terratest |
…rror OpenTofu/Terraform requires for_each keys to be known at plan time. Changed from map-based for_each to toset of keys derived from var.capabilities, ensuring keys are always static. Resource attributes now reference var.capabilities[each.value] instead of each.value.X. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The for_each on capability IAM resources was failing because role_arn == null is unknown at plan time when the calling module passes a resource ARN. Added create_iam_role boolean (default true) that callers set to false when they provide their own roles, ensuring for_each keys are always deterministic at plan time. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The AWS provider requires the aws_idc block to always be present when configuring an Argo CD capability. Changed from dynamic block (optional) to static block (required) and updated the variable type accordingly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The AWS provider requires aws_idc when argo_cd configuration is rendered, but users may not have an IDC instance set up initially. Changed aws_idc back to optional and only render the argo_cd configuration block when aws_idc is provided. The capability is still created, just without the argo_cd configuration block (can be configured later). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
TFLint flagged enabled_capabilities as unused after switching to key-based sets for for_each. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The AWS API requires configuration.argo_cd.aws_idc for ARGOCD capabilities. Previously, when aws_idc was null, the argo_cd block was skipped but the configuration block still rendered empty, causing an API error. Now: - Skip entire configuration block when aws_idc is not provided - Add validation to give a clear error if aws_idc is missing Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Lock files should not be committed in reusable modules as they constrain consumers' provider versions unnecessarily. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
/terratest |
Port the EKS Auto Mode documentation from README.md back to README.yaml so it persists through readme generation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
/terratest |
…es to auto mode managed add-ons - Add data.aws_partition.current to examples/complete for GovCloud/China partition support instead of hardcoded "arn:aws:" prefixes - Rename "Capabilities" section to "Auto Mode Managed Add-ons" in docs to avoid confusion with EKS Capabilities (Argo CD, ACK, KRO) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
/terratest |
|
/terratest |
|
/terratest |
|
These changes were released in v4.9.0. |
Summary
compute_config,storage_config, andelastic_load_balancingobject variables for fully configurable Auto Mode supportcompute_config,storage_config, andelastic_load_balancingonaws_eks_clustersts:TagSessionto cluster role trust policy when Auto Mode enabledAmazonEKSComputePolicy,AmazonEKSBlockStoragePolicy,AmazonEKSLoadBalancingPolicy,AmazonEKSNetworkingPolicy)auto_mode_enabledoutputbootstrap_self_managed_addons = falsewhen Auto Mode is enabled>= 5.79.0All defaults preserve current behavior -- this is additive and non-breaking.
Test plan
terraform validatepasses (verified locally)terraform planon existing cluster shows no changes (backward compat)aws eks describe-clusterbootstrap_self_managed_addonsis correctly auto-set🤖 Generated with Claude Code