From 33128c18154ff49876e7a0ea112ddf4e6e22dd9d Mon Sep 17 00:00:00 2001 From: Gonzalo Rojas Date: Thu, 27 Aug 2026 11:18:33 -0300 Subject: [PATCH] fix: pin every moving version default Four defaults in this line resolved at deploy time rather than at apply time, so a pod restart could pull a different build with no change on our side: - eks traffic_manager_version was "latest" - agent_repos_scope pointed at scopes.git#main - cert_manager_version was declared but never wired to its helm_release, so installs tracked whatever charts.jetstack.io served while the README showed a number. grep found one occurrence of the variable: its own declaration. - prometheus had no version argument and no variable for one, so it could not be pinned without editing the module. Each default is now the version it currently resolved to, so behaviour is unchanged today and stops drifting tomorrow. Nothing is required and nothing is removed: every caller that already passes a value is unaffected. The scopes ref is the exception worth reading twice. main has moved past v1.15.1, so pinning the tag steps back to the last tagged release rather than holding today's branch tip. agent_traffic_manager_tag is new, with a default. Pinning the traffic manager meant passing the whole image string through extra_envs; only the tag is exposed now, and extra_envs still wins the merge so the old way keeps working. The agent module had no outputs and no tests, so its rendered values could not be asserted on. It now exports rendered_values and agent_repos, matching what nullplatform/base already does. --- infrastructure/commons/cert_manager/main.tf | 1 + .../tests/cert_manager_aws.tftest.hcl | 16 ++++ .../commons/cert_manager/variables.tf | 4 +- .../commons/prometheus/.terraform.lock.hcl | 25 +++++++ infrastructure/commons/prometheus/main.tf | 1 + .../prometheus/tests/prometheus.tftest.hcl | 30 ++++++++ .../commons/prometheus/variables.tf | 6 ++ nullplatform/agent/locals.tf | 1 + nullplatform/agent/outputs.tf | 14 ++++ .../agent/tests/agent_values.tftest.hcl | 74 +++++++++++++++++++ nullplatform/agent/variables.tf | 11 ++- .../eks/tests/eks.tftest.hcl | 23 +++++- .../container_orchestration/eks/variables.tf | 4 +- 13 files changed, 204 insertions(+), 6 deletions(-) create mode 100644 infrastructure/commons/prometheus/.terraform.lock.hcl create mode 100644 infrastructure/commons/prometheus/tests/prometheus.tftest.hcl create mode 100644 nullplatform/agent/outputs.tf create mode 100644 nullplatform/agent/tests/agent_values.tftest.hcl diff --git a/infrastructure/commons/cert_manager/main.tf b/infrastructure/commons/cert_manager/main.tf index 284dbc7e9..5ec6eda99 100644 --- a/infrastructure/commons/cert_manager/main.tf +++ b/infrastructure/commons/cert_manager/main.tf @@ -3,6 +3,7 @@ resource "helm_release" "cert_manager" { repository = "https://charts.jetstack.io" chart = "cert-manager" namespace = var.cert_manager_namespace + version = var.cert_manager_version create_namespace = true disable_webhooks = false diff --git a/infrastructure/commons/cert_manager/tests/cert_manager_aws.tftest.hcl b/infrastructure/commons/cert_manager/tests/cert_manager_aws.tftest.hcl index d7b52522f..8d0822f4a 100644 --- a/infrastructure/commons/cert_manager/tests/cert_manager_aws.tftest.hcl +++ b/infrastructure/commons/cert_manager/tests/cert_manager_aws.tftest.hcl @@ -100,3 +100,19 @@ run "rejects_invalid_aws_identity_mode" { expect_failures = [var.aws_identity_mode] } + +################################################################################ +# Version pinning +################################################################################ + +# cert_manager_version was declared with a default and never referenced: grep found one +# occurrence, its own declaration. The helm_release had no version argument, so installs +# tracked whatever charts.jetstack.io served while the README showed a number. +run "cert_manager_version_reaches_the_release" { + command = plan + + assert { + condition = helm_release.cert_manager.version == "v1.21.1" + error_message = "cert_manager_version must be wired to the helm_release, not merely declared" + } +} diff --git a/infrastructure/commons/cert_manager/variables.tf b/infrastructure/commons/cert_manager/variables.tf index 0ed891cab..c3552adf3 100644 --- a/infrastructure/commons/cert_manager/variables.tf +++ b/infrastructure/commons/cert_manager/variables.tf @@ -62,9 +62,9 @@ variable "private_domain_name" { ############################################################################### variable "cert_manager_version" { - description = "The version of cert-manager Helm chart to deploy" + description = "The version of cert-manager Helm chart to deploy. Was declared but never wired to the helm_release, so installs tracked whatever the chart repository served; the default is the version that resolved to as of 2026-08-27, which keeps behaviour unchanged while removing the drift." type = string - default = "1.18.2" + default = "v1.21.1" } variable "cert_manager_namespace" { diff --git a/infrastructure/commons/prometheus/.terraform.lock.hcl b/infrastructure/commons/prometheus/.terraform.lock.hcl new file mode 100644 index 000000000..89971bbf5 --- /dev/null +++ b/infrastructure/commons/prometheus/.terraform.lock.hcl @@ -0,0 +1,25 @@ +# This file is maintained automatically by "tofu init". +# Manual edits may be lost in future updates. + +provider "registry.opentofu.org/hashicorp/helm" { + version = "3.2.0" + constraints = "~> 3.0" + hashes = [ + "h1:thNSoWm4pdgTEO1XBi1n5V2nwsdSA6EVEXP3qPZiFcA=", + "zh:1a214581dee54ec4e9afa4050e54f6c187aed4b51b2d2ac929c58706b65e1159", + "zh:2f8ba94af93011768ed1fffd4d25b980cd764f2d49c9f13475512ec48464da0b", + "zh:36373bca4f374e95f654def79e0b12df0c8f2e01c634db80ff2737ded29062e3", + "zh:3a26b5c3e47b2bbc01faa0fa9fe816ea4f74f1f8f4555dfad7f62ee38266aaff", + "zh:51bc637700f13cfc1f7c6a8c03a4b5b5755338419912d945dbfe5ccb0ddbf614", + "zh:53f32f91afcb682209de124d8a994e591687023ae7c0dfed6184c5511778b16b", + "zh:6f4434327ed466b2b5be0d5f4aa537bca71ebd7f7fe05468065aeda52dfc8896", + "zh:7394e8c6f5027fa21699e46c9a45bee1ce87fd8dd98f2e89e8d414ec70c8e4e1", + "zh:d90b855a0990e3aa6445afe66de730c2f4651f39699c6b6345ca02f9af2a1a08", + "zh:d9b7246e6af0f75155ed532855014f888e9e5613242c89e321b4e0f9b54f5726", + "zh:dabd399ca36172c15d176a24cf199ac886ef191f7131e806d76a8dc209e5beb8", + "zh:e57987397be46dc123365f16c3bec4dd0615453c8bfdcfc1b7c9f3202a09c516", + "zh:ec88430b833b943b38d02f70b33250c72bb6ffa3b3d22360990a41390161a2b2", + "zh:f8ea01b57982e9ed9ad3a750b1caba261a478f5b0ddcac78a4502d886fd2fc74", + "zh:fb63819037158205ebf42b649c3a8ec308234ffe987a5dbd4444e1e0683f1170", + ] +} diff --git a/infrastructure/commons/prometheus/main.tf b/infrastructure/commons/prometheus/main.tf index e5f5e247e..d6abb7440 100644 --- a/infrastructure/commons/prometheus/main.tf +++ b/infrastructure/commons/prometheus/main.tf @@ -3,6 +3,7 @@ resource "helm_release" "prometheus" { repository = "https://prometheus-community.github.io/helm-charts" chart = "prometheus" namespace = var.prometheus_namespace + version = var.prometheus_version create_namespace = true disable_webhooks = false diff --git a/infrastructure/commons/prometheus/tests/prometheus.tftest.hcl b/infrastructure/commons/prometheus/tests/prometheus.tftest.hcl new file mode 100644 index 000000000..4a7827296 --- /dev/null +++ b/infrastructure/commons/prometheus/tests/prometheus.tftest.hcl @@ -0,0 +1,30 @@ +mock_provider "helm" {} + +################################################################################ +# Version pinning +################################################################################ + +# The helm_release carried no version argument and the module exposed no variable for one, +# so every apply resolved to whatever prometheus-community served latest, with no diff to +# review and no way to pin without editing the module. +run "prometheus_version_reaches_the_release" { + command = plan + + assert { + condition = helm_release.prometheus.version == "29.27.0" + error_message = "prometheus_version must be wired to the helm_release so the deployed chart is a decision" + } +} + +run "prometheus_version_is_overridable" { + command = plan + + variables { + prometheus_version = "29.26.0" + } + + assert { + condition = helm_release.prometheus.version == "29.26.0" + error_message = "prometheus_version must be settable by the caller" + } +} diff --git a/infrastructure/commons/prometheus/variables.tf b/infrastructure/commons/prometheus/variables.tf index 502083c4c..630bcc1d3 100644 --- a/infrastructure/commons/prometheus/variables.tf +++ b/infrastructure/commons/prometheus/variables.tf @@ -1,3 +1,9 @@ +variable "prometheus_version" { + description = "Helm chart version for the prometheus-community/prometheus chart. The helm_release carried no version at all, so every apply resolved to whatever the repository served latest; the default is the version that resolved to as of 2026-08-27, which keeps behaviour unchanged while removing the drift." + type = string + default = "29.27.0" +} + variable "nullplatform_port" { description = "Port number for nullplatform service communication" type = number diff --git a/nullplatform/agent/locals.tf b/nullplatform/agent/locals.tf index 1a039b8ba..7674adb22 100644 --- a/nullplatform/agent/locals.tf +++ b/nullplatform/agent/locals.tf @@ -42,6 +42,7 @@ locals { CLUSTER_NAME = var.cluster_name NAMESPACE = var.namespace IMAGE_TAG = var.image_tag + TRAFFIC_CONTAINER_IMAGE = "public.ecr.aws/nullplatform/k8s-traffic-manager:${var.agent_traffic_manager_tag}" DOMAIN = var.domain DNS_TYPE = var.dns_type USE_ACCOUNT_SLUG = var.use_account_slug diff --git a/nullplatform/agent/outputs.tf b/nullplatform/agent/outputs.tf new file mode 100644 index 000000000..e4d8f17a0 --- /dev/null +++ b/nullplatform/agent/outputs.tf @@ -0,0 +1,14 @@ +############################################ +# Rendered Helm Values (used in tests) +############################################ + +output "rendered_values" { + description = "The rendered Helm values passed to the agent chart." + value = local.nullplatform_agent_values + sensitive = true +} + +output "agent_repos" { + description = "The comma-separated repository list handed to the agent's git command executor." + value = local.agent_repos +} diff --git a/nullplatform/agent/tests/agent_values.tftest.hcl b/nullplatform/agent/tests/agent_values.tftest.hcl new file mode 100644 index 000000000..e4075a05d --- /dev/null +++ b/nullplatform/agent/tests/agent_values.tftest.hcl @@ -0,0 +1,74 @@ +mock_provider "helm" {} +mock_provider "nullplatform" {} + +variables { + api_key = "test-api-key" + cluster_name = "my-cluster" + tags_selectors = { dimension = "prod" } + cloud_provider = "aws" + aws_iam_role_arn = "arn:aws:iam::123456789012:role/agent" + image_tag = "0.9.2" +} + +################################################################################ +# Traffic manager image +################################################################################ + +# Pinning the traffic manager used to mean passing the whole image string through +# extra_envs. The registry now lives in the module and only the tag is exposed. +run "traffic_manager_image_is_assembled_from_the_tag" { + command = plan + + assert { + condition = strcontains(output.rendered_values, "TRAFFIC_CONTAINER_IMAGE: \"public.ecr.aws/nullplatform/k8s-traffic-manager:1.8.0\"") + error_message = "TRAFFIC_CONTAINER_IMAGE should be built from the repository default and the pinned tag" + } +} + +run "extra_envs_still_overrides_the_traffic_manager_image" { + command = plan + + variables { + extra_envs = { + TRAFFIC_CONTAINER_IMAGE = "public.ecr.aws/nullplatform/k8s-traffic-manager@sha256:abc123" + } + } + + # extra_envs is merged last, so the previous way of doing this keeps working. That is what + # makes exposing the tag an addition rather than a breaking change. + assert { + condition = strcontains(output.rendered_values, "TRAFFIC_CONTAINER_IMAGE: \"public.ecr.aws/nullplatform/k8s-traffic-manager@sha256:abc123\"") + error_message = "extra_envs must keep precedence over the assembled image" + } +} + +################################################################################ +# Scope repository +################################################################################ + +run "scope_repo_is_pinned_to_a_tag" { + command = plan + + assert { + condition = !strcontains(output.agent_repos, "#main") + error_message = "the scope repo default must not point at a moving branch" + } + + assert { + condition = strcontains(output.agent_repos, "scopes.git#v1.15.1") + error_message = "the scope repo default should be pinned to the released tag" + } +} + +run "scope_repo_is_overridable" { + command = plan + + variables { + agent_repos_scope = "https://github.com/nullplatform/scopes.git#v1.14.0" + } + + assert { + condition = strcontains(output.agent_repos, "scopes.git#v1.14.0") + error_message = "callers must still be able to choose their own ref" + } +} diff --git a/nullplatform/agent/variables.tf b/nullplatform/agent/variables.tf index 105d1d2e2..f62ab81f5 100644 --- a/nullplatform/agent/variables.tf +++ b/nullplatform/agent/variables.tf @@ -95,10 +95,19 @@ variable "namespace" { variable "agent_repos_scope" { description = "Git repository URL containing agent scope configurations (format: repo#branch)" type = string - default = "https://github.com/nullplatform/scopes.git#main" + # Pinned to the last tagged release rather than the branch tip. main has moved past + # v1.15.1, so this is a deliberate step back to a ref that cannot change under a running + # agent -- see VERSIONS.md. + default = "https://github.com/nullplatform/scopes.git#v1.15.1" } # List of additional Git repositories used for extended agent configuration +variable "agent_traffic_manager_tag" { + description = "Image tag for the traffic manager, published to the agent as TRAFFIC_CONTAINER_IMAGE. Pinning this used to mean passing the whole image string through extra_envs; the registry lives here so only the tag is exposed. extra_envs still takes precedence for anyone who needs a digest or a mirrored path." + type = string + default = "1.8.0" +} + variable "agent_repos_extra" { description = "List of additional Git repositories used for extended agent configuration" type = list(string) diff --git a/nullplatform/container_orchestration/eks/tests/eks.tftest.hcl b/nullplatform/container_orchestration/eks/tests/eks.tftest.hcl index fb53d0ae5..ae3f9d3cc 100644 --- a/nullplatform/container_orchestration/eks/tests/eks.tftest.hcl +++ b/nullplatform/container_orchestration/eks/tests/eks.tftest.hcl @@ -216,7 +216,7 @@ run "with_traffic_manager_port" { } assert { - condition = strcontains(nullplatform_provider_config.eks_config.attributes, "\"version\":\"latest\"") + condition = strcontains(nullplatform_provider_config.eks_config.attributes, "\"version\":\"1.8.0\"") error_message = "Setting the port must not drop the traffic manager version" } } @@ -342,3 +342,24 @@ run "with_all_options" { error_message = "Dimensions should contain Environment=staging" } } + +################################################################################ +# Version pinning +################################################################################ + +# The default was "latest", so an apply with no code change could move the deployed +# traffic manager. Nothing in the module surface changed -- callers who already pass a +# version are unaffected -- but the default no longer drifts. +run "traffic_manager_version_default_is_pinned" { + command = plan + + assert { + condition = strcontains(nullplatform_provider_config.eks_config.attributes, "\"version\":\"1.8.0\"") + error_message = "the default traffic manager version must be a fixed release, not latest" + } + + assert { + condition = !strcontains(nullplatform_provider_config.eks_config.attributes, "latest") + error_message = "no attribute may reference a moving tag" + } +} diff --git a/nullplatform/container_orchestration/eks/variables.tf b/nullplatform/container_orchestration/eks/variables.tf index 4ac57957b..4a51ff449 100644 --- a/nullplatform/container_orchestration/eks/variables.tf +++ b/nullplatform/container_orchestration/eks/variables.tf @@ -112,9 +112,9 @@ variable "service_account_name" { } variable "traffic_manager_version" { - description = "Tag for the traffic manager sidecar container" + description = "Pinned rather than tracking latest: a moving tag means a pod restart can pull a different build with no apply in between. Tag for the traffic manager sidecar container" type = string - default = "latest" + default = "1.8.0" } variable "traffic_manager_port" {