Skip to content
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

Reviewed the documentation and lambdas #1035

Merged
merged 41 commits into from
Aug 11, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
66bd078
chore: Bump aws-sdk (#752) (#909)
dependabot[bot] Jun 18, 2021
77c8e13
chore: Bump aws-sdk (#752) (#908)
dependabot[bot] Jun 18, 2021
fe0a126
chore: Bump aws-sdk (#752) (#887)
dependabot[bot] Jun 18, 2021
174b4ca
chore: Bump aws-sdk (#752) (#885)
dependabot[bot] Jun 18, 2021
0f1493f
chore: Bump aws-sdk (#752) (#889)
dependabot[bot] Jun 18, 2021
5314469
chore: Bump aws-sdk (#752) (#892)
dependabot[bot] Jun 18, 2021
640ef5f
chore: Bump aws-sdk (#752) (#907)
dependabot[bot] Jun 18, 2021
1658635
chore: Bump aws-sdk (#752) (#864)
dependabot[bot] Jun 18, 2021
a401c0d
chore: Bump eslint in /modules/webhook/lambdas/webhook (#918)
dependabot[bot] Jun 24, 2021
870280b
chore: Bump typescript (#929)
dependabot[bot] Jun 24, 2021
0fdf8cf
chore: Bump @typescript-eslint/eslint-plugin (#928)
dependabot[bot] Jun 24, 2021
a6b9a29
chore: Bump typescript in /modules/webhook/lambdas/webhook (#926)
dependabot[bot] Jun 24, 2021
b5096bb
feat: Added support for white listing of repositories (#915)
ravenolf Jul 7, 2021
97ef7fb
chore(examples): Upgrade default example to terraform 1.x.x
npalm Jul 14, 2021
11a6f57
chore(doc): Remove org level admin permission #801
npalm Jul 14, 2021
68635dd
feat: make delay of webhook event configurable (#990)
npalm Jul 15, 2021
6278c17
fix: change module exports and upgrade vercel to latest release (#1005)
PatrickMennen Jul 26, 2021
d5611b0
fix: reduce permission required for session manager (#1018)
npalm Jul 28, 2021
fb3fd99
Merge branch 'develop' of https://github.com/philips-labs/terraform-a…
aadrijnberg Aug 3, 2021
e15ec0c
Merge branch 'philips-labs-develop' into develop
aadrijnberg Aug 3, 2021
86e7912
feat: Store lambda secrets paramaters in Paramater Store (#941)
mcaulifn Aug 4, 2021
0896a15
feat: Adding support for new workflow_job event. (#1019)
npalm Aug 5, 2021
8b6a392
chore: Bump @types/node in /modules/webhook/lambdas/webhook
dependabot[bot] Aug 5, 2021
c23e0ed
chore: Bump tar from 4.4.13 to 4.4.15 in /.release
dependabot[bot] Aug 5, 2021
d467c90
chore: Bump jest-mock-extended in /modules/runners/lambdas/runners
dependabot[bot] Aug 5, 2021
3c87a68
chore: Bump @types/node
dependabot[bot] Aug 5, 2021
5b64c6b
Merge branch 'develop' of github.com:philips-labs/terraform-aws-githu…
aadrijnberg Aug 5, 2021
4f1a25e
fix: handle situation of no prereleases correctly, and propagate lamb…
aadrijnberg Jul 18, 2021
833724e
chore (doc): small improvements
aadrijnberg Jul 18, 2021
791b8a3
fix: wait for scaleDown to have finished before calling callback
aadrijnberg Jul 18, 2021
18b2b45
chore: code consistency and improvements
aadrijnberg Jul 18, 2021
a5ba472
fix: tests were not always correct or incomplete or duplicate
aadrijnberg Jul 18, 2021
6c0b11a
fix: use consistent reporting back to caller
aadrijnberg Jul 18, 2021
aae4735
chore(release): 0.15.1 [skip ci]
semantic-release-bot Jul 13, 2021
f1b3f47
Update CHANGELOG
aadrijnberg Aug 3, 2021
cf6b198
fix: CONTRIBUTION.md should refer develop iso master branch
aadrijnberg Aug 3, 2021
0f52a77
Cleanup
aadrijnberg Aug 5, 2021
cc92fa4
Incorporate review comments and fix build issue
aadrijnberg Aug 6, 2021
2586abe
Merge branch 'develop' of github.com:philips-labs/terraform-aws-githu…
aadrijnberg Aug 6, 2021
ce1afb8
Merge branch 'develop' into my-fixes
aadrijnberg Aug 6, 2021
acae897
Update modules/runners/README.md
aadrijnberg Aug 10, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 18 additions & 18 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@

We'd love for you to contribute to our source code and to make the Forest even better than it is today! Here are the guidelines we'd like you to follow:

- [Question or Problem?](#question)
- [Issues and Bugs](#issue)
- [Feature Requests](#feature)
- [Submission Guidelines](#submit)
- [Further Info](#info)
* [Question or Problem?](#question)
* [Issues and Bugs](#issue)
* [Feature Requests](#feature)
* [Submission Guidelines](#submit)
* [Further Info](#info)

## <a name="question"></a> Got a Question or Problem?

Expand All @@ -27,7 +27,6 @@ You can request a new feature by submitting an issue to our [Github Repository][
* **Major Changes** that you wish to contribute to the project should be discussed first on our [Slack group][slack] so that we can better coordinate our efforts, prevent duplication of work, and help you to craft the change so that it is successfully accepted into the project.
* **Small Changes** can be crafted and submitted to the [Github Repository][github] as a Pull Request.


## <a name="docs"></a> Want a Doc Fix?

If you want to help improve the docs, it's a good idea to let others know what you're working on to minimize duplication of effort. Create a new issue (or comment on a related existing one) to let others know what you're working on.
Expand All @@ -37,6 +36,7 @@ For large fixes, please build and test the documentation before submitting the M
## <a name="submit"></a> Submission Guidelines

### Submitting an Issue

Before you submit your issue search the archive, maybe your question was already answered.

If your issue appears to be a bug, and hasn't been reported, open a new issue. Help us to maximize the effort we can spend fixing issues and adding new features, by not reporting duplicate issues. Providing the following information will increase the chances of your issue being dealt with quickly:
Expand All @@ -58,18 +58,18 @@ Before you submit your merge request consider the following guidelines:
* Make your changes in a new git branch:

```shell
git checkout -b my-fix-branch master
git checkout -b my-fix-branch develop
```

* Create your patch, **including appropriate test cases**.
* Run the test suite and ensure that all tests pass.
* Add a line in the CHANGELOG.md under Unreleased. This will be used form generating the release notes.
* Install [pre-commit hooks](https://pre-commit.com/). The hooks runs some basic checks and update the docs. The commit will run the hooks, you can invoke the hooks manually `pre-commit run --all-files` as well.
* Commit your changes using a descriptive commit message.

```shell
git commit -a
```

Note: the optional commit `-a` command line option will automatically "add" and "rm" edited files.

* Build your changes locally to ensure all the tests pass:
Expand All @@ -79,7 +79,7 @@ Before you submit your merge request consider the following guidelines:
git push origin my-fix-branch
```

In Github, send a pull request to original master branch: f.e. `terraform-aws-vpc:master`.
In Github, send a pull request to original develop branch: f.e. `terraform-aws-vpc:develop`.
If we suggest changes, then:

* Make the required updates.
Expand All @@ -89,10 +89,10 @@ If we suggest changes, then:

If the PR gets too outdated we may ask you to rebase and force push to update the PR:

```shell
git rebase master -i
git push origin my-fix-branch -f
```
```shell
git rebase develop -i
git push origin my-fix-branch -f
```

_WARNING: Squashing or reverting commits and force-pushing thereafter may remove Github comments on code that were previously made by you or others in your commits. Avoid any form of rebasing unless necessary._

Expand All @@ -109,10 +109,10 @@ from the main (upstream) repository:
git push origin --delete my-fix-branch
```

* Check out the master branch:
* Check out the develop branch:

```shell
git checkout master -f
git checkout develop -f
```

* Delete the local branch:
Expand All @@ -121,10 +121,10 @@ from the main (upstream) repository:
git branch -D my-fix-branch
```

* Update your master with the latest upstream version:
* Update your develop with the latest upstream version:

```shell
git pull --ff upstream master
git pull --ff upstream develop
```

## <a name="info"></a> Info
Expand All @@ -136,5 +136,5 @@ Use the badge to sign-up.
[![Slack](https://philips-software-slackin.now.sh/badge.svg)](https://philips-software-slackin.now.sh)

[contribute]: CONTRIBUTING.md
[github]: https://github.com/philips-lam/terraform-aws-github-runner/issues
[github]: https://github.com/philips-labs/terraform-aws-github-runner/issues
[slack]: https://philips-software.slack.com/home
4 changes: 2 additions & 2 deletions examples/default/README.md
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
# Action runners deployment default example

This modules shows how to create GitHub action runners. Lambda release will be downloaded from GitHub.
This module shows how to create GitHub action runners. Lambda release will be downloaded from GitHub.

## Usages

Steps for the full setup, such as creating a GitHub app can be found in the root module's [README](../../README.md). First download the Lambda releases from GitHub. Alternatively you can build the lambdas locally with Node or Docker, there is a simple build script in `<root>/.ci/build.sh`. In the `main.tf` you can simple remove the location of the lambda zip files, the default location will work in this case.
Steps for the full setup, such as creating a GitHub app can be found in the root module's [README](../../README.md). First download the Lambda releases from GitHub. Alternatively you can build the lambdas locally with Node or Docker, there is a simple build script in `<root>/.ci/build.sh`. In the `main.tf` you can simply remove the location of the lambda zip files, the default location will work in this case.

> Ensure you have set the version in `lambdas-download/main.tf` for running the example. The version needs to be set to a GitHub release version, see https://github.com/philips-labs/terraform-aws-github-runner/releases

Expand Down
4 changes: 2 additions & 2 deletions examples/permissions-boundary/README.md
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
# Action runners deployed with permissions boundary

This modules shows how to create GitHub action runners with permissions boundaries and paths used in role, policies, and instance profiles.
This module shows how to create GitHub action runners with permissions boundaries and paths used in role, policies, and instance profiles.

## Usages

Steps for the full setup, such as creating a GitHub app can be find the module [README](../../README.md). First create the deploy role and boundary policies. This steps required an admin user.
Steps for the full setup, such as creating a GitHub app can be find the module [README](../../README.md). First create the deploy role and boundary policies. These steps require an admin user.

> Ensure you have set the version in `lambdas-download/main.tf` for running the example. The version needs to be set to a GitHub release version, see https://github.com/philips-labs/terraform-aws-github-runner/releases

Expand Down
4 changes: 2 additions & 2 deletions examples/ubuntu/README.md
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
# Action runners deployment ubuntu example

This modules shows how to create GitHub action runners using an Ubuntu AMI. Lambda release will be downloaded from GitHub.
This module shows how to create GitHub action runners using an Ubuntu AMI. Lambda release will be downloaded from GitHub.

## Usages

Steps for the full setup, such as creating a GitHub app can be found in the root module's [README](../../README.md). First download the Lambda releases from GitHub. Alternatively you can build the lambdas locally with Node or Docker, there is a simple build script in `<root>/.ci/build.sh`. In the `main.tf` you can simple remove the location of the lambda zip files, the default location will work in this case.
Steps for the full setup, such as creating a GitHub app can be found in the root module's [README](../../README.md). First download the Lambda releases from GitHub. Alternatively you can build the lambdas locally with Node or Docker, there is a simple build script in `<root>/.ci/build.sh`. In the `main.tf` you can simply remove the location of the lambda zip files, the default location will work in this case.

> Ensure you have set the version in `lambdas-download/main.tf` for running the example. The version needs to be set to a GitHub release version, see https://github.com/philips-labs/terraform-aws-github-runner/releases

Expand Down
2 changes: 1 addition & 1 deletion examples/ubuntu/providers.tf
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ terraform {
provider "aws" {
region = local.aws_region

// If you use roles with specific permissons please add your role
// If you use roles with specific permissions please add your role
// assume_role {
// role_arn = "arn:aws:iam::123456789012:role/MyAdminRole"
// }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ import { handle } from './syncer/handler';

// eslint-disable-next-line
export const handler = async (event: any, context: any, callback: any): Promise<void> => {
await handle();
callback();
try {
await handle();
callback(null);
} catch (e) {
callback(e);
}
};
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,30 @@ describe('Synchronize action distribution.', () => {
expect(mockS3.upload).toBeCalledTimes(0);
});

it('Distribution is up-to-date with latest release when there are no prereleases.', async () => {
process.env.GITHUB_RUNNER_ALLOW_PRERELEASE_BINARIES = 'true';
const releases = listReleases.slice(1);

mockOctokit.repos.listReleases.mockImplementation(() => ({
data: releases,
}));
mockS3.getObjectTagging.mockImplementation(() => {
return {
promise() {
return Promise.resolve({ TagSet: [{ Key: 'name', Value: 'actions-runner-linux-x64-2.272.0.tar.gz' }] });
},
};
});

await handle();
expect(mockOctokit.repos.listReleases).toBeCalledTimes(1);
expect(mockS3.getObjectTagging).toBeCalledWith({
Bucket: bucketName,
Key: bucketObjectKey,
});
expect(mockS3.upload).toBeCalledTimes(0);
});

it('Distribution is up-to-date with latest prerelease.', async () => {
process.env.GITHUB_RUNNER_ALLOW_PRERELEASE_BINARIES = 'true';
mockS3.getObjectTagging.mockImplementation(() => {
Expand Down Expand Up @@ -95,6 +119,32 @@ describe('Synchronize action distribution.', () => {
expect(s3JsonBody['Tagging']).toEqual('name=actions-runner-linux-x64-2.272.0.tar.gz');
});

it('Distribution should update to release if there are no pre-releases.', async () => {
process.env.GITHUB_RUNNER_ALLOW_PRERELEASE_BINARIES = 'true';
const releases = listReleases.slice(1);

mockOctokit.repos.listReleases.mockImplementation(() => ({
data: releases,
}));
mockS3.getObjectTagging.mockImplementation(() => {
return {
promise() {
return Promise.resolve({ TagSet: [{ Key: 'name', Value: 'actions-runner-linux-x64-0.tar.gz' }] });
},
};
});

await handle();
expect(mockOctokit.repos.listReleases).toBeCalledTimes(1);
expect(mockS3.getObjectTagging).toBeCalledWith({
Bucket: bucketName,
Key: bucketObjectKey,
});
expect(mockS3.upload).toBeCalledTimes(1);
const s3JsonBody = mockS3.upload.mock.calls[0][0];
expect(s3JsonBody['Tagging']).toEqual('name=actions-runner-linux-x64-2.272.0.tar.gz');
});

it('Distribution should update to prerelease.', async () => {
process.env.GITHUB_RUNNER_ALLOW_PRERELEASE_BINARIES = 'true';
mockS3.getObjectTagging.mockImplementation(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ async function getLinuxReleaseAsset(
const latestReleaseIndex = assetsList.data.findIndex((a) => a.prerelease === false);

let asset = undefined;
if (fetchPrereleaseBinaries && latestPrereleaseIndex < latestReleaseIndex) {
if (fetchPrereleaseBinaries && latestPrereleaseIndex != -1 && latestPrereleaseIndex < latestReleaseIndex) {
asset = assetsList.data[latestPrereleaseIndex];
} else if (latestReleaseIndex != -1) {
asset = assetsList.data[latestReleaseIndex];
Expand Down Expand Up @@ -86,6 +86,7 @@ async function uploadToS3(s3: S3, cacheObject: CacheObject, actionRunnerReleaseA
});
}).catch((error) => {
console.error(`Exception: ${error}`);
throw error;
});
}

Expand Down
4 changes: 2 additions & 2 deletions modules/runners/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@ This module creates resources required to run the GitHub action runner on AWS EC

### Action runners on EC2

The action runners are created via a launch template, on launch template only the subnet needs to be provided. During launch the installation is handled via a user data script. The configuration is fetched from SSM parameter store.
The action runners are created via a launch template; in the launch template only the subnet needs to be provided. During launch the installation is handled via a user data script. The configuration is fetched from SSM parameter store.

### Lambda scale up

The scale up lambda is triggered by events on a SQS queue. Events on this queued are delayed, which will will give the workflow some time to start running on available runners. For each event the lambda will check the workflow is still queued and no other limits are reached. In that case the lambda will create a new EC2 instance. The lambda only needs to know which launch template to use and which subnets are available. From the available subnets a random one will be chosen. Once the instance is created the event is assumed as handled, and we assume the workflow wil start at some moment once the created instance is ready.
The scale up lambda is triggered by events on a SQS queue. Events on this queue are delayed, which will give the workflow some time to start running on available runners. For each event the lambda will check if the workflow is still queued and no other limits are reached. In that case the lambda will create a new EC2 instance. The lambda only needs to know which launch template to use and which subnets are available. From the available subnets a random one will be chosen. Once the instance is created the event is assumed as handled, and we assume the workflow wil start at some moment once the created instance is ready.

### Lambda scale down

Expand Down
2 changes: 1 addition & 1 deletion modules/runners/lambdas/runners/src/lambda.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export const scaleUp = async (event: SQSEvent, context: Context, callback: any):

export const scaleDown = async (event: ScheduledEvent, context: Context, callback: any): Promise<void> => {
try {
scaleDownAction();
await scaleDownAction();
callback(null);
} catch (e) {
console.error(e);
Expand Down
21 changes: 19 additions & 2 deletions modules/runners/lambdas/runners/src/scale-runners/runners.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { listRunners, createRunner } from './runners';
import { listRunners, createRunner, terminateRunner, RunnerInfo } from './runners';

const mockEC2 = { describeInstances: jest.fn(), runInstances: jest.fn() };
const mockEC2 = { describeInstances: jest.fn(), runInstances: jest.fn(), terminateInstances: jest.fn() };
const mockSSM = { putParameter: jest.fn() };
jest.mock('aws-sdk', () => ({
EC2: jest.fn().mockImplementation(() => mockEC2),
Expand Down Expand Up @@ -102,6 +102,23 @@ describe('list instances', () => {
});
});

describe('terminate runner', () => {
const mockTerminateInstances = { promise: jest.fn() };
beforeEach(() => {
jest.clearAllMocks();
mockEC2.terminateInstances.mockImplementation(() => mockTerminateInstances);
mockTerminateInstances.promise.mockReturnThis();
});
it('calls terminate instances with the right instance ids', async () => {
const runner: RunnerInfo = {
instanceId: 'instance-2',
};
await terminateRunner(runner);

expect(mockEC2.terminateInstances).toBeCalledWith({ InstanceIds: [runner.instanceId] });
});
});

describe('create runner', () => {
const mockRunInstances = { promise: jest.fn() };
const mockPutParameter = { promise: jest.fn() };
Expand Down
8 changes: 4 additions & 4 deletions modules/runners/lambdas/runners/src/scale-runners/runners.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@ import { EC2, SSM } from 'aws-sdk';

export interface RunnerInfo {
instanceId: string;
launchTime: Date | undefined;
repo: string | undefined;
org: string | undefined;
launchTime?: Date;
repo?: string;
org?: string;
}

export interface ListRunnerFilters {
runnerType?: 'Org' | 'Repo';
runnerOwner?: string;
environment: string | undefined;
environment?: string;
}

export interface RunnerInputParameters {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,11 @@ const DEFAULT_IDLE_COUNT = 1;
const now = moment.tz(new Date(), 'America/Los_Angeles');

function getConfig(cronTabs: string[]): ScalingDownConfigList {
const result: ScalingDownConfigList = [];
for (const cron of cronTabs) {
result.push({
cron: cron,
idleCount: DEFAULT_IDLE_COUNT,
timeZone: DEFAULT_TIMEZONE,
});
}
return result;
return cronTabs.map((cron) => ({
cron: cron,
idleCount: DEFAULT_IDLE_COUNT,
timeZone: DEFAULT_TIMEZONE,
}));
}

describe('scaleDownConfig', () => {
Expand Down
Loading