diff --git a/pkg/model/openstackmodel/firewall.go b/pkg/model/openstackmodel/firewall.go index 04c18a7181..847f160433 100644 --- a/pkg/model/openstackmodel/firewall.go +++ b/pkg/model/openstackmodel/firewall.go @@ -456,7 +456,6 @@ func (b *FirewallModelBuilder) addProtokubeRules(c *fi.ModelBuilderContext, sgMa // Build - schedule security groups and security group rule tasks for Openstack func (b *FirewallModelBuilder) Build(c *fi.ModelBuilderContext) error { - roles := []kops.InstanceGroupRole{kops.InstanceGroupRoleMaster, kops.InstanceGroupRoleNode} if b.UsesSSHBastion() { roles = append(roles, kops.InstanceGroupRoleBastion) diff --git a/pkg/model/openstackmodel/servergroup.go b/pkg/model/openstackmodel/servergroup.go index 1ed6b745f1..46c33ee80c 100644 --- a/pkg/model/openstackmodel/servergroup.go +++ b/pkg/model/openstackmodel/servergroup.go @@ -110,11 +110,12 @@ func (b *ServerGroupModelBuilder) buildInstances(c *fi.ModelBuilderContext, sg * } // Create instance port task portTask := &openstacktasks.Port{ - Name: fi.String(fmt.Sprintf("%s-%s", "port", *instanceName)), - Network: b.LinkToNetwork(), - SecurityGroups: securityGroups, - Subnets: subnets, - Lifecycle: b.Lifecycle, + Name: fi.String(fmt.Sprintf("%s-%s", "port", *instanceName)), + Network: b.LinkToNetwork(), + SecurityGroups: securityGroups, + AdditionalSecurityGroups: ig.Spec.AdditionalSecurityGroups, + Subnets: subnets, + Lifecycle: b.Lifecycle, } c.AddTask(portTask) @@ -129,6 +130,7 @@ func (b *ServerGroupModelBuilder) buildInstances(c *fi.ModelBuilderContext, sg * Role: fi.String(string(ig.Spec.Role)), Port: portTask, Metadata: igMeta, + SecurityGroups: ig.Spec.AdditionalSecurityGroups, AvailabilityZone: az, } if igUserData != nil { diff --git a/pkg/model/openstackmodel/servergroup_test.go b/pkg/model/openstackmodel/servergroup_test.go index b4fe0e7573..bc4a7584e4 100644 --- a/pkg/model/openstackmodel/servergroup_test.go +++ b/pkg/model/openstackmodel/servergroup_test.go @@ -2299,6 +2299,97 @@ func Test_ServerGroupModelBuilder(t *testing.T) { } }, }, + { + desc: "adds additional security groups", + cluster: &kops.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster", + }, + Spec: kops.ClusterSpec{ + MasterPublicName: "master-public-name", + CloudConfig: &kops.CloudConfiguration{ + Openstack: &kops.OpenstackConfiguration{}, + }, + Subnets: []kops.ClusterSubnetSpec{ + { + Region: "region", + }, + }, + }, + }, + instanceGroups: []*kops.InstanceGroup{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Spec: kops.InstanceGroupSpec{ + Role: kops.InstanceGroupRoleNode, + Image: "image-node", + MinSize: i32(1), + MaxSize: i32(1), + MachineType: "blc.2-4", + Subnets: []string{"subnet"}, + AdditionalSecurityGroups: []string{ + "additional-sg", + }, + }, + }, + }, + expectedTasksBuilder: func(cluster *kops.Cluster, instanceGroups []*kops.InstanceGroup) map[string]fi.Task { + clusterLifecycle := fi.LifecycleSync + nodeServerGroup := &openstacktasks.ServerGroup{ + Name: s("cluster-node"), + ClusterName: s("cluster"), + IGName: s("node"), + Policies: []string{"anti-affinity"}, + Lifecycle: &clusterLifecycle, + MaxSize: i32(1), + } + nodePort := &openstacktasks.Port{ + Name: s("port-node-1-cluster"), + Network: &openstacktasks.Network{Name: s("cluster")}, + SecurityGroups: []*openstacktasks.SecurityGroup{ + {Name: s("nodes.cluster")}, + }, + AdditionalSecurityGroups: []string{ + "additional-sg", + }, + Subnets: []*openstacktasks.Subnet{ + {Name: s("subnet.cluster")}, + }, + Lifecycle: &clusterLifecycle, + } + nodeInstance := &openstacktasks.Instance{ + Name: s("node-1-cluster"), + Region: s("region"), + Flavor: s("blc.2-4"), + Image: s("image-node"), + SSHKey: s("kubernetes.cluster-ba_d8_85_a0_5b_50_b0_01_e0_b2_b0_ae_5d_f6_7a_d1"), + ServerGroup: nodeServerGroup, + Tags: []string{"KubernetesCluster:cluster"}, + Role: s("Node"), + Port: nodePort, + UserData: s(mustUserdataForClusterInstance(cluster, instanceGroups[0])), + Metadata: map[string]string{ + "KubernetesCluster": "cluster", + "k8s": "cluster", + "KopsInstanceGroup": "node", + "KopsRole": "Node", + "ig_generation": "0", + "cluster_generation": "0", + }, + AvailabilityZone: s("subnet"), + SecurityGroups: []string{ + "additional-sg", + }, + } + return map[string]fi.Task{ + "ServerGroup/cluster-node": nodeServerGroup, + "Instance/node-1-cluster": nodeInstance, + "Port/port-node-1-cluster": nodePort, + } + }, + }, } for _, testCase := range tests { @@ -2368,6 +2459,10 @@ func Test_ServerGroupModelBuilder(t *testing.T) { t.Run("creates a task for "+taskName, func(t *testing.T) { compareLBListeners(t, actual, expected) }) + case *openstacktasks.SecurityGroup: + t.Run("creates a task for "+taskName, func(t *testing.T) { + compareSecurityGroups(t, actual, expected) + }) default: t.Errorf("found a task with name %q and type %T", taskName, expected) } @@ -2412,7 +2507,14 @@ func comparePorts(t *testing.T, actualTask fi.Task, expected *openstacktasks.Por } compareStrings(t, "Name", actual.Name, expected.Name) - compareSecurityGroups(t, actual.SecurityGroups, expected.SecurityGroups) + compareSecurityGroupLists(t, actual.SecurityGroups, expected.SecurityGroups) + sort.Strings(actual.AdditionalSecurityGroups) + sort.Strings(expected.AdditionalSecurityGroups) + actualSgs := strings.Join(actual.AdditionalSecurityGroups, " ") + expectedSgs := strings.Join(expected.AdditionalSecurityGroups, " ") + if actualSgs != expectedSgs { + t.Errorf("AdditionalSecurityGroups differ: %q instead of %q", actualSgs, expectedSgs) + } compareLifecycles(t, actual.Lifecycle, expected.Lifecycle) if actual.Network == nil { t.Fatal("Network is nil") @@ -2503,6 +2605,13 @@ func compareInstances(t *testing.T, actualTask fi.Task, expected *openstacktasks if !reflect.DeepEqual(actual.Metadata, expected.Metadata) { t.Errorf("Metadata differ:\n%v\n\tinstead of\n%v", actual.Metadata, expected.Metadata) } + sort.Strings(actual.SecurityGroups) + sort.Strings(expected.SecurityGroups) + actualSgs := strings.Join(actual.SecurityGroups, " ") + expectedSgs := strings.Join(expected.SecurityGroups, " ") + if actualSgs != expectedSgs { + t.Errorf("SecurityGroups differ: %q instead of %q", actualSgs, expectedSgs) + } } func compareLoadbalancers(t *testing.T, actualTask fi.Task, expected *openstacktasks.LB) { @@ -2517,7 +2626,7 @@ func compareLoadbalancers(t *testing.T, actualTask fi.Task, expected *openstackt compareStrings(t, "Name", actual.Name, expected.Name) compareLifecycles(t, actual.Lifecycle, expected.Lifecycle) compareStrings(t, "Subnet", actual.Subnet, expected.Subnet) - compareSecurityGroups(t, []*openstacktasks.SecurityGroup{actual.SecurityGroup}, []*openstacktasks.SecurityGroup{expected.SecurityGroup}) + compareSecurityGroupLists(t, []*openstacktasks.SecurityGroup{actual.SecurityGroup}, []*openstacktasks.SecurityGroup{expected.SecurityGroup}) } func compareLBPools(t *testing.T, actualTask fi.Task, expected *openstacktasks.LBPool) { @@ -2620,7 +2729,22 @@ func compareLifecycles(t *testing.T, actual, expected *fi.Lifecycle) { } } -func compareSecurityGroups(t *testing.T, actual, expected []*openstacktasks.SecurityGroup) { +func compareSecurityGroups(t *testing.T, actualTask fi.Task, expected *openstacktasks.SecurityGroup) { + t.Helper() + if pointersAreBothNil(t, "SecurityGroup", actualTask, expected) { + return + } + actual, ok := actualTask.(*openstacktasks.SecurityGroup) + if !ok { + t.Fatalf("task is not a security group task, got %T", actualTask) + } + + compareStrings(t, "Name", actual.Name, expected.Name) + compareLifecycles(t, actual.Lifecycle, expected.Lifecycle) + compareStrings(t, "Description", actual.Description, expected.Description) +} + +func compareSecurityGroupLists(t *testing.T, actual, expected []*openstacktasks.SecurityGroup) { sgs := make([]string, len(actual)) for i, sg := range actual { sgs[i] = *sg.Name diff --git a/upup/pkg/fi/cloudup/openstacktasks/BUILD.bazel b/upup/pkg/fi/cloudup/openstacktasks/BUILD.bazel index d4e22122e4..13eaff5e9e 100644 --- a/upup/pkg/fi/cloudup/openstacktasks/BUILD.bazel +++ b/upup/pkg/fi/cloudup/openstacktasks/BUILD.bazel @@ -1,4 +1,4 @@ -load("@io_bazel_rules_go//go:def.bzl", "go_library") +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "go_default_library", @@ -64,3 +64,15 @@ go_library( "//vendor/k8s.io/klog:go_default_library", ], ) + +go_test( + name = "go_default_test", + srcs = ["port_test.go"], + embed = [":go_default_library"], + deps = [ + "//upup/pkg/fi:go_default_library", + "//upup/pkg/fi/cloudup/openstack:go_default_library", + "//vendor/github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/security/groups:go_default_library", + "//vendor/github.com/gophercloud/gophercloud/openstack/networking/v2/ports:go_default_library", + ], +) diff --git a/upup/pkg/fi/cloudup/openstacktasks/instance.go b/upup/pkg/fi/cloudup/openstacktasks/instance.go index dc96c776d0..b197d821f7 100644 --- a/upup/pkg/fi/cloudup/openstacktasks/instance.go +++ b/upup/pkg/fi/cloudup/openstacktasks/instance.go @@ -42,6 +42,7 @@ type Instance struct { UserData *string Metadata map[string]string AvailabilityZone *string + SecurityGroups []string Lifecycle *fi.Lifecycle } @@ -161,8 +162,9 @@ func (_ *Instance) RenderOpenstack(t *openstack.OpenstackAPITarget, a, e, change Port: fi.StringValue(e.Port.ID), }, }, - Metadata: e.Metadata, - ServiceClient: t.Cloud.ComputeClient(), + Metadata: e.Metadata, + ServiceClient: t.Cloud.ComputeClient(), + SecurityGroups: e.SecurityGroups, } if e.UserData != nil { opt.UserData = []byte(*e.UserData) diff --git a/upup/pkg/fi/cloudup/openstacktasks/port.go b/upup/pkg/fi/cloudup/openstacktasks/port.go index e12abb733a..e28f2743e1 100644 --- a/upup/pkg/fi/cloudup/openstacktasks/port.go +++ b/upup/pkg/fi/cloudup/openstacktasks/port.go @@ -19,6 +19,7 @@ package openstacktasks import ( "fmt" + secgroup "github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/security/groups" "github.com/gophercloud/gophercloud/openstack/networking/v2/ports" "k8s.io/klog" "k8s.io/kops/upup/pkg/fi" @@ -27,12 +28,13 @@ import ( //go:generate fitask -type=Port type Port struct { - ID *string - Name *string - Network *Network - Subnets []*Subnet - SecurityGroups []*SecurityGroup - Lifecycle *fi.Lifecycle + ID *string + Name *string + Network *Network + Subnets []*Subnet + SecurityGroups []*SecurityGroup + AdditionalSecurityGroups []string + Lifecycle *fi.Lifecycle } // GetDependencies returns the dependencies of the Port task @@ -59,12 +61,31 @@ func (s *Port) CompareWithID() *string { } func NewPortTaskFromCloud(cloud openstack.OpenstackCloud, lifecycle *fi.Lifecycle, port *ports.Port, find *Port) (*Port, error) { - sgs := make([]*SecurityGroup, len(port.SecurityGroups)) - for i, sgid := range port.SecurityGroups { - sgs[i] = &SecurityGroup{ + additionalSecurityGroupIDs := map[string]struct{}{} + if find != nil { + for _, sg := range find.AdditionalSecurityGroups { + opt := secgroup.ListOpts{ + Name: sg, + } + gs, err := cloud.ListSecurityGroups(opt) + if err != nil { + continue + } + if len(gs) == 0 { + continue + } + additionalSecurityGroupIDs[gs[0].ID] = struct{}{} + } + } + sgs := []*SecurityGroup{} + for _, sgid := range port.SecurityGroups { + if _, ok := additionalSecurityGroupIDs[sgid]; ok { + continue + } + sgs = append(sgs, &SecurityGroup{ ID: fi.String(sgid), Lifecycle: lifecycle, - } + }) } subnets := make([]*Subnet, len(port.FixedIPs)) for i, subn := range port.FixedIPs { @@ -84,6 +105,7 @@ func NewPortTaskFromCloud(cloud openstack.OpenstackCloud, lifecycle *fi.Lifecycl } if find != nil { find.ID = actual.ID + actual.AdditionalSecurityGroups = find.AdditionalSecurityGroups } return actual, nil } @@ -129,26 +151,13 @@ func (_ *Port) CheckChanges(a, e, changes *Port) error { return nil } -func (_ *Port) RenderOpenstack(t *openstack.OpenstackAPITarget, a, e, changes *Port) error { +func (*Port) RenderOpenstack(t *openstack.OpenstackAPITarget, a, e, changes *Port) error { if a == nil { klog.V(2).Infof("Creating Port with name: %q", fi.StringValue(e.Name)) - sgs := make([]string, len(e.SecurityGroups)) - for i, sg := range e.SecurityGroups { - sgs[i] = fi.StringValue(sg.ID) - } - fixedIPs := make([]ports.IP, len(e.Subnets)) - for i, subn := range e.Subnets { - fixedIPs[i] = ports.IP{ - SubnetID: fi.StringValue(subn.ID), - } - } - - opt := ports.CreateOpts{ - Name: fi.StringValue(e.Name), - NetworkID: fi.StringValue(e.Network.ID), - SecurityGroups: &sgs, - FixedIPs: fixedIPs, + opt, err := portCreateOptsFromPortTask(a, e, changes) + if err != nil { + return fmt.Errorf("Error creating port cloud opts: %v", err) } v, err := t.Cloud.CreatePort(opt) @@ -164,3 +173,26 @@ func (_ *Port) RenderOpenstack(t *openstack.OpenstackAPITarget, a, e, changes *P klog.V(2).Infof("Using an existing Openstack port, id=%s", fi.StringValue(e.ID)) return nil } + +func portCreateOptsFromPortTask(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 + } + fixedIPs := make([]ports.IP, len(e.Subnets)) + for i, subn := range e.Subnets { + fixedIPs[i] = ports.IP{ + SubnetID: fi.StringValue(subn.ID), + } + } + + return ports.CreateOpts{ + Name: fi.StringValue(e.Name), + NetworkID: fi.StringValue(e.Network.ID), + SecurityGroups: &sgs, + FixedIPs: fixedIPs, + }, nil +} diff --git a/upup/pkg/fi/cloudup/openstacktasks/port_test.go b/upup/pkg/fi/cloudup/openstacktasks/port_test.go new file mode 100644 index 0000000000..580ae06579 --- /dev/null +++ b/upup/pkg/fi/cloudup/openstacktasks/port_test.go @@ -0,0 +1,659 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package openstacktasks + +import ( + "fmt" + "reflect" + "sort" + "testing" + + sg "github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/security/groups" + "github.com/gophercloud/gophercloud/openstack/networking/v2/ports" + "k8s.io/kops/upup/pkg/fi" + "k8s.io/kops/upup/pkg/fi/cloudup/openstack" +) + +func Test_Port_GetDependencies(t *testing.T) { + tasks := map[string]fi.Task{ + "foo": &SecurityGroup{Name: fi.String("security-group")}, + "bar": &Subnet{Name: fi.String("subnet")}, + "baz": &Instance{Name: fi.String("instance")}, + "qux": &FloatingIP{Name: fi.String("fip")}, + "xxx": &Network{Name: fi.String("network")}, + } + + port := &Port{} + + actual := port.GetDependencies(tasks) + + expected := []fi.Task{ + &Subnet{Name: fi.String("subnet")}, + &Network{Name: fi.String("network")}, + &SecurityGroup{Name: fi.String("security-group")}, + } + + actualSorted := sortedTasks(actual) + expectedSorted := sortedTasks(expected) + sort.Sort(actualSorted) + sort.Sort(expectedSorted) + + if !reflect.DeepEqual(expectedSorted, actualSorted) { + t.Errorf("Dependencies differ:\n%v\n\tinstead of\n%v", actualSorted, expectedSorted) + } +} + +func Test_NewPortTaskFromCloud(t *testing.T) { + syncLifecycle := fi.LifecycleSync + tests := []struct { + desc string + lifecycle fi.Lifecycle + cloud openstack.OpenstackCloud + cloudPort *ports.Port + foundPort *Port + modifiedFoundPort *Port + expectedPortTask *Port + expectedError error + }{ + { + desc: "empty cloud port found port nil", + lifecycle: fi.LifecycleSync, + cloud: &portCloud{}, + cloudPort: &ports.Port{}, + foundPort: nil, + modifiedFoundPort: nil, + expectedPortTask: &Port{ + ID: fi.String(""), + Name: fi.String(""), + Network: &Network{ID: fi.String("")}, + SecurityGroups: []*SecurityGroup{}, + Subnets: []*Subnet{}, + Lifecycle: &syncLifecycle, + }, + expectedError: nil, + }, + { + desc: "empty cloud port found port not nil", + lifecycle: fi.LifecycleSync, + cloud: &portCloud{}, + cloudPort: &ports.Port{}, + foundPort: &Port{}, + modifiedFoundPort: &Port{ID: fi.String("")}, + expectedPortTask: &Port{ + ID: fi.String(""), + Name: fi.String(""), + Network: &Network{ID: fi.String("")}, + SecurityGroups: []*SecurityGroup{}, + Subnets: []*Subnet{}, + Lifecycle: &syncLifecycle, + }, + expectedError: nil, + }, + { + desc: "fully populated cloud port found port not nil", + lifecycle: fi.LifecycleSync, + cloud: &portCloud{}, + cloudPort: &ports.Port{ + ID: "id", + Name: "name", + NetworkID: "networkID", + FixedIPs: []ports.IP{ + {SubnetID: "subnet-a"}, + {SubnetID: "subnet-b"}, + }, + SecurityGroups: []string{ + "sg-1", + "sg-2", + }, + }, + foundPort: &Port{}, + modifiedFoundPort: &Port{ID: fi.String("id")}, + expectedPortTask: &Port{ + ID: fi.String("id"), + Name: fi.String("name"), + Network: &Network{ID: fi.String("networkID")}, + SecurityGroups: []*SecurityGroup{ + {ID: fi.String("sg-1"), Lifecycle: &syncLifecycle}, + {ID: fi.String("sg-2"), Lifecycle: &syncLifecycle}, + }, + Subnets: []*Subnet{ + {ID: fi.String("subnet-a"), Lifecycle: &syncLifecycle}, + {ID: fi.String("subnet-b"), Lifecycle: &syncLifecycle}, + }, + Lifecycle: &syncLifecycle, + }, + expectedError: nil, + }, + { + desc: "cloud port found port not nil honors additional security groups", + lifecycle: fi.LifecycleSync, + cloud: &portCloud{ + listSecurityGroups: map[string][]sg.SecGroup{ + "add-1": { + {ID: "add-1", Name: "add-1"}, + }, + "add-2": { + {ID: "add-2", Name: "add-2"}, + }, + }, + }, + cloudPort: &ports.Port{ + ID: "id", + Name: "name", + NetworkID: "networkID", + FixedIPs: []ports.IP{ + {SubnetID: "subnet-a"}, + {SubnetID: "subnet-b"}, + }, + SecurityGroups: []string{ + "sg-1", + "sg-2", + "add-1", + "add-2", + }, + }, + foundPort: &Port{ + AdditionalSecurityGroups: []string{ + "add-1", + "add-2", + }, + }, + modifiedFoundPort: &Port{ + ID: fi.String("id"), + AdditionalSecurityGroups: []string{ + "add-1", + "add-2", + }, + }, + expectedPortTask: &Port{ + ID: fi.String("id"), + Name: fi.String("name"), + Network: &Network{ID: fi.String("networkID")}, + SecurityGroups: []*SecurityGroup{ + {ID: fi.String("sg-1"), Lifecycle: &syncLifecycle}, + {ID: fi.String("sg-2"), Lifecycle: &syncLifecycle}, + }, + AdditionalSecurityGroups: []string{ + "add-1", + "add-2", + }, + Subnets: []*Subnet{ + {ID: fi.String("subnet-a"), Lifecycle: &syncLifecycle}, + {ID: fi.String("subnet-b"), Lifecycle: &syncLifecycle}, + }, + Lifecycle: &syncLifecycle, + }, + expectedError: nil, + }, + } + + for _, testCase := range tests { + t.Run(testCase.desc, func(t *testing.T) { + actual, err := NewPortTaskFromCloud(testCase.cloud, &testCase.lifecycle, testCase.cloudPort, testCase.foundPort) + + if !reflect.DeepEqual(err, testCase.expectedError) { + t.Errorf("Error differs:\n%v\n\tinstead of\n%v", err, testCase.expectedError) + } + + if !reflect.DeepEqual(actual, testCase.expectedPortTask) { + t.Errorf("Port task differs:\n%v\n\tinstead of\n%v", actual, testCase.expectedPortTask) + } + + if !reflect.DeepEqual(testCase.foundPort, testCase.modifiedFoundPort) { + t.Errorf("Found Port task differs:\n%v\n\tinstead of\n%v", testCase.foundPort, testCase.modifiedFoundPort) + } + }) + } +} + +func Test_Port_Find(t *testing.T) { + syncLifecycle := fi.LifecycleSync + tests := []struct { + desc string + context *fi.Context + port *Port + expectedPortTask *Port + expectedError error + }{ + { + desc: "nothing found", + context: &fi.Context{ + Cloud: &portCloud{}, + }, + port: &Port{ + Name: fi.String("name"), + Lifecycle: &syncLifecycle, + }, + expectedPortTask: nil, + expectedError: nil, + }, + { + desc: "port found", + context: &fi.Context{ + Cloud: &portCloud{ + listPorts: []ports.Port{ + { + ID: "id", + Name: "name", + NetworkID: "networkID", + FixedIPs: []ports.IP{ + {SubnetID: "subnet-a"}, + {SubnetID: "subnet-b"}, + }, + SecurityGroups: []string{ + "sg-1", + "sg-2", + }, + }, + }, + }, + }, + port: &Port{ + Name: fi.String("name"), + Lifecycle: &syncLifecycle, + }, + expectedPortTask: &Port{ + ID: fi.String("id"), + Name: fi.String("name"), + Network: &Network{ID: fi.String("networkID")}, + SecurityGroups: []*SecurityGroup{ + {ID: fi.String("sg-1"), Lifecycle: &syncLifecycle}, + {ID: fi.String("sg-2"), Lifecycle: &syncLifecycle}, + }, + Subnets: []*Subnet{ + {ID: fi.String("subnet-a"), Lifecycle: &syncLifecycle}, + {ID: fi.String("subnet-b"), Lifecycle: &syncLifecycle}, + }, + Lifecycle: &syncLifecycle, + }, + expectedError: nil, + }, + { + desc: "multiple ports found", + context: &fi.Context{ + Cloud: &portCloud{ + listPorts: []ports.Port{ + { + ID: "id-1", + Name: "name", + }, + { + ID: "id-2", + Name: "name", + }, + }, + }, + }, + port: &Port{ + Name: fi.String("name"), + Lifecycle: &syncLifecycle, + }, + expectedPortTask: nil, + expectedError: fmt.Errorf("found multiple ports with name: name"), + }, + { + desc: "error listing ports", + context: &fi.Context{ + Cloud: &portCloud{ + listPorts: []ports.Port{ + { + ID: "id-1", + Name: "name", + }, + }, + listPortsError: fmt.Errorf("list error"), + }, + }, + port: &Port{ + Name: fi.String("name"), + Lifecycle: &syncLifecycle, + }, + expectedPortTask: nil, + expectedError: fmt.Errorf("list error"), + }, + } + + for _, testCase := range tests { + t.Run(testCase.desc, func(t *testing.T) { + actual, err := testCase.port.Find(testCase.context) + + if !reflect.DeepEqual(err, testCase.expectedError) { + t.Errorf("Error differs:\n%v\n\tinstead of\n%v", err, testCase.expectedError) + } + + if !reflect.DeepEqual(actual, testCase.expectedPortTask) { + t.Errorf("Port task differs:\n%v\n\tinstead of\n%v", actual, testCase.expectedPortTask) + } + }) + } +} + +func Test_Port_CheckChanges(t *testing.T) { + tests := []struct { + desc string + actual *Port + expected *Port + changes *Port + expectedError error + }{ + { + desc: "actual nil all required fields set", + actual: nil, + expected: &Port{ + Name: fi.String("name"), + Network: &Network{ID: fi.String("networkID")}, + }, + expectedError: nil, + }, + { + desc: "actual nil required field Name nil", + actual: nil, + expected: &Port{ + Name: nil, + Network: &Network{ID: fi.String("networkID")}, + }, + expectedError: fi.RequiredField("Name"), + }, + { + desc: "actual nil required field Network nil", + actual: nil, + expected: &Port{ + Name: fi.String("name"), + Network: nil, + }, + expectedError: fi.RequiredField("Network"), + }, + { + desc: "actual not nil all changeable fields set", + actual: &Port{ + Name: fi.String("name"), + Network: nil, + }, + expected: &Port{ + Name: fi.String("name"), + Network: nil, + }, + changes: &Port{ + Name: nil, + Network: &Network{ID: fi.String("networkID")}, + }, + expectedError: nil, + }, + { + desc: "actual not nil unchangeable field Name set", + actual: &Port{ + Name: fi.String("name"), + Network: nil, + }, + expected: &Port{ + Name: fi.String("name"), + Network: &Network{ID: fi.String("networkID")}, + }, + changes: &Port{ + Name: fi.String("name"), + Network: nil, + }, + expectedError: fi.CannotChangeField("Name"), + }, + { + desc: "actual not nil unchangeable field Network set", + actual: &Port{ + Name: fi.String("name"), + Network: nil, + }, + expected: &Port{ + Name: nil, + Network: &Network{ID: fi.String("networkID")}, + }, + changes: &Port{ + Name: nil, + Network: &Network{ID: fi.String("networkID")}, + }, + expectedError: fi.CannotChangeField("Network"), + }, + } + + for _, testCase := range tests { + t.Run(testCase.desc, func(t *testing.T) { + var port Port + err := (&port).CheckChanges(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) + } + }) + } +} + +func Test_Port_RenderOpenstack(t *testing.T) { + tests := []struct { + desc string + target *openstack.OpenstackAPITarget + actual *Port + expected *Port + changes *Port + expectedCloudPort *ports.Port + expectedAfter *Port + expectedError error + }{ + { + desc: "actual not nil", + actual: &Port{ + ID: fi.String("actual-id"), + Name: fi.String("name"), + Network: &Network{ID: fi.String("networkID")}, + }, + expected: &Port{ + ID: fi.String("expected-id"), + Name: fi.String("name"), + Network: &Network{ID: fi.String("networkID")}, + }, + expectedAfter: &Port{ + ID: fi.String("actual-id"), + Name: fi.String("name"), + Network: &Network{ID: fi.String("networkID")}, + }, + expectedCloudPort: nil, + expectedError: nil, + }, + { + desc: "actual nil success", + target: &openstack.OpenstackAPITarget{ + Cloud: &portCloud{ + createPort: &ports.Port{ + ID: "cloud-id", + Name: "name", + NetworkID: "networkID", + FixedIPs: []ports.IP{ + {SubnetID: "subnet-a"}, + {SubnetID: "subnet-b"}, + }, + SecurityGroups: []string{ + "sg-1", + "sg-2", + }, + }, + }, + }, + actual: nil, + 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")}, + }, + Subnets: []*Subnet{ + {ID: fi.String("subnet-a")}, + {ID: fi.String("subnet-b")}, + }, + }, + expectedAfter: &Port{ + ID: fi.String("cloud-id"), + Name: fi.String("name"), + Network: &Network{ID: fi.String("networkID")}, + SecurityGroups: []*SecurityGroup{ + {ID: fi.String("sg-1")}, + {ID: fi.String("sg-2")}, + }, + Subnets: []*Subnet{ + {ID: fi.String("subnet-a")}, + {ID: fi.String("subnet-b")}, + }, + }, + expectedCloudPort: &ports.Port{ + ID: "id", + Name: "name", + NetworkID: "networkID", + FixedIPs: []ports.IP{ + {SubnetID: "subnet-a"}, + {SubnetID: "subnet-b"}, + }, + SecurityGroups: []string{ + "sg-1", + "sg-2", + }, + }, + expectedError: nil, + }, + { + desc: "actual nil cloud error", + target: &openstack.OpenstackAPITarget{ + Cloud: &portCloud{ + createPortError: fmt.Errorf("port create error"), + }, + }, + actual: nil, + expected: &Port{ + ID: fi.String("expected-id"), + Name: fi.String("name"), + Network: &Network{ID: fi.String("networkID")}, + }, + expectedAfter: &Port{ + ID: fi.String("expected-id"), + Name: fi.String("name"), + Network: &Network{ID: fi.String("networkID")}, + }, + expectedCloudPort: nil, + expectedError: fmt.Errorf("Error creating port: port create error"), + }, + } + + for _, testCase := range tests { + t.Run(testCase.desc, func(t *testing.T) { + var port Port + err := (&port).RenderOpenstack(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) + } + + if !reflect.DeepEqual(testCase.expected, testCase.expectedAfter) { + t.Errorf("Expected Port task differs:\n%v\n\tinstead of\n%v", testCase.expected, testCase.expectedAfter) + } + }) + } +} + +func Test_Port_createOptsFromPortTask(t *testing.T) { + tests := []struct { + desc string + actual *Port + expected *Port + changes *Port + expectedCreateOpts ports.CreateOptsBuilder + expectedError error + }{ + { + desc: "all fields set", + 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-1", + "add-2", + }, + Subnets: []*Subnet{ + {ID: fi.String("subnet-a")}, + {ID: fi.String("subnet-b")}, + }, + }, + expectedCreateOpts: ports.CreateOpts{ + Name: "name", + NetworkID: "networkID", + SecurityGroups: &[]string{ + "sg-1", + "sg-2", + "add-1", + "add-2", + }, + FixedIPs: []ports.IP{ + {SubnetID: "subnet-a"}, + {SubnetID: "subnet-b"}, + }, + }, + }, + } + + for _, testCase := range tests { + t.Run(testCase.desc, func(t *testing.T) { + opts, err := portCreateOptsFromPortTask(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) + } + + if !reflect.DeepEqual(testCase.expectedCreateOpts, opts) { + t.Errorf("Port create opts differs:\n%v\n\tinstead of\n%v", opts, testCase.expectedCreateOpts) + } + }) + } +} + +type portCloud struct { + openstack.OpenstackCloud + listPorts []ports.Port + listPortsError error + createPort *ports.Port + createPortError error + listSecurityGroups map[string][]sg.SecGroup + listSecurityGroupsError error +} + +func (p *portCloud) ListPorts(opt ports.ListOptsBuilder) ([]ports.Port, error) { + return p.listPorts, p.listPortsError +} + +func (p *portCloud) CreatePort(opt ports.CreateOptsBuilder) (*ports.Port, error) { + return p.createPort, p.createPortError +} + +func (p *portCloud) ListSecurityGroups(opt sg.ListOpts) ([]sg.SecGroup, error) { + return p.listSecurityGroups[opt.Name], p.listSecurityGroupsError +} + +type sortedTasks []fi.Task + +func (s sortedTasks) Len() int { return len(s) } +func (s sortedTasks) Swap(i, j int) { s[i], s[j] = s[j], s[i] } +func (s sortedTasks) Less(i, j int) bool { return fmt.Sprintf("%v", s[i]) < fmt.Sprintf("%v", s[j]) }