#terraform-aws-modules (2022-02)

Left a comment. I haven’t worked too much with permission boundaries, but I feel like each IAM role should have its own permission boundary enabled rather than only being able to pass one permission boundary policy for all 3 roles.

Cool, have made this change and pushed

Thanks @Yonatan Koren

Looks perfect

Releasing this with another PR (cloudposse/terraform-aws-datadog-lambda-forwarder/pull/29)

are you specifically talking about this module? https://github.com/cloudposse/terraform-aws-datadog-lambda-forwarder

or DD in genral?

in general @jose.amengual how folks have attacked the above trade-offs. The module is very neat though

I think the DD implementation of the lambda forwarder is absolutely terrible

they have 3 different pieces of code to do 1 thing

right, the implementation is one thing. But the other one is “out of all the options to skin the cat, which one is closer to the sweet spot” , that is what i’m trying to get my head around really. Sadly i see only half way house which is very hard to pitch with L1 because you know eventually you end up paying on AWS side (CW bill will kill you) and DD side.

All i want really is a simple:

• cost efficient solution

• simple/ clear path implementation If i’d say: Use CW Metrics stream ( and still be able to use the DD percentile metrics - you can’t btw, have just raised a Feature request with DD) to push Metrics and maybe with “transport mechanism” provided by Firehose push logs to DD. If you need APM for Lambda service only get also the Extension layer.

All that is simple, rather than current approach….

@DaniC (he/him) I want to defer some of the implementation questions to those who were more heavily involved with this module, namely @jose.amengual and @RB.

However I think this module, created ~7 months ago, was written with the intention to conform to the documentation on the Datadog site on the subject and have logs aggregated in Datadog.

What you are describing is an optimization that is nevertheless possible with the Datadog Forwarder, as documented in the project’s README for logs / monitoring. This document mentions Kinesis, whereas the previously-linked document on the DD website doesn’t at all.
• for estates where you have
2k lambdas (hence 2k CW log groups) how do you manage the subscription filter for DD fwd if the apps are being rollout w/o TF (say cdk ) ? I haven’t done this and I’m assuming you mean that you have an environment with 2000+ log groups managed outside of Terraform. If the prefixes are deterministic, you can probably use the cloudwatch_log_groups data source in the TF configuration instantiating our module, then supply

 cloudwatch_forwarder_log_groups = {
    for log_group in data.cloudwatch_log_groups.default :
      log_group.name => {
        name           = log_group.name
        filter_pattern = "" }
      }
  }

2k sounds like an excessive amount. Are they all in one region + account? Cloud Posse’s term for the TF configuration that instantiates our modules is a component. These are configured for each environment (region) and stage (account) and managed via atmos. So we’d probably never have 2k app log groups in one account. But regardless not having a single monolithic AWS account for everything is a standard practice which may or may not apply to the legacy environment you’re dealing with, so maybe if the accounts are divided per SDLC environment, even if they are all in one region, you’re only dealing with 2000 / 3 = ~600-700 per account? Still a ton but maybe doable with what I described above.

As for your other questions, I’m not technically familiar enough (yet) with the specifics of the DD Forwarder to comment on them. But I think if there’s anything that can be adjusted for our module to potentially support an optimized log forwarding solution as you’re describing above on top of the current, officially-documented solution, then it definitely has its place in our module.

@Erik Osterman (Cloud Posse) maybe you have some additional insight too

@Yonatan Koren first massive thanks for writing such a detail response !!
What you are describing is an optimization that is nevertheless possible with the Datadog Forwarder, as documented in the project’s README for logs / monitoring. This document mentions Kinesis, whereas the previously-linked document on the DD website doesn’t at all. oh, wasn’t aware of it, being new to DD i’ve only followed their official docs which …leaves a lot of room for improvements. I’ve tried to get good insights from their support folks but …the help is very basic (although they are very good and raising the ticket/ Zendesk does a brilliant job)
2k sounds like an excessive amount. Are they all in one region + account? yes same acc, same region. Is because i have so many full deployed envs …
But regardless not having a single monolithic AWS account for everything is a standard practice which may or may not apply to the legacy environment you’re dealing with, so maybe if the accounts are divided per SDLC environment, even if they are all in one region, you’re only dealing with 2000 / 3 = ~600-700 per account? Still a ton but maybe doable with what I described above. totally agree, i have a mountain to climb in getting the whole estate to a saner state, is coming

Yeah I’m not sure how well Terraform is going to handle 2k keys for the cloudwatch_log_groups data source in the state. But I would still give it a go. Not much else to do about your legacy environment short of migrating it, which may totally be impractical if you don’t have the buy-in from the business types from above, and/or the resources to do it.

Currently only using DD lambda forwarder to pull ALB logs out of an S3 bucket and get them into DD

Services and lambdas log to CloudWatch and we have an optional Kinesis firehose to spray them into DD

Not a big fan of this forwarder honestly

but saves me writing something to do it

Thing is, if you have 2K+ Lambda log groups, you aren’t going to want to subscribe them all to the same firehose, so you’d need to shard that and it’ll get messy

The lambda/service module for each service controls the CWL group and individual firehose for that lambda/service

Yes that means a single firehose per log group, but isolation++

Current client are not tight on budget.

We have individual services that can saturate a firehose in prod

Can hit ~40M RPM

Interesting point about a firehose for everything, haven’t considered the implication. However in my case, we are tight on budget so we’ll need to take that into account.

and for metrics what do you do @joshmyers ? you using Metrics Streams -> or you went with pooling from Metrics API using DD crawlers/ aws integration ?

DD scraping metrics from CloudWatch (no doubt some lag)

Indeed, that is what i saw too and my users are getting itchy about it. Sadly if you move away from DD scraping metrics you lose the percentile metrics which is absent (feature request already raised but …) in Metrics stream

Have not used Metrics Stream

How are your app metrics getting into Cloudwatch?

Afk right now but will respond soon

Are you sure there’s no issue open for this? I feel like @RB raised this already but maybe my memory is lying to me

https://github.com/cloudposse/terraform-external-module-artifact/issues nope

I think we could switch for format and use %v with positional parameters

Not as easy to understand. Or we just switch to replace and just manually replace those 3 options

Our longer term plans is to convert to using Dockerized lambdas with public Docker images on ECR

Yeah I thought format could work

in the short term, i think we should fork that template provider and release an arm build.

there is a community version already with an arm build.

https://github.com/gxben/terraform-provider-template

and to use it you can set this in your versions.tf file

terraform {
  required_version = ">= 1.0.0"

  required_providers {
    template = {
      source  = "gxben/template"
      # version has to be set explicitly instead of using a > sign
      version = "= 2.2.0-m1"
    }
  }
}

ya and a format function would work if someone was to pr it in

whoops, I wrote this before seeing @nitrocode’s response, although his suggestion was to ping in Slack. very coincidental!

please take a look at https://github.com/cloudposse/terraform-aws-ecs-alb-service-task/blob/master/examples/complete/main.tf

the ECS cluster itself is just 2 lines of code

resource "aws_ecs_cluster" "default" {
  name = module.this.id
  tags = module.this.tags
}

for all the rest, we have modules

also https://github.com/cloudposse/terraform-aws-ecs-web-app/blob/master/examples/complete/main.tf

oh wow i didn’t know we provisioned the ecs cluster anywhere

@Andriy Knysh (Cloud Posse) yeah, I understand the potential simplicity of creating an ECS cluster but still my infrastructure is entirely module based due to terragrunt.

That makes our options..

1. write a module to use cloudposse’s naming/context/null-label which just creates an ECS cluster a. if we go the “write it ourselves” route, we can fork the ASG module to include cluster creation
2. use the public terraform-aws-modules ecs cluster module a. we lose cloudposse’s standardized naming. Again, we can just fork For this reason, our ideal solution is to use a cloudposse module, whether we contribute it or not. But it is also easy enough for us to just use our fork of the ASG module.

@RB Those are test files, AFAIK there is no module that provisions the cluster

no module for the cluster b/c it’s just 2 lines of code

terragrunt does not work with plain TF resources?

no, only with modules. there has been discussion of working with resource,data, but it has been avoided because that blurs the lines between terragrunt and terraform

in your code, where you are using the ASG module, can you add those line to create the cluster?

from your code, you can create a top-level module and include the ASG module and other TF resources

yes but so far we have not had to maintain a module at all. This is what our terragrunt looks like …

terraform {
  # Set to fork, version locked to commit hash
  source = "[email protected]:kevcube/terraform-aws-ec2-autoscale-group.git?ref=bfe85953cf"
}

inputs = {
  name = "ecs"

  image_id                  = "ami-0071e84f5b73239a6"
  instance_type             = "t4g.small"
  iam_instance_profile_name = dependency.instance_profile.outputs.instance_profile

  subnet_ids         = dependency.subnets.outputs.private_subnet_ids
  security_group_ids = [
    dependency.database.outputs.security_group_id,
    dependency.bastion.outputs.security_group_id
    ]

  min_size = 1
  max_size = 2

  key_name = "Kevin"

  capacity_rebalance     = true
  create_ecs_cluster     = true
  update_default_version = true
}

dependency "instance_profile" {
  config_path = "../../../global/iam/ecs-container-instance"
}

dependency "bastion" {
  config_path = "../../security/bastion"
}

dependency "subnets" {
  config_path = "../../network/vpc-main/subnets"
}

dependency "database" {
  config_path = "../../database/postgres-main"
}

include "root" {
  path = find_in_parent_folders("root.hcl")
}

include "aws" {
  path = find_in_parent_folders("aws.hcl")
}

and then at higher levels we define other elements of null-label naming

we prob can create a module for that (it’s a few lines of code, but anyway) we’ll look into that

We would be required to either..

1. create a module that wraps ASG module and includes ECS cluster
2. Use our fork of ASG that includes ECS cluster
3. use a module that exclusively defines ECS cluster

@Andriy Knysh (Cloud Posse) I would greatly appreciate that!

and yes, cluster definition can be done in a few lines of code, but as @RB mentioned in the PR, configuration for ECS clusters has become slightly more advanced over time, now there are multiple resources that can be involved like aws_ecs_cluster_capacity_providers

The links work in the github repo since they are all relative

https://github.com/cloudposse/terraform-aws-ecs-container-definition

ah ha! thank you very much @RB

Ya, I really wonder why HashiCorp refuses to fix this long standing problem since day one with the registry