-
Notifications
You must be signed in to change notification settings - Fork 4k
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): add support for ProxyConfiguration in ECS TaskDefinition #4007
Conversation
Thanks so much for taking the time to contribute to the AWS CDK ❤️ We will shortly assign someone to review this pull request and help get it
|
Continuous integration build failed |
c3e80f8
to
ca09e3d
Compare
4db79cb
to
1cc32e3
Compare
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 |
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 |
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.
Thanks for taking the time to contribute @iamhopaul123! I like the overall approach, maintaining consistency with other constructs within the ECS module. Had a couple comments, let me know if you have any questions
packages/@aws-cdk/aws-ecs/lib/proxy-configuration/app-mesh-proxy-configuration.ts
Show resolved
Hide resolved
packages/@aws-cdk/aws-ecs/lib/proxy-configuration/app-mesh-proxy-configuration.ts
Outdated
Show resolved
Hide resolved
packages/@aws-cdk/aws-ecs/lib/proxy-configuration/app-mesh-proxy-configuration.ts
Show resolved
Hide resolved
packages/@aws-cdk/aws-ecs/lib/proxy-configuration/app-mesh-proxy-configuration.ts
Outdated
Show resolved
Hide resolved
packages/@aws-cdk/aws-ecs/lib/proxy-configuration/generic-proxy-configuration.ts
Outdated
Show resolved
Hide resolved
packages/@aws-cdk/aws-ecs/lib/proxy-configuration/generic-proxy-configuration.ts
Outdated
Show resolved
Hide resolved
packages/@aws-cdk/aws-ecs/lib/proxy-configuration/proxy-configurations.ts
Outdated
Show resolved
Hide resolved
131fbd9
to
745d1ac
Compare
Pull request has been modified.
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.
Can you also add an integration test for the AppMeshProxyConfiguration?
packages/@aws-cdk/aws-ecs/lib/proxy-configuration/app-mesh-proxy-configuration.ts
Show resolved
Hide resolved
packages/@aws-cdk/aws-ecs/lib/proxy-configuration/app-mesh-proxy-configuration.ts
Show resolved
Hide resolved
packages/@aws-cdk/aws-ecs/lib/proxy-configuration/app-mesh-proxy-configuration.ts
Outdated
Show resolved
Hide resolved
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Pull request has been modified.
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 |
2f52df9
to
9305684
Compare
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 |
packages/@aws-cdk/aws-ecs/lib/proxy-configuration/app-mesh-proxy-configuration.ts
Outdated
Show resolved
Hide resolved
69b1f47
to
c94e940
Compare
configuration class
ee18897
to
a0083a3
Compare
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 |
packages/@aws-cdk/aws-ecs/lib/proxy-configuration/app-mesh-proxy-configuration.ts
Outdated
Show resolved
Hide resolved
a0083a3
to
de2196d
Compare
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.
Ship it! Thanks for taking the time to address the feedback!
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Thank you for contributing! Your pull request is now being automatically merged. |
Add proxyConfiguration support for ECS task definition.
Addresses #3977
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license