cloudposse / terraform-aws-ssm-patch-manager

Terraform module to provision AWS SSM Patch Manager maintenance window tasks, targets, patch baseline, patch groups and an s3 bucket for storing patch task logs
https://cloudposse.com/accelerate
Apache License 2.0
22 stars 17 forks source link

Count resolution error when providing a bucket_id #41

Closed AdamTylerLynch closed 4 months ago

AdamTylerLynch commented 5 months ago

Describe the Bug

When providing a value for bucket_id, module can not plan or apply.

Expected Behavior

Ability to create my own bucket and provide it as an input

Steps to Reproduce

##---------------------------------------------------
## AWS S3 Bucket for storing logs
##---------------------------------------------------
# S3 bucket for storing logs
module "log_storage" {
  source = "cloudposse/s3-log-storage/aws"
  name      = "account-guardian-logs-${data.aws_caller_identity.current.account_id}"
  namespace = "ga"

  s3_object_ownership           = "BucketOwnerPreferred"
  bucket_key_enabled            = true
  versioning_enabled            = true
  lifecycle_rule_enabled        = false
  lifecycle_configuration_rules = [local.lifecycle_configuration_rule]
}

##---------------------------------------------------
## AWS Systems Manager Patch Manager
##---------------------------------------------------

module "ssm-patch-manager-critical-al2" {
  source  = "cloudposse/ssm-patch-manager/aws"
  version = "0.6.0"
  region = data.aws_region.current.name

  approved_patches_compliance_level = "CRITICAL"

  namespace = "ga-account-guardian"
  operating_system = "AMAZON_LINUX_2"
  reboot_option = "NoReboot"
  scan_patch_groups = ["al2-automatic"]
  install_patch_groups = ["al2-automatic"]
  bucket_id = module.log_storage.bucket_id

  patch_baseline_approval_rules = [
    {
      approve_after_days  = 0
      compliance_level    = "HIGH"
      enable_non_security = false
      patch_baseline_filters = [
        {
          name   = "PRODUCT"
          values = ["AmazonLinux2", "AmazonLinux2.0"]
        },
        {
          name   = "CLASSIFICATION"
          values = ["Security"]
        },
        {
          name   = "SEVERITY"
          values = ["Critical", "Important"]
        }
      ]
    }
  ]

  // Schedule for the maintenance window
  // Install, and then scan to see if there are any missing patches/reboots needed
  install_maintenance_window_schedule = "cron(0 6 * * ? *)" // Run every day at 6:00 AM
  scan_maintenance_window_schedule = "cron(0 7 * * ? *)" // Run every day at 7:00 AM
}

Screenshots

No response

Environment

terraform --version Terraform v1.4.2 on darwin_amd64

cat .terraform/modules/modules.json | jq
{
  "Modules": [
    {
      "Key": "",
      "Source": "",
      "Dir": "."
    },
    {
      "Key": "log_storage",
      "Source": "registry.terraform.io/cloudposse/s3-log-storage/aws",
      "Version": "1.4.2",
      "Dir": ".terraform/modules/log_storage"
    },
    {
      "Key": "log_storage.aws_s3_bucket",
      "Source": "registry.terraform.io/cloudposse/s3-bucket/aws",
      "Version": "3.1.2",
      "Dir": ".terraform/modules/log_storage.aws_s3_bucket"
    },
    {
      "Key": "log_storage.aws_s3_bucket.s3_user",
      "Source": "registry.terraform.io/cloudposse/iam-s3-user/aws",
      "Version": "1.1.0",
      "Dir": ".terraform/modules/log_storage.aws_s3_bucket.s3_user"
    },
    {
      "Key": "log_storage.aws_s3_bucket.s3_user.s3_user",
      "Source": "registry.terraform.io/cloudposse/iam-system-user/aws",
      "Version": "1.0.0",
      "Dir": ".terraform/modules/log_storage.aws_s3_bucket.s3_user.s3_user"
    },
    {
      "Key": "log_storage.aws_s3_bucket.s3_user.s3_user.store_write",
      "Source": "registry.terraform.io/cloudposse/ssm-parameter-store/aws",
      "Version": "0.10.0",
      "Dir": ".terraform/modules/log_storage.aws_s3_bucket.s3_user.s3_user.store_write"
    },
    {
      "Key": "log_storage.aws_s3_bucket.s3_user.s3_user.store_write.this",
      "Source": "registry.terraform.io/cloudposse/label/null",
      "Version": "0.25.0",
      "Dir": ".terraform/modules/log_storage.aws_s3_bucket.s3_user.s3_user.store_write.this"
    },
    {
      "Key": "log_storage.aws_s3_bucket.s3_user.s3_user.this",
      "Source": "registry.terraform.io/cloudposse/label/null",
      "Version": "0.25.0",
      "Dir": ".terraform/modules/log_storage.aws_s3_bucket.s3_user.s3_user.this"
    },
    {
      "Key": "log_storage.aws_s3_bucket.s3_user.this",
      "Source": "registry.terraform.io/cloudposse/label/null",
      "Version": "0.25.0",
      "Dir": ".terraform/modules/log_storage.aws_s3_bucket.s3_user.this"
    },
    {
      "Key": "log_storage.aws_s3_bucket.this",
      "Source": "registry.terraform.io/cloudposse/label/null",
      "Version": "0.25.0",
      "Dir": ".terraform/modules/log_storage.aws_s3_bucket.this"
    },
    {
      "Key": "log_storage.bucket_name",
      "Source": "registry.terraform.io/cloudposse/label/null",
      "Version": "0.25.0",
      "Dir": ".terraform/modules/log_storage.bucket_name"
    },
    {
      "Key": "log_storage.this",
      "Source": "registry.terraform.io/cloudposse/label/null",
      "Version": "0.25.0",
      "Dir": ".terraform/modules/log_storage.this"
    },
    {
      "Key": "ssm-patch-manager-critical-al2",
      "Source": "registry.terraform.io/cloudposse/ssm-patch-manager/aws",
      "Version": "0.6.0",
      "Dir": ".terraform/modules/ssm-patch-manager-critical-al2"
    },
    {
      "Key": "ssm-patch-manager-critical-al2.install_window_label",
      "Source": "registry.terraform.io/cloudposse/label/null",
      "Version": "0.25.0",
      "Dir": ".terraform/modules/ssm-patch-manager-critical-al2.install_window_label"
    },
    {
      "Key": "ssm-patch-manager-critical-al2.scan_window_label",
      "Source": "registry.terraform.io/cloudposse/label/null",
      "Version": "0.25.0",
      "Dir": ".terraform/modules/ssm-patch-manager-critical-al2.scan_window_label"
    },
    {
      "Key": "ssm-patch-manager-critical-al2.ssm_patch_log_s3_bucket",
      "Source": "registry.terraform.io/cloudposse/s3-bucket/aws",
      "Version": "4.0.0",
      "Dir": ".terraform/modules/ssm-patch-manager-critical-al2.ssm_patch_log_s3_bucket"
    },
    {
      "Key": "ssm-patch-manager-critical-al2.ssm_patch_log_s3_bucket.s3_user",
      "Source": "registry.terraform.io/cloudposse/iam-s3-user/aws",
      "Version": "1.2.0",
      "Dir": ".terraform/modules/ssm-patch-manager-critical-al2.ssm_patch_log_s3_bucket.s3_user"
    },
    {
      "Key": "ssm-patch-manager-critical-al2.ssm_patch_log_s3_bucket.s3_user.s3_user",
      "Source": "registry.terraform.io/cloudposse/iam-system-user/aws",
      "Version": "1.0.0",
      "Dir": ".terraform/modules/ssm-patch-manager-critical-al2.ssm_patch_log_s3_bucket.s3_user.s3_user"
    },
    {
      "Key": "ssm-patch-manager-critical-al2.ssm_patch_log_s3_bucket.s3_user.s3_user.store_write",
      "Source": "registry.terraform.io/cloudposse/ssm-parameter-store/aws",
      "Version": "0.10.0",
      "Dir": ".terraform/modules/ssm-patch-manager-critical-al2.ssm_patch_log_s3_bucket.s3_user.s3_user.store_write"
    },
    {
      "Key": "ssm-patch-manager-critical-al2.ssm_patch_log_s3_bucket.s3_user.s3_user.store_write.this",
      "Source": "registry.terraform.io/cloudposse/label/null",
      "Version": "0.25.0",
      "Dir": ".terraform/modules/ssm-patch-manager-critical-al2.ssm_patch_log_s3_bucket.s3_user.s3_user.store_write.this"
    },
    {
      "Key": "ssm-patch-manager-critical-al2.ssm_patch_log_s3_bucket.s3_user.s3_user.this",
      "Source": "registry.terraform.io/cloudposse/label/null",
      "Version": "0.25.0",
      "Dir": ".terraform/modules/ssm-patch-manager-critical-al2.ssm_patch_log_s3_bucket.s3_user.s3_user.this"
    },
    {
      "Key": "ssm-patch-manager-critical-al2.ssm_patch_log_s3_bucket.s3_user.this",
      "Source": "registry.terraform.io/cloudposse/label/null",
      "Version": "0.25.0",
      "Dir": ".terraform/modules/ssm-patch-manager-critical-al2.ssm_patch_log_s3_bucket.s3_user.this"
    },
    {
      "Key": "ssm-patch-manager-critical-al2.ssm_patch_log_s3_bucket.this",
      "Source": "registry.terraform.io/cloudposse/label/null",
      "Version": "0.25.0",
      "Dir": ".terraform/modules/ssm-patch-manager-critical-al2.ssm_patch_log_s3_bucket.this"
    },
    {
      "Key": "ssm-patch-manager-critical-al2.ssm_patch_log_s3_bucket_label",
      "Source": "registry.terraform.io/cloudposse/label/null",
      "Version": "0.25.0",
      "Dir": ".terraform/modules/ssm-patch-manager-critical-al2.ssm_patch_log_s3_bucket_label"
    },
    {
      "Key": "ssm-patch-manager-critical-al2.this",
      "Source": "registry.terraform.io/cloudposse/label/null",
      "Version": "0.25.0",
      "Dir": ".terraform/modules/ssm-patch-manager-critical-al2.this"
    },
    {
      "Key": "ssm-patch-manager-critical-ubuntu20",
      "Source": "registry.terraform.io/cloudposse/ssm-patch-manager/aws",
      "Version": "0.6.0",
      "Dir": ".terraform/modules/ssm-patch-manager-critical-ubuntu20"
    },
    {
      "Key": "ssm-patch-manager-critical-ubuntu20.install_window_label",
      "Source": "registry.terraform.io/cloudposse/label/null",
      "Version": "0.25.0",
      "Dir": ".terraform/modules/ssm-patch-manager-critical-ubuntu20.install_window_label"
    },
    {
      "Key": "ssm-patch-manager-critical-ubuntu20.scan_window_label",
      "Source": "registry.terraform.io/cloudposse/label/null",
      "Version": "0.25.0",
      "Dir": ".terraform/modules/ssm-patch-manager-critical-ubuntu20.scan_window_label"
    },
    {
      "Key": "ssm-patch-manager-critical-ubuntu20.ssm_patch_log_s3_bucket",
      "Source": "registry.terraform.io/cloudposse/s3-bucket/aws",
      "Version": "4.0.0",
      "Dir": ".terraform/modules/ssm-patch-manager-critical-ubuntu20.ssm_patch_log_s3_bucket"
    },
    {
      "Key": "ssm-patch-manager-critical-ubuntu20.ssm_patch_log_s3_bucket.s3_user",
      "Source": "registry.terraform.io/cloudposse/iam-s3-user/aws",
      "Version": "1.2.0",
      "Dir": ".terraform/modules/ssm-patch-manager-critical-ubuntu20.ssm_patch_log_s3_bucket.s3_user"
    },
    {
      "Key": "ssm-patch-manager-critical-ubuntu20.ssm_patch_log_s3_bucket.s3_user.s3_user",
      "Source": "registry.terraform.io/cloudposse/iam-system-user/aws",
      "Version": "1.0.0",
      "Dir": ".terraform/modules/ssm-patch-manager-critical-ubuntu20.ssm_patch_log_s3_bucket.s3_user.s3_user"
    },
    {
      "Key": "ssm-patch-manager-critical-ubuntu20.ssm_patch_log_s3_bucket.s3_user.s3_user.store_write",
      "Source": "registry.terraform.io/cloudposse/ssm-parameter-store/aws",
      "Version": "0.10.0",
      "Dir": ".terraform/modules/ssm-patch-manager-critical-ubuntu20.ssm_patch_log_s3_bucket.s3_user.s3_user.store_write"
    },
    {
      "Key": "ssm-patch-manager-critical-ubuntu20.ssm_patch_log_s3_bucket.s3_user.s3_user.store_write.this",
      "Source": "registry.terraform.io/cloudposse/label/null",
      "Version": "0.25.0",
      "Dir": ".terraform/modules/ssm-patch-manager-critical-ubuntu20.ssm_patch_log_s3_bucket.s3_user.s3_user.store_write.this"
    },
    {
      "Key": "ssm-patch-manager-critical-ubuntu20.ssm_patch_log_s3_bucket.s3_user.s3_user.this",
      "Source": "registry.terraform.io/cloudposse/label/null",
      "Version": "0.25.0",
      "Dir": ".terraform/modules/ssm-patch-manager-critical-ubuntu20.ssm_patch_log_s3_bucket.s3_user.s3_user.this"
    },
    {
      "Key": "ssm-patch-manager-critical-ubuntu20.ssm_patch_log_s3_bucket.s3_user.this",
      "Source": "registry.terraform.io/cloudposse/label/null",
      "Version": "0.25.0",
      "Dir": ".terraform/modules/ssm-patch-manager-critical-ubuntu20.ssm_patch_log_s3_bucket.s3_user.this"
    },
    {
      "Key": "ssm-patch-manager-critical-ubuntu20.ssm_patch_log_s3_bucket.this",
      "Source": "registry.terraform.io/cloudposse/label/null",
      "Version": "0.25.0",
      "Dir": ".terraform/modules/ssm-patch-manager-critical-ubuntu20.ssm_patch_log_s3_bucket.this"
    },
    {
      "Key": "ssm-patch-manager-critical-ubuntu20.ssm_patch_log_s3_bucket_label",
      "Source": "registry.terraform.io/cloudposse/label/null",
      "Version": "0.25.0",
      "Dir": ".terraform/modules/ssm-patch-manager-critical-ubuntu20.ssm_patch_log_s3_bucket_label"
    },
    {
      "Key": "ssm-patch-manager-critical-ubuntu20.this",
      "Source": "registry.terraform.io/cloudposse/label/null",
      "Version": "0.25.0",
      "Dir": ".terraform/modules/ssm-patch-manager-critical-ubuntu20.this"
    }
  ]
}

Additional Context

Plan: 31 to add, 0 to change, 0 to destroy. ╷ │ Error: Invalid count argument │ │ on .terraform/modules/ssm-patch-manager-critical-al2/ssm_log_bucket.tf line 19, in data "aws_iam_policy_document" "bucket_policy": │ 19: count = local.create_log_bucket ? 1 : 0 │ │ The "count" value depends on resource attributes that cannot be determined until apply, so Terraform cannot predict how many │ instances will be created. To work around this, use the -target argument to first apply only the resources that the count │ depends on. ╵ ╷ │ Error: Invalid count argument │ │ on .terraform/modules/ssm-patch-manager-critical-al2/ssm_log_bucket.tf line 42, in module "ssm_patch_log_s3_bucket": │ 42: count = local.create_log_bucket ? 1 : 0 │ │ The "count" value depends on resource attributes that cannot be determined until apply, so Terraform cannot predict how many │ instances will be created. To work around this, use the -target argument to first apply only the resources that the count │ depends on. ╵ ╷ │ Error: Invalid count argument │ │ on .terraform/modules/ssm-patch-manager-critical-ubuntu20/ssm_log_bucket.tf line 19, in data "aws_iam_policy_document" "bucket_policy": │ 19: count = local.create_log_bucket ? 1 : 0 │ │ The "count" value depends on resource attributes that cannot be determined until apply, so Terraform cannot predict how many │ instances will be created. To work around this, use the -target argument to first apply only the resources that the count │ depends on. ╵ ╷ │ Error: Invalid count argument │ │ on .terraform/modules/ssm-patch-manager-critical-ubuntu20/ssm_log_bucket.tf line 42, in module "ssm_patch_log_s3_bucket": │ 42: count = local.create_log_bucket ? 1 : 0 │ │ The "count" value depends on resource attributes that cannot be determined until apply, so Terraform cannot predict how many │ instances will be created. To work around this, use the -target argument to first apply only the resources that the count │ depends on.

Gowiem commented 5 months ago

@AdamTylerLynch 👋 This felt odd to me since the actual count conditional was not dependent on anything that has to do with list or map types, which is where this type of problem comes up the most. I dug in and quickly setup your test with the following configurations:

##---------------------------------------------------
## AWS S3 Bucket for storing logs
##---------------------------------------------------
# S3 bucket for storing logs
module "log_storage" {
  source  = "cloudposse/s3-log-storage/aws"
  version = "1.4.2"

  name      = "account-guardian-logs"
  namespace = "ga"

  s3_object_ownership           = "BucketOwnerPreferred"
  bucket_key_enabled            = true
  versioning_enabled            = true
  lifecycle_rule_enabled        = false
  lifecycle_configuration_rules = []
}

##---------------------------------------------------
## AWS Systems Manager Patch Manager
##---------------------------------------------------

module "ssm-patch-manager-critical-al2" {
  source  = "cloudposse/ssm-patch-manager/aws"
  version = "0.6.0"

  region = "us-east-1"
  approved_patches_compliance_level = "CRITICAL"

  namespace = "ga-account-guardian"
  operating_system = "AMAZON_LINUX_2"
  reboot_option = "NoReboot"
  scan_patch_groups = ["al2-automatic"]
  install_patch_groups = ["al2-automatic"]
  bucket_id = module.log_storage.bucket_id

  patch_baseline_approval_rules = [
    {
      approve_after_days  = 0
      compliance_level    = "HIGH"
      enable_non_security = false
      patch_baseline_filters = [
        {
          name   = "PRODUCT"
          values = ["AmazonLinux2", "AmazonLinux2.0"]
        },
        {
          name   = "CLASSIFICATION"
          values = ["Security"]
        },
        {
          name   = "SEVERITY"
          values = ["Critical", "Important"]
        }
      ]
    }
  ]

  // Schedule for the maintenance window
  // Install, and then scan to see if there are any missing patches/reboots needed
  install_maintenance_window_schedule = "cron(0 6 * * ? *)" // Run every day at 6:00 AM
  scan_maintenance_window_schedule = "cron(0 7 * * ? *)" // Run every day at 7:00 AM
}

provider "aws" {
  region = "us-east-1"
}

terraform {
  required_version = ">= 1.0.0"

  required_providers {
    aws = {
      source  = "hashicorp/aws"
      version = ">= 4.1"
    }
  }
}

Here are my notes from testing in different scenarios:

  1. Using tofu 1.6.1 -- The above root module planned, applied, and destroyed without issue.
  2. Using terraform 1.7.3 -- The above root module planned, applied, and destroyed without issue.
  3. Using terraform 1.4.2 -- Plan went 💥
  4. Using terraform 1.5.7 -- Plan went 💥
  5. Using terraform 1.6.6 -- The above root module planned, applied, and destroyed without issue.

Net result is that I probably spent 40 minutes on this cause it got me curious, but I did figure it out 😁 In basically doing a git bisect across many different versions, I was able to pin it down to a change in 1.6, which led me to finding this fix / enhancement in the changelog: https://github.com/hashicorp/terraform/pull/33234

Anyway, I'm not sure why someone else hasn't run into this before... but upgrading TF to 1.6.0+ will do the trick for you. I know that is not a great option though... so another option is we add a simple bool var, log_bucket_enabled, that will allow us to easily skirt around this issue. Happy to either help out here and do an upgrade of this module to support that use-case and clean it up in general as it looks like it needs it (e.g. var.region is required + not used 👎).

One thing I found interesting from looking at issues... Mart was saying this was intended back in early 1.0 days: https://github.com/hashicorp/terraform/issues/28962. I'm surprised by that!

AdamTylerLynch commented 4 months ago

@Gowiem thank you friend! Yes, upgrading the Terraform version to 1.7.4 fixed this issue.

Gowiem commented 4 months ago

Reopening this issue as I want us on @cloudposse/contributors to get this fixed as others will run into this.

I figured the issue out last night, but I didn't write it up all that well. To sum things up, the root cause is that we're using local.create_log_bucket (here) to determine the creation of multiple resources within the module. That local depends on checking if the given var.bucket_id is null and if a consumer passes a literal value for that bucket_id, then all is good. But if the consumer creates the bucket alongside their consumption of this module in their root module and passes a reference to that resource's ID (e.g. module.log_storage.bucket_id) then we end up hitting this issue in Terraform: https://github.com/hashicorp/terraform/issues/28962. This will affect any Terraform version prior to 1.6, which is a lot of folks and is why I think this needs to be addressed.

We can get this fixed by adding a log_bucket_creation_enabled variable that we depend on instead of the bucket_id == null check. That will let pre-1.6 terraform not complain about this and properly fix the bug.

We should also do some general cleanup of this module, other PRs, and issues. I will take a stab at this next time I have a clean hour or so.