Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Count resolution error when providing a bucket_id #41

Closed
AdamTylerLynch opened this issue Feb 21, 2024 · 3 comments · Fixed by #42
Closed

Count resolution error when providing a bucket_id #41

AdamTylerLynch opened this issue Feb 21, 2024 · 3 comments · Fixed by #42
Assignees
Labels
bug 🐛 An issue with the system good first issue Good for newcomers help wanted Extra attention is needed

Comments

@AdamTylerLynch
Copy link
Contributor

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

  • provider registry.terraform.io/hashicorp/aws v5.37.0
  • provider registry.terraform.io/hashicorp/time v0.10.0
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.

@AdamTylerLynch AdamTylerLynch added the bug 🐛 An issue with the system label Feb 21, 2024
@Gowiem Gowiem self-assigned this Feb 21, 2024
@Gowiem
Copy link
Member

Gowiem commented Feb 22, 2024

@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: hashicorp/terraform#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: hashicorp/terraform#28962. I'm surprised by that!

@Gowiem Gowiem added good first issue Good for newcomers help wanted Extra attention is needed labels Feb 22, 2024
@AdamTylerLynch
Copy link
Contributor Author

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

@Gowiem
Copy link
Member

Gowiem commented Feb 22, 2024

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: hashicorp/terraform#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.

@Gowiem Gowiem reopened this Feb 22, 2024
Gowiem added a commit that referenced this issue Feb 24, 2024
* fix!: removes region variable as not used and not needed

* chore: cleanup potential issues

* fix!: converts bucket_id to list(string) (fix #41) + removes label

* chore: bump s3-bucket to latest (4.0.1)

* feat: adds approve_until_date support (closes #8)

* chore: updates example to show patching (closes #40)

* feat: adds s3_bucket_prefix_scan_logs var (closes #14)

* chore: readme changes

* chore: make tf fmt happy 👍
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 An issue with the system good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
2 participants