From 6faf6564679f225b29801ea5cc180720ff5f84e4 Mon Sep 17 00:00:00 2001 From: John Seekins Date: Wed, 11 Oct 2023 16:44:34 -0600 Subject: [PATCH 1/2] support dockerLabels Signed-off-by: John Seekins --- README.md | 3 ++ action.yml | 3 ++ dist/index.js | 19 +++++++++ index.js | 19 +++++++++ index.test.js | 111 ++++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 155 insertions(+) diff --git a/README.md b/README.md index c251d044..d16605ac 100644 --- a/README.md +++ b/README.md @@ -50,6 +50,9 @@ input of the second: environment-variables: | LOG_LEVEL=info ENVIRONMENT=prod + docker-labels: | + SERVICE=service + VERSION=version - name: Modify Amazon ECS task definition with second container id: render-app-container diff --git a/action.yml b/action.yml index ff4b9ce6..f6133706 100644 --- a/action.yml +++ b/action.yml @@ -22,6 +22,9 @@ inputs: log-configuration-options: description: "Create/Override options inside logConfiguration. Each variable is of the form key=value, you can specify multiple variables with multi-line YAML strings." required: false + docker-labels: + description: "Create/Override options inside dockerLabels. Each variable is key=value, you can specify multiple variables with multi-line YAML." + required: false outputs: task-definition: description: 'The path to the rendered task definition file' diff --git a/dist/index.js b/dist/index.js index 78011330..c50da19d 100644 --- a/dist/index.js +++ b/dist/index.js @@ -20,6 +20,7 @@ async function run() { const logConfigurationLogDriver = core.getInput("log-configuration-log-driver", { required: false }); const logConfigurationOptions = core.getInput("log-configuration-options", { required: false }); + const dockerLabels = core.getInput('docker-labels', { required: false }); // Parse the task definition const taskDefPath = path.isAbsolute(taskDefinitionFile) ? @@ -103,6 +104,24 @@ async function run() { }) } + if (dockerLabels) { + // If dockerLabels object is missing, create it + if (!containerDef.dockerLabels) { containerDef.dockerLabels = {} } + + // Get pairs by splitting on newlines + dockerLabels.split('\n').forEach(function (label) { + // Trim whitespace + label = label.trim(); + if (label && label.length) { + if (label.indexOf("=") == -1 ) { + throw new Error(`Can't parse logConfiguration option ${label}. Must be in key=value format, one per line`); + } + const [key, value] = label.split("="); + containerDef.dockerLabels[key] = value; + } + }) + } + // Write out a new task definition file var updatedTaskDefFile = tmp.fileSync({ tmpdir: process.env.RUNNER_TEMP, diff --git a/index.js b/index.js index 8be46604..a957841c 100644 --- a/index.js +++ b/index.js @@ -14,6 +14,7 @@ async function run() { const logConfigurationLogDriver = core.getInput("log-configuration-log-driver", { required: false }); const logConfigurationOptions = core.getInput("log-configuration-options", { required: false }); + const dockerLabels = core.getInput('docker-labels', { required: false }); // Parse the task definition const taskDefPath = path.isAbsolute(taskDefinitionFile) ? @@ -97,6 +98,24 @@ async function run() { }) } + if (dockerLabels) { + // If dockerLabels object is missing, create it + if (!containerDef.dockerLabels) { containerDef.dockerLabels = {} } + + // Get pairs by splitting on newlines + dockerLabels.split('\n').forEach(function (label) { + // Trim whitespace + label = label.trim(); + if (label && label.length) { + if (label.indexOf("=") == -1 ) { + throw new Error(`Can't parse logConfiguration option ${label}. Must be in key=value format, one per line`); + } + const [key, value] = label.split("="); + containerDef.dockerLabels[key] = value; + } + }) + } + // Write out a new task definition file var updatedTaskDefFile = tmp.fileSync({ tmpdir: process.env.RUNNER_TEMP, diff --git a/index.test.js b/index.test.js index e02fade2..8a0448e8 100644 --- a/index.test.js +++ b/index.test.js @@ -224,6 +224,117 @@ describe('Render task definition', () => { expect(core.setFailed).toBeCalledWith('Task definition file does not exist: does-not-exist-task-definition.json'); }); + test('renders a task definition at an absolute path, and with initial docker labels empty', async () => { + core.getInput = jest + .fn() + .mockReturnValueOnce('/hello/task-definition.json') // task-definition + .mockReturnValueOnce('web') // container-name + .mockReturnValueOnce('nginx:latest') // image + .mockReturnValueOnce('EXAMPLE=here') // environment-variables + .mockReturnValueOnce('key1=value1\nkey2=value2'); // docker-labels + + jest.mock('/hello/task-definition.json', () => ({ + family: 'task-def-family', + containerDefinitions: [ + { + name: "web", + image: "some-other-image" + } + ] + }), { virtual: true }); + + await run(); + + expect(tmp.fileSync).toHaveBeenNthCalledWith(1, { + tmpdir: '/home/runner/work/_temp', + prefix: 'task-definition-', + postfix: '.json', + keep: true, + discardDescriptor: true + }); + + expect(fs.writeFileSync).toHaveBeenNthCalledWith(1, 'new-task-def-file-name', + JSON.stringify({ + family: 'task-def-family', + containerDefinitions: [ + { + name: "web", + image: "nginx:latest", + environment: [ + { + name: "EXAMPLE", + value: "here" + } + ], + dockerLabels : { + "key1":"value1", + "key2":"value2" + } + } + ] + }, null, 2) + ); + expect(core.setOutput).toHaveBeenNthCalledWith(1, 'task-definition', 'new-task-def-file-name'); + }); + + test('renders a task definition at an absolute path, and change existed docker labels empty', async () => { + core.getInput = jest + .fn() + .mockReturnValueOnce('/hello/task-definition.json') // task-definition + .mockReturnValueOnce('web') // container-name + .mockReturnValueOnce('nginx:latest') // image + .mockReturnValueOnce('EXAMPLE=here') // environment-variables + .mockReturnValueOnce('key1=update_value1\nkey2=update_value2\nkey3=value3'); // docker-labels + + jest.mock('/hello/task-definition.json', () => ({ + family: 'task-def-family', + containerDefinitions: [ + { + name: "web", + image: "some-other-image", + dockerLabels : { + "key1":"value1", + "key2":"value2" + } + } + ] + }), { virtual: true }); + + await run(); + + expect(tmp.fileSync).toHaveBeenNthCalledWith(1, { + tmpdir: '/home/runner/work/_temp', + prefix: 'task-definition-', + postfix: '.json', + keep: true, + discardDescriptor: true + }); + + expect(fs.writeFileSync).toHaveBeenNthCalledWith(1, 'new-task-def-file-name', + JSON.stringify({ + family: 'task-def-family', + containerDefinitions: [ + { + name: "web", + image: "nginx:latest", + environment: [ + { + name: "EXAMPLE", + value: "here" + } + ], + dockerLabels : { + "key1":"update_value1", + "key2":"update_value2", + "key3":"value3" + } + } + ] + }, null, 2) + ); + expect(core.setOutput).toHaveBeenNthCalledWith(1, 'task-definition', 'new-task-def-file-name'); + }); + test('error returned for non-JSON task definition contents', async () => { jest.mock('./non-json-task-definition.json', () => ("hello"), { virtual: true }); From 27cdedf2a8f402c463a973415920ab7284a5c673 Mon Sep 17 00:00:00 2001 From: John Seekins Date: Fri, 13 Oct 2023 08:31:23 -0600 Subject: [PATCH 2/2] fix broken tests and slightly increase test coverage Signed-off-by: John Seekins --- index.test.js | 98 ++++++++++++++++++++++----------------------------- 1 file changed, 43 insertions(+), 55 deletions(-) diff --git a/index.test.js b/index.test.js index 8a0448e8..eb15b112 100644 --- a/index.test.js +++ b/index.test.js @@ -224,24 +224,16 @@ describe('Render task definition', () => { expect(core.setFailed).toBeCalledWith('Task definition file does not exist: does-not-exist-task-definition.json'); }); - test('renders a task definition at an absolute path, and with initial docker labels empty', async () => { + test('renders a task definition with docker labels', async () => { core.getInput = jest .fn() - .mockReturnValueOnce('/hello/task-definition.json') // task-definition - .mockReturnValueOnce('web') // container-name - .mockReturnValueOnce('nginx:latest') // image - .mockReturnValueOnce('EXAMPLE=here') // environment-variables - .mockReturnValueOnce('key1=value1\nkey2=value2'); // docker-labels - - jest.mock('/hello/task-definition.json', () => ({ - family: 'task-def-family', - containerDefinitions: [ - { - name: "web", - image: "some-other-image" - } - ] - }), { virtual: true }); + .mockReturnValueOnce('task-definition.json') + .mockReturnValueOnce('web') + .mockReturnValueOnce('nginx:latest') + .mockReturnValueOnce('EXAMPLE=here') + .mockReturnValueOnce('awslogs') + .mockReturnValueOnce('awslogs-create-group=true\nawslogs-group=/ecs/web\nawslogs-region=us-east-1\nawslogs-stream-prefix=ecs') + .mockReturnValueOnce('key1=value1\nkey2=value2'); await run(); @@ -251,7 +243,7 @@ describe('Render task definition', () => { postfix: '.json', keep: true, discardDescriptor: true - }); + }); expect(fs.writeFileSync).toHaveBeenNthCalledWith(1, 'new-task-def-file-name', JSON.stringify({ @@ -261,30 +253,56 @@ describe('Render task definition', () => { name: "web", image: "nginx:latest", environment: [ + { + name: "FOO", + value: "bar" + }, + { + name: "DONT-TOUCH", + value: "me" + }, + { + name: "HELLO", + value: "world" + }, { name: "EXAMPLE", value: "here" } ], + logConfiguration: { + logDriver: "awslogs", + options: { + "awslogs-create-group": "true", + "awslogs-group": "/ecs/web", + "awslogs-region": "us-east-1", + "awslogs-stream-prefix": "ecs" + } + }, dockerLabels : { "key1":"value1", "key2":"value2" } + }, + { + name: "sidecar", + image: "hello" } ] }, null, 2) ); - expect(core.setOutput).toHaveBeenNthCalledWith(1, 'task-definition', 'new-task-def-file-name'); }); - test('renders a task definition at an absolute path, and change existed docker labels empty', async () => { + test('renders a task definition at an absolute path with bad docker labels', async () => { core.getInput = jest .fn() - .mockReturnValueOnce('/hello/task-definition.json') // task-definition - .mockReturnValueOnce('web') // container-name - .mockReturnValueOnce('nginx:latest') // image - .mockReturnValueOnce('EXAMPLE=here') // environment-variables - .mockReturnValueOnce('key1=update_value1\nkey2=update_value2\nkey3=value3'); // docker-labels + .mockReturnValueOnce('/hello/task-definition.json') + .mockReturnValueOnce('web') + .mockReturnValueOnce('nginx:latest') + .mockReturnValueOnce('EXAMPLE=here') + .mockReturnValueOnce('awslogs') + .mockReturnValueOnce('awslogs-create-group=true\nawslogs-group=/ecs/web\nawslogs-region=us-east-1\nawslogs-stream-prefix=ecs') + .mockReturnValueOnce('key1=update_value1\nkey2\nkey3=value3'); jest.mock('/hello/task-definition.json', () => ({ family: 'task-def-family', @@ -302,37 +320,7 @@ describe('Render task definition', () => { await run(); - expect(tmp.fileSync).toHaveBeenNthCalledWith(1, { - tmpdir: '/home/runner/work/_temp', - prefix: 'task-definition-', - postfix: '.json', - keep: true, - discardDescriptor: true - }); - - expect(fs.writeFileSync).toHaveBeenNthCalledWith(1, 'new-task-def-file-name', - JSON.stringify({ - family: 'task-def-family', - containerDefinitions: [ - { - name: "web", - image: "nginx:latest", - environment: [ - { - name: "EXAMPLE", - value: "here" - } - ], - dockerLabels : { - "key1":"update_value1", - "key2":"update_value2", - "key3":"value3" - } - } - ] - }, null, 2) - ); - expect(core.setOutput).toHaveBeenNthCalledWith(1, 'task-definition', 'new-task-def-file-name'); + expect(core.setFailed).toBeCalledWith('Can\'t parse logConfiguration option key2. Must be in key=value format, one per line'); }); test('error returned for non-JSON task definition contents', async () => {