From 8f89e586cea78c89440609ecfe9f48caea9c7800 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 19 May 2026 15:44:22 +0000 Subject: [PATCH] ci: fix marketplace-id false positives + clean tfsec/tflint findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The marketplace-id-consistency gate was grepping every publisher = "..." in a module file and taking the alphabetical first — which caught the Canonical bastion VM image in ha-hot-hot/azure and the Microsoft.Azure.Extensions VMSS extensions in unlimited-scale/azure as "drift" even though neither is a HailBytes marketplace image. Scope the check to publishers inside the marketplace_plans local block via an awk brace-depth walker so legitimate non-marketplace publishers in the same file no longer trip it. While the CI was open, tighten everything else surfaced by the same run: - tfsec ignores moved inline (above the offending attribute, not the resource) with rationale for: customer-facing ALB (intentionally public, gated by allowed_cidrs SG), SNS topic CMK (opt-in via var.enable_customer_managed_key), ALB access-log bucket SSE (AWS only supports SSE-S3 on ALB log buckets), flow-logs IAM DescribeLogGroups wildcard (AWS rejects ARN scoping on that action). - Key Vault network_acls block added in ha-hot-hot/azure and unlimited-scale/azure with new vars key_vault_network_default_action (default Allow, preserves current behavior) and key_vault_ip_rules, letting customers opt into a Deny default without a breaking change. Wrappers forward both vars. - ha-hot-hot/azure: NSG now actually attaches to lb_subnet_id via azurerm_subnet_network_security_group_association — previously the NSG was created with allow-https rules but never associated, so the rules enforced nothing. - unlimited-scale/azure: var.allowed_cidrs was declared but unused (silent security gap in the example). Now wired through a new NSG built from allowed_cidrs, optionally associated with vm_subnet_id via associate_vm_subnet_nsg (default true; set false when the customer's landing zone already manages the subnet NSG). - Drop unused declarations flagged by tflint: local.db_arn in ha-hot-hot/azure, local.db_id in ha-hot-hot/aws, data.azurerm_subscription.current in ha-hot-hot/azure. - Replace the deprecated aquasecurity/tfsec-sarif-action@v0.1.4 (bundles a Node 16 runtime) with a direct tfsec binary install in the workflow. Same SARIF upload path, no Node 16 deprecation banner. --- .github/workflows/ci.yml | 67 ++++++++++++++++------ modules/asm-azure-autoscale/main.tf | 53 +++++++++-------- modules/asm-azure-autoscale/variables.tf | 18 ++++++ modules/asm-azure-ha/main.tf | 46 ++++++++------- modules/asm-azure-ha/variables.tf | 12 ++++ modules/ha-hot-hot/aws/main.tf | 16 ++++-- modules/ha-hot-hot/azure/main.tf | 23 +++++++- modules/ha-hot-hot/azure/variables.tf | 18 ++++++ modules/sat-azure-autoscale/main.tf | 53 +++++++++-------- modules/sat-azure-autoscale/variables.tf | 18 ++++++ modules/sat-azure-ha/main.tf | 46 ++++++++------- modules/sat-azure-ha/variables.tf | 12 ++++ modules/unlimited-scale/aws/main.tf | 47 +++++++++++---- modules/unlimited-scale/azure/main.tf | 49 ++++++++++++++++ modules/unlimited-scale/azure/variables.tf | 24 ++++++++ 15 files changed, 374 insertions(+), 128 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5871f33..df897cd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -197,17 +197,26 @@ jobs: # Upload SARIF for code-scanning UI. Module authors get the # findings in the PR file-view, not just the run summary. security-events: write + env: + TFSEC_VERSION: v1.28.13 steps: - name: Checkout uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - - name: Run tfsec - uses: aquasecurity/tfsec-sarif-action@9a83b5c3524f825c020e356335855741fd02745f # v0.1.4 - with: - sarif_file: tfsec.sarif - working_directory: modules - # Soft-fail on the SARIF run; the gate below uses tfsec - # directly so we can scope severity and inline-disable rules. + - name: Install tfsec + # aquasecurity/tfsec-sarif-action@v0.1.4 was the previous step here, + # but it bundles a Node 16 runtime that GitHub flags as deprecated. + # tfsec itself (the binary) is fine — install it directly from the + # GitHub release and we sidestep the action runtime entirely. + run: | + set -eo pipefail + curl -sSfL -o /usr/local/bin/tfsec \ + "https://github.com/aquasecurity/tfsec/releases/download/${TFSEC_VERSION}/tfsec-linux-amd64" + chmod +x /usr/local/bin/tfsec + tfsec --version + + - name: Run tfsec (SARIF, no fail) + run: tfsec modules --format sarif --out tfsec.sarif --soft-fail - name: Upload SARIF to GitHub code-scanning uses: github/codeql-action/upload-sarif@28deaeda66b76a05916b6923827895f2b14ab387 # v3.27.5 @@ -220,14 +229,7 @@ jobs: # The SARIF path uploads everything; here we re-run with --severity # so the build fails on HIGH+ and tolerates MEDIUM/LOW that are # already triaged. - run: | - docker run --rm \ - -v "${{ github.workspace }}:/src" \ - aquasec/tfsec:v1.28.13 \ - /src/modules \ - --minimum-severity HIGH \ - --format default \ - --no-color + run: tfsec modules --minimum-severity HIGH --format default --no-color # --------------------------------------------------------------------- # Examples validation — every modules/*/aws/examples/* and @@ -331,11 +333,40 @@ jobs: fail=0 for tf in $(grep -rl 'marketplace_plans' modules/ --include='*.tf'); do - got_publisher=$(grep -oP 'publisher\s*=\s*"\K[^"]+' "$tf" | sort -u | head -1) - if [ -n "${got_publisher}" ] && [ "${got_publisher}" != "${want_publisher}" ]; then - echo "::error file=${tf}::Azure publisher drift: want=${want_publisher} got=${got_publisher}" + # Scope the publisher check to the marketplace_plans local block + # only. The same .tf files legitimately reference other publishers + # (Canonical for a Postgres-on-VM bastion image, Microsoft.Azure.Extensions + # for VMSS CustomScript extensions) and we don't want those to + # trigger a marketplace-drift error. + in_block_publishers=$(awk ' + /marketplace_plans[[:space:]]*=[[:space:]]*\{/ { depth=1; next } + depth > 0 { + for (i = 1; i <= length($0); i++) { + c = substr($0, i, 1) + if (c == "{") depth++ + else if (c == "}") { depth--; if (depth == 0) exit } + } + if (match($0, /publisher[[:space:]]*=[[:space:]]*"[^"]+"/)) { + s = substr($0, RSTART, RLENGTH) + sub(/^publisher[[:space:]]*=[[:space:]]*"/, "", s) + sub(/"$/, "", s) + print s + } + } + ' "$tf" | sort -u) + + if [ -z "${in_block_publishers}" ]; then + echo "::error file=${tf}::No publisher found inside marketplace_plans block" fail=1 + continue fi + while IFS= read -r got_publisher; do + if [ "${got_publisher}" != "${want_publisher}" ]; then + echo "::error file=${tf}::Azure publisher drift inside marketplace_plans: want=${want_publisher} got=${got_publisher}" + fail=1 + fi + done <<< "${in_block_publishers}" + if grep -q "${want_asm_offer}" "$tf" && ! grep -q "${want_sat_offer}" "$tf"; then # Should mention both products' offers. echo "::error file=${tf}::Azure SAT offer missing — should reference both ${want_asm_offer} and ${want_sat_offer}" diff --git a/modules/asm-azure-autoscale/main.tf b/modules/asm-azure-autoscale/main.tf index 52591ec..ba00420 100644 --- a/modules/asm-azure-autoscale/main.tf +++ b/modules/asm-azure-autoscale/main.tf @@ -3,30 +3,35 @@ module "this" { product = "asm" - resource_group_name = var.resource_group_name - location = var.location - vm_subnet_id = var.vm_subnet_id - db_delegated_subnet_id = var.db_delegated_subnet_id - private_dns_zone_id = var.private_dns_zone_id - allowed_cidrs = var.allowed_cidrs - admin_username = var.admin_username - ssh_public_key = var.ssh_public_key - vmss_min_count = var.vmss_min_count - vmss_max_count = var.vmss_max_count - vmss_default_count = var.vmss_default_count - vm_size = var.vm_size - target_cpu_percent = var.target_cpu_percent - db_sku_name = var.db_sku_name - db_storage_mb = var.db_storage_mb - db_version = var.db_version - db_backup_retention_days = var.db_backup_retention_days - db_replica_count = var.db_replica_count - environment = var.environment - name_prefix = var.name_prefix - alert_email = var.alert_email - accept_marketplace_terms = var.accept_marketplace_terms - marketplace_sku_override = var.marketplace_sku_override - marketplace_image_version = var.marketplace_image_version + resource_group_name = var.resource_group_name + location = var.location + vm_subnet_id = var.vm_subnet_id + db_delegated_subnet_id = var.db_delegated_subnet_id + private_dns_zone_id = var.private_dns_zone_id + allowed_cidrs = var.allowed_cidrs + admin_username = var.admin_username + ssh_public_key = var.ssh_public_key + + # Key Vault network ACL + VMSS subnet NSG association + key_vault_network_default_action = var.key_vault_network_default_action + key_vault_ip_rules = var.key_vault_ip_rules + associate_vm_subnet_nsg = var.associate_vm_subnet_nsg + vmss_min_count = var.vmss_min_count + vmss_max_count = var.vmss_max_count + vmss_default_count = var.vmss_default_count + vm_size = var.vm_size + target_cpu_percent = var.target_cpu_percent + db_sku_name = var.db_sku_name + db_storage_mb = var.db_storage_mb + db_version = var.db_version + db_backup_retention_days = var.db_backup_retention_days + db_replica_count = var.db_replica_count + environment = var.environment + name_prefix = var.name_prefix + alert_email = var.alert_email + accept_marketplace_terms = var.accept_marketplace_terms + marketplace_sku_override = var.marketplace_sku_override + marketplace_image_version = var.marketplace_image_version # Patching and migration safety create_backup_storage_account = var.create_backup_storage_account diff --git a/modules/asm-azure-autoscale/variables.tf b/modules/asm-azure-autoscale/variables.tf index 82e0cd1..1c4530f 100644 --- a/modules/asm-azure-autoscale/variables.tf +++ b/modules/asm-azure-autoscale/variables.tf @@ -18,6 +18,24 @@ variable "admin_username" { type = string } variable "ssh_public_key" { type = string } +variable "key_vault_network_default_action" { + description = "Default action for the Key Vault network ACL. 'Allow' preserves the pre-network-ACL behavior; set 'Deny' once you've added the operator IP to key_vault_ip_rules and the Microsoft.KeyVault service endpoint on vm_subnet_id." + type = string + default = "Allow" +} + +variable "key_vault_ip_rules" { + description = "IPv4 addresses or CIDRs allowed to reach the Key Vault data plane. Required only when key_vault_network_default_action = Deny and you don't have Private Link configured." + type = list(string) + default = [] +} + +variable "associate_vm_subnet_nsg" { + description = "Associate the module-managed NSG (built from allowed_cidrs) with vm_subnet_id. Set false if the subnet already has an NSG attached by your landing-zone tooling." + type = bool + default = true +} + variable "vmss_min_count" { type = number default = 3 diff --git a/modules/asm-azure-ha/main.tf b/modules/asm-azure-ha/main.tf index 7b12179..ae27cbc 100644 --- a/modules/asm-azure-ha/main.tf +++ b/modules/asm-azure-ha/main.tf @@ -3,27 +3,31 @@ module "this" { product = "asm" - resource_group_name = var.resource_group_name - location = var.location - vm_subnet_id = var.vm_subnet_id - db_delegated_subnet_id = var.db_delegated_subnet_id - private_dns_zone_id = var.private_dns_zone_id - lb_subnet_id = var.lb_subnet_id - allowed_cidrs = var.allowed_cidrs - admin_username = var.admin_username - ssh_public_key = var.ssh_public_key - environment = var.environment - name_prefix = var.name_prefix - vm_size = var.vm_size - data_disk_size_gb = var.data_disk_size_gb - db_sku_name = var.db_sku_name - db_storage_mb = var.db_storage_mb - db_version = var.db_version - db_backup_retention_days = var.db_backup_retention_days - db_high_availability_mode = var.db_high_availability_mode - accept_marketplace_terms = var.accept_marketplace_terms - marketplace_sku_override = var.marketplace_sku_override - marketplace_image_version = var.marketplace_image_version + resource_group_name = var.resource_group_name + location = var.location + vm_subnet_id = var.vm_subnet_id + db_delegated_subnet_id = var.db_delegated_subnet_id + private_dns_zone_id = var.private_dns_zone_id + lb_subnet_id = var.lb_subnet_id + allowed_cidrs = var.allowed_cidrs + admin_username = var.admin_username + ssh_public_key = var.ssh_public_key + + # Key Vault network ACL + key_vault_network_default_action = var.key_vault_network_default_action + key_vault_ip_rules = var.key_vault_ip_rules + environment = var.environment + name_prefix = var.name_prefix + vm_size = var.vm_size + data_disk_size_gb = var.data_disk_size_gb + db_sku_name = var.db_sku_name + db_storage_mb = var.db_storage_mb + db_version = var.db_version + db_backup_retention_days = var.db_backup_retention_days + db_high_availability_mode = var.db_high_availability_mode + accept_marketplace_terms = var.accept_marketplace_terms + marketplace_sku_override = var.marketplace_sku_override + marketplace_image_version = var.marketplace_image_version # Patching and migration safety db_mode = var.db_mode diff --git a/modules/asm-azure-ha/variables.tf b/modules/asm-azure-ha/variables.tf index 15acbdd..fb8c37a 100644 --- a/modules/asm-azure-ha/variables.tf +++ b/modules/asm-azure-ha/variables.tf @@ -42,6 +42,18 @@ variable "ssh_public_key" { type = string } +variable "key_vault_network_default_action" { + description = "Default action for the Key Vault network ACL. 'Allow' preserves the pre-network-ACL behavior; set 'Deny' once you've added the operator IP to key_vault_ip_rules and the Microsoft.KeyVault service endpoint on vm_subnet_id." + type = string + default = "Allow" +} + +variable "key_vault_ip_rules" { + description = "IPv4 addresses or CIDRs allowed to reach the Key Vault data plane. Required only when key_vault_network_default_action = Deny and you don't have Private Link configured." + type = list(string) + default = [] +} + variable "environment" { type = string default = "prod" diff --git a/modules/ha-hot-hot/aws/main.tf b/modules/ha-hot-hot/aws/main.tf index f20e880..cb93bb0 100644 --- a/modules/ha-hot-hot/aws/main.tf +++ b/modules/ha-hot-hot/aws/main.tf @@ -40,7 +40,6 @@ locals { db_host = coalesce(one(aws_db_instance.main[*].address), one(aws_instance.db_ec2[*].private_ip)) db_port = local.use_rds ? coalesce(one(aws_db_instance.main[*].port), 5432) : 5432 db_arn = coalesce(one(aws_db_instance.main[*].arn), one(aws_instance.db_ec2[*].arn)) - db_id = coalesce(one(aws_db_instance.main[*].id), one(aws_instance.db_ec2[*].id)) # Redis is required for HA: both app instances must share session state and # the worker-lock heartbeat through the same Redis. The module provisions @@ -642,8 +641,12 @@ data "aws_region" "current" {} # ----- ALB ----- resource "aws_lb" "main" { - name = "${local.name_prefix}-alb" - internal = false + name = "${local.name_prefix}-alb" + # The HailBytes SAT / ASM console is customer-facing by design; the ALB sits + # in public subnets behind a security group that only allows ingress from + # var.allowed_cidrs. Customers who want a fully private deployment can front + # the module with their own internal ALB or API Gateway. + internal = false #tfsec:ignore:aws-elb-alb-not-public load_balancer_type = "application" security_groups = [aws_security_group.alb.id] subnets = var.public_subnet_ids @@ -742,8 +745,11 @@ resource "aws_wafv2_web_acl_association" "alb" { # ----- SNS topic for patching alerts ----- resource "aws_sns_topic" "alerts" { - name = "${local.name_prefix}-alerts" - kms_master_key_id = var.enable_customer_managed_key ? aws_kms_key.main[0].id : "alias/aws/sns" + name = "${local.name_prefix}-alerts" + # CMK is opt-in via var.enable_customer_managed_key. tfsec's static analysis + # evaluates the false branch ("alias/aws/sns") of the ternary; customers who + # set enable_customer_managed_key = true get the module-owned CMK. + kms_master_key_id = var.enable_customer_managed_key ? aws_kms_key.main[0].id : "alias/aws/sns" #tfsec:ignore:aws-sns-topic-encryption-use-cmk tags = local.common_tags } diff --git a/modules/ha-hot-hot/azure/main.tf b/modules/ha-hot-hot/azure/main.tf index 76cfaf4..2e29a78 100644 --- a/modules/ha-hot-hot/azure/main.tf +++ b/modules/ha-hot-hot/azure/main.tf @@ -40,7 +40,6 @@ locals { use_vm_db = var.db_mode == "vm" db_host = local.use_flexible_server ? one(azurerm_postgresql_flexible_server.main[*].fqdn) : one(azurerm_linux_virtual_machine.db_vm[*].private_ip_address) - db_arn = local.use_flexible_server ? one(azurerm_postgresql_flexible_server.main[*].id) : one(azurerm_linux_virtual_machine.db_vm[*].id) create_backup_storage = var.create_backup_storage_account backup_storage_account_name = local.create_backup_storage ? azurerm_storage_account.backup[0].name : var.backup_storage_account_name @@ -81,6 +80,18 @@ resource "azurerm_key_vault" "main" { soft_delete_retention_days = 30 enable_rbac_authorization = true tags = local.common_tags + + network_acls { + # default_action is wired through var.key_vault_network_default_action so + # customers can opt into "Deny" once they've added the operator IP to + # key_vault_ip_rules and a Microsoft.KeyVault service endpoint on + # vm_subnet_id. Defaulting to "Allow" preserves pre-ACL behavior; + # data-plane access is still gated by RBAC and the AzureServices bypass. + default_action = var.key_vault_network_default_action #tfsec:ignore:azure-keyvault-specify-network-acl + bypass = "AzureServices" + ip_rules = var.key_vault_ip_rules + virtual_network_subnet_ids = [var.vm_subnet_id] + } } resource "random_password" "db" { @@ -140,6 +151,14 @@ resource "azurerm_network_security_rule" "lb_https_in" { network_security_group_name = azurerm_network_security_group.lb.name } +# Attach the NSG to the LB frontend subnet so the allow-https-* rules +# actually take effect. Without this association the rules exist on the +# NSG but the subnet routes traffic unfiltered. +resource "azurerm_subnet_network_security_group_association" "lb" { + subnet_id = var.lb_subnet_id + network_security_group_id = azurerm_network_security_group.lb.id +} + # ----- Load Balancer ----- resource "azurerm_public_ip" "lb" { @@ -572,8 +591,6 @@ resource "azurerm_role_assignment" "db_vm_kv_reader" { # ----- Backup Storage Account + immutable container ----- -data "azurerm_subscription" "current" {} - resource "azurerm_storage_account" "backup" { count = local.create_backup_storage ? 1 : 0 name = coalesce(var.backup_storage_account_name, substr(replace("${local.name_prefix}backup", "-", ""), 0, 24)) diff --git a/modules/ha-hot-hot/azure/variables.tf b/modules/ha-hot-hot/azure/variables.tf index 692a3b8..0f85355 100644 --- a/modules/ha-hot-hot/azure/variables.tf +++ b/modules/ha-hot-hot/azure/variables.tf @@ -48,6 +48,24 @@ variable "ssh_public_key" { type = string } +# ----- Key Vault network ACL ----- + +variable "key_vault_network_default_action" { + description = "Default action for the Key Vault network ACL. 'Allow' preserves the pre-network-ACL behavior (public endpoint open, RBAC-gated); set 'Deny' once you've added the operator IP to key_vault_ip_rules and the Microsoft.KeyVault service endpoint on vm_subnet_id. AzureServices bypass is always on so the VMs' managed identities can read secrets either way." + type = string + default = "Allow" + validation { + condition = contains(["Allow", "Deny"], var.key_vault_network_default_action) + error_message = "key_vault_network_default_action must be one of: Allow, Deny." + } +} + +variable "key_vault_ip_rules" { + description = "IPv4 addresses or CIDRs allowed to reach the Key Vault data plane (typically the operator IP running terraform apply, or your bastion's egress NAT). Required only when default_action = Deny and you don't have Private Link configured." + type = list(string) + default = [] +} + # ----- Optional ----- variable "environment" { diff --git a/modules/sat-azure-autoscale/main.tf b/modules/sat-azure-autoscale/main.tf index f33d71e..4fd7e1a 100644 --- a/modules/sat-azure-autoscale/main.tf +++ b/modules/sat-azure-autoscale/main.tf @@ -3,30 +3,35 @@ module "this" { product = "sat" - resource_group_name = var.resource_group_name - location = var.location - vm_subnet_id = var.vm_subnet_id - db_delegated_subnet_id = var.db_delegated_subnet_id - private_dns_zone_id = var.private_dns_zone_id - allowed_cidrs = var.allowed_cidrs - admin_username = var.admin_username - ssh_public_key = var.ssh_public_key - vmss_min_count = var.vmss_min_count - vmss_max_count = var.vmss_max_count - vmss_default_count = var.vmss_default_count - vm_size = var.vm_size - target_cpu_percent = var.target_cpu_percent - db_sku_name = var.db_sku_name - db_storage_mb = var.db_storage_mb - db_version = var.db_version - db_backup_retention_days = var.db_backup_retention_days - db_replica_count = var.db_replica_count - environment = var.environment - name_prefix = var.name_prefix - alert_email = var.alert_email - accept_marketplace_terms = var.accept_marketplace_terms - marketplace_sku_override = var.marketplace_sku_override - marketplace_image_version = var.marketplace_image_version + resource_group_name = var.resource_group_name + location = var.location + vm_subnet_id = var.vm_subnet_id + db_delegated_subnet_id = var.db_delegated_subnet_id + private_dns_zone_id = var.private_dns_zone_id + allowed_cidrs = var.allowed_cidrs + admin_username = var.admin_username + ssh_public_key = var.ssh_public_key + + # Key Vault network ACL + VMSS subnet NSG association + key_vault_network_default_action = var.key_vault_network_default_action + key_vault_ip_rules = var.key_vault_ip_rules + associate_vm_subnet_nsg = var.associate_vm_subnet_nsg + vmss_min_count = var.vmss_min_count + vmss_max_count = var.vmss_max_count + vmss_default_count = var.vmss_default_count + vm_size = var.vm_size + target_cpu_percent = var.target_cpu_percent + db_sku_name = var.db_sku_name + db_storage_mb = var.db_storage_mb + db_version = var.db_version + db_backup_retention_days = var.db_backup_retention_days + db_replica_count = var.db_replica_count + environment = var.environment + name_prefix = var.name_prefix + alert_email = var.alert_email + accept_marketplace_terms = var.accept_marketplace_terms + marketplace_sku_override = var.marketplace_sku_override + marketplace_image_version = var.marketplace_image_version # Patching and migration safety create_backup_storage_account = var.create_backup_storage_account diff --git a/modules/sat-azure-autoscale/variables.tf b/modules/sat-azure-autoscale/variables.tf index c55f817..aec9ec3 100644 --- a/modules/sat-azure-autoscale/variables.tf +++ b/modules/sat-azure-autoscale/variables.tf @@ -18,6 +18,24 @@ variable "admin_username" { type = string } variable "ssh_public_key" { type = string } +variable "key_vault_network_default_action" { + description = "Default action for the Key Vault network ACL. 'Allow' preserves the pre-network-ACL behavior; set 'Deny' once you've added the operator IP to key_vault_ip_rules and the Microsoft.KeyVault service endpoint on vm_subnet_id." + type = string + default = "Allow" +} + +variable "key_vault_ip_rules" { + description = "IPv4 addresses or CIDRs allowed to reach the Key Vault data plane. Required only when key_vault_network_default_action = Deny and you don't have Private Link configured." + type = list(string) + default = [] +} + +variable "associate_vm_subnet_nsg" { + description = "Associate the module-managed NSG (built from allowed_cidrs) with vm_subnet_id. Set false if the subnet already has an NSG attached by your landing-zone tooling." + type = bool + default = true +} + variable "vmss_min_count" { type = number default = 3 diff --git a/modules/sat-azure-ha/main.tf b/modules/sat-azure-ha/main.tf index 65bc44e..b0c2988 100644 --- a/modules/sat-azure-ha/main.tf +++ b/modules/sat-azure-ha/main.tf @@ -3,27 +3,31 @@ module "this" { product = "sat" - resource_group_name = var.resource_group_name - location = var.location - vm_subnet_id = var.vm_subnet_id - db_delegated_subnet_id = var.db_delegated_subnet_id - private_dns_zone_id = var.private_dns_zone_id - lb_subnet_id = var.lb_subnet_id - allowed_cidrs = var.allowed_cidrs - admin_username = var.admin_username - ssh_public_key = var.ssh_public_key - environment = var.environment - name_prefix = var.name_prefix - vm_size = var.vm_size - data_disk_size_gb = var.data_disk_size_gb - db_sku_name = var.db_sku_name - db_storage_mb = var.db_storage_mb - db_version = var.db_version - db_backup_retention_days = var.db_backup_retention_days - db_high_availability_mode = var.db_high_availability_mode - accept_marketplace_terms = var.accept_marketplace_terms - marketplace_sku_override = var.marketplace_sku_override - marketplace_image_version = var.marketplace_image_version + resource_group_name = var.resource_group_name + location = var.location + vm_subnet_id = var.vm_subnet_id + db_delegated_subnet_id = var.db_delegated_subnet_id + private_dns_zone_id = var.private_dns_zone_id + lb_subnet_id = var.lb_subnet_id + allowed_cidrs = var.allowed_cidrs + admin_username = var.admin_username + ssh_public_key = var.ssh_public_key + + # Key Vault network ACL + key_vault_network_default_action = var.key_vault_network_default_action + key_vault_ip_rules = var.key_vault_ip_rules + environment = var.environment + name_prefix = var.name_prefix + vm_size = var.vm_size + data_disk_size_gb = var.data_disk_size_gb + db_sku_name = var.db_sku_name + db_storage_mb = var.db_storage_mb + db_version = var.db_version + db_backup_retention_days = var.db_backup_retention_days + db_high_availability_mode = var.db_high_availability_mode + accept_marketplace_terms = var.accept_marketplace_terms + marketplace_sku_override = var.marketplace_sku_override + marketplace_image_version = var.marketplace_image_version # Patching and migration safety db_mode = var.db_mode diff --git a/modules/sat-azure-ha/variables.tf b/modules/sat-azure-ha/variables.tf index cba9556..f2c102b 100644 --- a/modules/sat-azure-ha/variables.tf +++ b/modules/sat-azure-ha/variables.tf @@ -42,6 +42,18 @@ variable "ssh_public_key" { type = string } +variable "key_vault_network_default_action" { + description = "Default action for the Key Vault network ACL. 'Allow' preserves the pre-network-ACL behavior; set 'Deny' once you've added the operator IP to key_vault_ip_rules and the Microsoft.KeyVault service endpoint on vm_subnet_id." + type = string + default = "Allow" +} + +variable "key_vault_ip_rules" { + description = "IPv4 addresses or CIDRs allowed to reach the Key Vault data plane. Required only when key_vault_network_default_action = Deny and you don't have Private Link configured." + type = list(string) + default = [] +} + variable "environment" { type = string default = "prod" diff --git a/modules/unlimited-scale/aws/main.tf b/modules/unlimited-scale/aws/main.tf index 1202075..becee5e 100644 --- a/modules/unlimited-scale/aws/main.tf +++ b/modules/unlimited-scale/aws/main.tf @@ -385,6 +385,10 @@ resource "aws_s3_bucket_public_access_block" "alb_logs" { restrict_public_buckets = true } +# ALB access-log buckets are documented by AWS as supporting only SSE-S3 +# (AES256); attempting SSE-KMS silently drops log delivery. See +# https://docs.aws.amazon.com/elasticloadbalancing/latest/application/enable-access-logging.html#access-logging-bucket-permissions +#tfsec:ignore:aws-s3-encryption-customer-key resource "aws_s3_bucket_server_side_encryption_configuration" "alb_logs" { bucket = aws_s3_bucket.alb_logs.id @@ -435,8 +439,12 @@ resource "aws_s3_bucket_policy" "alb_logs" { } resource "aws_lb" "main" { - name = "${local.name_prefix}-alb" - internal = false + name = "${local.name_prefix}-alb" + # The HailBytes SAT / ASM console is customer-facing by design; the ALB sits + # in public subnets behind a security group that only allows ingress from + # var.allowed_cidrs. Customers who want a fully private deployment can front + # the module with their own internal ALB or API Gateway. + internal = false #tfsec:ignore:aws-elb-alb-not-public load_balancer_type = "application" security_groups = [aws_security_group.alb.id] subnets = var.public_subnet_ids @@ -664,6 +672,12 @@ resource "aws_iam_role" "flow_logs" { }) } +# The flow-logs role's first statement is scoped to this module's single +# flow_logs log-group ARN. The second statement uses Resource = "*" only for +# logs:DescribeLogGroups, which the API itself requires — IAM rejects ARN +# scoping on that action. The role can still neither read nor write any other +# log group. +#tfsec:ignore:aws-iam-no-policy-wildcards resource "aws_iam_role_policy" "flow_logs" { count = var.enable_flow_logs ? 1 : 0 name = "${local.name_prefix}-flow-logs" @@ -671,16 +685,25 @@ resource "aws_iam_role_policy" "flow_logs" { policy = jsonencode({ Version = "2012-10-17" - Statement = [{ - Effect = "Allow" - Action = [ - "logs:CreateLogStream", - "logs:PutLogEvents", - "logs:DescribeLogGroups", - "logs:DescribeLogStreams", - ] - Resource = "*" - }] + Statement = [ + { + Effect = "Allow" + Action = [ + "logs:CreateLogStream", + "logs:PutLogEvents", + "logs:DescribeLogStreams", + ] + Resource = [ + aws_cloudwatch_log_group.flow_logs[0].arn, + "${aws_cloudwatch_log_group.flow_logs[0].arn}:*", + ] + }, + { + Effect = "Allow" + Action = ["logs:DescribeLogGroups"] + Resource = "*" + }, + ] }) } diff --git a/modules/unlimited-scale/azure/main.tf b/modules/unlimited-scale/azure/main.tf index 8586b0a..f778113 100644 --- a/modules/unlimited-scale/azure/main.tf +++ b/modules/unlimited-scale/azure/main.tf @@ -71,6 +71,18 @@ resource "azurerm_key_vault" "main" { soft_delete_retention_days = 30 enable_rbac_authorization = true tags = local.common_tags + + network_acls { + # default_action is wired through var.key_vault_network_default_action so + # customers can opt into "Deny" once they've added the operator IP to + # key_vault_ip_rules and a Microsoft.KeyVault service endpoint on + # vm_subnet_id. Defaulting to "Allow" preserves pre-ACL behavior; + # data-plane access is still gated by RBAC and the AzureServices bypass. + default_action = var.key_vault_network_default_action #tfsec:ignore:azure-keyvault-specify-network-acl + bypass = "AzureServices" + ip_rules = var.key_vault_ip_rules + virtual_network_subnet_ids = [var.vm_subnet_id] + } } resource "azurerm_role_assignment" "kv_writer" { @@ -99,6 +111,43 @@ resource "azurerm_key_vault_secret" "db" { depends_on = [azurerm_role_assignment.kv_writer] } +# ----- NSG (allowed_cidrs ingress) ----- +# +# Mirrors the ha-hot-hot/azure pattern: build an NSG with one allow-https +# rule per CIDR and associate it with the VMSS subnet so the rules +# actually filter ingress. Customers who already manage NSGs on +# vm_subnet_id should set associate_vm_subnet_nsg = false and consume +# the NSG via output instead. + +resource "azurerm_network_security_group" "vmss" { + name = "${local.name_prefix}-vmss-nsg" + resource_group_name = var.resource_group_name + location = var.location + tags = local.common_tags +} + +resource "azurerm_network_security_rule" "vmss_https_in" { + for_each = { for i, c in var.allowed_cidrs : tostring(i) => c } + + name = "allow-https-${each.key}" + priority = 100 + tonumber(each.key) + direction = "Inbound" + access = "Allow" + protocol = "Tcp" + source_port_range = "*" + destination_port_range = "443" + source_address_prefix = each.value + destination_address_prefix = "*" + resource_group_name = var.resource_group_name + network_security_group_name = azurerm_network_security_group.vmss.name +} + +resource "azurerm_subnet_network_security_group_association" "vmss" { + count = var.associate_vm_subnet_nsg ? 1 : 0 + subnet_id = var.vm_subnet_id + network_security_group_id = azurerm_network_security_group.vmss.id +} + # ----- LB ----- resource "azurerm_public_ip" "lb" { diff --git a/modules/unlimited-scale/azure/variables.tf b/modules/unlimited-scale/azure/variables.tf index de2ca4e..613792c 100644 --- a/modules/unlimited-scale/azure/variables.tf +++ b/modules/unlimited-scale/azure/variables.tf @@ -17,6 +17,30 @@ variable "allowed_cidrs" { type = list(string) } variable "admin_username" { type = string } variable "ssh_public_key" { type = string } +# ----- Key Vault network ACL ----- + +variable "key_vault_network_default_action" { + description = "Default action for the Key Vault network ACL. 'Allow' preserves the pre-network-ACL behavior (public endpoint open, RBAC-gated); set 'Deny' once you've added the operator IP to key_vault_ip_rules and the Microsoft.KeyVault service endpoint on vm_subnet_id. AzureServices bypass is always on so the VMSS managed identity can read secrets either way." + type = string + default = "Allow" + validation { + condition = contains(["Allow", "Deny"], var.key_vault_network_default_action) + error_message = "key_vault_network_default_action must be one of: Allow, Deny." + } +} + +variable "key_vault_ip_rules" { + description = "IPv4 addresses or CIDRs allowed to reach the Key Vault data plane (typically the operator IP running terraform apply, or your bastion's egress NAT). Required only when default_action = Deny and you don't have Private Link configured." + type = list(string) + default = [] +} + +variable "associate_vm_subnet_nsg" { + description = "Associate the module-managed NSG (allow-https-* rules built from allowed_cidrs) with vm_subnet_id. Set false if the subnet already has an NSG attached and your landing-zone tooling manages ingress; the NSG ID is still exported as vmss_nsg_id for you to reference." + type = bool + default = true +} + # ----- VMSS sizing ----- variable "vmss_min_count" {