Add lambda container image support by bnusunny · Pull Request #16512 · hashicorp/terraform-provider-aws

bnusunny · 2020-12-01T20:00:54Z

Community Note

Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Relates OR Closes #0000

Release note for CHANGELOG:

ENHANCEMENTS

* resource/aws_lambda_function: Lambda support for Container Image

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccAWSLambdaFunction_imageConfig'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSLambdaFunction_imageConfig -timeout 120m
=== RUN   TestAccAWSLambdaFunction_imageConfig
=== PAUSE TestAccAWSLambdaFunction_imageConfig
=== CONT  TestAccAWSLambdaFunction_imageConfig
--- PASS: TestAccAWSLambdaFunction_imageConfig (244.64s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       247.662s

$ make testacc TESTARGS='-run=TestAccAWSLambdaFunction_basic'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSLambdaFunction_basic -timeout 120m
=== RUN   TestAccAWSLambdaFunction_basic
=== PAUSE TestAccAWSLambdaFunction_basic
=== CONT  TestAccAWSLambdaFunction_basic
--- PASS: TestAccAWSLambdaFunction_basic (90.28s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       93.439s

# Conflicts: # website/docs/r/lambda_function.html.markdown

… functions

stack72 · 2020-12-01T20:04:47Z

+				Optional:      true,
+				ConflictsWith: []string{"filename", "s3_bucket", "s3_key", "s3_object_version"},
+			},
+			"package_type": {


Can't this just be computed based on s3 / code or image?

I mean, you can now end up in a situation where a user will choose a ZIP and an image which isn't correct

(I know there's a validator but why even need it)

What if AWS adds support for another archive type later on? im assuming the API was designed to be open for expansion in the future, we should follow the API. https://www.joelonsoftware.com/2002/11/11/the-law-of-leaky-abstractions/ and such...

Good catch. We typically don't automatically set parameters like this. We should, however, add validation along the lines of:

if package_type is Image, image_uri must be set and none of filename or s3_* can be set

if package_type is Zip, image_uri and image_config cannot be set and either filename or some combination of s3_* must be set

DrFaust92 · 2020-12-01T20:06:43Z

 				Type:          schema.TypeString,
 				Optional:      true,
-				ConflictsWith: []string{"s3_bucket", "s3_key", "s3_object_version"},
+				ConflictsWith: []string{"s3_bucket", "s3_key", "s3_object_version", "image_uri"},


Maybe a good chance to substitute with ExactlyOneOf: []string{"filename", "s3_bucket", "image_uri"} this will save the custom check below.

Good idea. We should probably also set RequiredWith: []string{"s3_key"} on s3_bucket

DrFaust92 · 2020-12-01T20:09:29Z

+				Default:      lambda.PackageTypeZip,
+				ValidateFunc: validation.StringInSlice(lambda.PackageType_Values(), false),
+			},
+			"image_config": {


this doesnt conflict with zip type arguments?

gdavison

A couple tweaks to the tests

gdavison · 2020-12-01T20:36:19Z

+					testAccCheckResourceAttrRegionalARN(resourceName, "arn", "lambda", fmt.Sprintf("function:%s", funcName)),
+					testAccCheckAwsLambdaFunctionInvokeArn(resourceName, &conf),
+					resource.TestCheckResourceAttr(resourceName, "package_type", lambda.PackageTypeImage),
+					resource.TestCheckResourceAttrSet(resourceName, "image_uri"),


This isn't needed since we're checking the value below

Suggested change

resource.TestCheckResourceAttrSet(resourceName, "image_uri"),

gdavison · 2020-12-01T20:41:21Z

+					testAccCheckAwsLambdaFunctionInvokeArn(resourceName, &conf),
+					resource.TestCheckResourceAttr(resourceName, "package_type", lambda.PackageTypeImage),
+					resource.TestCheckResourceAttrSet(resourceName, "image_uri"),
+					resource.TestMatchResourceAttr(resourceName, "image_uri", regexp.MustCompile("lambda-image-function:latest$")),


We can check for this to match imageLatestID from above

Suggested change

resource.TestMatchResourceAttr(resourceName, "image_uri", regexp.MustCompile("lambda-image-function:latest$")),

resource.TestCheckResourceAttr(resourceName, "image_uri", imageLatestID),

gdavison · 2020-12-01T20:41:43Z

+				Config: testAccAWSLambdaImageConfigUpdateCode(funcName, policyName, roleName, sgName, imageV1ID),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckAwsLambdaFunctionExists(resourceName, funcName, &conf),
+					resource.TestMatchResourceAttr(resourceName, "image_uri", regexp.MustCompile("lambda-image-function:v1$")),


Suggested change

resource.TestMatchResourceAttr(resourceName, "image_uri", regexp.MustCompile("lambda-image-function:v1$")),

resource.TestCheckResourceAttr(resourceName, "image_uri", imageV1ID),

gdavison · 2020-12-01T20:42:04Z

+				Config: testAccAWSLambdaImageConfigUpdateConfig(funcName, policyName, roleName, sgName, imageV2ID),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckAwsLambdaFunctionExists(resourceName, funcName, &conf),
+					resource.TestMatchResourceAttr(resourceName, "image_uri", regexp.MustCompile("lambda-image-function:v2$")),


Suggested change

resource.TestMatchResourceAttr(resourceName, "image_uri", regexp.MustCompile("lambda-image-function:v2$")),

resource.TestCheckResourceAttr(resourceName, "image_uri", imageV2ID),

gdavison · 2020-12-01T20:57:50Z

-	if !hasFilename && !bucketOk && !keyOk && !versionOk {
-		return errors.New("filename or s3_* attributes must be set")
+	if !hasFilename && !bucketOk && !keyOk && !versionOk && !hasImageUri {
+		return errors.New("filename, s3_* or image_uri attributes must be set")


This change is causing a failure in the test TestAccAWSLambdaFunction_expectFilenameAndS3Attributes

gdavison · 2020-12-01T21:35:20Z

Thanks for the validation suggestions, @stack72 and @DrFaust92. We're going to add those in later so that we can get the container support released today

ghost · 2020-12-01T22:32:14Z

This has been released in version 3.19.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 for triage. Thanks!

ghost · 2021-01-01T17:12:17Z

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!

bnusunny added 15 commits November 16, 2020 12:28

add OCI image support to aws_lambda_function resource

a82489b

Merge branch 'master' into f-aws_lambda_function-oci-image-support

c5c8955

# Conflicts: # website/docs/r/lambda_function.html.markdown

address code review comments

d278949

address code review comments

2d08991

replace tab with spaces

298a59c

Merge branch 'master' into f-aws_lambda_function-oci-image-support

5f9f905

remove debug code

13e93ad

update test case

28edd73

use %w for error format

1eaa4c4

include link to lambda doc

869fbba

Add CustomizeDiff to validate handler and runtime for zip type lambda…

383064c

… functions

fix link to lambda doc

270c960

update description for the link to lambda doc

4f23b1b

fetch lambda image ids from environment variables.

58a86d0

Merge branch 'master' into f-aws_lambda_function-oci-image-support

1b9c740

bnusunny requested a review from a team as a code owner December 1, 2020 20:00

ghost added size/XL Managed by automation to categorize the size of a PR. service/lambda Issues and PRs that pertain to the lambda service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. labels Dec 1, 2020

bflad assigned gdavison Dec 1, 2020

bflad added this to the v3.19.0 milestone Dec 1, 2020

bflad added the enhancement Requests to existing resources that expand the functionality or scope. label Dec 1, 2020

bflad linked an issue Dec 1, 2020 that may be closed by this pull request

Lambda Container Support #16506

Closed

stack72 reviewed Dec 1, 2020

View reviewed changes

DrFaust92 reviewed Dec 1, 2020

View reviewed changes

bnusunny added 2 commits December 2, 2020 04:34

fix formatting

44bd184

update import order

6b9f1f7

gdavison reviewed Dec 1, 2020

View reviewed changes

github-actions Bot added the needs-triage Waiting for first response or review from a maintainer. label Dec 1, 2020

update test cases

eba4514

gdavison removed the needs-triage Waiting for first response or review from a maintainer. label Dec 1, 2020

gdavison merged commit a47e360 into hashicorp:master Dec 1, 2020

gdavison added a commit that referenced this pull request Dec 1, 2020

Update CHANGELOG for #16512

f0bbb99

gdavison mentioned this pull request Dec 1, 2020

resource/lambda_function: Enhance plan-time validation #16515

Closed

This was referenced Dec 2, 2020

Add support for creating Lambdas that use Container Images terraform-aws-modules/terraform-aws-lambda#79

Closed

feat: Add support for creating lambdas that use Container Images terraform-aws-modules/terraform-aws-lambda#80

Merged

ewbankkit mentioned this pull request Dec 27, 2020

Error "image_uri" is not expected here. #16911

Closed

ghost locked as resolved and limited conversation to collaborators Jan 1, 2021

justinretzolk added the partner Contribution from a partner. label May 16, 2024

	resource.TestMatchResourceAttr(resourceName, "image_uri", regexp.MustCompile("lambda-image-function:latest$")),
	resource.TestCheckResourceAttr(resourceName, "image_uri", imageLatestID),

Conversation

bnusunny commented Dec 1, 2020

Community Note

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DrFaust92 Dec 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gdavison left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gdavison commented Dec 1, 2020

Uh oh!

ghost commented Dec 1, 2020

Uh oh!

ghost commented Jan 1, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

DrFaust92 Dec 1, 2020 •

edited

Loading