Merge pull request #292 from aws-actions/arjraman

Removed docker username mask, refactored and formatted code, and adde…
aws-actions · May 19, 2022 · c81a5ff · c81a5ff
2 parents 023e457 + 58c1548
commit c81a5ff
Show file tree

Hide file tree

Showing 7 changed files with 469 additions and 455 deletions.
diff --git a/action.yml b/action.yml
@@ -10,6 +10,7 @@ inputs:
  skip-logout:
  description: 'Whether to skip explicit logout of the registries during post-job cleanup. Exists for backward compatibility on self-hosted runners. Not recommended.'
  required: false
+ default: 'false'
 outputs:
  registry:
  description: 'The URI of the ECR registry i.e. aws_account_id.dkr.ecr.region.amazonaws.com. If multiple registries are provided as inputs, this output will not be set.'

diff --git a/cleanup.js b/cleanup.js
@@ -7,19 +7,21 @@ const exec = require('@actions/exec');
  */
 
 async function cleanup() {
- try {
+  try {
  const registriesState = core.getState('registries');
+
  if (registriesState) {
  const registries = registriesState.split(',');
  const failedLogouts = [];
 
+ // Logout of each registry
  for (const registry of registries) {
- core.debug(`Logging out registry ${registry}`);
+ core.debug(`Logging out of registry ${registry}`);
 
  // Execute the docker logout command
  let doLogoutStdout = '';
  let doLogoutStderr = '';
- const exitCode = await exec.exec('docker', ["logout", registry], {
+ const exitCode = await exec.exec('docker', ['logout', registry], {
  silent: true,
  ignoreReturnCode: true,
  listeners: {
@@ -31,8 +33,7 @@ async function cleanup() {
  }
  }
  });
-
- if (exitCode != 0) {
+ if (exitCode !== 0) {
  core.debug(doLogoutStdout);
  core.error(`Could not logout registry ${registry}: ${doLogoutStderr}`);
  failedLogouts.push(registry);

diff --git a/cleanup.test.js b/cleanup.test.js
@@ -7,94 +7,94 @@ jest.mock('@actions/exec');
 
 describe('Logout from ECR', () => {
 
-  beforeEach(() => {
-  jest.clearAllMocks();
-
-  core.getState.mockReturnValue(
-  '123456789012.dkr.ecr.aws-region-1.amazonaws.com,111111111111.dkr.ecr.aws-region-1.amazonaws.com');
-  exec.exec.mockReturnValue(0);
-  });
-
-  test('logs out docker client for registries in action state', async () => {
-  await cleanup();
-
-  expect(core.getState).toHaveBeenCalledWith('registries');
-
-  expect(exec.exec).toHaveBeenCalledTimes(2);
-  expect(exec.exec).toHaveBeenNthCalledWith(1,
-  'docker',
-  ['logout', '123456789012.dkr.ecr.aws-region-1.amazonaws.com'],
-  expect.anything());
-  expect(exec.exec).toHaveBeenNthCalledWith(2,
-  'docker',
-  ['logout', '111111111111.dkr.ecr.aws-region-1.amazonaws.com'],
-  expect.anything());
-
-  expect(core.setFailed).toHaveBeenCalledTimes(0);
-  });
-
-  test('handles zero registries', async () => {
-  core.getState.mockReturnValue('');
-
-  await cleanup();
-
-  expect(core.getState).toHaveBeenCalledWith('registries');
-
-  expect(exec.exec).toHaveBeenCalledTimes(0);
-  expect(core.setFailed).toHaveBeenCalledTimes(0);
-  });
-
-  test('error is caught by core.setFailed for failed docker logout', async () => {
-  exec.exec.mockReturnValue(1);
-
-  await cleanup();
-
-  expect(core.setFailed).toBeCalled();
-  });
-
-  test('continues to attempt logouts after a failed logout', async () => {
-  core.getState.mockReturnValue(
-  '123456789012.dkr.ecr.aws-region-1.amazonaws.com,111111111111.dkr.ecr.aws-region-1.amazonaws.com,222222222222.dkr.ecr.aws-region-1.amazonaws.com');
-  exec.exec
-  .mockImplementationOnce((commandLine, args, options) => {
-  options.listeners.stdout("stdout of ");
-  options.listeners.stdout("registry 1");
-  options.listeners.stderr("stderr of ");
-  options.listeners.stderr("registry 1");
-  return(1);
-  })
-  .mockImplementationOnce((commandLine, args, options) => {
-  options.listeners.stdout("stdout of ");
-  options.listeners.stdout("registry 2");
-  options.listeners.stderr("stderr of ");
-  options.listeners.stderr("registry 2");
-  return(1);
-  })
-  .mockReturnValueOnce(0);
-
-  await cleanup();
-
-  expect(core.getState).toHaveBeenCalledWith('registries');
-
-  expect(exec.exec).toHaveBeenCalledTimes(3);
-  expect(exec.exec).toHaveBeenNthCalledWith(1,
-  'docker',
-  ['logout', '123456789012.dkr.ecr.aws-region-1.amazonaws.com'],
-  expect.anything());
-  expect(exec.exec).toHaveBeenNthCalledWith(2,
-  'docker',
-  ['logout', '111111111111.dkr.ecr.aws-region-1.amazonaws.com'],
-  expect.anything());
-  expect(exec.exec).toHaveBeenNthCalledWith(3,
-  'docker',
-  ['logout', '222222222222.dkr.ecr.aws-region-1.amazonaws.com'],
-  expect.anything());
-
-  expect(core.error).toHaveBeenCalledTimes(2);
-  expect(core.error).toHaveBeenNthCalledWith(1, 'Could not logout registry 123456789012.dkr.ecr.aws-region-1.amazonaws.com: stderr of registry 1');
-  expect(core.error).toHaveBeenNthCalledWith(2, 'Could not logout registry 111111111111.dkr.ecr.aws-region-1.amazonaws.com: stderr of registry 2');
-
-  expect(core.setFailed).toHaveBeenCalledTimes(1);
-  expect(core.setFailed).toHaveBeenCalledWith('Failed to logout: 123456789012.dkr.ecr.aws-region-1.amazonaws.com,111111111111.dkr.ecr.aws-region-1.amazonaws.com');
-  });
+ beforeEach(() => {
+ jest.clearAllMocks();
+
+ core.getState.mockReturnValue(
+ '123456789012.dkr.ecr.aws-region-1.amazonaws.com,111111111111.dkr.ecr.aws-region-1.amazonaws.com');
+ exec.exec.mockReturnValue(0);
+ });
+
+ test('logs out docker client for registries in action state', async () => {
+ await cleanup();
+
+ expect(core.getState).toHaveBeenCalledWith('registries');
+
+ expect(exec.exec).toHaveBeenCalledTimes(2);
+ expect(exec.exec).toHaveBeenNthCalledWith(1,
+ 'docker',
+ ['logout', '123456789012.dkr.ecr.aws-region-1.amazonaws.com'],
+ expect.anything());
+ expect(exec.exec).toHaveBeenNthCalledWith(2,
+ 'docker',
+ ['logout', '111111111111.dkr.ecr.aws-region-1.amazonaws.com'],
+ expect.anything());
+
+ expect(core.setFailed).toHaveBeenCalledTimes(0);
+ });
+
+ test('handles zero registries', async () => {
+ core.getState.mockReturnValue('');
+
+ await cleanup();
+
+ expect(core.getState).toHaveBeenCalledWith('registries');
+
+ expect(exec.exec).toHaveBeenCalledTimes(0);
+ expect(core.setFailed).toHaveBeenCalledTimes(0);
+ });
+
+ test('error is caught by core.setFailed for failed docker logout', async () => {
+ exec.exec.mockReturnValue(1);
+
+ await cleanup();
+
+ expect(core.setFailed).toBeCalled();
+ });
+
+ test('continues to attempt logouts after a failed logout', async () => {
+ core.getState.mockReturnValue(
+ '123456789012.dkr.ecr.aws-region-1.amazonaws.com,111111111111.dkr.ecr.aws-region-1.amazonaws.com,222222222222.dkr.ecr.aws-region-1.amazonaws.com');
+ exec.exec
+ .mockImplementationOnce((commandLine, args, options) => {
+ options.listeners.stdout('stdout of ');
+ options.listeners.stdout('registry 1');
+ options.listeners.stderr('stderr of ');
+ options.listeners.stderr('registry 1');
+ return(1);
+ })
+ .mockImplementationOnce((commandLine, args, options) => {
+ options.listeners.stdout('stdout of ');
+ options.listeners.stdout('registry 2');
+ options.listeners.stderr('stderr of ');
+ options.listeners.stderr('registry 2');
+ return(1);
+ })
+ .mockReturnValueOnce(0);
+
+ await cleanup();
+
+ expect(core.getState).toHaveBeenCalledWith('registries');
+
+ expect(exec.exec).toHaveBeenCalledTimes(3);
+ expect(exec.exec).toHaveBeenNthCalledWith(1,
+ 'docker',
+ ['logout', '123456789012.dkr.ecr.aws-region-1.amazonaws.com'],
+ expect.anything());
+ expect(exec.exec).toHaveBeenNthCalledWith(2,
+ 'docker',
+ ['logout', '111111111111.dkr.ecr.aws-region-1.amazonaws.com'],
+ expect.anything());
+ expect(exec.exec).toHaveBeenNthCalledWith(3,
+ 'docker',
+ ['logout', '222222222222.dkr.ecr.aws-region-1.amazonaws.com'],
+ expect.anything());
+
+ expect(core.error).toHaveBeenCalledTimes(2);
+ expect(core.error).toHaveBeenNthCalledWith(1, 'Could not logout registry 123456789012.dkr.ecr.aws-region-1.amazonaws.com: stderr of registry 1');
+ expect(core.error).toHaveBeenNthCalledWith(2, 'Could not logout registry 111111111111.dkr.ecr.aws-region-1.amazonaws.com: stderr of registry 2');
+
+ expect(core.setFailed).toHaveBeenCalledTimes(1);
+ expect(core.setFailed).toHaveBeenCalledWith('Failed to logout: 123456789012.dkr.ecr.aws-region-1.amazonaws.com,111111111111.dkr.ecr.aws-region-1.amazonaws.com');
+ });
 });
diff --git a/dist/cleanup/index.js b/dist/cleanup/index.js
@@ -1423,19 +1423,21 @@ const exec = __webpack_require__(986);
  */
 
 async function cleanup() {
- try {
+  try {
  const registriesState = core.getState('registries');
+
  if (registriesState) {
  const registries = registriesState.split(',');
  const failedLogouts = [];
 
+ // Logout of each registry
  for (const registry of registries) {
- core.debug(`Logging out registry ${registry}`);
+ core.debug(`Logging out of registry ${registry}`);
 
  // Execute the docker logout command
  let doLogoutStdout = '';
  let doLogoutStderr = '';
- const exitCode = await exec.exec('docker', ["logout", registry], {
+ const exitCode = await exec.exec('docker', ['logout', registry], {
  silent: true,
  ignoreReturnCode: true,
  listeners: {
@@ -1447,8 +1449,7 @@ async function cleanup() {
  }
  }
  });
-
- if (exitCode != 0) {
+ if (exitCode !== 0) {
  core.debug(doLogoutStdout);
  core.error(`Could not logout registry ${registry}: ${doLogoutStderr}`);
  failedLogouts.push(registry);

diff --git a/dist/index.js b/dist/index.js
@@ -523,17 +523,18 @@ const exec = __webpack_require__(4986);
 const aws = __webpack_require__(9350);
 
 function replaceSpecialCharacters(registryUri) {
- return registryUri.replace(/[^a-zA-Z0-9_]+/g, '_')
+ return registryUri.replace(/[^a-zA-Z0-9_]+/g, '_');
 }
 
 async function run() {
+ // Get inputs
+ const skipLogout = core.getInput('skip-logout', { required: false }) === 'true';
+ const registries = core.getInput('registries', { required: false });
+
  const registryUriState = [];
- const skipLogout = core.getInput('skip-logout', { required: false });
 
  try {
- const registries = core.getInput('registries', { required: false });
-
- // Get the ECR authorization token
+ // Get the ECR authorization token(s)
  const ecr = new aws.ECR({
  customUserAgent: 'amazon-ecr-login-for-github-actions'
  });
@@ -551,14 +552,17 @@ async function run() {
  throw new Error('Could not retrieve an authorization token from Amazon ECR');
  }
 
+ // Login to each registry
  for (const authData of authTokenResponse.authorizationData) {
  const authToken = Buffer.from(authData.authorizationToken, 'base64').toString('utf-8');
  const creds = authToken.split(':', 2);
  const proxyEndpoint = authData.proxyEndpoint;
  const registryUri = proxyEndpoint.replace(/^https?:\/\//,'');
 
- if (authTokenResponse.authorizationData.length == 1) {
- // output the registry URI if this action is doing a single registry login
+ core.debug(`Logging in to registry ${registryUri}`);
+
+ // output the registry URI if this action is doing a single registry login
+ if (authTokenResponse.authorizationData.length === 1) {
  core.setOutput('registry', registryUri);
  }
 
@@ -577,15 +581,14 @@ async function run() {
  }
  }
  });
-
- if (exitCode != 0) {
+ if (exitCode !== 0) {
  core.debug(doLoginStdout);
- throw new Error('Could not login: ' + doLoginStderr);
+ throw new Error(`Could not login to ${proxyEndpoint}: ${doLoginStderr}`);
  }
 
+ // Output docker username and password
  const secretSuffix = replaceSpecialCharacters(registryUri)
- core.setSecret(creds[0])
- core.setSecret(creds[1])
+ core.setSecret(creds[1]);
  core.setOutput(`docker_username_${secretSuffix}`, creds[0]);
  core.setOutput(`docker_password_${secretSuffix}`, creds[1]);
 
@@ -599,7 +602,7 @@ async function run() {
  // Pass the logged-in registry URIs to the post action for logout
  if (registryUriState.length) {
  if (!skipLogout) {
-  core.saveState('registries', registryUriState.join());
+ core.saveState('registries', registryUriState.join());
  }
  core.debug(`'skip-logout' is ${skipLogout} for ${registryUriState.length} registries.`);
  }
@@ -612,7 +615,7 @@ module.exports = {
 
 /* istanbul ignore next */
 if (require.main === require.cache[eval('__filename')]) {
-  run();
+ run();
 }