fdmsantos / terraform-aws-kinesis-firehose

Dynamic Terraform module, which creates a Kinesis Firehose Stream and others resources like Cloudwatch, IAM Roles and Security Groups that integrate with Kinesis Firehose. Supports all destinations and all Kinesis Firehose Features.
https://registry.terraform.io/modules/fdmsantos/kinesis-firehose/aws/latest
Apache License 2.0
7 stars 6 forks source link

Prevent perpetual differences during the terraform plan/apply #14

Closed sgorshkov85 closed 2 months ago

sgorshkov85 commented 2 months ago

Hello @fdmsantos,

I work with your module to deploy a firehose stream with configured lambda transformation. There are a set of parameters that cannot be(at least for now). From the AWS documentation: Parameters with default values, including NumberOfRetries(default: 3), RoleArn(default: firehose role ARN), BufferSizeInMBs(default: 1), and BufferIntervalInSeconds(default: 60), are not stored in terraform state. To prevent perpetual differences, it is therefore recommended to only include parameters with non-default values.

https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/kinesis_firehose_delivery_stream

In the current version v3.3.0 the module always trying to set those parameters so in the plan it will be like this:

      ~ extended_s3_configuration {
            # (9 unchanged attributes hidden)

          ~ processing_configuration {
                # (1 unchanged attribute hidden)

              ~ processors {
                    # (1 unchanged attribute hidden)

                  + parameters {
                      + parameter_name  = "BufferIntervalInSeconds"
                      + parameter_value = "60"
                    }
                  + parameters {
                      + parameter_name  = "NumberOfRetries"
                      + parameter_value = "3"
                    }
                  + parameters {
                      + parameter_name  = "RoleArn"
                      + parameter_value = "arn:aws:iam::1111111111:role/env1-dev-test1-firehose-vpcflowlog-stream"
                    }

Setting custom values(suggested here) won't fix everything if the engineer wants to use the default RoleArn.

Please add logic to make applying each parameter optional.
An easy fix that works for me: variable.tf

variable "transform_lambda_default_parameters" {
  description = "Set parameters with default values for the data transformation lambda: NumberOfRetries, RoleArn, BufferSizeInMBs, BufferIntervalInSeconds"
  type        = bool
  default     = false
}

locals.tf

  lambda_processor = var.enable_lambda_transform ? {
    type = "Lambda"
    parameters = var.transform_lambda_default_parameters ? [
      {
        name  = "LambdaArn"
        value = var.transform_lambda_arn
      },
      ] : [
      {
        name  = "LambdaArn"
        value = var.transform_lambda_arn
      },
      {
        name  = "BufferSizeInMBs"
        value = var.transform_lambda_buffer_size
      },
      {
        name  = "BufferIntervalInSeconds"
        value = var.transform_lambda_buffer_interval
      },
      {
        name  = "NumberOfRetries"
        value = var.transform_lambda_number_retries
      },
      {
        name  = "RoleArn"
        value = var.transform_lambda_role_arn != null ? var.transform_lambda_role_arn : local.firehose_role_arn
      },
    ]
  } : null

Example of the module execution:

module "firehose" {
  count                               = var.flow_log_destination_type == "kinesis-data-firehose" ? 1 : 0
  source                              = "git::https://gitlab.test/test/terraform-modules/terraform-aws-kinesis-firehose.git//?ref=initial"
  name                                = "${var.name}-firehose-vpcflowlog-stream"
  input_source                        = "direct-put"
  destination                         = "s3" # or destination = "extended_s3"
  s3_bucket_arn                       = var.flow_log_s3_bucket_arn
  s3_prefix                           = local.s3_prefix
  s3_error_output_prefix              = local.s3_error_output_prefix
  s3_compression_format               = "GZIP"
  enable_lambda_transform             = true
  transform_lambda_arn                = module.lambda_data_transformation.lambda_function_arns["vpc_flowlogs_transformamtion_lambda"]
  transform_lambda_default_parameters = true
  tags                                = merge(var.tags, { purpose = "Stream transformed VPC Flow logs to the access S3" }, { LogDeliveryEnabled = "true" })
}

Probably it makes sense to add more complex logic to set parameters. My approach limits configuration:

  1. Use all custom settings
  2. Use defaults only
fdmsantos commented 2 months ago

Thanks for open the issue and all the work already done . I'll look into it.

fdmsantos commented 2 months ago

This bug is fixed on version 3.4.0.

If you are using the default values, you don't need to make any changes. In the new version, if you omit the variables (transform_lambda_buffer_size, transform_lambda_buffer_interval, transform_lambda_number_retries), their default value will be null, and the module won't configure them. These variables should only be defined when using custom values. It's supported to change default values for specific parameters (as you suggested).

Thanks for this issue and for your efforts to help resolve it.