Tag Security Group created for AWS ELB with same additional tags as ELB

Reuse the
service.beta.kubernetes.io/aws-load-balancer-additional-resource-tags
annotation for these tags.

We think that this is a simpler and less intrusive approach than adding
a new annotation, and most users will want the same set of tags applied.

Signed-off-by: Brett Kochendorfer <brett.kochendorfer@gmail.com>
This commit is contained in:
Andrew Garrett 2018-01-24 20:16:45 +00:00 committed by Brett Kochendorfer
parent 47d61ef472
commit 20a682b643
2 changed files with 16 additions and 15 deletions

View File

@ -2815,7 +2815,7 @@ func (c *Cloud) removeSecurityGroupIngress(securityGroupID string, removePermiss
// Makes sure the security group exists. // Makes sure the security group exists.
// For multi-cluster isolation, name must be globally unique, for example derived from the service UUID. // For multi-cluster isolation, name must be globally unique, for example derived from the service UUID.
// Returns the security group id or error // 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, annotations map[string]string) (string, error) {
groupID := "" groupID := ""
attempt := 0 attempt := 0
for { for {
@ -2881,7 +2881,7 @@ func (c *Cloud) ensureSecurityGroup(name string, description string) (string, er
return "", fmt.Errorf("created security group, but id was not returned: %s", name) 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, getLoadBalancerAdditionalTags(annotations))
if err != nil { if err != nil {
// If we retry, ensureClusterTags will recover from this - it // If we retry, ensureClusterTags will recover from this - it
// will add the missing tags. We could delete the security // will add the missing tags. We could delete the security
@ -3097,7 +3097,7 @@ func getPortSets(annotation string) (ports *portSets) {
// attached to ELB created by a service. List always consist of at least // 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 // 1 member which is an SG created for this service or a SG from the Global config. Extra groups can be
// specified via annotation // specified via annotation
func (c *Cloud) buildELBSecurityGroupList(serviceName types.NamespacedName, loadBalancerName, annotation string) ([]string, error) { func (c *Cloud) buildELBSecurityGroupList(serviceName types.NamespacedName, loadBalancerName string, annotations map[string]string) ([]string, error) {
var err error var err error
var securityGroupID string var securityGroupID string
@ -3107,7 +3107,7 @@ func (c *Cloud) buildELBSecurityGroupList(serviceName types.NamespacedName, load
// Create a security group for the load balancer // Create a security group for the load balancer
sgName := "k8s-elb-" + loadBalancerName sgName := "k8s-elb-" + loadBalancerName
sgDescription := fmt.Sprintf("Security group for Kubernetes ELB %s (%v)", loadBalancerName, serviceName) sgDescription := fmt.Sprintf("Security group for Kubernetes ELB %s (%v)", loadBalancerName, serviceName)
securityGroupID, err = c.ensureSecurityGroup(sgName, sgDescription) securityGroupID, err = c.ensureSecurityGroup(sgName, sgDescription, annotations)
if err != nil { if err != nil {
glog.Errorf("Error creating load balancer security group: %q", err) glog.Errorf("Error creating load balancer security group: %q", err)
return nil, err return nil, err
@ -3115,7 +3115,7 @@ func (c *Cloud) buildELBSecurityGroupList(serviceName types.NamespacedName, load
} }
sgList := []string{securityGroupID} sgList := []string{securityGroupID}
for _, extraSG := range strings.Split(annotation, ",") { for _, extraSG := range strings.Split(annotations[ServiceAnnotationLoadBalancerExtraSecurityGroups], ",") {
extraSG = strings.TrimSpace(extraSG) extraSG = strings.TrimSpace(extraSG)
if len(extraSG) > 0 { if len(extraSG) > 0 {
sgList = append(sgList, extraSG) sgList = append(sgList, extraSG)
@ -3413,7 +3413,7 @@ func (c *Cloud) EnsureLoadBalancer(clusterName string, apiService *v1.Service, n
loadBalancerName := cloudprovider.GetLoadBalancerName(apiService) loadBalancerName := cloudprovider.GetLoadBalancerName(apiService)
serviceName := types.NamespacedName{Namespace: apiService.Namespace, Name: apiService.Name} 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 { if err != nil {
return nil, err return nil, err
} }

View File

@ -1124,19 +1124,20 @@ func TestLBExtraSecurityGroupsAnnotation(t *testing.T) {
awsServices := newMockedFakeAWSServices(TestClusterId) awsServices := newMockedFakeAWSServices(TestClusterId)
c, _ := newAWSCloud(strings.NewReader("[global]"), awsServices) c, _ := newAWSCloud(strings.NewReader("[global]"), awsServices)
sg1 := "sg-000001" sg1 := map[string]string{ServiceAnnotationLoadBalancerExtraSecurityGroups: "sg-000001"}
sg2 := "sg-000002" sg2 := map[string]string{ServiceAnnotationLoadBalancerExtraSecurityGroups: "sg-000002"}
sg3 := map[string]string{ServiceAnnotationLoadBalancerExtraSecurityGroups: "sg-000001, sg-000002"}
tests := []struct { tests := []struct {
name string name string
extraSGsAnnotation string annotations map[string]string
expectedSGs []string expectedSGs []string
}{ }{
{"No extra SG annotation", "", []string{}}, {"No extra SG annotation", map[string]string{}, []string{}},
{"Empty extra SGs specified", ", ,,", []string{}}, {"Empty extra SGs specified", map[string]string{ServiceAnnotationLoadBalancerExtraSecurityGroups: ", ,,"}, []string{}},
{"SG specified", sg1, []string{sg1}}, {"SG specified", sg1, []string{sg1[ServiceAnnotationLoadBalancerExtraSecurityGroups]}},
{"Multiple SGs specified", fmt.Sprintf("%s, %s", sg1, sg2), []string{sg1, sg2}}, {"Multiple SGs specified", sg3, []string{sg1[ServiceAnnotationLoadBalancerExtraSecurityGroups], sg2[ServiceAnnotationLoadBalancerExtraSecurityGroups]}},
} }
awsServices.ec2.(*MockedFakeEC2).expectDescribeSecurityGroups(TestClusterId, "k8s-elb-aid", "cluster.test") awsServices.ec2.(*MockedFakeEC2).expectDescribeSecurityGroups(TestClusterId, "k8s-elb-aid", "cluster.test")
@ -1145,7 +1146,7 @@ func TestLBExtraSecurityGroupsAnnotation(t *testing.T) {
t.Run(test.name, func(t *testing.T) { t.Run(test.name, func(t *testing.T) {
serviceName := types.NamespacedName{Namespace: "default", Name: "myservice"} 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") assert.NoError(t, err, "buildELBSecurityGroupList failed")
extraSGs := sgList[1:] extraSGs := sgList[1:]
assert.True(t, sets.NewString(test.expectedSGs...).Equal(sets.NewString(extraSGs...)), assert.True(t, sets.NewString(test.expectedSGs...).Equal(sets.NewString(extraSGs...)),