Merge pull request #8007 from mitch000001/automated-cherry-pick-of-#8…

…004-origin-release-1.16

Automated cherry pick of #8004: fix(openstack): fix additional security groups on instance groups

upup/pkg/fi/cloudup/openstacktasks/port.go

@@ -161,7 +161,7 @@ func (*Port) RenderOpenstack(t *openstack.OpenstackAPITarget, a, e, changes *Por
 	if a == nil {
 		klog.V(2).Infof("Creating Port with name: %q", fi.StringValue(e.Name))
 
-		opt, err := portCreateOptsFromPortTask(a, e, changes)
+		opt, err := portCreateOptsFromPortTask(t, a, e, changes)
 		if err != nil {
 			return fmt.Errorf("Error creating port cloud opts: %v", err)
 		}
@@ -191,13 +191,23 @@ func (*Port) RenderOpenstack(t *openstack.OpenstackAPITarget, a, e, changes *Por
 	return nil
 }
 
-func portCreateOptsFromPortTask(a, e, changes *Port) (ports.CreateOptsBuilder, error) {
+func portCreateOptsFromPortTask(t *openstack.OpenstackAPITarget, a, e, changes *Port) (ports.CreateOptsBuilder, error) {
 	sgs := make([]string, len(e.SecurityGroups)+len(e.AdditionalSecurityGroups))
 	for i, sg := range e.SecurityGroups {
 		sgs[i] = fi.StringValue(sg.ID)
 	}
 	for i, sg := range e.AdditionalSecurityGroups {
-		sgs[i+len(e.SecurityGroups)] = sg
+		opt := secgroup.ListOpts{
+			Name: sg,
+		}
+		gs, err := t.Cloud.ListSecurityGroups(opt)
+		if err != nil {
+			continue
+		}
+		if len(gs) == 0 {
+			return nil, fmt.Errorf("Additional SecurityGroup not found for name %s", sg)
+		}
+		sgs[i+len(e.SecurityGroups)] = gs[0].ID
 	}
 	fixedIPs := make([]ports.IP, len(e.Subnets))
 	for i, subn := range e.Subnets {

upup/pkg/fi/cloudup/openstacktasks/port_test.go

@@ -594,6 +594,7 @@ func Test_Port_RenderOpenstack(t *testing.T) {
 func Test_Port_createOptsFromPortTask(t *testing.T) {
 	tests := []struct {
 		desc               string
+		target             *openstack.OpenstackAPITarget
 		actual             *Port
 		expected           *Port
 		changes            *Port
@@ -602,6 +603,18 @@ func Test_Port_createOptsFromPortTask(t *testing.T) {
 	}{
 		{
 			desc: "all fields set",
+			target: &openstack.OpenstackAPITarget{
+				Cloud: &portCloud{
+					listSecurityGroups: map[string][]sg.SecGroup{
+						"add-1": {
+							{ID: "add-1-id", Name: "add-1"},
+						},
+						"add-2": {
+							{ID: "add-2-id", Name: "add-2"},
+						},
+					},
+				},
+			},
 			expected: &Port{
 				ID:      fi.String("expected-id"),
 				Name:    fi.String("name"),
@@ -625,20 +638,49 @@ func Test_Port_createOptsFromPortTask(t *testing.T) {
 				SecurityGroups: &[]string{
 					"sg-1",
 					"sg-2",
-					"add-1",
-					"add-2",
+					"add-1-id",
+					"add-2-id",
 				},
 				FixedIPs: []ports.IP{
 					{SubnetID: "subnet-a"},
 					{SubnetID: "subnet-b"},
 				},
 			},
 		},
+		{
+			desc: "nonexisting additional security groups",
+			target: &openstack.OpenstackAPITarget{
+				Cloud: &portCloud{
+					listSecurityGroups: map[string][]sg.SecGroup{
+						"add-1": {
+							{ID: "add-1-id", Name: "add-1"},
+						},
+					},
+				},
+			},
+			expected: &Port{
+				ID:      fi.String("expected-id"),
+				Name:    fi.String("name"),
+				Network: &Network{ID: fi.String("networkID")},
+				SecurityGroups: []*SecurityGroup{
+					{ID: fi.String("sg-1")},
+					{ID: fi.String("sg-2")},
+				},
+				AdditionalSecurityGroups: []string{
+					"add-2",
+				},
+				Subnets: []*Subnet{
+					{ID: fi.String("subnet-a")},
+					{ID: fi.String("subnet-b")},
+				},
+			},
+			expectedError: fmt.Errorf("Additional SecurityGroup not found for name add-2"),
+		},
 	}
 
 	for _, testCase := range tests {
 		t.Run(testCase.desc, func(t *testing.T) {
-			opts, err := portCreateOptsFromPortTask(testCase.actual, testCase.expected, testCase.changes)
+			opts, err := portCreateOptsFromPortTask(testCase.target, testCase.actual, testCase.expected, testCase.changes)
 
 			if !reflect.DeepEqual(err, testCase.expectedError) {
 				t.Errorf("Error differs:\n%v\n\tinstead of\n%v", err, testCase.expectedError)