✨ Add optional direction field to securityRules to allow Inbound and …

…Outbound rules

api/v1alpha3/azurecluster_conversion.go

@@ -58,6 +58,20 @@ func (src *AzureCluster) ConvertTo(dstRaw conversion.Hub) error { // nolint
 	dst.Spec.NetworkSpec.APIServerLB.FrontendIPsCount = restored.Spec.NetworkSpec.APIServerLB.FrontendIPsCount
 	dst.Spec.NetworkSpec.NodeOutboundLB = restored.Spec.NetworkSpec.NodeOutboundLB
 
+	for i, restoredSubnet := range restored.Spec.NetworkSpec.Subnets {
+		for j, dstSubnet := range dst.Spec.NetworkSpec.Subnets {
+			if dstSubnet.Name == restoredSubnet.Name {
+				for k, restoredSecurityRule := range restoredSubnet.SecurityGroup.SecurityRules {
+					for l, dstSecurityRule := range dstSubnet.SecurityGroup.SecurityRules {
+						if dstSecurityRule.Name == restoredSecurityRule.Name {
+							dst.Spec.NetworkSpec.Subnets[j].SecurityGroup.SecurityRules[l].Direction = restored.Spec.NetworkSpec.Subnets[i].SecurityGroup.SecurityRules[k].Direction
+						}
+					}
+				}
+			}
+		}
+	}
+
 	return nil
 }
 

api/v1alpha4/azurecluster_default.go

@@ -94,6 +94,7 @@ func (c *AzureCluster) setSubnetDefaults() {
 	if cpSubnet.SecurityGroup.Name == "" {
 		cpSubnet.SecurityGroup.Name = generateControlPlaneSecurityGroupName(c.ObjectMeta.Name)
 	}
+	setSecurityRuleDefaults(&cpSubnet.SecurityGroup)
 
 	if nodeSubnet.Name == "" {
 		nodeSubnet.Name = generateNodeSubnetName(c.ObjectMeta.Name)
@@ -104,6 +105,7 @@ func (c *AzureCluster) setSubnetDefaults() {
 	if nodeSubnet.SecurityGroup.Name == "" {
 		nodeSubnet.SecurityGroup.Name = generateNodeSecurityGroupName(c.ObjectMeta.Name)
 	}
+	setSecurityRuleDefaults(&nodeSubnet.SecurityGroup)
 	if nodeSubnet.RouteTable.Name == "" {
 		nodeSubnet.RouteTable.Name = generateNodeRouteTableName(c.ObjectMeta.Name)
 	}
@@ -112,6 +114,14 @@ func (c *AzureCluster) setSubnetDefaults() {
 	c.Spec.NetworkSpec.UpdateNodeSubnet(nodeSubnet)
 }
 
+func setSecurityRuleDefaults(sg *SecurityGroup) {
+	for i := range sg.SecurityRules {
+		if sg.SecurityRules[i].Direction == "" {
+			sg.SecurityRules[i].Direction = SecurityRuleDirectionInbound
+		}
+	}
+}
+
 func (c *AzureCluster) setAPIServerLBDefaults() {
 	lb := &c.Spec.NetworkSpec.APIServerLB
 	if lb.Type == "" {

api/v1alpha4/azurecluster_default_test.go

@@ -21,6 +21,8 @@ import (
 	"reflect"
 	"testing"
 
+	"github.com/Azure/go-autorest/autorest/to"
+
 	"k8s.io/utils/pointer"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -527,6 +529,78 @@ func TestSubnetDefaults(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "subnets with custom security group",
+			cluster: &AzureCluster{
+				ObjectMeta: v1.ObjectMeta{
+					Name: "cluster-test",
+				},
+				Spec: AzureClusterSpec{
+					NetworkSpec: NetworkSpec{
+						Subnets: Subnets{
+							{
+								Name: "cluster-test-controlplane-subnet",
+								Role: "control-plane",
+								SecurityGroup: SecurityGroup{
+									Name: "my-custom-sg",
+									SecurityRules: []SecurityRule{
+										{
+											Name:             "allow_port_50000",
+											Description:      "allow port 50000",
+											Protocol:         "*",
+											Priority:         2202,
+											SourcePorts:      to.StringPtr("*"),
+											DestinationPorts: to.StringPtr("*"),
+											Source:           to.StringPtr("*"),
+											Destination:      to.StringPtr("*"),
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			output: &AzureCluster{
+				ObjectMeta: v1.ObjectMeta{
+					Name: "cluster-test",
+				},
+				Spec: AzureClusterSpec{
+					NetworkSpec: NetworkSpec{
+						Subnets: Subnets{
+							{
+								Name:       "cluster-test-controlplane-subnet",
+								Role:       "control-plane",
+								CIDRBlocks: []string{DefaultControlPlaneSubnetCIDR},
+								SecurityGroup: SecurityGroup{
+									Name: "my-custom-sg",
+									SecurityRules: []SecurityRule{
+										{
+											Name:             "allow_port_50000",
+											Description:      "allow port 50000",
+											Protocol:         "*",
+											Priority:         2202,
+											SourcePorts:      to.StringPtr("*"),
+											DestinationPorts: to.StringPtr("*"),
+											Source:           to.StringPtr("*"),
+											Destination:      to.StringPtr("*"),
+											Direction:        SecurityRuleDirectionInbound,
+										},
+									},
+								},
+							},
+							{
+								Role:          SubnetNode,
+								Name:          "cluster-test-node-subnet",
+								CIDRBlocks:    []string{DefaultNodeSubnetCIDR},
+								SecurityGroup: SecurityGroup{Name: "cluster-test-node-nsg"},
+								RouteTable:    RouteTable{Name: "cluster-test-node-routetable"},
+							},
+						},
+					},
+				},
+			},
+		},
 	}
 
 	for _, c := range cases {

api/v1alpha4/types.go

@@ -115,35 +115,48 @@ type RouteTable struct {
 type SecurityGroupProtocol string
 
 const (
-	// SecurityGroupProtocolAll is a wildcard for all IP protocols
+	// SecurityGroupProtocolAll is a wildcard for all IP protocols.
 	SecurityGroupProtocolAll = SecurityGroupProtocol("*")
-
-	// SecurityGroupProtocolTCP represents the TCP protocol in ingress rules
+	// SecurityGroupProtocolTCP represents the TCP protocol.
 	SecurityGroupProtocolTCP = SecurityGroupProtocol("Tcp")
-
-	// SecurityGroupProtocolUDP represents the UDP protocol in ingress rules
+	// SecurityGroupProtocolUDP represents the UDP protocol.
 	SecurityGroupProtocolUDP = SecurityGroupProtocol("Udp")
+	// SecurityGroupProtocolICMP represents the ICMP protocol.
+	SecurityGroupProtocolICMP = SecurityGroupProtocol("Icmp")
 )
 
-// SecurityRule defines an Azure ingress rule for security groups.
-type SecurityRule struct {
-	Name        string                `json:"name"`
-	Description string                `json:"description"`
-	Protocol    SecurityGroupProtocol `json:"protocol"`
+// SecurityRuleDirection defines the direction type for a security group rule.
+type SecurityRuleDirection string
 
-	// Priority - A number between 100 and 4096. Each rule should have a unique value for priority. Rules are processed in priority order, with lower numbers processed before higher numbers. Once traffic matches a rule, processing stops.
-	Priority int32 `json:"priority,omitempty"`
+const (
+	// SecurityRuleDirectionInbound defines an ingress security rule.
+	SecurityRuleDirectionInbound = SecurityRuleDirection("Inbound")
 
-	// SourcePorts - The source port or range. Integer or range between 0 and 65535. Asterix '*' can also be used to match all ports.
-	SourcePorts *string `json:"sourcePorts,omitempty"`
+	// SecurityRuleDirectionOutbound defines an egress security rule.
+	SecurityRuleDirectionOutbound = SecurityRuleDirection("Outbound")
+)
 
-	// DestinationPorts - The destination port or range. Integer or range between 0 and 65535. Asterix '*' can also be used to match all ports.
+// SecurityRule defines an Azure security rule for security groups.
+type SecurityRule struct {
+	// Name is a unique name within the network security group.
+	Name string `json:"name"`
+	// A description for this rule. Restricted to 140 chars.
+	Description string `json:"description"`
+	// Protocol specifies the protocol type. "Tcp", "Udp", "Icmp", or "*".
+	// +kubebuilder:validation:Enum=Tcp;Udp;Icmp;*
+	Protocol SecurityGroupProtocol `json:"protocol"`
+	// Direction indicates whether the rule applies to inbound, or outbound traffic. "Inbound" or "Outbound".
+	// +kubebuilder:validation:Enum=Inbound;Outbound
+	Direction SecurityRuleDirection `json:"direction"`
+	// Priority is a number between 100 and 4096. Each rule should have a unique value for priority. Rules are processed in priority order, with lower numbers processed before higher numbers. Once traffic matches a rule, processing stops.
+	Priority int32 `json:"priority,omitempty"`
+	// SourcePorts specifies source port or range. Integer or range between 0 and 65535. Asterix '*' can also be used to match all ports.
+	SourcePorts *string `json:"sourcePorts,omitempty"`
+	// DestinationPorts specifies the destination port or range. Integer or range between 0 and 65535. Asterix '*' can also be used to match all ports.
 	DestinationPorts *string `json:"destinationPorts,omitempty"`
-
-	// Source - The CIDR or source IP range. Asterix '*' can also be used to match all source IPs. Default tags such as 'VirtualNetwork', 'AzureLoadBalancer' and 'Internet' can also be used. If this is an ingress rule, specifies where network traffic originates from.
+	// Source specifies the CIDR or source IP range. Asterix '*' can also be used to match all source IPs. Default tags such as 'VirtualNetwork', 'AzureLoadBalancer' and 'Internet' can also be used. If this is an ingress rule, specifies where network traffic originates from.
 	Source *string `json:"source,omitempty"`
-
-	// Destination - The destination address prefix. CIDR or destination IP range. Asterix '*' can also be used to match all source IPs. Default tags such as 'VirtualNetwork', 'AzureLoadBalancer' and 'Internet' can also be used.
+	// Destination is the destination address prefix. CIDR or destination IP range. Asterix '*' can also be used to match all source IPs. Default tags such as 'VirtualNetwork', 'AzureLoadBalancer' and 'Internet' can also be used.
 	Destination *string `json:"destination,omitempty"`
 }
 

azure/converters/rules.go

@@ -33,7 +33,6 @@ func SecurityRuleToSDK(rule infrav1.SecurityRule) network.SecurityRule {
 			DestinationAddressPrefix: rule.Destination,
 			DestinationPortRange:     rule.DestinationPorts,
 			Access:                   network.SecurityRuleAccessAllow,
-			Direction:                network.SecurityRuleDirectionInbound,
 			Priority:                 to.Int32Ptr(rule.Priority),
 		},
 	}
@@ -45,6 +44,15 @@ func SecurityRuleToSDK(rule infrav1.SecurityRule) network.SecurityRule {
 		secRule.Protocol = network.SecurityRuleProtocolTCP
 	case infrav1.SecurityGroupProtocolUDP:
 		secRule.Protocol = network.SecurityRuleProtocolUDP
+	case infrav1.SecurityGroupProtocolICMP:
+		secRule.Protocol = network.SecurityRuleProtocolIcmp
+	}
+
+	switch rule.Direction {
+	case infrav1.SecurityRuleDirectionOutbound:
+		secRule.Direction = network.SecurityRuleDirectionOutbound
+	case infrav1.SecurityRuleDirectionInbound:
+		secRule.Direction = network.SecurityRuleDirectionInbound
 	}
 
 	return secRule

azure/scope/cluster.go

@@ -511,6 +511,7 @@ func (s *ClusterScope) SetFailureDomain(id string, spec clusterv1.FailureDomainS
 }
 
 // SetControlPlaneSecurityRules sets the default security rules of the control plane subnet.
+// Note that this is not done in a webhook as it requires a valid Cluster object to exist to get the API Server port.
 func (s *ClusterScope) SetControlPlaneSecurityRules() {
 	if s.ControlPlaneSubnet().SecurityGroup.SecurityRules == nil {
 		subnet := s.ControlPlaneSubnet()
@@ -520,6 +521,7 @@ func (s *ClusterScope) SetControlPlaneSecurityRules() {
 				Description:      "Allow SSH",
 				Priority:         2200,
 				Protocol:         infrav1.SecurityGroupProtocolTCP,
+				Direction:        infrav1.SecurityRuleDirectionInbound,
 				Source:           to.StringPtr("*"),
 				SourcePorts:      to.StringPtr("*"),
 				Destination:      to.StringPtr("*"),
@@ -530,6 +532,7 @@ func (s *ClusterScope) SetControlPlaneSecurityRules() {
 				Description:      "Allow K8s API Server",
 				Priority:         2201,
 				Protocol:         infrav1.SecurityGroupProtocolTCP,
+				Direction:        infrav1.SecurityRuleDirectionInbound,
 				Source:           to.StringPtr("*"),
 				SourcePorts:      to.StringPtr("*"),
 				Destination:      to.StringPtr("*"),
@@ -541,6 +544,7 @@ func (s *ClusterScope) SetControlPlaneSecurityRules() {
 }
 
 // SetDNSName sets the API Server public IP DNS name.
+// Note: this logic exists only for purposes of ensuring backwards compatibility for old clusters created without an APIServerLB, and should be removed in the future.
 func (s *ClusterScope) SetDNSName() {
 	// for back compat, set the old API Server defaults if no API Server Spec has been set by new webhooks.
 	lb := s.APIServerLB()
@@ -564,6 +568,7 @@ func (s *ClusterScope) SetDNSName() {
 		lb.DeepCopyInto(s.APIServerLB())
 	}
 	// Generate valid FQDN if not set.
+	// Note: this function uses the AzureCluster subscription ID.
 	if !s.IsAPIServerPrivate() && s.APIServerPublicIP().DNSName == "" {
 		s.APIServerPublicIP().DNSName = s.GenerateFQDN(s.APIServerPublicIP().Name)
 	}

config/crd/bases/infrastructure.cluster.x-k8s.io_azureclusters.yaml

@@ -534,33 +534,47 @@ spec:
                             securityRules:
                               description: SecurityRules is a slice of Azure security rules for security groups.
                               items:
-                                description: SecurityRule defines an Azure ingress rule for security groups.
+                                description: SecurityRule defines an Azure security rule for security groups.
                                 properties:
                                   description:
+                                    description: A description for this rule. Restricted to 140 chars.
                                     type: string
                                   destination:
-                                    description: Destination - The destination address prefix. CIDR or destination IP range. Asterix '*' can also be used to match all source IPs. Default tags such as 'VirtualNetwork', 'AzureLoadBalancer' and 'Internet' can also be used.
+                                    description: Destination is the destination address prefix. CIDR or destination IP range. Asterix '*' can also be used to match all source IPs. Default tags such as 'VirtualNetwork', 'AzureLoadBalancer' and 'Internet' can also be used.
                                     type: string
                                   destinationPorts:
-                                    description: DestinationPorts - The destination port or range. Integer or range between 0 and 65535. Asterix '*' can also be used to match all ports.
+                                    description: DestinationPorts specifies the destination port or range. Integer or range between 0 and 65535. Asterix '*' can also be used to match all ports.
+                                    type: string
+                                  direction:
+                                    description: Direction indicates whether the rule applies to inbound, or outbound traffic. "Inbound" or "Outbound".
+                                    enum:
+                                    - Inbound
+                                    - Outbound
                                     type: string
                                   name:
+                                    description: Name is a unique name within the network security group.
                                     type: string
                                   priority:
-                                    description: Priority - A number between 100 and 4096. Each rule should have a unique value for priority. Rules are processed in priority order, with lower numbers processed before higher numbers. Once traffic matches a rule, processing stops.
+                                    description: Priority is a number between 100 and 4096. Each rule should have a unique value for priority. Rules are processed in priority order, with lower numbers processed before higher numbers. Once traffic matches a rule, processing stops.
                                     format: int32
                                     type: integer
                                   protocol:
-                                    description: SecurityGroupProtocol defines the protocol type for a security group rule.
+                                    description: Protocol specifies the protocol type. "Tcp", "Udp", "Icmp", or "*".
+                                    enum:
+                                    - Tcp
+                                    - Udp
+                                    - Icmp
+                                    - '*'
                                     type: string
                                   source:
-                                    description: Source - The CIDR or source IP range. Asterix '*' can also be used to match all source IPs. Default tags such as 'VirtualNetwork', 'AzureLoadBalancer' and 'Internet' can also be used. If this is an ingress rule, specifies where network traffic originates from.
+                                    description: Source specifies the CIDR or source IP range. Asterix '*' can also be used to match all source IPs. Default tags such as 'VirtualNetwork', 'AzureLoadBalancer' and 'Internet' can also be used. If this is an ingress rule, specifies where network traffic originates from.
                                     type: string
                                   sourcePorts:
-                                    description: SourcePorts - The source port or range. Integer or range between 0 and 65535. Asterix '*' can also be used to match all ports.
+                                    description: SourcePorts specifies source port or range. Integer or range between 0 and 65535. Asterix '*' can also be used to match all ports.
                                     type: string
                                 required:
                                 - description
+                                - direction
                                 - name
                                 - protocol
                                 type: object

controllers/azurecluster_reconciler.go

@@ -163,6 +163,8 @@ func (s *azureClusterService) Delete(ctx context.Context) error {
 	return nil
 }
 
+// setFailureDomainsForLocation sets the AzureCluster Status failure domains based on which Azure Availability Zones are available in the cluster location.
+// Note that this is not done in a webhook as it requires API calls to fetch the availability zones.
 func (s *azureClusterService) setFailureDomainsForLocation(ctx context.Context) error {
 	zones, err := s.skuCache.GetZones(ctx, s.scope.Location())
 	if err != nil {

docs/book/src/topics/custom-vnet.md

@@ -63,13 +63,13 @@ If no CIDR block is provided, `10.0.0.0/8` will be used by default, with default
 
 Whenever using custom vnet and subnet names and/or a different vnet resource group, please make sure to update the `azure.json` content part of both the nodes and control planes' `kubeadmConfigSpec` accordingly before creating the cluster.
 
-### Custom Ingress Rules
+### Custom Security Rules
 
-Ingress rules can also be customized as part of the subnet specification in a custom network spec.
-Note that ingress rules for the Kubernetes API Server port (default 6443) and SSH (22) are automatically added to the controlplane subnet only if Ingress Rules aren't specified.
-It is the responsibility of the user to supply those rules themselves if using custom ingresses.
+Security rules can also be customized as part of the subnet specification in a custom network spec.
+Note that ingress rules for the Kubernetes API Server port (default 6443) and SSH (22) are automatically added to the controlplane subnet only if security rules aren't specified.
+It is the responsibility of the user to supply those rules themselves if using custom rules.
 
-Here is an illustrative example of customizing ingresses that builds on the one above by adding an ingress rule to the control plane nodes:
+Here is an illustrative example of customizing rules that builds on the one above by adding an egress rule to the control plane nodes:
 
 ```yaml
 apiVersion: infrastructure.cluster.x-k8s.io/v1alpha4
@@ -94,6 +94,7 @@ spec:
           securityRules:
             - name: "allow_ssh"
               description: "allow SSH"
+              direction: "Inbound"
               priority: 2200
               protocol: "*"
               destination: "*"
@@ -102,6 +103,7 @@ spec:
               sourcePorts: "*"
             - name: "allow_apiserver"
               description: "Allow K8s API Server"
+              direction: "Inbound"
               priority: 2201
               protocol: "*"
               destination: "*"
@@ -110,8 +112,9 @@ spec:
               sourcePorts: "*"
             - name: "allow_port_50000"
               description: "allow port 50000"
+              direction: "Outbound"
               priority: 2202
-              protocol: "*"
+              protocol: "Tcp"
               destination: "*"
               destinationPorts: "50000"
               source: "*"