-
Notifications
You must be signed in to change notification settings - Fork 9.6k
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: Deprecated aws_lambda_function nodejs runtime in favor of nodejs4.3 #9724
Conversation
Hi @Ninir Thanks for the PR here - this is a good way of handling it :) I would love to see a set of tests that validates the nodejs version to show that nodejs throws an error rather than 4.3 Thanks Paul |
02b59e5
to
baed1c6
Compare
Hi @stack72 Just added required tests as asked. The $ make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSLambdaFunction_runtimeValidation_'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/11/17 15:07:12 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSLambdaFunction_runtimeValidation_ -timeout 120m
=== RUN TestAccAWSLambdaFunction_runtimeValidation_noRuntime
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_noRuntime (0.01s)
=== RUN TestAccAWSLambdaFunction_runtimeValidation_nodeJs
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_nodeJs (0.01s)
=== RUN TestAccAWSLambdaFunction_runtimeValidation_nodeJs43
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_nodeJs43 (37.02s)
=== RUN TestAccAWSLambdaFunction_runtimeValidation_python27
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_python27 (35.18s)
=== RUN TestAccAWSLambdaFunction_runtimeValidation_java8
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_java8 (39.71s)
PASS
ok github.com/hashicorp/terraform/builtin/providers/aws 111.964s |
6d4b610
to
46a3c4d
Compare
Hi @Ninir Is this ready to be merged yet - i.e has the version been deprecated? Paul |
Hi @stack72 |
@Ninir looks like the deprecation finally happened this is us-east-1 this is eu-west-1 I wonder if the API rejects it too? Paul |
Checking it right now Paul :) |
Tests all pass here for me @Ninir so if we can confirm that the runtime has finally been deprecated, then we can merge this
|
The API still allows it... Created it, no issues, and the console show "nodejs" (but it is not selectable as you showed in the console). 😢 |
ok, we should make the behaviour that of the console - that's the right case here IMO |
Unfortunately this has broken any Lambda function test that relies on the default value. The above test runs only ran a subset of the tests. I'm fixing that in #10791 |
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. |
This aims to fix #8348, which is about deprecating nodejs in favor of nodejs4.3 since it will reach end of life in a few hours. (Reference: http://docs.aws.amazon.com/fr_fr/lambda/latest/dg/nodejs-prog-model-using-old-runtime.html)
Excerpt:
As you could read in the related issue, this is a breaking change since the
runtime
attribute is now required.This enforces that the runtime is set and to the appropriate version, which should not be nodejs anymore.
Also, this fixes some mixed whitespaces (tabs and spaces) in the related tests. Tell me if you want a separate PR :)
Note: At the time of this PR creation, the nodejs is not yet deprecated (will check in a few hours)