diff --git a/deploy/Chart/templates/dynamic-rp/deployment.yaml b/deploy/Chart/templates/dynamic-rp/deployment.yaml index 8fad1f55f5..43165c3d83 100644 --- a/deploy/Chart/templates/dynamic-rp/deployment.yaml +++ b/deploy/Chart/templates/dynamic-rp/deployment.yaml @@ -1,4 +1,7 @@ {{- $appversion := include "radius.versiontag" . }} +{{- if ne .Values.dynamicrp.terraform.path "/terraform" }} +{{- fail "dynamicrp.terraform.path must be \"/terraform\" -- the runtime configmap (deploy/Chart/templates/dynamic-rp/configmaps.yaml) and pkg/recipes/terraform/install.go both hard-code /terraform, so overriding the mount path produces an inconsistent deployment and silently breaks Terraform execution/caching." }} +{{- end }} apiVersion: apps/v1 kind: Deployment metadata: @@ -86,17 +89,20 @@ spec: # Determine download URL TERRAFORM_URL="{{ .Values.global.terraform.downloadUrl }}" - - # If no custom URL provided, fetch latest version from HashiCorp API + + # If no custom URL provided, build the URL from the pinned version. + # Version MUST match terraformVersion in + # pkg/recipes/terraform/version.go so the pre-mounted binary + # and the runtime download fallback agree -- otherwise recipes + # behave differently between cold and warm starts. if [[ -z "$TERRAFORM_URL" ]]; then - echo "Fetching latest Terraform version from HashiCorp API..." - LATEST_VERSION=$(wget -qO- "https://api.releases.hashicorp.com/v1/releases/terraform/latest" | grep -o '"version":"[^"]*"' | cut -d'"' -f4) - if [[ -z "$LATEST_VERSION" ]]; then - echo "ERROR: Failed to fetch latest Terraform version from API" + TERRAFORM_VERSION="{{ .Values.global.terraform.version }}" + if [[ -z "$TERRAFORM_VERSION" ]]; then + echo "ERROR: global.terraform.version is empty and no global.terraform.downloadUrl is set" exit 2 fi - echo "Latest Terraform version: $LATEST_VERSION" - TERRAFORM_URL="https://releases.hashicorp.com/terraform/${LATEST_VERSION}/terraform_${LATEST_VERSION}_linux_${TERRAFORM_ARCH}.zip" + echo "Pinned Terraform version: $TERRAFORM_VERSION" + TERRAFORM_URL="https://releases.hashicorp.com/terraform/${TERRAFORM_VERSION}/terraform_${TERRAFORM_VERSION}_linux_${TERRAFORM_ARCH}.zip" fi echo "Download URL: $TERRAFORM_URL" @@ -116,10 +122,23 @@ spec: unzip terraform.zip || { echo "ERROR: Failed to extract terraform"; exit 5; } echo "Installing terraform binary..." + # Path/marker MUST match constants in + # pkg/recipes/terraform/install.go (defaultGlobalTerraformBinary, + # defaultGlobalMarkerFile). The runtime hard-codes + # /terraform/.terraform-global, so this path is hard-coded too -- + # do NOT derive it from .Values.dynamicrp.terraform.path or + # overrides to that value will silently reintroduce the + # path-mismatch bug this fix addresses. + GLOBAL_DIR="/terraform/.terraform-global" + mkdir -p "$GLOBAL_DIR" || { echo "ERROR: Failed to create global terraform directory"; exit 6; } + cp terraform "$GLOBAL_DIR/terraform" || { echo "ERROR: Failed to copy terraform"; exit 6; } + chmod +x "$GLOBAL_DIR/terraform" || { echo "ERROR: Failed to make terraform executable"; exit 7; } + touch "$GLOBAL_DIR/.terraform-ready" || { echo "ERROR: Failed to create terraform-ready marker"; exit 8; } + + # Keep a copy at the legacy path and write the legacy marker so any + # tooling that still references {{ .Values.dynamicrp.terraform.path }}/terraform continues to work. cp terraform "{{ .Values.dynamicrp.terraform.path }}/terraform" || { echo "ERROR: Failed to copy terraform"; exit 6; } chmod +x "{{ .Values.dynamicrp.terraform.path }}/terraform" || { echo "ERROR: Failed to make terraform executable"; exit 7; } - - # Create marker file to indicate pre-mounted binary is available echo "pre-mounted" > "{{ .Values.dynamicrp.terraform.path }}/.terraform-source" echo "Terraform binary successfully pre-downloaded and installed" diff --git a/deploy/Chart/templates/rp/deployment.yaml b/deploy/Chart/templates/rp/deployment.yaml index c015d9c2d7..bae66529c1 100644 --- a/deploy/Chart/templates/rp/deployment.yaml +++ b/deploy/Chart/templates/rp/deployment.yaml @@ -1,4 +1,7 @@ {{- $appversion := include "radius.versiontag" . }} +{{- if ne .Values.rp.terraform.path "/terraform" }} +{{- fail "rp.terraform.path must be \"/terraform\" -- the runtime configmap (deploy/Chart/templates/rp/configmaps.yaml) and pkg/recipes/terraform/install.go both hard-code /terraform, so overriding the mount path produces an inconsistent deployment and silently breaks Terraform execution/caching." }} +{{- end }} apiVersion: apps/v1 kind: Deployment metadata: @@ -86,17 +89,20 @@ spec: # Determine download URL TERRAFORM_URL="{{ .Values.global.terraform.downloadUrl }}" - - # If no custom URL provided, fetch latest version from HashiCorp API + + # If no custom URL provided, build the URL from the pinned version. + # Version MUST match terraformVersion in + # pkg/recipes/terraform/version.go so the pre-mounted binary + # and the runtime download fallback agree -- otherwise recipes + # behave differently between cold and warm starts. if [[ -z "$TERRAFORM_URL" ]]; then - echo "Fetching latest Terraform version from HashiCorp API..." - LATEST_VERSION=$(wget -qO- "https://api.releases.hashicorp.com/v1/releases/terraform/latest" | grep -o '"version":"[^"]*"' | cut -d'"' -f4) - if [[ -z "$LATEST_VERSION" ]]; then - echo "ERROR: Failed to fetch latest Terraform version from API" + TERRAFORM_VERSION="{{ .Values.global.terraform.version }}" + if [[ -z "$TERRAFORM_VERSION" ]]; then + echo "ERROR: global.terraform.version is empty and no global.terraform.downloadUrl is set" exit 2 fi - echo "Latest Terraform version: $LATEST_VERSION" - TERRAFORM_URL="https://releases.hashicorp.com/terraform/${LATEST_VERSION}/terraform_${LATEST_VERSION}_linux_${TERRAFORM_ARCH}.zip" + echo "Pinned Terraform version: $TERRAFORM_VERSION" + TERRAFORM_URL="https://releases.hashicorp.com/terraform/${TERRAFORM_VERSION}/terraform_${TERRAFORM_VERSION}_linux_${TERRAFORM_ARCH}.zip" fi echo "Download URL: $TERRAFORM_URL" @@ -122,10 +128,23 @@ spec: unzip terraform.zip || { echo "ERROR: Failed to extract terraform"; exit 5; } echo "Installing terraform binary..." + # Path/marker MUST match constants in + # pkg/recipes/terraform/install.go (defaultGlobalTerraformBinary, + # defaultGlobalMarkerFile). The runtime hard-codes + # /terraform/.terraform-global, so this path is hard-coded too -- + # do NOT derive it from .Values.rp.terraform.path or overrides to + # that value will silently reintroduce the path-mismatch bug this + # fix addresses. + GLOBAL_DIR="/terraform/.terraform-global" + mkdir -p "$GLOBAL_DIR" || { echo "ERROR: Failed to create global terraform directory"; exit 6; } + cp terraform "$GLOBAL_DIR/terraform" || { echo "ERROR: Failed to copy terraform"; exit 6; } + chmod +x "$GLOBAL_DIR/terraform" || { echo "ERROR: Failed to make terraform executable"; exit 7; } + touch "$GLOBAL_DIR/.terraform-ready" || { echo "ERROR: Failed to create terraform-ready marker"; exit 8; } + + # Keep a copy at the legacy path and write the legacy marker so any + # tooling that still references {{ .Values.rp.terraform.path }}/terraform continues to work. cp terraform "{{ .Values.rp.terraform.path }}/terraform" || { echo "ERROR: Failed to copy terraform"; exit 6; } chmod +x "{{ .Values.rp.terraform.path }}/terraform" || { echo "ERROR: Failed to make terraform executable"; exit 7; } - - # Create marker file to indicate pre-mounted binary is available echo "pre-mounted" > "{{ .Values.rp.terraform.path }}/.terraform-source" echo "Terraform binary successfully pre-downloaded and installed" diff --git a/deploy/Chart/tests/terraform_init_test.yaml b/deploy/Chart/tests/terraform_init_test.yaml new file mode 100644 index 0000000000..7df486d964 --- /dev/null +++ b/deploy/Chart/tests/terraform_init_test.yaml @@ -0,0 +1,96 @@ +suite: test terraform init container path mismatch fix +templates: + - rp/deployment.yaml + - dynamic-rp/deployment.yaml +tests: + # Regression test for https://github.com/radius-project/radius/pull/11880 + # The pre-mounted Terraform binary path written by the init container MUST + # match the constants in pkg/recipes/terraform/install.go: + # defaultGlobalTerraformBinary = "/terraform/.terraform-global/terraform" + # defaultGlobalMarkerFile = "/terraform/.terraform-global/.terraform-ready" + # If these strings drift, ensureGlobalTerraformBinary falls through to + # downloadAndInstallTerraform and the init container's work is wasted. + - it: rp init container writes terraform binary to the global path expected by install.go + asserts: + - matchRegex: + path: spec.template.spec.initContainers[?(@.name=='terraform-init')].command[2] + pattern: 'GLOBAL_DIR="/terraform/\.terraform-global"' + template: rp/deployment.yaml + - matchRegex: + path: spec.template.spec.initContainers[?(@.name=='terraform-init')].command[2] + pattern: 'cp terraform "\$GLOBAL_DIR/terraform"' + template: rp/deployment.yaml + - matchRegex: + path: spec.template.spec.initContainers[?(@.name=='terraform-init')].command[2] + pattern: 'touch "\$GLOBAL_DIR/\.terraform-ready"' + template: rp/deployment.yaml + + - it: dynamic-rp init container writes terraform binary to the global path expected by install.go + asserts: + - matchRegex: + path: spec.template.spec.initContainers[?(@.name=='terraform-init')].command[2] + pattern: 'GLOBAL_DIR="/terraform/\.terraform-global"' + template: dynamic-rp/deployment.yaml + - matchRegex: + path: spec.template.spec.initContainers[?(@.name=='terraform-init')].command[2] + pattern: 'cp terraform "\$GLOBAL_DIR/terraform"' + template: dynamic-rp/deployment.yaml + - matchRegex: + path: spec.template.spec.initContainers[?(@.name=='terraform-init')].command[2] + pattern: 'touch "\$GLOBAL_DIR/\.terraform-ready"' + template: dynamic-rp/deployment.yaml + + - it: rp template fails when rp.terraform.path is overridden away from /terraform + set: + rp.terraform.path: "/other" + asserts: + - failedTemplate: + errorMessage: 'rp.terraform.path must be "/terraform" -- the runtime configmap (deploy/Chart/templates/rp/configmaps.yaml) and pkg/recipes/terraform/install.go both hard-code /terraform, so overriding the mount path produces an inconsistent deployment and silently breaks Terraform execution/caching.' + template: rp/deployment.yaml + + - it: dynamic-rp template fails when dynamicrp.terraform.path is overridden away from /terraform + set: + dynamicrp.terraform.path: "/other" + asserts: + - failedTemplate: + errorMessage: 'dynamicrp.terraform.path must be "/terraform" -- the runtime configmap (deploy/Chart/templates/dynamic-rp/configmaps.yaml) and pkg/recipes/terraform/install.go both hard-code /terraform, so overriding the mount path produces an inconsistent deployment and silently breaks Terraform execution/caching.' + template: dynamic-rp/deployment.yaml + + - it: rp template still fails when rp.terraform.path is overridden, even with terraform pre-download disabled + set: + global.terraform.enabled: false + rp.terraform.path: "/other" + asserts: + - failedTemplate: + errorMessage: 'rp.terraform.path must be "/terraform" -- the runtime configmap (deploy/Chart/templates/rp/configmaps.yaml) and pkg/recipes/terraform/install.go both hard-code /terraform, so overriding the mount path produces an inconsistent deployment and silently breaks Terraform execution/caching.' + template: rp/deployment.yaml + + - it: dynamic-rp template still fails when dynamicrp.terraform.path is overridden, even with terraform pre-download disabled + set: + global.terraform.enabled: false + dynamicrp.terraform.path: "/other" + asserts: + - failedTemplate: + errorMessage: 'dynamicrp.terraform.path must be "/terraform" -- the runtime configmap (deploy/Chart/templates/dynamic-rp/configmaps.yaml) and pkg/recipes/terraform/install.go both hard-code /terraform, so overriding the mount path produces an inconsistent deployment and silently breaks Terraform execution/caching.' + template: dynamic-rp/deployment.yaml + + # Regression test for the second half of #11880: after fixing the + # binary-path mismatch the runtime actually uses the pre-mounted + # binary, so its version MUST match terraformVersion in + # pkg/recipes/terraform/version.go (currently "1.14.9"). If these + # drift the runtime download fallback and the pre-mount install + # different versions, causing recipes to behave differently between + # cold and warm starts (see Test_TerraformRecipe_Context failure). + - it: rp init container pins terraform version to match pkg/recipes/terraform/version.go + asserts: + - matchRegex: + path: spec.template.spec.initContainers[?(@.name=='terraform-init')].command[2] + pattern: 'TERRAFORM_VERSION="1\.14\.9"' + template: rp/deployment.yaml + + - it: dynamic-rp init container pins terraform version to match pkg/recipes/terraform/version.go + asserts: + - matchRegex: + path: spec.template.spec.initContainers[?(@.name=='terraform-init')].command[2] + pattern: 'TERRAFORM_VERSION="1\.14\.9"' + template: dynamic-rp/deployment.yaml diff --git a/deploy/Chart/values.yaml b/deploy/Chart/values.yaml index e44a57783d..e738ba0975 100644 --- a/deploy/Chart/values.yaml +++ b/deploy/Chart/values.yaml @@ -64,9 +64,16 @@ global: enabled: true # Init image used to pre-download Terraform binaries. initImage: "alpine:3.23.4" - # URL for downloading Terraform binary - # Leave empty to automatically fetch the latest version from HashiCorp - # Or provide a complete direct download URL for custom sources + # Terraform version to pre-download. MUST match terraformVersion in + # pkg/recipes/terraform/version.go -- the runtime download fallback + # (pkg/recipes/terraform/install.go::downloadAndInstallTerraform) + # pins to that version, so the pre-mounted binary must match or + # recipes will see behavior differences between cold and warm starts. + version: "1.14.9" + # URL for downloading Terraform binary. Leave empty to download the + # version specified by `version` from releases.hashicorp.com. Provide + # a complete direct download URL to override (e.g., for air-gapped or + # custom builds); when set, `version` is ignored. downloadUrl: "" # Configure Terraform log level for terraform-exec execution # Valid values: TRACE, DEBUG, INFO, WARN, ERROR, OFF diff --git a/pkg/recipes/terraform/version_test.go b/pkg/recipes/terraform/version_test.go index 9ac63a65cc..b781276069 100644 --- a/pkg/recipes/terraform/version_test.go +++ b/pkg/recipes/terraform/version_test.go @@ -17,6 +17,7 @@ limitations under the License. package terraform import ( + "fmt" "os" "path/filepath" "runtime" @@ -53,3 +54,41 @@ func TestTerraformVersionMatchesFile(t *testing.T) { "terraformVersion default in version.go must match .terraform-version at repo root", ) } + +// TestTerraformVersionMatchesHelmChart guarantees that the +// global.terraform.version default in deploy/Chart/values.yaml stays +// in sync with terraformVersion. The chart's terraform-init init +// container pre-downloads this version into the shared volume that +// the runtime (install.go::ensureGlobalTerraformBinary) consumes. If +// the chart default drifts from terraformVersion, the runtime's +// downloadAndInstallTerraform fallback installs a different version +// than the init container, so recipes behave differently between cold +// and warm starts (see #11880 / Test_TerraformRecipe_Context failure). +func TestTerraformVersionMatchesHelmChart(t *testing.T) { + _, thisFile, _, ok := runtime.Caller(0) + require.True(t, ok, "unable to determine test file location") + + dir := filepath.Dir(thisFile) + for { + if _, err := os.Stat(filepath.Join(dir, "go.mod")); err == nil { + break + } + parent := filepath.Dir(dir) + require.NotEqual(t, parent, dir, "could not locate repository root (go.mod)") + dir = parent + } + + contents, err := os.ReadFile(filepath.Join(dir, "deploy", "Chart", "values.yaml")) + require.NoError(t, err, "failed to read deploy/Chart/values.yaml") + + // Look for the literal line ` version: ""` inside + // the global.terraform block. A literal substring check is sufficient + // because the indentation uniquely identifies that nested key. + expected := fmt.Sprintf(" version: %q", terraformVersion) + require.Contains(t, + string(contents), + expected, + "global.terraform.version in deploy/Chart/values.yaml must match terraformVersion in pkg/recipes/terraform/version.go (%q)", + terraformVersion, + ) +}