From 46ab6d5c3c2fc7ed5f31772ef6ed61bf15628ae2 Mon Sep 17 00:00:00 2001 From: Markus Maga Date: Mon, 6 Dec 2021 11:28:43 +0100 Subject: [PATCH 1/2] fix(ecr): use ec2 instance credentials when no credentials are provided Signed-off-by: Markus Maga --- __tests__/docker.test.ts | 78 +++++++++++++++++++++++++++++++++++++++- src/docker.ts | 8 +++-- 2 files changed, 83 insertions(+), 3 deletions(-) diff --git a/__tests__/docker.test.ts b/__tests__/docker.test.ts index 8691b87..144e513 100644 --- a/__tests__/docker.test.ts +++ b/__tests__/docker.test.ts @@ -1,4 +1,5 @@ -import {loginStandard, logout} from '../src/docker'; +import {loginECR, loginStandard, logout} from '../src/docker'; +import * as aws from '../src/aws'; import * as path from 'path'; @@ -47,3 +48,78 @@ test('logout calls exec', async () => { ignoreReturnCode: true }); }); + +test('loginECR sets AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY if username and password is set', async () => { + const execSpy: jest.SpyInstance = jest.spyOn(aws, 'getDockerLoginCmds'); + execSpy.mockImplementation(() => Promise.resolve([])); + jest.spyOn(aws, 'getCLI').mockImplementation(() => Promise.resolve('')); + jest.spyOn(aws, 'getCLIVersion').mockImplementation(() => Promise.resolve('')); + jest.spyOn(aws, 'getRegion').mockImplementation(() => ''); + jest.spyOn(aws, 'getAccountIDs').mockImplementation(() => []); + jest.spyOn(aws, 'isPubECR').mockImplementation(() => false); + + const username: string = 'dbowie'; + const password: string = 'groundcontrol'; + const registry: string = 'https://ghcr.io'; + + await loginECR(registry, username, password); + + expect(process.env.AWS_ACCESS_KEY_ID).toEqual(username); + expect(process.env.AWS_SECRET_ACCESS_KEY).toEqual(password); +}); + +test('loginECR keeps AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY if set', async () => { + const execSpy: jest.SpyInstance = jest.spyOn(aws, 'getDockerLoginCmds'); + execSpy.mockImplementation(() => Promise.resolve([])); + jest.spyOn(aws, 'getCLI').mockImplementation(() => Promise.resolve('')); + jest.spyOn(aws, 'getCLIVersion').mockImplementation(() => Promise.resolve('')); + jest.spyOn(aws, 'getRegion').mockImplementation(() => ''); + jest.spyOn(aws, 'getAccountIDs').mockImplementation(() => []); + jest.spyOn(aws, 'isPubECR').mockImplementation(() => false); + + process.env.AWS_ACCESS_KEY_ID = 'banana'; + process.env.AWS_SECRET_ACCESS_KEY = 'supersecret'; + + await loginECR('ecr.aws', '', ''); + + expect(process.env.AWS_ACCESS_KEY_ID).toEqual('banana'); + expect(process.env.AWS_SECRET_ACCESS_KEY).toEqual('supersecret'); +}); + +test('loginECR overrides AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY if username and password set', async () => { + const execSpy: jest.SpyInstance = jest.spyOn(aws, 'getDockerLoginCmds'); + execSpy.mockImplementation(() => Promise.resolve([])); + jest.spyOn(aws, 'getCLI').mockImplementation(() => Promise.resolve('')); + jest.spyOn(aws, 'getCLIVersion').mockImplementation(() => Promise.resolve('')); + jest.spyOn(aws, 'getRegion').mockImplementation(() => ''); + jest.spyOn(aws, 'getAccountIDs').mockImplementation(() => []); + jest.spyOn(aws, 'isPubECR').mockImplementation(() => false); + + process.env.AWS_ACCESS_KEY_ID = 'banana'; + process.env.AWS_SECRET_ACCESS_KEY = 'supersecret'; + const username = 'myotheruser'; + const password = 'providedpassword'; + + await loginECR('ecr.aws', username, password); + + expect(process.env.AWS_ACCESS_KEY_ID).toEqual(username); + expect(process.env.AWS_SECRET_ACCESS_KEY).toEqual(password); +}); + +test('loginECR does not set AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY if not set', async () => { + const execSpy: jest.SpyInstance = jest.spyOn(aws, 'getDockerLoginCmds'); + execSpy.mockImplementation(() => Promise.resolve([])); + jest.spyOn(aws, 'getCLI').mockImplementation(() => Promise.resolve('')); + jest.spyOn(aws, 'getCLIVersion').mockImplementation(() => Promise.resolve('')); + jest.spyOn(aws, 'getRegion').mockImplementation(() => ''); + jest.spyOn(aws, 'getAccountIDs').mockImplementation(() => []); + jest.spyOn(aws, 'isPubECR').mockImplementation(() => false); + + delete process.env.AWS_ACCESS_KEY_ID; + delete process.env.AWS_SECRET_ACCESS_KEY; + + await loginECR('ecr.aws', '', ''); + + expect('AWS_ACCESS_KEY_ID' in process.env).toEqual(false); + expect('AWS_SECRET_ACCESS_KEY' in process.env).toEqual(false); +}); diff --git a/src/docker.ts b/src/docker.ts index 8f21f68..1178575 100644 --- a/src/docker.ts +++ b/src/docker.ts @@ -62,8 +62,12 @@ export async function loginECR(registry: string, username: string, password: str core.info(`AWS ECR detected with ${region} region`); } - process.env.AWS_ACCESS_KEY_ID = username || process.env.AWS_ACCESS_KEY_ID; - process.env.AWS_SECRET_ACCESS_KEY = password || process.env.AWS_SECRET_ACCESS_KEY; + if (username) { + process.env.AWS_ACCESS_KEY_ID = username; + } + if (password) { + process.env.AWS_SECRET_ACCESS_KEY = password; + } core.info(`Retrieving docker login command through AWS CLI ${cliVersion} (${cliPath})...`); const loginCmds = await aws.getDockerLoginCmds(cliVersion, registry, region, accountIDs); From f6476db6e93cd944a4dd6b91df795144a417221b Mon Sep 17 00:00:00 2001 From: Markus Maga Date: Tue, 14 Dec 2021 14:39:35 +0100 Subject: [PATCH 2/2] chore: update dist Signed-off-by: Markus Maga --- dist/index.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/dist/index.js b/dist/index.js index 80ea456..6e8d198 100644 --- a/dist/index.js +++ b/dist/index.js @@ -262,8 +262,12 @@ function loginECR(registry, username, password) { else { core.info(`AWS ECR detected with ${region} region`); } - process.env.AWS_ACCESS_KEY_ID = username || process.env.AWS_ACCESS_KEY_ID; - process.env.AWS_SECRET_ACCESS_KEY = password || process.env.AWS_SECRET_ACCESS_KEY; + if (username) { + process.env.AWS_ACCESS_KEY_ID = username; + } + if (password) { + process.env.AWS_SECRET_ACCESS_KEY = password; + } core.info(`Retrieving docker login command through AWS CLI ${cliVersion} (${cliPath})...`); const loginCmds = yield aws.getDockerLoginCmds(cliVersion, registry, region, accountIDs); core.info(`Logging into ${registry}...`);