Skip to content

Commit d3d97e2

Browse files
Pacho20Pacho20
committed
Add support for new SGR protocols in VPC
The latest vpc-go-sdk introduces two new security group rule protocol types: any and individual. This commit adds full compatibility for these new protocols in the VPC implementation.
1 parent acda62d commit d3d97e2

File tree

4 files changed

+132
-24
lines changed

4 files changed

+132
-24
lines changed

‎api/vpc/v1beta2/types.go‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,9 @@ var (
164164
)
165165

166166
const (
167+
// VPCSecurityGroupRuleProtocolAnyType is a string representation of the 'SecurityGroupRuleProtocolAny' type.
168+
VPCSecurityGroupRuleProtocolAnyType = "*vpcv1.SecurityGroupRuleProtocolAny"
169+
167170
// VPCSecurityGroupRuleProtocolIcmptcpudpType is a string representation of the 'SecurityGroupRuleProtocolIcmptcpudp' type.
168171
VPCSecurityGroupRuleProtocolIcmptcpudpType = "*vpcv1.SecurityGroupRuleProtocolIcmptcpudp"
169172

@@ -172,6 +175,9 @@ const (
172175

173176
// VPCSecurityGroupRuleProtocolTcpudpType is a string representation of the 'SecurityGroupRuleSecurityGroupRuleProtocolTcpudp' type.
174177
VPCSecurityGroupRuleProtocolTcpudpType = "*vpcv1.SecurityGroupRuleSecurityGroupRuleProtocolTcpudp"
178+
179+
// VPCSecurityGroupRuleProtocolIndividualType is a string representation of the 'SecurityGroupRuleProtocolIndividual' type.
180+
VPCSecurityGroupRuleProtocolIndividualType = "*vpcv1.SecurityGroupRuleProtocolIndividual"
175181
)
176182

177183
// VPCSecurityGroupRuleAction represents the actions for a Security Group Rule.
@@ -197,10 +203,12 @@ const (
197203
)
198204

199205
// VPCSecurityGroupRuleProtocol represents the protocols for a Security Group Rule.
200-
// +kubebuilder:validation:Enum=icmp_tcp_udp;icmp;tcp;udp
206+
// +kubebuilder:validation:Pattern=`^(any|icmp_tcp_udp|icmp|tcp|udp|ah|esp|gre|ip_in_ip|l2tp|rsvp|sctp|vrrp|number_(?:0|2|3|5|[7-9]|1[0-6]|1[8-9]|[2-3][0-9]|4[0-5]|4[89]|5[2-9]|[6-9][0-9]|10[0-9]|11[0-1]|11[3-4]|11[6-9]|12[0-9]|13[0-1]|13[3-9]|1[4-9][0-9]|2[0-4][0-9]|25[0-5]))$`
201207
type VPCSecurityGroupRuleProtocol string
202208

203209
const (
210+
// VPCSecurityGroupRuleProtocolAny defines the Rule is for any network protocols.
211+
VPCSecurityGroupRuleProtocolAny VPCSecurityGroupRuleProtocol = vpcv1.NetworkACLRuleProtocolAnyConst
204212
// VPCSecurityGroupRuleProtocolIcmpTCPUDP defines the Rule is for ICMP, TCP and UDP protocols.
205213
VPCSecurityGroupRuleProtocolIcmpTCPUDP VPCSecurityGroupRuleProtocol = vpcv1.NetworkACLRuleProtocolIcmpTCPUDPConst
206214
// VPCSecurityGroupRuleProtocolIcmp defiens the Rule is for ICMP network protocol.
@@ -399,6 +407,7 @@ type VPCSecurityGroupRuleRemote struct {
399407
// VPCSecurityGroupRulePrototype defines a VPC Security Group Rule's traffic specifics for a series of remotes (destinations or sources).
400408
// +kubebuilder:validation:XValidation:rule="self.protocol != 'icmp' ? (!has(self.icmpCode) && !has(self.icmpType)) : true",message="icmpCode and icmpType are only supported for VPCSecurityGroupRuleProtocolIcmp protocol"
401409
// +kubebuilder:validation:XValidation:rule="self.protocol == 'icmp' ? !has(self.portRange) : true",message="portRange is not valid for VPCSecurityGroupRuleProtocolIcmp protocol"
410+
// +kubebuilder:validation:XValidation:rule="(self.protocol != 'tcp' && self.protocol != 'udp') ? !has(self.portRange) : true",message="portRange is not valid for protocol"
402411
// +kubebuilder:validation:XValidation:rule="self.protocol == 'icmp_tcp_udp' ? !has(self.portRange) : true",message="portRange is not valid for VPCSecurityGroupRuleProtocolIcmpTCPUDP protocol"
403412
type VPCSecurityGroupRulePrototype struct {
404413
// icmpCode is the ICMP code for the Rule.

‎cloud/scope/vpc/cluster_v2.go‎

Lines changed: 106 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"fmt"
2323
"net/http"
2424
"reflect"
25+
"regexp"
2526

2627
"github.com/go-logr/logr"
2728

@@ -61,6 +62,9 @@ const (
6162
privateLBSuffix = "private"
6263
// publicLBSuffix is used to tag a default Load Balancer name as public.
6364
publicLBSuffix = "public"
65+
66+
// individualSgrRegex is used to check if the VPCSecurityGroupRuleProtocolIndividual is valid.
67+
individualSgrRegex = "^(ah|esp|gre|ip_in_ip|l2tp|rsvp|sctp|vrrp|number_(?:0|2|3|5|[7-9]|1[0-6]|1[8-9]|[2-3][0-9]|4[0-5]|4[89]|5[2-9]|[6-9][0-9]|10[0-9]|11[0-1]|11[3-4]|11[6-9]|12[0-9]|13[0-1]|13[3-9]|1[4-9][0-9]|2[0-4][0-9]|25[0-5]))$"
6468
)
6569

6670
// ClusterScopeParamsV2 defines the input parameters used to create a new ClusterScopeV2.
@@ -89,6 +93,8 @@ type ClusterScopeV2 struct {
8993
Cluster *clusterv1.Cluster
9094
IBMVPCCluster *infrav1.IBMVPCCluster
9195
ServiceEndpoint []endpoints.ServiceEndpoint
96+
97+
individualSgrRegexp *regexp.Regexp
9298
}
9399

94100
// NewClusterScopeV2 creates a new ClusterScopeV2 from the supplied parameters.
@@ -179,6 +185,9 @@ func NewClusterScopeV2(params ClusterScopeParamsV2) (*ClusterScopeV2, error) {
179185
return nil, fmt.Errorf("failed to create resource manager client: %w", err)
180186
}
181187

188+
// Compile the regexp object for the VPCSecurityGroupRuleProtocolIndividualType
189+
individualSgrRegexp := regexp.MustCompile(individualSgrRegex)
190+
182191
clusterScope := &ClusterScopeV2{
183192
Logger: params.Logger,
184193
Client: params.Client,
@@ -190,6 +199,7 @@ func NewClusterScopeV2(params ClusterScopeParamsV2) (*ClusterScopeV2, error) {
190199
ResourceControllerClient: resourceControllerClient,
191200
ResourceManagerClient: resourceManagerClient,
192201
VPCClient: vpcClient,
202+
individualSgrRegexp: individualSgrRegexp,
193203
}
194204
return clusterScope, nil
195205
}
@@ -1455,6 +1465,25 @@ func (s *ClusterScopeV2) findOrCreateSecurityGroupRule(ctx context.Context, secu
14551465
for _, existingRuleIntf := range existingSecurityGroupRules.Rules {
14561466
// Perform analysis of the existingRuleIntf, based on its Protocol type, further analysis is performed based on remaining attributes to find if the specific Rule and Remote match
14571467
switch reflect.TypeOf(existingRuleIntf).String() {
1468+
case infrav1.VPCSecurityGroupRuleProtocolAnyType:
1469+
// If our Remote doesn't define the Any Protocol, we don't need further checks, move on to next Rule
1470+
if securityGroupRulePrototype.Protocol != infrav1.VPCSecurityGroupRuleProtocolAny {
1471+
continue
1472+
}
1473+
existingRule := existingRuleIntf.(*vpcv1.SecurityGroupRuleProtocolAny)
1474+
// If the Remote doesn't have the same Direction as the Rule, no further checks are necessary
1475+
if securityGroupRule.Direction != infrav1.VPCSecurityGroupRuleDirection(*existingRule.Direction) {
1476+
continue
1477+
}
1478+
if found, err := s.checkSecurityGroupRuleProtocolAny(ctx, securityGroupRulePrototype, remote, existingRule); err != nil {
1479+
return fmt.Errorf("error failure checking security group rule protocol any: %w", err)
1480+
} else if found {
1481+
// If we found the matching IBM Cloud Security Group Rule for the defined SecurityGroupRule and Remote, we can stop checking IBM Cloud Security Group Rules for this remote and move onto the next remote.
1482+
// The expectation is that only one IBM Cloud Security Group Rule will match, but if at least one matches the defined SecurityGroupRule, that is sufficient.
1483+
log.V(3).Info("security group rule any protocol match found")
1484+
remoteMatch = true
1485+
break
1486+
}
14581487
case infrav1.VPCSecurityGroupRuleProtocolIcmptcpudpType:
14591488
// If our Remote doesn't define icmp_tcp_udp Protocols, we don't need further checks, move on to next Rule
14601489
if securityGroupRulePrototype.Protocol != infrav1.VPCSecurityGroupRuleProtocolIcmpTCPUDP {
@@ -1469,7 +1498,6 @@ func (s *ClusterScopeV2) findOrCreateSecurityGroupRule(ctx context.Context, secu
14691498
return fmt.Errorf("error failure checking security group rule protocol icmp_tcp_udp: %w", err)
14701499
} else if found {
14711500
// If we found the matching IBM Cloud Security Group Rule for the defined SecurityGroupRule and Remote, we can stop checking IBM Cloud Security Group Rules for this remote and move onto the next remote.
1472-
// The expectation is that only one IBM Cloud Security Group Rule will match, but if at least one matches the defined SecurityGroupRule, that is sufficient.
14731501
log.V(3).Info("security group rule icmp_tcp_udp protocol match found")
14741502
remoteMatch = true
14751503
break
@@ -1510,6 +1538,26 @@ func (s *ClusterScopeV2) findOrCreateSecurityGroupRule(ctx context.Context, secu
15101538
remoteMatch = true
15111539
break
15121540
}
1541+
case infrav1.VPCSecurityGroupRuleProtocolIndividualType:
1542+
matched := s.individualSgrRegexp.MatchString(string(securityGroupRulePrototype.Protocol))
1543+
1544+
// If our Remote doesn't define one of the individual Protocol, we don't need further checks, move on to next Rule
1545+
if !matched {
1546+
continue
1547+
}
1548+
existingRule := existingRuleIntf.(*vpcv1.SecurityGroupRuleProtocolIndividual)
1549+
// If the Remote doesn't have the same Direction as the Rule, no further checks are necessary
1550+
if securityGroupRule.Direction != infrav1.VPCSecurityGroupRuleDirection(*existingRule.Direction) {
1551+
continue
1552+
}
1553+
if found, err := s.checkSecurityGroupRuleProtocolIndividual(ctx, securityGroupRulePrototype, remote, existingRule); err != nil {
1554+
return fmt.Errorf("error failure checking security group rule protocol %s: %w", string(securityGroupRulePrototype.Protocol), err) // TODO: Which protocol should be part of the error message?
1555+
} else if found {
1556+
// If we found the matching IBM Cloud Security Group Rule for the defined SecurityGroupRule and Remote, we can stop checking IBM Cloud Security Group Rules for this remote and move onto the next remote.
1557+
log.V(3).Info("security group rule individual protocol match found", "protocol", string(securityGroupRulePrototype.Protocol))
1558+
remoteMatch = true
1559+
break
1560+
}
15131561
default:
15141562
// This is an unexpected IBM Cloud Security Group Rule Prototype, log it and move on
15151563
log.V(3).Info("unexpected security group rule prototype", "securityGroupRulePrototype", reflect.TypeOf(existingRuleIntf).String())
@@ -1527,6 +1575,18 @@ func (s *ClusterScopeV2) findOrCreateSecurityGroupRule(ctx context.Context, secu
15271575
return nil
15281576
}
15291577

1578+
// checkSecurityGroupRuleProtocolAny analyzes an IBM Cloud Security Group Rule designated for 'any' protocols, to verify if the supplied Rule and Remote match the attributes from the existing 'any' Rule.
1579+
func (s *ClusterScopeV2) checkSecurityGroupRuleProtocolAny(ctx context.Context, _ infrav1.VPCSecurityGroupRulePrototype, securityGroupRuleRemote infrav1.VPCSecurityGroupRuleRemote, existingRule *vpcv1.SecurityGroupRuleProtocolAny) (bool, error) {
1580+
log := ctrl.LoggerFrom(ctx)
1581+
if exists, err := s.checkSecurityGroupRulePrototypeRemote(ctx, securityGroupRuleRemote, existingRule.Remote); err != nil {
1582+
return false, fmt.Errorf("error failed checking security group rule all remote: %w", err)
1583+
} else if exists {
1584+
log.V(3).Info("security group rule all protocols match")
1585+
return true, nil
1586+
}
1587+
return false, nil
1588+
}
1589+
15301590
// checkSecurityGroupRuleProtocolIcmpTCPUDP analyzes an IBM Cloud Security Group Rule designated for 'icmp_tcp_udp' protocols, to verify if the supplied Rule and Remote match the attributes from the existing 'icmp_tcp_udp' Rule.
15311591
func (s *ClusterScopeV2) checkSecurityGroupRuleProtocolIcmpTCPUDP(ctx context.Context, _ infrav1.VPCSecurityGroupRulePrototype, securityGroupRuleRemote infrav1.VPCSecurityGroupRuleRemote, existingRule *vpcv1.SecurityGroupRuleProtocolIcmptcpudp) (bool, error) {
15321592
log := ctrl.LoggerFrom(ctx)
@@ -1583,6 +1643,23 @@ func (s *ClusterScopeV2) checkSecurityGroupRuleProtocolTcpudp(ctx context.Contex
15831643
return false, nil
15841644
}
15851645

1646+
// checkSecurityGroupRuleProtocolIndividual analyzes an IBM Cloud Security Group Rule designated for individual protocols, to verify if the supplied Rule and Remote match the attributes from the existing individual Rule.
1647+
func (s *ClusterScopeV2) checkSecurityGroupRuleProtocolIndividual(ctx context.Context, securityGroupRulePrototype infrav1.VPCSecurityGroupRulePrototype, securityGroupRuleRemote infrav1.VPCSecurityGroupRuleRemote, existingRule *vpcv1.SecurityGroupRuleProtocolIndividual) (bool, error) {
1648+
log := ctrl.LoggerFrom(ctx)
1649+
// Check the protocol next to verify it matches
1650+
if securityGroupRulePrototype.Protocol != infrav1.VPCSecurityGroupRuleProtocol(*existingRule.Protocol) {
1651+
return false, nil
1652+
}
1653+
1654+
if exists, err := s.checkSecurityGroupRulePrototypeRemote(ctx, securityGroupRuleRemote, existingRule.Remote); err != nil {
1655+
return false, fmt.Errorf("error failed checking security group rule all remote: %w", err)
1656+
} else if exists {
1657+
log.V(3).Info("security group rule all protocols match")
1658+
return true, nil
1659+
}
1660+
return false, nil
1661+
}
1662+
15861663
func (s *ClusterScopeV2) checkSecurityGroupRulePrototypeRemote(ctx context.Context, securityGroupRuleRemote infrav1.VPCSecurityGroupRuleRemote, existingRemote vpcv1.SecurityGroupRuleRemoteIntf) (bool, error) { //nolint: gocyclo
15871664
log := ctrl.LoggerFrom(ctx)
15881665
// NOTE(cjschaef): We only currently monitor Remote, not Local, as we don't support defining Local in SecurityGroup/SecurityGroupRule.
@@ -1706,6 +1783,13 @@ func (s *ClusterScopeV2) createSecurityGroupRule(ctx context.Context, securityGr
17061783
return fmt.Errorf("error failed to create security group rule remote: %w", err)
17071784
}
17081785
switch securityGroupRulePrototype.Protocol {
1786+
case infrav1.VPCSecurityGroupRuleProtocolAny:
1787+
prototype := &vpcv1.SecurityGroupRulePrototypeSecurityGroupRuleProtocolAnyPrototype{
1788+
Direction: ptr.To(string(securityGroupRule.Direction)),
1789+
Protocol: ptr.To(string(securityGroupRulePrototype.Protocol)),
1790+
Remote: prototypeRemote,
1791+
}
1792+
options.SetSecurityGroupRulePrototype(prototype)
17091793
case infrav1.VPCSecurityGroupRuleProtocolIcmpTCPUDP:
17101794
prototype := &vpcv1.SecurityGroupRulePrototypeSecurityGroupRuleProtocolIcmptcpudpPrototype{
17111795
Direction: ptr.To(string(securityGroupRule.Direction)),
@@ -1738,8 +1822,21 @@ func (s *ClusterScopeV2) createSecurityGroupRule(ctx context.Context, securityGr
17381822
}
17391823
options.SetSecurityGroupRulePrototype(prototype)
17401824
default:
1741-
// This should not be possible, provided the strict kubebuilder enforcements
1742-
return fmt.Errorf("error failed creating security group rule, unknown protocol")
1825+
// Check if protocol is part of the supported list else error
1826+
// If part of the supported list add it as Individual prototype
1827+
matched := s.individualSgrRegexp.MatchString(string(securityGroupRulePrototype.Protocol))
1828+
1829+
if matched {
1830+
prototype := &vpcv1.SecurityGroupRulePrototypeSecurityGroupRuleProtocolIndividualPrototype{
1831+
Direction: ptr.To(string(securityGroupRule.Direction)),
1832+
Protocol: ptr.To(string(securityGroupRulePrototype.Protocol)),
1833+
Remote: prototypeRemote,
1834+
}
1835+
options.SetSecurityGroupRulePrototype(prototype)
1836+
} else {
1837+
// This should not be possible, provided the strict kubebuilder enforcements
1838+
return fmt.Errorf("error failed creating security group rule, unknown protocol")
1839+
}
17431840
}
17441841

17451842
log.V(3).Info("Creating Security Group Rule for Security Group", "securityGroupID", securityGroupID, "direction", securityGroupRule.Direction, "protocol", securityGroupRulePrototype.Protocol, "prototypeRemote", prototypeRemote)
@@ -1753,6 +1850,9 @@ func (s *ClusterScopeV2) createSecurityGroupRule(ctx context.Context, securityGr
17531850
// Typecast the resulting SecurityGroupRuleIntf, to retrieve the ID for logging
17541851
var ruleID *string
17551852
switch reflect.TypeOf(securityGroupRuleIntfDetails).String() {
1853+
case infrav1.VPCSecurityGroupRuleProtocolAnyType:
1854+
rule := securityGroupRuleIntfDetails.(*vpcv1.SecurityGroupRuleProtocolAny)
1855+
ruleID = rule.ID
17561856
case infrav1.VPCSecurityGroupRuleProtocolIcmptcpudpType:
17571857
rule := securityGroupRuleIntfDetails.(*vpcv1.SecurityGroupRuleProtocolIcmptcpudp)
17581858
ruleID = rule.ID
@@ -1762,6 +1862,9 @@ func (s *ClusterScopeV2) createSecurityGroupRule(ctx context.Context, securityGr
17621862
case infrav1.VPCSecurityGroupRuleProtocolTcpudpType:
17631863
rule := securityGroupRuleIntfDetails.(*vpcv1.SecurityGroupRuleSecurityGroupRuleProtocolTcpudp)
17641864
ruleID = rule.ID
1865+
case infrav1.VPCSecurityGroupRuleProtocolIndividualType:
1866+
rule := securityGroupRuleIntfDetails.(*vpcv1.SecurityGroupRuleProtocolIndividual)
1867+
ruleID = rule.ID
17651868
}
17661869
log.V(3).Info("Created Security Group Rule", "ruleID", ruleID)
17671870
return nil

‎config/crd/bases/infrastructure.cluster.x-k8s.io_ibmvpcclusters.yaml‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -898,11 +898,7 @@ spec:
898898
protocol:
899899
description: protocol defines the traffic protocol
900900
used for the Security Group Rule.
901-
enum:
902-
- icmp_tcp_udp
903-
- icmp
904-
- tcp
905-
- udp
901+
pattern: ^(any|icmp_tcp_udp|icmp|tcp|udp|ah|esp|gre|ip_in_ip|l2tp|rsvp|sctp|vrrp|number_(?:0|2|3|5|[7-9]|1[0-6]|1[8-9]|[2-3][0-9]|4[0-5]|4[89]|5[2-9]|[6-9][0-9]|10[0-9]|11[0-1]|11[3-4]|11[6-9]|12[0-9]|13[0-1]|13[3-9]|1[4-9][0-9]|2[0-4][0-9]|25[0-5]))$
906902
type: string
907903
remotes:
908904
description: |-
@@ -977,6 +973,9 @@ spec:
977973
protocol
978974
rule: 'self.protocol == ''icmp'' ? !has(self.portRange)
979975
: true'
976+
- message: portRange is not valid for protocol
977+
rule: '(self.protocol != ''tcp'' && self.protocol
978+
!= ''udp'') ? !has(self.portRange) : true'
980979
- message: portRange is not valid for VPCSecurityGroupRuleProtocolIcmpTCPUDP
981980
protocol
982981
rule: 'self.protocol == ''icmp_tcp_udp'' ? !has(self.portRange)
@@ -1035,11 +1034,7 @@ spec:
10351034
protocol:
10361035
description: protocol defines the traffic protocol
10371036
used for the Security Group Rule.
1038-
enum:
1039-
- icmp_tcp_udp
1040-
- icmp
1041-
- tcp
1042-
- udp
1037+
pattern: ^(any|icmp_tcp_udp|icmp|tcp|udp|ah|esp|gre|ip_in_ip|l2tp|rsvp|sctp|vrrp|number_(?:0|2|3|5|[7-9]|1[0-6]|1[8-9]|[2-3][0-9]|4[0-5]|4[89]|5[2-9]|[6-9][0-9]|10[0-9]|11[0-1]|11[3-4]|11[6-9]|12[0-9]|13[0-1]|13[3-9]|1[4-9][0-9]|2[0-4][0-9]|25[0-5]))$
10431038
type: string
10441039
remotes:
10451040
description: |-
@@ -1114,6 +1109,9 @@ spec:
11141109
protocol
11151110
rule: 'self.protocol == ''icmp'' ? !has(self.portRange)
11161111
: true'
1112+
- message: portRange is not valid for protocol
1113+
rule: '(self.protocol != ''tcp'' && self.protocol
1114+
!= ''udp'') ? !has(self.portRange) : true'
11171115
- message: portRange is not valid for VPCSecurityGroupRuleProtocolIcmpTCPUDP
11181116
protocol
11191117
rule: 'self.protocol == ''icmp_tcp_udp'' ? !has(self.portRange)

0 commit comments

Comments
 (0)