-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
resource/aws_lambda_function: Add support for lambda_function vpc_config update #1080
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! :)
Just left a question, since I may not have the answer at the moment! 😄
Tests are passing, except one, but not related to this PR.
$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSLambdaFunction_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSLambdaFunction_ -timeout 120m
=== RUN TestAccAWSLambdaFunction_importLocalFile
--- PASS: TestAccAWSLambdaFunction_importLocalFile (36.93s)
=== RUN TestAccAWSLambdaFunction_importLocalFile_VPC
--- PASS: TestAccAWSLambdaFunction_importLocalFile_VPC (38.61s)
=== RUN TestAccAWSLambdaFunction_importS3
--- PASS: TestAccAWSLambdaFunction_importS3 (34.91s)
=== RUN TestAccAWSLambdaFunction_basic
--- PASS: TestAccAWSLambdaFunction_basic (33.76s)
=== RUN TestAccAWSLambdaFunction_updateRuntime
--- PASS: TestAccAWSLambdaFunction_updateRuntime (45.12s)
=== RUN TestAccAWSLambdaFunction_expectFilenameAndS3Attributes
--- PASS: TestAccAWSLambdaFunction_expectFilenameAndS3Attributes (13.35s)
=== RUN TestAccAWSLambdaFunction_envVariables
--- PASS: TestAccAWSLambdaFunction_envVariables (76.47s)
=== RUN TestAccAWSLambdaFunction_encryptedEnvVariables
--- PASS: TestAccAWSLambdaFunction_encryptedEnvVariables (67.73s)
=== RUN TestAccAWSLambdaFunction_versioned
--- PASS: TestAccAWSLambdaFunction_versioned (34.12s)
=== RUN TestAccAWSLambdaFunction_DeadLetterConfig
--- PASS: TestAccAWSLambdaFunction_DeadLetterConfig (32.94s)
=== RUN TestAccAWSLambdaFunction_DeadLetterConfigUpdated
--- PASS: TestAccAWSLambdaFunction_DeadLetterConfigUpdated (46.52s)
=== RUN TestAccAWSLambdaFunction_nilDeadLetterConfig
--- PASS: TestAccAWSLambdaFunction_nilDeadLetterConfig (14.62s)
=== RUN TestAccAWSLambdaFunction_tracingConfig
--- PASS: TestAccAWSLambdaFunction_tracingConfig (39.72s)
=== RUN TestAccAWSLambdaFunction_VPC
--- PASS: TestAccAWSLambdaFunction_VPC (28.35s)
=== RUN TestAccAWSLambdaFunction_VPCUpdate
--- PASS: TestAccAWSLambdaFunction_VPCUpdate (45.24s)
=== RUN TestAccAWSLambdaFunction_VPC_withInvocation
--- FAIL: TestAccAWSLambdaFunction_VPC_withInvocation (22.98s)
testing.go:428: Step 0 error: Check failed: Check 2/2 error: AccessDeniedException: The role defined for the function cannot be assumed by Lambda.
status code: 403, request id: 4c787308-6623-11e7-a93c-b1f86a08b491
=== RUN TestAccAWSLambdaFunction_s3
--- PASS: TestAccAWSLambdaFunction_s3 (30.79s)
=== RUN TestAccAWSLambdaFunction_localUpdate
--- PASS: TestAccAWSLambdaFunction_localUpdate (41.85s)
=== RUN TestAccAWSLambdaFunction_localUpdate_nameOnly
--- PASS: TestAccAWSLambdaFunction_localUpdate_nameOnly (42.12s)
=== RUN TestAccAWSLambdaFunction_s3Update_basic
--- PASS: TestAccAWSLambdaFunction_s3Update_basic (57.16s)
=== RUN TestAccAWSLambdaFunction_s3Update_unversioned
--- PASS: TestAccAWSLambdaFunction_s3Update_unversioned (43.68s)
=== RUN TestAccAWSLambdaFunction_runtimeValidation_noRuntime
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_noRuntime (32.91s)
=== RUN TestAccAWSLambdaFunction_runtimeValidation_nodeJs
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_nodeJs (35.92s)
=== RUN TestAccAWSLambdaFunction_runtimeValidation_nodeJs43
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_nodeJs43 (33.05s)
=== RUN TestAccAWSLambdaFunction_runtimeValidation_python27
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_python27 (35.92s)
=== RUN TestAccAWSLambdaFunction_runtimeValidation_java8
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_java8 (34.10s)
=== RUN TestAccAWSLambdaFunction_tags
--- PASS: TestAccAWSLambdaFunction_tags (65.32s)
=== RUN TestAccAWSLambdaFunction_runtimeValidation_python36
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_python36 (33.33s)
return errors.New("vpc_config is <nil>") | ||
} | ||
|
||
if vpcConfig != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without looking too much into the Terraform core, could this be something other than nil, like empty or so?
(Just wondering if the check is needed, since the check above guard us from nil structures, but may be missing something)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just an extra panic guard :)
…fig update Fixes: #1073 Changing to not ForceNew on any chances to vpc_config Also, updates to `dead_letter_config` were supported in the code, but the schema said that it would ForceNew on changes. So removed this restriction
3cb32b7
to
aad7b11
Compare
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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks! |
Fixes: #1073
Changing to not ForceNew on any chances to vpc_config
Also, updates to
dead_letter_config
were supported in the code, butthe schema said that it would ForceNew on changes. So removed this
restriction