Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,4 @@ override.tf.json
# Ignore CLI configuration files
.terraformrc
terraform.rc
*.local.md
1 change: 1 addition & 0 deletions main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,7 @@ module "comet_eks" {

# EKS Auto Mode (mutually exclusive with Karpenter)
enable_auto_mode = var.eks_enable_auto_mode
disable_auto_mode = var.eks_disable_auto_mode
auto_mode_node_pools = var.eks_auto_mode_node_pools

# Karpenter prerequisites
Expand Down
25 changes: 17 additions & 8 deletions modules/comet_eks/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -244,14 +244,15 @@ module "eks" {

# EKS Auto Mode. When enabled, the control plane provisions nodes via the
# built-in node pools and the upstream module auto-creates/wires the Auto Mode
# node IAM role (so node_role_arn is intentionally omitted). The block is
# always sent — enabled = false explicitly disables Auto Mode so a cluster that
# previously had it on can be turned back off (a bare null would omit the
# argument and leave the last-applied config in place).
compute_config = {
enabled = var.enable_auto_mode
node_pools = var.enable_auto_mode ? var.auto_mode_node_pools : []
}
# node IAM role (so node_role_arn is intentionally omitted).
#
# null, not { enabled = false } — EKS rejects an explicit disable on a cluster
# that never had Auto Mode. To turn it off on one that does, see
# disable_auto_mode.
compute_config = var.enable_auto_mode ? {
enabled = !var.disable_auto_mode
node_pools = var.disable_auto_mode ? [] : var.auto_mode_node_pools
} : null
Comment thread
GuySaar8 marked this conversation as resolved.
Comment on lines +252 to +255

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Contradictory flags silently strand Auto Mode nodes

The eks_disable_auto_mode=true path reaches the ternary with enable_auto_mode=false and disable_auto_mode=true, which returns null instead of an explicit disable, so planning can remove Auto Mode protections while Auto Mode remains active — should we reject this contradictory combination or handle it explicitly before planning?

Severity

Want Baz to fix this for you? Activate Fixer

Other fix methods

Fix in Cursor

Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
modules/comet_eks/main.tf around lines 252-255, fix the `compute_config` logic and its
inputs so `disable_auto_mode` cannot be true when `enable_auto_mode` is false. Add
validation or a plan-time precondition that rejects this contradictory combination
before planning, rather than allowing the ternary to return null and disabling the Auto
Mode-dependent security protections. Preserve the documented requirement that disabling
Auto Mode requires it to be explicitly enabled.


# Bake the Karpenter discovery tag directly into the node SG so it is never
# dropped when Terraform modifies the security group during subsequent applies.
Expand Down Expand Up @@ -312,6 +313,10 @@ module "eks" {
} : {},
{
configuration_values = local.coredns_config
# OVERWRITE so the native add-on adopts objects the old eks_blueprints_addons
# Helm release owns, instead of failing on ConfigurationConflict. DND-1573.
resolve_conflicts_on_create = "OVERWRITE"
resolve_conflicts_on_update = "OVERWRITE"
Comment thread
GuySaar8 marked this conversation as resolved.
}
)
} : {},
Expand All @@ -328,6 +333,10 @@ module "eks" {
role_arn = aws_iam_role.external_dns[0].arn
service_account = "external-dns"
}]
# As cert-manager above. NOTE: resolve_conflicts does not cover Pod Identity
# associations — a pre-existing external-dns one must be deleted first.
resolve_conflicts_on_create = "OVERWRITE"
resolve_conflicts_on_update = "OVERWRITE"
Comment on lines +338 to +339

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pre-existing association blocks addon migration

OVERWRITE does not resolve ownership conflicts for the external-dns pod_identity_associations, so EKS rejects addon creation when a brownfield association exists and leaves the transition incomplete — should we delete the existing association before recreating it through the addon operation?

Severity web_search

Want Baz to fix this for you? Activate Fixer

Other fix methods

Fix in Cursor

Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
`modules/comet_eks/main.tf` around lines 338-339, update the external-dns addon logic
and its `pod_identity_associations` configuration; `resolve_conflicts_on_create/update =
"OVERWRITE"` cannot resolve an already-existing EKS Pod Identity association. Add an
explicit, ordered brownfield migration that detects and deletes the pre-existing
external-dns association before creating the addon association, then lets the addon
recreate and own it, or fail fast with a clear remediation message when safe deletion
cannot be automated. Ensure the solution is idempotent and does not leave the addon
partially configured.

},
var.eks_external_dns_addon_version != null ? {
addon_version = var.eks_external_dns_addon_version
Expand Down
12 changes: 12 additions & 0 deletions modules/comet_eks/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -725,6 +725,18 @@ variable "enable_karpenter" {
}
}

variable "disable_auto_mode" {
description = <<-EOT
Send an explicit Auto Mode disable while keeping enable_auto_mode = true. One
apply, to turn Auto Mode off on a cluster that has it — the coexistence SG
rules and addon pinning stay in place while AWS drains the nodes. Setting
enable_auto_mode = false instead omits compute_config, which would strip those
rules while the nodes still run. Clear both once the nodes are gone.
EOT
type = bool
default = false
}

variable "enable_auto_mode" {
description = <<-EOT
Enable EKS Auto Mode. When true, the EKS control plane can provision nodes
Expand Down
6 changes: 6 additions & 0 deletions variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,12 @@ variable "eks_enable_auto_mode" {
default = false
}

variable "eks_disable_auto_mode" {
description = "Send an explicit Auto Mode disable while keeping eks_enable_auto_mode = true. One apply, to turn Auto Mode off on a cluster that has it. See disable_auto_mode in modules/comet_eks/variables.tf."
type = bool
default = false
}

variable "eks_auto_mode_node_pools" {
description = "Built-in EKS Auto Mode node pools to enable when eks_enable_auto_mode = true. Common values: \"system\", \"general-purpose\". Custom NodePool/NodeClass CRDs are managed via GitOps (ArgoCD), not this module."
type = list(string)
Expand Down