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

fix(stepfunctions-tasks): specify tags in BatchSubmitJob properties #26349

Merged
merged 4 commits into from
Jul 25, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -758,7 +758,7 @@
"JobQueueArn"
]
},
"\",\"Parameters\":{\"foo.$\":\"$.bar\"},\"ContainerOverrides\":{\"Environment\":[{\"Name\":\"key\",\"Value\":\"value\"}],\"ResourceRequirements\":[{\"Type\":\"MEMORY\",\"Value\":\"256\"},{\"Type\":\"VCPU\",\"Value\":\"1\"}]},\"RetryStrategy\":{\"Attempts\":3},\"Timeout\":{\"AttemptDurationSeconds\":60}}}}}"
"\",\"Parameters\":{\"foo.$\":\"$.bar\"},\"ContainerOverrides\":{\"Environment\":[{\"Name\":\"key\",\"Value\":\"value\"}],\"ResourceRequirements\":[{\"Type\":\"MEMORY\",\"Value\":\"256\"},{\"Type\":\"VCPU\",\"Value\":\"1\"}]},\"RetryStrategy\":{\"Attempts\":3},\"Tags\":{\"key\":\"value\"},\"Timeout\":{\"AttemptDurationSeconds\":60}}}}}"
]
]
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ class RunBatchStack extends cdk.Stack {
}),
attempts: 3,
taskTimeout: sfn.Timeout.duration(cdk.Duration.seconds(60)),
tags: {
key: 'value',
},
});

const definition = new sfn.Pass(this, 'Start', {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -361,3 +361,89 @@ test('Task throws if attempt duration is less than 60 sec', () => {
/attempt duration must be greater than 60 seconds./,
);
});

test('supports passing tags', () => {
// WHEN
const task = new BatchSubmitJob(stack, 'Task', {
jobDefinitionArn: batchJobDefinition.jobDefinitionArn,
jobQueueArn: batchJobQueue.jobQueueArn,
jobName: sfn.JsonPath.stringAt('$.jobName'),
tags: {
test: 'this is a tag',
},
});

// THEN
expect(stack.resolve(task.toStateJson())).toEqual({
Type: 'Task',
Resource: {
'Fn::Join': [
'',
[
'arn:',
{
Ref: 'AWS::Partition',
},
':states:::batch:submitJob.sync',
],
],
},
End: true,
Parameters: {
'JobDefinition': { Ref: 'JobDefinition24FFE3ED' },
'JobName.$': '$.jobName',
'JobQueue': {
'Fn::GetAtt': [
'JobQueueEE3AD499',
'JobQueueArn',
],
},
'Tags': {
test: 'this is a tag',
},
},
});
});

test('throws if tags has invalid value', () => {
expect(() => {
const tags: { [key: string]: string } = {};
for (let i = 0; i < 100; i++) {
tags[i] = 'tag';
}
new BatchSubmitJob(stack, 'Task1', {
jobDefinitionArn: batchJobDefinition.jobDefinitionArn,
jobName: 'JobName',
jobQueueArn: batchJobQueue.jobQueueArn,
tags,
});
}).toThrow(
/Maximum tag number of entries is 50./,
);

expect(() => {
const keyTooLong = 'k'.repeat(150);
const tags: { [key: string]: string } = {};
tags[keyTooLong] = 'tag';
new BatchSubmitJob(stack, 'Task2', {
jobDefinitionArn: batchJobDefinition.jobDefinitionArn,
jobName: 'JobName',
jobQueueArn: batchJobQueue.jobQueueArn,
tags,
});
}).toThrow(
/Tag key size must be between 1 and 128/,
);

expect(() => {
const tags = { key: 'k'.repeat(300) };
new BatchSubmitJob(stack, 'Task3', {
jobDefinitionArn: batchJobDefinition.jobDefinitionArn,
jobName: 'JobName',
jobQueueArn: batchJobQueue.jobQueueArn,
tags,
});
}).toThrow(
/Tag value maximum size is 256/,
);
});
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,13 @@ export interface BatchSubmitJobProps extends sfn.TaskStateBaseProps {
* @default 1
*/
readonly attempts?: number;

/**
* The tags applied to the job request.
*
* @default {} - no tags
*/
readonly tags?: { [key: string]: string };
}

/**
Expand Down Expand Up @@ -212,6 +219,8 @@ export class BatchSubmitJob extends sfn.TaskStateBase {
});
}

this.validateTags(props.tags);

this.taskPolicies = this.configurePolicyStatements();
}

Expand Down Expand Up @@ -255,7 +264,7 @@ export class BatchSubmitJob extends sfn.TaskStateBase {
this.props.attempts !== undefined
? { Attempts: this.props.attempts }
: undefined,

Tags: this.props.tags,
Timeout: timeout
? { AttemptDurationSeconds: timeout }
: undefined,
Expand Down Expand Up @@ -337,4 +346,24 @@ export class BatchSubmitJob extends sfn.TaskStateBase {
ResourceRequirements: resources.length ? resources : undefined,
};
}

/**
* Validate the specified tags according to
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the function is very well written and self explanatory. Do we really need the docstring? What happens if the URL will change in the future?

* https://docs.aws.amazon.com/batch/latest/APIReference/API_SubmitJob.html
*/
private validateTags(tags?: { [key: string]: string }) {
if (tags === undefined) return;
const tagEntries = Object.entries(tags);
if (tagEntries.length > 50) {
throw new Error(`Maximum tag number of entries is 50. Received ${tagEntries.length}.`);
}
for (const [key, value] of tagEntries) {
if (key.length < 1 || key.length > 128) {
throw new Error(`Tag key size must be between 1 and 128, but got ${key.length}.`);
}
if (value.length > 256) {
throw new Error(`Tag value maximum size is 256, but got ${value.length}.`);
}
}
}
}