Add tests for SetControlPlaneSecurityRules

Signed-off-by: Danil-Grigorev <[email protected]>

Author: Danil-Grigorev

azure/scope/cluster.go

@@ -266,7 +266,7 @@ func (s *ClusterScope) LBSpecs() []azure.ResourceSpecGetter {
 			BackendPoolName:      s.APIServerLB().BackendPool.Name,
 			IdleTimeoutInMinutes: s.APIServerLB().IdleTimeoutInMinutes,
 			AdditionalTags:       s.AdditionalTags(),
-			AdditionalPorts:      s.ControlPlaneAdditionalLBPorts(),
+			AdditionalPorts:      s.AdditionalAPIServerLBPorts(),
 		}
 
 		if s.APIServerLB().FrontendIPs != nil {
@@ -300,7 +300,7 @@ func (s *ClusterScope) LBSpecs() []azure.ResourceSpecGetter {
 			BackendPoolName:      s.APIServerLB().BackendPool.Name + "-internal",
 			IdleTimeoutInMinutes: s.APIServerLB().IdleTimeoutInMinutes,
 			AdditionalTags:       s.AdditionalTags(),
-			AdditionalPorts:      s.ControlPlaneAdditionalLBPorts(),
+			AdditionalPorts:      s.AdditionalAPIServerLBPorts(),
 		}
 
 		privateIPFound := false
@@ -773,8 +773,8 @@ func (s *ClusterScope) ControlPlaneOutboundLB() *infrav1.LoadBalancerSpec {
 	return s.AzureCluster.Spec.NetworkSpec.ControlPlaneOutboundLB
 }
 
-// ControlPlaneAdditionalLBPorts returns the additional API server ports list.
-func (s *ClusterScope) ControlPlaneAdditionalLBPorts() []infrav1.LoadBalancerPort {
+// AdditionalAPIServerLBPorts returns the additional API server ports list.
+func (s *ClusterScope) AdditionalAPIServerLBPorts() []infrav1.LoadBalancerPort {
 	return s.AzureCluster.Spec.NetworkSpec.AdditionalAPIServerLBPorts
 }
 

azure/scope/cluster_test.go

@@ -242,50 +242,165 @@ func TestAPIServerHost(t *testing.T) {
 }
 
 func TestGettingSecurityRules(t *testing.T) {
-	g := NewWithT(t)
-
-	cluster := &clusterv1.Cluster{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "my-cluster",
-			Namespace: "default",
+	tests := []struct {
+		name              string
+		cluster           *clusterv1.Cluster
+		azureCluster      *infrav1.AzureCluster
+		expectedRuleCount int
+	}{
+		{
+			name: "default control plane subnet with no rules should have 2 security rules defaulted",
+			cluster: &clusterv1.Cluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "my-cluster",
+					Namespace: "default",
+				},
+			},
+			azureCluster: &infrav1.AzureCluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "my-azure-cluster",
+				},
+				Spec: infrav1.AzureClusterSpec{
+					AzureClusterClassSpec: infrav1.AzureClusterClassSpec{
+						SubscriptionID: "123",
+						IdentityRef: &corev1.ObjectReference{
+							Kind: infrav1.AzureClusterIdentityKind,
+						},
+					},
+					ControlPlaneEnabled: true,
+					NetworkSpec: infrav1.NetworkSpec{
+						Subnets: infrav1.Subnets{
+							{
+								SubnetClassSpec: infrav1.SubnetClassSpec{
+									Role: infrav1.SubnetNode,
+									Name: "node",
+								},
+							},
+						},
+					},
+				},
+			},
+			expectedRuleCount: 2,
 		},
-	}
-
-	azureCluster := &infrav1.AzureCluster{
-		ObjectMeta: metav1.ObjectMeta{
-			Name: "my-azure-cluster",
+		{
+			name: "additional rules are preserved",
+			cluster: &clusterv1.Cluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "my-cluster",
+					Namespace: "default",
+				},
+			},
+			azureCluster: &infrav1.AzureCluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "my-azure-cluster",
+				},
+				Spec: infrav1.AzureClusterSpec{
+					AzureClusterClassSpec: infrav1.AzureClusterClassSpec{
+						SubscriptionID: "123",
+						IdentityRef: &corev1.ObjectReference{
+							Kind: infrav1.AzureClusterIdentityKind,
+						},
+					},
+					ControlPlaneEnabled: true,
+					NetworkSpec: infrav1.NetworkSpec{
+						Subnets: infrav1.Subnets{
+							{
+								SecurityGroup: infrav1.SecurityGroup{
+									SecurityGroupClass: infrav1.SecurityGroupClass{
+										SecurityRules: []infrav1.SecurityRule{{
+											Name:             "allow_9345",
+											Description:      "Allow port 9345",
+											Priority:         2200,
+											Protocol:         infrav1.SecurityGroupProtocolTCP,
+											Direction:        infrav1.SecurityRuleDirectionInbound,
+											Source:           ptr.To("*"),
+											SourcePorts:      ptr.To("*"),
+											Destination:      ptr.To("*"),
+											DestinationPorts: ptr.To("9345"),
+											Action:           infrav1.SecurityRuleActionAllow,
+										}},
+									},
+								},
+								SubnetClassSpec: infrav1.SubnetClassSpec{
+									Role: infrav1.SubnetControlPlane,
+									Name: string(infrav1.SubnetControlPlane),
+								},
+							},
+						},
+					},
+				},
+			},
+			expectedRuleCount: 3,
 		},
-		Spec: infrav1.AzureClusterSpec{
-			AzureClusterClassSpec: infrav1.AzureClusterClassSpec{
-				SubscriptionID: "123",
-				IdentityRef: &corev1.ObjectReference{
-					Kind: infrav1.AzureClusterIdentityKind,
+		{
+			name: "override rules are accepted",
+			cluster: &clusterv1.Cluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "my-cluster",
+					Namespace: "default",
 				},
 			},
-			ControlPlaneEnabled: true,
-			NetworkSpec: infrav1.NetworkSpec{
-				Subnets: infrav1.Subnets{
-					{
-						SubnetClassSpec: infrav1.SubnetClassSpec{
-							Role: infrav1.SubnetNode,
-							Name: "node",
+			azureCluster: &infrav1.AzureCluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "my-azure-cluster",
+				},
+				Spec: infrav1.AzureClusterSpec{
+					AzureClusterClassSpec: infrav1.AzureClusterClassSpec{
+						SubscriptionID: "123",
+						IdentityRef: &corev1.ObjectReference{
+							Kind: infrav1.AzureClusterIdentityKind,
+						},
+					},
+					ControlPlaneEnabled: true,
+					NetworkSpec: infrav1.NetworkSpec{
+						Subnets: infrav1.Subnets{
+							{
+								SecurityGroup: infrav1.SecurityGroup{
+									SecurityGroupClass: infrav1.SecurityGroupClass{
+										SecurityRules: []infrav1.SecurityRule{{
+											Name:             "deny_ssh",
+											Description:      "Deny SSH",
+											Priority:         2200,
+											Protocol:         infrav1.SecurityGroupProtocolTCP,
+											Direction:        infrav1.SecurityRuleDirectionInbound,
+											Source:           ptr.To("*"),
+											SourcePorts:      ptr.To("*"),
+											Destination:      ptr.To("*"),
+											DestinationPorts: ptr.To("22"),
+											Action:           infrav1.SecurityRuleActionDeny,
+										}},
+									},
+								},
+								SubnetClassSpec: infrav1.SubnetClassSpec{
+									Role: infrav1.SubnetControlPlane,
+									Name: string(infrav1.SubnetControlPlane),
+								},
+							},
 						},
 					},
 				},
 			},
+			expectedRuleCount: 2,
 		},
 	}
-	azureCluster.Default()
 
-	clusterScope := &ClusterScope{
-		Cluster:      cluster,
-		AzureCluster: azureCluster,
-	}
-	clusterScope.SetControlPlaneSecurityRules()
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
 
-	subnet, err := clusterScope.AzureCluster.Spec.NetworkSpec.GetControlPlaneSubnet()
-	g.Expect(err).NotTo(HaveOccurred())
-	g.Expect(subnet.SecurityGroup.SecurityRules).To(HaveLen(2))
+			tt.azureCluster.Default()
+
+			clusterScope := &ClusterScope{
+				Cluster:      tt.cluster,
+				AzureCluster: tt.azureCluster,
+			}
+			clusterScope.SetControlPlaneSecurityRules()
+
+			subnet, err := clusterScope.AzureCluster.Spec.NetworkSpec.GetControlPlaneSubnet()
+			g.Expect(err).NotTo(HaveOccurred())
+			g.Expect(subnet.SecurityGroup.SecurityRules).To(HaveLen(tt.expectedRuleCount))
+		})
+	}
 }
 
 func TestPublicIPSpecs(t *testing.T) {