go-runner implementation #1320

icarus-sullivan · 2022-01-14T10:01:50Z

Description

Clumsily added Go support for offline lambdas.

Motivation and Context

Why is this change required
Adding a new supported runtime isn't a requirement.

What problem does is solve?
Allows someone to run go lambdas offline.

How Has This Been Tested?

Please describe in detail how you tested your changes.

yarn linked a local copy of my changes to test in a go serverless project.
Tested api-based lambdas ONLY
Added a go integration test

Include details of your testing environment, and the tests you ran to
macOS Catalina
vs. 10.15.4

icarus-sullivan · 2022-01-14T10:03:57Z

src/lambda/handler-runner/go-runner/GoRunner.js

+      const sts = new AWS.STS()
+
+      const { Credentials } = await sts
+        .getSessionToken({
+          DurationSeconds: MIN_ALLOWED_SESSION_DURATION_S, // 900-inf
+        })
+        .promise()


Uncertain about this, but credentials are not passed into go without setting them in env.

icarus-sullivan · 2022-01-14T10:04:37Z

src/lambda/handler-runner/go-runner/GoRunner.js

+    const out = handlerCode.replace(
+      '"github.com/aws/aws-lambda-go/lambda"',
+      'lambda "github.com/icarus-sullivan/mock-lambda"',
+    )


Clone code and swap out lambda import, mock-lambda looks for the LAMBDA_EVENT and LAMBDA_CONTEXT and tries to process it.

icarus-sullivan · 2022-01-14T10:12:17Z

src/lambda/handler-runner/go-runner/GoRunner.js

+      )
+    }
+
+    const cp = spawnSync(`go`, ['run', cwdPath], {


Pros vs Cons of calling the handler like this:

Pros:

Always gets the latest code, you can modify the handler and rerun it and voila newest changes

It is less invasive in respect to copying and pre-compiling code elsewhere to be called

Cons:

It is somewhat slow as it compiles before it is run

... I'm sure there are more, this is just some ideas

icarus-sullivan · 2022-01-15T04:53:55Z

@medikoo What do I need to get this peer reviewed? What metrics should I be shooting for?

icarus-sullivan · 2022-01-15T08:58:01Z

src/lambda/handler-runner/go-runner/GoRunner.js

+        IS_LAMBDA_AUTHORIZER:
+          event.type === 'REQUEST' || event.type === 'TOKEN',
+        IS_LAMBDA_REQUEST_AUTHORIZER: event.type === 'REQUEST',
+        IS_LAMBDA_TOKEN_AUTHORIZER: event.type === 'TOKEN',


Added initial support for custom authorizers

medikoo

Thank you @icarus-sullivan, that looks great. There's just one file that I believe was committed in by mistake, please remove it and we should be good

medikoo · 2022-01-17T11:10:08Z

yarn.lock

@@ -0,0 +1,9593 @@
+# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.


Let's not add this file

Sounds good, I removed the file in the PR. Glad to contribute :D

icarus-sullivan · 2022-01-17T22:17:40Z

src/lambda/handler-runner/go-runner/GoRunner.js

+    }
+
+    // Clean up after we created the temporary file
+    this.cleanup()


I put this in to aggressively cleanup tmp files, but as a side-effect it might wipe sts session info on each call. One could argue that it is a good thing to only retain session credentials to the lambda invocation, but it might slowdown subsequent requests when obtaining new sessions. As a security concern I think it should be left in imo, but I'm open to changing it.

medikoo · 2022-01-18T12:25:07Z

src/lambda/handler-runner/go-runner/GoRunner.js

+    )
+
+    try {
+      mkdirSync(this.#tmpPath, { recursive: true })


Let's use just async functions (and async/await syntax)

icarus-sullivan · 2022-01-19T05:46:48Z

src/lambda/handler-runner/go-runner/GoRunner.js

+    }
+
+    // Make sure we have the mock-lambda runner
+    sync('go', ['get', 'github.com/icarus-sullivan/mock-lambda@e065469'])


Constructors aren't created in an async manner, so using sync here only.

medikoo

Thank you @icarus-sullivan !

medikoo

@icarus-sullivan unfortunately CI crashes, there must be some issues, please check the errors

icarus-sullivan · 2022-01-20T21:08:37Z

@medikoo Saw that, tried to reproduce yesterday in some test pipelines. Still debugging but might be slow, just giving a status update.

icarus-sullivan · 2022-01-22T03:04:19Z

@medikoo Found the issue, it's related to the sts call in the pipeline. Going to try to use the aws config env variable and see if that helps. Again, might be a while - its been a busy week.

icarus-sullivan · 2022-01-22T04:59:20Z

@medikoo Okay, think I resolved the issue. Whenever you are ready to run the tests.

medikoo

Thank you @icarus-sullivan !

Francismb · 2022-02-12T21:51:54Z

Great to see this. I noticed that this is rather incompatible with the way the serverless template generates its code. When the serverless template generates its code, the yaml file points to bin files/built go lang files. It would be worth having something in the readme IMO.

uh-zz · 2022-02-21T05:27:10Z

@icarus-sullivan
Hi!
Thank you for adding the go runtime.
I saw your implementation.

Just recently, I was looking for an alternative to --useDocker
#1334

So, I thought I'd try this one.
After looking at the source, I confirmed that it refers to main.go in the project root and executes the go run command, instead of the handler defined in serverless.yml (or serverless.ts).

If I can make it so that it executes the binary file specified in the handler of serverless.yaml (or serverless.ts), it will behave as I expect.

icarus-sullivan · 2022-02-22T18:48:28Z

@uh-zz

The runner copies the path and code outlined in the handler and generates a main.go file. So it should be referencing the handler path correctly but generating a mock lambda runner on-the-fly. Let me know if that makes sense.

uh-zz · 2022-03-07T02:28:08Z

@icarus-sullivan

Thank you for confirming and sorry for the late reply.

The runner copies the path and code outlined in the handler and generates a main.go file.

I checked and the handler path seems to point to the project root(same directory as serverless.ts).
(I checked the this.#handlerPath used here with print debugging)

serverless-offline/src/lambda/handler-runner/go-runner/GoRunner.js

Line 83 in 0f7ec23

const handlerCode = await readFile(`${this.#handlerPath}.go`, 'utf8')

From this, I am assuming that the following configuration is expected.

./project
├── go.mod
├── go.sum
├── main.go
└── serverless.ts

However, in my project, I am developing the following configuration.

./project
├── a
│   └── b
│       └── c
│           ├── go.mod
│           ├── go.sum
│           └── main.go
├── bin
│   └── go-binary // go-binary is binary of a/b/c/main.go
└── serverless.ts

Therefore, I want to run the handler(in this case, bin/go-binary) configured in serverless.ts when started with serverless-offline.

icarus-sullivan changed the title ~~Initial go-runner implementation~~ WIP: go-runner implementation Jan 14, 2022

icarus-sullivan commented Jan 14, 2022

View reviewed changes

medikoo marked this pull request as draft January 14, 2022 10:52

medikoo changed the title ~~WIP: go-runner implementation~~ go-runner implementation Jan 14, 2022

medikoo assigned icarus-sullivan Jan 14, 2022

icarus-sullivan force-pushed the go-runner branch 2 times, most recently from eb738ba to c6581e8 Compare January 15, 2022 08:43

icarus-sullivan commented Jan 15, 2022

View reviewed changes

icarus-sullivan marked this pull request as ready for review January 17, 2022 00:15

medikoo requested changes Jan 17, 2022

View reviewed changes

icarus-sullivan force-pushed the go-runner branch from c6581e8 to b37743d Compare January 17, 2022 21:08

icarus-sullivan requested a review from medikoo January 17, 2022 21:10

icarus-sullivan commented Jan 17, 2022

View reviewed changes

medikoo reviewed Jan 18, 2022

View reviewed changes

icarus-sullivan force-pushed the go-runner branch from b37743d to 0cb9256 Compare January 19, 2022 05:46

icarus-sullivan commented Jan 19, 2022

View reviewed changes

medikoo approved these changes Jan 19, 2022

View reviewed changes

medikoo requested changes Jan 19, 2022

View reviewed changes

Initial go-runner implementation

a2d14a3

icarus-sullivan force-pushed the go-runner branch from 01401e9 to a2d14a3 Compare January 22, 2022 04:57

medikoo approved these changes Jan 25, 2022

View reviewed changes

medikoo merged commit 6bb54fd into dherault:master Jan 25, 2022

icarus-sullivan mentioned this pull request Feb 3, 2022

Support for Golang #1304

Closed

uh-zz mentioned this pull request Mar 21, 2022

feat: Use go build command instead of go run command #1356

Merged

and3rson mentioned this pull request Mar 28, 2022

Golang handler confusion #1358

Open

seanvm mentioned this pull request Mar 28, 2023

Go Lambda broken for non-ApiGateway handlers with custom request/response types (e.g AppSync) #1677

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

go-runner implementation #1320

go-runner implementation #1320

icarus-sullivan commented Jan 14, 2022

icarus-sullivan Jan 14, 2022

icarus-sullivan Jan 14, 2022

icarus-sullivan Jan 14, 2022

icarus-sullivan commented Jan 15, 2022

icarus-sullivan Jan 15, 2022

medikoo left a comment

medikoo Jan 17, 2022

icarus-sullivan Jan 17, 2022

icarus-sullivan Jan 17, 2022

medikoo Jan 18, 2022

icarus-sullivan Jan 19, 2022

medikoo left a comment

medikoo left a comment

icarus-sullivan commented Jan 20, 2022

icarus-sullivan commented Jan 22, 2022

icarus-sullivan commented Jan 22, 2022

medikoo left a comment

Francismb commented Feb 12, 2022

uh-zz commented Feb 21, 2022

icarus-sullivan commented Feb 22, 2022

uh-zz commented Mar 7, 2022

		@@ -0,0 +1,9593 @@
		# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.

go-runner implementation #1320

go-runner implementation #1320

Conversation

icarus-sullivan commented Jan 14, 2022

Description

Motivation and Context

How Has This Been Tested?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

icarus-sullivan commented Jan 15, 2022

Choose a reason for hiding this comment

medikoo 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

medikoo left a comment

Choose a reason for hiding this comment

medikoo left a comment

Choose a reason for hiding this comment

icarus-sullivan commented Jan 20, 2022

icarus-sullivan commented Jan 22, 2022

icarus-sullivan commented Jan 22, 2022

medikoo left a comment

Choose a reason for hiding this comment

Francismb commented Feb 12, 2022

uh-zz commented Feb 21, 2022

icarus-sullivan commented Feb 22, 2022

uh-zz commented Mar 7, 2022