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.

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(ecs): support secret environment variables #2994

Merged
merged 30 commits into from
Jul 29, 2019
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
146f13e
feat(ecs): support secret environment variables
jogold Jun 21, 2019
5384d55
Merge branch 'master' into ecs-secrets
jogold Jun 21, 2019
bf91871
Merge branch 'master' into ecs-secrets
jogold Jun 21, 2019
a0bb714
add IAM permissions
jogold Jun 24, 2019
2a5155b
Merge branch 'master' into ecs-secrets
jogold Jun 24, 2019
d697273
disable-awslint:ref-via-interface
jogold Jun 24, 2019
d1c31bf
Merge branch 'master' into ecs-secrets
jogold Jun 26, 2019
3c75145
Merge branch 'master' into ecs-secrets
Jul 1, 2019
a242dca
Merge branch 'master' into ecs-secrets
jogold Jul 3, 2019
67b0abe
Merge branch 'ecs-secrets' of github.com:jogold/aws-cdk into ecs-secrets
jogold Jul 3, 2019
f8d0efc
scheduled task
jogold Jul 3, 2019
f3cb37a
typo
jogold Jul 4, 2019
39909d7
typo
jogold Jul 4, 2019
43f8244
typo
jogold Jul 4, 2019
3a6a374
Merge branch 'master' into ecs-secrets
jogold Jul 22, 2019
f300b28
secrets and non breaking
jogold Jul 22, 2019
bcc350a
JSDoc
jogold Jul 22, 2019
17e0964
non breaking renderContainerDefinition
jogold Jul 22, 2019
d629929
renderKV
jogold Jul 22, 2019
80d4495
base props
jogold Jul 22, 2019
199de19
polymorphism
jogold Jul 22, 2019
037f9e4
base props
jogold Jul 22, 2019
6cca252
JSDoc for queue env vars
jogold Jul 23, 2019
aedbbb8
make base props internal
jogold Jul 23, 2019
410cfd5
remove note about scheduled fargate tasks
jogold Jul 23, 2019
5f5cf12
update README
jogold Jul 23, 2019
6298b4d
Merge branch 'master' into ecs-secrets
Jul 23, 2019
865d7f2
revert base props
jogold Jul 23, 2019
a2032a6
Merge branch 'ecs-secrets' of github.com:jogold/aws-cdk into ecs-secrets
jogold Jul 23, 2019
f27f8c5
Merge branch 'master' into ecs-secrets
jogold Jul 25, 2019
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
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,13 @@ export interface LoadBalancedServiceBaseProps {
*/
readonly environment?: { [key: string]: string };

/**
* Secret environment variables to pass to the container
*
* @default - No secret environment variables.
*/
readonly secrets?: { [key: string]: ecs.Secret };

/**
* Whether to create an AWS log driver
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,13 @@ export interface QueueProcessingServiceBaseProps {
*/
readonly environment?: { [key: string]: string };

/**
* Secret environment variables to pass to the container
*
* @default - No secret environment variables.
*/
readonly secrets?: { [key: string]: ecs.Secret };
jogold marked this conversation as resolved.
Show resolved Hide resolved

/**
* A queue for which to process items from.
*
Expand Down Expand Up @@ -89,18 +96,27 @@ export abstract class QueueProcessingServiceBase extends cdk.Construct {
* Environment variables that will include the queue name
*/
public readonly environment: { [key: string]: string };

/**
* Secret environment variables
*/
public readonly secrets?: { [key: string]: ecs.Secret };

/**
* The minimum number of tasks to run
*/
public readonly desiredCount: number;

/**
* The maximum number of instances for autoscaling to scale up to
*/
public readonly maxCapacity: number;

/**
* The scaling interval for autoscaling based off an SQS Queue size
*/
public readonly scalingSteps: autoscaling.ScalingInterval[];

/**
* The AwsLogDriver to use for logging if logging is enabled.
*/
Expand All @@ -122,6 +138,7 @@ export abstract class QueueProcessingServiceBase extends cdk.Construct {

// Add the queue name to environment variables
this.environment = { ...(props.environment || {}), QUEUE_NAME: this.sqsQueue.queueName };
this.secrets = props.secrets;

// Determine the desired task count (minimum) and maximum scaling capacity
this.desiredCount = props.desiredTaskCount || 1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,13 @@ export interface ScheduledTaskBaseProps {
* @default none
*/
readonly environment?: { [key: string]: string };

/**
* Secret environment variables to pass to the container
*
* @default - No secret environment variables.
*/
readonly secrets?: { [key: string]: ecs.Secret };
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ export class LoadBalancedEc2Service extends LoadBalancedServiceBase {
memoryLimitMiB: props.memoryLimitMiB,
memoryReservationMiB: props.memoryReservationMiB,
environment: props.environment,
secrets: props.secrets,
logging: this.logDriver,
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ export class QueueProcessingEc2Service extends QueueProcessingServiceBase {
cpu: props.cpu,
command: props.command,
environment: this.environment,
secrets: this.secrets,
logging: this.logDriver
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ export class ScheduledEc2Task extends ScheduledTaskBase {
cpu: props.cpu,
command: props.command,
environment: props.environment,
secrets: props.secrets,
logging: this.createAWSLogDriver(this.node.id)
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,8 @@ export class LoadBalancedFargateService extends LoadBalancedServiceBase {
const container = taskDefinition.addContainer(containerName, {
image: props.image,
logging: this.logDriver,
environment: props.environment
environment: props.environment,
secrets: props.secrets,
});

container.addPortMappings({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ export class QueueProcessingFargateService extends QueueProcessingServiceBase {
image: props.image,
command: props.command,
environment: this.environment,
secrets: this.secrets,
logging: this.logDriver
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ export class ScheduledFargateTask extends ScheduledTaskBase {
image: props.image,
command: props.command,
environment: props.environment,
secrets: props.secrets,
logging: this.createAWSLogDriver(this.node.id)
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -665,11 +665,7 @@
"Cpu": 1,
"Environment": [
{
"Name": "name",
"Value": "TRIGGER"
},
{
"Name": "value",
"Name": "TRIGGER",
"Value": "CloudWatch Events"
}
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class EventStack extends cdk.Stack {
desiredTaskCount: 2,
memoryLimitMiB: 512,
cpu: 1,
environment: { name: 'TRIGGER', value: 'CloudWatch Events' },
environment: { TRIGGER: 'CloudWatch Events' },
schedule: events.Schedule.rate(cdk.Duration.minutes(1)),
});
/// !hide
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ export = {
desiredTaskCount: 2,
memoryLimitMiB: 512,
cpu: 2,
environment: { name: 'TRIGGER', value: 'CloudWatch Events' },
environment: { TRIGGER: 'CloudWatch Events' },
schedule: events.Schedule.expression('rate(1 minute)')
});

Expand All @@ -111,11 +111,7 @@ export = {
Cpu: 2,
Environment: [
{
Name: "name",
Value: "TRIGGER"
},
{
Name: "value",
Name: "TRIGGER",
Value: "CloudWatch Events"
}
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -258,11 +258,7 @@
{
"Environment": [
{
"Name": "name",
"Value": "TRIGGER"
},
{
"Name": "value",
"Name": "TRIGGER",
"Value": "CloudWatch Events"
}
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class EventStack extends cdk.Stack {
desiredTaskCount: 2,
memoryLimitMiB: 512,
cpu: 256,
environment: { name: 'TRIGGER', value: 'CloudWatch Events' },
environment: { TRIGGER: 'CloudWatch Events' },
schedule: events.Schedule.rate(cdk.Duration.minutes(2)),
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ export = {
desiredTaskCount: 2,
memoryLimitMiB: 512,
cpu: 2,
environment: { name: 'TRIGGER', value: 'CloudWatch Events' },
environment: { TRIGGER: 'CloudWatch Events' },
schedule: events.Schedule.expression('rate(1 minute)')
});

Expand All @@ -103,11 +103,7 @@ export = {
{
Environment: [
{
Name: "name",
Value: "TRIGGER"
},
{
Name: "value",
Name: "TRIGGER",
Value: "CloudWatch Events"
}
],
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-ecs/lib/base/task-definition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ export class TaskDefinition extends TaskDefinitionBase {
});

const taskDef = new CfnTaskDefinition(this, 'Resource', {
containerDefinitions: Lazy.anyValue({ produce: () => this.containers.map(x => x.renderContainerDefinition()) }),
containerDefinitions: Lazy.anyValue({ produce: () => this.containers.map(x => x.renderContainerDefinition(this)) }),
volumes: Lazy.anyValue({ produce: () => this.volumes }),
executionRoleArn: Lazy.stringValue({ produce: () => this.executionRole && this.executionRole.roleArn }),
family: this.family,
Expand Down
69 changes: 67 additions & 2 deletions packages/@aws-cdk/aws-ecs/lib/container-definition.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import iam = require('@aws-cdk/aws-iam');
import secretsmanager = require('@aws-cdk/aws-secretsmanager');
import ssm = require('@aws-cdk/aws-ssm');
import cdk = require('@aws-cdk/core');
import { NetworkMode, TaskDefinition } from './base/task-definition';
import { ContainerImage, ContainerImageConfig } from './container-image';
Expand All @@ -7,6 +9,44 @@ import { LinuxParameters } from './linux-parameters';
import { LogDriver, LogDriverConfig } from './log-drivers/log-driver';

/**
* Properties for a Secret
*/
export interface SecretProps {
/**
* A SSM parameter that will be retrieved at container startup.
*/
readonly parameter?: ssm.IParameter;

/**
* An AWS Secrets Manager secret that will be retrieved at container startup.
*/
readonly secret?: secretsmanager.ISecret
}

/**
* A secret environment variable.
*/
export class Secret {
/**
* Creates an environment variable value from a parameter stored in AWS
* Systems Manager Parameter Store.
*/
public static fromSsmParameter(parameter: ssm.IParameter) {
return new Secret({ parameter });
}

/**
* Creates a environment variable value from a secret stored in AWS Secrets
* Manager.
*/
public static fromSecretsManager(secret: secretsmanager.ISecret) {
return new Secret({ secret });
}

constructor(public readonly props: SecretProps) {}
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a bit unnecessarily convoluted I think. Why not just accept an “arn” string here? This will remove the need for SecretProps and the special casing in renderContainerDefintion

Copy link
Contributor Author

@jogold jogold Jul 22, 2019

Choose a reason for hiding this comment

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

How do you suggest granting read (secret.grantRead() or parameter.grantRead()) to the task execution role in renderContainerDefinition? With a .fromArn() there? (there's no way to import from ARN currently in SSM, works for AWS Secrets)

Copy link
Contributor Author

@jogold jogold Jul 22, 2019

Choose a reason for hiding this comment

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

It is also possible to do taskDefintion.obtainExecutionRole().addToPolicy(...) but I find that duplicating SSM/Secrets read IAM permissions is not a elegant solution...

Copy link
Contributor

Choose a reason for hiding this comment

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

Something like that (polymorphism ;-)):

abstract class Secret {

  public static fromSecretsManager(secret: ISecret): Secret {
    return {
      arn: secret.secretArn,
      grantRead: grantee => secret.grantRead(grantee)
    };
  }
  
  public abstract readonly arn: string;
  public abstract grantRead(grantee: IGrantee): Grant;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice

}

/*
* The options for creating a container definition.
*/
export interface ContainerDefinitionOptions {
Expand Down Expand Up @@ -89,6 +129,13 @@ export interface ContainerDefinitionOptions {
*/
readonly environment?: { [key: string]: string };

/**
* The secret environment variables to pass to the container.
*
* @default - No secret environment variables.
*/
readonly secrets?: { [key: string]: Secret };

/**
* Specifies whether the container is marked essential.
*
Expand Down Expand Up @@ -412,8 +459,25 @@ export class ContainerDefinition extends cdk.Construct {

/**
* Render this container definition to a CloudFormation object
*
* @param taskDefinition [disable-awslint:ref-via-interface] (made optional to avoid breaking change)
*/
public renderContainerDefinition(): CfnTaskDefinition.ContainerDefinitionProperty {
public renderContainerDefinition(taskDefinition?: TaskDefinition): CfnTaskDefinition.ContainerDefinitionProperty {
const secrets = [];
for (const [k, v] of Object.entries(this.props.secrets || {})) {
if (v.props.parameter) {
secrets.push({ name: k, valueFrom: v.props.parameter.parameterArn });
if (taskDefinition) {
v.props.parameter.grantRead(taskDefinition.obtainExecutionRole());
}
} else if (v.props.secret) {
secrets.push({ name: k, valueFrom: v.props.secret.secretArn });
if (taskDefinition) {
v.props.secret.grantRead(taskDefinition.obtainExecutionRole());
}
}
}

return {
command: this.props.command,
cpu: this.props.cpu,
Expand All @@ -439,7 +503,8 @@ export class ContainerDefinition extends cdk.Construct {
volumesFrom: this.volumesFrom.map(renderVolumeFrom),
workingDirectory: this.props.workingDirectory,
logConfiguration: this.logDriverConfig,
environment: this.props.environment && renderKV(this.props.environment, 'name', 'value'),
environment: this.props.environment && renderKV(this.props.environment, 'name', 'value'),
secrets: secrets.length !== 0 ? secrets : undefined,
extraHosts: this.props.extraHosts && renderKV(this.props.extraHosts, 'hostname', 'ipAddress'),
healthCheck: this.props.healthCheck && renderHealthCheck(this.props.healthCheck),
links: this.links,
Expand Down
Loading