From 628527a9112ac2f82898a3a1ec2ea470aa1d5bd2 Mon Sep 17 00:00:00 2001 From: "Prince Nagar (BEYONDSOFT CONSULTING INC)" Date: Wed, 2 Sep 2026 13:09:41 +0530 Subject: [PATCH 1/2] feat: secure Terraform plan hand-off via Blob Storage (Azure DevOps) Ports the GitHub production repo's secure plan-storage design (commits dd24dec/12ea394 on the sibling GitHub bootstrap repo) to the Azure DevOps pipelines, closing Azure/Azure-Landing-Zones#4174 for the Azure DevOps bootstrap module. Terraform: - variables.cicd.tf: plan_storage_retention_days, show_plan_in_pipeline_logs, use_storage_account_for_plan. - locals.cicd.tf: plan_storage_container_names plus backend collision/duplicate-name validation lists. - main.azure.storage.tf: plan-container validation resource, merge plan containers into the storage account, add a lifecycle rule for plan blob retention. Also fixes a pre-existing bug where the state container's role_assignments did not filter disabled identities. - main.azuredevops.variable.groups.tf: publish USE_STORAGE_ACCOUNT_FOR_PLAN, SHOW_PLAN_IN_PIPELINE_LOGS, and (conditionally) PLAN_STORAGE_CONTAINER_NAME per environment. - variables.environments.tf: bundled-CD identity validation. Pipelines: - New helpers/terraform-plan-{workspace,upload,download,delete, cleanup}.yaml isolate the plan blob lifecycle. - terraform-plan.yaml / terraform-apply.yaml keep the existing OIDC auth blocks and route the plan file through Agent.TempDirectory instead of the sources directory. - cd-template.yaml / ci-template.yaml gate log/artifact exposure on the two feature flags, drop tfplan/tfplan.json from the published artifact, and fix a pre-existing Build.ArtifactsStagingDirectory typo (missing 's'). Docs/tests: - _header.md documents the new secure plan hand-off behavior. - tests/contract/Test-PipelinePlanStorage.ps1: 66-assertion static contract test adapted for Azure Pipelines YAML (raw-text regex rather than object-model navigation, since ADO's "each" template expression syntax isn't a standard YAML construct). - .github/workflows/plan-storage-contract.yml wires the contract test into CI. - README.md regenerated via avm pre-commit. use_storage_account_for_plan defaults to true (secure by default); show_plan_in_pipeline_logs defaults to false. Storage account module pin (avm-res-storage-storageaccount 0.6.8) unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .github/workflows/plan-storage-contract.yml | 27 ++ README.md | 40 ++- _header.md | 15 +- locals.cicd.tf | 12 + main.azure.storage.tf | 61 ++++- main.azuredevops.variable.groups.tf | 16 ++ .../terraform/templates/cd-template.yaml | 63 ++++- .../terraform/templates/ci-template.yaml | 17 +- .../templates/helpers/terraform-apply.yaml | 3 +- .../helpers/terraform-plan-cleanup.yaml | 14 + .../helpers/terraform-plan-delete.yaml | 44 ++++ .../helpers/terraform-plan-download.yaml | 52 ++++ .../helpers/terraform-plan-upload.yaml | 70 +++++ .../helpers/terraform-plan-workspace.yaml | 22 ++ .../templates/helpers/terraform-plan.yaml | 39 ++- tests/contract/Test-PipelinePlanStorage.ps1 | 244 ++++++++++++++++++ variables.cicd.tf | 23 ++ variables.environments.tf | 9 + 18 files changed, 742 insertions(+), 29 deletions(-) create mode 100644 .github/workflows/plan-storage-contract.yml create mode 100644 pipelines/terraform/templates/helpers/terraform-plan-cleanup.yaml create mode 100644 pipelines/terraform/templates/helpers/terraform-plan-delete.yaml create mode 100644 pipelines/terraform/templates/helpers/terraform-plan-download.yaml create mode 100644 pipelines/terraform/templates/helpers/terraform-plan-upload.yaml create mode 100644 pipelines/terraform/templates/helpers/terraform-plan-workspace.yaml create mode 100644 tests/contract/Test-PipelinePlanStorage.ps1 diff --git a/.github/workflows/plan-storage-contract.yml b/.github/workflows/plan-storage-contract.yml new file mode 100644 index 0000000..6adac62 --- /dev/null +++ b/.github/workflows/plan-storage-contract.yml @@ -0,0 +1,27 @@ +--- +name: Plan Storage Contract Tests + +on: + pull_request: + paths: + - 'pipelines/terraform/templates/ci-template.yaml' + - 'pipelines/terraform/templates/cd-template.yaml' + - 'pipelines/terraform/templates/helpers/terraform-plan*.yaml' + - 'tests/contract/Test-PipelinePlanStorage.ps1' + - '.github/workflows/plan-storage-contract.yml' + workflow_dispatch: + +jobs: + contract: + name: Verify plan storage pipeline contract + runs-on: ubuntu-latest + permissions: + contents: read + steps: + - name: Checkout Code + uses: actions/checkout@v4 + + - name: Run contract tests + shell: pwsh + run: | + ./tests/contract/Test-PipelinePlanStorage.ps1 diff --git a/README.md b/README.md index 487c338..d85a469 100644 --- a/README.md +++ b/README.md @@ -27,12 +27,25 @@ This Azure Verified Module (AVM) pattern module bootstraps a complete, opinionat ### Azure resources -- **Terraform state** — when `deployment_mode = "terraform"`, provisions a hardened storage account (private endpoint, no public access) for Terraform remote state per environment. +- **Terraform state & plan storage** — when `deployment_mode = "terraform"`, provisions a hardened storage account (private endpoint, no public access) for Terraform remote state per environment, and, when `use_storage_account_for_plan = true` (the default), a dedicated `-tfplan` container per environment for secure plan hand-off between the CD `plan` and `apply` stages. - **Networking** — provisions a virtual network with dedicated subnets for agents and private endpoints, or accepts a pre-existing VNet / subnets in BYO mode. - **Private DNS** — manages private DNS zones for private endpoints, with an opt-out (`azure_alz_platform_landing_zone_mode_enabled`) for ALZ platforms that manage DNS centrally via Azure Policy. - **Identity** — creates the per-environment UAMIs used by the service connections, plus (when `agent_authentication_method = "uami"`) the UAMI used by the agent pool. - **Resource groups** — creates dedicated resource groups for identity, state, agents, and networking (or reuses an existing VNet's resource group in BYO mode). +### Secure Terraform plan hand-off + +- **Default-secure** — `use_storage_account_for_plan = true` by default. The CD `plan` stage uploads the binary plan to a per-environment Blob container (`-tfplan`) instead of bundling it into the Azure Pipelines artifact; the `apply` stage downloads the exact same blob by build ID and deletes it after a successful apply. +- **Legacy fallback** — set `use_storage_account_for_plan = false` to revert to shipping the plan inside the Azure Pipelines build artifact. This is less secure (plan contents can include sensitive values and are retained per your organization's pipeline artifact retention policy) and is provided only for compatibility with self-managed/BYO template repos that haven't adopted the new templates. +- **Custom template repositories are not auto-secured** — if you set `azuredevops_existing_template_repository_name` or a custom pipeline template path, the module does not modify your pipeline YAML. You must adopt the upload/download/delete steps yourself for storage-backed hand-off to apply. +- **`show_plan_in_pipeline_logs`** — defaults to `false`. Enabling it prints the full plan to the pipeline log, visible to anyone with read access to the project/pipeline runs. Only enable if your organization has explicitly accepted that exposure. +- **`plan_storage_retention_days`** (default `7`) — a storage lifecycle policy rule deletes abandoned plan blobs, snapshots, and previous versions after this many days. This is a backstop only; successful applies delete their own plan blob immediately. +- **Recoverability** — the plan container inherits the storage account's blob versioning/soft-delete settings, so an accidentally-deleted plan blob may still be recoverable within your soft-delete window. +- **Trusted-admin threat boundary** — this feature keeps plan contents out of Azure Pipelines artifact storage. It does not protect against an Azure user with Storage Blob Data Contributor/Owner-equivalent access to the storage account — the same trust boundary as Terraform remote state. +- **Stale/concurrent plans** — the `apply` stage always downloads the blob written by its own `plan` stage run (keyed by `$(Build.BuildId)`), never "the latest" blob, so a concurrent or superseded run cannot apply a stranger's plan. +- **Non-retroactive upgrade** — enabling this on an existing deployment only takes effect for the next `plan`/`apply` cycle; it does not migrate plans already in flight. +- **Upgrading an existing storage account** — `storage_management_policy_rule` replaces the account's *entire* lifecycle policy, not just this module's rules. If you already manage that storage account's lifecycle policy outside this module, applying this upgrade will silently overwrite those rules. Before upgrading, check existing rules with `az storage account management-policy show` and fold them into your Terraform config first. + ## Authentication required to use the module The module talks to two control planes: **Azure Resource Manager** and **Azure DevOps**. Configure provider authentication via environment variables — the module itself does not accept any provider credentials as input variables. @@ -107,6 +120,7 @@ The following resources are used by this module: - [modtm_telemetry.telemetry](https://registry.terraform.io/providers/azure/modtm/latest/docs/resources/telemetry) (resource) - [random_string.unique_name](https://registry.terraform.io/providers/hashicorp/random/latest/docs/resources/string) (resource) - [random_uuid.telemetry](https://registry.terraform.io/providers/hashicorp/random/latest/docs/resources/uuid) (resource) +- [terraform_data.plan_storage_container_validation](https://registry.terraform.io/providers/hashicorp/terraform/latest/docs/resources/data) (resource) - [time_sleep.agents_user_assigned_managed_identity_propagation](https://registry.terraform.io/providers/hashicorp/time/latest/docs/resources/sleep) (resource) - [time_sleep.service_connection_teardown](https://registry.terraform.io/providers/hashicorp/time/latest/docs/resources/sleep) (resource) - [azapi_client_config.current](https://registry.terraform.io/providers/Azure/azapi/latest/docs/data-sources/client_config) (data source) @@ -454,6 +468,14 @@ Type: `string` Default: `null` +### [plan\_storage\_retention\_days](#input\_plan\_storage\_retention\_days) + +Description: The number of days after which abandoned Terraform plan base blobs, snapshots, and previous versions are eligible for lifecycle deletion. + +Type: `number` + +Default: `7` + ### [resource\_name\_environment](#input\_resource\_name\_environment) Description: The name segment for the management environment (used for naming Azure infrastructure resources, not deployment environments). @@ -522,6 +544,14 @@ Type: `string` Default: `"dema"` +### [show\_plan\_in\_pipeline\_logs](#input\_show\_plan\_in\_pipeline\_logs) + +Description: Whether to print the full Terraform plan in pipeline logs. Enabling this can expose sensitive values. + +Type: `bool` + +Default: `false` + ### [tags](#input\_tags) Description: (Optional) Tags of the resource. @@ -530,6 +560,14 @@ Type: `map(string)` Default: `null` +### [use\_storage\_account\_for\_plan](#input\_use\_storage\_account\_for\_plan) + +Description: Whether to use the Terraform state Storage Account for secure plan hand-off. Set to false to use the legacy CI/CD artifact hand-off. + +Type: `bool` + +Default: `true` + ## Outputs The following outputs are exported: diff --git a/_header.md b/_header.md index 20f3560..c47f72d 100644 --- a/_header.md +++ b/_header.md @@ -25,12 +25,25 @@ This Azure Verified Module (AVM) pattern module bootstraps a complete, opinionat ### Azure resources -- **Terraform state** — when `deployment_mode = "terraform"`, provisions a hardened storage account (private endpoint, no public access) for Terraform remote state per environment. +- **Terraform state & plan storage** — when `deployment_mode = "terraform"`, provisions a hardened storage account (private endpoint, no public access) for Terraform remote state per environment, and, when `use_storage_account_for_plan = true` (the default), a dedicated `-tfplan` container per environment for secure plan hand-off between the CD `plan` and `apply` stages. - **Networking** — provisions a virtual network with dedicated subnets for agents and private endpoints, or accepts a pre-existing VNet / subnets in BYO mode. - **Private DNS** — manages private DNS zones for private endpoints, with an opt-out (`azure_alz_platform_landing_zone_mode_enabled`) for ALZ platforms that manage DNS centrally via Azure Policy. - **Identity** — creates the per-environment UAMIs used by the service connections, plus (when `agent_authentication_method = "uami"`) the UAMI used by the agent pool. - **Resource groups** — creates dedicated resource groups for identity, state, agents, and networking (or reuses an existing VNet's resource group in BYO mode). +### Secure Terraform plan hand-off + +- **Default-secure** — `use_storage_account_for_plan = true` by default. The CD `plan` stage uploads the binary plan to a per-environment Blob container (`-tfplan`) instead of bundling it into the Azure Pipelines artifact; the `apply` stage downloads the exact same blob by build ID and deletes it after a successful apply. +- **Legacy fallback** — set `use_storage_account_for_plan = false` to revert to shipping the plan inside the Azure Pipelines build artifact. This is less secure (plan contents can include sensitive values and are retained per your organization's pipeline artifact retention policy) and is provided only for compatibility with self-managed/BYO template repos that haven't adopted the new templates. +- **Custom template repositories are not auto-secured** — if you set `azuredevops_existing_template_repository_name` or a custom pipeline template path, the module does not modify your pipeline YAML. You must adopt the upload/download/delete steps yourself for storage-backed hand-off to apply. +- **`show_plan_in_pipeline_logs`** — defaults to `false`. Enabling it prints the full plan to the pipeline log, visible to anyone with read access to the project/pipeline runs. Only enable if your organization has explicitly accepted that exposure. +- **`plan_storage_retention_days`** (default `7`) — a storage lifecycle policy rule deletes abandoned plan blobs, snapshots, and previous versions after this many days. This is a backstop only; successful applies delete their own plan blob immediately. +- **Recoverability** — the plan container inherits the storage account's blob versioning/soft-delete settings, so an accidentally-deleted plan blob may still be recoverable within your soft-delete window. +- **Trusted-admin threat boundary** — this feature keeps plan contents out of Azure Pipelines artifact storage. It does not protect against an Azure user with Storage Blob Data Contributor/Owner-equivalent access to the storage account — the same trust boundary as Terraform remote state. +- **Stale/concurrent plans** — the `apply` stage always downloads the blob written by its own `plan` stage run (keyed by `$(Build.BuildId)`), never "the latest" blob, so a concurrent or superseded run cannot apply a stranger's plan. +- **Non-retroactive upgrade** — enabling this on an existing deployment only takes effect for the next `plan`/`apply` cycle; it does not migrate plans already in flight. +- **Upgrading an existing storage account** — `storage_management_policy_rule` replaces the account's *entire* lifecycle policy, not just this module's rules. If you already manage that storage account's lifecycle policy outside this module, applying this upgrade will silently overwrite those rules. Before upgrading, check existing rules with `az storage account management-policy show` and fold them into your Terraform config first. + ## Authentication required to use the module The module talks to two control planes: **Azure Resource Manager** and **Azure DevOps**. Configure provider authentication via environment variables — the module itself does not accept any provider credentials as input variables. diff --git a/locals.cicd.tf b/locals.cicd.tf index 137a1fb..3efd579 100644 --- a/locals.cicd.tf +++ b/locals.cicd.tf @@ -13,4 +13,16 @@ locals { ) has_approvers = var.azuredevops_existing_approvers_group_origin_id != null || length(var.approvers) > 0 has_template_repo = var.azuredevops_existing_template_repository_name != null || local.create_template_repository + + plan_storage_container_backend_collisions = [ + for container_name in values(local.plan_storage_container_names) : container_name + if contains(keys(local.environments), container_name) + ] + plan_storage_container_duplicate_names = [ + for name in distinct(values(local.plan_storage_container_names)) : name + if length([for v in values(local.plan_storage_container_names) : v if v == name]) > 1 + ] + plan_storage_container_names = var.deployment_mode == "terraform" && var.use_storage_account_for_plan ? { for env_key, env_value in local.environments : env_key => ( + length(env_key) <= 56 ? "${env_key}-tfplan" : "tfplan-${substr(sha256(env_key), 0, 32)}" + ) } : {} } diff --git a/main.azure.storage.tf b/main.azure.storage.tf index 53614f5..053f830 100644 --- a/main.azure.storage.tf +++ b/main.azure.storage.tf @@ -13,6 +13,21 @@ module "private_dns_zone_storage_account" { } } +resource "terraform_data" "plan_storage_container_validation" { + count = var.deployment_mode == "terraform" && var.use_storage_account_for_plan ? 1 : 0 + + lifecycle { + precondition { + condition = length(local.plan_storage_container_backend_collisions) == 0 + error_message = "Computed plan storage container name(s) collide with an existing backend state container: ${join(", ", local.plan_storage_container_backend_collisions)}." + } + precondition { + condition = length(local.plan_storage_container_duplicate_names) == 0 + error_message = "Computed plan storage container name(s) collide across environments: ${join(", ", local.plan_storage_container_duplicate_names)}." + } + } +} + module "storage_account" { source = "Azure/avm-res-storage-storageaccount/azurerm" version = "0.6.8" @@ -23,15 +38,26 @@ module "storage_account" { resource_group_name = module.resource_group["state"].name account_replication_type = "ZRS" account_tier = "Standard" - containers = { for env_key, env_value in local.environments : env_key => { - name = env_key - public_access = "None" - role_assignments = { for identity_key, identity_value in env_value.identities : "uami-${identity_key}" => { - role_definition_id_or_name = "Storage Blob Data Contributor" - principal_id = module.user_assigned_managed_identity["${env_key}-${identity_key}"].principal_id - } } + containers = merge( + { for env_key, env_value in local.environments : env_key => { + name = env_key + public_access = "None" + role_assignments = { for identity_key, identity_value in env_value.identities : "uami-${identity_key}" => { + role_definition_id_or_name = "Storage Blob Data Contributor" + principal_id = module.user_assigned_managed_identity["${env_key}-${identity_key}"].principal_id + } if identity_value.enabled } + } + }, + { for env_key, container_name in local.plan_storage_container_names : container_name => { + name = container_name + public_access = "None" + role_assignments = { for identity_key, identity_value in local.environments[env_key].identities : "uami-${identity_key}" => { + role_definition_id_or_name = "Storage Blob Data Contributor" + principal_id = module.user_assigned_managed_identity["${env_key}-${identity_key}"].principal_id + } if identity_value.enabled } + } } - } + ) network_rules = local.use_private_networking ? {} : null private_endpoints = local.use_private_networking ? { blob = { name = local.resource_names.storage_account_private_endpoint_name @@ -42,4 +68,23 @@ module "storage_account" { } : {} private_endpoints_manage_dns_zone_group = !var.azure_alz_platform_landing_zone_mode_enabled public_network_access_enabled = !local.use_private_networking + storage_management_policy_rule = { for env_key, container_name in local.plan_storage_container_names : env_key => { + name = "plan${replace(env_key, "/[^a-zA-Z0-9]/", "")}" + enabled = true + filters = { + blob_types = ["blockBlob"] + prefix_match = ["${container_name}/runs/"] + } + actions = { + base_blob = { + delete_after_days_since_modification_greater_than = var.plan_storage_retention_days + } + snapshot = { + delete_after_days_since_creation_greater_than = var.plan_storage_retention_days + } + version = { + delete_after_days_since_creation = var.plan_storage_retention_days + } + } + } } } diff --git a/main.azuredevops.variable.groups.tf b/main.azuredevops.variable.groups.tf index 2498989..66808fe 100644 --- a/main.azuredevops.variable.groups.tf +++ b/main.azuredevops.variable.groups.tf @@ -29,6 +29,22 @@ resource "azuredevops_variable_group" "this" { name = "BACKEND_AZURE_STORAGE_ACCOUNT_CONTAINER_NAME" value = each.key } + variable { + name = "USE_STORAGE_ACCOUNT_FOR_PLAN" + value = var.use_storage_account_for_plan ? "true" : "false" + } + variable { + name = "SHOW_PLAN_IN_PIPELINE_LOGS" + value = var.show_plan_in_pipeline_logs ? "true" : "false" + } + dynamic "variable" { + for_each = var.use_storage_account_for_plan && contains(keys(local.plan_storage_container_names), each.key) ? [1] : [] + + content { + name = "PLAN_STORAGE_CONTAINER_NAME" + value = local.plan_storage_container_names[each.key] + } + } } resource "azuredevops_variable_group" "bicep" { diff --git a/pipelines/terraform/templates/cd-template.yaml b/pipelines/terraform/templates/cd-template.yaml index 5abda11..f052259 100644 --- a/pipelines/terraform/templates/cd-template.yaml +++ b/pipelines/terraform/templates/cd-template.yaml @@ -38,6 +38,9 @@ stages: - template: helpers/terraform-installer.yaml parameters: terraformVersion: ${{ parameters.terraform_cli_version }} + - template: helpers/terraform-plan-workspace.yaml + parameters: + purpose: 'plan' - template: helpers/terraform-init.yaml parameters: serviceConnection: ${{ environment.service_connection_name_read }} @@ -52,6 +55,18 @@ stages: root_module_folder_relative_path: ${{ parameters.root_module_folder_relative_path }} additionalVariables: $(ADDITIONAL_ENVIRONMENT_VARIABLES) varFilePath: $(VAR_FILE_PATH) + - pwsh: | + terraform ` + -chdir="${{ parameters.root_module_folder_relative_path }}" ` + show ` + "$(alzPlanDir)/tfplan" + displayName: Show the Plan for Review + condition: and(succeeded(), eq(variables['SHOW_PLAN_IN_PIPELINE_LOGS'], 'true')) + - template: helpers/terraform-plan-upload.yaml + parameters: + serviceConnection: ${{ environment.service_connection_name_read }} + # The plan file lives in the agent temp directory, so it is structurally absent from the + # sources directory this artifact is built from (Azure/Azure-Landing-Zones#4174). - task: CopyFiles@2 displayName: Create Module Artifact inputs: @@ -64,21 +79,31 @@ stages: !**/.terraform/**/* !**/.git/**/* !**/.pipelines/**/* - TargetFolder: '$(Build.ArtifactsStagingDirectory)' + !**/tfplan + !**/tfplan.json + TargetFolder: '$(Build.ArtifactStagingDirectory)' CleanTargetFolder: true OverWrite: true + - pwsh: | + # Legacy behaviour: hand the plan off inside the pipeline artifact, where anyone with + # pipeline read access can download it. Only reached when the storage hand-off is + # explicitly disabled. The plan must land at the same root-module-relative path the + # apply stage reads it back from, which is not necessarily the artifact root. + $destinationFolder = Join-Path $env:ALZ_STAGING_DIR "${{ parameters.root_module_folder_relative_path }}" + New-Item -Path $destinationFolder -ItemType Directory -Force | Out-Null + Copy-Item -Path "$(alzPlanDir)/tfplan" -Destination (Join-Path $destinationFolder "tfplan") -Force + Write-Host "##vso[task.logissue type=warning]USE_STORAGE_ACCOUNT_FOR_PLAN is not 'true', so the Terraform plan file is published inside the pipeline artifact and is readable by anyone with pipeline read access." + displayName: Add Plan to Module Artifact + condition: and(succeeded(), ne(variables['USE_STORAGE_ACCOUNT_FOR_PLAN'], 'true')) + env: + ALZ_STAGING_DIR: $(Build.ArtifactStagingDirectory) - task: PublishPipelineArtifact@1 displayName: Publish Module Artifact inputs: - targetPath: '$(Build.ArtifactsStagingDirectory)' + targetPath: '$(Build.ArtifactStagingDirectory)' artifact: 'module_${{ environment.name }}' publishLocation: 'pipeline' - - pwsh: | - terraform ` - -chdir="${{ parameters.root_module_folder_relative_path }}" ` - show ` - tfplan - displayName: Show the Plan for Review + - template: helpers/terraform-plan-cleanup.yaml - ${{ if or(eq(environment.name, parameters.environment), eq(parameters.environment, 'All')) }}: - stage: ${{ environment.name }}_apply @@ -109,6 +134,22 @@ stages: - template: helpers/terraform-installer.yaml parameters: terraformVersion: ${{ parameters.terraform_cli_version }} + - template: helpers/terraform-plan-workspace.yaml + parameters: + purpose: 'apply' + - pwsh: | + # Legacy fallback: the plan travelled inside the module artifact. Copy it into the + # same agent-temp working directory the storage hand-off would have used, so the + # apply step below always reads from one place regardless of which path was taken. + $legacyPlanFile = Join-Path "$(Build.SourcesDirectory)/${{ parameters.root_module_folder_relative_path }}" "tfplan" + Copy-Item -Path $legacyPlanFile -Destination (Join-Path $env:ALZ_PLAN_DIR "tfplan") -Force + displayName: Stage Legacy Plan from Artifact + condition: and(succeeded(), ne(variables['USE_STORAGE_ACCOUNT_FOR_PLAN'], 'true')) + env: + ALZ_PLAN_DIR: $(alzPlanDir) + - template: helpers/terraform-plan-download.yaml + parameters: + serviceConnection: ${{ environment.service_connection_name_write }} - template: helpers/terraform-init.yaml parameters: serviceConnection: ${{ environment.service_connection_name_write }} @@ -121,3 +162,9 @@ stages: terraform_action: ${{ parameters.terraform_action }} serviceConnection: ${{ environment.service_connection_name_write }} root_module_folder_relative_path: ${{ parameters.root_module_folder_relative_path }} + # Best-effort immediate cleanup on the happy path only. A failed apply deliberately leaves + # the blob in place; the container lifecycle policy is the guaranteed eventual cleanup. + - template: helpers/terraform-plan-delete.yaml + parameters: + serviceConnection: ${{ environment.service_connection_name_write }} + - template: helpers/terraform-plan-cleanup.yaml diff --git a/pipelines/terraform/templates/ci-template.yaml b/pipelines/terraform/templates/ci-template.yaml index 51862fe..29b2b37 100644 --- a/pipelines/terraform/templates/ci-template.yaml +++ b/pipelines/terraform/templates/ci-template.yaml @@ -63,6 +63,9 @@ stages: - template: helpers/terraform-installer.yaml parameters: terraformVersion: ${{ parameters.terraform_cli_version }} + - template: helpers/terraform-plan-workspace.yaml + parameters: + purpose: 'validate' - template: helpers/terraform-init.yaml parameters: serviceConnection: ${{ environment.service_connection_name_read }} @@ -77,8 +80,8 @@ stages: additionalVariables: $(ADDITIONAL_ENVIRONMENT_VARIABLES) varFilePath: $(VAR_FILE_PATH) - pwsh: | - terraform -chdir="${{ parameters.root_module_folder_relative_path }}" show -json tfplan > tfplan.json - $planJson = Get-Content -Raw tfplan.json + terraform -chdir="${{ parameters.root_module_folder_relative_path }}" show -json "$(alzPlanDir)/tfplan" > "$(alzPlanDir)/tfplan.json" + $planJson = Get-Content -Raw "$(alzPlanDir)/tfplan.json" $planObject = ConvertFrom-Json $planJson -Depth 100 $items = @{} @@ -93,9 +96,7 @@ stages: Write-Host "Plan Summary" Write-Host (ConvertTo-Json $items -Depth 10) displayName: Terraform Plan Summary - - task: PublishPipelineArtifact@1 - displayName: Publish Plan Artifact - inputs: - targetPath: 'tfplan.json' - artifact: 'plan_${{ environment.name }}' - publishLocation: 'pipeline' + # The plan (and its JSON rendering, which still embeds every resolved resource attribute) + # is never published as a pipeline artifact - it is destroyed with the rest of the agent + # temp directory below (Azure/Azure-Landing-Zones#4174). + - template: helpers/terraform-plan-cleanup.yaml diff --git a/pipelines/terraform/templates/helpers/terraform-apply.yaml b/pipelines/terraform/templates/helpers/terraform-apply.yaml index 6d7ea00..7792bc4 100644 --- a/pipelines/terraform/templates/helpers/terraform-apply.yaml +++ b/pipelines/terraform/templates/helpers/terraform-apply.yaml @@ -32,9 +32,10 @@ steps: $arguments += "-chdir=${{ parameters.root_module_folder_relative_path }}" $arguments += "apply" $arguments += "-auto-approve" - $arguments += "tfplan" + $arguments += $env:ALZ_PLAN_FILE Write-Host "Running: $command $arguments" & $command $arguments env: ARM_OIDC_REQUEST_TOKEN: $(System.AccessToken) + ALZ_PLAN_FILE: $(alzPlanDir)/tfplan diff --git a/pipelines/terraform/templates/helpers/terraform-plan-cleanup.yaml b/pipelines/terraform/templates/helpers/terraform-plan-cleanup.yaml new file mode 100644 index 0000000..17b789e --- /dev/null +++ b/pipelines/terraform/templates/helpers/terraform-plan-cleanup.yaml @@ -0,0 +1,14 @@ +--- +steps: + # The path arrives as an environment variable rather than an inline $(alzPlanDir) macro so that an + # unset variable degrades to a harmless literal string instead of being evaluated as PowerShell. + - pwsh: | + $planDir = $env:ALZ_PLAN_DIR + if (-not [string]::IsNullOrEmpty($planDir) -and (Test-Path -Path $planDir)) { + Remove-Item -Path $planDir -Recurse -Force + Write-Host "Removed plan working directory '$planDir'." + } + displayName: Clean Up Plan Working Directory + condition: always() + env: + ALZ_PLAN_DIR: $(alzPlanDir) diff --git a/pipelines/terraform/templates/helpers/terraform-plan-delete.yaml b/pipelines/terraform/templates/helpers/terraform-plan-delete.yaml new file mode 100644 index 0000000..e11e71f --- /dev/null +++ b/pipelines/terraform/templates/helpers/terraform-plan-delete.yaml @@ -0,0 +1,44 @@ +--- +parameters: + - name: serviceConnection + +steps: + - task: AzureCLI@2 + displayName: Delete Applied Plan from Storage + condition: and(succeeded(), eq(variables['USE_STORAGE_ACCOUNT_FOR_PLAN'], 'true')) + inputs: + azureSubscription: ${{ parameters.serviceConnection }} + scriptType: pscore + scriptLocation: inlineScript + inlineScript: | + $storageAccount = $env:ALZ_STORAGE_ACCOUNT + $container = $env:ALZ_PLAN_CONTAINER + $blobName = "runs/$env:ALZ_BUILD_ID/tfplan" + + $maxAttempts = 3 + $delaySeconds = 5 + $deleted = $false + for ($attempt = 1; $attempt -le $maxAttempts; $attempt++) { + az storage blob delete ` + --account-name $storageAccount ` + --container-name $container ` + --name $blobName ` + --auth-mode login ` + --only-show-errors + if ($LASTEXITCODE -eq 0) { + $deleted = $true + break + } + Write-Host "Delete attempt $attempt failed with exit code $LASTEXITCODE." + if ($attempt -lt $maxAttempts) { + Start-Sleep -Seconds $delaySeconds + $delaySeconds *= 2 + } + } + if (-not $deleted) { + Write-Warning "Failed to delete applied plan blob '$blobName' after $maxAttempts attempts. It will be removed automatically by the container's lifecycle policy." + } + env: + ALZ_STORAGE_ACCOUNT: $(BACKEND_AZURE_STORAGE_ACCOUNT_NAME) + ALZ_PLAN_CONTAINER: $(PLAN_STORAGE_CONTAINER_NAME) + ALZ_BUILD_ID: $(Build.BuildId) diff --git a/pipelines/terraform/templates/helpers/terraform-plan-download.yaml b/pipelines/terraform/templates/helpers/terraform-plan-download.yaml new file mode 100644 index 0000000..0368a8d --- /dev/null +++ b/pipelines/terraform/templates/helpers/terraform-plan-download.yaml @@ -0,0 +1,52 @@ +--- +parameters: + - name: serviceConnection + +steps: + - task: AzureCLI@2 + displayName: Download Plan from Storage + condition: and(succeeded(), eq(variables['USE_STORAGE_ACCOUNT_FOR_PLAN'], 'true')) + inputs: + azureSubscription: ${{ parameters.serviceConnection }} + scriptType: pscore + scriptLocation: inlineScript + inlineScript: | + $planFile = Join-Path $env:ALZ_PLAN_DIR "tfplan" + $storageAccount = $env:ALZ_STORAGE_ACCOUNT + $container = $env:ALZ_PLAN_CONTAINER + $blobName = "runs/$env:ALZ_BUILD_ID/tfplan" + + $maxAttempts = 3 + $delaySeconds = 5 + $downloaded = $false + for ($attempt = 1; $attempt -le $maxAttempts; $attempt++) { + az storage blob download ` + --account-name $storageAccount ` + --container-name $container ` + --name $blobName ` + --file $planFile ` + --auth-mode login ` + --only-show-errors + if ($LASTEXITCODE -eq 0) { + $downloaded = $true + break + } + Write-Host "Download attempt $attempt failed with exit code $LASTEXITCODE." + if ($attempt -lt $maxAttempts) { + Start-Sleep -Seconds $delaySeconds + $delaySeconds *= 2 + } + } + if (-not $downloaded) { + throw "Failed to download plan blob '$blobName' after $maxAttempts attempts. Failing closed: no fallback to a stale or missing plan." + } + if (-not (Test-Path -Path $planFile) -or (Get-Item -Path $planFile).Length -eq 0) { + throw "Downloaded plan blob '$blobName' is missing or empty." + } + + Write-Host "Downloaded plan blob '$blobName' to '$planFile'." + env: + ALZ_PLAN_DIR: $(alzPlanDir) + ALZ_STORAGE_ACCOUNT: $(BACKEND_AZURE_STORAGE_ACCOUNT_NAME) + ALZ_PLAN_CONTAINER: $(PLAN_STORAGE_CONTAINER_NAME) + ALZ_BUILD_ID: $(Build.BuildId) diff --git a/pipelines/terraform/templates/helpers/terraform-plan-upload.yaml b/pipelines/terraform/templates/helpers/terraform-plan-upload.yaml new file mode 100644 index 0000000..25a4fc6 --- /dev/null +++ b/pipelines/terraform/templates/helpers/terraform-plan-upload.yaml @@ -0,0 +1,70 @@ +--- +parameters: + - name: serviceConnection + +steps: + - task: AzureCLI@2 + displayName: Upload Plan to Storage + condition: and(succeeded(), eq(variables['USE_STORAGE_ACCOUNT_FOR_PLAN'], 'true')) + inputs: + azureSubscription: ${{ parameters.serviceConnection }} + scriptType: pscore + scriptLocation: inlineScript + inlineScript: | + $planFile = Join-Path $env:ALZ_PLAN_DIR "tfplan" + $storageAccount = $env:ALZ_STORAGE_ACCOUNT + $container = $env:ALZ_PLAN_CONTAINER + $blobName = "runs/$env:ALZ_BUILD_ID/tfplan" + + if (-not (Test-Path -Path $planFile)) { + throw "Plan file '$planFile' was not produced by the plan step." + } + $localLength = (Get-Item -Path $planFile).Length + if ($localLength -eq 0) { + throw "Plan file '$planFile' is empty." + } + + $maxAttempts = 3 + $delaySeconds = 5 + $uploaded = $false + for ($attempt = 1; $attempt -le $maxAttempts; $attempt++) { + az storage blob upload ` + --account-name $storageAccount ` + --container-name $container ` + --name $blobName ` + --file $planFile ` + --auth-mode login ` + --overwrite ` + --only-show-errors + if ($LASTEXITCODE -eq 0) { + $uploaded = $true + break + } + Write-Host "Upload attempt $attempt failed with exit code $LASTEXITCODE." + if ($attempt -lt $maxAttempts) { + Start-Sleep -Seconds $delaySeconds + $delaySeconds *= 2 + } + } + if (-not $uploaded) { + throw "Failed to upload plan blob '$blobName' after $maxAttempts attempts." + } + + $remoteLength = az storage blob show ` + --account-name $storageAccount ` + --container-name $container ` + --name $blobName ` + --auth-mode login ` + --query properties.contentLength ` + --only-show-errors ` + -o tsv + if ([string]::IsNullOrEmpty($remoteLength) -or [int64]$remoteLength -ne $localLength) { + throw "Uploaded plan blob '$blobName' content length ($remoteLength) does not match local file ($localLength)." + } + + Write-Host "Uploaded plan blob '$blobName' ($localLength bytes) to container '$container'." + env: + ALZ_PLAN_DIR: $(alzPlanDir) + ALZ_STORAGE_ACCOUNT: $(BACKEND_AZURE_STORAGE_ACCOUNT_NAME) + ALZ_PLAN_CONTAINER: $(PLAN_STORAGE_CONTAINER_NAME) + ALZ_BUILD_ID: $(Build.BuildId) diff --git a/pipelines/terraform/templates/helpers/terraform-plan-workspace.yaml b/pipelines/terraform/templates/helpers/terraform-plan-workspace.yaml new file mode 100644 index 0000000..e62eacc --- /dev/null +++ b/pipelines/terraform/templates/helpers/terraform-plan-workspace.yaml @@ -0,0 +1,22 @@ +--- +parameters: + - name: purpose + default: 'plan' + +steps: + - pwsh: | + # Keep the plan file out of $(Build.SourcesDirectory) so it can never be swept into the published + # pipeline artifact (Azure/Azure-Landing-Zones#4174). The job attempt is part of the *local* path + # only, never the remote blob key, so a retried job cannot pick up residue from an earlier attempt. + $planDir = Join-Path $env:ALZ_AGENT_TEMP "alz-${{ parameters.purpose }}-$env:ALZ_BUILD_ID-$env:ALZ_JOB_ATTEMPT" + if (Test-Path -Path $planDir) { + Remove-Item -Path $planDir -Recurse -Force + } + New-Item -Path $planDir -ItemType Directory -Force | Out-Null + Write-Host "##vso[task.setvariable variable=alzPlanDir]$planDir" + Write-Host "Plan working directory: $planDir" + displayName: Prepare Plan Working Directory + env: + ALZ_AGENT_TEMP: $(Agent.TempDirectory) + ALZ_BUILD_ID: $(Build.BuildId) + ALZ_JOB_ATTEMPT: $(System.JobAttempt) diff --git a/pipelines/terraform/templates/helpers/terraform-plan.yaml b/pipelines/terraform/templates/helpers/terraform-plan.yaml index b216615..f18259b 100644 --- a/pipelines/terraform/templates/helpers/terraform-plan.yaml +++ b/pipelines/terraform/templates/helpers/terraform-plan.yaml @@ -40,6 +40,11 @@ steps: $env:ARM_OIDC_AZURE_SERVICE_CONNECTION_ID = $env:AZURESUBSCRIPTION_SERVICE_CONNECTION_ID $env:ARM_USE_OIDC = "true" + # Keep the plan file and its captured output outside the sources directory so neither can be + # swept into the published pipeline artifact (Azure/Azure-Landing-Zones#4174). + $planFile = Join-Path $env:ALZ_PLAN_DIR "tfplan" + $planLogFile = Join-Path $env:ALZ_PLAN_DIR "tfplan.log" + # Run Terraform Plan $command = "terraform" $arguments = @() @@ -50,7 +55,7 @@ steps: $arguments += "-var-file=$varFilePath" } - $arguments += "-out=tfplan" + $arguments += "-out=$planFile" $arguments += "-input=false" if ($env:TERRAFORM_ACTION -eq 'destroy') { @@ -58,8 +63,38 @@ steps: } Write-Host "Running: $command $arguments" - & $command $arguments + & $command $arguments *> $planLogFile + $planExitCode = $LASTEXITCODE + + # Only print the full captured output when the pipeline is explicitly configured to show plan + # content. A failed plan can still have streamed resource attribute values to the log before the + # error occurred, so the failure path prints just Terraform's own diagnostic block(s) - its boxed + # "Error:"/"Warning:" sections - never the resource diff. + $showPlanInPipelineLogs = $env:ALZ_SHOW_PLAN_IN_PIPELINE_LOGS -eq 'true' + if ($showPlanInPipelineLogs) { + Get-Content -Path $planLogFile | Write-Host + } elseif ($planExitCode -ne 0) { + $logContent = Get-Content -Path $planLogFile -Raw + $diagnosticBlocks = [regex]::Matches($logContent, "(?ms)^\u2577.*?^\u2575") + if ($diagnosticBlocks.Count -gt 0) { + foreach ($block in $diagnosticBlocks) { + Write-Host $block.Value + } + } else { + Write-Host "Terraform plan failed. Set SHOW_PLAN_IN_PIPELINE_LOGS to true on the pipeline variable group to see full plan output for troubleshooting." + } + } else { + Write-Host "Terraform plan succeeded. Set SHOW_PLAN_IN_PIPELINE_LOGS to true on the pipeline variable group to see the full plan output." + } + + Remove-Item -Path $planLogFile -Force -ErrorAction SilentlyContinue + + if ($planExitCode -ne 0) { + throw "Terraform plan failed with exit code $planExitCode." + } env: TERRAFORM_ACTION: ${{ coalesce(parameters.terraform_action, 'apply') }} ARM_OIDC_REQUEST_TOKEN: $(System.AccessToken) + ALZ_PLAN_DIR: $(alzPlanDir) + ALZ_SHOW_PLAN_IN_PIPELINE_LOGS: $(SHOW_PLAN_IN_PIPELINE_LOGS) diff --git a/tests/contract/Test-PipelinePlanStorage.ps1 b/tests/contract/Test-PipelinePlanStorage.ps1 new file mode 100644 index 0000000..a75471a --- /dev/null +++ b/tests/contract/Test-PipelinePlanStorage.ps1 @@ -0,0 +1,244 @@ +#Requires -Version 7.4 +<# +.SYNOPSIS + Contract tests for the secure Terraform plan storage feature (issue #4174) + as implemented in the Azure Pipelines templates. +.DESCRIPTION + Statically inspects the pipeline template YAML (cd-template.yaml, ci-template.yaml, + and the pipelines/terraform/templates/helpers/terraform-plan-*.yaml helpers) and + proves, without needing Azure/Azure DevOps credentials or a live pipeline run, that + the documented security contract for plan hand-off still holds in the template + SOURCE: + - the same computed container + an execution-bound (not attempt-bound) blob key + is used across upload/download/delete + - working directories only ever use $(Agent.TempDirectory) + - the plan file is excluded from the (unencrypted) pipeline artifact when secure + storage is enabled, and the legacy fallback still copies it in explicitly when + secure storage is disabled + - cleanup steps always run, even on failure + - the two feature flags are always compared with exact 'true' strings + - blob keys never embed the job attempt number + - there is no blob listing, no "latest" blob convention, and no shared-key + ("--account-key") auth anywhere + - every blob operation uses --auth-mode login, the upload is verified by + content-length comparison, and all three operations are bounded-retry loops + (not unbounded) + - no plan JSON ever ships in the published pipeline artifact + - upload/download failures fail closed (throw); a post-success delete failure + fails open (warns only, so a completed apply is never marked failed just + because blob cleanup could not run) + This is a static contract test, not an execution test: it proves the template TEXT + matches the contract. It does not prove runtime behavior - that is the job of the + runtime-evidence gate. +.PARAMETER RepoRoot + Path to the module root. Defaults to two levels above this script + (tests/contract/Test-PipelinePlanStorage.ps1 -> module root). +#> +[CmdletBinding()] +param( + [string]$RepoRoot = (Split-Path -Parent (Split-Path -Parent $PSScriptRoot)) +) + +$ErrorActionPreference = 'Stop' + +$templatesDir = Join-Path $RepoRoot 'pipelines/terraform/templates' +$helpersDir = Join-Path $templatesDir 'helpers' + +$paths = @{ + cd = Join-Path $templatesDir 'cd-template.yaml' + ci = Join-Path $templatesDir 'ci-template.yaml' + workspace = Join-Path $helpersDir 'terraform-plan-workspace.yaml' + upload = Join-Path $helpersDir 'terraform-plan-upload.yaml' + download = Join-Path $helpersDir 'terraform-plan-download.yaml' + delete = Join-Path $helpersDir 'terraform-plan-delete.yaml' + cleanup = Join-Path $helpersDir 'terraform-plan-cleanup.yaml' + plan = Join-Path $helpersDir 'terraform-plan.yaml' +} + +foreach ($p in $paths.GetEnumerator()) { + if (-not (Test-Path -Path $p.Value)) { + throw "Template not found: $($p.Value)" + } +} + +$raw = @{} +foreach ($p in $paths.GetEnumerator()) { + $raw[$p.Key] = Get-Content -Raw -Path $p.Value +} + +$script:failures = [System.Collections.Generic.List[string]]::new() +$script:passCount = 0 + +function Assert-Contract { + param( + [Parameter(Mandatory)][bool]$Condition, + [Parameter(Mandatory)][string]$Description + ) + if ($Condition) { + Write-Host " [PASS] $Description" -ForegroundColor Green + $script:passCount++ + } + else { + Write-Host " [FAIL] $Description" -ForegroundColor Red + $script:failures.Add($Description) + } +} + +Write-Host "=== Secure plan storage contract tests (issue #4174, Azure DevOps) ===" -ForegroundColor Cyan + +# --- 1. Same computed container + execution-bound key across upload/download/delete --- +Write-Host "`n-- Consistent, execution-bound blob addressing --" +$blobNamePattern = '\$blobName\s*=\s*"runs/\$env:ALZ_BUILD_ID/tfplan"' +foreach ($pair in @( + @{ Name = 'upload'; Raw = $raw.upload }, + @{ Name = 'download'; Raw = $raw.download }, + @{ Name = 'delete'; Raw = $raw.delete } + )) { + Assert-Contract -Condition ($pair.Raw -match $blobNamePattern) ` + -Description "The $($pair.Name) step keys the blob as 'runs/`$(Build.BuildId)/tfplan' (execution-bound, not attempt-bound)." + Assert-Contract -Condition ($pair.Raw -match 'ALZ_BUILD_ID:\s*\$\(Build\.BuildId\)') ` + -Description "The $($pair.Name) step maps ALZ_BUILD_ID from `$(Build.BuildId)." + Assert-Contract -Condition ($pair.Raw -match '\$container\s*=\s*\$env:ALZ_PLAN_CONTAINER' -and $pair.Raw -match 'ALZ_PLAN_CONTAINER:\s*\$\(PLAN_STORAGE_CONTAINER_NAME\)') ` + -Description "The $($pair.Name) step reads the container name from the PLAN_STORAGE_CONTAINER_NAME pipeline variable." + Assert-Contract -Condition ($pair.Raw -notmatch 'JobAttempt') ` + -Description "The $($pair.Name) step's blob key does not embed System.JobAttempt." + Assert-Contract -Condition ($pair.Raw -match "condition:\s*and\(succeeded\(\),\s*eq\(variables\['USE_STORAGE_ACCOUNT_FOR_PLAN'\],\s*'true'\)\)") ` + -Description "The $($pair.Name) step only runs when USE_STORAGE_ACCOUNT_FOR_PLAN is exactly 'true'." +} + +# --- 2. Agent.TempDirectory paths only --- +Write-Host "`n-- Working directories use Agent.TempDirectory only --" +Assert-Contract -Condition ($raw.workspace -match 'ALZ_AGENT_TEMP:\s*\$\(Agent\.TempDirectory\)') ` + -Description "The plan workspace step maps ALZ_AGENT_TEMP from `$(Agent.TempDirectory)." +Assert-Contract -Condition ($raw.workspace -match '\$planDir\s*=\s*Join-Path\s+\$env:ALZ_AGENT_TEMP') ` + -Description "The plan workspace step builds its working directory under `$env:ALZ_AGENT_TEMP." +Assert-Contract -Condition ($raw.workspace -match 'ALZ_JOB_ATTEMPT:\s*\$\(System\.JobAttempt\)' -and $raw.workspace -match '\$env:ALZ_JOB_ATTEMPT') ` + -Description "The plan workspace step includes the job attempt only in the *local* directory name, never the blob key." +Assert-Contract -Condition ($raw.workspace -match "task\.setvariable variable=alzPlanDir") ` + -Description "The plan workspace step publishes the resolved directory as the 'alzPlanDir' pipeline variable for later steps." + +# --- 3 & 4. Plan excluded from secure artifact staging; legacy fallback copies it explicitly --- +Write-Host "`n-- Plan file staging matches the USE_STORAGE_ACCOUNT_FOR_PLAN flag --" +Assert-Contract -Condition ($raw.cd -match '!\*\*/tfplan\s*\r?\n\s*!\*\*/tfplan\.json') ` + -Description "Create Module Artifact excludes **/tfplan and **/tfplan.json from the staged content." +Assert-Contract -Condition ($raw.cd -match 'Add Plan to Module Artifact' -and $raw.cd -match "ne\(variables\['USE_STORAGE_ACCOUNT_FOR_PLAN'\],\s*'true'\)") ` + -Description "Copying tfplan into the module artifact staging directory is guarded by 'USE_STORAGE_ACCOUNT_FOR_PLAN != true'." +Assert-Contract -Condition ($raw.cd.IndexOf('Copy-Item') -ge 0 -and $raw.cd.IndexOf('Add Plan to Module Artifact') -ge 0 -and ($raw.cd.IndexOf('Add Plan to Module Artifact') - $raw.cd.IndexOf('Copy-Item')) -gt 0 -and ($raw.cd.IndexOf('Add Plan to Module Artifact') - $raw.cd.IndexOf('Copy-Item')) -lt 400) ` + -Description "The legacy (non-storage-account) fallback explicitly copies tfplan into the staging directory." +Assert-Contract -Condition (([regex]::Matches($raw.cd, "targetPath:\s*'\`$\(Build\.ArtifactStagingDirectory\)'")).Count -ge 1) ` + -Description "Publish Module Artifact publishes `$(Build.ArtifactStagingDirectory) using the correctly-spelled predefined variable (not the ArtifactsStagingDirectory typo)." +Assert-Contract -Condition ($raw.cd -notmatch 'ArtifactsStagingDirectory') ` + -Description "No step references the non-existent `$(Build.ArtifactsStagingDirectory) variable." + +# --- 5. always() cleanup --- +Write-Host "`n-- Working directory cleanup always runs --" +Assert-Contract -Condition ($raw.cleanup -match 'condition:\s*always\(\)') ` + -Description "The Clean Up Plan Working Directory helper runs unconditionally (condition: always())." +Assert-Contract -Condition (([regex]::Matches($raw.cd, 'terraform-plan-cleanup\.yaml')).Count -ge 2) ` + -Description "cd-template.yaml invokes the cleanup helper in both the plan and apply jobs." +Assert-Contract -Condition (([regex]::Matches($raw.ci, 'terraform-plan-cleanup\.yaml')).Count -ge 1) ` + -Description "ci-template.yaml invokes the cleanup helper in the validate/plan job." + +# --- 6. Exact 'true' string comparisons for both feature flags --- +Write-Host "`n-- Feature flags are compared as exact 'true' strings --" +$flagNames = @('USE_STORAGE_ACCOUNT_FOR_PLAN', 'SHOW_PLAN_IN_PIPELINE_LOGS') +foreach ($fileInfo in @( + @{ Name = 'ci-template.yaml'; Raw = $raw.ci }, + @{ Name = 'cd-template.yaml'; Raw = $raw.cd }, + @{ Name = 'terraform-plan-upload.yaml'; Raw = $raw.upload }, + @{ Name = 'terraform-plan-download.yaml'; Raw = $raw.download }, + @{ Name = 'terraform-plan-delete.yaml'; Raw = $raw.delete } + )) { + foreach ($flag in $flagNames) { + $comparisonLines = ($fileInfo.Raw -split "`n") | Where-Object { $_ -match [regex]::Escape($flag) -and $_ -match '(eq\(|ne\(|-eq |==)' } + foreach ($line in $comparisonLines) { + Assert-Contract -Condition ($line -match "'true'") ` + -Description "$($fileInfo.Name): comparison against $flag uses a quoted 'true' string ($($line.Trim()))." + } + } +} + +# --- 7. No blob listing, no "latest" convention, no shared-key auth --- +Write-Host "`n-- No blob listing, no latest-blob convention, no shared-key auth --" +foreach ($pair in @( + @{ Name = 'upload'; Raw = $raw.upload }, + @{ Name = 'download'; Raw = $raw.download }, + @{ Name = 'delete'; Raw = $raw.delete } + )) { + Assert-Contract -Condition ($pair.Raw -notmatch 'az storage blob list') ` + -Description "The $($pair.Name) step does not list blobs (it addresses an exact known key only)." + Assert-Contract -Condition ($pair.Raw -notmatch '--account-key') ` + -Description "The $($pair.Name) step does not authenticate with a storage account shared key." + Assert-Contract -Condition ($pair.Raw -notmatch '(?i)\blatest\b') ` + -Description "The $($pair.Name) step does not use a mutable 'latest' blob alias." +} + +# --- 8. --auth-mode login + content validation + bounded retries --- +Write-Host "`n-- Auth mode, content validation, bounded retries --" +foreach ($pair in @( + @{ Name = 'upload'; Raw = $raw.upload; Command = 'az storage blob upload' }, + @{ Name = 'download'; Raw = $raw.download; Command = 'az storage blob download' }, + @{ Name = 'delete'; Raw = $raw.delete; Command = 'az storage blob delete' } + )) { + Assert-Contract -Condition ($pair.Raw -match [regex]::Escape($pair.Command)) ` + -Description "The $($pair.Name) step calls '$($pair.Command)'." + Assert-Contract -Condition ($pair.Raw -match '--auth-mode login') ` + -Description "The $($pair.Name) step authenticates with --auth-mode login (Microsoft Entra ID, not a shared key)." + Assert-Contract -Condition ($pair.Raw -match '\$maxAttempts\s*=\s*\d+' -and $pair.Raw -match 'for\s*\(\$attempt\s*=\s*1;\s*\$attempt\s*-le\s*\$maxAttempts') ` + -Description "The $($pair.Name) step retries a bounded number of times (not an unbounded loop)." +} +Assert-Contract -Condition ($raw.upload -match 'az storage blob show' -and $raw.upload -match '--query properties\.contentLength') ` + -Description "The upload step verifies the uploaded blob's content length against the local plan file." +Assert-Contract -Condition ($raw.download -match '\(Get-Item\s+-Path\s+\$planFile\)\.Length\s+-eq\s+0') ` + -Description "The download step rejects an empty downloaded plan file." + +# --- 9. No plan JSON ships in the published artifact --- +Write-Host "`n-- No plan JSON ships in the published pipeline artifact --" +Assert-Contract -Condition ($raw.cd -notmatch "targetPath:\s*'tfplan\.json'" -and $raw.cd -notmatch "artifact:\s*'plan_") ` + -Description "cd-template.yaml never publishes a standalone plan/tfplan.json artifact." +Assert-Contract -Condition ($raw.ci -notmatch 'PublishPipelineArtifact') ` + -Description "ci-template.yaml no longer publishes the plan JSON as a pipeline artifact (Terraform Plan Summary only prints counts to the log)." +Assert-Contract -Condition ($raw.ci -match [regex]::Escape('show -json "$(alzPlanDir)/tfplan" > "$(alzPlanDir)/tfplan.json"')) ` + -Description "ci-template.yaml's Terraform Plan Summary writes tfplan.json under the agent-temp plan directory, never under the sources/artifact directory." + +# --- 10. Fail-closed upload/download, fail-open post-success delete --- +Write-Host "`n-- Fail-closed upload/download; fail-open (warn-only) cleanup delete --" +Assert-Contract -Condition ($raw.upload -match 'if\s*\(-not\s*\$uploaded\)\s*\{\s*\r?\n\s*throw') ` + -Description "Upload failure throws (fails the pipeline closed) rather than silently continuing." +Assert-Contract -Condition ($raw.download -match 'if\s*\(-not\s*\$downloaded\)\s*\{\s*\r?\n\s*throw') ` + -Description "Download failure throws (fails the pipeline closed) rather than silently continuing." +Assert-Contract -Condition ($raw.download -match 'Test-Path[^\r\n]*\$planFile\)[\s\S]{0,40}-or[\s\S]{0,80}-eq 0\)\s*\{\s*\r?\n\s*throw') ` + -Description "A missing or empty downloaded plan file throws (fails the pipeline closed)." +Assert-Contract -Condition ($raw.delete -match 'if\s*\(-not\s*\$deleted\)\s*\{\s*\r?\n\s*Write-Warning') ` + -Description "Post-apply delete failure only warns (a completed apply is not marked failed just because blob cleanup could not run)." +$applyIdx = $raw.cd.IndexOf('terraform-apply.yaml') +$deleteIdx = $raw.cd.IndexOf('terraform-plan-delete.yaml') +Assert-Contract -Condition ($applyIdx -ge 0 -and $deleteIdx -gt $applyIdx) ` + -Description "cd-template.yaml invokes terraform-plan-delete.yaml only after terraform-apply.yaml, i.e. only on the happy path (a failed apply stops the job before delete runs)." + +# --- 11. Failed-plan output is redacted unless explicitly enabled --- +Write-Host "`n-- Failed-plan log output respects SHOW_PLAN_IN_PIPELINE_LOGS --" +$failureBlockMatch = [regex]::Match($raw.plan, '(?s)\}\s*elseif\s*\(\$planExitCode\s*-ne\s*0\)\s*\{.*?\}\s*else\s*\{.*?\}') +Assert-Contract -Condition $failureBlockMatch.Success ` + -Description "terraform-plan.yaml: the three-way branch (shown / failed-redacted / succeeded-redacted) was found for inspection." +Assert-Contract -Condition ($raw.plan -match 'Get-Content -Path \$planLogFile \| Write-Host') ` + -Description "terraform-plan.yaml: the full captured plan log is only ever dumped inside the `$showPlanInPipelineLogs branch." +Assert-Contract -Condition ($raw.plan -match 'diagnosticBlocks') ` + -Description "terraform-plan.yaml: when the flag is not true and the plan failed, only Terraform's own diagnostic block(s) are extracted from the log." +if ($failureBlockMatch.Success) { + Assert-Contract -Condition ($failureBlockMatch.Value -notmatch 'Get-Content -Path \$planLogFile \| Write-Host') ` + -Description "terraform-plan.yaml: the redacted failure branch never falls back to dumping the entire raw captured log." +} +Assert-Contract -Condition ($raw.plan -match 'Remove-Item -Path \$planLogFile -Force -ErrorAction SilentlyContinue') ` + -Description "terraform-plan.yaml: the captured plan log file itself is removed after being processed, so it cannot leak via a later artifact-staging step." + +# --- Summary --- +Write-Host "`n=== Summary: $($script:passCount) passed, $($script:failures.Count) failed ===" -ForegroundColor Cyan +if ($script:failures.Count -gt 0) { + Write-Host "`nFailed checks:" -ForegroundColor Red + foreach ($f in $script:failures) { + Write-Host " - $f" -ForegroundColor Red + } + exit 1 +} +exit 0 diff --git a/variables.cicd.tf b/variables.cicd.tf index 92b0a4e..59b2b5a 100644 --- a/variables.cicd.tf +++ b/variables.cicd.tf @@ -89,3 +89,26 @@ variable "example_module_path" { default = null description = "The absolute path to the example module to seed into the created repository." } + +variable "plan_storage_retention_days" { + type = number + default = 7 + description = "The number of days after which abandoned Terraform plan base blobs, snapshots, and previous versions are eligible for lifecycle deletion." + + validation { + condition = var.plan_storage_retention_days > 0 && floor(var.plan_storage_retention_days) == var.plan_storage_retention_days + error_message = "plan_storage_retention_days must be a positive whole number." + } +} + +variable "show_plan_in_pipeline_logs" { + type = bool + default = false + description = "Whether to print the full Terraform plan in pipeline logs. Enabling this can expose sensitive values." +} + +variable "use_storage_account_for_plan" { + type = bool + default = true + description = "Whether to use the Terraform state Storage Account for secure plan hand-off. Set to false to use the legacy CI/CD artifact hand-off." +} diff --git a/variables.environments.tf b/variables.environments.tf index 179baec..14db978 100644 --- a/variables.environments.tf +++ b/variables.environments.tf @@ -83,4 +83,13 @@ DESCRIPTION condition = alltrue([for k, v in var.environments : v.identities.read.enabled || v.identities.write.enabled]) error_message = "Each environment must have at least one identity enabled (read or write)." } + validation { + condition = alltrue([ + for k, v in var.environments : ( + !(contains(coalesce(v.identities.read.allowed_template_keys, ["ci", "cd"]), "cd") && contains(coalesce(v.identities.write.allowed_template_keys, ["cd"]), "cd")) + || (v.identities.read.enabled && v.identities.write.enabled) + ) + ]) + error_message = "When the 'cd' workflow is configured to use both the read and write identities (bundled CD), both identities.read.enabled and identities.write.enabled must be true." + } } From 6bc82ed3519229bdaa65b868563e246a615a3907 Mon Sep 17 00:00:00 2001 From: "Prince Nagar (BEYONDSOFT CONSULTING INC)" Date: Wed, 2 Sep 2026 16:36:00 +0530 Subject: [PATCH 2/2] docs: clarify plan retention approval window Explain that retention must exceed the expected plan-to-apply approval wait and that expired plans should be regenerated. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- README.md | 4 ++-- _header.md | 2 +- variables.cicd.tf | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index d85a469..e093587 100644 --- a/README.md +++ b/README.md @@ -39,7 +39,7 @@ This Azure Verified Module (AVM) pattern module bootstraps a complete, opinionat - **Legacy fallback** — set `use_storage_account_for_plan = false` to revert to shipping the plan inside the Azure Pipelines build artifact. This is less secure (plan contents can include sensitive values and are retained per your organization's pipeline artifact retention policy) and is provided only for compatibility with self-managed/BYO template repos that haven't adopted the new templates. - **Custom template repositories are not auto-secured** — if you set `azuredevops_existing_template_repository_name` or a custom pipeline template path, the module does not modify your pipeline YAML. You must adopt the upload/download/delete steps yourself for storage-backed hand-off to apply. - **`show_plan_in_pipeline_logs`** — defaults to `false`. Enabling it prints the full plan to the pipeline log, visible to anyone with read access to the project/pipeline runs. Only enable if your organization has explicitly accepted that exposure. -- **`plan_storage_retention_days`** (default `7`) — a storage lifecycle policy rule deletes abandoned plan blobs, snapshots, and previous versions after this many days. This is a backstop only; successful applies delete their own plan blob immediately. +- **`plan_storage_retention_days`** (default `7`) — a storage lifecycle policy rule deletes abandoned plan blobs, snapshots, and previous versions after this many days. This is a backstop only; successful applies delete their own plan blob immediately. Choose a value longer than the longest expected plan-to-apply approval wait; if a plan expires before approval, rerun the pipeline to generate a fresh plan. - **Recoverability** — the plan container inherits the storage account's blob versioning/soft-delete settings, so an accidentally-deleted plan blob may still be recoverable within your soft-delete window. - **Trusted-admin threat boundary** — this feature keeps plan contents out of Azure Pipelines artifact storage. It does not protect against an Azure user with Storage Blob Data Contributor/Owner-equivalent access to the storage account — the same trust boundary as Terraform remote state. - **Stale/concurrent plans** — the `apply` stage always downloads the blob written by its own `plan` stage run (keyed by `$(Build.BuildId)`), never "the latest" blob, so a concurrent or superseded run cannot apply a stranger's plan. @@ -470,7 +470,7 @@ Default: `null` ### [plan\_storage\_retention\_days](#input\_plan\_storage\_retention\_days) -Description: The number of days after which abandoned Terraform plan base blobs, snapshots, and previous versions are eligible for lifecycle deletion. +Description: The number of days after which abandoned Terraform plan base blobs, snapshots, and previous versions are eligible for lifecycle deletion. Choose a value longer than the longest expected plan-to-apply approval wait; expired plans must be regenerated. Type: `number` diff --git a/_header.md b/_header.md index c47f72d..28ba32d 100644 --- a/_header.md +++ b/_header.md @@ -37,7 +37,7 @@ This Azure Verified Module (AVM) pattern module bootstraps a complete, opinionat - **Legacy fallback** — set `use_storage_account_for_plan = false` to revert to shipping the plan inside the Azure Pipelines build artifact. This is less secure (plan contents can include sensitive values and are retained per your organization's pipeline artifact retention policy) and is provided only for compatibility with self-managed/BYO template repos that haven't adopted the new templates. - **Custom template repositories are not auto-secured** — if you set `azuredevops_existing_template_repository_name` or a custom pipeline template path, the module does not modify your pipeline YAML. You must adopt the upload/download/delete steps yourself for storage-backed hand-off to apply. - **`show_plan_in_pipeline_logs`** — defaults to `false`. Enabling it prints the full plan to the pipeline log, visible to anyone with read access to the project/pipeline runs. Only enable if your organization has explicitly accepted that exposure. -- **`plan_storage_retention_days`** (default `7`) — a storage lifecycle policy rule deletes abandoned plan blobs, snapshots, and previous versions after this many days. This is a backstop only; successful applies delete their own plan blob immediately. +- **`plan_storage_retention_days`** (default `7`) — a storage lifecycle policy rule deletes abandoned plan blobs, snapshots, and previous versions after this many days. This is a backstop only; successful applies delete their own plan blob immediately. Choose a value longer than the longest expected plan-to-apply approval wait; if a plan expires before approval, rerun the pipeline to generate a fresh plan. - **Recoverability** — the plan container inherits the storage account's blob versioning/soft-delete settings, so an accidentally-deleted plan blob may still be recoverable within your soft-delete window. - **Trusted-admin threat boundary** — this feature keeps plan contents out of Azure Pipelines artifact storage. It does not protect against an Azure user with Storage Blob Data Contributor/Owner-equivalent access to the storage account — the same trust boundary as Terraform remote state. - **Stale/concurrent plans** — the `apply` stage always downloads the blob written by its own `plan` stage run (keyed by `$(Build.BuildId)`), never "the latest" blob, so a concurrent or superseded run cannot apply a stranger's plan. diff --git a/variables.cicd.tf b/variables.cicd.tf index 59b2b5a..73210f9 100644 --- a/variables.cicd.tf +++ b/variables.cicd.tf @@ -93,7 +93,7 @@ variable "example_module_path" { variable "plan_storage_retention_days" { type = number default = 7 - description = "The number of days after which abandoned Terraform plan base blobs, snapshots, and previous versions are eligible for lifecycle deletion." + description = "The number of days after which abandoned Terraform plan base blobs, snapshots, and previous versions are eligible for lifecycle deletion. Choose a value longer than the longest expected plan-to-apply approval wait; expired plans must be regenerated." validation { condition = var.plan_storage_retention_days > 0 && floor(var.plan_storage_retention_days) == var.plan_storage_retention_days