Skip to content

Commit b408363

Browse files
Danil-GrigorevDanil-Grigorev
committed
Add tests for SetControlPlaneSecurityRules
Signed-off-by: Danil-Grigorev <[email protected]>
1 parent cb32d14 commit b408363

File tree

3 files changed

+158
-43
lines changed

3 files changed

+158
-43
lines changed

‎api/v1beta1/types.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -908,9 +908,9 @@ func (s SubnetSpec) IsIPv6Enabled() bool {
908908
}
909909

910910
// GetSecurityRuleByDestination returns security group rule, which matches provided destination ports.
911-
func (s SubnetSpec) GetSecurityRuleByDestination(ports string) *SecurityRule {
911+
func (s SubnetSpec) GetSecurityRuleByDestination(port string) *SecurityRule {
912912
for _, rule := range s.SecurityGroup.SecurityRules {
913-
if rule.DestinationPorts != nil && *rule.DestinationPorts == ports {
913+
if rule.DestinationPorts != nil && *rule.DestinationPorts == port {
914914
return &rule
915915
}
916916
}

‎azure/scope/cluster.go‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -266,7 +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.ControlPlaneAdditionalLBPorts(),
269+
AdditionalPorts: s.AdditionalAPIServerLBPorts(),
270270
}
271271

272272
if s.APIServerLB().FrontendIPs != nil {
@@ -300,7 +300,7 @@ func (s *ClusterScope) LBSpecs() []azure.ResourceSpecGetter {
300300
BackendPoolName: s.APIServerLB().BackendPool.Name + "-internal",
301301
IdleTimeoutInMinutes: s.APIServerLB().IdleTimeoutInMinutes,
302302
AdditionalTags: s.AdditionalTags(),
303-
AdditionalPorts: s.ControlPlaneAdditionalLBPorts(),
303+
AdditionalPorts: s.AdditionalAPIServerLBPorts(),
304304
}
305305

306306
privateIPFound := false
@@ -773,8 +773,8 @@ func (s *ClusterScope) ControlPlaneOutboundLB() *infrav1.LoadBalancerSpec {
773773
return s.AzureCluster.Spec.NetworkSpec.ControlPlaneOutboundLB
774774
}
775775

776-
// ControlPlaneAdditionalLBPorts returns the additional API server ports list.
777-
func (s *ClusterScope) ControlPlaneAdditionalLBPorts() []infrav1.LoadBalancerPort {
776+
// AdditionalAPIServerLBPorts returns the additional API server ports list.
777+
func (s *ClusterScope) AdditionalAPIServerLBPorts() []infrav1.LoadBalancerPort {
778778
return s.AzureCluster.Spec.NetworkSpec.AdditionalAPIServerLBPorts
779779
}
780780

@@ -1030,8 +1030,8 @@ func (s *ClusterScope) SetControlPlaneSecurityRules() {
10301030

10311031
subnet := s.ControlPlaneSubnet()
10321032

1033-
missing_ssh := subnet.GetSecurityRuleByDestination("22") == nil
1034-
if missing_ssh {
1033+
missingSSH := subnet.GetSecurityRuleByDestination("22") == nil
1034+
if missingSSH {
10351035
subnet.SecurityGroup.SecurityRules = append(subnet.SecurityGroup.SecurityRules,
10361036
infrav1.SecurityRule{
10371037
Name: "allow_ssh",
@@ -1049,8 +1049,8 @@ func (s *ClusterScope) SetControlPlaneSecurityRules() {
10491049

10501050
port := strconv.Itoa(int(s.APIServerPort()))
10511051

1052-
missing_api_port := subnet.GetSecurityRuleByDestination(port) == nil
1053-
if missing_api_port {
1052+
missingAPIPort := subnet.GetSecurityRuleByDestination(port) == nil
1053+
if missingAPIPort {
10541054
subnet.SecurityGroup.SecurityRules = append(subnet.SecurityGroup.SecurityRules, infrav1.SecurityRule{
10551055
Name: "allow_apiserver",
10561056
Description: "Allow K8s API Server",
@@ -1065,7 +1065,7 @@ func (s *ClusterScope) SetControlPlaneSecurityRules() {
10651065
})
10661066
}
10671067

1068-
if missing_ssh || missing_api_port {
1068+
if missingSSH || missingAPIPort {
10691069
s.AzureCluster.Spec.NetworkSpec.UpdateControlPlaneSubnet(subnet)
10701070
}
10711071
}

‎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)