Fix outboundType validation and add more checks

Issue: rancher#709

Makefile

@@ -31,7 +31,7 @@ MOCKGEN_VER := v0.4.0
 MOCKGEN_BIN := mockgen
 MOCKGEN := $(BIN_DIR)/$(MOCKGEN_BIN)-$(MOCKGEN_VER)
 
-GINKGO_VER := v2.20.2
+GINKGO_VER := v2.22.0
 GINKGO_BIN := ginkgo
 GINKGO := $(BIN_DIR)/$(GINKGO_BIN)-$(GINKGO_VER)
 

controller/aks-cluster-config-handler.go

@@ -448,6 +448,25 @@ func (h *Handler) validateConfig(config *aksv1.AKSClusterConfig) error {
 	if aks.String(config.Spec.NetworkPolicy) == string(armcontainerservice.NetworkPolicyAzure) && aks.String(config.Spec.NetworkPlugin) != string(armcontainerservice.NetworkPluginAzure) {
 		return fmt.Errorf("azure network policy can be used only with Azure CNI network plugin for [%s (id: %s)] cluster", config.Spec.ClusterName, config.Name)
 	}
+
+	outboundType := strings.ToLower(aks.String(config.Spec.OutboundType))
+	if outboundType != "" {
+		if outboundType != strings.ToLower(string(armcontainerservice.OutboundTypeLoadBalancer)) &&
+			outboundType != strings.ToLower(string(armcontainerservice.OutboundTypeUserDefinedRouting)) &&
+			outboundType != strings.ToLower(string(armcontainerservice.OutboundTypeManagedNATGateway)) &&
+			outboundType != strings.ToLower(string(armcontainerservice.OutboundTypeUserAssignedNATGateway)) {
+			return fmt.Errorf("invalid outbound type value [%s] for [%s (id: %s)] cluster config", outboundType, config.Spec.ClusterName, config.Name)
+		}
+		if outboundType == strings.ToLower(string(armcontainerservice.OutboundTypeUserDefinedRouting)) {
+			if aks.String(config.Spec.NetworkPlugin) != string(armcontainerservice.NetworkPluginAzure) {
+				return fmt.Errorf("user defined routing can be used only with Azure CNI network plugin for [%s (id: %s)] cluster", config.Spec.ClusterName, config.Name)
+			}
+			if config.Spec.Subnet == nil || aks.String(config.Spec.Subnet) == "" {
+				return fmt.Errorf("subnet must be provided for cluster [%s (id: %s)] config when user defined routing is used", config.Spec.ClusterName, config.Name)
+			}
+		}
+	}
+
 	cannotBeNilErrorAzurePlugin := "field [%s] must be provided for cluster [%s (id: %s)] config when Azure CNI network plugin is used"
 	if aks.String(config.Spec.NetworkPlugin) == string(armcontainerservice.NetworkPluginAzure) {
 		if config.Spec.VirtualNetwork == nil {

controller/aks-cluster-config-handler_test.go

@@ -299,6 +299,13 @@ var _ = Describe("validateConfig", func() {
 		Expect(handler.validateConfig(aksConfig)).To(Succeed())
 	})
 
+	It("should successfully validate if user defined routing is used with subnet", func() {
+		aksConfig.Spec.OutboundType = to.Ptr("userDefinedRouting")
+		aksConfig.Spec.Subnet = to.Ptr("test-subnet-1")
+		aksConfig.Spec.NetworkPlugin = to.Ptr(string(armcontainerservice.NetworkPluginAzure))
+		Expect(handler.validateConfig(aksConfig)).To(Succeed())
+	})
+
 	It("should fail if listing aks cluster config fails", func() {
 		brokenAksFactory, err := aksv1controllers.NewFactoryFromConfig(&rest.Config{})
 		Expect(err).NotTo(HaveOccurred())
@@ -464,6 +471,19 @@ var _ = Describe("validateConfig", func() {
 		aksConfig.Spec.NetworkServiceCIDR = nil
 		Expect(handler.validateConfig(aksConfig)).NotTo(Succeed())
 	})
+
+	It("should fail if user defined routing is used and subnet is empty", func() {
+		aksConfig.Spec.OutboundType = to.Ptr("userDefinedRouting")
+		aksConfig.Spec.Subnet = to.Ptr("")
+		Expect(handler.validateConfig(aksConfig)).NotTo(Succeed())
+	})
+
+	It("should fail if user defined routing is used and kubenet as network plugin", func() {
+		aksConfig.Spec.OutboundType = to.Ptr("userDefinedRouting")
+		aksConfig.Spec.Subnet = to.Ptr("test-subnet-1")
+		aksConfig.Spec.NetworkPlugin = to.Ptr(string(armcontainerservice.NetworkPluginKubenet))
+		Expect(handler.validateConfig(aksConfig)).NotTo(Succeed())
+	})
 })
 
 var _ = Describe("createCluster", func() {

pkg/aks/create.go

@@ -80,16 +80,18 @@ func createManagedCluster(ctx context.Context, cred *Credentials, workplacesClie
 
 	networkProfile := &armcontainerservice.NetworkProfile{}
 
-	switch String(spec.OutboundType) {
-	case string(armcontainerservice.OutboundTypeLoadBalancer):
+	switch strings.ToLower(String(spec.OutboundType)) {
+	case strings.ToLower(string(armcontainerservice.OutboundTypeLoadBalancer)):
 		networkProfile.OutboundType = to.Ptr(armcontainerservice.OutboundTypeLoadBalancer)
-	case string(armcontainerservice.OutboundTypeUserDefinedRouting):
+	case strings.ToLower(string(armcontainerservice.OutboundTypeUserDefinedRouting)):
 		networkProfile.OutboundType = to.Ptr(armcontainerservice.OutboundTypeUserDefinedRouting)
+	case strings.ToLower(string(armcontainerservice.OutboundTypeManagedNATGateway)):
+		networkProfile.OutboundType = to.Ptr(armcontainerservice.OutboundTypeManagedNATGateway)
 	case "":
 		networkProfile.OutboundType = to.Ptr(armcontainerservice.OutboundTypeLoadBalancer)
 	}
 
-	switch String(spec.NetworkPolicy) {
+	switch strings.ToLower(String(spec.NetworkPolicy)) {
 	case string(armcontainerservice.NetworkPolicyAzure):
 		networkProfile.NetworkPolicy = to.Ptr(armcontainerservice.NetworkPolicyAzure)
 	case string(armcontainerservice.NetworkPolicyCalico):
@@ -100,7 +102,7 @@ func createManagedCluster(ctx context.Context, cred *Credentials, workplacesClie
 		return nil, fmt.Errorf("networkPolicy '%s' is not supported", String(spec.NetworkPolicy))
 	}
 
-	switch String(spec.NetworkPlugin) {
+	switch strings.ToLower(String(spec.NetworkPlugin)) {
 	case string(armcontainerservice.NetworkPluginAzure):
 		networkProfile.NetworkPlugin = to.Ptr(armcontainerservice.NetworkPluginAzure)
 	case string(armcontainerservice.NetworkPluginKubenet):
@@ -117,13 +119,13 @@ func createManagedCluster(ctx context.Context, cred *Credentials, workplacesClie
 
 	networkProfile.LoadBalancerSKU = to.Ptr(armcontainerservice.LoadBalancerSKUStandard)
 
-	if String(spec.LoadBalancerSKU) == string(armcontainerservice.LoadBalancerSKUBasic) {
+	if strings.EqualFold(String(spec.LoadBalancerSKU), string(armcontainerservice.LoadBalancerSKUBasic)) {
 		logrus.Warnf("LoadBalancerSKU 'basic' is not supported")
 		networkProfile.LoadBalancerSKU = to.Ptr(armcontainerservice.LoadBalancerSKUBasic)
 	}
 
 	// Disable standard loadbalancer for UserDefinedRouting and use routing created by user pre-defined table for egress
-	if String(spec.OutboundType) == string(armcontainerservice.OutboundTypeUserDefinedRouting) {
+	if strings.EqualFold(String(spec.OutboundType), string(armcontainerservice.OutboundTypeUserDefinedRouting)) {
 		networkProfile.LoadBalancerSKU = nil
 	}
 

pkg/aks/create_test.go

@@ -176,7 +176,20 @@ var _ = Describe("newManagedCluster", func() {
 					ID: to.Ptr("test-workspace-id"),
 				},
 			}, nil)
-		clusterSpec.OutboundType = to.Ptr("userDefinedRouting")
+		clusterSpec.OutboundType = to.Ptr("userdefinedrouting")
+		managedCluster, err := createManagedCluster(ctx, cred, workplacesClientMock, clusterSpec, "test-phase")
+		Expect(err).ToNot(HaveOccurred())
+		Expect(*managedCluster.Properties.NetworkProfile.OutboundType).To(Equal(armcontainerservice.OutboundTypeUserDefinedRouting))
+	})
+
+	It("should successfully create managed cluster with outboundtype UserDefinedRouting", func() {
+		workplacesClientMock.EXPECT().Get(ctx, String(clusterSpec.LogAnalyticsWorkspaceGroup), String(clusterSpec.LogAnalyticsWorkspaceName), nil).
+			Return(armoperationalinsights.WorkspacesClientGetResponse{
+				Workspace: armoperationalinsights.Workspace{
+					ID: to.Ptr("test-workspace-id"),
+				},
+			}, nil)
+		clusterSpec.OutboundType = to.Ptr("UserDefinedRouting")
 		managedCluster, err := createManagedCluster(ctx, cred, workplacesClientMock, clusterSpec, "test-phase")
 		Expect(err).ToNot(HaveOccurred())
 		Expect(*managedCluster.Properties.NetworkProfile.OutboundType).To(Equal(armcontainerservice.OutboundTypeUserDefinedRouting))