diff --git a/modules/runners/lambdas/runners/src/aws/runners.test.ts b/modules/runners/lambdas/runners/src/aws/runners.test.ts index 74446a6850..15a1772ab4 100644 --- a/modules/runners/lambdas/runners/src/aws/runners.test.ts +++ b/modules/runners/lambdas/runners/src/aws/runners.test.ts @@ -15,41 +15,53 @@ const ORG_NAME = 'SomeAwesomeCoder'; const REPO_NAME = `${ORG_NAME}/some-amazing-library`; const ENVIRONMENT = 'unit-test-environment'; -describe('list instances', () => { - const mockDescribeInstances = { promise: jest.fn() }; - beforeEach(() => { - jest.clearAllMocks(); - mockEC2.describeInstances.mockImplementation(() => mockDescribeInstances); - const mockRunningInstances: AWS.EC2.DescribeInstancesResult = { - Reservations: [ +const mockDescribeInstances = { promise: jest.fn() }; +mockEC2.describeInstances.mockImplementation(() => mockDescribeInstances); +const mockRunningInstances: AWS.EC2.DescribeInstancesResult = { + Reservations: [ + { + Instances: [ { - Instances: [ - { - LaunchTime: new Date('2020-10-10T14:48:00.000+09:00'), - InstanceId: 'i-1234', - Tags: [ - { Key: 'Application', Value: 'github-action-runner' }, - { Key: 'Type', Value: 'Org' }, - { Key: 'Owner', Value: 'CoderToCat' }, - ], - }, - { - LaunchTime: new Date('2020-10-11T14:48:00.000+09:00'), - InstanceId: 'i-5678', - Tags: [ - { Key: 'Owner', Value: REPO_NAME }, - { Key: 'Type', Value: 'Repo' }, - { Key: 'Application', Value: 'github-action-runner' }, - ], - }, + LaunchTime: new Date('2020-10-10T14:48:00.000+09:00'), + InstanceId: 'i-1234', + Tags: [ + { Key: 'ghr:Application', Value: 'github-action-runner' }, + { Key: 'Type', Value: 'Org' }, + { Key: 'Owner', Value: 'CoderToCat' }, ], }, ], - }; - mockDescribeInstances.promise.mockReturnValue(mockRunningInstances); + }, + ], +}; +const mockRunningInstancesLegacy: AWS.EC2.DescribeInstancesResult = { + Reservations: [ + { + Instances: [ + { + LaunchTime: new Date('2020-10-11T14:48:00.000+09:00'), + InstanceId: 'i-5678', + Tags: [ + { Key: 'Owner', Value: REPO_NAME }, + { Key: 'Type', Value: 'Repo' }, + { Key: 'Application', Value: 'github-action-runner' }, + ], + }, + ], + }, + ], +}; + +describe('list instances', () => { + beforeEach(() => { + jest.resetModules(); + jest.clearAllMocks(); }); it('returns a list of instances', async () => { + mockDescribeInstances.promise + .mockReturnValueOnce(mockRunningInstances) + .mockReturnValueOnce(mockRunningInstancesLegacy); const resp = await listEC2Runners(); expect(resp.length).toBe(2); expect(resp).toContainEqual({ @@ -67,41 +79,61 @@ describe('list instances', () => { }); it('calls EC2 describe instances', async () => { + mockDescribeInstances.promise + .mockReturnValueOnce(mockRunningInstances) + .mockReturnValueOnce(mockRunningInstancesLegacy); await listEC2Runners(); expect(mockEC2.describeInstances).toBeCalled(); }); it('filters instances on repo name', async () => { + mockDescribeInstances.promise + .mockReturnValueOnce(mockRunningInstances) + .mockReturnValueOnce(mockRunningInstancesLegacy); await listEC2Runners({ runnerType: 'Repo', runnerOwner: REPO_NAME, environment: undefined }); expect(mockEC2.describeInstances).toBeCalledWith({ Filters: [ - { Name: 'tag:Application', Values: ['github-action-runner'] }, { Name: 'instance-state-name', Values: ['running', 'pending'] }, { Name: 'tag:Type', Values: ['Repo'] }, { Name: 'tag:Owner', Values: [REPO_NAME] }, + { Name: 'tag:ghr:Application', Values: ['github-action-runner'] }, + ], + }); + expect(mockEC2.describeInstances).toBeCalledWith({ + Filters: [ + { Name: 'instance-state-name', Values: ['running', 'pending'] }, + { Name: 'tag:Type', Values: ['Repo'] }, + { Name: 'tag:Owner', Values: [REPO_NAME] }, + { Name: 'tag:Application', Values: ['github-action-runner'] }, ], }); }); it('filters instances on org name', async () => { + mockDescribeInstances.promise + .mockReturnValueOnce(mockRunningInstances) + .mockReturnValueOnce(mockRunningInstancesLegacy); await listEC2Runners({ runnerType: 'Org', runnerOwner: ORG_NAME, environment: undefined }); expect(mockEC2.describeInstances).toBeCalledWith({ Filters: [ - { Name: 'tag:Application', Values: ['github-action-runner'] }, { Name: 'instance-state-name', Values: ['running', 'pending'] }, { Name: 'tag:Type', Values: ['Org'] }, { Name: 'tag:Owner', Values: [ORG_NAME] }, + { Name: 'tag:ghr:Application', Values: ['github-action-runner'] }, ], }); }); it('filters instances on environment', async () => { + mockDescribeInstances.promise + .mockReturnValueOnce(mockRunningInstances) + .mockReturnValueOnce(mockRunningInstancesLegacy); await listEC2Runners({ environment: ENVIRONMENT }); expect(mockEC2.describeInstances).toBeCalledWith({ Filters: [ - { Name: 'tag:Application', Values: ['github-action-runner'] }, { Name: 'instance-state-name', Values: ['running', 'pending'] }, { Name: 'tag:ghr:environment', Values: [ENVIRONMENT] }, + { Name: 'tag:ghr:Application', Values: ['github-action-runner'] }, ], }); }); @@ -123,7 +155,7 @@ describe('list instances', () => { }, ], }; - mockDescribeInstances.promise.mockReturnValue(noInstances); + mockDescribeInstances.promise.mockReturnValueOnce(noInstances).mockReturnValueOnce(noInstances); const resp = await listEC2Runners(); expect(resp.length).toBe(0); }); @@ -142,7 +174,7 @@ describe('list instances', () => { }, ], }; - mockDescribeInstances.promise.mockReturnValue(noInstances); + mockDescribeInstances.promise.mockReturnValueOnce(noInstances).mockReturnValue({}); const resp = await listEC2Runners(); expect(resp.length).toBe(1); }); @@ -459,7 +491,7 @@ function expectedCreateFleetRequest(expectedValues: ExpectedFleetRequestValues): { ResourceType: 'instance', Tags: [ - { Key: 'Application', Value: 'github-action-runner' }, + { Key: 'ghr:Application', Value: 'github-action-runner' }, { Key: 'Type', Value: expectedValues.type }, { Key: 'Owner', Value: REPO_NAME }, ], diff --git a/modules/runners/lambdas/runners/src/aws/runners.ts b/modules/runners/lambdas/runners/src/aws/runners.ts index cf1c503158..6784853a01 100644 --- a/modules/runners/lambdas/runners/src/aws/runners.ts +++ b/modules/runners/lambdas/runners/src/aws/runners.ts @@ -45,24 +45,47 @@ export interface RunnerInputParameters { amiIdSsmParameterName?: string; } +interface Ec2Filter { + Name: string; + Values: string[]; +} + export async function listEC2Runners(filters: ListRunnerFilters | undefined = undefined): Promise { - const ec2Statuses = filters?.statuses ? filters.statuses : ['running', 'pending']; - const ec2 = new EC2(); - const ec2Filters = [ - { Name: 'tag:Application', Values: ['github-action-runner'] }, - { Name: 'instance-state-name', Values: ec2Statuses }, - ]; + const ec2Filters = constructFilters(filters); + const runners: RunnerList[] = []; + for (const filter of ec2Filters) { + runners.push(...(await getRunners(filter))); + } + return runners; +} +function constructFilters(filters?: ListRunnerFilters): Ec2Filter[][] { + const ec2Statuses = filters?.statuses ? filters.statuses : ['running', 'pending']; + const ec2Filters: Ec2Filter[][] = []; + const ec2FiltersBase = [{ Name: 'instance-state-name', Values: ec2Statuses }]; if (filters) { if (filters.environment !== undefined) { - ec2Filters.push({ Name: 'tag:ghr:environment', Values: [filters.environment] }); + ec2FiltersBase.push({ Name: 'tag:ghr:environment', Values: [filters.environment] }); } if (filters.runnerType && filters.runnerOwner) { - ec2Filters.push({ Name: `tag:Type`, Values: [filters.runnerType] }); - ec2Filters.push({ Name: `tag:Owner`, Values: [filters.runnerOwner] }); + ec2FiltersBase.push({ Name: `tag:Type`, Values: [filters.runnerType] }); + ec2FiltersBase.push({ Name: `tag:Owner`, Values: [filters.runnerOwner] }); } } + // ***Deprecation Notice*** + // Support for legacy `Application` tag keys + // will be removed in next major release. + for (const key of ['tag:ghr:Application', 'tag:Application']) { + const filter = [...ec2FiltersBase]; + filter.push({ Name: key, Values: ['github-action-runner'] }); + ec2Filters.push(filter); + } + return ec2Filters; +} + +async function getRunners(ec2Filters: Ec2Filter[]): Promise { + const ec2 = new EC2(); const runners: RunnerList[] = []; let nextToken; let hasNext = true; @@ -191,7 +214,7 @@ export async function createRunner(runnerParameters: RunnerInputParameters): Pro { ResourceType: 'instance', Tags: [ - { Key: 'Application', Value: 'github-action-runner' }, + { Key: 'ghr:Application', Value: 'github-action-runner' }, { Key: 'Type', Value: runnerParameters.runnerType }, { Key: 'Owner', Value: runnerParameters.runnerOwner }, ], diff --git a/modules/runners/policies/lambda-scale-down.json b/modules/runners/policies/lambda-scale-down.json index 6789abff58..f745ba52bf 100644 --- a/modules/runners/policies/lambda-scale-down.json +++ b/modules/runners/policies/lambda-scale-down.json @@ -1,48 +1,62 @@ { - "Version": "2012-10-17", - "Statement": [ - { - "Effect": "Allow", - "Action": [ - "ec2:DescribeInstances", - "ec2:DescribeTags" - ], - "Resource": [ - "*" - ] - }, - { - "Effect": "Allow", - "Action": [ - "ec2:TerminateInstances" - ], - "Resource": [ - "*" - ], - "Condition": { - "StringEquals": { - "ec2:ResourceTag/Application": "github-action-runner" - } - } - }, - { - "Effect": "Allow", - "Action": [ - "ssm:GetParameter" - ], - "Resource": [ - "${github_app_key_base64_arn}", - "${github_app_id_arn}" - ] + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Action": [ + "ec2:DescribeInstances", + "ec2:DescribeTags" + ], + "Resource": [ + "*" + ] + }, + { + "Effect": "Allow", + "Action": [ + "ec2:TerminateInstances" + ], + "Resource": [ + "*" + ], + "Condition": { + "StringEquals": { + "ec2:ResourceTag/ghr:Application": "github-action-runner" + } + } + }, + { + "Effect": "Allow", + "Action": [ + "ec2:TerminateInstances" + ], + "Resource": [ + "*" + ], + "Condition": { + "StringEquals": { + "ec2:ResourceTag/Application": "github-action-runner" + } + } + }, + { + "Effect": "Allow", + "Action": [ + "ssm:GetParameter" + ], + "Resource": [ + "${github_app_key_base64_arn}", + "${github_app_id_arn}" + ] %{ if kms_key_arn != "" ~} - }, - { - "Effect": "Allow", - "Action": [ - "kms:Decrypt" - ], - "Resource": "${kms_key_arn}" + }, + { + "Effect": "Allow", + "Action": [ + "kms:Decrypt" + ], + "Resource": "${kms_key_arn}" %{ endif ~} - } - ] + } + ] }