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(batch): add secrets props to job definition #19506

Closed
wants to merge 10 commits into from

Conversation

yoshizawa56
Copy link
Contributor

closes #10976

Add a secrets property to batch.JobDefinitionContainer. This interface is almost the same as ecs.ContainerDefinitionOptions.

@gitpod-io
Copy link

gitpod-io bot commented Mar 22, 2022

@github-actions github-actions bot added the @aws-cdk/aws-batch Related to AWS Batch label Mar 22, 2022
@@ -112,6 +112,13 @@ export interface JobDefinitionContainer {
*/
readonly environment?: { [key: string]: string };

/**
* The environment variables from secrets manager or ssm parameter store
Copy link
Contributor

Choose a reason for hiding this comment

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

ecs.Secret will not support SSM parameter store though.

Don't you want core.SecretValue ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Well I guess not if you need the ARN. But in any case, the SSM comment here is inaccurate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To my understanding, ecs.Secret supports SSM parameter store through fromSsmParameter method.
Is it correct?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I was thinking of ssm.Secret.

@madeline-k madeline-k removed their assignment Mar 23, 2022
@pradoz
Copy link

pradoz commented Apr 7, 2022

@rix0rrr Anything else to be done here besides updating the comment?

rix0rrr
rix0rrr previously approved these changes Apr 13, 2022
@mergify mergify bot dismissed rix0rrr’s stale review April 13, 2022 12:52

Pull request has been modified.

@yoshizawa56
Copy link
Contributor Author

@rix0rrr What should I do next? This is my first OSS Contribution, please tell me.

@github-actions
Copy link

github-actions bot commented May 6, 2022

This PR has been in the MERGE CONFLICTS state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week.

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
  • Commit ID: 378f9fe
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@github-actions
Copy link

github-actions bot commented Jun 3, 2022

This PR has been in the BUILD FAILING state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week.

@github-actions
Copy link

This PR has been deemed to be abandoned, and will be automatically closed. Please create a new PR for these changes if you think this decision has been made in error.

@github-actions github-actions bot added the closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. label Jun 10, 2022
@github-actions github-actions bot closed this Jun 10, 2022
mergify bot pushed a commit that referenced this pull request Jul 5, 2022
Add a secrets property to batch.JobDefinitionContainer. This interface is almost the same as ecs.ContainerDefinitionOptions.

This is reopen PR of #19506

closes #10976

----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [x] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
daschaa pushed a commit to daschaa/aws-cdk that referenced this pull request Jul 9, 2022
Add a secrets property to batch.JobDefinitionContainer. This interface is almost the same as ecs.ContainerDefinitionOptions.

This is reopen PR of aws#19506

closes aws#10976

----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [x] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-batch Related to AWS Batch closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Batch] Add "Secrets" properties in interface JobDefinitionContainer
5 participants