diff --git a/pkg/cloudprovider/providers/aws/aws.go b/pkg/cloudprovider/providers/aws/aws.go index 83ad8819542..e6d953967cb 100644 --- a/pkg/cloudprovider/providers/aws/aws.go +++ b/pkg/cloudprovider/providers/aws/aws.go @@ -2845,8 +2845,9 @@ func (c *Cloud) removeSecurityGroupIngress(securityGroupID string, removePermiss // Makes sure the security group exists. // For multi-cluster isolation, name must be globally unique, for example derived from the service UUID. +// Additional tags can be specified // Returns the security group id or error -func (c *Cloud) ensureSecurityGroup(name string, description string) (string, error) { +func (c *Cloud) ensureSecurityGroup(name string, description string, additionalTags map[string]string) (string, error) { groupID := "" attempt := 0 for { @@ -2912,7 +2913,7 @@ func (c *Cloud) ensureSecurityGroup(name string, description string) (string, er return "", fmt.Errorf("created security group, but id was not returned: %s", name) } - err := c.tagging.createTags(c.ec2, groupID, ResourceLifecycleOwned, nil) + err := c.tagging.createTags(c.ec2, groupID, ResourceLifecycleOwned, additionalTags) if err != nil { // If we retry, ensureClusterTags will recover from this - it // will add the missing tags. We could delete the security @@ -3126,9 +3127,10 @@ func getPortSets(annotation string) (ports *portSets) { // buildELBSecurityGroupList returns list of SecurityGroups which should be // attached to ELB created by a service. List always consist of at least -// 1 member which is an SG created for this service or a SG from the Global config. Extra groups can be -// specified via annotation -func (c *Cloud) buildELBSecurityGroupList(serviceName types.NamespacedName, loadBalancerName, annotation string) ([]string, error) { +// 1 member which is an SG created for this service or a SG from the Global config. +// Extra groups can be specified via annotation, as can extra tags for any +// new groups. +func (c *Cloud) buildELBSecurityGroupList(serviceName types.NamespacedName, loadBalancerName string, annotations map[string]string) ([]string, error) { var err error var securityGroupID string @@ -3138,7 +3140,7 @@ func (c *Cloud) buildELBSecurityGroupList(serviceName types.NamespacedName, load // Create a security group for the load balancer sgName := "k8s-elb-" + loadBalancerName sgDescription := fmt.Sprintf("Security group for Kubernetes ELB %s (%v)", loadBalancerName, serviceName) - securityGroupID, err = c.ensureSecurityGroup(sgName, sgDescription) + securityGroupID, err = c.ensureSecurityGroup(sgName, sgDescription, getLoadBalancerAdditionalTags(annotations)) if err != nil { glog.Errorf("Error creating load balancer security group: %q", err) return nil, err @@ -3146,7 +3148,7 @@ func (c *Cloud) buildELBSecurityGroupList(serviceName types.NamespacedName, load } sgList := []string{securityGroupID} - for _, extraSG := range strings.Split(annotation, ",") { + for _, extraSG := range strings.Split(annotations[ServiceAnnotationLoadBalancerExtraSecurityGroups], ",") { extraSG = strings.TrimSpace(extraSG) if len(extraSG) > 0 { sgList = append(sgList, extraSG) @@ -3444,7 +3446,7 @@ func (c *Cloud) EnsureLoadBalancer(ctx context.Context, clusterName string, apiS loadBalancerName := cloudprovider.GetLoadBalancerName(apiService) serviceName := types.NamespacedName{Namespace: apiService.Namespace, Name: apiService.Name} - securityGroupIDs, err := c.buildELBSecurityGroupList(serviceName, loadBalancerName, annotations[ServiceAnnotationLoadBalancerExtraSecurityGroups]) + securityGroupIDs, err := c.buildELBSecurityGroupList(serviceName, loadBalancerName, annotations) if err != nil { return nil, err } diff --git a/pkg/cloudprovider/providers/aws/aws_test.go b/pkg/cloudprovider/providers/aws/aws_test.go index 5e9601730b7..8ea62cae386 100644 --- a/pkg/cloudprovider/providers/aws/aws_test.go +++ b/pkg/cloudprovider/providers/aws/aws_test.go @@ -1132,19 +1132,20 @@ func TestLBExtraSecurityGroupsAnnotation(t *testing.T) { awsServices := newMockedFakeAWSServices(TestClusterId) c, _ := newAWSCloud(CloudConfig{}, awsServices) - sg1 := "sg-000001" - sg2 := "sg-000002" + sg1 := map[string]string{ServiceAnnotationLoadBalancerExtraSecurityGroups: "sg-000001"} + sg2 := map[string]string{ServiceAnnotationLoadBalancerExtraSecurityGroups: "sg-000002"} + sg3 := map[string]string{ServiceAnnotationLoadBalancerExtraSecurityGroups: "sg-000001, sg-000002"} tests := []struct { name string - extraSGsAnnotation string - expectedSGs []string + annotations map[string]string + expectedSGs []string }{ - {"No extra SG annotation", "", []string{}}, - {"Empty extra SGs specified", ", ,,", []string{}}, - {"SG specified", sg1, []string{sg1}}, - {"Multiple SGs specified", fmt.Sprintf("%s, %s", sg1, sg2), []string{sg1, sg2}}, + {"No extra SG annotation", map[string]string{}, []string{}}, + {"Empty extra SGs specified", map[string]string{ServiceAnnotationLoadBalancerExtraSecurityGroups: ", ,,"}, []string{}}, + {"SG specified", sg1, []string{sg1[ServiceAnnotationLoadBalancerExtraSecurityGroups]}}, + {"Multiple SGs specified", sg3, []string{sg1[ServiceAnnotationLoadBalancerExtraSecurityGroups], sg2[ServiceAnnotationLoadBalancerExtraSecurityGroups]}}, } awsServices.ec2.(*MockedFakeEC2).expectDescribeSecurityGroups(TestClusterId, "k8s-elb-aid", "cluster.test") @@ -1153,7 +1154,7 @@ func TestLBExtraSecurityGroupsAnnotation(t *testing.T) { t.Run(test.name, func(t *testing.T) { serviceName := types.NamespacedName{Namespace: "default", Name: "myservice"} - sgList, err := c.buildELBSecurityGroupList(serviceName, "aid", test.extraSGsAnnotation) + sgList, err := c.buildELBSecurityGroupList(serviceName, "aid", test.annotations) assert.NoError(t, err, "buildELBSecurityGroupList failed") extraSGs := sgList[1:] assert.True(t, sets.NewString(test.expectedSGs...).Equal(sets.NewString(extraSGs...)),