Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Kinesis firehose delivery stream, default lambda transform params not stored in state #9827

Closed
pelicanpie opened this issue Aug 20, 2019 · 14 comments · Fixed by #35137
Closed
Labels
bug Addresses a defect in current functionality. service/firehose Issues and PRs that pertain to the firehose service.
Milestone

Comments

@pelicanpie
Copy link

pelicanpie commented Aug 20, 2019

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Terraform Version

Terraform v0.11.11
provider: aws version = "~> 2.0"

Affected Resource(s)

  • aws_kinesis_firehose_delivery_stream (processing configuration)

Terraform Configuration Files

resource "aws_kinesis_firehose_delivery_stream" "foo_log_stream" {
  name        = "foo-logs-${var.env_name}"
  destination = "extended_s3"

  extended_s3_configuration {
    role_arn   = "${var.firehose_log_stream_role_arn}"
    bucket_arn = "${aws_s3_bucket.foo_log_bucket.arn}"
    prefix     = "firehose/"

    buffer_size = 3

    processing_configuration {
      enabled = "true"

      processors {
        type = "Lambda"

        parameters = [
          {
            parameter_name  = "LambdaArn"
            parameter_value = "${module.foo-log-transformer.foo_log_transformer_arn}:$LATEST"
          },
          {
            parameter_name  = "BufferIntervalInSeconds"
            parameter_value = 900
          },
          {
            parameter_name  = "BufferSizeInMBs"
            parameter_value = 3
          },
        ]
      }
    }
  }
}

Expected Behavior

No diff detected when running terraform plan

Actual Behavior

Terraform will perform the following actions:

  ~ module.foo.aws_kinesis_firehose_delivery_stream.foo_log_stream
      extended_s3_configuration.0.processing_configuration.0.processors.0.parameters.#:                 "2" => "3"
      extended_s3_configuration.0.processing_configuration.0.processors.0.parameters.2.parameter_name:  "" => "BufferSizeInMBs"
      extended_s3_configuration.0.processing_configuration.0.processors.0.parameters.2.parameter_value: "" => "3"


Plan: 0 to add, 1 to change, 0 to destroy.

Steps to Reproduce

  1. Create an aws_kinesis_firehose_delivery_stream with extended_s3 destination
  2. Set the processing configuration to true and specify a lambda processor with the parameters set to default values (BufferSizeInMBs = 3 and/or BufferIntervalInSeconds = 60)
  3. terraform apply
  4. terraform plan

For the parameters with values that match a default value, the parameter will be ignored and not stored in the terraform state file due to this part of the code: https://github.com/terraform-providers/terraform-provider-aws/blob/d94146963be840982ef4a232d6c012a697a811ab/aws/resource_aws_kinesis_firehose_delivery_stream.go#L570-L575

This results in terraform plan picking up a change in state for the ignored parameter.

The BufferSizeInMBs and BufferIntervalInSeconds parameters are also required so we can't leave them out if we want them to be set to the default values and have a clean terraform plan.

Important Factoids

References

@ghost ghost added the service/firehose Issues and PRs that pertain to the firehose service. label Aug 20, 2019
@github-actions github-actions bot added the needs-triage Waiting for first response or review from a maintainer. label Aug 20, 2019
@elrob
Copy link
Contributor

elrob commented Sep 1, 2020

I also couldn't find these defaults anywhere in the docs. I guessed this was the reason for the plan suggesting a diff and this issue suggests I was correct.

Another improvement would be to document these defaults and explain the diff behaviour. https://github.com/terraform-providers/terraform-provider-aws/blob/d94146963be840982ef4a232d6c012a697a811ab/aws/resource_aws_kinesis_firehose_delivery_stream.go#L558

elrob added a commit to elrob/terraform-provider-aws that referenced this issue Sep 1, 2020
@ewbankkit ewbankkit added bug Addresses a defect in current functionality. and removed needs-triage Waiting for first response or review from a maintainer. labels Jun 28, 2021
anGie44 pushed a commit that referenced this issue Dec 1, 2021
@mattyo161
Copy link

Not sure if this is helpful to others but I found this issue was causing my plan applies to fail with the following error. I did some testing and found a few workarounds related to this issue and #19936.

If you set only one of the parameters that have defaults like BufferIntervalInSeconds = 900 instead of 60s and if you leave BufferSizeInMBs blank then it will fail to apply the change with the error:

InvalidArgumentException: Both BufferSizeInMBs and BufferIntervalInSeconds are required to configure buffering for lambda processor.

I found the following workarounds:

  1. don't specify parameters BufferSizeInMBs or BufferIntervalInSeconds defaults will be used, other changes will be applied successfully, and plans won't detect perpetual changes.
  2. if you need to change one of the values you are best off to change both of them to something other then the defaults like BufferIntervalInSeconds=900 and BufferSizeInMBs=2 all changes will be applied, and plans won't detect perpetual changes.
  3. If you only want to change one of the values like BufferIntervalInSeconds=900 then you must set the other value to the default BufferSizeInMBs=3 if you don't do that you will get the error above. When you do set both, your changes will be applied but your plans will have perpetual changes detected.

If the code could be updated to support one of those required parameters getting set, and it would set the other parameter with the default value when the AWS API is called or whatever is causing that error that would be very helpful.

I am currently using TF 0.13 and AWS Provider 3.65, I tested with the 3.75.1 and 4.10.0 of the providers as well. I have not tried newer versions of TF but I assume they will have the same problem.

@antonxo
Copy link

antonxo commented May 17, 2022

I have the same problem, Terraform 1.1.9, aws provider 4.13.0.
@mattyo161 your 2nd suggested approach worked for me, thanks.

@jjuraszek
Copy link

happens also with other processors (not lambda):

processors {
        type = "MetadataExtraction"
        parameters {
          parameter_name  = "JsonParsingEngine"
          parameter_value = "JQ-1.6"
        }
        parameters {
          parameter_name  = "MetadataExtractionQuery"
          parameter_value = "{date:(.timestamp/1000 | strftime(\"%Y-%m-%d\"))}"
        }
}

dstrelau added a commit to honeycombio/terraform-aws-integrations that referenced this issue Nov 16, 2022
There are several bugs open against the TF provider
([eg](hashicorp/terraform-provider-aws#9827))
about KFH processor `parameters` not "sticking" and resulting in
infinite diffs. This appears to be something related to not saving the
values in the statefile if they're the defaults.

We're experiencing this internally with `BufferSizeInMBs` trying to be
reset each apply ([internal TF
link](https://app.terraform.io/app/honeycomb/workspaces/prod/runs/run-W9obHXLNwKGNA5gY))

So … let's try not to use the defaults. While we're at it, allow setting
these parameters dynamically, and also sort the keys to try and avoid
any out-of-order plan shenanigans.
@stewartcampbell
Copy link

I found that even when setting both to non default values, I was still seeing a perpetual diff. The solution was to configure the params in this specific order in the TF code.

          parameters {
            parameter_name  = "LambdaArn"
            parameter_value = "${module.metric_stream_filter.lambda_function_arn}:$LATEST"
          }
          parameters {
            parameter_name  = "BufferSizeInMBs"
            parameter_value = "1"
          }
          parameters {
            parameter_name  = "BufferIntervalInSeconds"
            parameter_value = "65"
          }

@jjuraszek
Copy link

jjuraszek commented Nov 29, 2022

I found that even when setting both to non default values, I was still seeing a perpetual diff. The solution was to configure the params in this specific order in the TF code.

          parameters {
            parameter_name  = "LambdaArn"
            parameter_value = "${module.metric_stream_filter.lambda_function_arn}:$LATEST"
          }
          parameters {
            parameter_name  = "BufferSizeInMBs"
            parameter_value = "1"
          }
          parameters {
            parameter_name  = "BufferIntervalInSeconds"
            parameter_value = "65"
          }

same solution works for JsonParsingEngine. The order of parameters has to be MetadataExtractionQuery and JsonParsingEngine as second one. Thanks @stewartcampbell for the inspiration 👍

@johnsonaj
Copy link
Contributor

Thank you for keeping this issue updated. After some investigation there are a few things within the current implementation that make resolving this difficult.

  1. The AWS API returns default results that are not in the configuration, causing the next terraform plan to show a diff since they are explicitly excluded from state.
  2. Writing these additional values to the state would limit terraform's ability to do drift detection since the config would not match the state, causing similar inconsistent plan/apply results.
  3. Because of the nested nature of these attributes, and that it is a list type, suppressing the diff is difficult and can cause inconsistencies.

For these reasons the path forward for resolution would be an upgrade from the Terraform Plugin SDKv2 to the Terraform Plugin Framework, which will provide features that allow better handling of nested attributes. This will take some time to upgrade but the Framework will provide better flexibility to fix this issue and for future enhancements.

@ghost
Copy link

ghost commented Mar 2, 2023

This issue still happens on plan/apply the state, each time Terraform suggests changing these values as commented before in this conversation.

+ parameters {
                      + parameter_name  = "BufferIntervalInSeconds"
                      + parameter_value = "60"
                    }

Are there any updates on the development of a solution for this?

Thank you very much in advance.

@rick96perez
Copy link

This issue still happens on plan/apply the state, each time Terraform suggests changing these values as commented before in this conversation.

+ parameters {
                      + parameter_name  = "BufferIntervalInSeconds"
                      + parameter_value = "60"
                    }

Are there any updates on the development of a solution for this?

Thank you very much in advance.

Yea issue happens but I guess it is because of the default paramater being 60.
As a temporary fix, I set it as 61 and I get "No changes" now. So try setting it to 61

@cwong-archy
Copy link

I found that even when setting both to non default values, I was still seeing a perpetual diff. The solution was to configure the params in this specific order in the TF code.

          parameters {
            parameter_name  = "LambdaArn"
            parameter_value = "${module.metric_stream_filter.lambda_function_arn}:$LATEST"
          }
          parameters {
            parameter_name  = "BufferSizeInMBs"
            parameter_value = "1"
          }
          parameters {
            parameter_name  = "BufferIntervalInSeconds"
            parameter_value = "65"
          }

same solution works for JsonParsingEngine. The order of parameters has to be MetadataExtractionQuery and JsonParsingEngine as second one. Thanks @stewartcampbell for the inspiration 👍

same issue we're facing. we are setting MetadataExtractionQuery and JsonParsingEngine only -

      ~ extended_s3_configuration {
            # (8 unchanged attributes hidden)

          ~ processing_configuration {
                # (1 unchanged attribute hidden)

              ~ processors {
                    # (1 unchanged attribute hidden)

                  ~ parameters {
                      ~ parameter_name  = "MetadataExtractionQuery" -> "JsonParsingEngine"
                      ~ parameter_value = "{month:.event_timestamp| strftime(\"%m\"),day:.event_timestamp| strftime(\"%d\")}" -> "JQ-1.6"
                    }
                  ~ parameters {
                      ~ parameter_name  = "JsonParsingEngine" -> "MetadataExtractionQuery"
                      ~ parameter_value = "JQ-1.6" -> "{month:.event_timestamp| strftime(\"%m\"),day:.event_timestamp| strftime(\"%d\")}"
                    }
                }
...
Plan: 0 to add, 1 to change, 0 to destroy.

@victor7070
Copy link

@cwong-archy
try to add the parameters like:
image

@cwong-archy
Copy link

@cwong-archy try to add the parameters like: image

hey @victor7070 . THANK YOU SOOO MUCH!!!! this worked!!!!
why order matters, i dont know. ¯_(ツ)_/¯

this is what we have at the moment -

 121   │       # BUG: params not stored in tf state
 122   │       # https://github.com/hashicorp/terraform-provider-aws/issues/9827
 123   │       # https://github.com/hashicorp/terraform-provider-aws/issues/19936
 124   │       processors {
 125   │         type = "MetadataExtraction"
 126   │
 127   │         parameters {
 128   │           parameter_name  = "JsonParsingEngine"
 129   │           parameter_value = "JQ-1.6"
 130   │         }
 131   │
 132   │         parameters {
 133   │           parameter_name  = "MetadataExtractionQuery"
 134   │           parameter_value = var.partitioning_keys
 135   │         }
 136   │       }
 137   │     }
 138   │   }
 139   │
 140   │   tags = var.tags
 141   │ }

when i swapped the 2 params order and it shows no changes -

@@ -125,13 +125,13 @@ resource "aws_kinesis_firehose_delivery_stream" "this" {
         type = "MetadataExtraction"

         parameters {
-          parameter_name  = "JsonParsingEngine"
-          parameter_value = "JQ-1.6"
+          parameter_name  = "MetadataExtractionQuery"
+          parameter_value = var.partitioning_keys
         }

         parameters {
-          parameter_name  = "MetadataExtractionQuery"
-          parameter_value = var.partitioning_keys
+          parameter_name  = "JsonParsingEngine"
+          parameter_value = "JQ-1.6"
         }
       }
     }

ewbankkit pushed a commit that referenced this issue Jan 5, 2024
This is the correct value, though 3 MB may have been at some point in
the past. This addresses the perpetual differences found in the state
currently due to the default creation being 1 MB, and state default
expecting 3 MB. References can be seen in #9827 and #19936. This also
fixes #33014.
@github-actions github-actions bot added this to the v5.32.0 milestone Jan 5, 2024
Copy link

This functionality has been released in v5.32.0 of the Terraform AWS Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Addresses a defect in current functionality. service/firehose Issues and PRs that pertain to the firehose service.
Projects
None yet