feat(lambda-go): higher level construct for golang lambdas #11842

corymhall · 2020-12-02T19:42:40Z

introducing a higher level construct to define golang lambda functions. This was inspired by the
other high level constructs for Nodejs and Python

I'm setting a couple of things by default.

Environment Variables

GOOS=linux
GOARCH=amd64
GO111MODULE=on

AssetHashType

AssetHashType.OUTPUT
I've added some details in the README on when you could change this to INPUT. I'm not really sure what the majority use case would be though.

Go Modules

I am requiring the use of Go modules which were added in Go 1.11 (current version is Go 1.15). I think at this point most Go developers will probably be using Go modules (for a comparison the aws-sdk-go-v2 requires 1.15)

Runtime

Runtime.PROVIDED_AL2

The GO_1_X runtime doesn't support newer Lambda features like Lambda extensions, and the aws-lambda-go library supports the provided runtimes out of the box.

Also, I copied over a lot of the boilerplate (package.json, LICENSE, etc) from the lambda-nodejs package and just did a search and replace, let me know if I got any of it wrong.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

gitpod-io · 2020-12-02T19:42:44Z

corymhall · 2020-12-03T17:17:51Z

It looks like the tests are failing due to docker rate limit errors trying to pull the lambci/lambda:build-go1.x image. It looks like that image has not been added to the ECR cache or the build role doesn't have access to it.

corymhall · 2020-12-14T18:21:34Z

@eladb finally got all the tests passing.

jogold

Nice work!

Don't know go enough to correctly review this but sharing my experience from @aws-cdk/aws-lambda-nodejs.

Wonder if the common logic/structure of @aws-cdk/aws-lambda-nodejs and @aws-cdk/aws-lambda-golang should be centralized somewhere...

packages/@aws-cdk/aws-lambda-golang/package.json

packages/@aws-cdk/aws-lambda-golang/README.md

packages/@aws-cdk/aws-lambda-golang/lib/types.ts

packages/@aws-cdk/aws-lambda-golang/test/integ.function.ts

...cdk/aws-lambda-golang/test/lambda-handler-vendor/vendor/github.com/aws/aws-lambda-go/LICENSE

corymhall · 2020-12-15T14:38:59Z

Nice work!

Don't know go enough to correctly review this but sharing my experience from @aws-cdk/aws-lambda-nodejs.

Wonder if the common logic/structure of @aws-cdk/aws-lambda-nodejs and @aws-cdk/aws-lambda-golang should be centralized somewhere...

I tried to follow the aws-lambda-nodejs structure as closely as possible, so I definitely think the common logic/structure could be centralized somewhere. It would also make adding additional languages much easier.

corymhall · 2020-12-30T15:30:16Z

@jogold any additional feedback?

jogold · 2020-12-31T16:23:11Z

@jogold any additional feedback?

No, not from my side. @eladb will review this now I presume.

Happy new year 🍾

corymhall · 2021-01-19T20:07:25Z

@eladb let me know what the next steps are for this PR. Thanks!

gjohnson · 2021-02-17T18:40:59Z

@eladb how can we help get this through?

eladb · 2021-03-18T06:14:40Z

@corymhall Still on my radar. Apologies again for taking so long. Stay tuned.

introducing a higher level construct to define golang lambda functions. This was inspired by the other high level constructs for Nodejs and Python Co-authored-by: gjohnson <[email protected]>

corymhall · 2021-04-19T18:27:52Z

@jogold I think I need a little help with the integration tests. The asset hash that is generated is constantly changing so the tests keep failing. I've tried keeping everything as controlled as possible, even using AssetHashType.CUSTOM, but the hash that is generated in the PR build is always different, even when I manually update the integ.function.expected.json with the hash values from the previous PR build.

jogold · 2021-04-20T10:09:04Z

@jogold I think I need a little help with the integration tests. The asset hash that is generated is constantly changing so the tests keep failing. I've tried keeping everything as controlled as possible, even using AssetHashType.CUSTOM, but the hash that is generated in the PR build is always different, even when I manually update the integ.function.expected.json with the hash values from the previous PR build.

The problem is here I think:

https://github.com/aws/aws-cdk/pull/11842/files#diff-3cd2554616e8813aef0f17fcd6abdb7ba6cf73ac48418b3e50651779dd3da6ccR55

with the private readonly props: BundlingProps, all the props are "leaking" into the bundling options passed to Code.fromAsset(). Because you specify an absolute path for entry and because bundling options are taken into account in the asset hash even for custom you don't get the same hash locally and in CI:

aws-cdk/packages/@aws-cdk/core/lib/asset-staging.ts

Lines 491 to 494 in b3c288d

    
           // If we're bundling an asset, include the bundling configuration in the hash 
        
           if (bundling) { 
        
             hash.update(JSON.stringify(bundling)); 
        
           }

There's the same problem in aws-lambda-nodejs in fact but it's currently solved with a /// !cdk-integ pragma:ignore-assets at the top of the integ test file. Then you can revert to the default asset type.

Still there's something to do to avoid those unstable hashes.

jogold · 2021-04-20T10:47:55Z

packages/@aws-cdk/aws-lambda-golang/lib/bundling.ts

+    return Code.fromAsset(path.dirname(options.modFilePath), {
+      assetHashType: options.assetHashType ?? cdk.AssetHashType.OUTPUT,
+      assetHash: options.assetHash,
+      bundling: new Bundling(options),


Related to #11842 (comment)

Maybe the solution is to delete props before passing this to bundling.

Or explicitly only pass the public properties...

jogold · 2021-04-20T10:59:36Z

There's the same problem in aws-lambda-nodejs in fact but it's currently solved with a /// !cdk-integ pragma:ignore-assets at the top of the integ test file. Then you can revert to the default asset type.

Edit: I don't have the problem because I'm always using AssetHashType.OUTPUT.

packages/@aws-cdk/aws-lambda-golang/lib/bundling.ts

Co-authored-by: Jonathan Goldwasser <[email protected]>

jogold · 2021-04-20T11:54:52Z

Now you can switch to your default again for AssetHashType, run yarn build && yarn integ integ.function.js --dry-run, commit and it should be OK 🤞

corymhall · 2021-04-20T12:48:54Z

Now you can switch to your default again for AssetHashType, run yarn build && yarn integ integ.function.js --dry-run, commit and it should be OK 🤞

@jogold thanks for the help!

It looks like the bitnami/golang images on ECR now include immutable image tags (I don't think they did when I started this). I think one of the reasons I wasn't using AssetHashType.OUTPUT in the integration test is that the Golang binary can change if the environment that is was compiled in is different. I think what was happening was that the bitnami/golang:1.15 image that I had cached locally wasn't the same as what was being pulled in the build environment causing the hash to be different.

I'm hoping that by using an immutable tag public.ecr.aws/bitnami/golang:1.16.3-debian-10-r1 this shouldn't happen.

jogold · 2021-04-20T12:56:34Z

I'm hoping that by using an immutable tag public.ecr.aws/bitnami/golang:1.16.3-debian-10-r1 this shouldn't happen.

mmm... it should have worked without your latest change. When using DockerImage.fromBuild() the hash for the image is calculated like that:

aws-cdk/packages/@aws-cdk/core/lib/bundling.ts

Lines 273 to 279 in 96d4071

    
           // Fingerprints the directory containing the Dockerfile we're building and 
        
           // differentiates the fingerprint based on build arguments. We do this so 
        
           // we can provide a stable image hash. Otherwise, the image ID will be 
        
           // different every time the Docker layer cache is cleared, due primarily to 
        
           // timestamps. 
        
           const hash = FileSystem.fingerprint(path, { extraHash: JSON.stringify(options) }); 
        
           return new DockerImage(tag, hash);

I would say that the hashes committed in eb0a479 (#11842) were wrong?

If I go back before your latest change and run the integ test locally I don't get the same hash as you (i get 1ba34f533c874cc53ce2f8acc433e8429e3f37415f81881545723a93beb08747).

Anyway it's fixed now and should not happen anymore.

corymhall · 2021-04-20T13:05:30Z

mmm... it should have worked without your latest change. When using DockerImage.fromBuild() the hash for the image is calculated like that:

I was meaning that the docker image that I had cached locally for public.ecr.aws/bitnami/golang:1.15 may have had go version 1.15.3 whereas the image that was pulled in the build environment (or that your machine pulled) may have been a newer image that had go version 1.15.11. The difference in go version would have caused the generated binary to be different.

jogold · 2021-04-20T13:08:00Z

The difference in go version would have caused the generated binary to be different.

But your default is to use AssetHashType.SOURCE...

corymhall · 2021-04-20T13:20:31Z

The difference in go version would have caused the generated binary to be different.

But your default is to use AssetHashType.SOURCE...

Good catch! Looks like I forgot to update the property documentation to match the behavior.

aws-cdk/packages/@aws-cdk/aws-lambda-golang/lib/bundling.ts

Line 37 in ca358dd

assetHashType: options.assetHashType ?? cdk.AssetHashType.OUTPUT,

eladb

@corymhall finally I've picked this up. Apologies it took ages. This is an initial round of feedback, mostly focused on the README. Let's get this merged!

eladb · 2021-01-11T10:07:28Z

packages/@aws-cdk/aws-lambda-golang/README.md

+With this information the construct can determine what commands to run. You will
+generally fall into two scenarios:
+
+1. You are using vendoring (indicated by the presence of a `vendor` folder)
+   In this case `go build` will be run with `-mod=vendor` set
+2. You are not using vendoring (indicated by the absence of a `vendor` folder)
+   If you are not vendoring then `go build` will be run without `-mod=vendor`
+    since the default behavior is to download dependencies


It's not 100% clear how bundling result is different between the two modes? I think in both modes the eventual bundle needs to vend all dependencies, no? Wouldn't it simplify if we always require vending?

If you are not vendoring your dependencies in a vendor directory in the project, then Go will download dependencies into the module cache. If you have a vendor directory then Go will use the dependencies there and not make any network calls. If you are using -mod=readonly or -mod=mod then Go will use the module cache or make network calls to download/update dependencies in the module cache. go build docs.

Some people prefer to vendor their dependencies and check it in to source control, while others prefer to have Go download the dependencies into the module cache during build.

eladb · 2021-01-11T10:08:02Z

packages/@aws-cdk/aws-lambda-golang/README.md

+
+## Docker
+
+To force bundling in a docker container, set the `forceDockerBundling` prop to `true`. This


A few words what does "bundling in a docker container" means and in what cases you might want to use it

eladb · 2021-05-02T09:16:02Z

packages/@aws-cdk/aws-lambda-golang/README.md

+
+This library provides constructs for Golang Lambda functions.
+
+To use this module you will need to have Docker installed.


Is it possible to offer a local bundling experience that does not require docker to be installed? (similar to how NodejsFunction allows you to install esbuild locally)

Good catch, this comment should be removed since it does support local bundling.

eladb · 2021-05-02T09:16:51Z

packages/@aws-cdk/aws-lambda-golang/README.md

+Define a `GolangFunction`:
+
+```ts
+new lambda.GolangFunction(this, 'handler', {


Should this be GolangFunction or just GoFunction (same question for the module name nodejs-go).

@MrArnoldPalmer @RomainMuller curious what you'll think

I like this better too, I'll update.

eladb · 2021-05-02T09:17:39Z

packages/@aws-cdk/aws-lambda-golang/README.md

+});
+```
+
+By default, if a folder path is provided as the `entry`, the construct will assume there is a go entry file (i.e. `main.go`).


Suggested change

By default, if a folder path is provided as the `entry`, the construct will assume there is a go entry file (i.e. `main.go`).

By default, if `entry` point to a directory, the construct will assume there is a Go entry file called `main.go`.

What happens if entry is a file?

It works both ways (file or directory). I've updated the property docs to explain what happens either way.

eladb · 2021-05-02T09:28:50Z

packages/@aws-cdk/aws-lambda-golang/lib/Dockerfile

@@ -0,0 +1,12 @@
+# The correct AWS SAM build image based on the runtime of the function will be
+# passed as build arg. The default allows to do `docker build .` when testing.
+ARG IMAGE=lambci/lambda:build-go1.x


Let's use the official images from the public ECR repo - e.g. https://gallery.ecr.aws/sam/build-provided

I'm using the image specified in the lambda.Runtime.GO_1_X.bundlingDockerImage

aws-cdk/packages/@aws-cdk/aws-lambda/lib/runtime.ts

Lines 161 to 163 in b4956ea

public static readonly GO_1_X = new Runtime('go1.x', RuntimeFamily.GO, {

bundlingDockerImage: 'lambci/lambda:build-go1.x',

});

The SAM team has not created bundling docker images for Go yet, and based on the docs doesn't actually support building Go functions in a container (only local build supported). The only Go image on the public ECR is the bitnami golang image. I'm using this in the integration test so I could update it to this, but then we should probably update the lambda.Runtime.GO_1_X.bundlingDockerImage as well.

eladb · 2021-05-02T09:29:08Z

packages/@aws-cdk/aws-lambda-golang/lib/bundling.ts

+  /**
+   * Path to go.mod file
+   */
+  readonly modFilePath: string;


Suggested change

readonly modFilePath: string;

readonly moduleDir: string;

Also updated with some additional logic that verifies that you are providing a directory, and if you are providing a file, then it validates that it is a go.mod file.

packages/@aws-cdk/aws-lambda-golang/lib/bundling.ts

packages/@aws-cdk/aws-lambda-golang/lib/function.ts

gitpod-io · 2021-05-04T17:25:00Z

Pull request has been modified.

eladb

Impressive work. Thanks for this awesome contribution! 🎉

@corymhall I expect this to take some time to mature. Would it be okay if we assigned any issues related to this module to you?

mergify · 2021-05-05T07:10:24Z

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

aws-cdk-automation · 2021-05-05T07:35:33Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
Commit ID: eee00e7
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2021-05-05T07:36:01Z

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

corymhall · 2021-05-05T12:42:18Z

@corymhall I expect this to take some time to mature. Would it be okay if we assigned any issues related to this module to you?

@eladb yeah definitely!

introducing a higher level construct to define golang lambda functions. This was inspired by the other high level constructs for Nodejs and Python I'm setting a couple of things by default. *Environment Variables* * `GOOS=linux` * `GOARCH=amd64` * `GO111MODULE=on` *AssetHashType* * `AssetHashType.OUTPUT` I've added some details in the README on when you could change this to `INPUT`. I'm not really sure what the majority use case would be though. *Go Modules* I am requiring the use of Go modules which were added in Go 1.11 (current version is Go 1.15). I think at this point most Go developers will probably be using Go modules (for a comparison the [aws-sdk-go-v2](https://github.com/aws/aws-sdk-go-v2) requires 1.15) *Runtime* * `Runtime.PROVIDED_AL2` The `GO_1_X` runtime doesn't support newer Lambda features like Lambda extensions, and the [aws-lambda-go](https://github.com/aws/aws-lambda-go) library supports the provided runtimes out of the box. Also, I copied over a lot of the boilerplate (package.json, LICENSE, etc) from the lambda-nodejs package and just did a search and replace, let me know if I got any of it wrong. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

SomayaB assigned eladb Dec 2, 2020

jogold reviewed Dec 14, 2020

View reviewed changes

corymhall and others added 19 commits April 19, 2021 17:22

feat(lambda-golang): higher level construct for golang lambdas

ea74c13

introducing a higher level construct to define golang lambda functions. This was inspired by the other high level constructs for Nodejs and Python Co-authored-by: gjohnson <[email protected]>

fixing docker test

a89a249

switching AssetHashType default to OUTPUT

3a2051a

fixing linting error

dae01d3

adding cgoEnabled parameter

2e61282

trying to fix integration tests

fcc0124

trying to fix integration tests

93fd174

adding aws-lambda-golang dependency to other packages

3eaace3

fixing based on comments

e413b6c

removing vendor folder from integration tests

9d27860

fixing LICENSE

4b418a5

fixing NOTICE

628f1b6

fixing new lint errors

4f2e689

fixes due to updates to core

93ad838

fixing integration tests

ff4c2eb

use bundlingImage instead of bundlingDockerImage

dc1ea39

fixing integration test

720f71a

hopefully fixing integ tests

351b089

hopefully fixing integ tests again

3e98006

jogold reviewed Apr 20, 2021

View reviewed changes

packages/@aws-cdk/aws-lambda-golang/lib/bundling.ts Outdated Show resolved Hide resolved

Update packages/@aws-cdk/aws-lambda-golang/lib/bundling.ts

4623e2c

Co-authored-by: Jonathan Goldwasser <[email protected]>

corymhall added 2 commits April 20, 2021 12:09

updating integ tests

eb0a479

using immutable image tag

ca358dd

fixing property documentation for assetHashType

c739156

eladb previously requested changes May 2, 2021

View reviewed changes

updates based on pr comments

da8835f

fixing package name

bb3dbd4

eladb changed the title ~~feat(lambda-golang): higher level construct for golang lambdas~~ feat(lambda-go): higher level construct for golang lambdas May 5, 2021

eladb approved these changes May 5, 2021

View reviewed changes

Merge branch 'master' into aws-lambda-golang

eee00e7

mergify bot merged commit 0948cc7 into aws:master May 5, 2021


		## Docker

		To force bundling in a docker container, set the `forceDockerBundling` prop to `true`. This


		This library provides constructs for Golang Lambda functions.

		To use this module you will need to have Docker installed.

	By default, if a folder path is provided as the `entry`, the construct will assume there is a go entry file (i.e. `main.go`).
	By default, if `entry` point to a directory, the construct will assume there is a Go entry file called `main.go`.

	public static readonly GO_1_X = new Runtime('go1.x', RuntimeFamily.GO, {
	bundlingDockerImage: 'lambci/lambda:build-go1.x',
	});

feat(lambda-go): higher level construct for golang lambdas #11842

feat(lambda-go): higher level construct for golang lambdas #11842

Conversation

corymhall commented Dec 2, 2020 • edited Loading

gitpod-io bot commented Dec 2, 2020 • edited Loading

corymhall commented Dec 3, 2020

corymhall commented Dec 14, 2020

jogold left a comment

Choose a reason for hiding this comment

corymhall commented Dec 15, 2020

corymhall commented Dec 30, 2020

jogold commented Dec 31, 2020 • edited Loading

corymhall commented Jan 19, 2021

gjohnson commented Feb 17, 2021

eladb commented Mar 18, 2021

corymhall commented Apr 19, 2021

jogold commented Apr 20, 2021 • edited Loading

Choose a reason for hiding this comment

jogold commented Apr 20, 2021

jogold commented Apr 20, 2021

corymhall commented Apr 20, 2021

jogold commented Apr 20, 2021

corymhall commented Apr 20, 2021

jogold commented Apr 20, 2021

corymhall commented Apr 20, 2021

eladb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gitpod-io bot commented May 4, 2021

eladb left a comment

Choose a reason for hiding this comment

mergify bot commented May 5, 2021

aws-cdk-automation commented May 5, 2021

AWS CodeBuild CI Report

mergify bot commented May 5, 2021

corymhall commented May 5, 2021

corymhall commented Dec 2, 2020 •

edited

Loading

gitpod-io bot commented Dec 2, 2020 •

edited

Loading

jogold commented Dec 31, 2020 •

edited

Loading

jogold commented Apr 20, 2021 •

edited

Loading