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

Jump to bottom

feat: support for setting environment secrets, and other task-def params #204

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

feat: support for setting environment secrets, and other task-def params #204

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
eebe842
feat: support for setting environment secrets, and other task-def params
pierre-wehbe Nov 5, 2022
3a7d129
feat: support for setting environment secrets, and other task-def par…
pierre-wehbe Nov 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,17 @@ To insert the image URI `amazon/amazon-ecs-sample:latest` as the image for the `
uses: aws-actions/amazon-ecs-render-task-definition@v1
with:
task-definition: task-definition.json
family: "core-service"
cpu: "1024"
memory: "2048"
executionRoleArn: "arn:aws:iam::xxxxxxxxxxxx:role/x"
taskRoleArn: "arn:aws:iam::xxxxxxxxxxxx:role/x"
container-name: web
awslogs-group: "ecs/web"
awslogs-region: "us-west-2"
image: amazon/amazon-ecs-sample:latest
environment-variables: "LOG_LEVEL=info"
environment-secrets: "SECRET_VAR=arn:aws:secretsmanager:us-west-x:xxxxxxxxxxxx:secret:prod/pkey"

- name: Deploy to Amazon ECS service
uses: aws-actions/amazon-ecs-deploy-task-definition@v1
Expand Down
24 changes: 24 additions & 0 deletions action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,30 @@ inputs:
environment-variables:
description: 'Variables to add to the container. Each variable is of the form KEY=value, you can specify multiple variables with multi-line YAML strings.'
required: false
environment-secrets:
Copy link

Choose a reason for hiding this comment

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

@joshmello can you think about how to add option to get secrets from AWS AppConfig? For example. we can set arn of config. It's will be very useful. Or just read env from file.
(asking because have a lot of env to setup)

Copy link

Choose a reason for hiding this comment

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

by "environment-secrets" you mean both value from SSM and from Secret Manager right ?

description: 'Secrets to add to the container. Each secret is of the form KEY=value, you can specify multiple variables with multi-line YAML strings.'
required: false
family:
description: 'task-def family'
required: false
cpu:
description: 'CPU'
required: false
memory:
description: 'Memory'
required: false
executionRoleArn:
description: 'executionRoleArn'
required: false
taskRoleArn:
description: 'taskRoleArn'
required: false
awslogs-group:
description: 'awslogs-group'
required: false
awslogs-region:
description: 'awslogs-region'
required: false
Comment on lines +23 to +42
Copy link

Choose a reason for hiding this comment

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

Question; is the expectation here that some of this changes between github action execution?

Until now, we haven't added these fields into the github action config because we supply a task def json and then we read the parameters that change and inline them into the task definition. But I'm wondering if there is a usecase that we've missed.

Copy link

Choose a reason for hiding this comment

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

Yeah some of these might not be needed like the logs, and task arns since you can just use the names. To be honest I think the biggest item needed is secret arns since they do contain the AWS Account Id and would prefer not to commit that in the repo.

outputs:
task-definition:
description: 'The path to the rendered task definition file'
Expand Down
73 changes: 72 additions & 1 deletion dist/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1190,10 +1190,20 @@ async function run() {
try {
// Get inputs
const taskDefinitionFile = core.getInput('task-definition', { required: true });
const containerName = core.getInput('container-name', { required: true });

const family = core.getInput('family', { required: false });
const cpu = core.getInput('cpu', { required: false });
const memory = core.getInput('memory', { required: false });
const executionRoleArn = core.getInput('executionRoleArn', { required: false });
const taskRoleArn = core.getInput('taskRoleArn', { required: false });

const containerName = core.getInput('container-name', { required: false });
const imageURI = core.getInput('image', { required: true });
const awslogsGroup = core.getInput('awslogs-group', { required: false });
const awslogsRegion = core.getInput('awslogs-region', { required: false });

const environmentVariables = core.getInput('environment-variables', { required: false });
const environmentSecrets = core.getInput('environment-secrets', { required: false });

// Parse the task definition
const taskDefPath = path.isAbsolute(taskDefinitionFile) ?
Expand All @@ -1216,6 +1226,26 @@ async function run() {
}
containerDef.image = imageURI;

if (family) {
taskDefContents.family = family;
}

if (cpu) {
taskDefContents.cpu = cpu;
}

if (memory) {
taskDefContents.memory = memory;
}

if (executionRoleArn) {
taskDefContents.executionRoleArn = executionRoleArn;
}

if (taskRoleArn) {
taskDefContents.taskRoleArn = taskRoleArn;
}

if (environmentVariables) {

// If environment array is missing, create it
Expand Down Expand Up @@ -1253,6 +1283,47 @@ async function run() {
})
}

if (environmentSecrets) {

// If environment array is missing, create it
if (!Array.isArray(containerDef.secrets)) {
containerDef.secrets = [];
}

// Get pairs by splitting on newlines
environmentSecrets.split('\n').forEach(function (line) {
// Trim whitespace
const trimmedLine = line.trim();
// Skip if empty
if (trimmedLine.length === 0) { return; }
// Split on =
const separatorIdx = trimmedLine.indexOf("=");
// If there's nowhere to split
if (separatorIdx === -1) {
throw new Error(`Cannot parse the environment secret '${trimmedLine}'. Environment secret pairs must be of the form KEY=value.`);
}
// Build object
const secret = {
name: trimmedLine.substring(0, separatorIdx),
valueFrom: trimmedLine.substring(separatorIdx + 1),
};

// Search container definition environment for one matching name
const variableDef = containerDef.secrets.find((e) => e.name == secret.name);
if (variableDef) {
// If found, update
variableDef.valueFrom = secret.valueFrom;
} else {
// Else, create
containerDef.secrets.push(secret);
}
})
}

if (awslogsGroup && awslogsRegion && containerDef.logConfiguration && containerDef.logConfiguration.options) {
containerDef.logConfiguration.options["awslogs-group"] = awslogsGroup;
containerDef.logConfiguration.options["awslogs-region"] = awslogsRegion;
}

// Write out a new task definition file
var updatedTaskDefFile = tmp.fileSync({
Expand Down
73 changes: 72 additions & 1 deletion index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,20 @@ async function run() {
try {
// Get inputs
const taskDefinitionFile = core.getInput('task-definition', { required: true });
const containerName = core.getInput('container-name', { required: true });

const family = core.getInput('family', { required: false });
const cpu = core.getInput('cpu', { required: false });
const memory = core.getInput('memory', { required: false });
const executionRoleArn = core.getInput('executionRoleArn', { required: false });
const taskRoleArn = core.getInput('taskRoleArn', { required: false });

const containerName = core.getInput('container-name', { required: false });
const imageURI = core.getInput('image', { required: true });
const awslogsGroup = core.getInput('awslogs-group', { required: false });
const awslogsRegion = core.getInput('awslogs-region', { required: false });

const environmentVariables = core.getInput('environment-variables', { required: false });
const environmentSecrets = core.getInput('environment-secrets', { required: false });

// Parse the task definition
const taskDefPath = path.isAbsolute(taskDefinitionFile) ?
Expand All @@ -33,6 +43,26 @@ async function run() {
}
containerDef.image = imageURI;

if (family) {
taskDefContents.family = family;
}

if (cpu) {
taskDefContents.cpu = cpu;
}

if (memory) {
taskDefContents.memory = memory;
}

if (executionRoleArn) {
taskDefContents.executionRoleArn = executionRoleArn;
}

if (taskRoleArn) {
taskDefContents.taskRoleArn = taskRoleArn;
}

if (environmentVariables) {

// If environment array is missing, create it
Expand Down Expand Up @@ -70,6 +100,47 @@ async function run() {
})
}

if (environmentSecrets) {

// If environment array is missing, create it
if (!Array.isArray(containerDef.secrets)) {
containerDef.secrets = [];
}

// Get pairs by splitting on newlines
environmentSecrets.split('\n').forEach(function (line) {
Copy link

Choose a reason for hiding this comment

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

Nit; this could be turned into a .map() that maps each environment secret line to type { name: string, valueFrom: string } and then we could merge these values into containerDef.secrets outside of the forEach loop.

// Trim whitespace
const trimmedLine = line.trim();
// Skip if empty
if (trimmedLine.length === 0) { return; }
// Split on =
const separatorIdx = trimmedLine.indexOf("=");
// If there's nowhere to split
if (separatorIdx === -1) {
throw new Error(`Cannot parse the environment secret '${trimmedLine}'. Environment secret pairs must be of the form KEY=value.`);
}
// Build object
const secret = {
name: trimmedLine.substring(0, separatorIdx),
valueFrom: trimmedLine.substring(separatorIdx + 1),
};

// Search container definition environment for one matching name
const variableDef = containerDef.secrets.find((e) => e.name == secret.name);
if (variableDef) {
// If found, update
variableDef.valueFrom = secret.valueFrom;
} else {
// Else, create
containerDef.secrets.push(secret);
}
})
}

if (awslogsGroup && awslogsRegion && containerDef.logConfiguration && containerDef.logConfiguration.options) {
Copy link

Choose a reason for hiding this comment

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

These options look specific to awslogs log driver. I believe we should check if the logConfiguration.logsDriver is set to awslogs before we change these properties in the task def.

containerDef.logConfiguration.options["awslogs-group"] = awslogsGroup;
containerDef.logConfiguration.options["awslogs-region"] = awslogsRegion;
Comment on lines +141 to +142
Copy link

Choose a reason for hiding this comment

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

Nit; It might also be worthwhile to also include the option to set awslogs-stream-prefix and awslogs-create-group as well because that'll ship the complete set of AWS logs options for customers.

}

// Write out a new task definition file
var updatedTaskDefFile = tmp.fileSync({
Expand Down
84 changes: 80 additions & 4 deletions index.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,17 @@ describe('Render task definition', () => {
core.getInput = jest
.fn()
.mockReturnValueOnce('task-definition.json') // task-definition
.mockReturnValueOnce('task-def-family-modified') // family
.mockReturnValueOnce('2048') // cpu
.mockReturnValueOnce('4096') // memory
.mockReturnValueOnce('arn:aws:iam::xxxxxxxxxxxx:role/new') // executionRoleArn
.mockReturnValueOnce('arn:aws:iam::xxxxxxxxxxxx:role/new') // taskRoleArn
.mockReturnValueOnce('web') // container-name
.mockReturnValueOnce('nginx:latest') // image
.mockReturnValueOnce('FOO=bar\nHELLO=world'); // environment-variables
.mockReturnValueOnce('/ecs/new') // awslogs-group
.mockReturnValueOnce('us-west-1') // awslogs-region
.mockReturnValueOnce('FOO=bar\nHELLO=world') // environment-variables
.mockReturnValueOnce('FOO=bar\nHELLO=world'); // environment-secrets

process.env = Object.assign(process.env, { GITHUB_WORKSPACE: __dirname });
process.env = Object.assign(process.env, { RUNNER_TEMP: '/home/runner/work/_temp' });
Expand All @@ -30,10 +38,23 @@ describe('Render task definition', () => {

jest.mock('./task-definition.json', () => ({
family: 'task-def-family',
cpu: "1024",
memory: "2048",
executionRoleArn: "arn:aws:iam::xxxxxxxxxxxx:role/old",
taskRoleArn: "arn:aws:iam::xxxxxxxxxxxx:role/old",
containerDefinitions: [
{
name: "web",
image: "some-other-image",
logConfiguration: {
logDriver: "awslogs",
options: {
"awslogs-group": "/ecs/old",
"awslogs-region": "us-west-2",
"awslogs-stream-prefix": "ecs",
"awslogs-create-group": "true"
}
},
environment: [
{
name: "FOO",
Expand All @@ -43,6 +64,16 @@ describe('Render task definition', () => {
name: "DONT-TOUCH",
value: "me"
}
],
secrets: [
{
name: "FOO",
valueFrom: "not bar"
},
{
name: "DONT-TOUCH",
valueFrom: "me"
}
]
},
{
Expand All @@ -64,11 +95,24 @@ describe('Render task definition', () => {
});
expect(fs.writeFileSync).toHaveBeenNthCalledWith(1, 'new-task-def-file-name',
JSON.stringify({
family: 'task-def-family',
family: 'task-def-family-modified',
cpu: "2048",
memory: "4096",
executionRoleArn: "arn:aws:iam::xxxxxxxxxxxx:role/new",
taskRoleArn: "arn:aws:iam::xxxxxxxxxxxx:role/new",
containerDefinitions: [
{
name: "web",
image: "nginx:latest",
logConfiguration: {
logDriver: "awslogs",
options: {
"awslogs-group": "/ecs/new",
"awslogs-region": "us-west-1",
"awslogs-stream-prefix": "ecs",
"awslogs-create-group": "true"
}
},
environment: [
{
name: "FOO",
Expand All @@ -82,6 +126,20 @@ describe('Render task definition', () => {
name: "HELLO",
value: "world"
}
],
secrets: [
{
name: "FOO",
valueFrom: "bar"
},
{
name: "DONT-TOUCH",
valueFrom: "me"
},
{
name: "HELLO",
valueFrom: "world"
}
]
},
{
Expand All @@ -98,9 +156,17 @@ describe('Render task definition', () => {
core.getInput = jest
.fn()
.mockReturnValueOnce('/hello/task-definition.json') // task-definition
.mockReturnValueOnce('task-def-family') // family
.mockReturnValueOnce('2048') // cpu
.mockReturnValueOnce('4096') // memory
.mockReturnValueOnce('arn:aws:iam::xxxxxxxxxxxx:role/new') // executionRoleArn
.mockReturnValueOnce('arn:aws:iam::xxxxxxxxxxxx:role/new') // taskRoleArn
.mockReturnValueOnce('web') // container-name
.mockReturnValueOnce('nginx:latest') // image
.mockReturnValueOnce('EXAMPLE=here'); // environment-variables
.mockReturnValueOnce('/ecs/new') // awslogs-group
.mockReturnValueOnce('us-west-1') // awslogs-region
.mockReturnValueOnce('EXAMPLE=here') // environment-variables
.mockReturnValueOnce('EXAMPLE=here'); // environment-secrets
jest.mock('/hello/task-definition.json', () => ({
family: 'task-def-family',
containerDefinitions: [
Expand Down Expand Up @@ -132,9 +198,19 @@ describe('Render task definition', () => {
name: "EXAMPLE",
value: "here"
}
],
secrets: [
{
name: "EXAMPLE",
valueFrom: "here"
}
]
}
]
],
cpu: "2048",
memory: "4096",
executionRoleArn: "arn:aws:iam::xxxxxxxxxxxx:role/new",
taskRoleArn: "arn:aws:iam::xxxxxxxxxxxx:role/new",
}, null, 2)
);
expect(core.setOutput).toHaveBeenNthCalledWith(1, 'task-definition', 'new-task-def-file-name');
Expand Down