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

provider/aws: Fix panic on nil dead_letter_config #14964

Merged
merged 1 commit into from
May 31, 2017

Conversation

grubernaut
Copy link
Contributor

Fixes a panic where specifying a nil target_arn for a dead_letter_config inside the aws_lambda_function resource would throw a panic.
Now, we return a nice error to the user instead of throwing a panic and stacktrace.

$ make testacc TEST=./builtin/providers/aws TESTARGS="-run=TestAccAWSLambdaFunction_nilDeadLetterConfig"
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/05/31 10:22:26 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSLambdaFunction_nilDeadLetterConfig -timeout 120m
=== RUN   TestAccAWSLambdaFunction_nilDeadLetterConfig
--- PASS: TestAccAWSLambdaFunction_nilDeadLetterConfig (20.86s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    20.884s

Fixes: #14961

Fixes a panic where specifying a nil `target_arn` for a `dead_letter_config` inside the `aws_lambda_function` resource would throw a panic.
Now, we return a nice error to the user instead of throwing a panic and stacktrace.

```
$ make testacc TEST=./builtin/providers/aws TESTARGS="-run=TestAccAWSLambdaFunction_nilDeadLetterConfig"
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/05/31 10:22:26 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSLambdaFunction_nilDeadLetterConfig -timeout 120m
=== RUN   TestAccAWSLambdaFunction_nilDeadLetterConfig
--- PASS: TestAccAWSLambdaFunction_nilDeadLetterConfig (20.86s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    20.884s
```
Copy link
Contributor

@apparentlymart apparentlymart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There seems to be a bug in the helper/schema package that's allowing this nil map to get through, and it would be most satisfying to get to the bottom of that and fix it properly, but this more tactical fix seems like the right thing to do in the short term so we can get rid of this crasher bug.

@grubernaut grubernaut merged commit 0845ab8 into master May 31, 2017
@grubernaut grubernaut deleted the b-fix-panic-nil-dead-letter-config branch May 31, 2017 18:56
@brikis98
Copy link
Contributor

Thx for the quick fix!

One question: instead of an error, is it possible to just disable the dead letter queue setting if an empty string is passed in? Otherwise, since the dead letter queue settings are in an inline block, there is no easy way to make this a configurable setting in a Terraform module. I brought this up before in #14037, but it sounds like a general-purpose solution for this sort of thing is some ways off, so perhaps in the short term, we can make things a bit easier?

@egarbi
Copy link
Contributor

egarbi commented Oct 19, 2017

@brikis98 do you know any workaround to avoid to create a different module only to add dead_letter_config support? Agree with you this is really annoying.

@brikis98
Copy link
Contributor

@egarbi Unfortunately, I haven't found any better solutions :-\

@ghost
Copy link

ghost commented Apr 6, 2020

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.

@ghost ghost locked and limited conversation to collaborators Apr 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

aws_lambda_function with dead_letter_config causes a crash
4 participants