From 7c7e691c5f4e57b3f0e24227af000e40fbb7394f Mon Sep 17 00:00:00 2001
From: Soren Mathiasen <smo@tradeshift.com>
Date: Fri, 26 Jan 2018 12:54:27 +0100
Subject: [PATCH] Include more information when multiple security groups are
 tagged

When trying to create ELB we can sometime fail if there is more then one AWS
security group tagged. It very useful to get the list of security groups printed in
the error message.

**Release note**:

```release-note
  Include the list of security groups when failing with the errors that more then one is tagged
```
---
 pkg/cloudprovider/providers/aws/aws.go      |  6 +++++-
 pkg/cloudprovider/providers/aws/aws_test.go | 23 +++++++++++++++++++++
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/pkg/cloudprovider/providers/aws/aws.go b/pkg/cloudprovider/providers/aws/aws.go
index 31faa871d68..f0c221b774e 100644
--- a/pkg/cloudprovider/providers/aws/aws.go
+++ b/pkg/cloudprovider/providers/aws/aws.go
@@ -3653,7 +3653,11 @@ func findSecurityGroupForInstance(instance *ec2.Instance, taggedSecurityGroups m
 		// We create instances with one SG
 		// If users create multiple SGs, they must tag one of them as being k8s owned
 		if len(tagged) != 1 {
-			return nil, fmt.Errorf("Multiple tagged security groups found for instance %s; ensure only the k8s security group is tagged", instanceID)
+			taggedGroups := ""
+			for _, v := range tagged {
+				taggedGroups += fmt.Sprintf("%s(%s) ", *v.GroupId, *v.GroupName)
+			}
+			return nil, fmt.Errorf("Multiple tagged security groups found for instance %s; ensure only the k8s security group is tagged; the tagged groups were %v", instanceID, taggedGroups)
 		}
 		return tagged[0], nil
 	}
diff --git a/pkg/cloudprovider/providers/aws/aws_test.go b/pkg/cloudprovider/providers/aws/aws_test.go
index 5e9601730b7..b5368e0fb1f 100644
--- a/pkg/cloudprovider/providers/aws/aws_test.go
+++ b/pkg/cloudprovider/providers/aws/aws_test.go
@@ -1302,6 +1302,29 @@ func TestEnsureLoadBalancerHealthCheck(t *testing.T) {
 	})
 }
 
+func TestFindSecurityGroupForInstance(t *testing.T) {
+	groups := map[string]*ec2.SecurityGroup{"sg123": {GroupId: aws.String("sg123")}}
+	id, err := findSecurityGroupForInstance(&ec2.Instance{SecurityGroups: []*ec2.GroupIdentifier{{GroupId: aws.String("sg123"), GroupName: aws.String("my_group")}}}, groups)
+	if err != nil {
+		t.Error()
+	}
+	assert.Equal(t, *id.GroupId, "sg123")
+	assert.Equal(t, *id.GroupName, "my_group")
+}
+
+func TestFindSecurityGroupForInstanceMultipleTagged(t *testing.T) {
+	groups := map[string]*ec2.SecurityGroup{"sg123": {GroupId: aws.String("sg123")}}
+	_, err := findSecurityGroupForInstance(&ec2.Instance{
+		SecurityGroups: []*ec2.GroupIdentifier{
+			{GroupId: aws.String("sg123"), GroupName: aws.String("my_group")},
+			{GroupId: aws.String("sg123"), GroupName: aws.String("another_group")},
+		},
+	}, groups)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "sg123(my_group)")
+	assert.Contains(t, err.Error(), "sg123(another_group)")
+}
+
 func newMockedFakeAWSServices(id string) *FakeAWSServices {
 	s := NewFakeAWSServices(id)
 	s.ec2 = &MockedFakeEC2{FakeEC2Impl: s.ec2.(*FakeEC2Impl)}