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(ecs-patterns): pass securityGroups to ScheduledFargateTask #14220

Closed
wants to merge 4 commits into from

Conversation

crazymykl
Copy link

@crazymykl crazymykl commented Apr 16, 2021

closes #5213

This is a minimal approach.


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

@gitpod-io
Copy link

gitpod-io bot commented Apr 16, 2021

@mergify
Copy link
Contributor

mergify bot commented Apr 16, 2021

Title does not follow the guidelines of Conventional Commits. Please adjust title before merge.

@crazymykl crazymykl changed the title feature(aws-ecs-patterns): pass securityGroups to ScheduledFargateTask feat: pass securityGroups to ScheduledFargateTask Apr 16, 2021
@gitpod-io
Copy link

gitpod-io bot commented May 6, 2021

@SoManyHs SoManyHs self-assigned this May 13, 2021
@SoManyHs SoManyHs added the @aws-cdk/aws-ecs-patterns Related to ecs-patterns library label May 13, 2021
@SoManyHs SoManyHs changed the title feat: pass securityGroups to ScheduledFargateTask feat(ecs-patterns): pass securityGroups to ScheduledFargateTask May 13, 2021
@fongie
Copy link

fongie commented May 26, 2021

just struggled with this in my project: this would be a nice feature

@RajarsiGit
Copy link

just struggled with this in my project: this would be a nice feature

Hi, @fongie can you share you code please, wherein you have overcome this issue using some way? Thanks

Copy link

@RajarsiGit RajarsiGit left a comment

Choose a reason for hiding this comment

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

Looks good to me

@fongie
Copy link

fongie commented May 27, 2021

just struggled with this in my project: this would be a nice feature

Hi, @fongie can you share you code please, wherein you have overcome this issue using some way? Thanks

Hi! Sure, but I warn you it's not pretty and there might be a better way too :)

  private addSecurityGroupToScheduledTask(group: SecurityGroup, task: ScheduledFargateTask) {
    const rule = task.eventRule as any as CfnRule
    const targets = rule.targets as any as RuleTargetConfig[]
    const config = targets[0].ecsParameters!.networkConfiguration as any
    const groups = config.awsVpcConfiguration.securityGroups as Array<string>
    groups.push(group.securityGroupId)
  }

This adds the security group you pass in to the method to the security groups of the task. Then, you can configure connections via the security group (like allowfrom, to, etc). Note that it also keeps the default security group.

Copy link
Contributor

@SoManyHs SoManyHs left a comment

Choose a reason for hiding this comment

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

This is great! Do you think you could also add this to the ScheduledEc2Task as well?

@SoManyHs SoManyHs added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Jun 9, 2021
@SoManyHs
Copy link
Contributor

I went ahead and added the change to support Ec2 patterns on top of your work here: #15096

…te-task.ts

Co-authored-by: Hsing-Hui Hsu <hsinghui@amazon.com>
@crazymykl
Copy link
Author

Nice. I was having some trouble figuring out how to unit test that case.

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

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

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

@SoManyHs
Copy link
Contributor

Just curious, is it critical that you can reference the securityGroup as an output property on the ScheduledFargateTask after it is created? I.e. https://github.com/aws/aws-cdk/pull/14220/files#diff-7ebbca721fecf8f6c66398af515f1d264cbdf3116d34ef912aab19a55e06994aR39-R43?

mergify bot pushed a commit that referenced this pull request Jun 15, 2021
… pattern (#15096)

Closes #5213, #14220

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@SoManyHs
Copy link
Contributor

Resolved by #15096

@SoManyHs SoManyHs closed this Jun 15, 2021
hollanddd pushed a commit to hollanddd/aws-cdk that referenced this pull request Aug 26, 2021
… pattern (aws#15096)

Closes aws#5213, aws#14220

----

*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-ecs-patterns Related to ecs-patterns library response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

(ecs-patterns): use existing Security Group with ScheduledFargateTask
5 participants