feat: logout docker registries in post step (#70)

* feat: logout docker registries in post step * attempt to logout all registries, even if some fail Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
aws-actions · Aug 9, 2020 · 6cfbb32 · 6cfbb32
1 parent a004fcf
commit 6cfbb32
Show file tree

Hide file tree

Showing 7 changed files with 211 additions and 9 deletions.
diff --git a/README.md b/README.md
@@ -29,10 +29,6 @@ Logs in the local Docker client to one or more Amazon ECR registries.
       run: |
         docker build -t $ECR_REGISTRY/$ECR_REPOSITORY:$IMAGE_TAG .
         docker push $ECR_REGISTRY/$ECR_REPOSITORY:$IMAGE_TAG
-
-    - name: Logout of Amazon ECR
-      if: always()
-      run: docker logout ${{ steps.login-ecr.outputs.registry }}
 ```
 
 See [action.yml](action.yml) for the full documentation for this action's inputs and outputs.

diff --git a/action.yml b/action.yml
@@ -13,3 +13,4 @@ outputs:
 runs:
   using: 'node12'
   main: 'dist/index.js'
+  post: 'dist/cleanup/index.js'
diff --git a/cleanup.js b/cleanup.js
@@ -0,0 +1,57 @@
+const core = require('@actions/core');
+const exec = require('@actions/exec');
+
+/**
+ * When the GitHub Actions job is done, remove saved ECR credentials from the
+ * local Docker engine in the job's environment.
+ */
+
+async function cleanup() {
+ try {
+    const registriesState = core.getState('registries');
+    if (registriesState) {
+      const registries = registriesState.split(',');
+      const failedLogouts = [];
+
+      for (const registry of registries) {
+        core.debug(`Logging out registry ${registry}`);
+
+        // Execute the docker logout command
+        let doLogoutStdout = '';
+        let doLogoutStderr = '';
+        const exitCode = await exec.exec('docker logout', [registry], {
+          silent: true,
+          ignoreReturnCode: true,
+          listeners: {
+            stdout: (data) => {
+              doLogoutStdout += data.toString();
+            },
+            stderr: (data) => {
+              doLogoutStderr += data.toString();
+            }
+          }
+        });
+
+        if (exitCode != 0) {
+          core.debug(doLogoutStdout);
+          core.error(`Could not logout registry ${registry}: ${doLogoutStderr}`);
+          failedLogouts.push(registry);
+        }
+      }
+
+      if (failedLogouts.length) {
+        throw new Error(`Failed to logout: ${failedLogouts.join(',')}`);
+      }
+    }
+  }
+  catch (error) {
+    core.setFailed(error.message);
+  }
+}
+
+module.exports = cleanup;
+
+/* istanbul ignore next */
+if (require.main === module) {
+  cleanup();
+}
diff --git a/cleanup.test.js b/cleanup.test.js
@@ -0,0 +1,85 @@
+const cleanup = require('./cleanup.js');
+const core = require('@actions/core');
+const exec = require('@actions/exec');
+
+jest.mock('@actions/core');
+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.mockReturnValueOnce(1).mockReturnValueOnce(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: ');
+        expect(core.error).toHaveBeenNthCalledWith(2, 'Could not logout registry 111111111111.dkr.ecr.aws-region-1.amazonaws.com: ');
+
+        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/index.js b/index.js
@@ -3,6 +3,8 @@ const exec = require('@actions/exec');
 const aws = require('aws-sdk');
 
 async function run() {
+  const registryUriState = [];
+
   try {
     const registries = core.getInput('registries', { required: false });
 
@@ -28,11 +30,11 @@ async function run() {
       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
-        const registryId = proxyEndpoint.replace(/^https?:\/\//,'');
-        core.setOutput('registry', registryId);
+        core.setOutput('registry', registryUri);
       }
 
       // Execute the docker login command
@@ -55,11 +57,18 @@ async function run() {
         core.debug(doLoginStdout);
         throw new Error('Could not login: ' + doLoginStderr);
       }
+
+      registryUriState.push(registryUri);
     }
   }
   catch (error) {
     core.setFailed(error.message);
   }
+
+  // Pass the logged-in registry URIs to the post action for logout
+  if (registryUriState.length) {
+    core.saveState('registries', registryUriState.join());
+  }
 }
 
 module.exports = run;

diff --git a/index.test.js b/index.test.js
@@ -1,4 +1,4 @@
-const run = require('.');
+const run = require('./index.js');
 const core = require('@actions/core');
 const exec = require('@actions/exec');
 
@@ -45,6 +45,8 @@ describe('Login to ECR', () => {
             'docker login',
             ['-u', 'hello', '-p', 'world', 'https://123456789012.dkr.ecr.aws-region-1.amazonaws.com'],
             expect.anything());
+        expect(core.saveState).toHaveBeenCalledTimes(1);
+        expect(core.saveState).toHaveBeenCalledWith('registries', '123456789012.dkr.ecr.aws-region-1.amazonaws.com');
     });
 
     test('gets auth token from ECR and logins the Docker client for each provided registry', async () => {
@@ -83,6 +85,8 @@ describe('Login to ECR', () => {
             'docker login',
             ['-u', 'foo', '-p', 'bar', 'https://111111111111.dkr.ecr.aws-region-1.amazonaws.com'],
             expect.anything());
+        expect(core.saveState).toHaveBeenCalledTimes(1);
+        expect(core.saveState).toHaveBeenCalledWith('registries', '123456789012.dkr.ecr.aws-region-1.amazonaws.com,111111111111.dkr.ecr.aws-region-1.amazonaws.com');
     });
 
     test('outputs the registry ID if a single registry is provided in the input', async () => {
@@ -114,6 +118,8 @@ describe('Login to ECR', () => {
             'docker login',
             ['-u', 'foo', '-p', 'bar', 'https://111111111111.dkr.ecr.aws-region-1.amazonaws.com'],
             expect.anything());
+        expect(core.saveState).toHaveBeenCalledTimes(1);
+        expect(core.saveState).toHaveBeenCalledWith('registries', '111111111111.dkr.ecr.aws-region-1.amazonaws.com');
     });
 
     test('error is caught by core.setFailed for failed docker login', async () => {
@@ -122,6 +128,52 @@ describe('Login to ECR', () => {
         await run();
 
         expect(core.setFailed).toBeCalled();
+        expect(core.setOutput).toHaveBeenCalledWith('registry', '123456789012.dkr.ecr.aws-region-1.amazonaws.com');
+        expect(core.saveState).toHaveBeenCalledTimes(0);
+    });
+
+    test('logged-in registries are saved as state even if the action fails', async () => {
+        exec.exec.mockReturnValue(1).mockReturnValueOnce(0);
+
+        core.getInput = jest.fn().mockReturnValueOnce('123456789012,111111111111');
+        mockEcrGetAuthToken.mockImplementation(() => {
+            return {
+                promise() {
+                    return Promise.resolve({
+                        authorizationData: [
+                            {
+                                authorizationToken: Buffer.from('hello:world').toString('base64'),
+                                proxyEndpoint: 'https://123456789012.dkr.ecr.aws-region-1.amazonaws.com'
+                            },
+                            {
+                                authorizationToken: Buffer.from('foo:bar').toString('base64'),
+                                proxyEndpoint: 'https://111111111111.dkr.ecr.aws-region-1.amazonaws.com'
+                            }
+                       ]
+                    });
+                }
+            };
+        });
+
+        await run();
+
+        expect(mockEcrGetAuthToken).toHaveBeenCalledWith({
+            registryIds: ['123456789012','111111111111']
+        });
+        expect(core.setOutput).toHaveBeenCalledTimes(0);
+        expect(exec.exec).toHaveBeenCalledTimes(2);
+        expect(exec.exec).toHaveBeenNthCalledWith(1,
+            'docker login',
+            ['-u', 'hello', '-p', 'world', 'https://123456789012.dkr.ecr.aws-region-1.amazonaws.com'],
+            expect.anything());
+        expect(exec.exec).toHaveBeenNthCalledWith(2,
+            'docker login',
+            ['-u', 'foo', '-p', 'bar', 'https://111111111111.dkr.ecr.aws-region-1.amazonaws.com'],
+            expect.anything());
+
+        expect(core.setFailed).toBeCalled();
+        expect(core.saveState).toHaveBeenCalledTimes(1);
+        expect(core.saveState).toHaveBeenCalledWith('registries', '123456789012.dkr.ecr.aws-region-1.amazonaws.com');
     });
 
     test('error is caught by core.setFailed for ECR call', async () => {
@@ -132,5 +184,7 @@ describe('Login to ECR', () => {
         await run();
 
         expect(core.setFailed).toBeCalled();
+        expect(core.setOutput).toHaveBeenCalledTimes(0);
+        expect(core.saveState).toHaveBeenCalledTimes(0);
     });
 });
diff --git a/package.json b/package.json
@@ -5,8 +5,8 @@
   "main": "index.js",
   "scripts": {
     "lint": "eslint **.js",
-    "package": "ncc build index.js -o dist",
-    "test": "eslint **.js && jest --coverage"
+    "package": "ncc build index.js -o dist && ncc build cleanup.js -o dist/cleanup",
+    "test": "eslint **.js && jest --coverage --verbose"
   },
   "repository": {
     "type": "git",