Skip to content

Commit 65c2014

Browse files
Danil-GrigorevDanil-Grigorev
authored
✨ Allow more permissive extensibility for securityRules and additional CP LoadBalancers (#5525)
* Allow more permissive extensibility for securityRules Signed-off-by: Danil-Grigorev <[email protected]> * Add field for additional LB ports to CP nodes Signed-off-by: Danil-Grigorev <[email protected]> * Add e2e test for RKE2 ClusterClass Signed-off-by: Danil-Grigorev <[email protected]> * Add tests for SetControlPlaneSecurityRules Signed-off-by: Danil-Grigorev <[email protected]> * Add LB units for additonal ports Signed-off-by: Danil-Grigorev <[email protected]> * Increase timeout for RKE2 CP wait Signed-off-by: Danil-Grigorev <[email protected]> --------- Signed-off-by: Danil-Grigorev <[email protected]>
1 parent 0c26c46 commit 65c2014

29 files changed

+1796
-56
lines changed

‎api/v1beta1/types.go‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,9 +111,23 @@ type NetworkSpec struct {
111111
// +optional
112112
ControlPlaneOutboundLB *LoadBalancerSpec `json:"controlPlaneOutboundLB,omitempty"`
113113

114+
// AdditionalAPIServerLBPorts specifies extra inbound ports for the APIServer load balancer.
115+
// Each port specified (e.g., 9345) creates an inbound rule where the frontend port and the backend port are the same.
116+
// +optional
117+
AdditionalAPIServerLBPorts []LoadBalancerPort `json:"additionalAPIServerLBPorts,omitempty"`
118+
114119
NetworkClassSpec `json:",inline"`
115120
}
116121

122+
// LoadBalancerPort specifies additional port for the API server load balancer.
123+
type LoadBalancerPort struct {
124+
// Name for the additional port within LB definition
125+
Name string `json:"name"`
126+
127+
// Port for the LB definition
128+
Port int32 `json:"port"`
129+
}
130+
117131
// VnetSpec configures an Azure virtual network.
118132
type VnetSpec struct {
119133
// ResourceGroup is the name of the resource group of the existing virtual network
@@ -893,6 +907,17 @@ func (s SubnetSpec) IsIPv6Enabled() bool {
893907
return false
894908
}
895909

910+
// GetSecurityRuleByDestination returns security group rule, which matches provided destination ports.
911+
func (s SubnetSpec) GetSecurityRuleByDestination(port string) *SecurityRule {
912+
for _, rule := range s.SecurityGroup.SecurityRules {
913+
if rule.DestinationPorts != nil && *rule.DestinationPorts == port {
914+
return &rule
915+
}
916+
}
917+
918+
return nil
919+
}
920+
896921
// SecurityProfile specifies the Security profile settings for a
897922
// virtual machine or virtual machine scale set.
898923
type SecurityProfile struct {

‎api/v1beta1/types_template.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,11 @@ type NetworkTemplateSpec struct {
7575
// This is different from APIServerLB, and is used only in private clusters (optionally) for enabling outbound traffic.
7676
// +optional
7777
ControlPlaneOutboundLB *LoadBalancerClassSpec `json:"controlPlaneOutboundLB,omitempty"`
78+
79+
// AdditionalAPIServerLBPorts is the configuration for the additional inbound control-plane load balancer ports
80+
// Each port specified (e.g., 9345) creates an inbound rule where the frontend port and the backend port are the same.
81+
// +optional
82+
AdditionalAPIServerLBPorts []LoadBalancerPort `json:"additionalAPIServerLBPorts,omitempty"`
7883
}
7984

8085
// GetSubnetTemplate returns the subnet template based on the subnet role.

‎api/v1beta1/zz_generated.deepcopy.go‎

Lines changed: 25 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎azure/scope/cluster.go‎

Lines changed: 35 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,7 @@ func (s *ClusterScope) LBSpecs() []azure.ResourceSpecGetter {
266266
BackendPoolName: s.APIServerLB().BackendPool.Name,
267267
IdleTimeoutInMinutes: s.APIServerLB().IdleTimeoutInMinutes,
268268
AdditionalTags: s.AdditionalTags(),
269+
AdditionalPorts: s.AdditionalAPIServerLBPorts(),
269270
}
270271

271272
if s.APIServerLB().FrontendIPs != nil {
@@ -299,6 +300,7 @@ func (s *ClusterScope) LBSpecs() []azure.ResourceSpecGetter {
299300
BackendPoolName: s.APIServerLB().BackendPool.Name + "-internal",
300301
IdleTimeoutInMinutes: s.APIServerLB().IdleTimeoutInMinutes,
301302
AdditionalTags: s.AdditionalTags(),
303+
AdditionalPorts: s.AdditionalAPIServerLBPorts(),
302304
}
303305

304306
privateIPFound := false
@@ -771,6 +773,11 @@ func (s *ClusterScope) ControlPlaneOutboundLB() *infrav1.LoadBalancerSpec {
771773
return s.AzureCluster.Spec.NetworkSpec.ControlPlaneOutboundLB
772774
}
773775

776+
// AdditionalAPIServerLBPorts returns the additional API server ports list.
777+
func (s *ClusterScope) AdditionalAPIServerLBPorts() []infrav1.LoadBalancerPort {
778+
return s.AzureCluster.Spec.NetworkSpec.AdditionalAPIServerLBPorts
779+
}
780+
774781
// APIServerLBName returns the API Server LB name.
775782
func (s *ClusterScope) APIServerLBName() string {
776783
apiServerLB := s.APIServerLB()
@@ -1020,9 +1027,12 @@ func (s *ClusterScope) SetControlPlaneSecurityRules() {
10201027
if !s.ControlPlaneEnabled() {
10211028
return
10221029
}
1023-
if s.ControlPlaneSubnet().SecurityGroup.SecurityRules == nil {
1024-
subnet := s.ControlPlaneSubnet()
1025-
subnet.SecurityGroup.SecurityRules = infrav1.SecurityRules{
1030+
1031+
subnet := s.ControlPlaneSubnet()
1032+
1033+
missingSSH := subnet.GetSecurityRuleByDestination("22") == nil
1034+
if missingSSH {
1035+
subnet.SecurityGroup.SecurityRules = append(subnet.SecurityGroup.SecurityRules,
10261036
infrav1.SecurityRule{
10271037
Name: "allow_ssh",
10281038
Description: "Allow SSH",
@@ -1034,20 +1044,28 @@ func (s *ClusterScope) SetControlPlaneSecurityRules() {
10341044
Destination: ptr.To("*"),
10351045
DestinationPorts: ptr.To("22"),
10361046
Action: infrav1.SecurityRuleActionAllow,
1037-
},
1038-
infrav1.SecurityRule{
1039-
Name: "allow_apiserver",
1040-
Description: "Allow K8s API Server",
1041-
Priority: 2201,
1042-
Protocol: infrav1.SecurityGroupProtocolTCP,
1043-
Direction: infrav1.SecurityRuleDirectionInbound,
1044-
Source: ptr.To("*"),
1045-
SourcePorts: ptr.To("*"),
1046-
Destination: ptr.To("*"),
1047-
DestinationPorts: ptr.To(strconv.Itoa(int(s.APIServerPort()))),
1048-
Action: infrav1.SecurityRuleActionAllow,
1049-
},
1050-
}
1047+
})
1048+
}
1049+
1050+
port := strconv.Itoa(int(s.APIServerPort()))
1051+
1052+
missingAPIPort := subnet.GetSecurityRuleByDestination(port) == nil
1053+
if missingAPIPort {
1054+
subnet.SecurityGroup.SecurityRules = append(subnet.SecurityGroup.SecurityRules, infrav1.SecurityRule{
1055+
Name: "allow_apiserver",
1056+
Description: "Allow K8s API Server",
1057+
Priority: 2201,
1058+
Protocol: infrav1.SecurityGroupProtocolTCP,
1059+
Direction: infrav1.SecurityRuleDirectionInbound,
1060+
Source: ptr.To("*"),
1061+
SourcePorts: ptr.To("*"),
1062+
Destination: ptr.To("*"),
1063+
DestinationPorts: ptr.To(port),
1064+
Action: infrav1.SecurityRuleActionAllow,
1065+
})
1066+
}
1067+
1068+
if missingSSH || missingAPIPort {
10511069
s.AzureCluster.Spec.NetworkSpec.UpdateControlPlaneSubnet(subnet)
10521070
}
10531071
}

‎azure/scope/cluster_test.go‎

Lines changed: 147 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -242,50 +242,165 @@ func TestAPIServerHost(t *testing.T) {
242242
}
243243

244244
func TestGettingSecurityRules(t *testing.T) {
245-
g := NewWithT(t)
246-
247-
cluster := &clusterv1.Cluster{
248-
ObjectMeta: metav1.ObjectMeta{
249-
Name: "my-cluster",
250-
Namespace: "default",
245+
tests := []struct {
246+
name string
247+
cluster *clusterv1.Cluster
248+
azureCluster *infrav1.AzureCluster
249+
expectedRuleCount int
250+
}{
251+
{
252+
name: "default control plane subnet with no rules should have 2 security rules defaulted",
253+
cluster: &clusterv1.Cluster{
254+
ObjectMeta: metav1.ObjectMeta{
255+
Name: "my-cluster",
256+
Namespace: "default",
257+
},
258+
},
259+
azureCluster: &infrav1.AzureCluster{
260+
ObjectMeta: metav1.ObjectMeta{
261+
Name: "my-azure-cluster",
262+
},
263+
Spec: infrav1.AzureClusterSpec{
264+
AzureClusterClassSpec: infrav1.AzureClusterClassSpec{
265+
SubscriptionID: "123",
266+
IdentityRef: &corev1.ObjectReference{
267+
Kind: infrav1.AzureClusterIdentityKind,
268+
},
269+
},
270+
ControlPlaneEnabled: true,
271+
NetworkSpec: infrav1.NetworkSpec{
272+
Subnets: infrav1.Subnets{
273+
{
274+
SubnetClassSpec: infrav1.SubnetClassSpec{
275+
Role: infrav1.SubnetNode,
276+
Name: "node",
277+
},
278+
},
279+
},
280+
},
281+
},
282+
},
283+
expectedRuleCount: 2,
251284
},
252-
}
253-
254-
azureCluster := &infrav1.AzureCluster{
255-
ObjectMeta: metav1.ObjectMeta{
256-
Name: "my-azure-cluster",
285+
{
286+
name: "additional rules are preserved",
287+
cluster: &clusterv1.Cluster{
288+
ObjectMeta: metav1.ObjectMeta{
289+
Name: "my-cluster",
290+
Namespace: "default",
291+
},
292+
},
293+
azureCluster: &infrav1.AzureCluster{
294+
ObjectMeta: metav1.ObjectMeta{
295+
Name: "my-azure-cluster",
296+
},
297+
Spec: infrav1.AzureClusterSpec{
298+
AzureClusterClassSpec: infrav1.AzureClusterClassSpec{
299+
SubscriptionID: "123",
300+
IdentityRef: &corev1.ObjectReference{
301+
Kind: infrav1.AzureClusterIdentityKind,
302+
},
303+
},
304+
ControlPlaneEnabled: true,
305+
NetworkSpec: infrav1.NetworkSpec{
306+
Subnets: infrav1.Subnets{
307+
{
308+
SecurityGroup: infrav1.SecurityGroup{
309+
SecurityGroupClass: infrav1.SecurityGroupClass{
310+
SecurityRules: []infrav1.SecurityRule{{
311+
Name: "allow_9345",
312+
Description: "Allow port 9345",
313+
Priority: 2200,
314+
Protocol: infrav1.SecurityGroupProtocolTCP,
315+
Direction: infrav1.SecurityRuleDirectionInbound,
316+
Source: ptr.To("*"),
317+
SourcePorts: ptr.To("*"),
318+
Destination: ptr.To("*"),
319+
DestinationPorts: ptr.To("9345"),
320+
Action: infrav1.SecurityRuleActionAllow,
321+
}},
322+
},
323+
},
324+
SubnetClassSpec: infrav1.SubnetClassSpec{
325+
Role: infrav1.SubnetControlPlane,
326+
Name: string(infrav1.SubnetControlPlane),
327+
},
328+
},
329+
},
330+
},
331+
},
332+
},
333+
expectedRuleCount: 3,
257334
},
258-
Spec: infrav1.AzureClusterSpec{
259-
AzureClusterClassSpec: infrav1.AzureClusterClassSpec{
260-
SubscriptionID: "123",
261-
IdentityRef: &corev1.ObjectReference{
262-
Kind: infrav1.AzureClusterIdentityKind,
335+
{
336+
name: "override rules are accepted",
337+
cluster: &clusterv1.Cluster{
338+
ObjectMeta: metav1.ObjectMeta{
339+
Name: "my-cluster",
340+
Namespace: "default",
263341
},
264342
},
265-
ControlPlaneEnabled: true,
266-
NetworkSpec: infrav1.NetworkSpec{
267-
Subnets: infrav1.Subnets{
268-
{
269-
SubnetClassSpec: infrav1.SubnetClassSpec{
270-
Role: infrav1.SubnetNode,
271-
Name: "node",
343+
azureCluster: &infrav1.AzureCluster{
344+
ObjectMeta: metav1.ObjectMeta{
345+
Name: "my-azure-cluster",
346+
},
347+
Spec: infrav1.AzureClusterSpec{
348+
AzureClusterClassSpec: infrav1.AzureClusterClassSpec{
349+
SubscriptionID: "123",
350+
IdentityRef: &corev1.ObjectReference{
351+
Kind: infrav1.AzureClusterIdentityKind,
352+
},
353+
},
354+
ControlPlaneEnabled: true,
355+
NetworkSpec: infrav1.NetworkSpec{
356+
Subnets: infrav1.Subnets{
357+
{
358+
SecurityGroup: infrav1.SecurityGroup{
359+
SecurityGroupClass: infrav1.SecurityGroupClass{
360+
SecurityRules: []infrav1.SecurityRule{{
361+
Name: "deny_ssh",
362+
Description: "Deny SSH",
363+
Priority: 2200,
364+
Protocol: infrav1.SecurityGroupProtocolTCP,
365+
Direction: infrav1.SecurityRuleDirectionInbound,
366+
Source: ptr.To("*"),
367+
SourcePorts: ptr.To("*"),
368+
Destination: ptr.To("*"),
369+
DestinationPorts: ptr.To("22"),
370+
Action: infrav1.SecurityRuleActionDeny,
371+
}},
372+
},
373+
},
374+
SubnetClassSpec: infrav1.SubnetClassSpec{
375+
Role: infrav1.SubnetControlPlane,
376+
Name: string(infrav1.SubnetControlPlane),
377+
},
378+
},
272379
},
273380
},
274381
},
275382
},
383+
expectedRuleCount: 2,
276384
},
277385
}
278-
azureCluster.Default()
279386

280-
clusterScope := &ClusterScope{
281-
Cluster: cluster,
282-
AzureCluster: azureCluster,
283-
}
284-
clusterScope.SetControlPlaneSecurityRules()
387+
for _, tt := range tests {
388+
t.Run(tt.name, func(t *testing.T) {
389+
g := NewWithT(t)
285390

286-
subnet, err := clusterScope.AzureCluster.Spec.NetworkSpec.GetControlPlaneSubnet()
287-
g.Expect(err).NotTo(HaveOccurred())
288-
g.Expect(subnet.SecurityGroup.SecurityRules).To(HaveLen(2))
391+
tt.azureCluster.Default()
392+
393+
clusterScope := &ClusterScope{
394+
Cluster: tt.cluster,
395+
AzureCluster: tt.azureCluster,
396+
}
397+
clusterScope.SetControlPlaneSecurityRules()
398+
399+
subnet, err := clusterScope.AzureCluster.Spec.NetworkSpec.GetControlPlaneSubnet()
400+
g.Expect(err).NotTo(HaveOccurred())
401+
g.Expect(subnet.SecurityGroup.SecurityRules).To(HaveLen(tt.expectedRuleCount))
402+
})
403+
}
289404
}
290405

291406
func TestPublicIPSpecs(t *testing.T) {

0 commit comments

Comments
 (0)