From fbcf9624c60385e34da5956c139a98c7182bc627 Mon Sep 17 00:00:00 2001 From: Vince Prignano Date: Wed, 11 Oct 2023 08:34:06 -0700 Subject: [PATCH] When tagging security groups, ensure cloud provide tag is only on lb Signed-off-by: Vince Prignano --- .../services/securitygroup/securitygroups.go | 13 +- .../securitygroup/securitygroups_test.go | 192 ++++++++++++++++++ 2 files changed, 204 insertions(+), 1 deletion(-) diff --git a/pkg/cloud/services/securitygroup/securitygroups.go b/pkg/cloud/services/securitygroup/securitygroups.go index df5812f42b..802a016493 100644 --- a/pkg/cloud/services/securitygroup/securitygroups.go +++ b/pkg/cloud/services/securitygroup/securitygroups.go @@ -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, diff --git a/pkg/cloud/services/securitygroup/securitygroups_test.go b/pkg/cloud/services/securitygroup/securitygroups_test.go index 425f45c133..a292da31ee 100644 --- a/pkg/cloud/services/securitygroup/securitygroups_test.go +++ b/pkg/cloud/services/securitygroup/securitygroups_test.go @@ -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 {