#terraform (2024-06)

Referring to Cloud Posse’s office hours?

Maybe it was this https://sweetops.slack.com/archives/CB2PXUHLL/p1712930437003109

Hey Erik, good to see you are still going at it!

I’ve been bouncing around DevOps jobs the last 6 months so I haven’t been attending office hours but I plan on changing that starting this week.

That looks like what I was thinking. Thanks for the link!

@george.m.sedky

Some people on my team haven’t used Terraform so I was doing a little demo today. We started talking about pain points, one of them being managing really large Terraform deployments for all of prod.

The example was “I want to deploy an EC2”, the tool should ideally know that I need

• VPC

• Subnets

• IGW/NAT

• AMI

• EBS w/ KMS key for encryption

• And so on… Terraform is great but it’s pretty “dumb” compared to where we’ll be in a few years with AI and tools like Stakpak. It may not be a popular idea since managing Terraform, and bringing that knowledge of how to do it well, counts for a big chunk of what many DevOps Engineers are being paid to do.

The tool I’m thinking of had a different name and it would’ve been something you shared on at least a year ago. If I can dig it up I’ll share it. Same idea as Stakpak though but I don’t think it was using AI.

Hey @Taylor Turner this is exactly what we’re working on at Stakpak I’d love to arrange a call sometime to learn about your use case we’re doing a lot of product discovery interviews this week

It’s not good practices to bake the provider instantiation in the consumable module itself

https://developer.hashicorp.com/terraform/language/modules/develop/providers#provider-version-constraints-in-modules

It should declare a required provider with a minimum version. But not a provider block.

The provider should be instantiated in the root dir where the reusable module is called

cool, that’s what I thought but couldn’t find confirmation. thank you for this quick help. cheers!

Some combination of import blocks and moved blocks is probably your best bet. The moved block is one of the most handy tools added, in my view.

Also, just to clarify - you cannot destroy/rebuild the cluster?

yeah cannot destroy rebuild :(

I haven’t messed with moved blocks yet

just brainstorming best possible options atm

I suspect it will be very difficult to use any off the shelf terraform module, because the shape of infrastructure will always be different.

Personally I prefer my security group rules inline for exclusive management by the security group resource, so no

I managed to fix the ‘old’ SG resource we have by switching around the order of the rules so the TF state (which was ordered arbitrarily) matched the AWS console, but it did require me to first delete the individual rules and then reapply them which made me unhappy for the 2 seconds they weren’t applied

We updated our security group module to use these rules

but it’s worth reading this: https://cloudposse.com/devops/security-and-compliance/the-difficulty-of-managing-aws-security-groups-with-terraform/

Hmm… It would be nice if they had a new security group v2 resource that enforced inline rules (no clickops rules) and tagging individual rules so you get the best of both worlds.

https://github.com/hashicorp/terraform-provider-aws/issues/37914

We updated our security group module to use these rules am I doing something silly? I’m looking at https://github.com/cloudposse/terraform-aws-security-group/security

and it doesn’t seem to be using the new “ingress/egress” rule resources

Hmm… It would be nice if they had a new security group v2 resource that enforced inline rules (no clickops rules) and tagging individual rules so you get the best of both worlds. yeah, that’s been a desirable feature for years haha. it’s one of the many reasons we were considering onboarding a product like spacelift for drift detection

On that note, we provide a GitHub action for drift detection if using Atmos. https://atmos.tools/integrations/github-actions/atmos-terraform-drift-detection/

There’s no way to detect drift in security group rules if they are clickopsed which is the issue with using the individual rule resources vs inline resources

@Zing https://github.com/cloudposse/terraform-aws-security-group/issues/46 is for the new resources

At least, no way using just terraform. You’d have to query the APIs directly, and compare against the config.

thanks @RB,

security groups have been such a headache for us. I’m debating between creating the security groups outside of the eks module rather than using the in-module one atm

That’s certainly an option. Most of the modules, if not all, provide a way to pass in your own security group

what are your personal preferences?

im mainly concerned with scenarios with rule modifications causing downtime

My personal preference is the same as lorens

yeah I generally like that approach as well, but it seems all the folks out there these days advise against it :p

tbh, I don’t even know why… inline rules just seems… better?

it changes the responsibility a bit. folks sometimes want different processes (or different teams) to manage the creation of the security group, vs the rules in the security group. inline rules can also make some use cases more difficult, like rules that cross-reference other sgs

but if you can change your operating model to one that supports inline rules, then i feel like it works a lot better. and i strongly prefer having a resource that manages the “container” (the security group) and its “items” (the rules) together, so that terraform can alert on drift in the items in the container (e.g. unmanaged rules added to the sg)

i use the container/items terminology for a number of things with similar concerns, like iam roles (container) and policy attachments (items), or route tables (container) and routes (items)

unfortunately terraform providers have been moving the other direction, and instead are aligning a resource purely to a single api action, so we see less and less inline support for “items” and their exclusive management, which ultimately makes drift detection effectively impossible within pure terraform

which ultimately makes drift detection effectively impossible within pure terraform I think “drift” is not an entirely fair characterization. You can certainly catch the drift of what is managed in Terraform. You cannot catch things managed outside of that Terraform. That may or may not be drift, if other systems.

I actually am the outlier. I like the current trajectory, enabling a lose coupling between root modules. E.g. one root module provisions a cluster and security group. Another service managed by different root module, that perhaps provisions node pools that work with the cluster, can add then modify security groups as needed that it needs to the cluster’s security group. It’s similar to a hub/spoke. As infrastructure state gets decomposed into it’s various components, this is a nice to have.

I acknowledge that operating model, and it has its use cases, I just don’t prefer it for everything.

You can certainly catch the drift of what is managed in Terraform. You cannot catch things managed outside of that Terraform. You can’t even be aware of drift outside of Terraform, without exclusive management features. That’s the whole point. That “in Terraform” is a massive caveat. It leaves a gaping hole, which gets nicely plugged by exclusive management features at the container level. All I’m arguing for is that it is a valid model with desirable characteristics, and it would be nice if terraform providers put effort into meeting the use case. It is also valid and desirable to support the pure “attachment” model.

I look at that more as IaC coverage than drift.

E.g. if you use a EKS cluster, and any operators, there will likely be TONS of resources provisioned not managed by Terraform.

That’s not drift. It’s just managed by something else.

like i said, there is a place for both

Security Groups might be a special case though. From a security perspective, I could appreciate wanting tighter controls.

once you start thinking about it, there are lots of resources where it’s rather nice to manage everything from the container level, rather than as separate resources. iam roles and users and groups, certainly. s3 buckets used to work this way, which was nice from the perspective of managing the ordering of actions and conflicting api calls. it certainly does put more work on the code for the resource itself, which is of course why the providers don’t want to do it

Are you properly setting a unique attribute for each instance

the module takes care of the ids, I only set the cluster id. The id is unique (the name is formed from the tenant,+environment+name)

I think I’ll need to see more code

This is the cluster related code

module “ecs_cluster” { source = “cloudposse/ecs-cluster/aws” version = “~>0.6”

enabled = var.enabled context = module.label.context

container_insights_enabled = var.container_insights_enabled logging = var.logging log_configuration = var.log_configuration capacity_providers_fargate = true tags = var.tags depends_on = [module.label] }

most options are defaulted. The context includes tenant and environment.

Where’s the for_each?

I’m managing a local module that uses the cloudposse modules

module "ecs_clusters" {
  for_each = { for v in var.ecs_clusters : "${v.tenant}-${v.environment}-${v.cluster_name}" => v }
  source   = "./modules/ecs"

  vpc_id              = module.vpc.vpc_id
  subnet_ids          = module.vpc.private_subnets
  public_subnet_ids   = module.vpc.public_subnets
  dns_domain          = each.value.dns_domain == null ? var.dns_domain : each.value.dns_domain
  zone_id             = each.value.dns_domain == null ? data.cloudflare_zone.cloudflare_zone[0].id : data.cloudflare_zone.cloudflare_zones[each.value.dns_domain].id
  # If a certificate is provided, pass that. If not, dns_domain determins if we use the root cert for the workspace or create a new one
  cloudwatch_alarm            = each.value.cloudwatch_alarm
  container_definitions       = var.container_definitions
  ecr_repos                   = var.ecr_repos
  enforced_dns_root           = each.value.enforced_dns_root
  task_execution_IAM_role_arn = aws_iam_role.roles["ecs_task_execution_role"].arn

  cluster_name                     = each.value.cluster_name
  enabled                          = each.value.enabled
  alb_enable_logs                  = each.value.alb_enable_logs
  alb_url                          = each.value.alb_url
  https_listener_arn               = each.value.https_listener_arn
  redis_cluster_enabled            = each.value.redis_cluster_enabled
  sdk_redis_enabled                = each.value.sdk_redis_enabled
  tenant                           = each.value.tenant
  namespace                        = each.value.tenant
  environment                      = each.value.environment
  container_insights_enabled       = each.value.container_insights_enabled
  logging                          = each.value.logging
  log_configuration                = each.value.log_configuration
  ecs_metric_topic_subscriptions   = each.value.ecs_metric_topic_subscriptions
  ecs_critical_topic_subscriptions = each.value.ecs_critical_topic_subscriptions
  ecs_services                     = [for v in var.ecs_services : v if v.ecs_cluster_name == each.key]
  tags                             = each.value.tags
  depends_on                       = [aws_ecr_repository.ecs_ecr_repos, aws_ecr_repository.ecr_repos, aws_iam_role_policy.role_policies]
}

Yes, so what’s missing from this example is usng var.attributes to a unique instance

otherwise the resources will be duplicated

At least, that was the way we intended it to be done. We haven’t tried other ways.

Also, word of caution - you’re building a factory inside of a root module: your terraform state will be massive, slow and brittle.

Thanks. As I said, I’m actually using a module for all ECS related stuff. It’s consumed in the root module by this ecs_cluster module. let me look at the attributes so I can understand what is missing and the impact of modifying any of that. Again, thanks a lot

I have the following debug print in all environments and they are identical aws-cli/2.16.7 Python/3.11.8 Linux/5.15.0-1063-aws exe/x86_64.ubuntu.20 { “UserId”: “AROAYKRB4XIA3M6HSSAPV:GitHubActions”, “Account”: “00000000000”, “Arn”: “arnsts:assumed-role/prod-github-action/GitHubActions” } Terraform v1.5.7 on linux_amd64

Python 3.11.9

maybe a silly question here: did you try to ssh into the self-hosted github runner and retrieve the secret via CLI? I just want to ensure that is not a permission issue

Anchor, not a silly question at all. I did and I can access the secret that way.

Are you using VPC endpoints? https://docs.aws.amazon.com/secretsmanager/latest/userguide/vpc-endpoint-overview.html

No, I’m not using a VPC Endpoint.. Still haven’t resolved the issue but working around it instead.

Have you looked in cloudtrail to see what APIs TF is invoking and with which parameters? Setting TF_LOG to debug should also reveal info about the API being called

I do see the API calls after setting TF_LOG to debug. I don’t see any requests in cloudtrail relating to secrets manager.

hmm. what about in cloudtrial? whats the list of API invocations made by the TF user?

Yes, and I would do my best to answer questions, but I can’t promise I remember many of the details. I was so confused by it at first that I had to ask some questions to the team at re:Invent. In my case, I also have to worry about tag replication because I use tags to mark objects as virus clean before anything other than the scanner can read it.

Did you have your S3 buckets created already?

No I manage the buckets too. So I create bucket A in region A along with a role to allow replication to it. Then I create bucket B in region B, a role to allow replication to it, and add the replication rule to bucket B toward bucket A. Then I add the replication rule to bucket A toward bucket B.

Ok that makes sense, are you using a module or resource when you attach the replication rules. I was trying to do it all in 1 pass with the module in TF but because the buckets don’t exist yet I can’t make that work.

I think you’re right that you won’t be able to do that. In my case, that isn’t a concern because they are separate states. Since this is bidirectional and cross-region, and each region (in this case) is managed by a separate state, I couldn’t do them all at once anyway. To answer your question, I am using a module, but it is internal, so I can’t easily share it. I try to put everything I can into the public, but this has some business logic in the module. I gave a talk about it internally too with some nice diagrams. Would be good to generalize the module and share the diagrams if the time arises.

Makes sense. Thanks for the input!

There isn’t anything interesting the replication part though. Here is the config for just that resource:

resource "aws_s3_bucket_replication_configuration" "this" {
  count = var.replica != null && var.versioning ? 1 : 0

  role   = var.replica.role_arn
  bucket = aws_s3_bucket.this.id

  rule {
    status = "Enabled"

    destination {
      bucket = var.replica.arn
    }

    dynamic "delete_marker_replication" {
      for_each = var.replica.delete_marker_replication ? [1] : []

      content {
        status = "Enabled"
      }
    }

    dynamic "filter" {
      for_each = var.replica.filters == null ? [1] : []

      content {}
    }

    dynamic "filter" {
      for_each = var.replica.filters != null ? [1] : []

      content {
        prefix = var.replica.filters.prefix != null && length(var.replica.filters.tags) == 0 ? var.replica.filters.prefix : null

        dynamic "tag" {
          for_each = var.replica.filters.prefix == null && length(var.replica.filters.tags) == 1 ? [var.replica.filters.tags[0]] : []

          content {
            key   = tag.key
            value = tag.value
          }
        }

        dynamic "and" {
          for_each = (var.replica.filters.prefix == null ? 0 : 1) + length(var.replica.filters.tags) > 1 ? [1] : []

          content {
            prefix = var.replica.filters.prefix
            tags   = var.replica.filters.tags
          }
        }
      }
    }

    dynamic "source_selection_criteria" {
      for_each = var.replica.modifications ? [1] : []

      content {
        replica_modifications {
          status = "Enabled"
        }
      }
    }
  }

  depends_on = [aws_s3_bucket_versioning.this]
}

with variable def:

variable "replica" {
  description = "Configuration for replication target."
  default     = null

  type = object({
    arn                       = string
    delete_marker_replication = optional(bool, true)
    modifications             = optional(bool, true)
    role_arn                  = string

    filters = optional(object({
      prefix = optional(string)
      tags   = optional(map(string), {})
    }))
  })
}

variable "versioning" {
  description = "Enable bucket versioning. defaults true"
  type        = bool
  default     = true
}

This is only part of the module and it is probably of no value to you, but just in case.

I reviewed it and seems good but we pass the tests

Thanks, will keep an eye on it

do you want to create another PR addressing the feedback? and mention this current PR?

I have subscribed to the above PR, I see there is one small change request for the default value of manage_admin_user_password , though I think, the issue is with the type rather than value - which should be boolean instead of string

https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/rds_cluster#rdsaurora-managed-master-passwords-via-secrets-manager-default-kms-key

so master_password needs to be removed from the resource instantiation for this to work, but is you set it to null is basically like if it was not there but if you set it to "" then it has a value and it complains

Yes, but shouldn’t the manage_admin_user_password be of type bool instead of string , Here: https://github.com/cloudposse/terraform-aws-rds-cluster/blob/523bb16c94d0a72066408b11a4b41bee3db4e07a/variables.tf#L64 As it could be a bit misleading with the description: https://github.com/cloudposse/terraform-aws-rds-cluster/blob/523bb16c94d0a72066408b11a4b41bee3db4e07a/variables.tf#L66

And set the condition something like:

master_password = (!var.manage_admin_user_password && !local.ignore_admin_credentials) ? var.admin_password : null

Hey @jose.amengual, there hasn’t been any update on this, is this still in works?

someone else will have to take it over

you can go and create a new PR and mention this one as reference

@Andriy Knysh (Cloud Posse)

Cloud Posse develops and maintains these two Terraform providers (written in Go

https://github.com/cloudposse/terraform-provider-utils

https://github.com/cloudposse/terraform-provider-awsutils

@Jason it’s not easy to understand the issues you are facing. Maybe if you provided more details, we could point you in the right direction

It’s fine I got it working

Not sure how but it started working

@Ben Smith (Cloud Posse) any ideas on this one?

It looks like this has been a problem with the module for a little while now: • Count Bug • Supposed Bugfix • Fix for new bug • Support list or map I’d recommend updating to the latest module version, atleast "0.72.0" which is what our component currently uses. It also looks like the map variable will provide more consistent results and less drift in the future. task_exec_policy_arns_map so something like:

task_exec_policy_arns_map = {
"my-sid" = module.pdf_ecs_iam_policy.policy_arn
}

Thank you for explanation. For the moment I am forced to use this version of a module )) But I will convince my dev fellows to do upgrades))

Hmm if youre forced to use that version, maybe try hardcoding your task policy arns as a temporary debugging step. the logic defined here:

for_each = local.create_task_role ? toset(var.task_policy_arns) : toset([])
│  ├────────────────
│  │ local.create_task_role is true
│  │ var.task_policy_arns is list of string with 1 element

` looks fine, you should be iterating on a single set of string - the one arn. perhaps try a hardcoded arn.

Though I would suggest making a new component or module if you could with the upgrade

This is our main.tf of our ecs-service component

I believe the behavior is for specs evaluated “later” (-var-file) to override specs evaluated “earlier” (.auto.tfvars)

I think in TF 0.12 this was possible, I think

but I can do a simple merge

Damn. 0.12 was 5 years ago. I don’t need that headache!

Looks like it was 0.12 that first had the current behavior, full override, no merge… https://developer.hashicorp.com/terraform/language/values/variables#variable-definition-precedence

ohhh

will, I will have to do a merge() , not a big deal

I believe it merges, but it’s a shallow merge. Only top-level keys, not a deep merge like with atmos.

Also be advised that merge() also is a shallow merge.

then join will have to be

are you creating the clusters with Terraform?

If yes, you can add variable validation for these variables https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/rds_cluster#rds-multi-az-cluster

https://developer.hashicorp.com/terraform/tutorials/configuration-language/custom-conditions

if you are using Atmos, you can also add OPA policies or JSONSchema to the components and validate the variables and even the correct combination of the variables

https://atmos.tools/core-concepts/components/validation

no I just need a sentinel policy that can restric elasticsearch clusters from being created if they are not multi-az

@Jeremy G (Cloud Posse)

Attaching a subdomain to a LoadBalancer is managed by external-dns

the api doc seems equally confused… https://docs.aws.amazon.com/codeartifact/latest/APIReference/API_AssociateExternalConnection.html

Adds an existing external connection to a repository. One external connection is allowed per repository.
A repository can have one or more upstream repositories, or an external connection.

is that a limitation of the terraform provider ? you are able to add multiple external repositories from the aws console :

i think the terraform docs are confused entirely because the aws docs are confused

I was confused reading the terraform doc lol cuz i was getting something different when ready the aws doc lol

from terraform’s perspective, that hcl is a block, and you can have multiple blocks of the same type. so if it works at all, the syntax would be something like:

resource "aws_codeartifact_repository" "test" {
  repository = "example"
  domain     = aws_codeartifact_domain.example.domain

  external_connections {
    external_connection_name = "public:npmjs"
  }

  external_connections {
    external_connection_name = "public:maven-central"
  }
}

the aws api doc is what i linked, and the wording is very close to the terraform doc

that’s what i tried but didn’t work for me

i imagine that whoever wrote the resource for the aws provider was relying on the aws api docs, which seem to be either wrong or just confusing

so, they might have an artificial limitation on the resource at the moment

thanks for the feedback

You can email *Terraform Registry* [[email protected]](<mailto:[email protected])>

We’ve used it in the past

Responses take a while.

Thank you

Nothing in terraform has ever worked that way, far as I’m aware

Yeah, it’ll read/combine all the tf files in the current working directory

Yup, no such thing as a file-broundary scope

What do you want to accomplish?

could be handy

You could achieve something similar by convention, naming a local per file, and using it as a map

I wanted to reuse local values to format a policy document but one feed on the other local

@Jeremy White (Cloud Posse)

So, this module does need the support of other modules to work well. Rather than go extensively through it, it’s probably best to point you at the component that uses this module here: https://github.com/cloudposse/terraform-aws-components/blob/main/modules/ecs-service/main.tf

I think the documentation could be improved on the module for sure, as it should really point at the supporting modules that create the ALB and other backing resources

within the file linked about, here is where the alb module is introduced and configured

@Jeremy White (Cloud Posse) do we need a task to update the documentation on the module?

It doesn’t hurt to make one at this point. Just be sure to emphasize these points in the ticket: • the complete examples in most modules are really meant for terratest, and likely don’t express valid use cases. They shouldn’t be used as real-world examples unless the module is rather simple • the README.md refers to the alb module, but doesn’t actually provide sample configuration to it. That should be added • the README.md ought to point to the ecs-service component in the terraform-aws-components repo, which is a very complete and well documented example of how to use this module. I’d estimate it’s an hour of work to update the module’s docs

DEV-2413 : Update documentation on ecs-service module

@Andriy Knysh (Cloud Posse)

we are not using terraform cloud, but if you are only using it for the state storage, it looks like you can migrate it to an S3 backend (or any other cloud backend where you provision your infrastructure) - fewer dependencies on 3rd-parties @Kyle Johnson

resources:

The for_each meta-argument accepts a map or a set of strings, and creates an instance for each item in that map or set.

https://developer.hashicorp.com/terraform/language/meta-arguments/for_each

dynamic blocks:

Since the for_each argument accepts any collection or structural value, you can use a for expression or splat expression to transform an existing collection.

https://developer.hashicorp.com/terraform/language/expressions/dynamic-blocks

ok, so now the docs do explains it well

thanks Loren

“why the difference” is because for resources the for_each key is the resource address and must be unique. that means sets and maps meet the key uniqueness requirement. lists do not.

for dynamic blocks, there is no resource address and so there is no uniqueness requirement, so any collection-type is allowed. and moreover, dynamic blocks may have an ordered component, which can be honored by the list type, if the provider honors ordering for the resource type

thanks for the Answer Loren