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
2 changes: 1 addition & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ go test ./...
- `--aws-vpc-private-subnet-netmask` — per-AZ private subnet netmask (default `23`); pin to the existing value or subnets renumber
- `--aws-vpc-nat-eip-allocation-ids` — pre-allocated Elastic IP allocation IDs for the NAT gateways, one per AZ (repeatable)

On `scopes add`, the managed-VPC equivalents are `--vpc-secondary-cidr` and `--vpc-karpenter-discovery-tag`. Both are `dittocloud` mode only. The discovery tag falls back to the scope's `clusterName`; with neither set the node subnets carry no `karpenter.sh/discovery` tag and Karpenter will not find them.
On `scopes add`, the managed-VPC equivalents are `--vpc-secondary-cidr` and `--vpc-karpenter-discovery-tag`. Both are `dittocloud` mode only. The discovery tag defaults to the VPC ID even when the scope has a `clusterName`, matching the Valet EKS Karpenter selector. An explicit override must also be reflected in that selector.
- `--karpenter-discovery-tag-value` — value for `karpenter.sh/discovery` on the node subnets; defaults to `--cluster-name`
- `--controller-trusted-role-arns` — override CAPA controller trusted ARNs
- `--iam-trusted-role-arns` — override trust editor trusted ARNs
Expand Down
5 changes: 2 additions & 3 deletions cmd/internal/bootstrap/aws_scopes.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,8 @@ type AWSScopeVPC struct {
NATGatewayEIPAllocationIDs []string `yaml:"natGatewayEipAllocationIds,omitempty" json:"nat_gateway_eip_allocation_ids,omitempty"`
// KarpenterDiscoveryTagValue is the value of the karpenter.sh/discovery tag on
// the node subnets, which is how Karpenter finds where to launch nodes.
// Defaults to the scope's cluster name. Set it explicitly when a scope holds
// more than one cluster, which scopeTagPolicyVersion 0 allows, because the
// cluster name is then not a meaningful single value.
// Defaults to the VPC ID, including when the scope has a cluster name. Set it
// explicitly only when an existing subnet tag must be preserved.
KarpenterDiscoveryTagValue string `yaml:"karpenterDiscoveryTagValue,omitempty" json:"karpenter_discovery_tag_value,omitempty"`
}

Expand Down
2 changes: 1 addition & 1 deletion cmd/internal/bootstrap/aws_scopes_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ func awsScopesAddCmd() *cobra.Command {
cmd.Flags().String("vpc-name", "", "VPC name; required for dittocloud mode")
cmd.Flags().String("vpc-cidr", "", "VPC CIDR; required for dittocloud mode")
cmd.Flags().String("vpc-secondary-cidr", "", "Secondary VPC CIDR carrying pod, node, and database capacity; a /16 inside 100.64.0.0/10, dittocloud mode only")
cmd.Flags().String("vpc-karpenter-discovery-tag", "", "Value of the karpenter.sh/discovery tag on the node subnets; defaults to the scope cluster name, dittocloud mode only")
cmd.Flags().String("vpc-karpenter-discovery-tag", "", "Value of the karpenter.sh/discovery tag on the node subnets; defaults to the VPC ID, dittocloud mode only")
cmd.Flags().String("vpc-id", "", "VPC ID; required for existing mode and optional for capi mode")
return cmd
}
Expand Down
4 changes: 2 additions & 2 deletions cmd/internal/bootstrap/aws_scopes_command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,8 @@ func TestAWSScopesAddRecordsAnExplicitKarpenterDiscoveryTag(t *testing.T) {
}

func TestAWSScopesAddLeavesTheKarpenterDiscoveryTagUnsetByDefault(t *testing.T) {
// Unset means Terraform falls back to the scope's cluster name, so the field
// must not be written with an empty value that would defeat that coalesce.
// Unset means Terraform falls back to the VPC ID, so the field must not be
// written with an empty value that would defeat that fallback.
forceAWSScopesAddNonInteractive(t)
setAWSScopesAddReferenceSequence(t, testDefaultScopeRef)
scopesPath := filepath.Join(t.TempDir(), "scopes.yaml")
Expand Down
10 changes: 4 additions & 6 deletions docs/aws-multi-scope.md
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,10 @@ value on the node subnets.

That tag is how Karpenter finds where to launch nodes, and Terraform has to own it
because the CAPA controller boundary does not permit the `karpenter.sh` namespace.
Left unset, it falls back to the scope's `clusterName`, and a scope with neither
gets **no discovery tag at all** — Karpenter then falls back to whatever its
`EC2NodeClass` matches next, which for a Valet cluster is the
`kubernetes.io/cluster/*` tag CAPA applies to the DMZ subnets, not the node tier.
Set it explicitly when `scopeTagPolicyVersion` is `0` and the scope holds more than
one cluster, because a single `clusterName` is not meaningful there.
Left unset, it defaults to the VPC ID, including for a named scope. The Valet EKS
chart publishes that same VPC ID for Karpenter's `EC2NodeClass` selector. Set an
explicit value only to preserve an existing discovery tag during migration, and
ensure the cluster-side selector uses the same value before applying it.

The command only updates the scope file. It never initializes Terraform or
changes Terraform state. Run a separate normal bootstrap to review and apply
Expand Down
2 changes: 1 addition & 1 deletion terraform/aws/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ module "scoped_vpc" {
public_subnet_netmask = each.value.vpc.public_subnet_netmask
private_subnet_netmask = each.value.vpc.private_subnet_netmask
manage_kubernetes_cluster_tag = false
karpenter_discovery_tag_value = try(coalesce(each.value.vpc.karpenter_discovery_tag_value, each.value.cluster_name), null)
karpenter_discovery_tag_value = each.value.vpc.karpenter_discovery_tag_value
nat_gateway_name = each.value.vpc.nat_gateway_name
nat_gateway_eip_allocation_ids = each.value.vpc.nat_gateway_eip_allocation_ids
tags = merge(
Expand Down
13 changes: 5 additions & 8 deletions terraform/aws/scopes.tf
Original file line number Diff line number Diff line change
Expand Up @@ -70,14 +70,11 @@ locals {
default_private_subnet_netmask = (
local.default_scope != null ? local.default_scope.vpc.private_subnet_netmask : var.private_subnet_netmask
)
# The node subnets carry karpenter.sh/discovery. A scope may set the value
# explicitly and otherwise falls back to its own cluster name; the explicit form
# matters because scopeTagPolicyVersion 0 permits several clusters in one scope,
# where the cluster name is not a meaningful single value. The legacy path takes
# its own variable and falls back to the IAM cluster name.
default_karpenter_discovery_tag_value = local.default_scope != null ? try(
coalesce(local.default_scope.vpc.karpenter_discovery_tag_value, local.default_scope.cluster_name), null
) : try(
# In scope mode, leave an unset discovery value null so the VPC module tags
# node subnets with its VPC ID, matching the Valet EKS EC2NodeClass selector.
# A scope's cluster name identifies its IAM/tag-policy target, not its VPC.
# Preserve the legacy non-scope fallback to the cluster name.
default_karpenter_discovery_tag_value = local.default_scope != null ? local.default_scope.vpc.karpenter_discovery_tag_value : try(
coalesce(var.karpenter_discovery_tag_value, var.cluster_name), null
)
default_nat_gateway_eip_allocation_ids = (
Expand Down
62 changes: 53 additions & 9 deletions terraform/aws/tests/scope_validation.tftest.hcl
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,12 @@ mock_provider "aws" {
arn = "arn:aws:iam::123456789012:instance-profile/mock-profile"
}
}

mock_resource "aws_sqs_queue" {
defaults = {
arn = "arn:aws:sqs:ap-southeast-2:123456789012:mock-queue"
}
}
}

variables {
Expand Down Expand Up @@ -1214,11 +1220,10 @@ run "publishes_workload_networking_for_a_managed_scope" {
# trade-off is that two VPCs sharing it cannot be peered, because AWS rejects a
# peering connection on any overlapping CIDR; cross-VPC connectivity is
# PrivateLink or VPC Lattice instead.
# The node subnets carry karpenter.sh/discovery, and its value comes from the
# scope. An explicit value wins; otherwise the scope's cluster name is used. The
# explicit form matters because scopeTagPolicyVersion 0 permits several clusters
# in one scope, where a single cluster name is not meaningful.
run "prefers_an_explicit_karpenter_discovery_tag_over_the_cluster_name" {
# The node subnets carry karpenter.sh/discovery. An explicit value wins;
# otherwise both default and non-default scopes use their VPC IDs, even when
# cluster_name is set for IAM or scope tag-policy purposes.
run "prefers_an_explicit_karpenter_discovery_tag_over_the_vpc_id" {
command = plan

variables {
Expand All @@ -1241,11 +1246,11 @@ run "prefers_an_explicit_karpenter_discovery_tag_over_the_cluster_name" {

assert {
condition = local.default_karpenter_discovery_tag_value == "shared-workload"
error_message = "An explicit vpc.karpenter_discovery_tag_value must win over the scope cluster name."
error_message = "An explicit vpc.karpenter_discovery_tag_value must win over the VPC ID."
}
}

run "falls_back_to_the_cluster_name_for_the_karpenter_discovery_tag" {
run "defaults_named_scope_discovery_to_the_vpc_id" {
command = plan

variables {
Expand All @@ -1266,8 +1271,47 @@ run "falls_back_to_the_cluster_name_for_the_karpenter_discovery_tag" {
}

assert {
condition = local.default_karpenter_discovery_tag_value == "valet-dev"
error_message = "With no explicit value the scope cluster name must tag the node subnets."
condition = local.default_karpenter_discovery_tag_value == null
error_message = "A named default scope must leave discovery unset so the VPC module uses its VPC ID."
}
}

run "defaults_named_non_default_scope_discovery_to_the_vpc_id" {
command = apply

variables {
deployment_scopes = {
"dsc-01k2m8g7n4p6q9r3t5v8x1y2z3" = {
default = true
cluster_type = "eks"
region = "ap-southeast-2"
vpc = {
mode = "dittocloud"
name = "valet-default"
cidr = "10.214.0.0/20"
secondary_cidr = "100.64.0.0/16"
}
}
"dsc-01k2m8g7n4p6q9r3t5v8x1y2z4" = {
cluster_name = "v2-vnext-test"
cluster_type = "eks"
region = "ap-southeast-2"
vpc = {
mode = "dittocloud"
name = "valet-v2-vnext-test"
cidr = "10.221.0.0/20"
secondary_cidr = "100.64.0.0/16"
}
}
}
}

assert {
condition = (
module.vpc[0].karpenter_discovery_tag_value == module.vpc[0].vpc_id &&
module.scoped_vpc["dsc-01k2m8g7n4p6q9r3t5v8x1y2z4"].karpenter_discovery_tag_value == module.scoped_vpc["dsc-01k2m8g7n4p6q9r3t5v8x1y2z4"].vpc_id
)
error_message = "Both default and named non-default scopes must tag node subnets with their own VPC IDs."
}
}

Expand Down
2 changes: 1 addition & 1 deletion terraform/aws/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ variable "private_subnet_netmask" {
}

variable "karpenter_discovery_tag_value" {
description = "Value for the karpenter.sh/discovery tag on the node subnets, which is how Karpenter finds where to launch nodes. Defaults to cluster_name when set; the tag is omitted when neither is set. Terraform has to own this tag because the CAPA controller boundary does not permit the karpenter.sh namespace."
description = "Value for the karpenter.sh/discovery tag on the node subnets. In scope mode, an unset value defaults to the VPC ID even when cluster_name is set; legacy mode retains its cluster-name fallback. Terraform owns this tag because the CAPA controller boundary does not permit the karpenter.sh namespace."
type = string
default = null
nullable = true
Expand Down
4 changes: 4 additions & 0 deletions terraform/aws/vpc/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,10 @@ output "vpc_id" {
value = module.vpc.vpc_id
}

output "karpenter_discovery_tag_value" {
value = local.karpenter_discovery_value
}

# One ENIConfig per availability zone points VPC CNI custom networking at that
# zone's pod subnet. Without AWS_VPC_K8S_CNI_CUSTOM_NETWORK_CFG=true, an
# ENIConfig per AZ, and ENI_CONFIG_LABEL_DEF=topology.kubernetes.io/zone on the
Expand Down
2 changes: 1 addition & 1 deletion terraform/aws/vpc/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ variable "manage_kubernetes_cluster_tag" {
}

variable "karpenter_discovery_tag_value" {
description = "Value for the karpenter.sh/discovery tag on the node subnets, which is how Karpenter finds where to launch nodes. Defaults to kubernetes_cluster_name; the tag is omitted when neither is set."
description = "Value for the karpenter.sh/discovery tag on the node subnets, which is how Karpenter finds where to launch nodes. Defaults to kubernetes_cluster_name when supplied, otherwise the VPC ID."
type = string
default = null
nullable = true
Expand Down
Loading