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

Method addSecurityGroup does not change the deployed Cloudformation stack #5846

Closed
hencrice opened this issue Jan 17, 2020 · 2 comments
Closed
Assignees
Labels
@aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud guidance Question that needs advice or information. needs-triage This issue or PR still needs to be triaged.

Comments

@hencrice
Copy link
Contributor

addSecurityGroup does not seem to actually add an existing security group.

Reproduction Steps

The following 2 CDK apps result in identical Cloudformation stack template after cdk synth && cdk deploy is executed successfully :

import * as cdk from '@aws-cdk/core';
import * as ecs from '@aws-cdk/aws-ecs';
import * as ec2 from '@aws-cdk/aws-ec2';

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

    const vpc = new ec2.Vpc(this, 'VPC');

    const serverCluster = new ecs.Cluster(this, 'EcsServerCluster', { vpc: vpc })

    serverCluster.addCapacity('DefaultAutoScalingGroup', {
      instanceType: ec2.InstanceType.of(ec2.InstanceClass.T2, ec2.InstanceSize.MEDIUM),
    })
  }
}

Problematic stack:

import * as cdk from '@aws-cdk/core';
import * as ecs from '@aws-cdk/aws-ecs';
import * as ec2 from '@aws-cdk/aws-ec2';

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

    const vpc = new ec2.Vpc(this, 'VPC');

    const mySecurityGroup = ec2.SecurityGroup.fromSecurityGroupId(this, 'SG', 'sg-82870be7awdwafafawdwa', {
      mutable: true
    })

    const serverCluster = new ecs.Cluster(this, 'EcsServerCluster', { vpc: vpc })
    serverCluster.connections.addSecurityGroup(mySecurityGroup);

    serverCluster.addCapacity('DefaultAutoScalingGroup', {
      instanceType: ec2.InstanceType.of(ec2.InstanceClass.T2, ec2.InstanceSize.MEDIUM),
    })
  }
}

Error Log

N/A

Environment

  • CLI Version : 1.20.0 (build 021c521)
  • Framework Version:
  • OS :Alpine Linux 3.11 ()
  • Language :Typescript

Other

There appear to be a few related issues: #5635, #4891, #4678


This is 🐛 Bug Report

@hencrice hencrice added bug This issue is a bug. @aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud needs-triage This issue or PR still needs to be triaged. labels Jan 17, 2020
@rix0rrr rix0rrr added guidance Question that needs advice or information. and removed bug This issue is a bug. labels Jan 17, 2020
@rix0rrr
Copy link
Contributor

rix0rrr commented Jan 17, 2020

connections.addSecurityGroup(mySecurityGroup);

Only adds to the set of securitygroups that are managed by the connections object, it will not change the CloudFormation that gets generated for the Cluster. You need to pass any security groups you want to add to the Cluster to its constructor.

@rix0rrr rix0rrr closed this as completed Jan 17, 2020
@hencrice
Copy link
Contributor Author

hencrice commented Jan 17, 2020

Thanks for the answer. Do you happen to know why we wrote connections.connections. addSecurityGroup () instead of just connections.addSecurityGroup)() on

this.connections.connections.addSecurityGroup(...autoScalingGroup.connections.securityGroups);
? @rix0rrr

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud guidance Question that needs advice or information. needs-triage This issue or PR still needs to be triaged.
Projects
None yet
Development

No branches or pull requests

2 participants