Skip to content

Commit

Permalink
Deprecating and replacing securityGroups field from OpenStackMachineT…
Browse files Browse the repository at this point in the history 
…emplate ports

fixes: kubernetes-sigs#1251
  • Loading branch information
Anwar Hassen committed Jun 1, 2022
1 parent a22f805 commit 6b1a197
Show file tree
Hide file tree
Showing 12 changed files with 55 additions and 127 deletions.
19 changes: 19 additions & 0 deletions api/v1alpha4/conversion.go
View file
Expand Up @@ -198,6 +198,15 @@ func Convert_v1alpha4_PortOpts_To_v1alpha5_PortOpts(in *PortOpts, out *infrav1.P
if in.NetworkID != "" {
out.Network = &infrav1.NetworkFilter{ID: in.NetworkID}
}
// convert securityGroups
var v1alpha5securityGroups []infrav1.SecurityGroupParam
for _, v1alpha4SecurityGroupUID := range *in.SecurityGroups {
// Don't validate an explicit UUID if we were given one
if v1alpha4SecurityGroupUID != "" {
v1alpha5securityGroups = append(v1alpha5securityGroups, infrav1.SecurityGroupParam{UUID: v1alpha4SecurityGroupUID})
}
}
out.SecurityGroups = v1alpha5securityGroups
return nil
}

Expand All @@ -209,6 +218,16 @@ func Convert_v1alpha5_PortOpts_To_v1alpha4_PortOpts(in *infrav1.PortOpts, out *P
if in.Network != nil {
out.NetworkID = in.Network.ID
}
// convert securityGroups
var v1alpha4securityGroups []string
for _, v1alpha5SecurityGroups := range in.SecurityGroups {
// Don't validate an explicit UUID if we were given one
if v1alpha5SecurityGroups.UUID != "" {
v1alpha4securityGroups = append(v1alpha4securityGroups, v1alpha5SecurityGroups.UUID)
}
}
*out.SecurityGroups = v1alpha4securityGroups
// end of conversation
return nil
}

Expand Down
1 change: 0 additions & 1 deletion api/v1alpha4/conversion_test.go
View file
Expand Up @@ -264,7 +264,6 @@ func TestFuzzyConversion(t *testing.T) {
v1alpha5PortOpts.Network = nil
}
}
v1alpha5PortOpts.SecurityGroupFilters = nil
},
func(v1alpha5FixedIP *infrav1.FixedIP, c fuzz.Continue) {
c.FuzzNoCustom(v1alpha5FixedIP)
Expand Down
3 changes: 0 additions & 3 deletions api/v1alpha4/zz_generated.conversion.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 3 additions & 5 deletions api/v1alpha5/types.go
View file
Expand Up @@ -117,11 +117,9 @@ type PortOpts struct {
FixedIPs []FixedIP `json:"fixedIPs,omitempty"`
TenantID string `json:"tenantId,omitempty"`
ProjectID string `json:"projectId,omitempty"`
// The uuids of the security groups to assign to the instance
SecurityGroups *[]string `json:"securityGroups,omitempty"`
// The names, uuids, filters or any combination these of the security groups to assign to the instance
SecurityGroupFilters []SecurityGroupParam `json:"securityGroupFilters,omitempty"`
AllowedAddressPairs []AddressPair `json:"allowedAddressPairs,omitempty"`
// The names of the security groups to assign to the port
SecurityGroups []SecurityGroupParam `json:"securityGroups,omitempty"`
AllowedAddressPairs []AddressPair `json:"allowedAddressPairs,omitempty"`
// Enables and disables trunk at port level. If not provided, openStackMachine.Spec.Trunk is inherited.
Trunk *bool `json:"trunk,omitempty"`

Expand Down
9 changes: 0 additions & 9 deletions api/v1alpha5/zz_generated.deepcopy.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

48 changes: 12 additions & 36 deletions config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml
View file
Expand Up @@ -2779,9 +2779,9 @@ spec:
type: object
projectId:
type: string
securityGroupFilters:
description: The names, uuids, filters or any combination
these of the security groups to assign to the instance
securityGroups:
description: The names of the security groups to assign
to the port
items:
properties:
filter:
Expand Down Expand Up @@ -2823,12 +2823,6 @@ spec:
type: string
type: object
type: array
securityGroups:
description: The uuids of the security groups to assign
to the instance
items:
type: string
type: array
tags:
description: Tags applied to the port (and corresponding
trunk, if a trunk is configured.) These tags are applied
Expand Down Expand Up @@ -3298,9 +3292,9 @@ spec:
type: object
projectId:
type: string
securityGroupFilters:
description: The names, uuids, filters or any combination
these of the security groups to assign to the instance
securityGroups:
description: The names of the security groups to assign
to the port
items:
properties:
filter:
Expand Down Expand Up @@ -3342,12 +3336,6 @@ spec:
type: string
type: object
type: array
securityGroups:
description: The uuids of the security groups to assign
to the instance
items:
type: string
type: array
tags:
description: Tags applied to the port (and corresponding
trunk, if a trunk is configured.) These tags are applied
Expand Down Expand Up @@ -3678,9 +3666,9 @@ spec:
type: object
projectId:
type: string
securityGroupFilters:
description: The names, uuids, filters or any combination
these of the security groups to assign to the instance
securityGroups:
description: The names of the security groups to assign to
the port
items:
properties:
filter:
Expand Down Expand Up @@ -3722,12 +3710,6 @@ spec:
type: string
type: object
type: array
securityGroups:
description: The uuids of the security groups to assign to
the instance
items:
type: string
type: array
tags:
description: Tags applied to the port (and corresponding trunk,
if a trunk is configured.) These tags are applied in addition
Expand Down Expand Up @@ -3972,9 +3954,9 @@ spec:
type: object
projectId:
type: string
securityGroupFilters:
description: The names, uuids, filters or any combination
these of the security groups to assign to the instance
securityGroups:
description: The names of the security groups to assign to
the port
items:
properties:
filter:
Expand Down Expand Up @@ -4016,12 +3998,6 @@ spec:
type: string
type: object
type: array
securityGroups:
description: The uuids of the security groups to assign to
the instance
items:
type: string
type: array
tags:
description: Tags applied to the port (and corresponding trunk,
if a trunk is configured.) These tags are applied in addition
Expand Down
13 changes: 3 additions & 10 deletions config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml
View file
Expand Up @@ -1049,10 +1049,9 @@ spec:
type: object
projectId:
type: string
securityGroupFilters:
description: The names, uuids, filters or any
combination these of the security groups to
assign to the instance
securityGroups:
description: The names of the security groups
to assign to the port
items:
properties:
filter:
Expand Down Expand Up @@ -1094,12 +1093,6 @@ spec:
type: string
type: object
type: array
securityGroups:
description: The uuids of the security groups
to assign to the instance
items:
type: string
type: array
tags:
description: Tags applied to the port (and corresponding
trunk, if a trunk is configured.) These tags
Expand Down
12 changes: 3 additions & 9 deletions config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachines.yaml
View file
Expand Up @@ -1044,9 +1044,9 @@ spec:
type: object
projectId:
type: string
securityGroupFilters:
description: The names, uuids, filters or any combination these
of the security groups to assign to the instance
securityGroups:
description: The names of the security groups to assign to the
port
items:
properties:
filter:
Expand Down Expand Up @@ -1088,12 +1088,6 @@ spec:
type: string
type: object
type: array
securityGroups:
description: The uuids of the security groups to assign to the
instance
items:
type: string
type: array
tags:
description: Tags applied to the port (and corresponding trunk,
if a trunk is configured.) These tags are applied in addition
Expand Down
12 changes: 3 additions & 9 deletions config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachinetemplates.yaml
View file
Expand Up @@ -951,9 +951,9 @@ spec:
type: object
projectId:
type: string
securityGroupFilters:
description: The names, uuids, filters or any combination
these of the security groups to assign to the instance
securityGroups:
description: The names of the security groups to assign
to the port
items:
properties:
filter:
Expand Down Expand Up @@ -995,12 +995,6 @@ spec:
type: string
type: object
type: array
securityGroups:
description: The uuids of the security groups to assign
to the instance
items:
type: string
type: array
tags:
description: Tags applied to the port (and corresponding
trunk, if a trunk is configured.) These tags are applied
Expand Down
49 changes: 8 additions & 41 deletions pkg/cloud/services/networking/port.go
View file
Expand Up @@ -89,9 +89,14 @@ func (s *Service) GetOrCreatePort(eventObject runtime.Object, clusterName string
MACAddress: ap.MACAddress,
})
}
securityGroups, err = s.CollectPortSecurityGroups(eventObject, portOpts.SecurityGroups, portOpts.SecurityGroupFilters)
if err != nil {
return nil, err
if len(portOpts.SecurityGroups) > 0 {
portSecurityGroups, err := s.GetSecurityGroups(portOpts.SecurityGroups)
if err != nil {
return nil, fmt.Errorf("error getting security groups: %v", err)
}
if len(portSecurityGroups) > 0 {
*securityGroups = portSecurityGroups
}
}
// inherit port security groups from the instance if not explicitly specified
if securityGroups == nil || len(*securityGroups) == 0 {
Expand Down Expand Up @@ -258,41 +263,3 @@ func (s *Service) GarbageCollectErrorInstancesPort(eventObject runtime.Object, i

return nil
}

// CollectPortSecurityGroups collects distinct securityGroups from port.SecurityGroups and port.SecurityGroupFilter fields.
func (s *Service) CollectPortSecurityGroups(eventObject runtime.Object, portSecurityGroups *[]string, portSecurityGroupFilters []infrav1.SecurityGroupParam) (*[]string, error) {
var allSecurityGroupIDs []string
// security groups provided with the portSecurityGroupFilters fields
securityGroupFiltersByID, err := s.GetSecurityGroups(portSecurityGroupFilters)
if err != nil {
return portSecurityGroups, fmt.Errorf("error getting security groups: %v", err)
}
allSecurityGroupIDs = append(allSecurityGroupIDs, securityGroupFiltersByID...)
securityGroupCount := 0
// security groups provided with the portSecurityGroups fields
if portSecurityGroups != nil {
allSecurityGroupIDs = append(allSecurityGroupIDs, *portSecurityGroups...)
}
// generate unique values
uids := make(map[string]int)
for _, sg := range allSecurityGroupIDs {
if sg == "" {
continue
}
// count distinct values
_, ok := uids[sg]
if !ok {
securityGroupCount++
}
uids[sg] = 1
}
distinctSecurityGroupIDs := make([]string, 0, securityGroupCount)
// collect distict values
for key := range uids {
if key == "" {
continue
}
distinctSecurityGroupIDs = append(distinctSecurityGroupIDs, key)
}
return &distinctSecurityGroupIDs, nil
}
6 changes: 3 additions & 3 deletions pkg/cloud/services/networking/port_test.go
View file
Expand Up @@ -50,7 +50,7 @@ func Test_GetOrCreatePort(t *testing.T) {
// Other arbitrary variables passed in to the tests
instanceSecurityGroups := []string{"instance-secgroup"}
portSecurityGroups := []string{"port-secgroup"}

portSecurityGroupsPortOpts := []infrav1.SecurityGroupParam{{Name: "port-secgroup"}}
pointerToTrue := pointerTo(true)
pointerToFalse := pointerTo(false)

Expand Down Expand Up @@ -168,7 +168,7 @@ func Test_GetOrCreatePort(t *testing.T) {
}, {IPAddress: "192.168.1.50"}},
TenantID: tenantID,
ProjectID: projectID,
SecurityGroups: &portSecurityGroups,
SecurityGroups: portSecurityGroupsPortOpts,
AllowedAddressPairs: []infrav1.AddressPair{{
IPAddress: "10.10.10.10",
MACAddress: "f1:f1:f1:f1:f1:f1",
Expand Down Expand Up @@ -296,7 +296,7 @@ func Test_GetOrCreatePort(t *testing.T) {
infrav1.Network{
ID: netID,
PortOpts: &infrav1.PortOpts{
SecurityGroups: &portSecurityGroups,
SecurityGroups: portSecurityGroupsPortOpts,
},
},
&instanceSecurityGroups,
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/suites/e2e/e2e_test.go
View file
Expand Up @@ -226,7 +226,7 @@ var _ = Describe("e2e tests", func() {
Trunk: pointer.Bool(true),
},
{
SecurityGroupFilters: []infrav1.SecurityGroupParam{{Name: testSecurityGroupName}},
SecurityGroups: []infrav1.SecurityGroupParam{{Name: testSecurityGroupName}},
},
}

Expand Down

0 comments on commit 6b1a197

Please sign in to comment.