diff --git a/api/v1alpha5/conversion.go b/api/v1alpha5/conversion.go
index ed0bbe4571..e01b164813 100644
--- a/api/v1alpha5/conversion.go
+++ b/api/v1alpha5/conversion.go
@@ -687,55 +687,12 @@ func Convert_v1alpha5_Bastion_To_v1beta1_Bastion(in *Bastion, out *infrav1.Basti
func Convert_v1beta1_SecurityGroupStatus_To_v1alpha5_SecurityGroup(in *infrav1.SecurityGroupStatus, out *SecurityGroup, s conversion.Scope) error { //nolint:revive
out.ID = in.ID
out.Name = in.Name
- out.Rules = make([]SecurityGroupRule, len(in.Rules))
- for i, rule := range in.Rules {
- out.Rules[i] = SecurityGroupRule{
- ID: rule.ID,
- Direction: rule.Direction,
- }
- if rule.Description != nil {
- out.Rules[i].Description = *rule.Description
- }
- if rule.EtherType != nil {
- out.Rules[i].EtherType = *rule.EtherType
- }
- if rule.PortRangeMin != nil {
- out.Rules[i].PortRangeMin = *rule.PortRangeMin
- }
- if rule.PortRangeMax != nil {
- out.Rules[i].PortRangeMax = *rule.PortRangeMax
- }
- if rule.Protocol != nil {
- out.Rules[i].Protocol = *rule.Protocol
- }
- if rule.RemoteGroupID != nil {
- out.Rules[i].RemoteGroupID = *rule.RemoteGroupID
- }
- if rule.RemoteIPPrefix != nil {
- out.Rules[i].RemoteIPPrefix = *rule.RemoteIPPrefix
- }
- }
return nil
}
func Convert_v1alpha5_SecurityGroup_To_v1beta1_SecurityGroupStatus(in *SecurityGroup, out *infrav1.SecurityGroupStatus, s conversion.Scope) error { //nolint:revive
out.ID = in.ID
out.Name = in.Name
- out.Rules = make([]infrav1.SecurityGroupRuleStatus, len(in.Rules))
- for i, rule := range in.Rules {
- out.Rules[i] = infrav1.SecurityGroupRuleStatus{
- ID: rule.ID,
- Description: pointer.String(rule.Description),
- Direction: rule.Direction,
- EtherType: pointer.String(rule.EtherType),
- PortRangeMin: pointer.Int(rule.PortRangeMin),
- PortRangeMax: pointer.Int(rule.PortRangeMax),
- Protocol: pointer.String(rule.Protocol),
- RemoteGroupID: pointer.String(rule.RemoteGroupID),
- RemoteIPPrefix: pointer.String(rule.RemoteIPPrefix),
- }
- }
-
return nil
}
diff --git a/api/v1alpha6/types_conversion.go b/api/v1alpha6/types_conversion.go
index c43f8a23ac..6ec354ecee 100644
--- a/api/v1alpha6/types_conversion.go
+++ b/api/v1alpha6/types_conversion.go
@@ -364,63 +364,18 @@ func restorev1alpha6SecurityGroup(previous *SecurityGroup, dst *SecurityGroup) {
return
}
- for i, rule := range previous.Rules {
- dst.Rules[i].SecurityGroupID = rule.SecurityGroupID
- }
+ dst.Rules = previous.Rules
}
func Convert_v1beta1_SecurityGroupStatus_To_v1alpha6_SecurityGroup(in *infrav1.SecurityGroupStatus, out *SecurityGroup, _ apiconversion.Scope) error {
out.ID = in.ID
out.Name = in.Name
- out.Rules = make([]SecurityGroupRule, len(in.Rules))
- for i, rule := range in.Rules {
- out.Rules[i] = SecurityGroupRule{
- ID: rule.ID,
- Direction: rule.Direction,
- }
- if rule.Description != nil {
- out.Rules[i].Description = *rule.Description
- }
- if rule.EtherType != nil {
- out.Rules[i].EtherType = *rule.EtherType
- }
- if rule.PortRangeMin != nil {
- out.Rules[i].PortRangeMin = *rule.PortRangeMin
- }
- if rule.PortRangeMax != nil {
- out.Rules[i].PortRangeMax = *rule.PortRangeMax
- }
- if rule.Protocol != nil {
- out.Rules[i].Protocol = *rule.Protocol
- }
- if rule.RemoteGroupID != nil {
- out.Rules[i].RemoteGroupID = *rule.RemoteGroupID
- }
- if rule.RemoteIPPrefix != nil {
- out.Rules[i].RemoteIPPrefix = *rule.RemoteIPPrefix
- }
- }
return nil
}
func Convert_v1alpha6_SecurityGroup_To_v1beta1_SecurityGroupStatus(in *SecurityGroup, out *infrav1.SecurityGroupStatus, _ apiconversion.Scope) error {
out.ID = in.ID
out.Name = in.Name
- out.Rules = make([]infrav1.SecurityGroupRuleStatus, len(in.Rules))
- for i, rule := range in.Rules {
- out.Rules[i] = infrav1.SecurityGroupRuleStatus{
- ID: rule.ID,
- Description: pointer.String(rule.Description),
- Direction: rule.Direction,
- EtherType: pointer.String(rule.EtherType),
- PortRangeMin: pointer.Int(rule.PortRangeMin),
- PortRangeMax: pointer.Int(rule.PortRangeMax),
- Protocol: pointer.String(rule.Protocol),
- RemoteGroupID: pointer.String(rule.RemoteGroupID),
- RemoteIPPrefix: pointer.String(rule.RemoteIPPrefix),
- }
- }
-
return nil
}
diff --git a/api/v1alpha7/openstackcluster_conversion.go b/api/v1alpha7/openstackcluster_conversion.go
index cd31d29f7f..6f1f88e923 100644
--- a/api/v1alpha7/openstackcluster_conversion.go
+++ b/api/v1alpha7/openstackcluster_conversion.go
@@ -346,10 +346,6 @@ func restorev1alpha7ClusterStatus(previous *OpenStackClusterStatus, dst *OpenSta
}
func restorev1beta1ClusterStatus(previous *infrav1.OpenStackClusterStatus, dst *infrav1.OpenStackClusterStatus) {
- restorev1beta1SecurityGroupStatus(previous.ControlPlaneSecurityGroup, dst.ControlPlaneSecurityGroup)
- restorev1beta1SecurityGroupStatus(previous.WorkerSecurityGroup, dst.WorkerSecurityGroup)
- restorev1beta1SecurityGroupStatus(previous.BastionSecurityGroup, dst.BastionSecurityGroup)
-
// ReferencedResources have no equivalent in v1alpha7
if previous.Bastion != nil {
dst.Bastion.ReferencedResources = previous.Bastion.ReferencedResources
diff --git a/api/v1alpha7/types_conversion.go b/api/v1alpha7/types_conversion.go
index 7b966d1320..3c3d0629f2 100644
--- a/api/v1alpha7/types_conversion.go
+++ b/api/v1alpha7/types_conversion.go
@@ -18,7 +18,6 @@ package v1alpha7
import (
apiconversion "k8s.io/apimachinery/pkg/conversion"
- "k8s.io/utils/pointer"
infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1"
"sigs.k8s.io/cluster-api-provider-openstack/pkg/utils/optional"
@@ -40,9 +39,7 @@ func restorev1alpha7SecurityGroup(previous *SecurityGroup, dst *SecurityGroup) {
return
}
- for i, rule := range previous.Rules {
- dst.Rules[i].SecurityGroupID = rule.SecurityGroupID
- }
+ dst.Rules = previous.Rules
}
func Convert_v1alpha7_SecurityGroupFilter_To_v1beta1_SecurityGroupFilter(in *SecurityGroupFilter, out *infrav1.SecurityGroupFilter, s apiconversion.Scope) error {
@@ -142,39 +139,6 @@ func Convert_v1beta1_RouterFilter_To_v1alpha7_RouterFilter(in *infrav1.RouterFil
return nil
}
-func restorev1beta1SecurityGroupStatus(previous *infrav1.SecurityGroupStatus, dst *infrav1.SecurityGroupStatus) {
- if previous == nil || dst == nil {
- return
- }
-
- for i := range dst.Rules {
- dstRule := &dst.Rules[i]
-
- // Conversion from scalar to *scalar is lossy for zero values. We need to restore only nil values.
- if dstRule.Description != nil && *dstRule.Description == "" {
- dstRule.Description = previous.Rules[i].Description
- }
- if dstRule.EtherType != nil && *dstRule.EtherType == "" {
- dstRule.EtherType = previous.Rules[i].EtherType
- }
- if dstRule.PortRangeMin != nil && *dstRule.PortRangeMin == 0 {
- dstRule.PortRangeMin = previous.Rules[i].PortRangeMin
- }
- if dstRule.PortRangeMax != nil && *dstRule.PortRangeMax == 0 {
- dstRule.PortRangeMax = previous.Rules[i].PortRangeMax
- }
- if dstRule.Protocol != nil && *dstRule.Protocol == "" {
- dstRule.Protocol = previous.Rules[i].Protocol
- }
- if dstRule.RemoteGroupID != nil && *dstRule.RemoteGroupID == "" {
- dstRule.RemoteGroupID = previous.Rules[i].RemoteGroupID
- }
- if dstRule.RemoteIPPrefix != nil && *dstRule.RemoteIPPrefix == "" {
- dstRule.RemoteIPPrefix = previous.Rules[i].RemoteIPPrefix
- }
- }
-}
-
/* PortOpts */
func restorev1alpha7Port(previous *PortOpts, dst *PortOpts) {
@@ -300,20 +264,6 @@ func Convert_v1beta1_PortOpts_To_v1alpha7_PortOpts(in *infrav1.PortOpts, out *Po
func Convert_v1alpha7_SecurityGroup_To_v1beta1_SecurityGroupStatus(in *SecurityGroup, out *infrav1.SecurityGroupStatus, _ apiconversion.Scope) error {
out.ID = in.ID
out.Name = in.Name
- out.Rules = make([]infrav1.SecurityGroupRuleStatus, len(in.Rules))
- for i, rule := range in.Rules {
- out.Rules[i] = infrav1.SecurityGroupRuleStatus{
- ID: rule.ID,
- Description: pointer.String(rule.Description),
- Direction: rule.Direction,
- EtherType: pointer.String(rule.EtherType),
- PortRangeMin: pointer.Int(rule.PortRangeMin),
- PortRangeMax: pointer.Int(rule.PortRangeMax),
- Protocol: pointer.String(rule.Protocol),
- RemoteGroupID: pointer.String(rule.RemoteGroupID),
- RemoteIPPrefix: pointer.String(rule.RemoteIPPrefix),
- }
- }
return nil
}
@@ -321,34 +271,6 @@ func Convert_v1alpha7_SecurityGroup_To_v1beta1_SecurityGroupStatus(in *SecurityG
func Convert_v1beta1_SecurityGroupStatus_To_v1alpha7_SecurityGroup(in *infrav1.SecurityGroupStatus, out *SecurityGroup, _ apiconversion.Scope) error {
out.ID = in.ID
out.Name = in.Name
- out.Rules = make([]SecurityGroupRule, len(in.Rules))
- for i, rule := range in.Rules {
- out.Rules[i] = SecurityGroupRule{
- ID: rule.ID,
- Direction: rule.Direction,
- }
- if rule.Description != nil {
- out.Rules[i].Description = *rule.Description
- }
- if rule.EtherType != nil {
- out.Rules[i].EtherType = *rule.EtherType
- }
- if rule.PortRangeMin != nil {
- out.Rules[i].PortRangeMin = *rule.PortRangeMin
- }
- if rule.PortRangeMax != nil {
- out.Rules[i].PortRangeMax = *rule.PortRangeMax
- }
- if rule.Protocol != nil {
- out.Rules[i].Protocol = *rule.Protocol
- }
- if rule.RemoteGroupID != nil {
- out.Rules[i].RemoteGroupID = *rule.RemoteGroupID
- }
- if rule.RemoteIPPrefix != nil {
- out.Rules[i].RemoteIPPrefix = *rule.RemoteIPPrefix
- }
- }
return nil
}
diff --git a/api/v1beta1/types.go b/api/v1beta1/types.go
index 94625b20db..b7950d294e 100644
--- a/api/v1beta1/types.go
+++ b/api/v1beta1/types.go
@@ -448,10 +448,6 @@ type SecurityGroupStatus struct {
// id of the security group
// +kubebuilder:validation:Required
ID string `json:"id"`
-
- // list of security group rules
- // +optional
- Rules []SecurityGroupRuleStatus `json:"rules,omitempty"`
}
// SecurityGroupRuleSpec represent the basic information of the associated OpenStack
@@ -514,55 +510,6 @@ type SecurityGroupRuleSpec struct {
RemoteManagedGroups []ManagedSecurityGroupName `json:"remoteManagedGroups,omitempty"`
}
-type SecurityGroupRuleStatus struct {
- // id of the security group rule
- // +kubebuilder:validation:Required
- ID string `json:"id"`
-
- // description of the security group rule.
- // +optional
- Description *string `json:"description,omitempty"`
-
- // direction in which the security group rule is applied. The only values
- // allowed are "ingress" or "egress". For a compute instance, an ingress
- // security group rule is applied to incoming (ingress) traffic for that
- // instance. An egress rule is applied to traffic leaving the instance.
- // +kubebuilder:validation:Required
- // +kubebuilder:validation:enum=ingress;egress
- Direction string `json:"direction"`
-
- // etherType must be IPv4 or IPv6, and addresses represented in CIDR must match the
- // ingress or egress rules.
- // +kubebuilder:validation:enum=IPv4;IPv6
- // +optional
- EtherType *string `json:"etherType,omitempty"`
-
- // portRangeMin is a number in the range that is matched by the security group
- // rule. If the protocol is TCP or UDP, this value must be less than or equal
- // to the value of the portRangeMax attribute.
- // +optional
- PortRangeMin *int `json:"portRangeMin,omitempty"`
-
- // portRangeMax is a number in the range that is matched by the security group
- // rule. The portRangeMin attribute constrains the portRangeMax attribute.
- // +optional
- PortRangeMax *int `json:"portRangeMax,omitempty"`
-
- // protocol is the protocol that is matched by the security group rule.
- // +optional
- Protocol *string `json:"protocol,omitempty"`
-
- // remoteGroupID is the remote group ID to be associated with this security group rule.
- // You can specify either remoteGroupID or remoteIPPrefix or remoteManagedGroups.
- // +optional
- RemoteGroupID *string `json:"remoteGroupID,omitempty"`
-
- // remoteIPPrefix is the remote IP prefix to be associated with this security group rule.
- // You can specify either remoteGroupID or remoteIPPrefix or remoteManagedGroups.
- // +optional
- RemoteIPPrefix *string `json:"remoteIPPrefix,omitempty"`
-}
-
// +kubebuilder:validation:Enum=bastion;controlplane;worker
type ManagedSecurityGroupName string
diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go
index 73e1fb8368..9635d65ab9 100644
--- a/api/v1beta1/zz_generated.deepcopy.go
+++ b/api/v1beta1/zz_generated.deepcopy.go
@@ -652,17 +652,17 @@ func (in *OpenStackClusterStatus) DeepCopyInto(out *OpenStackClusterStatus) {
if in.ControlPlaneSecurityGroup != nil {
in, out := &in.ControlPlaneSecurityGroup, &out.ControlPlaneSecurityGroup
*out = new(SecurityGroupStatus)
- (*in).DeepCopyInto(*out)
+ **out = **in
}
if in.WorkerSecurityGroup != nil {
in, out := &in.WorkerSecurityGroup, &out.WorkerSecurityGroup
*out = new(SecurityGroupStatus)
- (*in).DeepCopyInto(*out)
+ **out = **in
}
if in.BastionSecurityGroup != nil {
in, out := &in.BastionSecurityGroup, &out.BastionSecurityGroup
*out = new(SecurityGroupStatus)
- (*in).DeepCopyInto(*out)
+ **out = **in
}
if in.Bastion != nil {
in, out := &in.Bastion, &out.Bastion
@@ -1336,66 +1336,9 @@ func (in *SecurityGroupRuleSpec) DeepCopy() *SecurityGroupRuleSpec {
return out
}
-// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
-func (in *SecurityGroupRuleStatus) DeepCopyInto(out *SecurityGroupRuleStatus) {
- *out = *in
- if in.Description != nil {
- in, out := &in.Description, &out.Description
- *out = new(string)
- **out = **in
- }
- if in.EtherType != nil {
- in, out := &in.EtherType, &out.EtherType
- *out = new(string)
- **out = **in
- }
- if in.PortRangeMin != nil {
- in, out := &in.PortRangeMin, &out.PortRangeMin
- *out = new(int)
- **out = **in
- }
- if in.PortRangeMax != nil {
- in, out := &in.PortRangeMax, &out.PortRangeMax
- *out = new(int)
- **out = **in
- }
- if in.Protocol != nil {
- in, out := &in.Protocol, &out.Protocol
- *out = new(string)
- **out = **in
- }
- if in.RemoteGroupID != nil {
- in, out := &in.RemoteGroupID, &out.RemoteGroupID
- *out = new(string)
- **out = **in
- }
- if in.RemoteIPPrefix != nil {
- in, out := &in.RemoteIPPrefix, &out.RemoteIPPrefix
- *out = new(string)
- **out = **in
- }
-}
-
-// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SecurityGroupRuleStatus.
-func (in *SecurityGroupRuleStatus) DeepCopy() *SecurityGroupRuleStatus {
- if in == nil {
- return nil
- }
- out := new(SecurityGroupRuleStatus)
- in.DeepCopyInto(out)
- return out
-}
-
// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
func (in *SecurityGroupStatus) DeepCopyInto(out *SecurityGroupStatus) {
*out = *in
- if in.Rules != nil {
- in, out := &in.Rules, &out.Rules
- *out = make([]SecurityGroupRuleStatus, len(*in))
- for i := range *in {
- (*in)[i].DeepCopyInto(&(*out)[i])
- }
- }
}
// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SecurityGroupStatus.
diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml
index f2e491f4b3..30ae084647 100644
--- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml
+++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml
@@ -6651,58 +6651,6 @@ spec:
name:
description: name of the security group
type: string
- rules:
- description: list of security group rules
- items:
- properties:
- description:
- description: description of the security group rule.
- type: string
- direction:
- description: |-
- direction in which the security group rule is applied. The only values
- allowed are "ingress" or "egress". For a compute instance, an ingress
- security group rule is applied to incoming (ingress) traffic for that
- instance. An egress rule is applied to traffic leaving the instance.
- type: string
- etherType:
- description: |-
- etherType must be IPv4 or IPv6, and addresses represented in CIDR must match the
- ingress or egress rules.
- type: string
- id:
- description: id of the security group rule
- type: string
- portRangeMax:
- description: |-
- portRangeMax is a number in the range that is matched by the security group
- rule. The portRangeMin attribute constrains the portRangeMax attribute.
- type: integer
- portRangeMin:
- description: |-
- portRangeMin is a number in the range that is matched by the security group
- rule. If the protocol is TCP or UDP, this value must be less than or equal
- to the value of the portRangeMax attribute.
- type: integer
- protocol:
- description: protocol is the protocol that is matched by
- the security group rule.
- type: string
- remoteGroupID:
- description: |-
- remoteGroupID is the remote group ID to be associated with this security group rule.
- You can specify either remoteGroupID or remoteIPPrefix or remoteManagedGroups.
- type: string
- remoteIPPrefix:
- description: |-
- remoteIPPrefix is the remote IP prefix to be associated with this security group rule.
- You can specify either remoteGroupID or remoteIPPrefix or remoteManagedGroups.
- type: string
- required:
- - direction
- - id
- type: object
- type: array
required:
- id
- name
@@ -6719,58 +6667,6 @@ spec:
name:
description: name of the security group
type: string
- rules:
- description: list of security group rules
- items:
- properties:
- description:
- description: description of the security group rule.
- type: string
- direction:
- description: |-
- direction in which the security group rule is applied. The only values
- allowed are "ingress" or "egress". For a compute instance, an ingress
- security group rule is applied to incoming (ingress) traffic for that
- instance. An egress rule is applied to traffic leaving the instance.
- type: string
- etherType:
- description: |-
- etherType must be IPv4 or IPv6, and addresses represented in CIDR must match the
- ingress or egress rules.
- type: string
- id:
- description: id of the security group rule
- type: string
- portRangeMax:
- description: |-
- portRangeMax is a number in the range that is matched by the security group
- rule. The portRangeMin attribute constrains the portRangeMax attribute.
- type: integer
- portRangeMin:
- description: |-
- portRangeMin is a number in the range that is matched by the security group
- rule. If the protocol is TCP or UDP, this value must be less than or equal
- to the value of the portRangeMax attribute.
- type: integer
- protocol:
- description: protocol is the protocol that is matched by
- the security group rule.
- type: string
- remoteGroupID:
- description: |-
- remoteGroupID is the remote group ID to be associated with this security group rule.
- You can specify either remoteGroupID or remoteIPPrefix or remoteManagedGroups.
- type: string
- remoteIPPrefix:
- description: |-
- remoteIPPrefix is the remote IP prefix to be associated with this security group rule.
- You can specify either remoteGroupID or remoteIPPrefix or remoteManagedGroups.
- type: string
- required:
- - direction
- - id
- type: object
- type: array
required:
- id
- name
@@ -6926,58 +6822,6 @@ spec:
name:
description: name of the security group
type: string
- rules:
- description: list of security group rules
- items:
- properties:
- description:
- description: description of the security group rule.
- type: string
- direction:
- description: |-
- direction in which the security group rule is applied. The only values
- allowed are "ingress" or "egress". For a compute instance, an ingress
- security group rule is applied to incoming (ingress) traffic for that
- instance. An egress rule is applied to traffic leaving the instance.
- type: string
- etherType:
- description: |-
- etherType must be IPv4 or IPv6, and addresses represented in CIDR must match the
- ingress or egress rules.
- type: string
- id:
- description: id of the security group rule
- type: string
- portRangeMax:
- description: |-
- portRangeMax is a number in the range that is matched by the security group
- rule. The portRangeMin attribute constrains the portRangeMax attribute.
- type: integer
- portRangeMin:
- description: |-
- portRangeMin is a number in the range that is matched by the security group
- rule. If the protocol is TCP or UDP, this value must be less than or equal
- to the value of the portRangeMax attribute.
- type: integer
- protocol:
- description: protocol is the protocol that is matched by
- the security group rule.
- type: string
- remoteGroupID:
- description: |-
- remoteGroupID is the remote group ID to be associated with this security group rule.
- You can specify either remoteGroupID or remoteIPPrefix or remoteManagedGroups.
- type: string
- remoteIPPrefix:
- description: |-
- remoteIPPrefix is the remote IP prefix to be associated with this security group rule.
- You can specify either remoteGroupID or remoteIPPrefix or remoteManagedGroups.
- type: string
- required:
- - direction
- - id
- type: object
- type: array
required:
- id
- name
diff --git a/docs/book/src/api/v1beta1/api.md b/docs/book/src/api/v1beta1/api.md
index ae9159d5fb..2b12258bef 100644
--- a/docs/book/src/api/v1beta1/api.md
+++ b/docs/book/src/api/v1beta1/api.md
@@ -4365,139 +4365,6 @@ You can specify either remoteGroupID or remoteIPPrefix or remoteManagedGroups.
-
SecurityGroupRuleStatus
-
-
-(Appears on:
-SecurityGroupStatus)
-
-
-
-
-
-
-Field |
-Description |
-
-
-
-
-
-id
-
-string
-
- |
-
- id of the security group rule
- |
-
-
-
-description
-
-string
-
- |
-
-(Optional)
- description of the security group rule.
- |
-
-
-
-direction
-
-string
-
- |
-
- direction in which the security group rule is applied. The only values
-allowed are “ingress” or “egress”. For a compute instance, an ingress
-security group rule is applied to incoming (ingress) traffic for that
-instance. An egress rule is applied to traffic leaving the instance.
- |
-
-
-
-etherType
-
-string
-
- |
-
-(Optional)
- etherType must be IPv4 or IPv6, and addresses represented in CIDR must match the
-ingress or egress rules.
- |
-
-
-
-portRangeMin
-
-int
-
- |
-
-(Optional)
- portRangeMin is a number in the range that is matched by the security group
-rule. If the protocol is TCP or UDP, this value must be less than or equal
-to the value of the portRangeMax attribute.
- |
-
-
-
-portRangeMax
-
-int
-
- |
-
-(Optional)
- portRangeMax is a number in the range that is matched by the security group
-rule. The portRangeMin attribute constrains the portRangeMax attribute.
- |
-
-
-
-protocol
-
-string
-
- |
-
-(Optional)
- protocol is the protocol that is matched by the security group rule.
- |
-
-
-
-remoteGroupID
-
-string
-
- |
-
-(Optional)
- remoteGroupID is the remote group ID to be associated with this security group rule.
-You can specify either remoteGroupID or remoteIPPrefix or remoteManagedGroups.
- |
-
-
-
-remoteIPPrefix
-
-string
-
- |
-
-(Optional)
- remoteIPPrefix is the remote IP prefix to be associated with this security group rule.
-You can specify either remoteGroupID or remoteIPPrefix or remoteManagedGroups.
- |
-
-
-
SecurityGroupStatus
@@ -4538,20 +4405,6 @@ string
id of the security group
-
-
-rules
-
-
-[]SecurityGroupRuleStatus
-
-
- |
-
-(Optional)
- list of security group rules
- |
-
ServerGroupFilter
diff --git a/pkg/cloud/services/networking/securitygroups.go b/pkg/cloud/services/networking/securitygroups.go
index 0b67e5fd92..76cc8e349e 100644
--- a/pkg/cloud/services/networking/securitygroups.go
+++ b/pkg/cloud/services/networking/securitygroups.go
@@ -18,6 +18,7 @@ package networking
import (
"fmt"
+ "slices"
"github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/attributestags"
"github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/security/groups"
@@ -45,52 +46,74 @@ func (s *Service) ReconcileSecurityGroups(openStackCluster *infrav1.OpenStackClu
return nil
}
+ bastionEnabled := openStackCluster.Spec.Bastion != nil && openStackCluster.Spec.Bastion.Enabled
+
secControlPlaneGroupName := getSecControlPlaneGroupName(clusterName)
secWorkerGroupName := getSecWorkerGroupName(clusterName)
- secGroupNames := map[string]string{
+ suffixToNameMap := map[string]string{
controlPlaneSuffix: secControlPlaneGroupName,
workerSuffix: secWorkerGroupName,
}
- if openStackCluster.Spec.Bastion != nil && openStackCluster.Spec.Bastion.Enabled {
+ if bastionEnabled {
secBastionGroupName := getSecBastionGroupName(clusterName)
- secGroupNames[bastionSuffix] = secBastionGroupName
+ suffixToNameMap[bastionSuffix] = secBastionGroupName
}
// create security groups first, because desired rules use group ids.
- for _, v := range secGroupNames {
- if err := s.createSecurityGroupIfNotExists(openStackCluster, v); err != nil {
+ observedSecGroupBySuffix := make(map[string]*groups.SecGroup)
+ for suffix, secGroupName := range suffixToNameMap {
+ group, err := s.getOrCreateSecurityGroup(openStackCluster, secGroupName)
+ if err != nil {
return err
}
+ observedSecGroupBySuffix[suffix] = group
+
+ normaliseTags := func(tags []string) []string {
+ tags = slices.Clone(tags)
+ slices.Sort(tags)
+ return slices.Compact(tags)
+ }
+
+ if !slices.Equal(normaliseTags(openStackCluster.Spec.Tags), normaliseTags(group.Tags)) {
+ _, err = s.client.ReplaceAllAttributesTags("security-groups", group.ID, attributestags.ReplaceAllOpts{
+ Tags: openStackCluster.Spec.Tags,
+ })
+ if err != nil {
+ return err
+ }
+ s.scope.Logger().V(6).Info("Updated tags for security group", "name", group.Name, "id", group.ID)
+ }
}
+
// create desired security groups
- desiredSecGroups, err := s.generateDesiredSecGroups(openStackCluster, secGroupNames)
+ desiredSecGroupsBySuffix, err := s.generateDesiredSecGroups(openStackCluster, suffixToNameMap, observedSecGroupBySuffix)
if err != nil {
return err
}
- observedSecGroups := make(map[string]*infrav1.SecurityGroupStatus)
- for k, desiredSecGroup := range desiredSecGroups {
- var err error
- observedSecGroups[k], err = s.getSecurityGroupByName(desiredSecGroup.Name)
+ for suffix := range desiredSecGroupsBySuffix {
+ desiredSecGroup := desiredSecGroupsBySuffix[suffix]
+ observedSecGroup, ok := observedSecGroupBySuffix[suffix]
+ if !ok {
+ // This should never happen
+ return fmt.Errorf("unable to reconcile security groups: security group %s not found", suffix)
+ }
+ err := s.reconcileGroupRules(&desiredSecGroup, observedSecGroup)
if err != nil {
return err
}
-
- if observedSecGroups[k].ID != "" {
- observedSecGroup, err := s.reconcileGroupRules(desiredSecGroup, *observedSecGroups[k])
- if err != nil {
- return err
- }
- observedSecGroups[k] = &observedSecGroup
- continue
- }
+ continue
}
- openStackCluster.Status.ControlPlaneSecurityGroup = observedSecGroups[controlPlaneSuffix]
- openStackCluster.Status.WorkerSecurityGroup = observedSecGroups[workerSuffix]
- openStackCluster.Status.BastionSecurityGroup = observedSecGroups[bastionSuffix]
+ openStackCluster.Status.ControlPlaneSecurityGroup = convertOSSecGroupToConfigSecGroup(observedSecGroupBySuffix[controlPlaneSuffix])
+ openStackCluster.Status.WorkerSecurityGroup = convertOSSecGroupToConfigSecGroup(observedSecGroupBySuffix[workerSuffix])
+ if bastionEnabled {
+ openStackCluster.Status.BastionSecurityGroup = convertOSSecGroupToConfigSecGroup(observedSecGroupBySuffix[bastionSuffix])
+ } else {
+ openStackCluster.Status.BastionSecurityGroup = nil
+ }
return nil
}
@@ -111,24 +134,22 @@ type resolvedSecurityGroupRuleSpec struct {
RemoteIPPrefix string `json:"remoteIPPrefix,omitempty"`
}
-func (r resolvedSecurityGroupRuleSpec) Matches(other infrav1.SecurityGroupRuleStatus) bool {
- return r.Description == *other.Description &&
+func (r resolvedSecurityGroupRuleSpec) Matches(other rules.SecGroupRule) bool {
+ return r.Description == other.Description &&
r.Direction == other.Direction &&
- r.EtherType == *other.EtherType &&
- r.PortRangeMin == *other.PortRangeMin &&
- r.PortRangeMax == *other.PortRangeMax &&
- r.Protocol == *other.Protocol &&
- r.RemoteGroupID == *other.RemoteGroupID &&
- r.RemoteIPPrefix == *other.RemoteIPPrefix
+ r.EtherType == other.EtherType &&
+ r.PortRangeMin == other.PortRangeMin &&
+ r.PortRangeMax == other.PortRangeMax &&
+ r.Protocol == other.Protocol &&
+ r.RemoteGroupID == other.RemoteGroupID &&
+ r.RemoteIPPrefix == other.RemoteIPPrefix
}
-func (s *Service) generateDesiredSecGroups(openStackCluster *infrav1.OpenStackCluster, secGroupNames map[string]string) (map[string]securityGroupSpec, error) {
+func (s *Service) generateDesiredSecGroups(openStackCluster *infrav1.OpenStackCluster, suffixToNameMap map[string]string, observedSecGroupsBySuffix map[string]*groups.SecGroup) (map[string]securityGroupSpec, error) {
if openStackCluster.Spec.ManagedSecurityGroups == nil {
return nil, nil
}
- desiredSecGroups := make(map[string]securityGroupSpec)
-
var secControlPlaneGroupID string
var secWorkerGroupID string
var secBastionGroupID string
@@ -139,12 +160,13 @@ func (s *Service) generateDesiredSecGroups(openStackCluster *infrav1.OpenStackCl
// For now, we only reference the control plane and worker security groups.
remoteManagedGroups := make(map[string]string)
- for i, v := range secGroupNames {
- secGroup, err := s.getSecurityGroupByName(v)
- if err != nil {
- return desiredSecGroups, err
+ for suffix := range suffixToNameMap {
+ secGroup, ok := observedSecGroupsBySuffix[suffix]
+ if !ok {
+ // This should never happen, as we should have created the security group earlier in this reconcile if it does not exist.
+ return nil, fmt.Errorf("unable to generate desired security group rules: security group for %s not found", suffix)
}
- switch i {
+ switch suffix {
case controlPlaneSuffix:
secControlPlaneGroupID = secGroup.ID
remoteManagedGroups[controlPlaneSuffix] = secControlPlaneGroupID
@@ -182,17 +204,19 @@ func (s *Service) generateDesiredSecGroups(openStackCluster *infrav1.OpenStackCl
// Instead, we append the rules for allNodes to the control plane and worker security groups.
allNodesRules, err := getAllNodesRules(remoteManagedGroups, openStackCluster.Spec.ManagedSecurityGroups.AllNodesSecurityGroupRules)
if err != nil {
- return desiredSecGroups, err
+ return nil, err
}
controlPlaneRules = append(controlPlaneRules, allNodesRules...)
workerRules = append(workerRules, allNodesRules...)
+ desiredSecGroupsBySuffix := make(map[string]securityGroupSpec)
+
if openStackCluster.Spec.Bastion != nil && openStackCluster.Spec.Bastion.Enabled {
controlPlaneRules = append(controlPlaneRules, getSGControlPlaneSSH(secBastionGroupID)...)
workerRules = append(workerRules, getSGWorkerSSH(secBastionGroupID)...)
- desiredSecGroups[bastionSuffix] = securityGroupSpec{
- Name: secGroupNames[bastionSuffix],
+ desiredSecGroupsBySuffix[bastionSuffix] = securityGroupSpec{
+ Name: suffixToNameMap[bastionSuffix],
Rules: append(
[]resolvedSecurityGroupRuleSpec{
{
@@ -209,16 +233,16 @@ func (s *Service) generateDesiredSecGroups(openStackCluster *infrav1.OpenStackCl
}
}
- desiredSecGroups[controlPlaneSuffix] = securityGroupSpec{
- Name: secGroupNames[controlPlaneSuffix],
+ desiredSecGroupsBySuffix[controlPlaneSuffix] = securityGroupSpec{
+ Name: suffixToNameMap[controlPlaneSuffix],
Rules: controlPlaneRules,
}
- desiredSecGroups[workerSuffix] = securityGroupSpec{
- Name: secGroupNames[workerSuffix],
+ desiredSecGroupsBySuffix[workerSuffix] = securityGroupSpec{
+ Name: suffixToNameMap[workerSuffix],
Rules: workerRules,
}
- return desiredSecGroups, nil
+ return desiredSecGroupsBySuffix, nil
}
// getAllNodesRules returns the rules for the allNodes security group that should be created.
@@ -345,7 +369,7 @@ func (s *Service) deleteSecurityGroup(openStackCluster *infrav1.OpenStackCluster
if err != nil {
return err
}
- if group.ID == "" {
+ if group == nil {
// nothing to do
return nil
}
@@ -361,7 +385,7 @@ func (s *Service) deleteSecurityGroup(openStackCluster *infrav1.OpenStackCluster
// reconcileGroupRules reconciles an already existing observed group by deleting rules not needed anymore and
// creating rules that are missing.
-func (s *Service) reconcileGroupRules(desired securityGroupSpec, observed infrav1.SecurityGroupStatus) (infrav1.SecurityGroupStatus, error) {
+func (s *Service) reconcileGroupRules(desired *securityGroupSpec, observed *groups.SecGroup) error {
var rulesToDelete []string
// fills rulesToDelete by calculating observed - desired
for _, observedRule := range observed.Rules {
@@ -382,7 +406,6 @@ func (s *Service) reconcileGroupRules(desired securityGroupSpec, observed infrav
}
rulesToCreate := []resolvedSecurityGroupRuleSpec{}
- reconciledRules := make([]infrav1.SecurityGroupRuleStatus, 0, len(desired.Rules))
// fills rulesToCreate by calculating desired - observed
// also adds rules which are in observed and desired to reconcileGroupRules.
for _, desiredRule := range desired.Rules {
@@ -394,7 +417,6 @@ func (s *Service) reconcileGroupRules(desired securityGroupSpec, observed infrav
for _, observedRule := range observed.Rules {
if r.Matches(observedRule) {
// add already existing rules to reconciledRules because we won't touch them anymore
- reconciledRules = append(reconciledRules, observedRule)
createRule = false
break
}
@@ -409,7 +431,7 @@ func (s *Service) reconcileGroupRules(desired securityGroupSpec, observed infrav
s.scope.Logger().V(6).Info("Deleting rule", "ID", rule, "name", observed.Name)
err := s.client.DeleteSecGroupRule(rule)
if err != nil {
- return infrav1.SecurityGroupStatus{}, err
+ return err
}
}
@@ -419,61 +441,44 @@ func (s *Service) reconcileGroupRules(desired securityGroupSpec, observed infrav
if r.RemoteGroupID == remoteGroupIDSelf {
r.RemoteGroupID = observed.ID
}
- newRule, err := s.createRule(observed.ID, r)
+ err := s.createRule(observed.ID, r)
if err != nil {
- return infrav1.SecurityGroupStatus{}, err
+ return err
}
- reconciledRules = append(reconciledRules, newRule)
}
- observed.Rules = reconciledRules
- if len(reconciledRules) == 0 {
- return infrav1.SecurityGroupStatus{}, nil
- }
-
- return observed, nil
+ return nil
}
-func (s *Service) createSecurityGroupIfNotExists(openStackCluster *infrav1.OpenStackCluster, groupName string) error {
+func (s *Service) getOrCreateSecurityGroup(openStackCluster *infrav1.OpenStackCluster, groupName string) (*groups.SecGroup, error) {
secGroup, err := s.getSecurityGroupByName(groupName)
if err != nil {
- return err
+ return nil, err
+ }
+ if secGroup != nil {
+ s.scope.Logger().V(6).Info("Reusing existing SecurityGroup", "name", groupName, "id", secGroup.ID)
+ return secGroup, nil
}
- if secGroup == nil || secGroup.ID == "" {
- s.scope.Logger().V(6).Info("Group doesn't exist, creating it", "name", groupName)
-
- createOpts := groups.CreateOpts{
- Name: groupName,
- Description: "Cluster API managed group",
- }
- s.scope.Logger().V(6).Info("Creating group", "name", groupName)
-
- group, err := s.client.CreateSecGroup(createOpts)
- if err != nil {
- record.Warnf(openStackCluster, "FailedCreateSecurityGroup", "Failed to create security group %s: %v", groupName, err)
- return err
- }
- if len(openStackCluster.Spec.Tags) > 0 {
- _, err = s.client.ReplaceAllAttributesTags("security-groups", group.ID, attributestags.ReplaceAllOpts{
- Tags: openStackCluster.Spec.Tags,
- })
- if err != nil {
- return err
- }
- }
+ s.scope.Logger().V(6).Info("Group doesn't exist, creating it", "name", groupName)
- record.Eventf(openStackCluster, "SuccessfulCreateSecurityGroup", "Created security group %s with id %s", groupName, group.ID)
- return nil
+ createOpts := groups.CreateOpts{
+ Name: groupName,
+ Description: "Cluster API managed group",
}
+ s.scope.Logger().V(6).Info("Creating group", "name", groupName)
- sInfo := fmt.Sprintf("Reuse Existing SecurityGroup %s with %s", groupName, secGroup.ID)
- s.scope.Logger().V(6).Info(sInfo)
+ group, err := s.client.CreateSecGroup(createOpts)
+ if err != nil {
+ record.Warnf(openStackCluster, "FailedCreateSecurityGroup", "Failed to create security group %s: %v", groupName, err)
+ return nil, err
+ }
- return nil
+ record.Eventf(openStackCluster, "SuccessfulCreateSecurityGroup", "Created security group %s with id %s", groupName, group.ID)
+ return group, nil
}
-func (s *Service) getSecurityGroupByName(name string) (*infrav1.SecurityGroupStatus, error) {
+func (s *Service) getSecurityGroupByName(name string) (*groups.SecGroup, error) {
opts := groups.ListOpts{
Name: name,
}
@@ -481,20 +486,20 @@ func (s *Service) getSecurityGroupByName(name string) (*infrav1.SecurityGroupSta
s.scope.Logger().V(6).Info("Attempting to fetch security group with", "name", name)
allGroups, err := s.client.ListSecGroup(opts)
if err != nil {
- return &infrav1.SecurityGroupStatus{}, err
+ return nil, err
}
switch len(allGroups) {
case 0:
- return &infrav1.SecurityGroupStatus{}, nil
+ return nil, nil
case 1:
- return convertOSSecGroupToConfigSecGroup(allGroups[0]), nil
+ return &allGroups[0], nil
}
- return &infrav1.SecurityGroupStatus{}, fmt.Errorf("more than one security group found named: %s", name)
+ return nil, fmt.Errorf("more than one security group found named: %s", name)
}
-func (s *Service) createRule(securityGroupID string, r resolvedSecurityGroupRuleSpec) (infrav1.SecurityGroupRuleStatus, error) {
+func (s *Service) createRule(securityGroupID string, r resolvedSecurityGroupRuleSpec) error {
dir := rules.RuleDirection(r.Direction)
proto := rules.RuleProtocol(r.Protocol)
etherType := rules.RuleEtherType(r.EtherType)
@@ -511,11 +516,11 @@ func (s *Service) createRule(securityGroupID string, r resolvedSecurityGroupRule
SecGroupID: securityGroupID,
}
s.scope.Logger().V(6).Info("Creating rule", "description", r.Description, "direction", dir, "portRangeMin", r.PortRangeMin, "portRangeMax", r.PortRangeMax, "proto", proto, "etherType", etherType, "remoteGroupID", r.RemoteGroupID, "remoteIPPrefix", r.RemoteIPPrefix, "securityGroupID", securityGroupID)
- rule, err := s.client.CreateSecGroupRule(createOpts)
+ _, err := s.client.CreateSecGroupRule(createOpts)
if err != nil {
- return infrav1.SecurityGroupRuleStatus{}, err
+ return err
}
- return convertOSSecGroupRuleToConfigSecGroupRule(*rule), nil
+ return nil
}
func getSecControlPlaneGroupName(clusterName string) string {
@@ -530,29 +535,10 @@ func getSecBastionGroupName(clusterName string) string {
return fmt.Sprintf("%s-cluster-%s-secgroup-%s", secGroupPrefix, clusterName, bastionSuffix)
}
-func convertOSSecGroupToConfigSecGroup(osSecGroup groups.SecGroup) *infrav1.SecurityGroupStatus {
- securityGroupRules := make([]infrav1.SecurityGroupRuleStatus, len(osSecGroup.Rules))
- for i, rule := range osSecGroup.Rules {
- securityGroupRules[i] = convertOSSecGroupRuleToConfigSecGroupRule(rule)
- }
+func convertOSSecGroupToConfigSecGroup(osSecGroup *groups.SecGroup) *infrav1.SecurityGroupStatus {
return &infrav1.SecurityGroupStatus{
- ID: osSecGroup.ID,
- Name: osSecGroup.Name,
- Rules: securityGroupRules,
- }
-}
-
-func convertOSSecGroupRuleToConfigSecGroupRule(osSecGroupRule rules.SecGroupRule) infrav1.SecurityGroupRuleStatus {
- return infrav1.SecurityGroupRuleStatus{
- ID: osSecGroupRule.ID,
- Direction: osSecGroupRule.Direction,
- Description: &osSecGroupRule.Description,
- EtherType: &osSecGroupRule.EtherType,
- PortRangeMin: &osSecGroupRule.PortRangeMin,
- PortRangeMax: &osSecGroupRule.PortRangeMax,
- Protocol: &osSecGroupRule.Protocol,
- RemoteGroupID: &osSecGroupRule.RemoteGroupID,
- RemoteIPPrefix: &osSecGroupRule.RemoteIPPrefix,
+ ID: osSecGroup.ID,
+ Name: osSecGroup.Name,
}
}
diff --git a/pkg/cloud/services/networking/securitygroups_test.go b/pkg/cloud/services/networking/securitygroups_test.go
index b88a81cd43..555e366559 100644
--- a/pkg/cloud/services/networking/securitygroups_test.go
+++ b/pkg/cloud/services/networking/securitygroups_test.go
@@ -19,13 +19,15 @@ import (
"reflect"
"testing"
+ "github.com/go-logr/logr"
"github.com/go-logr/logr/testr"
"github.com/golang/mock/gomock"
+ "github.com/google/go-cmp/cmp"
+ "github.com/google/uuid"
"github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/security/groups"
"github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/security/rules"
. "github.com/onsi/gomega"
"k8s.io/utils/pointer"
-
infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1"
"sigs.k8s.io/cluster-api-provider-openstack/pkg/clients/mock"
"sigs.k8s.io/cluster-api-provider-openstack/pkg/scope"
@@ -224,18 +226,25 @@ func TestGetAllNodesRules(t *testing.T) {
}
func TestGenerateDesiredSecGroups(t *testing.T) {
- mockCtrl := gomock.NewController(t)
- defer mockCtrl.Finish()
-
secGroupNames := map[string]string{
"controlplane": "k8s-cluster-mycluster-secgroup-controlplane",
"worker": "k8s-cluster-mycluster-secgroup-worker",
}
+ observedSecGroups := map[string]*groups.SecGroup{
+ "k8s-cluster-mycluster-secgroup-controlplane": {
+ ID: "0",
+ Name: "k8s-cluster-mycluster-secgroup-controlplane",
+ },
+ "k8s-cluster-mycluster-secgroup-worker": {
+ ID: "1",
+ Name: "k8s-cluster-mycluster-secgroup-worker",
+ },
+ }
+
tests := []struct {
name string
openStackCluster *infrav1.OpenStackCluster
- mockExpect func(m *mock.MockNetworkClientMockRecorder)
// We could also test the exact rules that are returned, but that'll be a lot data to write out.
// For now we just make sure that the number of rules is correct.
expectedNumberSecurityGroupRules int
@@ -244,7 +253,6 @@ func TestGenerateDesiredSecGroups(t *testing.T) {
{
name: "Valid openStackCluster with unmanaged securityGroups",
openStackCluster: &infrav1.OpenStackCluster{},
- mockExpect: func(m *mock.MockNetworkClientMockRecorder) {},
expectedNumberSecurityGroupRules: 0,
wantErr: false,
},
@@ -255,20 +263,6 @@ func TestGenerateDesiredSecGroups(t *testing.T) {
ManagedSecurityGroups: &infrav1.ManagedSecurityGroups{},
},
},
- mockExpect: func(m *mock.MockNetworkClientMockRecorder) {
- m.ListSecGroup(groups.ListOpts{Name: "k8s-cluster-mycluster-secgroup-controlplane"}).Return([]groups.SecGroup{
- {
- ID: "0",
- Name: "k8s-cluster-mycluster-secgroup-controlplane",
- },
- }, nil)
- m.ListSecGroup(groups.ListOpts{Name: "k8s-cluster-mycluster-secgroup-worker"}).Return([]groups.SecGroup{
- {
- ID: "1",
- Name: "k8s-cluster-mycluster-secgroup-worker",
- },
- }, nil)
- },
expectedNumberSecurityGroupRules: 12,
wantErr: false,
},
@@ -288,20 +282,6 @@ func TestGenerateDesiredSecGroups(t *testing.T) {
},
},
},
- mockExpect: func(m *mock.MockNetworkClientMockRecorder) {
- m.ListSecGroup(groups.ListOpts{Name: "k8s-cluster-mycluster-secgroup-controlplane"}).Return([]groups.SecGroup{
- {
- ID: "0",
- Name: "k8s-cluster-mycluster-secgroup-controlplane",
- },
- }, nil)
- m.ListSecGroup(groups.ListOpts{Name: "k8s-cluster-mycluster-secgroup-worker"}).Return([]groups.SecGroup{
- {
- ID: "1",
- Name: "k8s-cluster-mycluster-secgroup-worker",
- },
- }, nil)
- },
expectedNumberSecurityGroupRules: 16,
wantErr: false,
},
@@ -321,26 +301,15 @@ func TestGenerateDesiredSecGroups(t *testing.T) {
},
},
},
- mockExpect: func(m *mock.MockNetworkClientMockRecorder) {
- m.ListSecGroup(groups.ListOpts{Name: "k8s-cluster-mycluster-secgroup-controlplane"}).Return([]groups.SecGroup{
- {
- ID: "0",
- Name: "k8s-cluster-mycluster-secgroup-controlplane",
- },
- }, nil)
- m.ListSecGroup(groups.ListOpts{Name: "k8s-cluster-mycluster-secgroup-worker"}).Return([]groups.SecGroup{
- {
- ID: "1",
- Name: "k8s-cluster-mycluster-secgroup-worker",
- },
- }, nil)
- },
wantErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
+ mockCtrl := gomock.NewController(t)
+ defer mockCtrl.Finish()
+
g := NewWithT(t)
log := testr.New(t)
mockScopeFactory := scope.NewMockScopeFactory(mockCtrl, "")
@@ -349,9 +318,8 @@ func TestGenerateDesiredSecGroups(t *testing.T) {
if err != nil {
t.Fatalf("Failed to create service: %v", err)
}
- tt.mockExpect(mockScopeFactory.NetworkClient.EXPECT())
- gotSecurityGroups, err := s.generateDesiredSecGroups(tt.openStackCluster, secGroupNames)
+ gotSecurityGroups, err := s.generateDesiredSecGroups(tt.openStackCluster, secGroupNames, observedSecGroups)
if tt.wantErr {
g.Expect(err).To(HaveOccurred())
} else {
@@ -367,27 +335,35 @@ func TestGenerateDesiredSecGroups(t *testing.T) {
}
func TestReconcileGroupRules(t *testing.T) {
- mockCtrl := gomock.NewController(t)
- defer mockCtrl.Finish()
+ const (
+ sgID = "6260e813-af79-4592-8d1a-0f42dd26cc42"
+ sgRuleID = "52a532c4-2b44-4582-ba87-b64e62e19b1a"
+ sgLegacyRuleID = "a057dcc4-1535-469d-9d28-923cad9d4c56"
+ sgName = "k8s-cluster-mycluster-secgroup-controlplane"
+ )
tests := []struct {
- name string
- desiredSGSpecs securityGroupSpec
- observedSGStatus infrav1.SecurityGroupStatus
- mockExpect func(m *mock.MockNetworkClientMockRecorder)
- wantSGStatus infrav1.SecurityGroupStatus
+ name string
+ desiredSGSpec securityGroupSpec
+ observedSG groups.SecGroup
+ mockExpect func(m *mock.MockNetworkClientMockRecorder)
+ wantSGStatus infrav1.SecurityGroupStatus
}{
{
- name: "Empty desiredSGSpecs and observedSGStatus",
- desiredSGSpecs: securityGroupSpec{},
- observedSGStatus: infrav1.SecurityGroupStatus{},
- mockExpect: func(m *mock.MockNetworkClientMockRecorder) {},
- wantSGStatus: infrav1.SecurityGroupStatus{},
+ name: "Empty desiredSGSpec and observedSG",
+ desiredSGSpec: securityGroupSpec{},
+ observedSG: groups.SecGroup{
+ ID: sgID,
+ Name: sgName,
+ Rules: []rules.SecGroupRule{},
+ },
+ mockExpect: func(m *mock.MockNetworkClientMockRecorder) {},
+ wantSGStatus: infrav1.SecurityGroupStatus{},
},
{
- name: "Same desiredSGSpecs and observedSGStatus produces no changes",
- desiredSGSpecs: securityGroupSpec{
- Name: "k8s-cluster-mycluster-secgroup-controlplane",
+ name: "Same desiredSGSpec and observedSG produces no changes",
+ desiredSGSpec: securityGroupSpec{
+ Name: sgName,
Rules: []resolvedSecurityGroupRuleSpec{
{
Description: "Allow SSH",
@@ -401,46 +377,29 @@ func TestReconcileGroupRules(t *testing.T) {
},
},
},
- observedSGStatus: infrav1.SecurityGroupStatus{
- ID: "idSG",
- Name: "k8s-cluster-mycluster-secgroup-controlplane",
- Rules: []infrav1.SecurityGroupRuleStatus{
+ observedSG: groups.SecGroup{
+ ID: sgID,
+ Name: sgName,
+ Rules: []rules.SecGroupRule{
{
- Description: pointer.String("Allow SSH"),
+ Description: "Allow SSH",
Direction: "ingress",
- EtherType: pointer.String("IPv4"),
+ EtherType: "IPv4",
ID: "idSGRule",
- Protocol: pointer.String("tcp"),
- PortRangeMin: pointer.Int(22),
- PortRangeMax: pointer.Int(22),
- RemoteGroupID: pointer.String("1"),
- RemoteIPPrefix: pointer.String(""),
+ Protocol: "tcp",
+ PortRangeMin: 22,
+ PortRangeMax: 22,
+ RemoteGroupID: "1",
+ RemoteIPPrefix: "",
},
},
},
mockExpect: func(m *mock.MockNetworkClientMockRecorder) {},
- wantSGStatus: infrav1.SecurityGroupStatus{
- ID: "idSG",
- Name: "k8s-cluster-mycluster-secgroup-controlplane",
- Rules: []infrav1.SecurityGroupRuleStatus{
- {
- Description: pointer.String("Allow SSH"),
- Direction: "ingress",
- EtherType: pointer.String("IPv4"),
- ID: "idSGRule",
- Protocol: pointer.String("tcp"),
- PortRangeMin: pointer.Int(22),
- PortRangeMax: pointer.Int(22),
- RemoteGroupID: pointer.String("1"),
- RemoteIPPrefix: pointer.String(""),
- },
- },
- },
},
{
- name: "Different desiredSGSpecs and observedSGStatus produces changes",
- desiredSGSpecs: securityGroupSpec{
- Name: "k8s-cluster-mycluster-secgroup-controlplane",
+ name: "Different desiredSGSpec and observedSG produces changes",
+ desiredSGSpec: securityGroupSpec{
+ Name: sgName,
Rules: []resolvedSecurityGroupRuleSpec{
{
Description: "Allow SSH",
@@ -454,27 +413,27 @@ func TestReconcileGroupRules(t *testing.T) {
},
},
},
- observedSGStatus: infrav1.SecurityGroupStatus{
- ID: "idSG",
- Name: "k8s-cluster-mycluster-secgroup-controlplane",
- Rules: []infrav1.SecurityGroupRuleStatus{
+ observedSG: groups.SecGroup{
+ ID: sgID,
+ Name: sgName,
+ Rules: []rules.SecGroupRule{
{
- Description: pointer.String("Allow SSH legacy"),
+ ID: sgLegacyRuleID,
+ Description: "Allow SSH legacy",
Direction: "ingress",
- EtherType: pointer.String("IPv4"),
- ID: "idSGRuleLegacy",
- Protocol: pointer.String("tcp"),
- PortRangeMin: pointer.Int(222),
- PortRangeMax: pointer.Int(222),
- RemoteGroupID: pointer.String("2"),
- RemoteIPPrefix: pointer.String(""),
+ EtherType: "IPv4",
+ Protocol: "tcp",
+ PortRangeMin: 222,
+ PortRangeMax: 222,
+ RemoteGroupID: "2",
+ RemoteIPPrefix: "",
},
},
},
mockExpect: func(m *mock.MockNetworkClientMockRecorder) {
- m.DeleteSecGroupRule("idSGRuleLegacy").Return(nil)
+ m.DeleteSecGroupRule(sgLegacyRuleID).Return(nil)
m.CreateSecGroupRule(rules.CreateOpts{
- SecGroupID: "idSG",
+ SecGroupID: sgID,
Description: "Allow SSH",
Direction: "ingress",
EtherType: "IPv4",
@@ -483,7 +442,7 @@ func TestReconcileGroupRules(t *testing.T) {
PortRangeMax: 22,
RemoteGroupID: "1",
}).Return(&rules.SecGroupRule{
- ID: "idSGRule",
+ ID: sgRuleID,
Description: "Allow SSH",
Direction: "ingress",
EtherType: "IPv4",
@@ -493,28 +452,15 @@ func TestReconcileGroupRules(t *testing.T) {
RemoteGroupID: "1",
}, nil)
},
- wantSGStatus: infrav1.SecurityGroupStatus{
- ID: "idSG",
- Name: "k8s-cluster-mycluster-secgroup-controlplane",
- Rules: []infrav1.SecurityGroupRuleStatus{
- {
- Description: pointer.String("Allow SSH"),
- Direction: "ingress",
- EtherType: pointer.String("IPv4"),
- ID: "idSGRule",
- Protocol: pointer.String("tcp"),
- PortRangeMin: pointer.Int(22),
- PortRangeMax: pointer.Int(22),
- RemoteGroupID: pointer.String("1"),
- RemoteIPPrefix: pointer.String(""),
- },
- },
- },
},
}
- for _, tt := range tests {
+ for i := range tests {
+ tt := &tests[i]
t.Run(tt.name, func(t *testing.T) {
+ mockCtrl := gomock.NewController(t)
+ defer mockCtrl.Finish()
+
g := NewWithT(t)
log := testr.New(t)
mockScopeFactory := scope.NewMockScopeFactory(mockCtrl, "")
@@ -525,9 +471,131 @@ func TestReconcileGroupRules(t *testing.T) {
}
tt.mockExpect(mockScopeFactory.NetworkClient.EXPECT())
- sgStatus, err := s.reconcileGroupRules(tt.desiredSGSpecs, tt.observedSGStatus)
+ err = s.reconcileGroupRules(&tt.desiredSGSpec, &tt.observedSG)
g.Expect(err).To(BeNil())
- g.Expect(sgStatus).To(Equal(tt.wantSGStatus))
+ })
+ }
+}
+
+func TestService_ReconcileSecurityGroups(t *testing.T) {
+ const (
+ clusterName = "test-cluster"
+
+ controlPlaneSGName = "k8s-cluster-test-cluster-secgroup-controlplane"
+ workerSGName = "k8s-cluster-test-cluster-secgroup-worker"
+ bastionSGName = "k8s-cluster-test-cluster-secgroup-bastion"
+ )
+
+ tests := []struct {
+ name string
+ openStackClusterSpec infrav1.OpenStackClusterSpec
+ expectedClusterStatus infrav1.OpenStackClusterStatus
+ expect func(log logr.Logger, m *mock.MockNetworkClientMockRecorder)
+ wantErr bool
+ }{
+ {
+ name: "Do nothing if ManagedSecurityGroups is not enabled",
+ openStackClusterSpec: infrav1.OpenStackClusterSpec{},
+ expectedClusterStatus: infrav1.OpenStackClusterStatus{},
+ },
+ {
+ name: "Default control plane and worker security groups",
+ openStackClusterSpec: infrav1.OpenStackClusterSpec{
+ ManagedSecurityGroups: &infrav1.ManagedSecurityGroups{},
+ },
+ expect: func(log logr.Logger, m *mock.MockNetworkClientMockRecorder) {
+ m.ListSecGroup(groups.ListOpts{Name: controlPlaneSGName}).
+ Return([]groups.SecGroup{{ID: "0", Name: controlPlaneSGName}}, nil)
+ m.ListSecGroup(groups.ListOpts{Name: workerSGName}).
+ Return([]groups.SecGroup{{ID: "1", Name: workerSGName}}, nil)
+
+ // We expect a total of 12 rules to be created.
+ // Nothing actually looks at the generated
+ // rules, but we give them unique IDs anyway
+ m.CreateSecGroupRule(gomock.Any()).DoAndReturn(func(opts rules.CreateOpts) (*rules.SecGroupRule, error) {
+ log.Info("Created rule", "securityGroup", opts.SecGroupID, "description", opts.Description)
+ return &rules.SecGroupRule{ID: uuid.NewString()}, nil
+ }).Times(12)
+ },
+ expectedClusterStatus: infrav1.OpenStackClusterStatus{
+ ControlPlaneSecurityGroup: &infrav1.SecurityGroupStatus{
+ ID: "0",
+ Name: controlPlaneSGName,
+ },
+ WorkerSecurityGroup: &infrav1.SecurityGroupStatus{
+ ID: "1",
+ Name: workerSGName,
+ },
+ },
+ },
+ {
+ name: "Default control plane, worker, and bastion security groups",
+ openStackClusterSpec: infrav1.OpenStackClusterSpec{
+ Bastion: &infrav1.Bastion{
+ Enabled: true,
+ },
+ ManagedSecurityGroups: &infrav1.ManagedSecurityGroups{},
+ },
+ expect: func(log logr.Logger, m *mock.MockNetworkClientMockRecorder) {
+ m.ListSecGroup(groups.ListOpts{Name: controlPlaneSGName}).
+ Return([]groups.SecGroup{{ID: "0", Name: controlPlaneSGName}}, nil)
+ m.ListSecGroup(groups.ListOpts{Name: workerSGName}).
+ Return([]groups.SecGroup{{ID: "1", Name: workerSGName}}, nil)
+ m.ListSecGroup(groups.ListOpts{Name: bastionSGName}).
+ Return([]groups.SecGroup{{ID: "2", Name: bastionSGName}}, nil)
+
+ // We expect a total of 12 rules to be created.
+ // Nothing actually looks at the generated
+ // rules, but we give them unique IDs anyway
+ m.CreateSecGroupRule(gomock.Any()).DoAndReturn(func(opts rules.CreateOpts) (*rules.SecGroupRule, error) {
+ log.Info("Created rule", "securityGroup", opts.SecGroupID, "description", opts.Description)
+ return &rules.SecGroupRule{ID: uuid.NewString()}, nil
+ }).Times(17)
+ },
+ expectedClusterStatus: infrav1.OpenStackClusterStatus{
+ ControlPlaneSecurityGroup: &infrav1.SecurityGroupStatus{
+ ID: "0",
+ Name: controlPlaneSGName,
+ },
+ WorkerSecurityGroup: &infrav1.SecurityGroupStatus{
+ ID: "1",
+ Name: workerSGName,
+ },
+ BastionSecurityGroup: &infrav1.SecurityGroupStatus{
+ ID: "2",
+ Name: bastionSGName,
+ },
+ },
+ },
+ }
+ for i := range tests {
+ tt := &tests[i]
+ t.Run(tt.name, func(t *testing.T) {
+ mockCtrl := gomock.NewController(t)
+ defer mockCtrl.Finish()
+
+ g := NewWithT(t)
+ log := testr.New(t)
+ mockScopeFactory := scope.NewMockScopeFactory(mockCtrl, "")
+ scope := scope.NewWithLogger(mockScopeFactory, log)
+
+ s := &Service{
+ scope: scope,
+ client: mockScopeFactory.NetworkClient,
+ }
+ if tt.expect != nil {
+ tt.expect(log, mockScopeFactory.NetworkClient.EXPECT())
+ }
+ openStackCluster := &infrav1.OpenStackCluster{
+ Spec: tt.openStackClusterSpec,
+ }
+ err := s.ReconcileSecurityGroups(openStackCluster, clusterName)
+ if tt.wantErr {
+ g.Expect(err).ToNot(BeNil(), "ReconcileSecurityGroups")
+ } else {
+ g.Expect(err).To(BeNil(), "ReconcileSecurityGroups")
+ g.Expect(openStackCluster.Status).To(Equal(tt.expectedClusterStatus), cmp.Diff(openStackCluster.Status, tt.expectedClusterStatus))
+ }
})
}
}