#terraform-aws-modules (2023-09)

@Jeremy G (Cloud Posse)

@Andriy Knysh (Cloud Posse) @Jeremy White (Cloud Posse)

There’s a few ways to think of this. To be fair, this behavior adds some logic that might be unexpected to another user. Some folks would probably like to be aware of the stray character, rather than finding out later their bucket didn’t match the name they specified.

As such, it might be better to add a validation block to the variable … The benefit being you could have the error message emit the code change even. Such as:

validation {
    condition     = strcontains(var.bucket, ".")
    error_message = "Please filter out invalid characters using something like `replace( name, \".\", \" \")`"
  }

To be clear, that’s just a naive example. It’s probably better to form something like this stackoverflow answer

err, actually, starting to realize you’re speaking of the policy…

I guess statement Ids don’t form many contracts with other APIs, so the additional behavior proposed ought to be benign

Yes, it’s a formatting thing in the policy Sid. Bucket names take “.”, but policy Sids don’t.

and a little research suggests that the period is the only character that falls in this category. I expect to be wrong about that.

To be fair, the statement id ought to not be required in general. Maybe it would be even wiser to remove it altogether?

I agree, @Jeremy White (Cloud Posse). It’s a aws_lambda_permission with a single clause.

https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/lambda_permission#statement_id

It would need to be truly profound to not be clear in intent. Let’s just do that. Less is more

and apparently, TF will generate them if not provided. From those statement_id docs you linked above:
(Optional) A unique statement identifier. By default generated by Terraform.

I’ll still want an internal review with a team member, and opinions change a good deal between cohorts. But I doubt this will take much time to fix up.

Do you consume the component too?

or just the module?

@Eric D. Berg https://github.com/cloudposse/terraform-aws-datadog-lambda-forwarder/pull/64

We just use the module.

I’m not even sure what the component is. Can you tap me with the old Cluestick one time?

sure! the component style of datadog-lambda-forwarder lives here and is basically more opinionated than a vanilla terraform module. Components lean on using atmos for configuration… which means you define the terraform once, and then you specify all your configs for it in a catalog. Think of it as a way to keep your config separate from your implementation. The structure of an atmos style repo looks like this:

.
├── components
│   └── terraform
│       ├── some-terraform
│       │   └── component.yaml
│       ├── other-terraform
│       │   └── component.yaml
│       └── datadog-lambda-forwarder
│           └── component.yaml
└── stacks
    ├── catalog
    │   ├── my-app
    │   │   ├── defaults.yaml
    │   │   ├── non-prod.yaml
    │   │   └── prod.yaml
    │   └── other_defaults.yaml
    ├── orgs
    │   └── acme
    │       └── plat
    │           ├── dev
    │           │   └── eu-west-1
    │           │       └── infra.yaml
    │           ├── prod
    │           │   └── eu-west-1
    │           │       └── infra.yaml
    │           └── staging
    │               └── eu-west-1
    │                   └── infra.yaml
    └── workflows
        └── my_common_tasks.yaml

Where the catalog houses defaults, and the orgs hold only the specifics for a given environment… so you DRY up your config -and- your terraform. Atmos also has workflows, so you only need to run one command, and it’ll run terraform as many times as it takes for each component in a particular stack.

Despite how opinionated it is, it stays true to being ‘just terraform’. So you can always drop atmos and just run your modules with var files. In fact, that’s all atmos fundamentally does… write var-files and run terraform with them.

Also, the PR merged, so you should be good to go with the latest release of the datadog lambda forwarder.

Thanks, @Jeremy White (Cloud Posse)! Much appreciated!!