-
Notifications
You must be signed in to change notification settings - Fork 249
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(aws-fargate-secretsmanager): Create new construct #670
Conversation
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One comment to resolve, but everything substantive is fine and you can start building
* Adds environment variables to the container with the ARN and Name of the Secrets Manager secret | ||
* Add permissions to the container IAM role allowing it to publish to the Secrets Manager secret | ||
|
||
### Amazon Secrets Manager Secret |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Compare these with aws-lambda-secretmanager
- This should mention random name and random value
- Explore whether the KMS comment adds value - I don't think we have to do anything with KMS when using secret manager. It's possible the right thing to do is remove that comment from the lambda-secretmanager README file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
KMS comment does not add value
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My comments from the original README review were not addressed (mention that the secret created will have a random name and random value)
source/patterns/@aws-solutions-constructs/aws-fargate-secretsmanager/lib/index.ts
Outdated
Show resolved
Hide resolved
source/patterns/@aws-solutions-constructs/aws-fargate-secretsmanager/lib/index.ts
Outdated
Show resolved
Hide resolved
...rns/@aws-solutions-constructs/aws-fargate-secretsmanager/test/fargate-secretsmanager.test.ts
Outdated
Show resolved
Hide resolved
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Issue #669 , if available:
Description of changes:
Create a construct that links a Fargate Service to AWS Secrets Manager
Should create a VPC or accept an existing VPC
Should create a Fargate Service or accept an existing service
Should create a new secret object or accept an existing secret
Should grant the service IAM permission to read/write from the secret
Should set up environment variables in the service that identify the secret in the construct code so the container can read/write from the secret
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
fixes #669