feat(ecs-patterns): allow passthrough of security groups to service

[clementallen]

Closes #8953

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

[iamhopaul123]

Looks great! Could you add manual test to double check if this feature works as we expect?

[clementallen]

Looks great! Could you add manual test to double check if this feature works as we expect?

Thanks @iamhopaul123 - I'm not quite sure what you mean by adding a manual test? I've tested it with my own CDK projects and it works as expected.

[iamhopaul123]

I've tested it with my own CDK projects and it works as expected.

That's exactly what I meant. Would you mind adding security groups option to ec2 pattern as well? Otherwise it might be a bit odd to see fargate supports passthrough of SGs but ec2 doesn't...

[clementallen]

@iamhopaul123

Would you mind adding security groups option to ec2 pattern as well

Sure can do, and to the other patterns too such as NLB Fargate, multiple target groups etc?

[iamhopaul123]

that'll be great! thank you so much!

[clementallen]

@iamhopaul123

I'm not hugely familiar with the non-fargate Ec2 patterns, but it seems the Ec2TaskDefinition needs a networkMode of AWS_VPC if a securityGroup is provided. Should I also allow that to be passed in from top level pattern?

[iamhopaul123]

yeah that's true. But it should be already handled by ec2service so I don't think we should overkill the issue. At least we can be sure that i'll error out before deploying to the CFN.

[clementallen]

@iamhopaul123 if networkMode isn't passed, it will error before deploying whenever a security group is provided, meaning there isn't much use providing one :)

Would it be okay if I raised a separate issue for the EC2 security groups and kept this MR focused on Fargate?

[iamhopaul123]

Sounds good to me!

[iamhopaul123]

🚢

[mergify]

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
• Commit ID: 6c60d7f
• Result: SUCCEEDED
• Build Logs (available for 30 days)

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

[mergify]

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).