DestinationOptions is not supported when LogDestinationType is cloud-watch-logs

dhaven commented 3 years ago

Description

After last release 3.8 the vpc module tries to add the block to the aws_flow_log resource

 + destination_options {
          + file_format                = "plain-text"
          + hive_compatible_partitions = false
          + per_hour_partition         = false
        }

This fails with following message :

Error: error creating Flow Log (vpc-xxx): InvalidParameter: DestinationOptions is not supported when LogDestinationType is cloud-watch-logs

   with module.vpc.aws_flow_log.this[0],
   on .terraform/modules/vpc/vpc-flow-logs.tf line 16, in resource "aws_flow_log" "this":
   16: resource "aws_flow_log" "this" {

Seems to be caused by https://github.com/terraform-aws-modules/terraform-aws-vpc/pull/700

Versions

Terraform v1.0.2 on darwin_amd64

provider registry.terraform.io/hashicorp/aws v3.63.0
provider registry.terraform.io/hashicorp/cloudinit v2.2.0
provider registry.terraform.io/hashicorp/helm v2.3.0
provider registry.terraform.io/hashicorp/kubernetes v2.5.1
provider registry.terraform.io/hashicorp/local v2.1.0
provider registry.terraform.io/hashicorp/random v3.1.0
provider registry.terraform.io/hashicorp/tls v3.1.0
provider registry.terraform.io/integrations/github v4.16.0
provider registry.terraform.io/terraform-aws-modules/http v2.4.1

Reproduction

apply following module :

module "vpc" {
  source  = "terraform-aws-modules/vpc/aws"
  version = "~> 3.0"

  name = "${var.region}-vpc"
  cidr = "10.10.0.0/16"
  azs  = ["${var.region}a", "${var.region}b", "${var.region}c"]

  private_subnets = ["10.10.1.0/24", "10.10.2.0/24", "10.10.3.0/24"]
  private_subnet_tags = {
    "kubernetes.io/cluster/eks-${var.environment}" = "shared"
    "kubernetes.io/role/internal-elb"              = "1"
  }

  public_subnets = ["10.10.11.0/24", "10.10.12.0/24", "10.10.13.0/24"]
  public_subnet_tags = {
    "kubernetes.io/cluster/eks-${var.environment}" = "shared"
    "kubernetes.io/role/elb"                       = "1"
  }

  intra_subnets = ["10.10.21.0/24", "10.10.22.0/24", "10.10.23.0/24"]

  database_subnets             = ["10.10.31.0/24", "10.10.32.0/24", "10.10.33.0/24"]
  create_database_subnet_group = true

  elasticache_subnets             = ["10.10.41.0/24", "10.10.42.0/24", "10.10.43.0/24"]
  create_elasticache_subnet_group = true

  manage_default_route_table = true
  default_route_table_tags   = { DefaultRouteTable = true }

  enable_dns_hostnames = true
  enable_dns_support   = true

  enable_nat_gateway     = true
  single_nat_gateway     = false
  one_nat_gateway_per_az = true

  # Default security group - ingress/egress rules cleared to deny all
  manage_default_security_group  = true
  default_security_group_ingress = []
  default_security_group_egress  = []

  # VPC Flow Logs (Cloudwatch log group and IAM role will be created)
  enable_flow_log                                 = true
  create_flow_log_cloudwatch_log_group            = true
  create_flow_log_cloudwatch_iam_role             = true
  flow_log_max_aggregation_interval               = 60
  flow_log_cloudwatch_log_group_retention_in_days = 90

  tags = local.tags
}

Expected behavior

no error

Actual behavior

see description

kamialie commented 3 years ago

I think one way to fix it would be exposing map of destination_options parameters, instead of one by one, like flow_log_file_format, flow_log_per_hour_partition. And then use dynamic block in aws_flow_log resource. Not sure if it is preferred way to go, exposing a map, so just threw in an idea.

kamialie commented 3 years ago

I will prepare a PR in couple hours, once I get free, if it is not fixed and nobody opposes the idea😁

antonbabenko commented 3 years ago

Good catch. I think you can use dynamic block but no need to convert variables to a map.

kamialie commented 3 years ago

It's just as far as I know dynamic block doesn't work with count meta argument, which would be ideal here - just to check if destination type is cloudwatch. I could be wrong or not fully getting your vision.

antonbabenko commented 3 years ago

This is what I mean:

    dynamic "destination_options" {
      for_each = var.something != "cloudwatch" ? [true] : []
      content {
        foo = var.foo
      }
    }

kamialie commented 3 years ago

Yeah, I see now. Not beautiful, but does the job.

ghost commented 3 years ago

I started experiencing this same problem just minutes after a fix was released. Thanks guys! 😄

github-actions[bot] commented 2 years ago

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

terraform-aws-modules / terraform-aws-vpc