addSecurityGroup didn't work for ECS_pattern ALB Fargate

[ptaylor10]

I was not able to add another security group to the ECS Pattern for ALB Fargate service using Fargate.cluster.connections.addSecurityGroup() method.

Reproduction Steps

ecs-fargate.ts

export class ECSFargateStack extends cdk.Stack {
  constructor(scope: cdk.Construct, id: string, props?: cdk.StackProps) {
    super(scope, id, props);

    // Imports the VPC from the name provided
    const vpc = ec2.Vpc.fromLookup(this, this.stackName+'VPC', {
      vpcName: vpc_name
    })

    const laravelAppSecret = new secrets.Secret(this, this.stackName+'_LaravelAppKeySecret',{
      secretName: this.stackName+'-LaravelAppKey',
      generateSecretString: {
        includeSpace: false,
        passwordLength: 32,
        excludeCharacters: '"@/\\'
      }
    })

    // Builds and publishes the container image
    const image = new ecr_assets.DockerImageAsset(this, this.stackName+'_Image', {
      directory: __dirname+'/../docker/'+applicationName,
      buildArgs:{
        'tag':applicationName
      }
    })

    //Creates the ECS Fargate cluster, task, and application load balancer
    const fargateservice = new ecs_patterns.ApplicationLoadBalancedFargateService(this, this.stackName, {
      vpc: vpc,
      publicLoadBalancer: publicLoadBalancer,
      desiredCount: desiredCount,
      taskImageOptions: {
        image: ecs.ContainerImage.fromEcrRepository(image.repository),
        enableLogging: true,
        logDriver: new ecs.AwsLogDriver({ streamPrefix: this.stackName+'-'+this.region }),
        environment:{
          'APP_DEBUG': 'false',
          'DB_USERNAME': cdk.Fn.importValue('DBUser'),
          'DB_DATABASE': cdk.Fn.importValue('DBName'),
          'DB_HOST': cdk.Fn.importValue('DBHost'),
          'DB_PORT': '3306'
        },
        secrets: {
          'DB_PASSWORD': ecs.Secret.fromSecretsManager(secrets.Secret.fromSecretArn(this, this.stackName+'_DBPasswordSecret', cdk.Fn.importValue('DBSecretArn'))),
          'APP_KEY': ecs.Secret.fromSecretsManager(laravelAppSecret)
        },
        containerPort: 80,
        containerName: this.stackName,
      },
      cpu: cpu,
      memoryLimitMiB: memoryLimit
    })

    fargateservice.cluster.connections.addSecurityGroup(ec2.SecurityGroup.fromSecurityGroupId(this, this.stackName+'_DBSecurityGroup', cdk.Fn.importValue('DBSecurityGroupId')))
  }
}

db.ts

export class RDSStack extends cdk.Stack {
  constructor(scope: cdk.Construct, id: string, props?: cdk.StackProps) {
    super(scope, id, props);

    // Imports the VPC from the name provided
    const vpc = ec2.Vpc.fromLookup(this, this.stackName+'VPC', {
      vpcName: vpc_name
    })    

    //Creates secrets needed for the stack
    const dbSecret = new secrets.Secret(this, this.stackName+'_DBPasswordSecret',{
      secretName: this.stackName+'-DBPassword',
      generateSecretString: {
        includeSpace: false,
        passwordLength: 32,
        excludeCharacters: '"@/\\'
      }
    })

    //Creates Security Group and adds Ingress rule to open 3306 to everyone
    const sg =  new ec2.SecurityGroup(this, this.stackName+'_SecurityGroup', {
      securityGroupName: this.stackName+'_SecurityGroup',
      vpc: vpc,
      allowAllOutbound: allOutbound,
      description: 'Security Group to access '+this.stackName+' database'
    })
    sg.connections.allowInternally(ec2.Port.tcp(3306))

    const dbSubnetGroup = new rds.CfnDBSubnetGroup(this, this.stackName+'_DBSubnetGroup',{
      subnetIds: vpc.privateSubnets.map(subnet => subnet.subnetId),
      dbSubnetGroupDescription: 'Aurora Subnet Group'
    })
    const db = new rds.CfnDBCluster(this, this.stackName+'-AuroraRDSCluster', {
      engine: 'aurora',
      engineMode: 'serverless',
      engineVersion: '5.6',
      masterUsername: dbUsername,
      masterUserPassword: dbSecret.secretValue.toString(),
      dbSubnetGroupName: dbSubnetGroup.ref,
      availabilityZones: vpc.availabilityZones,
      databaseName: dbName,
      deletionProtection: dbdelectionprotection,
      backupRetentionPeriod: 30,
      scalingConfiguration:{
        minCapacity: dbMin,
        maxCapacity: dbMax,
        autoPause: dbAutoPause,
        secondsUntilAutoPause: dbAutoPauseSeconds
      },
      vpcSecurityGroupIds: [sg.securityGroupId],
    })

    //Creates Outputs that can be used by other templates in the same account
    new cdk.CfnOutput(this, this.stackName+'DBUser', {
      value: db.masterUsername as string, 
      exportName: 'DBUser'
    })
    new cdk.CfnOutput(this, this.stackName+'DBName', {
      value: db.databaseName as string,
      exportName: 'DBName'
    })
    new cdk.CfnOutput(this, this.stackName+'DBHost', {
      value: db.attrEndpointAddress,
      exportName: 'DBHost'
    })
    new cdk.CfnOutput(this, this.stackName+'DBPort', {
      value: db.attrEndpointPort,
      exportName: 'DBPort'
    })
    new cdk.CfnOutput(this, this.stackName+'DBSecretArn',{
      value: dbSecret.secretArn,
      exportName: 'DBSecretArn'
    })
    new cdk.CfnOutput(this, this.stackName+'DBSecurityGroupId',{
      value: sg.securityGroupId,
      exportName: 'DBSecurityGroupId'
    })
  }
}

Error Log

No real error messages just don't see the security group in the cloud formation json nor will the application connect to the database.

Environment

• **CLI Version : 1.19
• **Framework Version: 1.19
• **OS : Mac OSX
• **Language : typescript

Other

I am currently using this as a workaround as it has a similar effect:

ec2.SecurityGroup.fromSecurityGroupId(this, this.stackName+'_DBSecurityGroup', cdk.Fn.importValue('DBSecurityGroupId')).connections.allowFrom(fargateservice.service.connections, ec2.Port.tcp(3306))

---

This is :bug: Bug Report

[MichaelHindley]

I can confirm that the Fargate service does not have the passed connections.addSecuritGroup() SG once it's running

[trondhindenes]

this is a problem for regular ECS clusters as well. I can't find any way to actually add security group ingress settings to a CDK-created cluster, which is pretty wild. imho this is a high-pri bug. I would also say that we're seeing too many of these strange bugs and behaviors, more often than not forcing us to work with low-level cfn classes instead, since these are far more robust. I don't mind it, but its a shame.

[MrArnoldPalmer]

So the issue here is that ECS Clusters don't have security groups. When using ECS with EC2 (not fargate), the connections object on ecs.Cluster dictates the security groups on the EC2 instances within the cluster. This is something the CDK setup for convenience but its kinda confusing when you're using fargate or just in general when you see a connections object on the cluster when it doesn't really go there.

To set the security groups for your fargate services, you can do so using the connections object on the ecs.FargateService. You can access this on the .service property of your ALB/NLB fargate services in ecs-patterns.

This is pretty confusing for users imo. @SoManyHs @uttarasridhar @kohidave you guys might have an opinion here.

[MrArnoldPalmer]

relates to #7400

[MichaelHindley]

To set the security groups for your fargate services, you can do so using the connections object on the ecs.FargateService. You can access this on the .service property of your ALB/NLB fargate services in ecs-patterns.

If I do ecs.FargateService.connections.addSecurityGroup where ecs.FargateService is created by ecs-patterns.ApplicationLoadBalancedFargateService, the result is that the added SecurityGroup is added to the ALB in front, not on the Fargate service itself.

The above behaviour is quite confusing since it's added on service.connections, but there is also a loadBalancer.connections object that exposes the same addSecurityGroup method, neither of those results in a SecurityGroup added to the FargateService.

Is this a bug? Judging from the methods and the conversations here, service.connections.addSecurityGroup should result in a SecurityGroup on the Fargate service itself, not on the LoadBalancer?

(cdk 1.38.0)

[MichaelHindley]

Not sure if related or should be it's own ticket, but rdsInstance.connections.addSecurityGroup also does not add the passed securitygroup to the databases securitygroups.

[mescam]

I think I have noticed the same issue. Fargate construct allows to define security group only when creating an instance. Using connections.addSecurityGroup does not alter the cloudformation. But I will verify it again :)

[ap00rv]

To set the security groups for your fargate services, you can do so using the connections object on the ecs.FargateService. You can access this on the .service property of your ALB/NLB fargate services in ecs-patterns.

If I do ecs.FargateService.connections.addSecurityGroup where ecs.FargateService is created by ecs-patterns.ApplicationLoadBalancedFargateService, the result is that the added SecurityGroup is added to the ALB in front, not on the Fargate service itself.

The above behaviour is quite confusing since it's added on service.connections, but there is also a loadBalancer.connections object that exposes the same addSecurityGroup method, neither of those results in a SecurityGroup added to the FargateService.

Is this a bug? Judging from the methods and the conversations here, service.connections.addSecurityGroup should result in a SecurityGroup on the Fargate service itself, not on the LoadBalancer?

(cdk 1.38.0)

can confirm this behavior for NetworkLoadBalancedFargateService and I think it affects all NLB constructs in ecs-patterns module.

If we call the addSecurityGroup() method on Connections object of the fargate service, the security group does not get added to the networkConfiguration property of the service , instead its just added to the VPC.

To add a security group to the fargate service's networkConfiguration , we need to pass the SG in SecurityGroups or securityGroup property of the underlying FargateService construct, which is not possible in NetworkLoadBalancedFargateService.

Since the networkConfiguration property, configureAwsVpcNetworking () method & configureAwsVpcNetworkingWithSecurityGroups () method are protected , they cannot be accessed directly outside the BaseService class , we can't manually add SGs to it.

[ap00rv]

I am willing to send a PR for this by including an optional securityGroups prop that will get passed to the underlying ECS service. Let me know. Thanks .

[MrArnoldPalmer]

@ap00rv that is very interesting. This feels like a bug with the design of the connections on the construct where the networkConfiguration property should actually be a lazy value that computes the list of security groups from the connections property before synth.

If that makes sense and is possible, I believe it would be the preferred solution. Keeping the number of input properties down, especially on the higher level constructs like those in patterns, is something that we are trying to do. It would be easy to expose every single property of the underlying constructs as time goes on but then the abstraction of the pattern starts to become redundant.

If you're interested in exploring this in a PR definitely go for it. Some investigation may be necessary to see if we can accomplish this via connections. The connections object is used across constructs and if the patterns are not correctly leveraging it there may be other bugs stemming from this.

[spyoungtech]

@ap00rv that sounds awesome if you could make that PR. If the security group configurations can be initialized independently for the load balancer and service, that should provide a workaround to the immediate issue described here.

Above and beyond, ideally, adding a security group via albService.service.connections should also add security groups to the service rather than (or perhaps in addition to?) the LB, but this might end up being a breaking change, particularly considering that changing the service networking configuration requires resource replacement (whereas LB SG changes do not).

[neilferreira]

This is still an active issue, FWIW. @ap00rv have you been able to make any progress on it?

[automartin5000]

It's wild that this is still an issue 4 years later. Similar to @MichaelHindley, I've also seen this in the RDS construct. Are they related?