feat: support secrets in render task definition #152
Conversation
|
👍 we'd love to use this feature. What's blocking? |
|
Hi 👋 Any updates about when this will be merged? I think it's a needed feature 🙏 |
taken from aws-actions#152 Signed-off-by: Florian Greinus <[email protected]>
|
Hi there, any update on this? |
|
Hi @yehudacohen, thank you so much for your contribution. Apologies on the delay.
|
|
@yehudacohen great work on this! Looks like it is approved just need a quick fix for merge conflicts. Can't wait to use this 🙌 |
|
Any updates on this? Cheers! |
| @@ -68,9 +68,48 @@ async function run() { | |||
| containerDef.environment.push(variable); | |||
| } | |||
| }) | |||
| if (secrets) { | |||
| // If environment array is missing, create it | |||
There was a problem hiding this comment.
Comment Needs Update, from environment array to secrets array
| 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 secret '${trimmedLine}'. Secret pairs must be of the form NAME=valueFrom, | ||
| where valueFrom is an arn from parameter store or secrets manager. See AWS documentation for more information: | ||
| https://docs.aws.amazon.com/AmazonECS/latest/developerguide/specifying-sensitive-data.html.`); | ||
| } |
There was a problem hiding this comment.
Ideally, this (Lines 80 to Lines 91, I think) could be transferred to a common method as we are using same set of code when fetching environmentVariables as well.
| } | ||
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
nit: unnecessary whitespace
| @@ -94,13 +121,14 @@ describe('Render task definition', () => { | |||
| expect(core.setOutput).toHaveBeenNthCalledWith(1, 'task-definition', 'new-task-def-file-name'); | |||
| }); | |||
|
|
|||
| test('renders a task definition at an absolute path, and with initial environment empty', async () => { | |||
| test('renders a task definition at an absolute path, and with initial environment and secrets empty', async () => { | |||
There was a problem hiding this comment.
nit: I might have understood this wrong and I see that the naming was always this way but the test name here is not accurate as the secrets and environment provided is not empty?
|
waiting for this feature |
amazreech
changed the base branch from
master
to
supports-secrets-in-render-task-definition
amazreech
changed the base branch from
supports-secrets-in-render-task-definition
to
master
amazreech
changed the base branch from
master
to
supports-secrets-in-render-task-definition
Issue #, if available:
#20
Description of changes:
Add ability to add secrets to task definition. There is already a pull request here #143 which tries to do this, but it isn't backward compatible. This pull request is. It also doesn't add tests which this PR does.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.