Reviewed the documentation and lambdas #1035

aadrijnberg · 2021-08-03T15:50:48Z

To get into the subject, I read the documentation, studied the lambdas and their unit tests, and made some improvements.

All unit tests are still green, but I did not yet check end-2-end if the system still works as expected.

Bumps [aws-sdk](https://github.com/aws/aws-sdk-js) from 2.863.0 to 2.888.0. - [Release notes](https://github.com/aws/aws-sdk-js/releases) - [Changelog](https://github.com/aws/aws-sdk-js/blob/master/CHANGELOG.md) - [Commits](aws/aws-sdk-js@v2.863.0...v2.888.0) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

Bumps [eslint](https://github.com/eslint/eslint) from 7.28.0 to 7.29.0. - [Release notes](https://github.com/eslint/eslint/releases) - [Changelog](https://github.com/eslint/eslint/blob/master/CHANGELOG.md) - [Commits](eslint/eslint@v7.28.0...v7.29.0) --- updated-dependencies: - dependency-name: eslint dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

Bumps [typescript](https://github.com/Microsoft/TypeScript) from 4.3.3 to 4.3.4. - [Release notes](https://github.com/Microsoft/TypeScript/releases) - [Commits](microsoft/TypeScript@v4.3.3...v4.3.4) --- updated-dependencies: - dependency-name: typescript dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

Bumps [@typescript-eslint/eslint-plugin](https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/eslint-plugin) from 4.27.0 to 4.28.0. - [Release notes](https://github.com/typescript-eslint/typescript-eslint/releases) - [Changelog](https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/CHANGELOG.md) - [Commits](https://github.com/typescript-eslint/typescript-eslint/commits/v4.28.0/packages/eslint-plugin) --- updated-dependencies: - dependency-name: "@typescript-eslint/eslint-plugin" dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

Bumps [typescript](https://github.com/Microsoft/TypeScript) from 4.3.3 to 4.3.4. - [Release notes](https://github.com/Microsoft/TypeScript/releases) - [Commits](microsoft/TypeScript@v4.3.3...v4.3.4) --- updated-dependencies: - dependency-name: typescript dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* add white listing of repositories Signed-off-by: ravenolf <benahmed@soramitsu.co.jp> * fix variable naming Signed-off-by: ravenolf <benahmed@soramitsu.co.jp> * add unit test * update docs * add successful unit test Co-authored-by: ravenolf <benahmed@soramitsu.co.jp>

Org level admin permissions for the app are not required any more, removed from docs.

* bugfix: change module exports and upgrade vercel to latest release * bugfix: webhook.verify is now asynchronous

…ws-github-runner into philips-labs-develop # Conflicts: # README.md # modules/runner-binaries-syncer/lambdas/runner-binaries-syncer/yarn.lock # modules/runners/lambdas/runners/package.json # modules/runners/lambdas/runners/yarn.lock # modules/webhook/lambdas/webhook/src/webhook/handler.test.ts # modules/webhook/lambdas/webhook/src/webhook/handler.ts # variables.tf

* fix(scale): Refactor Runner Type and Owner (#871) * fix(scale): Refactor Runner Type and Owner * `environment` should not be optional * feat: support multiple instance types (#898) * fix(scale): Refactor Runner Type and Owner * `environment` should not be optional * feat(runners): Support Multiple Instance Types * Correcting failed launch logic * Updating tests * Test for all launch templates failing * Marking `instance_type` as deprecated * docs: fix lambda_security_group_ids incorrect description #738 (#902) close #738 * fix: scale down runners (#905) * fix: scale down runners * fix: scale down runners * chore: group upgrade lambda dependencies (#906) * chore: upgrade dependencies for lambda's * fix auth-app to 3.4.0, issues #904 addresses the issue * feat(runner): Move Lambda Vars to Parameter Store * Add test for ssm module (#1) * Add test for ssm module * Fixing lint * Removing KMS/GH Auth from scale-down * Add SSM permissions to runner policy * Allow custom key_id * Fixing for loop * Move SSM policy to Lambdas * Fixing function call * chore: Bump aws-sdk (#752) (#909) Bumps [aws-sdk](https://github.com/aws/aws-sdk-js) from 2.863.0 to 2.888.0. - [Release notes](https://github.com/aws/aws-sdk-js/releases) - [Changelog](https://github.com/aws/aws-sdk-js/blob/master/CHANGELOG.md) - [Commits](aws/aws-sdk-js@v2.863.0...v2.888.0) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore: Bump aws-sdk (#752) (#908) Bumps [aws-sdk](https://github.com/aws/aws-sdk-js) from 2.863.0 to 2.888.0. - [Release notes](https://github.com/aws/aws-sdk-js/releases) - [Changelog](https://github.com/aws/aws-sdk-js/blob/master/CHANGELOG.md) - [Commits](aws/aws-sdk-js@v2.863.0...v2.888.0) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore: Bump aws-sdk (#752) (#887) Bumps [aws-sdk](https://github.com/aws/aws-sdk-js) from 2.863.0 to 2.888.0. - [Release notes](https://github.com/aws/aws-sdk-js/releases) - [Changelog](https://github.com/aws/aws-sdk-js/blob/master/CHANGELOG.md) - [Commits](aws/aws-sdk-js@v2.863.0...v2.888.0) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore: Bump aws-sdk (#752) (#885) Bumps [aws-sdk](https://github.com/aws/aws-sdk-js) from 2.863.0 to 2.888.0. - [Release notes](https://github.com/aws/aws-sdk-js/releases) - [Changelog](https://github.com/aws/aws-sdk-js/blob/master/CHANGELOG.md) - [Commits](aws/aws-sdk-js@v2.863.0...v2.888.0) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore: Bump aws-sdk (#752) (#889) Bumps [aws-sdk](https://github.com/aws/aws-sdk-js) from 2.863.0 to 2.888.0. - [Release notes](https://github.com/aws/aws-sdk-js/releases) - [Changelog](https://github.com/aws/aws-sdk-js/blob/master/CHANGELOG.md) - [Commits](aws/aws-sdk-js@v2.863.0...v2.888.0) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore: Bump aws-sdk (#752) (#892) Bumps [aws-sdk](https://github.com/aws/aws-sdk-js) from 2.863.0 to 2.888.0. - [Release notes](https://github.com/aws/aws-sdk-js/releases) - [Changelog](https://github.com/aws/aws-sdk-js/blob/master/CHANGELOG.md) - [Commits](aws/aws-sdk-js@v2.863.0...v2.888.0) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore: Bump aws-sdk (#752) (#907) Bumps [aws-sdk](https://github.com/aws/aws-sdk-js) from 2.863.0 to 2.888.0. - [Release notes](https://github.com/aws/aws-sdk-js/releases) - [Changelog](https://github.com/aws/aws-sdk-js/blob/master/CHANGELOG.md) - [Commits](aws/aws-sdk-js@v2.863.0...v2.888.0) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore: Bump aws-sdk (#752) (#864) Bumps [aws-sdk](https://github.com/aws/aws-sdk-js) from 2.863.0 to 2.888.0. - [Release notes](https://github.com/aws/aws-sdk-js/releases) - [Changelog](https://github.com/aws/aws-sdk-js/blob/master/CHANGELOG.md) - [Commits](aws/aws-sdk-js@v2.863.0...v2.888.0) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore: Bump eslint in /modules/webhook/lambdas/webhook (#918) Bumps [eslint](https://github.com/eslint/eslint) from 7.28.0 to 7.29.0. - [Release notes](https://github.com/eslint/eslint/releases) - [Changelog](https://github.com/eslint/eslint/blob/master/CHANGELOG.md) - [Commits](eslint/eslint@v7.28.0...v7.29.0) --- updated-dependencies: - dependency-name: eslint dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore: Bump typescript (#929) Bumps [typescript](https://github.com/Microsoft/TypeScript) from 4.3.3 to 4.3.4. - [Release notes](https://github.com/Microsoft/TypeScript/releases) - [Commits](microsoft/TypeScript@v4.3.3...v4.3.4) --- updated-dependencies: - dependency-name: typescript dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore: Bump @typescript-eslint/eslint-plugin (#928) Bumps [@typescript-eslint/eslint-plugin](https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/eslint-plugin) from 4.27.0 to 4.28.0. - [Release notes](https://github.com/typescript-eslint/typescript-eslint/releases) - [Changelog](https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/CHANGELOG.md) - [Commits](https://github.com/typescript-eslint/typescript-eslint/commits/v4.28.0/packages/eslint-plugin) --- updated-dependencies: - dependency-name: "@typescript-eslint/eslint-plugin" dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore: Bump typescript in /modules/webhook/lambdas/webhook (#926) Bumps [typescript](https://github.com/Microsoft/TypeScript) from 4.3.3 to 4.3.4. - [Release notes](https://github.com/Microsoft/TypeScript/releases) - [Commits](microsoft/TypeScript@v4.3.3...v4.3.4) --- updated-dependencies: - dependency-name: typescript dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * feat: Added support for white listing of repositories (#915) * add white listing of repositories Signed-off-by: ravenolf <benahmed@soramitsu.co.jp> * fix variable naming Signed-off-by: ravenolf <benahmed@soramitsu.co.jp> * add unit test * update docs * add successful unit test Co-authored-by: ravenolf <benahmed@soramitsu.co.jp> * Need `,` after list item * Move Lambda Policy to data resource * Addressing PR comments, fixing lint * Refactoring Parameters to SSM Module * Fixing rebase * Using only key ARN as input value Co-authored-by: Niek Palm <npalm@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Sofiane Benahmed <38732323+ravenolf@users.noreply.github.com> Co-authored-by: ravenolf <benahmed@soramitsu.co.jp>

Added support for new workflow_job event, the check_run event will remain for backwards compatibility

Bumps [@types/node](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/node) from 15.14.7 to 16.4.12. - [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases) - [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/node) --- updated-dependencies: - dependency-name: "@types/node" dependency-type: direct:development update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>

Bumps [tar](https://github.com/npm/node-tar) from 4.4.13 to 4.4.15. - [Release notes](https://github.com/npm/node-tar/releases) - [Changelog](https://github.com/npm/node-tar/blob/main/CHANGELOG.md) - [Commits](isaacs/node-tar@v4.4.13...v4.4.15) --- updated-dependencies: - dependency-name: tar dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com>

npalm · 2021-08-05T10:59:14Z

@aadrijnberg thanks for the work, please can you do a rebase on develop so we see a clear diff of you changes

Bumps [jest-mock-extended](https://github.com/marchaos/jest-mock-extended) from 1.0.18 to 2.0.1. - [Release notes](https://github.com/marchaos/jest-mock-extended/releases) - [Commits](https://github.com/marchaos/jest-mock-extended/commits) --- updated-dependencies: - dependency-name: jest-mock-extended dependency-type: direct:development update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>

Bumps [@types/node](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/node) from 15.14.7 to 16.4.12. - [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases) - [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/node) --- updated-dependencies: - dependency-name: "@types/node" dependency-type: direct:development update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>

…b-runner into develop

…da error to caller.

Wait for scaleDown to have executed before returning to the caller that no error occurred.

- use question mark after field name i.s.o. Type | undefined - use more functional approach i.s.o. for-loops - import all items from a single dependency on a single line - remove unused imports - add missing return type to function - change payload type from any to string

- add test for terminateRunner - removed duplicate tests for scale-down - use assertions consistently -> ".not" changed to ".not.toBeCalled()" - set process.env.ENABLE_ORGANIZATION_RUNNERS before calling scaleDown, and assert the right call to have been called - remove "Once" from the paginate mock, as it is called more than once - fix inconsistent asserts for both repo and org functions in the scaleUp tests - remove expectedRunnerParams field overriding when they are already overridden to same value at a higher level - add test for all launches failing in the repo level tests - add test for decryption failing in hte websocket handler - add checking of the payload to the SQS request - change the X-Github-Event to "check_run" to really test what was intended to be tested

* apply patch for broken scale up lambda [#980](#980) ([b957e26](b957e26))

npalm

@aadrijnberg thanks! Did a quick check, a few findings so far. But requires a bit more work to check from my side.

CONTRIBUTING.md

CHANGELOG.md

modules/runners/lambdas/runners/src/scale-runners/scale-down.test.ts

npalm

Just some minor notes and change requested. Still need to test a deployment

npalm · 2021-08-06T09:05:51Z

modules/runners/lambdas/runners/src/scale-runners/runners.ts

-      }
-    }
-  }
+  const runners = runningInstances.Reservations?.flatMap((r) => (


It is a bit of taste, and the approach with map and flatMap looks elegant. But @gertjanmaas and I prefer a the variant with the for loop for readability. Maybe replacing the inner for with a map is an better option.

People more and more go from imperative to a functional style. It gets rid of boilerplate stuff, and focuses on the meat. If the outer is replaced with a for loop again, then I don't see benefit for the inner loop being a map operation. I reverted back to the old implementation.

modules/runners/lambdas/runners/src/scale-runners/scale-down.test.ts

…b-runner into develop

# Conflicts: # CHANGELOG.md

npalm · 2021-08-10T08:28:42Z

@gertjanmaas can you have a quick look as well?

gertjanmaas

One very small comment, otherwise looks good to me.

modules/runners/README.md

Co-authored-by: Gertjan Maas <gertjan@maas.codes>

npalm

Test an upgrade, LGTM. Thanks @aadrijnberg

dependabot bot and others added 20 commits July 7, 2021 14:18

chore(examples): Upgrade default example to terraform 1.x.x

97ef7fb

chore(doc): Remove org level admin permission #801

11a6f57

Org level admin permissions for the app are not required any more, removed from docs.

feat: make delay of webhook event configurable (#990)

68635dd

fix: change module exports and upgrade vercel to latest release (#1005)

6278c17

* bugfix: change module exports and upgrade vercel to latest release * bugfix: webhook.verify is now asynchronous

fix: reduce permission required for session manager (#1018)

d5611b0

Merge branch 'philips-labs-develop' into develop

e15ec0c

npalm self-requested a review August 3, 2021 19:33

mcaulifn and others added 4 commits August 4, 2021 13:30

feat: Adding support for new workflow_job event. (#1019)

0896a15

Added support for new workflow_job event, the check_run event will remain for backwards compatibility

dependabot bot and others added 4 commits August 5, 2021 13:19

Merge branch 'develop' of github.com:philips-labs/terraform-aws-githu…

5b64c6b

…b-runner into develop

fix: handle situation of no prereleases correctly, and propagate lamb…

4f1a25e

…da error to caller.

aadrijnberg and others added 9 commits August 5, 2021 14:15

chore (doc): small improvements

833724e

fix: wait for scaleDown to have finished before calling callback

791b8a3

Wait for scaleDown to have executed before returning to the caller that no error occurred.

fix: use consistent reporting back to caller

6c0b11a

chore(release): 0.15.1 [skip ci]

aae4735

* apply patch for broken scale up lambda [#980](#980) ([b957e26](b957e26))

Update CHANGELOG

f1b3f47

fix: CONTRIBUTION.md should refer develop iso master branch

cf6b198

Cleanup

0f52a77

npalm requested changes Aug 5, 2021

View reviewed changes

CONTRIBUTING.md Outdated Show resolved Hide resolved

CHANGELOG.md Outdated Show resolved Hide resolved

modules/runners/lambdas/runners/src/scale-runners/scale-down.test.ts Show resolved Hide resolved

npalm requested changes Aug 6, 2021

View reviewed changes

npalm force-pushed the develop branch from 3c87a68 to 153d699 Compare August 6, 2021 12:07

aadrijnberg added 3 commits August 6, 2021 15:30

Incorporate review comments and fix build issue

cc92fa4

Merge branch 'develop' of github.com:philips-labs/terraform-aws-githu…

2586abe

…b-runner into develop

Merge branch 'develop' into my-fixes

ce1afb8

# Conflicts: # CHANGELOG.md

gertjanmaas previously approved these changes Aug 10, 2021

View reviewed changes

modules/runners/README.md Outdated Show resolved Hide resolved

aadrijnberg dismissed gertjanmaas’s stale review via acae897 August 10, 2021 10:10

Update modules/runners/README.md

acae897

Co-authored-by: Gertjan Maas <gertjan@maas.codes>

npalm approved these changes Aug 11, 2021

View reviewed changes

npalm merged commit dd2c2d8 into philips-labs:develop Aug 11, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reviewed the documentation and lambdas #1035

Reviewed the documentation and lambdas #1035

aadrijnberg commented Aug 3, 2021

npalm commented Aug 5, 2021

npalm left a comment

npalm left a comment

npalm Aug 6, 2021

aadrijnberg Aug 6, 2021

npalm commented Aug 10, 2021

gertjanmaas left a comment

npalm left a comment

Reviewed the documentation and lambdas #1035

Reviewed the documentation and lambdas #1035

Conversation

aadrijnberg commented Aug 3, 2021

npalm commented Aug 5, 2021

npalm left a comment

Choose a reason for hiding this comment

npalm left a comment

Choose a reason for hiding this comment

npalm Aug 6, 2021

Choose a reason for hiding this comment

aadrijnberg Aug 6, 2021

Choose a reason for hiding this comment

npalm commented Aug 10, 2021

gertjanmaas left a comment

Choose a reason for hiding this comment

npalm left a comment

Choose a reason for hiding this comment