From 2bcf1ce8b5397bfab18ecf805dc914c2a83fddd9 Mon Sep 17 00:00:00 2001 From: James Nesbitt Date: Thu, 5 Nov 2026 13:25:18 +0200 Subject: [PATCH 1/7] [PRODENG-3929] Scope example ingress, split node roles, stop inlining creds Terraform half of PRODENG-3929. The test-side companion that supplies the new required variable is held until PR #686 merges, because it edits the same four files. aws-simple/launchpad.tf accepted every protocol and port from 0.0.0.0/0 on every manager, worker and MSR node, in a rule the file labelled [BAD RULE], and that module is what every CI smoke run provisions. External ingress is now limited to 22 and 5986 from var.admin_cidrs, and 443 and 6443 from var.admin_cidrs plus the VPC CIDR. The VPC CIDR is required on the two load-balanced ports because the MKE and MSR ingresses reach the nodes from inside the VPC rather than from an operator address, and their health checks would fail without it. Node-to-node traffic within the security group is unchanged. aws-complete/sg.tf opened 22 and 5985-5986 from anywhere; both are now sourced from var.admin_cidrs, and 5985 is removed rather than narrowed, because the Windows user data disables the HTTP listener and deletes its firewall rules, so the cleartext port is already dead on the instance. aws-complete's manager group served 443 and 6443 from anywhere; those follow the same admin_cidrs plus VPC CIDR rule. var.admin_cidrs is declared in both examples with no default and a non-empty validation, so a caller must state who may reach the cluster. A default would reintroduce the open rule silently. var.windows_password is declared sensitive in both examples. That masks terraform's own output only: the value still reaches the instance through user data, which IMDS serves unauthenticated, and the provider does not mark aws_launch_template.user_data sensitive. It does not satisfy the ticket's AC-2 and the PR says so. aws-complete used one IAM role for every node, so workers held the cloud-controller-manager permissions they never use. Managers and workers now carry separate roles and instance profiles, assigned by nodegroup role, which the provisioning module supports because instance_profile_name is set per nodegroup. The CCM control-plane policy attaches to the manager role only; the worker policy, which is ECR pull access, attaches to both because every node pulls images, as does the EBS CSI policy that was previously on the single role. The CCM policy is split into discovery actions, which AWS supports no resource-level permissions for, creation actions, which cannot be constrained by the tags of a resource that does not exist yet, and mutation of existing resources, which is constrained to resources carrying this cluster's kubernetes.io/cluster tag. The shape follows the policy published by the upstream cloud-provider-aws project; no ARNs are invented. Both examples rendered the MKE admin password and the WinRM password inline into the generated launchpad.yaml, contradicting the guidance added in PRODENG-3935 that credentials come from the environment. They now emit LAUNCHPAD_MKE_ADMIN_PASSWORD and LAUNCHPAD_WINRM_PASSWORD references, which launchpad expands through envsubst at config load; an unset variable fails loudly rather than installing with an empty password. Refs: PRODENG-3929 Written by AI: claude-opus-5 Signed-off-by: James Nesbitt --- examples/terraform/aws-complete/iam.tf | 24 +- examples/terraform/aws-complete/launchpad.tf | 23 +- examples/terraform/aws-complete/main.tf | 15 +- .../aws-complete/modules/aws-ccm/iam.tf | 235 ++++++++++-------- .../aws-complete/modules/aws-ccm/variables.tf | 9 +- .../modules/ebs-csi-driver/iam.tf | 4 +- .../modules/ebs-csi-driver/variables.tf | 6 +- examples/terraform/aws-complete/provision.tf | 2 +- examples/terraform/aws-complete/sg.tf | 12 +- .../aws-complete/terraform.tfvars.template | 5 + examples/terraform/aws-complete/variables.tf | 15 ++ examples/terraform/aws-simple/launchpad.tf | 66 ++++- .../aws-simple/terraform.tfvars.template | 6 + examples/terraform/aws-simple/variables.tf | 16 ++ 14 files changed, 300 insertions(+), 138 deletions(-) diff --git a/examples/terraform/aws-complete/iam.tf b/examples/terraform/aws-complete/iam.tf index f771126d2..6e2321334 100644 --- a/examples/terraform/aws-complete/iam.tf +++ b/examples/terraform/aws-complete/iam.tf @@ -1,5 +1,14 @@ -resource "aws_iam_role" "common_role" { - name = "common-iam-role-${var.name}" +// Manager and worker nodes carry separate roles so that the cloud-controller +// permissions, which only the control plane uses, are not held by every +// worker in the cluster. Both roles share the same trust policy. +resource "aws_iam_role" "manager_role" { + name = "manager-iam-role-${var.name}" + path = "/" + assume_role_policy = data.aws_iam_policy_document.assume_role.json +} + +resource "aws_iam_role" "worker_role" { + name = "worker-iam-role-${var.name}" path = "/" assume_role_policy = data.aws_iam_policy_document.assume_role.json } @@ -17,7 +26,12 @@ data "aws_iam_policy_document" "assume_role" { } } -resource "aws_iam_instance_profile" "common_profile" { - name = "common-instance-profile-${var.name}" - role = aws_iam_role.common_role.name +resource "aws_iam_instance_profile" "manager_profile" { + name = "manager-instance-profile-${var.name}" + role = aws_iam_role.manager_role.name +} + +resource "aws_iam_instance_profile" "worker_profile" { + name = "worker-instance-profile-${var.name}" + role = aws_iam_role.worker_role.name } diff --git a/examples/terraform/aws-complete/launchpad.tf b/examples/terraform/aws-complete/launchpad.tf index 78fa8b91d..78d24bb4b 100644 --- a/examples/terraform/aws-complete/launchpad.tf +++ b/examples/terraform/aws-complete/launchpad.tf @@ -52,7 +52,10 @@ locals { } } - // standard MCR/MKE/MSR firewall rules [here we just leave it open until we can figure this out] + // MCR/MKE/MSR firewall rules. The load-balanced ports accept the VPC CIDR + // as well as var.admin_cidrs, because the ingress load balancers reach the + // manager nodes from inside the VPC rather than from an administrator's + // address, and their health checks would fail without it. launchpad_securitygroups = { "manager" = { description = "Common security group for manager nodes" @@ -60,20 +63,20 @@ locals { tags = local.common_tags ingress_ipv4 = [ { - description : "Allow https traffic from anywhere" + description : "Allow https traffic from administrators and the load balancers" from_port : 443 to_port : 443 protocol : "tcp" self : false - cidr_blocks : ["0.0.0.0/0"] + cidr_blocks : concat(var.admin_cidrs, [var.network.cidr]) }, { - description : "Allow https traffic from anywhere for kube api server" + description : "Allow kube api server traffic from administrators and the load balancer" from_port : 6443 to_port : 6443 protocol : "tcp" self : false - cidr_blocks : ["0.0.0.0/0"] + cidr_blocks : concat(var.admin_cidrs, [var.network.cidr]) }, ] } @@ -182,6 +185,12 @@ resource "launchpad_config" "cluster" { } // ------- Ye old launchpad yaml (just for debugging) +// +// Credentials are referenced by environment variable rather than inlined: +// launchpad expands them through envsubst when it loads the config, and +// docs/usage/getting-started.md tells users to source credentials from the +// environment. Export LAUNCHPAD_MKE_ADMIN_PASSWORD, and +// LAUNCHPAD_WINRM_PASSWORD for Windows stacks, before running launchpad. output "launchpad_yaml" { description = "launchpad config file yaml (for debugging)" sensitive = true @@ -208,7 +217,7 @@ spec: winRM: address: ${h.winrm_address} user: ${h.winrm_user} - password: ${h.winrm_password} + password: $${LAUNCHPAD_WINRM_PASSWORD} useHTTPS: ${h.winrm_useHTTPS} insecure: ${h.winrm_insecure} %{~endfor} @@ -216,7 +225,7 @@ spec: version: ${var.launchpad.mke_version} imageRepo: docker.io/mirantis adminUsername: ${var.launchpad.mke_connect.username} - adminPassword: ${var.launchpad.mke_connect.password} + adminPassword: $${LAUNCHPAD_MKE_ADMIN_PASSWORD} installFlags: - "--san=${local.MKE_URL}" - "--nodeport-range=32768-35535" diff --git a/examples/terraform/aws-complete/main.tf b/examples/terraform/aws-complete/main.tf index 656e3b610..337bbc10f 100644 --- a/examples/terraform/aws-complete/main.tf +++ b/examples/terraform/aws-complete/main.tf @@ -24,12 +24,17 @@ locals { module "aws_ccm" { - source = "./modules/aws-ccm" - iam_role_name = aws_iam_role.common_role.name - cluster_name = var.name + source = "./modules/aws-ccm" + // The control-plane policy goes to managers only; the worker policy, which + // is ECR pull access, is needed by every node that runs containers. + control_plane_role_name = aws_iam_role.manager_role.name + worker_role_names = [aws_iam_role.manager_role.name, aws_iam_role.worker_role.name] + cluster_name = var.name } module "aws_ebs_csi_driver" { - source = "./modules/ebs-csi-driver" - iam_role_name = aws_iam_role.common_role.name + source = "./modules/ebs-csi-driver" + // Attached to both roles, as it was to the single role before: the CSI + // controller runs on a manager and the node plugin runs everywhere. + iam_role_names = [aws_iam_role.manager_role.name, aws_iam_role.worker_role.name] } diff --git a/examples/terraform/aws-complete/modules/aws-ccm/iam.tf b/examples/terraform/aws-complete/modules/aws-ccm/iam.tf index 58a4360f4..1f2ee6999 100644 --- a/examples/terraform/aws-complete/modules/aws-ccm/iam.tf +++ b/examples/terraform/aws-complete/modules/aws-ccm/iam.tf @@ -1,117 +1,154 @@ -resource "aws_iam_policy" "control_plane_policy" { - name = "control-plane-policy-${var.cluster_name}" - description = "Iam policy for attaching to the IAM instance profile" +// The AWS cloud controller manager's permissions are split into three +// statements, following the shape of the policy published by the upstream +// cloud-provider-aws project: +// +// - discovery calls, which AWS does not support resource-level permissions +// for at all, so they stay at Resource "*"; +// - calls that create new resources, which cannot be constrained by the tags +// of a resource that does not exist yet; +// - calls that mutate or delete existing resources, which are constrained to +// resources carrying this cluster's ownership tag. +// +// The cluster tag is the one the stack applies to everything it provisions +// (see local.kube_tags in the example root), which is also the tag the cloud +// controller manager puts on the resources it creates. +locals { + cluster_tag_key = "aws:ResourceTag/kubernetes.io/cluster/${var.cluster_name}" +} + +data "aws_iam_policy_document" "control_plane" { + // Discovery. No resource-level permissions exist for these actions. + statement { + sid = "Discovery" + effect = "Allow" + actions = [ + "autoscaling:DescribeAutoScalingGroups", + "autoscaling:DescribeLaunchConfigurations", + "autoscaling:DescribeTags", + "ec2:DescribeAvailabilityZones", + "ec2:DescribeInstances", + "ec2:DescribeRegions", + "ec2:DescribeRouteTables", + "ec2:DescribeSecurityGroups", + "ec2:DescribeSubnets", + "ec2:DescribeVolumes", + "ec2:DescribeVpcs", + "elasticloadbalancing:DescribeListeners", + "elasticloadbalancing:DescribeLoadBalancerAttributes", + "elasticloadbalancing:DescribeLoadBalancerPolicies", + "elasticloadbalancing:DescribeLoadBalancers", + "elasticloadbalancing:DescribeTargetGroups", + "elasticloadbalancing:DescribeTargetHealth", + "kms:DescribeKey", + ] + resources = ["*"] + } - policy = < 0 + error_message = "admin_cidrs must list at least one CIDR." + } +} + variable "windows_password" { description = "Password to use with windows & winrm" type = string + sensitive = true default = "" } diff --git a/examples/terraform/aws-simple/launchpad.tf b/examples/terraform/aws-simple/launchpad.tf index ad4c5416a..30c801ac6 100644 --- a/examples/terraform/aws-simple/launchpad.tf +++ b/examples/terraform/aws-simple/launchpad.tf @@ -62,14 +62,26 @@ locals { } } - // standard MCR/MKE/MSR firewall rules [here we just leave it open until we can figure this out] + // MCR/MKE/MSR firewall rules. + // + // Node-to-node traffic inside the group stays unrestricted: MKE, swarm and + // the kubelet use a wide and version-dependent port range between cluster + // members, and every member is in this security group. + // + // Everything arriving from outside the group is restricted to the ports the + // cluster actually serves, from the CIDRs the operator names in + // var.admin_cidrs. The two load-balanced ports additionally accept the VPC + // CIDR, because the MKE and MSR load balancers defined in + // local.launchpad_ingresses above reach the nodes from inside the VPC + // rather than from an administrator's address, and their health checks + // would fail if only var.admin_cidrs were allowed. launchpad_securitygroups = { "permissive" = { description = "Common SG for all cluster machines" nodegroups = [for n, ng in var.nodegroups : n] ingress_ipv4 = [ { - description : "Permissive internal traffic [BAD RULE]" + description : "Cluster-internal traffic between members of this group" from_port : 0 to_port : 0 protocol : "-1" @@ -77,12 +89,36 @@ locals { cidr_blocks : [] }, { - description : "Permissive external traffic [BAD RULE]" - from_port : 0 - to_port : 0 - protocol : "-1" + description : "SSH for launchpad host configuration" + from_port : 22 + to_port : 22 + protocol : "tcp" self : false - cidr_blocks : ["0.0.0.0/0"] + cidr_blocks : var.admin_cidrs + }, + { + description : "WinRM over HTTPS for launchpad host configuration" + from_port : 5986 + to_port : 5986 + protocol : "tcp" + self : false + cidr_blocks : var.admin_cidrs + }, + { + description : "MKE and MSR web/API, from administrators and from the load balancers" + from_port : 443 + to_port : 443 + protocol : "tcp" + self : false + cidr_blocks : concat(var.admin_cidrs, [var.network.cidr]) + }, + { + description : "Kubernetes API, from administrators and from the load balancer" + from_port : 6443 + to_port : 6443 + protocol : "tcp" + self : false + cidr_blocks : concat(var.admin_cidrs, [var.network.cidr]) } ] egress_ipv4 = [ @@ -136,7 +172,17 @@ locals { } // ------- Ye old launchpad yaml (just for debugging) - +// +// The credentials are referenced by environment variable rather than written +// into the document. launchpad expands these through envsubst when it loads +// the config (pkg/config), and docs/usage/getting-started.md tells users to +// source credentials from the environment; a generated file that inlines them +// contradicts that and leaves the password in every copy of the YAML, in +// terraform state, and in anything that prints either. The consumer exports +// LAUNCHPAD_MKE_ADMIN_PASSWORD and, for Windows stacks, +// LAUNCHPAD_WINRM_PASSWORD before running launchpad; an unset variable fails +// config loading loudly rather than silently installing with an empty +// password (PRODENG-3751). locals { launchpad_yaml = <<-EOT apiVersion: launchpad.mirantis.com/mke/v1.6 @@ -161,7 +207,7 @@ spec: winRM: address: ${h.winrm_address} user: ${h.winrm_user} - password: ${h.winrm_password} + password: $${LAUNCHPAD_WINRM_PASSWORD} useHTTPS: ${h.winrm_useHTTPS} insecure: ${h.winrm_insecure} %{~endfor} @@ -174,7 +220,7 @@ spec: version: ${var.launchpad.mke_version} imageRepo: docker.io/mirantis adminUsername: ${var.launchpad.mke_connect.username} - adminPassword: ${var.launchpad.mke_connect.password} + adminPassword: $${LAUNCHPAD_MKE_ADMIN_PASSWORD} installFlags: - "--san=${local.MKE_URL}" - "--default-node-orchestrator=kubernetes" diff --git a/examples/terraform/aws-simple/terraform.tfvars.template b/examples/terraform/aws-simple/terraform.tfvars.template index 66fc4a590..cc391a7b6 100644 --- a/examples/terraform/aws-simple/terraform.tfvars.template +++ b/examples/terraform/aws-simple/terraform.tfvars.template @@ -1,6 +1,12 @@ // used to name infrastructure (CHANGE THIS) //name = "" +// CIDRs allowed to reach the cluster from outside it (SSH, WinRM over HTTPS, +// MKE/MSR and the Kubernetes API). Required: there is no default, so that the +// previous allow-everything rule cannot come back silently. Narrow this to the +// machine running launchpad and the operators who need the UI. +admin_cidrs = ["203.0.113.4/32"] + aws = { region = "us-east-2" } diff --git a/examples/terraform/aws-simple/variables.tf b/examples/terraform/aws-simple/variables.tf index c0e2d2160..d61f301cc 100644 --- a/examples/terraform/aws-simple/variables.tf +++ b/examples/terraform/aws-simple/variables.tf @@ -68,9 +68,25 @@ variable "extra_tags" { default = {} } +variable "admin_cidrs" { + description = <<-EOT + CIDRs allowed to reach the cluster from outside it: SSH, WinRM, and the + MKE/MSR and Kubernetes API ports. Required and intentionally without a + default: the previous rule set accepted every port from 0.0.0.0/0, and a + default would reintroduce that silently. Supply the narrowest set that + covers the machine running launchpad and the operators who need the UI. + EOT + type = list(string) + validation { + condition = length(var.admin_cidrs) > 0 + error_message = "admin_cidrs must list at least one CIDR." + } +} + variable "windows_password" { description = "Password to use with windows & winrm" type = string + sensitive = true default = "" } From 5fd142812567b2f7134b41ee79dc7a2c4aeb0322 Mon Sep 17 00:00:00 2001 From: James Nesbitt Date: Thu, 5 Nov 2026 14:00:52 +0200 Subject: [PATCH 2/7] [PRODENG-3929] Restrict CCM tagging to tags applied during creation Review finding on PR #689. ec2:CreateTags and elasticloadbalancing:AddTags were granted without a condition, which defeated the tag condition on the mutation statement: a holder of the manager role could tag an unrelated security group, volume or load balancer with this cluster's kubernetes.io/cluster tag and then mutate or delete it under a statement that reads as constrained. Both actions are now conditioned on their service's CreateAction key, so tags can only be applied as part of the resource-creating calls this policy already grants. ec2:CreateAction is limited to CreateSecurityGroup and CreateVolume; elasticloadbalancing:CreateAction to CreateListener, CreateLoadBalancer and CreateTargetGroup. Both condition keys are listed for those actions in the AWS service reference, and the construction matches the AWS-managed AmazonEBSCSIDriverPolicy. Not verified without a live cluster: whether the cloud controller manager only ever tags during creation. If it tags an existing resource in a separate call, that call is now denied. The PR records this. Refs: PRODENG-3929 Written by AI: claude-opus-5 Signed-off-by: James Nesbitt --- .../aws-complete/modules/aws-ccm/iam.tf | 39 +++++++++++++++++-- 1 file changed, 36 insertions(+), 3 deletions(-) diff --git a/examples/terraform/aws-complete/modules/aws-ccm/iam.tf b/examples/terraform/aws-complete/modules/aws-ccm/iam.tf index 1f2ee6999..94a97975b 100644 --- a/examples/terraform/aws-complete/modules/aws-ccm/iam.tf +++ b/examples/terraform/aws-complete/modules/aws-ccm/iam.tf @@ -45,15 +45,13 @@ data "aws_iam_policy_document" "control_plane" { } // Creation. The resource does not exist when the call is made, so it cannot - // carry the cluster tag yet; the CCM tags what it creates immediately after. + // carry the cluster tag yet. statement { sid = "CreateClusterResources" effect = "Allow" actions = [ "ec2:CreateSecurityGroup", - "ec2:CreateTags", "ec2:CreateVolume", - "elasticloadbalancing:AddTags", "elasticloadbalancing:CreateListener", "elasticloadbalancing:CreateLoadBalancer", "elasticloadbalancing:CreateLoadBalancerListeners", @@ -64,6 +62,41 @@ data "aws_iam_policy_document" "control_plane" { resources = ["*"] } + // Tagging, restricted to tags applied as part of a create call. + // + // An unrestricted ec2:CreateTags or elasticloadbalancing:AddTags would + // defeat the MutateClusterResources statement below: a holder of this role + // could tag an unrelated security group, volume or load balancer as owned + // by this cluster and then mutate or delete it under a condition that reads + // as constrained. ec2:CreateAction and elasticloadbalancing:CreateAction + // restrict tagging to the resource-creating calls granted above, which is + // the same construction the AWS-managed AmazonEBSCSIDriverPolicy uses. + statement { + sid = "TagOnCreateOnly" + effect = "Allow" + actions = ["ec2:CreateTags"] + resources = ["*"] + + condition { + test = "StringEquals" + variable = "ec2:CreateAction" + values = ["CreateSecurityGroup", "CreateVolume"] + } + } + + statement { + sid = "TagLoadBalancersOnCreateOnly" + effect = "Allow" + actions = ["elasticloadbalancing:AddTags"] + resources = ["*"] + + condition { + test = "StringEquals" + variable = "elasticloadbalancing:CreateAction" + values = ["CreateListener", "CreateLoadBalancer", "CreateTargetGroup"] + } + } + // Mutation of resources this cluster owns. statement { sid = "MutateClusterResources" From c3b75b163acd079bfc3aed79b854a221bbc9d974 Mon Sep 17 00:00:00 2001 From: James Nesbitt Date: Thu, 5 Nov 2026 14:20:02 +0200 Subject: [PATCH 3/7] [PRODENG-3929] Keep caller user_data in aws-complete nodegroups Finding A from the post-merge review of #680, authorised into this ticket. aws-complete/provision.tf merged the platform data over the nodegroup definition, so the platform's user_data always won and a caller-supplied per-nodegroup user_data was discarded without warning. The composition and its rationale are copied verbatim from aws-simple/provision.tf rather than reimplemented: a caller's user_data replaces the platform default on ssh platforms, and is appended after it on winrm platforms, because there the platform default resets the local Administrator password and stands up the WinRM HTTPS listener launchpad needs in order to connect at all. Verified with terraform validate against the aws-complete example, with the helm provider pinned to ~> 2.17 in the scratch copy to work around the pre-existing provider-v3 failure on main. No apply. Refs: PRODENG-3929 Written by AI: claude-opus-5 Signed-off-by: James Nesbitt --- examples/terraform/aws-complete/provision.tf | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/examples/terraform/aws-complete/provision.tf b/examples/terraform/aws-complete/provision.tf index 987fd00c8..0d044107a 100644 --- a/examples/terraform/aws-complete/provision.tf +++ b/examples/terraform/aws-complete/provision.tf @@ -1,7 +1,23 @@ // locals calculated before the provision run locals { - // combine the nodegroup definition with the platform data - nodegroups_wplatform = { for k, ngd in var.nodegroups : k => merge(ngd, local.platforms_with_ami[ngd.platform]) } + // Combine the nodegroup definition with the platform data. The platform + // data wins on every key except user_data: a caller-supplied + // ngd.user_data (e.g. the firewall-cmd/ufw rules in + // terraform.tfvars.template) takes precedence over the platform's coded + // default user_data, EXCEPT for winrm platforms, where the platform's + // default is not merely a convenience default but a fixed requirement -- + // it resets the local Administrator password and stands up the WinRM + // HTTPS listener launchpad needs in order to connect at all. For those + // platforms the caller's user_data is appended after the required setup + // instead of replacing it, so a custom user_data on a Windows nodegroup + // can't accidentally disable WinRM connectivity. + nodegroups_wplatform = { for k, ngd in var.nodegroups : k => merge(ngd, local.platforms_with_ami[ngd.platform], { + user_data = ( + local.platforms_with_ami[ngd.platform].connection == "winrm" + ? join("\n", compact([local.platforms_with_ami[ngd.platform].user_data, ngd.user_data])) + : (ngd.user_data != "" ? ngd.user_data : local.platforms_with_ami[ngd.platform].user_data) + ) + }) } } # PROVISION MACHINES/NETWORK From e4df886aa69c8092ad84dbaa85d81be3a9ce4cf5 Mon Sep 17 00:00:00 2001 From: James Nesbitt Date: Thu, 5 Nov 2026 14:35:44 +0200 Subject: [PATCH 4/7] [PRODENG-3929] Require windows_password when Windows nodegroups are used Authorised by user ruling off w2's measurement on the empty-password fold, not from PRODENG-3929's text. Both examples defaulted windows_password to "" and both generators emit a password line unconditionally, so a stack with Windows nodegroups and no windows_password produced a launchpad.yaml instructing launchpad to authenticate as Administrator with an empty password. The ruling was to remove the default. Measured, that breaks stacks with no Windows node at all: test/smoke/smoke_test.go passes the variable only when a nodegroup platform starts with "windows_", so the three Linux-only smoke tests pass nothing; swarm_addr_pool_test.go, upgrade_test.go, integration_test.go and test_manager.go reference it zero times; and both terraform.tfvars.template files ship it commented out. Five call paths and both documented starting points would fail at plan on Linux-only stacks. The default is therefore null with a validation that fires only when a nodegroup uses a windows_ platform, which is the detection smoke_test.go:152 already uses. It fails at plan naming the variable and explaining what it is for, and it rejects an explicitly-passed empty string, which is the case indistinguishable from the default in the generated YAML. Linux-only stacks continue to plan. Consumers take a string, so each example derives local.windows_password, which substitutes "" for null; the validation guarantees that only happens when no Windows node is being provisioned. Cross-variable references in validation require Terraform 1.9 or later, stated in the variable description. Verified by plan against the aws-simple example: a Linux-only config raises no variable error; a config whose nodegroup platform is windows_2022 and which passes no password fails with "Invalid value for variable ... windows_password must be set to a non-empty value when any nodegroup uses a windows_ platform"; the same config passing windows_password = "" fails identically; a Linux-only config that does pass a password raises no error. aws-complete validates. No apply. Refs: PRODENG-3929 Written by AI: claude-opus-5 Signed-off-by: James Nesbitt --- examples/terraform/aws-complete/launchpad.tf | 2 +- examples/terraform/aws-complete/platform.tf | 8 ++++++- examples/terraform/aws-complete/variables.tf | 24 ++++++++++++++++++-- examples/terraform/aws-simple/launchpad.tf | 2 +- examples/terraform/aws-simple/platform.tf | 12 +++++++--- examples/terraform/aws-simple/variables.tf | 24 ++++++++++++++++++-- 6 files changed, 62 insertions(+), 10 deletions(-) diff --git a/examples/terraform/aws-complete/launchpad.tf b/examples/terraform/aws-complete/launchpad.tf index 78d24bb4b..09445f683 100644 --- a/examples/terraform/aws-complete/launchpad.tf +++ b/examples/terraform/aws-complete/launchpad.tf @@ -110,7 +110,7 @@ locals { winrm_address : ngn.public_ip winrm_user : ng.winrm_user - winrm_password : var.windows_password + winrm_password : local.windows_password winrm_useHTTPS : ng.winrm_useHTTPS winrm_insecure : ng.winrm_insecure } if contains(local.launchpad_roles, ng.role) && ng.connection == "winrm" }]...) diff --git a/examples/terraform/aws-complete/platform.tf b/examples/terraform/aws-complete/platform.tf index 30d4f3374..3f61a95f0 100644 --- a/examples/terraform/aws-complete/platform.tf +++ b/examples/terraform/aws-complete/platform.tf @@ -5,6 +5,12 @@ locals { // - this is unique to avoid repeated ami pulls for the same definition // - only node-group platforms are pulled to avoid pulling images data sources that are not used anywhere unique_used_platforms = distinct([for ngd in var.nodegroups : ngd.platform]) + + // var.windows_password is null on Linux-only stacks, where no Windows node + // exists to carry it; the consumers below take a string. Its validation + // rejects null when a windows_ platform is in use, so reaching here with + // null means no Windows node is being provisioned. + windows_password = var.windows_password == null ? "" : var.windows_password } module "platform" { @@ -13,7 +19,7 @@ module "platform" { version = "0.1.8" platform_key = local.unique_used_platforms[count.index] - windows_password = var.windows_password + windows_password = local.windows_password } // variables calculated after ami data is pulled diff --git a/examples/terraform/aws-complete/variables.tf b/examples/terraform/aws-complete/variables.tf index d30c0e5b2..9d65189c4 100644 --- a/examples/terraform/aws-complete/variables.tf +++ b/examples/terraform/aws-complete/variables.tf @@ -77,8 +77,28 @@ variable "admin_cidrs" { } variable "windows_password" { - description = "Password to use with windows & winrm" + description = <<-EOT + Password set on the local Administrator account of Windows nodes and used + by launchpad for WinRM. Required when any nodegroup runs a Windows + platform; leave unset for Linux-only stacks. + + The validation below references var.nodegroups, which Terraform supports + from 1.9 onwards. + EOT type = string sensitive = true - default = "" + default = null + + // Without this, a stack with Windows nodegroups and no windows_password + // generated a launchpad.yaml carrying "password:" with an empty value, and + // launchpad then authenticated as Administrator with no password. Failing + // here names the variable at plan time instead. + validation { + condition = ( + var.windows_password == null + ? !anytrue([for ng in var.nodegroups : startswith(ng.platform, "windows_")]) + : length(var.windows_password) > 0 + ) + error_message = "windows_password must be set to a non-empty value when any nodegroup uses a windows_ platform: it is the local Administrator password the nodes are configured with and the credential launchpad uses for WinRM. Leave it unset only for Linux-only stacks." + } } diff --git a/examples/terraform/aws-simple/launchpad.tf b/examples/terraform/aws-simple/launchpad.tf index 30c801ac6..dcd75c4ed 100644 --- a/examples/terraform/aws-simple/launchpad.tf +++ b/examples/terraform/aws-simple/launchpad.tf @@ -162,7 +162,7 @@ locals { winrm_address : ngn.public_ip winrm_user : ng.winrm_user - winrm_password : var.windows_password + winrm_password : local.windows_password winrm_useHTTPS : ng.winrm_useHTTPS winrm_insecure : ng.winrm_insecure } if contains(local.launchpad_roles, ng.role) && ng.connection == "winrm" }]...) diff --git a/examples/terraform/aws-simple/platform.tf b/examples/terraform/aws-simple/platform.tf index 6caef80a2..f39d0f518 100644 --- a/examples/terraform/aws-simple/platform.tf +++ b/examples/terraform/aws-simple/platform.tf @@ -1,5 +1,11 @@ // variables calculated before ami data is retrieved locals { + // var.windows_password is null on Linux-only stacks, where no Windows node + // exists to carry it; the consumers below take a string. Its validation + // rejects null when a windows_ platform is in use, so reaching here with + // null means no Windows node is being provisioned. + windows_password = var.windows_password == null ? "" : var.windows_password + // find the unique platforms actually used in the node_group_definitions unique_used_platforms = distinct([for ngd in var.nodegroups : ngd.platform]) @@ -46,7 +52,7 @@ module "platform" { version = "0.1.8" platform_key = local.upstream_platform_keys[count.index] - windows_password = var.windows_password + windows_password = local.windows_password } data "aws_ami" "local" { @@ -89,7 +95,7 @@ locals { upstream_platforms_with_ami = { for k, p in local.upstream_platform_keys : p => merge(module.platform[k].platform, { user_data = module.platform[k].platform.connection == "winrm" ? templatefile("${path.module}/userdata_windows.tpl", { - windows_administrator_password = var.windows_password + windows_administrator_password = local.windows_password }) : "" }) } @@ -100,7 +106,7 @@ locals { ami = data.aws_ami.local[p].id root_device_name = data.aws_ami.local[p].root_device_name user_data = def.connection == "winrm" ? templatefile("${path.module}/userdata_windows.tpl", { - windows_administrator_password = var.windows_password + windows_administrator_password = local.windows_password }) : "" }) if contains(local.local_platform_keys, p) } diff --git a/examples/terraform/aws-simple/variables.tf b/examples/terraform/aws-simple/variables.tf index d61f301cc..85f5dbb6d 100644 --- a/examples/terraform/aws-simple/variables.tf +++ b/examples/terraform/aws-simple/variables.tf @@ -84,10 +84,30 @@ variable "admin_cidrs" { } variable "windows_password" { - description = "Password to use with windows & winrm" + description = <<-EOT + Password set on the local Administrator account of Windows nodes and used + by launchpad for WinRM. Required when any nodegroup runs a Windows + platform; leave unset for Linux-only stacks. + + The validation below references var.nodegroups, which Terraform supports + from 1.9 onwards. + EOT type = string sensitive = true - default = "" + default = null + + // Without this, a stack with Windows nodegroups and no windows_password + // generated a launchpad.yaml carrying "password:" with an empty value, and + // launchpad then authenticated as Administrator with no password. Failing + // here names the variable at plan time instead. + validation { + condition = ( + var.windows_password == null + ? !anytrue([for ng in var.nodegroups : startswith(ng.platform, "windows_")]) + : length(var.windows_password) > 0 + ) + error_message = "windows_password must be set to a non-empty value when any nodegroup uses a windows_ platform: it is the local Administrator password the nodes are configured with and the credential launchpad uses for WinRM. Leave it unset only for Linux-only stacks." + } } variable "ssh_pk_location" { From 259de1d5e44ee862d15c37a58dfc0da0a500b7c6 Mon Sep 17 00:00:00 2001 From: James Nesbitt Date: Thu, 5 Nov 2026 14:47:15 +0200 Subject: [PATCH 5/7] [PRODENG-3929] Supply admin_cidrs and credential env from the test suites AC-4: the suites that provision these examples must keep working with the narrowed rules. This is the companion to the terraform changes on this branch, held until #686 merged because it edits the same files. The examples' admin_cidrs variable is required with no default, so every caller must state who may reach the cluster. test.AdminCIDRs reads SMOKE_ADMIN_CIDRS as a comma-separated list and falls back to 0.0.0.0/0 when it is unset, logging that it did. The fallback is the exposure the examples had before, but stated at the caller where it can be found and changed, rather than implied by a module default. Runners have no fixed egress address, and discovering one at run time would add an unauthenticated network call to every smoke run. The examples now emit LAUNCHPAD_MKE_ADMIN_PASSWORD and LAUNCHPAD_WINRM_PASSWORD references instead of inlining credentials, so the suites export them with test.ExportGeneratedCredentials before the generated YAML is parsed; launchpad resolves them through envsubst at load, and an unset reference fails loudly. TestGeneratedCredentialReferencesMatchExports pins the contract across the two languages: it extracts every $${NAME} reference from both examples' launchpad.tf and asserts each is a variable the suites export. Renaming either side would otherwise produce a config whose credentials expand to nothing, which only a live cluster would reveal. Verified by mutation: renaming the reference in aws-simple/launchpad.tf fails the test with "refers to LAUNCHPAD_MKE_ADMINPASSWORD, which no suite exports". TestAdminCIDRs covers the fallback and a configured list. Not demonstrated: that a smoke run passes end to end. Smoke needs live AWS and is gated, so what is shown is that the suites supply the variable and export the references, not that a provisioned cluster comes up. Refs: PRODENG-3929 Written by AI: claude-opus-5 Signed-off-by: James Nesbitt --- test/integration/integration_test.go | 10 +++++- test/smoke/smoke_test.go | 9 ++++- test/smoke/swarm_addr_pool_test.go | 16 ++++++--- test/smoke/upgrade_test.go | 6 ++++ test/terraform.go | 49 ++++++++++++++++++++++++++++ test/terraform_test.go | 47 ++++++++++++++++++++++++++ 6 files changed, 131 insertions(+), 6 deletions(-) diff --git a/test/integration/integration_test.go b/test/integration/integration_test.go index cca14d67b..125eef8cd 100644 --- a/test/integration/integration_test.go +++ b/test/integration/integration_test.go @@ -63,7 +63,14 @@ func TestMain(m *testing.M) { name := fmt.Sprintf("smoke-%s", uTestId) - MKE_CONNECT["password"] = test.GenerateRandomAlphaNumericString(12) + mkePassword := test.GenerateRandomAlphaNumericString(12) + MKE_CONNECT["password"] = mkePassword + + // The generated YAML refers to the MKE password by environment variable + // rather than inlining it; launchpad resolves it through envsubst when + // sp.Setup loads the config. This stack has no Windows nodes, so no WinRM + // reference is generated. + test.ExportGeneratedCredentials(t, mkePassword, "") options := test.TerraformOptions( t, @@ -72,6 +79,7 @@ func TestMain(m *testing.M) { map[string]any{ "name": name, "aws": AWS, + "admin_cidrs": test.AdminCIDRs(t), "network": NETWORK, "subnets": SUBNETS, "ssh_pk_location": tempSSHKeyPathDir, diff --git a/test/smoke/smoke_test.go b/test/smoke/smoke_test.go index ffc500e7e..225f2091b 100644 --- a/test/smoke/smoke_test.go +++ b/test/smoke/smoke_test.go @@ -129,6 +129,7 @@ func runSmokeTest(t *testing.T, cfg smokeConfig) { vars := map[string]interface{}{ "name": name, "aws": awsConfig, + "admin_cidrs": test.AdminCIDRs(t), "network": networkConfig, "subnets": subnets, "ssh_pk_location": tempSSHKeyPathDir, @@ -159,10 +160,16 @@ func runSmokeTest(t *testing.T, cfg smokeConfig) { break } } + windowsPassword := "" if hasWindows { - secretVars["windows_password"] = generateWindowsPassword(t) + windowsPassword = generateWindowsPassword(t) + secretVars["windows_password"] = windowsPassword } + // The generated YAML refers to these by environment variable rather than + // inlining them; launchpad resolves them through envsubst at load. + test.ExportGeneratedCredentials(t, mkePassword, windowsPassword) + options := test.TerraformOptions(t, "../../examples/terraform/aws-simple", vars, secretVars) terraformOptions := terraform.WithDefaultRetryableErrors(t, &options) diff --git a/test/smoke/swarm_addr_pool_test.go b/test/smoke/swarm_addr_pool_test.go index 15ae8e50b..7eda92cb2 100644 --- a/test/smoke/swarm_addr_pool_test.go +++ b/test/smoke/swarm_addr_pool_test.go @@ -158,9 +158,10 @@ func provisionSwarmPoolStack(t *testing.T, cfg smokeConfig) string { } vars := map[string]interface{}{ - "name": name, - "aws": awsConfig, - "network": networkConfig, + "name": name, + "aws": awsConfig, + "admin_cidrs": test.AdminCIDRs(t), + "network": networkConfig, "subnets": map[string]interface{}{ "main": map[string]interface{}{ "cidr": "172.31.0.0/17", @@ -177,6 +178,8 @@ func provisionSwarmPoolStack(t *testing.T, cfg smokeConfig) string { }, } + mkePassword := test.GenerateRandomAlphaNumericString(12) + // launchpad nests the MKE admin password, so it travels by TF_VAR_* rather // than as a "-var" argument. See test.TerraformOptions. secretVars := map[string]interface{}{ @@ -187,12 +190,17 @@ func provisionSwarmPoolStack(t *testing.T, cfg smokeConfig) string { "msr_version": cfg.MSRVersion, "mke_connect": map[string]interface{}{ "username": "admin", - "password": test.GenerateRandomAlphaNumericString(12), + "password": mkePassword, "insecure": true, }, }, } + // The generated YAML refers to the MKE password by environment variable + // rather than inlining it; launchpad resolves it through envsubst at load. + // This stack has no Windows nodes, so no WinRM reference is generated. + test.ExportGeneratedCredentials(t, mkePassword, "") + options := test.TerraformOptions(t, "../../examples/terraform/aws-simple", vars, secretVars) terraformOptions := terraform.WithDefaultRetryableErrors(t, &options) diff --git a/test/smoke/upgrade_test.go b/test/smoke/upgrade_test.go index 4d7c0e922..12a757155 100644 --- a/test/smoke/upgrade_test.go +++ b/test/smoke/upgrade_test.go @@ -89,6 +89,7 @@ func runUpgradeTest(t *testing.T, cfg upgradeConfig) { vars := map[string]interface{}{ "name": name, "aws": awsConfig, + "admin_cidrs": test.AdminCIDRs(t), "network": networkConfig, "subnets": subnets, "ssh_pk_location": tempSSHKeyPathDir, @@ -106,6 +107,11 @@ func runUpgradeTest(t *testing.T, cfg upgradeConfig) { "launchpad": launchpad, } + // The generated YAML refers to the MKE password by environment variable + // rather than inlining it; launchpad resolves it through envsubst at load. + // This stack has no Windows nodes, so no WinRM reference is generated. + test.ExportGeneratedCredentials(t, mkePassword, "") + options := test.TerraformOptions(t, "../../examples/terraform/aws-simple", vars, secretVars) terraformOptions := terraform.WithDefaultRetryableErrors(t, &options) diff --git a/test/terraform.go b/test/terraform.go index 13741fa4f..20adcaffa 100644 --- a/test/terraform.go +++ b/test/terraform.go @@ -2,12 +2,61 @@ package test import ( "encoding/json" + "os" + "strings" "testing" "github.com/gruntwork-io/terratest/modules/logger" "github.com/gruntwork-io/terratest/modules/terraform" ) +// Environment variables the examples' generated launchpad.yaml refers to +// instead of inlining credentials. launchpad expands them through envsubst +// when it loads the config, so a consumer of that YAML must export them +// before parsing it or config loading fails on an unset reference. +const ( + MKEAdminPasswordEnv = "LAUNCHPAD_MKE_ADMIN_PASSWORD" + WinRMPasswordEnv = "LAUNCHPAD_WINRM_PASSWORD" +) + +// AdminCIDRs returns the value for the examples' admin_cidrs variable, which +// scopes SSH, WinRM and the MKE/Kubernetes API to named networks. +// +// The variable is required and has no default, so that narrowing it is a +// decision a caller makes rather than something a module silently decides. +// These suites provision throwaway stacks from runners whose egress address +// is not known ahead of time, so the fallback is 0.0.0.0/0: the same exposure +// the examples had before, but stated at the caller where it can be found and +// changed. Set SMOKE_ADMIN_CIDRS (comma separated) to narrow it. +func AdminCIDRs(t *testing.T) []string { + t.Helper() + + configured := os.Getenv("SMOKE_ADMIN_CIDRS") + if configured == "" { + t.Logf("SMOKE_ADMIN_CIDRS is unset: provisioning with admin_cidrs = [0.0.0.0/0]") + + return []string{"0.0.0.0/0"} + } + + cidrs := strings.Split(configured, ",") + for i, cidr := range cidrs { + cidrs[i] = strings.TrimSpace(cidr) + } + + return cidrs +} + +// ExportGeneratedCredentials publishes the credentials the generated +// launchpad.yaml refers to, so that envsubst resolves them when the config is +// loaded. Pass an empty windowsPassword for a stack with no Windows nodes; +// the reference is only present in the document when a WinRM host is. +func ExportGeneratedCredentials(t *testing.T, mkePassword, windowsPassword string) { + t.Helper() + + t.Setenv(MKEAdminPasswordEnv, mkePassword) + t.Setenv(WinRMPasswordEnv, windowsPassword) +} + // TerraformOptions builds terraform.Options that keep credentials out of the // terraform command line. // diff --git a/test/terraform_test.go b/test/terraform_test.go index 0baeb3a0e..720b69548 100644 --- a/test/terraform_test.go +++ b/test/terraform_test.go @@ -6,6 +6,7 @@ import ( "os" "os/exec" "path/filepath" + "regexp" "strings" "sync" "testing" @@ -196,3 +197,49 @@ func copyFixture(t *testing.T, fixtureDir string) string { return dest } + +// TestGeneratedCredentialReferencesMatchExports pins the contract between the +// terraform examples and these suites: the examples emit environment-variable +// references in the launchpad.yaml they generate, and the suites export those +// variables before the YAML is parsed. A rename on either side leaves a +// config whose credentials expand to nothing, which only a live cluster would +// otherwise reveal. +func TestGeneratedCredentialReferencesMatchExports(t *testing.T) { + exported := map[string]bool{ + MKEAdminPasswordEnv: true, + WinRMPasswordEnv: true, + } + + // The examples escape "$" as "$$" for terraform's own template syntax, so + // the emitted document contains "${NAME}". + reference := regexp.MustCompile(`\$\$\{([A-Z0-9_]+)\}`) + + for _, example := range []string{"aws-simple", "aws-complete"} { + path := filepath.Join("..", "examples", "terraform", example, "launchpad.tf") + + content, err := os.ReadFile(path) // #nosec G304 -- path is a test constant + require.NoError(t, err) + + matches := reference.FindAllStringSubmatch(string(content), -1) + require.NotEmpty(t, matches, "%s emits no credential references: it may have gone back to inlining them", path) + + for _, match := range matches { + assert.True(t, exported[match[1]], + "%s refers to %s, which no suite exports; the generated config would load it as empty", path, match[1]) + } + } +} + +func TestAdminCIDRs(t *testing.T) { + t.Run("falls back to the open range when unset", func(t *testing.T) { + t.Setenv("SMOKE_ADMIN_CIDRS", "") + + assert.Equal(t, []string{"0.0.0.0/0"}, AdminCIDRs(t)) + }) + + t.Run("honours a configured list", func(t *testing.T) { + t.Setenv("SMOKE_ADMIN_CIDRS", "203.0.113.4/32, 198.51.100.0/24") + + assert.Equal(t, []string{"203.0.113.4/32", "198.51.100.0/24"}, AdminCIDRs(t)) + }) +} From cedd2bb614b3601ad6163f350d999233c4ea8f6a Mon Sep 17 00:00:00 2001 From: James Nesbitt Date: Thu, 5 Nov 2026 15:29:28 +0200 Subject: [PATCH 6/7] [PRODENG-3929] Constrain Classic ELB listener calls, allow owned-LB tagging Review findings on PR #689. CreateLoadBalancerListeners and CreateLoadBalancerPolicy operate on an existing Classic load balancer despite their names, so granting them unconditionally let the manager role add listeners and policies to load balancers this cluster does not own. Both move to the tag-constrained mutation statement. The cloud controller manager tags an existing load balancer during reconciliation when a Service carries additional resource tags (addLoadBalancerTags), and that request carries no CreateAction, so the create-only grant would have failed reconciliation. A second statement allows elasticloadbalancing:AddTags on load balancers already carrying this cluster's tag. The escalation stays closed: a resource that is not already this cluster's still cannot be tagged into it. windows_password's validation rejected an explicitly-passed empty string on Linux-only stacks, which have no host that would authenticate with it. The whole requirement is now gated on a Windows nodegroup being present, in both examples. The IAM role split replaces an existing stack's IAM resources, and provision-aws does not refresh running instances on a launch-template change, so nodes provisioned before this change keep an instance profile whose role has been destroyed and lose their permissions until replaced. That is recorded at the top of aws-complete/iam.tf. Enabling instance_refresh is deliberately not done here: it is a separate behavioural decision tracked under PRODENG-4062. Verified by plan against aws-simple, counting "Invalid value for variable": Linux-only 0, Linux-only with windows_password = "" 0 (previously 1), Windows nodegroup without a password 1, Windows with windows_password = "" 1, Windows with a password 0. aws-complete validates. No apply. Refs: PRODENG-3929 Written by AI: claude-opus-5 Signed-off-by: James Nesbitt --- examples/terraform/aws-complete/iam.tf | 10 ++++++ .../aws-complete/modules/aws-ccm/iam.tf | 31 +++++++++++++++++-- examples/terraform/aws-complete/variables.tf | 9 ++++-- examples/terraform/aws-simple/variables.tf | 9 ++++-- 4 files changed, 50 insertions(+), 9 deletions(-) diff --git a/examples/terraform/aws-complete/iam.tf b/examples/terraform/aws-complete/iam.tf index 6e2321334..ba58f847e 100644 --- a/examples/terraform/aws-complete/iam.tf +++ b/examples/terraform/aws-complete/iam.tf @@ -1,6 +1,16 @@ // Manager and worker nodes carry separate roles so that the cloud-controller // permissions, which only the control plane uses, are not held by every // worker in the cluster. Both roles share the same trust policy. +// +// APPLYING THIS TO AN EXISTING STACK REPLACES ITS IAM RESOURCES. The previous +// common_role, common_profile and their policy attachments are destroyed and +// the new profiles are attached to the launch templates, not to instances +// that are already running: provision-aws does not refresh instances on a +// launch-template change, so nodes provisioned before this change keep +// running with an instance profile whose role no longer exists, and the +// cloud controller manager and EBS CSI driver on them lose their permissions +// until the nodes are replaced. Replace the nodes, or recreate the stack, +// rather than re-applying in place against a live cluster. resource "aws_iam_role" "manager_role" { name = "manager-iam-role-${var.name}" path = "/" diff --git a/examples/terraform/aws-complete/modules/aws-ccm/iam.tf b/examples/terraform/aws-complete/modules/aws-ccm/iam.tf index 94a97975b..06a63eb49 100644 --- a/examples/terraform/aws-complete/modules/aws-ccm/iam.tf +++ b/examples/terraform/aws-complete/modules/aws-ccm/iam.tf @@ -46,6 +46,11 @@ data "aws_iam_policy_document" "control_plane" { // Creation. The resource does not exist when the call is made, so it cannot // carry the cluster tag yet. + // + // CreateLoadBalancerListeners and CreateLoadBalancerPolicy are NOT here: + // despite the names they operate on an existing Classic load balancer, so + // they belong with the mutations below and are constrained by the cluster + // tag like the rest of them. statement { sid = "CreateClusterResources" effect = "Allow" @@ -54,8 +59,6 @@ data "aws_iam_policy_document" "control_plane" { "ec2:CreateVolume", "elasticloadbalancing:CreateListener", "elasticloadbalancing:CreateLoadBalancer", - "elasticloadbalancing:CreateLoadBalancerListeners", - "elasticloadbalancing:CreateLoadBalancerPolicy", "elasticloadbalancing:CreateTargetGroup", "iam:CreateServiceLinkedRole", ] @@ -84,8 +87,15 @@ data "aws_iam_policy_document" "control_plane" { } } + // elasticloadbalancing:AddTags is additionally allowed on load balancers + // this cluster already owns, because the cloud controller manager tags an + // existing ELB during reconciliation when a Service carries additional + // resource tags (addLoadBalancerTags in aws_loadbalancer.go), and that + // request carries no CreateAction. Requiring the cluster tag to be present + // already keeps the escalation closed: a resource that is not already this + // cluster's cannot be tagged into it. statement { - sid = "TagLoadBalancersOnCreateOnly" + sid = "TagLoadBalancers" effect = "Allow" actions = ["elasticloadbalancing:AddTags"] resources = ["*"] @@ -97,6 +107,19 @@ data "aws_iam_policy_document" "control_plane" { } } + statement { + sid = "TagOwnedLoadBalancers" + effect = "Allow" + actions = ["elasticloadbalancing:AddTags"] + resources = ["*"] + + condition { + test = "StringEquals" + variable = local.cluster_tag_key + values = ["owned", "shared"] + } + } + // Mutation of resources this cluster owns. statement { sid = "MutateClusterResources" @@ -115,6 +138,8 @@ data "aws_iam_policy_document" "control_plane" { "elasticloadbalancing:ApplySecurityGroupsToLoadBalancer", "elasticloadbalancing:AttachLoadBalancerToSubnets", "elasticloadbalancing:ConfigureHealthCheck", + "elasticloadbalancing:CreateLoadBalancerListeners", + "elasticloadbalancing:CreateLoadBalancerPolicy", "elasticloadbalancing:DeleteListener", "elasticloadbalancing:DeleteLoadBalancer", "elasticloadbalancing:DeleteLoadBalancerListeners", diff --git a/examples/terraform/aws-complete/variables.tf b/examples/terraform/aws-complete/variables.tf index 9d65189c4..8cce71e01 100644 --- a/examples/terraform/aws-complete/variables.tf +++ b/examples/terraform/aws-complete/variables.tf @@ -93,11 +93,14 @@ variable "windows_password" { // generated a launchpad.yaml carrying "password:" with an empty value, and // launchpad then authenticated as Administrator with no password. Failing // here names the variable at plan time instead. + // + // The whole requirement is gated on a Windows nodegroup being present, so a + // Linux-only stack may leave the variable unset or pass an empty string -- + // neither reaches a host that would authenticate with it. validation { condition = ( - var.windows_password == null - ? !anytrue([for ng in var.nodegroups : startswith(ng.platform, "windows_")]) - : length(var.windows_password) > 0 + !anytrue([for ng in var.nodegroups : startswith(ng.platform, "windows_")]) + || (var.windows_password != null && length(var.windows_password) > 0) ) error_message = "windows_password must be set to a non-empty value when any nodegroup uses a windows_ platform: it is the local Administrator password the nodes are configured with and the credential launchpad uses for WinRM. Leave it unset only for Linux-only stacks." } diff --git a/examples/terraform/aws-simple/variables.tf b/examples/terraform/aws-simple/variables.tf index 85f5dbb6d..aa1641987 100644 --- a/examples/terraform/aws-simple/variables.tf +++ b/examples/terraform/aws-simple/variables.tf @@ -100,11 +100,14 @@ variable "windows_password" { // generated a launchpad.yaml carrying "password:" with an empty value, and // launchpad then authenticated as Administrator with no password. Failing // here names the variable at plan time instead. + // + // The whole requirement is gated on a Windows nodegroup being present, so a + // Linux-only stack may leave the variable unset or pass an empty string -- + // neither reaches a host that would authenticate with it. validation { condition = ( - var.windows_password == null - ? !anytrue([for ng in var.nodegroups : startswith(ng.platform, "windows_")]) - : length(var.windows_password) > 0 + !anytrue([for ng in var.nodegroups : startswith(ng.platform, "windows_")]) + || (var.windows_password != null && length(var.windows_password) > 0) ) error_message = "windows_password must be set to a non-empty value when any nodegroup uses a windows_ platform: it is the local Administrator password the nodes are configured with and the credential launchpad uses for WinRM. Leave it unset only for Linux-only stacks." } From a8c486031d90b8861b3cf0cc0c157c89675214fe Mon Sep 17 00:00:00 2001 From: James Nesbitt Date: Fri, 6 Nov 2026 09:55:34 +0200 Subject: [PATCH 7/7] [PRODENG-3929] Restore non-breaking example inputs: default admin_cidrs, variable credentials The security work on this branch changed the examples' input surface in two ways that break an existing external tfvars. This returns both to a shape that keeps working, without giving up the firewall scoping, the CIDR ingress control or the password handling the change was for. admin_cidrs now defaults to ["0.0.0.0/0"] in aws-simple and aws-complete instead of being required. A tfvars written against main previously failed at variable resolution with "No value for required variable"; it now plans. The default is open, which is a real risk, so it is documented as such at the declaration, in the tfvars template and in each example's README, together with the one-line change that narrows it. The 203.0.113.4/32 placeholder is gone: it was RFC 5737 documentation space that looked like a real address and answered no question the reader had. Credentials go back to being written from the terraform variables that carry them, so the generated launchpad.yaml is usable with nothing exported into the environment. Supplying them from the environment is still available and is now documented where a user will look, through terraform's own mechanism rather than a launchpad-specific one: TF_VAR_launchpad carries the object as JSON because the MKE password is nested inside it, and TF_VAR_windows_password carries the Windows credential. The environment augments the variables; it is no longer the only route. The harness helper that exported LAUNCHPAD_MKE_ADMIN_PASSWORD and LAUNCHPAD_WINRM_PASSWORD is removed along with its four call sites, and the test that pinned those references is removed rather than re-pinned: the generated document no longer refers to the environment, so the contract it asserted no longer exists. TestAdminCIDRs and the SMOKE_ADMIN_CIDRS override stay. windows_password keeps its sensitive marking and its Windows-gated validation. var.launchpad is left exactly as main has it. Written by AI: claude-opus-5 Signed-off-by: James Nesbitt --- examples/terraform/aws-complete/README.md | 41 ++++++++++++++++++- examples/terraform/aws-complete/launchpad.tf | 25 +++++++---- .../aws-complete/terraform.tfvars.template | 28 +++++++++++-- examples/terraform/aws-complete/variables.tf | 22 ++++++++-- examples/terraform/aws-simple/README.md | 37 +++++++++++++++++ examples/terraform/aws-simple/launchpad.tf | 30 ++++++++------ .../aws-simple/terraform.tfvars.template | 29 +++++++++++-- examples/terraform/aws-simple/variables.tf | 21 ++++++++-- test/integration/integration_test.go | 6 --- test/smoke/smoke_test.go | 4 -- test/smoke/swarm_addr_pool_test.go | 5 --- test/smoke/upgrade_test.go | 5 --- test/terraform.go | 29 ++----------- test/terraform_test.go | 33 --------------- 14 files changed, 202 insertions(+), 113 deletions(-) diff --git a/examples/terraform/aws-complete/README.md b/examples/terraform/aws-complete/README.md index 7877229d8..83bd829e6 100644 --- a/examples/terraform/aws-complete/README.md +++ b/examples/terraform/aws-complete/README.md @@ -9,4 +9,43 @@ - This configuration uses Launchpad terraform provider for installing MCR, MKE and MSR 2.9x on the provisioned infrstructure - After installing MKE, the configuration uses MKE terraform provider to create a kubernetes context to install kubernetes workloads - After creating a kubernetes context the same is used for installing AWS cloud controller manager and AWS EBS CSI driver -- The content of test.tf file is commented out and the resources in that file can be used testing the functionality of AWS cloud controller manager and AWS EBS CSI driver \ No newline at end of file +- The content of test.tf file is commented out and the resources in that file can be used testing the functionality of AWS cloud controller manager and AWS EBS CSI driver + +## Inputs worth knowing about + +### Network exposure — `admin_cidrs` + +`admin_cidrs` lists the CIDRs allowed to reach the cluster from outside it: +SSH, WinRM over HTTPS, and the MKE and Kubernetes API ports. + +It defaults to `["0.0.0.0/0"]` — open to the whole internet. That is a real +risk, and it is the exposure this example has always had; it is the default so +that an existing `terraform.tfvars` keeps working and a first run needs no +extra arguments. Narrow it whenever you can: + +```hcl +admin_cidrs = ["198.51.100.17/32", "203.0.113.0/24"] +``` + +Node-to-node traffic does not pass through these rules, so narrowing this does +not affect the cluster talking to itself. + +### Credentials + +The MKE admin password (`launchpad.mke_connect.password`) and +`windows_password` are ordinary terraform variables. Setting them in +`terraform.tfvars` is all that is required, and the generated +`launchpad_yaml` output is usable as-is. + +To keep a password off disk, use terraform's own environment mechanism — +terraform reads any input variable from `TF_VAR_`: + +```sh +export TF_VAR_launchpad='{"drain":false,"mcr_version":"25.0.13","mke_version":"3.8.8","msr_version":"","mke_connect":{"username":"admin","password":"...","insecure":false},"skip_create":false,"skip_destroy":true}' +export TF_VAR_windows_password='...' +``` + +The MKE password is nested inside the `launchpad` object, so `TF_VAR_launchpad` +carries that whole object as JSON. Either route works; the environment is an +option for keeping secrets out of a file, not a requirement for running the +example. \ No newline at end of file diff --git a/examples/terraform/aws-complete/launchpad.tf b/examples/terraform/aws-complete/launchpad.tf index 09445f683..182e4673b 100644 --- a/examples/terraform/aws-complete/launchpad.tf +++ b/examples/terraform/aws-complete/launchpad.tf @@ -186,11 +186,22 @@ resource "launchpad_config" "cluster" { // ------- Ye old launchpad yaml (just for debugging) // -// Credentials are referenced by environment variable rather than inlined: -// launchpad expands them through envsubst when it loads the config, and -// docs/usage/getting-started.md tells users to source credentials from the -// environment. Export LAUNCHPAD_MKE_ADMIN_PASSWORD, and -// LAUNCHPAD_WINRM_PASSWORD for Windows stacks, before running launchpad. +// The credentials are written from the terraform variables that carry them: +// var.launchpad.mke_connect.password and var.windows_password. The document +// this produces is usable as-is, with nothing exported into the environment +// first. +// +// To keep a password out of a tfvars file on disk, supply the variable +// through terraform's own environment mechanism instead -- TF_VAR_, +// which terraform reads for any input variable: +// +// export TF_VAR_windows_password='...' +// export TF_VAR_launchpad='{"drain":false,...,"mke_connect":{"username":"admin","password":"...","insecure":true}}' +// +// TF_VAR_launchpad takes the whole object as JSON because the MKE password +// is nested inside it; test/terraform.go does exactly this. Either route +// produces the same configuration -- the environment is an option for +// keeping secrets off disk, not a requirement for running the example. output "launchpad_yaml" { description = "launchpad config file yaml (for debugging)" sensitive = true @@ -217,7 +228,7 @@ spec: winRM: address: ${h.winrm_address} user: ${h.winrm_user} - password: $${LAUNCHPAD_WINRM_PASSWORD} + password: ${h.winrm_password} useHTTPS: ${h.winrm_useHTTPS} insecure: ${h.winrm_insecure} %{~endfor} @@ -225,7 +236,7 @@ spec: version: ${var.launchpad.mke_version} imageRepo: docker.io/mirantis adminUsername: ${var.launchpad.mke_connect.username} - adminPassword: $${LAUNCHPAD_MKE_ADMIN_PASSWORD} + adminPassword: ${var.launchpad.mke_connect.password} installFlags: - "--san=${local.MKE_URL}" - "--nodeport-range=32768-35535" diff --git a/examples/terraform/aws-complete/terraform.tfvars.template b/examples/terraform/aws-complete/terraform.tfvars.template index 99e8d08e9..36d5fc113 100644 --- a/examples/terraform/aws-complete/terraform.tfvars.template +++ b/examples/terraform/aws-complete/terraform.tfvars.template @@ -2,14 +2,34 @@ //name = "test" // CIDRs allowed to reach the cluster from outside it (SSH, WinRM over HTTPS, -// MKE and the Kubernetes API). Required: there is no default, so that the -// previous allow-everything rules cannot come back silently. -admin_cidrs = ["203.0.113.4/32"] +// MKE and the Kubernetes API). +// +// Defaults to ["0.0.0.0/0"] -- open to the whole internet. That is a real +// risk, and it is the exposure this example has always had; it is left as the +// default so an existing tfvars keeps working and a first run needs no extra +// arguments. Narrow it whenever you can: list the machine running launchpad +// and the operators who need the MKE UI. Uncomment and edit: +// +// admin_cidrs = ["198.51.100.17/32", "203.0.113.0/24"] aws = { region = "us-east-2" } +// Credentials: both the MKE admin password below and windows_password at the +// bottom of this file are ordinary terraform variables, and setting them here +// is all that is needed. +// +// If you would rather not keep a password in this file, terraform reads any +// input variable from TF_VAR_ in the environment instead: +// +// export TF_VAR_launchpad='{"drain":false,"mcr_version":"25.0.13","mke_version":"3.8.8","msr_version":"","mke_connect":{"username":"admin","password":"...","insecure":false},"skip_create":false,"skip_destroy":true}' +// export TF_VAR_windows_password='...' +// +// The MKE password is nested inside the launchpad object, so TF_VAR_launchpad +// carries that whole object as JSON. Omit the launchpad block below if you +// use it. Either route works; the environment is optional. + launchpad = { drain = false @@ -59,4 +79,6 @@ nodegroups = { // set a windows password, if you have windows nodes // windows passwords must match a pattern, or connections will fail. // use something like: `testp@ss!` +// Required only when a nodegroup uses a windows_ platform; may also be +// supplied as TF_VAR_windows_password instead of being written here. // windows_password = "" diff --git a/examples/terraform/aws-complete/variables.tf b/examples/terraform/aws-complete/variables.tf index 8cce71e01..a66a19c1c 100644 --- a/examples/terraform/aws-complete/variables.tf +++ b/examples/terraform/aws-complete/variables.tf @@ -64,12 +64,26 @@ variable "extra_tags" { variable "admin_cidrs" { description = <<-EOT - CIDRs allowed to reach the cluster from outside it: SSH, WinRM over HTTPS, - and the MKE and Kubernetes API ports. Required and intentionally without - a default: the previous rule set accepted those ports from 0.0.0.0/0, and - a default would reintroduce that silently. + CIDRs allowed to reach the cluster from outside it: SSH, WinRM over + HTTPS, and the MKE and Kubernetes API ports. + + Open by default. ["0.0.0.0/0"] means every one of those ports is + reachable from the whole internet, which is a real risk and not a + recommendation -- it is the same exposure this example has always had, + kept so that an existing tfvars still works and a first run still comes + up without extra arguments. + + Narrowing it is one line: set admin_cidrs to the addresses that actually + need in -- the machine running launchpad, and the operators who need the + MKE UI. For example: + + admin_cidrs = ["198.51.100.17/32", "203.0.113.0/24"] + + Node-to-node traffic does not come through here, so narrowing this does + not affect the cluster talking to itself. EOT type = list(string) + default = ["0.0.0.0/0"] validation { condition = length(var.admin_cidrs) > 0 error_message = "admin_cidrs must list at least one CIDR." diff --git a/examples/terraform/aws-simple/README.md b/examples/terraform/aws-simple/README.md index 2677a3421..42c701034 100644 --- a/examples/terraform/aws-simple/README.md +++ b/examples/terraform/aws-simple/README.md @@ -8,3 +8,40 @@ ### Launchpad +#### Network exposure — `admin_cidrs` + +`admin_cidrs` lists the CIDRs allowed to reach the cluster from outside it: +SSH, WinRM over HTTPS, and the MKE/MSR and Kubernetes API ports. + +It defaults to `["0.0.0.0/0"]` — open to the whole internet. That is a real +risk, and it is the exposure this example has always had; it is the default so +that an existing `terraform.tfvars` keeps working and a first run needs no +extra arguments. Narrow it whenever you can: + +```hcl +admin_cidrs = ["198.51.100.17/32", "203.0.113.0/24"] +``` + +Node-to-node traffic does not pass through these rules, so narrowing this does +not affect the cluster talking to itself. + +#### Credentials + +The MKE admin password (`launchpad.mke_connect.password`) and +`windows_password` are ordinary terraform variables. Setting them in +`terraform.tfvars` is all that is required, and the generated +`launchpad_yaml` output is usable as-is. + +To keep a password off disk, use terraform's own environment mechanism — +terraform reads any input variable from `TF_VAR_`: + +```sh +export TF_VAR_launchpad='{"drain":false,"mcr_channel":"stable-25.0.14","mke_version":"3.8.8","msr_version":"2.9.28","mke_connect":{"username":"admin","password":"...","insecure":true},"skip_create":false,"skip_destroy":true}' +export TF_VAR_windows_password='...' +``` + +The MKE password is nested inside the `launchpad` object, so `TF_VAR_launchpad` +carries that whole object as JSON. Either route works; the environment is an +option for keeping secrets out of a file, not a requirement for running the +example. + diff --git a/examples/terraform/aws-simple/launchpad.tf b/examples/terraform/aws-simple/launchpad.tf index dcd75c4ed..2e9a671bc 100644 --- a/examples/terraform/aws-simple/launchpad.tf +++ b/examples/terraform/aws-simple/launchpad.tf @@ -173,16 +173,22 @@ locals { // ------- Ye old launchpad yaml (just for debugging) // -// The credentials are referenced by environment variable rather than written -// into the document. launchpad expands these through envsubst when it loads -// the config (pkg/config), and docs/usage/getting-started.md tells users to -// source credentials from the environment; a generated file that inlines them -// contradicts that and leaves the password in every copy of the YAML, in -// terraform state, and in anything that prints either. The consumer exports -// LAUNCHPAD_MKE_ADMIN_PASSWORD and, for Windows stacks, -// LAUNCHPAD_WINRM_PASSWORD before running launchpad; an unset variable fails -// config loading loudly rather than silently installing with an empty -// password (PRODENG-3751). +// The credentials are written from the terraform variables that carry them: +// var.launchpad.mke_connect.password and var.windows_password. The document +// this produces is usable as-is, with nothing exported into the environment +// first. +// +// To keep a password out of a tfvars file on disk, supply the variable +// through terraform's own environment mechanism instead -- TF_VAR_, +// which terraform reads for any input variable: +// +// export TF_VAR_windows_password='...' +// export TF_VAR_launchpad='{"drain":false,...,"mke_connect":{"username":"admin","password":"...","insecure":true}}' +// +// TF_VAR_launchpad takes the whole object as JSON because the MKE password +// is nested inside it; test/terraform.go does exactly this. Either route +// produces the same configuration -- the environment is an option for +// keeping secrets off disk, not a requirement for running the example. locals { launchpad_yaml = <<-EOT apiVersion: launchpad.mirantis.com/mke/v1.6 @@ -207,7 +213,7 @@ spec: winRM: address: ${h.winrm_address} user: ${h.winrm_user} - password: $${LAUNCHPAD_WINRM_PASSWORD} + password: ${h.winrm_password} useHTTPS: ${h.winrm_useHTTPS} insecure: ${h.winrm_insecure} %{~endfor} @@ -220,7 +226,7 @@ spec: version: ${var.launchpad.mke_version} imageRepo: docker.io/mirantis adminUsername: ${var.launchpad.mke_connect.username} - adminPassword: $${LAUNCHPAD_MKE_ADMIN_PASSWORD} + adminPassword: ${var.launchpad.mke_connect.password} installFlags: - "--san=${local.MKE_URL}" - "--default-node-orchestrator=kubernetes" diff --git a/examples/terraform/aws-simple/terraform.tfvars.template b/examples/terraform/aws-simple/terraform.tfvars.template index cc391a7b6..701635508 100644 --- a/examples/terraform/aws-simple/terraform.tfvars.template +++ b/examples/terraform/aws-simple/terraform.tfvars.template @@ -2,15 +2,34 @@ //name = "" // CIDRs allowed to reach the cluster from outside it (SSH, WinRM over HTTPS, -// MKE/MSR and the Kubernetes API). Required: there is no default, so that the -// previous allow-everything rule cannot come back silently. Narrow this to the -// machine running launchpad and the operators who need the UI. -admin_cidrs = ["203.0.113.4/32"] +// MKE/MSR and the Kubernetes API). +// +// Defaults to ["0.0.0.0/0"] -- open to the whole internet. That is a real +// risk, and it is the exposure this example has always had; it is left as the +// default so an existing tfvars keeps working and a first run needs no extra +// arguments. Narrow it whenever you can: list the machine running launchpad +// and the operators who need the MKE UI. Uncomment and edit: +// +// admin_cidrs = ["198.51.100.17/32", "203.0.113.0/24"] aws = { region = "us-east-2" } +// Credentials: both the MKE admin password below and windows_password at the +// bottom of this file are ordinary terraform variables, and setting them here +// is all that is needed. +// +// If you would rather not keep a password in this file, terraform reads any +// input variable from TF_VAR_ in the environment instead: +// +// export TF_VAR_launchpad='{"drain":false,"mcr_channel":"stable-25.0.14","mke_version":"3.8.8","msr_version":"2.9.28","mke_connect":{"username":"admin","password":"...","insecure":true},"skip_create":false,"skip_destroy":true}' +// export TF_VAR_windows_password='...' +// +// The MKE password is nested inside the launchpad object, so TF_VAR_launchpad +// carries that whole object as JSON. Omit the launchpad block below if you +// use it. Either route works; the environment is optional. + launchpad = { drain = false @@ -81,4 +100,6 @@ nodegroups = { // set a windows password, if you have windows nodes // windows passwords must match a pattern, or connections will fail. // use something like: `testp@ss!` +// Required only when a nodegroup uses a windows_ platform; may also be +// supplied as TF_VAR_windows_password instead of being written here. // windows_password = "" diff --git a/examples/terraform/aws-simple/variables.tf b/examples/terraform/aws-simple/variables.tf index aa1641987..4c86f4867 100644 --- a/examples/terraform/aws-simple/variables.tf +++ b/examples/terraform/aws-simple/variables.tf @@ -71,12 +71,25 @@ variable "extra_tags" { variable "admin_cidrs" { description = <<-EOT CIDRs allowed to reach the cluster from outside it: SSH, WinRM, and the - MKE/MSR and Kubernetes API ports. Required and intentionally without a - default: the previous rule set accepted every port from 0.0.0.0/0, and a - default would reintroduce that silently. Supply the narrowest set that - covers the machine running launchpad and the operators who need the UI. + MKE/MSR and Kubernetes API ports. + + Open by default. ["0.0.0.0/0"] means every one of those ports is + reachable from the whole internet, which is a real risk and not a + recommendation -- it is the same exposure this example has always had, + kept so that an existing tfvars still works and a first run still comes + up without extra arguments. + + Narrowing it is one line: set admin_cidrs to the addresses that actually + need in -- the machine running launchpad, and the operators who need the + MKE UI. For example: + + admin_cidrs = ["198.51.100.17/32", "203.0.113.0/24"] + + Node-to-node traffic does not come through here, so narrowing this does + not affect the cluster talking to itself. EOT type = list(string) + default = ["0.0.0.0/0"] validation { condition = length(var.admin_cidrs) > 0 error_message = "admin_cidrs must list at least one CIDR." diff --git a/test/integration/integration_test.go b/test/integration/integration_test.go index 355c3ecad..f16bd8c84 100644 --- a/test/integration/integration_test.go +++ b/test/integration/integration_test.go @@ -82,12 +82,6 @@ func TestMain(m *testing.M) { mkePassword := test.GenerateRandomAlphaNumericString(12) MKE_CONNECT["password"] = mkePassword - // The generated YAML refers to the MKE password by environment variable - // rather than inlining it; launchpad resolves it through envsubst when - // sp.Setup loads the config. This stack has no Windows nodes, so no WinRM - // reference is generated. - test.ExportGeneratedCredentials(t, mkePassword, "") - options := test.TerraformOptions( t, // The path to where the Terraform tf chart is located diff --git a/test/smoke/smoke_test.go b/test/smoke/smoke_test.go index 1169a24d4..c542f0538 100644 --- a/test/smoke/smoke_test.go +++ b/test/smoke/smoke_test.go @@ -191,10 +191,6 @@ func runSmokeTest(t *testing.T, cfg smokeConfig) { secretVars["windows_password"] = windowsPassword } - // The generated YAML refers to these by environment variable rather than - // inlining them; launchpad resolves them through envsubst at load. - test.ExportGeneratedCredentials(t, mkePassword, windowsPassword) - options := test.TerraformOptions(t, "../../examples/terraform/aws-simple", vars, secretVars) terraformOptions := terraform.WithDefaultRetryableErrors(t, &options) diff --git a/test/smoke/swarm_addr_pool_test.go b/test/smoke/swarm_addr_pool_test.go index 7eda92cb2..c0e398052 100644 --- a/test/smoke/swarm_addr_pool_test.go +++ b/test/smoke/swarm_addr_pool_test.go @@ -196,11 +196,6 @@ func provisionSwarmPoolStack(t *testing.T, cfg smokeConfig) string { }, } - // The generated YAML refers to the MKE password by environment variable - // rather than inlining it; launchpad resolves it through envsubst at load. - // This stack has no Windows nodes, so no WinRM reference is generated. - test.ExportGeneratedCredentials(t, mkePassword, "") - options := test.TerraformOptions(t, "../../examples/terraform/aws-simple", vars, secretVars) terraformOptions := terraform.WithDefaultRetryableErrors(t, &options) diff --git a/test/smoke/upgrade_test.go b/test/smoke/upgrade_test.go index 12a757155..0aca50040 100644 --- a/test/smoke/upgrade_test.go +++ b/test/smoke/upgrade_test.go @@ -107,11 +107,6 @@ func runUpgradeTest(t *testing.T, cfg upgradeConfig) { "launchpad": launchpad, } - // The generated YAML refers to the MKE password by environment variable - // rather than inlining it; launchpad resolves it through envsubst at load. - // This stack has no Windows nodes, so no WinRM reference is generated. - test.ExportGeneratedCredentials(t, mkePassword, "") - options := test.TerraformOptions(t, "../../examples/terraform/aws-simple", vars, secretVars) terraformOptions := terraform.WithDefaultRetryableErrors(t, &options) diff --git a/test/terraform.go b/test/terraform.go index 20adcaffa..e4c4a109f 100644 --- a/test/terraform.go +++ b/test/terraform.go @@ -10,23 +10,13 @@ import ( "github.com/gruntwork-io/terratest/modules/terraform" ) -// Environment variables the examples' generated launchpad.yaml refers to -// instead of inlining credentials. launchpad expands them through envsubst -// when it loads the config, so a consumer of that YAML must export them -// before parsing it or config loading fails on an unset reference. -const ( - MKEAdminPasswordEnv = "LAUNCHPAD_MKE_ADMIN_PASSWORD" - WinRMPasswordEnv = "LAUNCHPAD_WINRM_PASSWORD" -) - // AdminCIDRs returns the value for the examples' admin_cidrs variable, which // scopes SSH, WinRM and the MKE/Kubernetes API to named networks. // -// The variable is required and has no default, so that narrowing it is a -// decision a caller makes rather than something a module silently decides. -// These suites provision throwaway stacks from runners whose egress address -// is not known ahead of time, so the fallback is 0.0.0.0/0: the same exposure -// the examples had before, but stated at the caller where it can be found and +// The variable defaults to 0.0.0.0/0 in the examples. These suites provision +// throwaway stacks from runners whose egress address is not known ahead of +// time, so they pass that same open range explicitly rather than inheriting +// it, which keeps the exposure stated at the caller where it can be found and // changed. Set SMOKE_ADMIN_CIDRS (comma separated) to narrow it. func AdminCIDRs(t *testing.T) []string { t.Helper() @@ -46,17 +36,6 @@ func AdminCIDRs(t *testing.T) []string { return cidrs } -// ExportGeneratedCredentials publishes the credentials the generated -// launchpad.yaml refers to, so that envsubst resolves them when the config is -// loaded. Pass an empty windowsPassword for a stack with no Windows nodes; -// the reference is only present in the document when a WinRM host is. -func ExportGeneratedCredentials(t *testing.T, mkePassword, windowsPassword string) { - t.Helper() - - t.Setenv(MKEAdminPasswordEnv, mkePassword) - t.Setenv(WinRMPasswordEnv, windowsPassword) -} - // TerraformOptions builds terraform.Options that keep credentials out of the // terraform command line. // diff --git a/test/terraform_test.go b/test/terraform_test.go index 720b69548..cab9a03ff 100644 --- a/test/terraform_test.go +++ b/test/terraform_test.go @@ -6,7 +6,6 @@ import ( "os" "os/exec" "path/filepath" - "regexp" "strings" "sync" "testing" @@ -198,38 +197,6 @@ func copyFixture(t *testing.T, fixtureDir string) string { return dest } -// TestGeneratedCredentialReferencesMatchExports pins the contract between the -// terraform examples and these suites: the examples emit environment-variable -// references in the launchpad.yaml they generate, and the suites export those -// variables before the YAML is parsed. A rename on either side leaves a -// config whose credentials expand to nothing, which only a live cluster would -// otherwise reveal. -func TestGeneratedCredentialReferencesMatchExports(t *testing.T) { - exported := map[string]bool{ - MKEAdminPasswordEnv: true, - WinRMPasswordEnv: true, - } - - // The examples escape "$" as "$$" for terraform's own template syntax, so - // the emitted document contains "${NAME}". - reference := regexp.MustCompile(`\$\$\{([A-Z0-9_]+)\}`) - - for _, example := range []string{"aws-simple", "aws-complete"} { - path := filepath.Join("..", "examples", "terraform", example, "launchpad.tf") - - content, err := os.ReadFile(path) // #nosec G304 -- path is a test constant - require.NoError(t, err) - - matches := reference.FindAllStringSubmatch(string(content), -1) - require.NotEmpty(t, matches, "%s emits no credential references: it may have gone back to inlining them", path) - - for _, match := range matches { - assert.True(t, exported[match[1]], - "%s refers to %s, which no suite exports; the generated config would load it as empty", path, match[1]) - } - } -} - func TestAdminCIDRs(t *testing.T) { t.Run("falls back to the open range when unset", func(t *testing.T) { t.Setenv("SMOKE_ADMIN_CIDRS", "")