Skip to content

Commit

Permalink
When tagging security groups, ensure cloud provide tag is only on lb
Browse files Browse the repository at this point in the history 
Signed-off-by: Vince Prignano <[email protected]>
  • Loading branch information
@vincepri
vincepri committed Oct 11, 2023
1 parent c0ab2ec commit fbcf962
Show file tree
Hide file tree
Showing 2 changed files with 204 additions and 1 deletion.
13 changes: 12 additions & 1 deletion pkg/cloud/services/securitygroup/securitygroups.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -648,9 +648,20 @@ func (s *Service) getDefaultSecurityGroup(role infrav1.SecurityGroupRole) *ec2.S

func (s *Service) getSecurityGroupTagParams(name, id string, role infrav1.SecurityGroupRole) infrav1.BuildParams {
additional := s.scope.AdditionalTags()

// Handle the cloud provider tag.
cloudProviderTag := infrav1.ClusterAWSCloudProviderTagKey(s.scope.Name())
if role == infrav1.SecurityGroupLB {
additional[infrav1.ClusterAWSCloudProviderTagKey(s.scope.Name())] = string(infrav1.ResourceLifecycleOwned)
additional[cloudProviderTag] = string(infrav1.ResourceLifecycleOwned)
} else if _, ok := additional[cloudProviderTag]; ok {
// If the cloud provider tag is set in more than one security group,
// the CCM will not be able to determine which security group to use;
// remove the tag from all security groups except the load balancer security group.
delete(additional, cloudProviderTag)
s.scope.Debug("Removing cloud provider owned tag from non load balancer security group",
"tag", cloudProviderTag, "name", name, "role", role, "id", id)
}

return infrav1.BuildParams{
ClusterName: s.scope.Name(),
Lifecycle: infrav1.ResourceLifecycleOwned,
Expand Down
192 changes: 192 additions & 0 deletions pkg/cloud/services/securitygroup/securitygroups_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,198 @@ func TestReconcileSecurityGroups(t *testing.T) {
}, nil).AnyTimes()
},
},
{
name: "additional tags includes cloud provider tag, only tag lb",
awsCluster: func(acl infrav1.AWSCluster) infrav1.AWSCluster {
acl.Spec.AdditionalTags = infrav1.Tags{
infrav1.ClusterAWSCloudProviderTagKey("test-cluster"): "owned",
}
return acl
},
input: &infrav1.NetworkSpec{
VPC: infrav1.VPCSpec{
ID: "vpc-securitygroups",
InternetGatewayID: aws.String("igw-01"),
Tags: infrav1.Tags{
infrav1.ClusterTagKey("test-cluster"): "owned",
},
},
Subnets: infrav1.Subnets{
infrav1.SubnetSpec{
ID: "subnet-securitygroups-private",
IsPublic: false,
AvailabilityZone: "us-east-1a",
},
infrav1.SubnetSpec{
ID: "subnet-securitygroups-public",
IsPublic: true,
NatGatewayID: aws.String("nat-01"),
AvailabilityZone: "us-east-1a",
},
},
},
expect: func(m *mocks.MockEC2APIMockRecorder) {
m.DescribeSecurityGroupsWithContext(context.TODO(), gomock.AssignableToTypeOf(&ec2.DescribeSecurityGroupsInput{})).
Return(&ec2.DescribeSecurityGroupsOutput{}, nil)

securityGroupBastion := m.CreateSecurityGroupWithContext(context.TODO(), gomock.Eq(&ec2.CreateSecurityGroupInput{
VpcId: aws.String("vpc-securitygroups"),
GroupName: aws.String("test-cluster-bastion"),
Description: aws.String("Kubernetes cluster test-cluster: bastion"),
TagSpecifications: []*ec2.TagSpecification{
{
ResourceType: aws.String("security-group"),
Tags: []*ec2.Tag{
{
Key: aws.String("Name"),
Value: aws.String("test-cluster-bastion"),
},
{
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/cluster/test-cluster"),
Value: aws.String("owned"),
},
{
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/role"),
Value: aws.String("bastion"),
},
},
},
},
})).
Return(&ec2.CreateSecurityGroupOutput{GroupId: aws.String("sg-bastion")}, nil)

m.AuthorizeSecurityGroupIngressWithContext(context.TODO(), gomock.AssignableToTypeOf(&ec2.AuthorizeSecurityGroupIngressInput{
GroupId: aws.String("sg-bastion"),
})).
Return(&ec2.AuthorizeSecurityGroupIngressOutput{}, nil).
After(securityGroupBastion)

securityGroupAPIServerLb := m.CreateSecurityGroupWithContext(context.TODO(), gomock.Eq(&ec2.CreateSecurityGroupInput{
VpcId: aws.String("vpc-securitygroups"),
GroupName: aws.String("test-cluster-apiserver-lb"),
Description: aws.String("Kubernetes cluster test-cluster: apiserver-lb"),
TagSpecifications: []*ec2.TagSpecification{
{
ResourceType: aws.String("security-group"),
Tags: []*ec2.Tag{
{
Key: aws.String("Name"),
Value: aws.String("test-cluster-apiserver-lb"),
},
{
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/cluster/test-cluster"),
Value: aws.String("owned"),
},
{
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/role"),
Value: aws.String("apiserver-lb"),
},
},
},
},
})).
Return(&ec2.CreateSecurityGroupOutput{GroupId: aws.String("sg-apiserver-lb")}, nil)

m.AuthorizeSecurityGroupIngressWithContext(context.TODO(), gomock.AssignableToTypeOf(&ec2.AuthorizeSecurityGroupIngressInput{
GroupId: aws.String("sg-apiserver-lb"),
})).
Return(&ec2.AuthorizeSecurityGroupIngressOutput{}, nil).
After(securityGroupAPIServerLb)

lbSecurityGroup := m.CreateSecurityGroupWithContext(context.TODO(), gomock.Eq(&ec2.CreateSecurityGroupInput{
VpcId: aws.String("vpc-securitygroups"),
GroupName: aws.String("test-cluster-lb"),
Description: aws.String("Kubernetes cluster test-cluster: lb"),
TagSpecifications: []*ec2.TagSpecification{
{
ResourceType: aws.String("security-group"),
Tags: []*ec2.Tag{
{
Key: aws.String("Name"),
Value: aws.String("test-cluster-lb"),
},
{
Key: aws.String("kubernetes.io/cluster/test-cluster"),
Value: aws.String("owned"),
},
{
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/cluster/test-cluster"),
Value: aws.String("owned"),
},
{
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/role"),
Value: aws.String("lb"),
},
},
},
},
})).Return(&ec2.CreateSecurityGroupOutput{GroupId: aws.String("sg-lb")}, nil)

m.AuthorizeSecurityGroupIngressWithContext(context.TODO(), gomock.AssignableToTypeOf(&ec2.AuthorizeSecurityGroupIngressInput{
GroupId: aws.String("sg-lb"),
})).
Return(&ec2.AuthorizeSecurityGroupIngressOutput{}, nil).
After(lbSecurityGroup)

securityGroupControl := m.CreateSecurityGroupWithContext(context.TODO(), gomock.Eq(&ec2.CreateSecurityGroupInput{
VpcId: aws.String("vpc-securitygroups"),
GroupName: aws.String("test-cluster-controlplane"),
Description: aws.String("Kubernetes cluster test-cluster: controlplane"),
TagSpecifications: []*ec2.TagSpecification{
{
ResourceType: aws.String("security-group"),
Tags: []*ec2.Tag{
{
Key: aws.String("Name"),
Value: aws.String("test-cluster-controlplane"),
},
{
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/cluster/test-cluster"),
Value: aws.String("owned"),
},
{
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/role"),
Value: aws.String("controlplane"),
},
},
},
},
})).
Return(&ec2.CreateSecurityGroupOutput{GroupId: aws.String("sg-control")}, nil)

m.AuthorizeSecurityGroupIngressWithContext(context.TODO(), gomock.AssignableToTypeOf(&ec2.AuthorizeSecurityGroupIngressInput{
GroupId: aws.String("sg-control"),
})).
Return(&ec2.AuthorizeSecurityGroupIngressOutput{}, nil).
After(securityGroupControl)

m.CreateSecurityGroupWithContext(context.TODO(), gomock.Eq(&ec2.CreateSecurityGroupInput{
VpcId: aws.String("vpc-securitygroups"),
GroupName: aws.String("test-cluster-node"),
Description: aws.String("Kubernetes cluster test-cluster: node"),
TagSpecifications: []*ec2.TagSpecification{
{
ResourceType: aws.String("security-group"),
Tags: []*ec2.Tag{
{
Key: aws.String("Name"),
Value: aws.String("test-cluster-node"),
},
{
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/cluster/test-cluster"),
Value: aws.String("owned"),
},
{
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/role"),
Value: aws.String("node"),
},
},
},
},
})).
Return(&ec2.CreateSecurityGroupOutput{GroupId: aws.String("sg-node")}, nil)
},
},
{
name: "managed vpc with overrides, returns error",
awsCluster: func(acl infrav1.AWSCluster) infrav1.AWSCluster {
Expand Down

0 comments on commit fbcf962

Please sign in to comment.