Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Include Environment Variables in Task Definitions with JSON Fragments #73

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

feat: Include Environment Variables in Task Definitions with JSON Fragments #73

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
7395878
task def merging
rojaswestall Nov 6, 2020
fb78f14
mergeWith dependency
rojaswestall Nov 6, 2020
0ab86bc
merge tests
rojaswestall Nov 6, 2020
973f34d
update action.yml
rojaswestall Nov 6, 2020
6b64c34
make image optional
rojaswestall Nov 6, 2020
2843105
update README
rojaswestall Nov 6, 2020
77ec5e7
update README
rojaswestall Nov 6, 2020
7a6b7e8
update README
rojaswestall Nov 6, 2020
07f05d6
mergeWith -> mergewith
rojaswestall Nov 6, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 92 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,98 @@ input of the second:
cluster: my-cluster
```

If containers in your task definition require different values depending on environment, you can specify a `merge` file that contains a JSON fragment to merge with the `task-definition`. `merge` task defintion JSON fragments can be used to modify any key/value pair in `task-definition`. If merging an array value, arrays from the `task-defition` and `merge` fragment will be concatenated.

`containerDefintions` and `name` within each container definition are required.

_task-def.json_

```json
{
"family": "task-def-family",
"containerDefinitions": [
{
"name": "web",
"image": "some-image"
}
]
}
```

_staging-vars.json_

```json
{
"containerDefinitions": [
{
"name": "web",
"environment": [
{
"name": "log_level",
"value": "debug"
}
]
}
]
}
```

_prod-vars.json_

```json
{
"containerDefinitions": [
{
"name": "web",
"environment": [
{
"name": "log_level",
"value": "info"
}
]
}
]
}
```

```yaml
- name: Add image to Amazon ECS task definition
id: render-image-in-task-def
uses: aws-actions/amazon-ecs-render-task-definition@v1
with:
task-definition: task-def.json
container-name: web
image: amazon/amazon-ecs-sample:latest

- name: Render Amazon ECS task definition for staging
id: render-staging-task-def
uses: aws-actions/amazon-ecs-render-task-definition@v1
with:
task-definition: ${{ steps.render-image-in-task-def.outputs.task-definition }}
merge: staging-vars.json

- name: Render Amazon ECS task definition for prod
id: render-prod-task-def
uses: aws-actions/amazon-ecs-render-task-definition@v1
with:
task-definition: ${{ steps.render-image-in-task-def.outputs.task-definition }}
merge: prod-vars.json

- name: Deploy to Staging
uses: aws-actions/amazon-ecs-deploy-task-definition@v1
with:
task-definition: ${{ steps.render-staging-task-def.outputs.task-definition }}
service: my-staging-service
cluster: my-staging-cluster

- name: Deploy to Prod
uses: aws-actions/amazon-ecs-deploy-task-definition@v1
with:
task-definition: ${{ steps.render-prod-task-def.outputs.task-definition }}
service: my-prod-service
cluster: my-prod-cluster
```

See [action.yml](action.yml) for the full documentation for this action's inputs and outputs.

## License Summary
Expand Down
5 changes: 4 additions & 1 deletion action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@ inputs:
required: true
image:
description: 'The URI of the container image to insert into the ECS task definition'
required: true
required: false
merge:
description: 'The path to a task definition JSON fragment file to merge with the task defintion'
required: false
outputs:
task-definition:
description: 'The path to the rendered task definition file'
Expand Down
50 changes: 47 additions & 3 deletions index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,26 @@ const path = require('path');
const core = require('@actions/core');
const tmp = require('tmp');
const fs = require('fs');
const mergeWith = require('lodash.mergewith');

// Customizer for lodash mergeWith
// allows arrays in the original task definition to contain
// values as opposed to only in the mergeFiles (otherwise
// they are overridden)
// https://lodash.com/docs/4.17.15#mergeWith
function customizer(objValue, srcValue) {
if (Array.isArray(objValue)) {
return objValue.concat(srcValue);
}
}

async function run() {
try {
// Get inputs
const taskDefinitionFile = core.getInput('task-definition', { required: true });
const containerName = core.getInput('container-name', { required: true });
const imageURI = core.getInput('image', { required: true });
const imageURI = core.getInput('image', { required: false });
const mergeFile = core.getInput('merge', { required: false });

// Parse the task definition
const taskDefPath = path.isAbsolute(taskDefinitionFile) ?
Expand All @@ -19,7 +32,7 @@ async function run() {
}
const taskDefContents = require(taskDefPath);

// Insert the image URI
// Get containerDef with name `containerName`
if (!Array.isArray(taskDefContents.containerDefinitions)) {
throw new Error('Invalid task definition format: containerDefinitions section is not present or is not an array');
}
Expand All @@ -29,7 +42,38 @@ async function run() {
if (!containerDef) {
throw new Error('Invalid task definition: Could not find container definition with matching name');
}
containerDef.image = imageURI;

// Check for imageURI
if(imageURI) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since neither input is strictly required now, can we add some debug statements to indicate which course(s) of action the workflow is taking, and throw an error if neither is provided? That way if I wanted to see what was happening I could see:

(image specified) -------> "inserting container image [IMAGE]"
(merge file specified) ---> "merging task definition fragment [FILE_NAME]"
(nothing specified) -----> ERROR: "No image or merge file specified. Please specify one or both to use this action."

// Insert the image URI
containerDef.image = imageURI;
}

// Check for mergeFile
if (mergeFile) {
// Parse the merge file
const mergeFilePath = path.isAbsolute(mergeFile) ?
mergeFile :
path.join(process.env.GITHUB_WORKSPACE, mergeFile);
if (!fs.existsSync(mergeFilePath)) {
throw new Error(`Merge file does not exist: ${mergeFile}`);
}
const mergeContents = require(mergeFilePath);

// Merge the merge file
if (!Array.isArray(mergeContents.containerDefinitions)) {
throw new Error('Invalid merge fragment definition: containerDefinitions section is not present or is not an array');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer we not require the presence of containerDefinitions, since there are top-level fields which can plausibly differ between environments (task-level memory/CPU, tags, task role, etc.). Is it possible to apply merge functionality across the entire task def vs. just this field?

}
const mergeDef = mergeContents.containerDefinitions.find(function(element) {
return element.name == containerName;
});
if (!mergeDef) {
throw new Error('Invalid merge fragment definition: Could not find container definition with matching name');
}

// mergeWith contents
mergeWith(containerDef, mergeDef, customizer);
}

// Write out a new task definition file
var updatedTaskDefFile = tmp.fileSync({
Expand Down
199 changes: 199 additions & 0 deletions index.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,177 @@ describe('Render task definition', () => {
expect(core.setOutput).toHaveBeenNthCalledWith(1, 'task-definition', 'new-task-def-file-name');
});

test('renders a task definition without an image specificied', async () => {
core.getInput = jest
.fn()
.mockReturnValueOnce('task-definition-no-image.json')
.mockReturnValueOnce('web')
.mockReturnValueOnce(undefined)
.mockReturnValueOnce('merge-no-image.json');

jest.mock('./task-definition-no-image.json', () => ({
family: 'task-def-family',
containerDefinitions: [
{
name: "web",
image: "nginx:latest"
}
]
}), { virtual: true });

jest.mock('./merge-no-image.json', () => ({
containerDefinitions: [
{
name: "web",
environment: [
{
name: "log_level",
value: "info"
}
]
}
]
}), {virtual: true});

await run();

expect(fs.writeFileSync).toHaveBeenNthCalledWith(1, 'new-task-def-file-name',
JSON.stringify({
family: 'task-def-family',
containerDefinitions: [
{
name: "web",
image: "nginx:latest",
environment: [
{
name: "log_level",
value: "info"
}
]
}
]
}, null, 2)
);
});

test('renders a task definition with a merge file', async () => {
core.getInput = jest
.fn()
.mockReturnValueOnce('task-definition.json') // task-definition
.mockReturnValueOnce('web') // container-name
.mockReturnValueOnce('nginx:latest') // image
.mockReturnValueOnce('merge.json'); // merge

jest.mock('./merge.json', () => ({
containerDefinitions: [
{
name: "web",
environment: [
{
name: "log_level",
value: "info"
}
]
}
]
}), {virtual: true});

await run();

expect(fs.writeFileSync).toHaveBeenNthCalledWith(1, 'new-task-def-file-name',
JSON.stringify({
family: 'task-def-family',
containerDefinitions: [
{
name: "web",
image: "nginx:latest",
environment: [
{
name: "log_level",
value: "info"
}
]
},
{
name: "sidecar",
image: "hello"
}
]
}, null, 2)
);
});

test('renders a task definition with a merge file to merge an array value', async () => {
core.getInput = jest
.fn()
.mockReturnValueOnce('task-definition.json') // task-definition
.mockReturnValueOnce('web') // container-name
.mockReturnValueOnce('nginx:latest') // image
.mockReturnValueOnce('merge-to-existing.json'); // merge

jest.mock('./merge-to-existing.json', () => ({
containerDefinitions: [
{
name: "web",
environment: [
{
name: "env",
value: "prod"
}
]
}
]
}), {virtual: true});

jest.mock('./task-definition.json', () => ({
family: 'task-def-family',
containerDefinitions: [
{
name: "web",
image: "some-other-image",
environment: [
{
name: "log_level",
value: "info"
}
]
},
{
name: "sidecar",
image: "hello"
}
]
}), { virtual: true });

await run();

expect(fs.writeFileSync).toHaveBeenNthCalledWith(1, 'new-task-def-file-name',
JSON.stringify({
family: 'task-def-family',
containerDefinitions: [
{
name: "web",
image: "nginx:latest",
environment: [
{
name: "log_level",
value: "info"
},
{
name: "env",
value: "prod"
}
]
},
{
name: "sidecar",
image: "hello"
}
]
}, null, 2)
);
});

test('error returned for missing task definition file', async () => {
fs.existsSync.mockReturnValue(false);
core.getInput = jest
Expand Down Expand Up @@ -173,4 +344,32 @@ describe('Render task definition', () => {

expect(core.setFailed).toBeCalledWith('Invalid task definition: Could not find container definition with matching name');
});

test('error returned for invalid merge file', async () => {
fs.existsSync.mockReturnValue(false);
core.getInput = jest
.fn()
.mockReturnValueOnce('task-definition.json')
.mockReturnValueOnce('web')
.mockReturnValueOnce('nginx:latest')
.mockReturnValueOnce('invalid-merge-file.json');

fs.existsSync.mockReturnValueOnce(JSON.stringify({
family: 'task-def-family',
containerDefinitions: [
{
name: "web",
image: "nginx:latest"
},
{
name: "sidecar",
image: "hello"
}
]
}));

await run();

expect(core.setFailed).toBeCalledWith('Merge file does not exist: invalid-merge-file.json');
});
});
Loading