Fixed bugs, updated tests, removed invalid options

docs-web/configuration/transportserver-resource.md

@@ -206,6 +206,7 @@ loadBalancingMethod: least_conn
      - No
    * - ``loadBalancingMethod``
      - The method used to load balance the upstream servers. By default, connections are distributed between the servers using a weighted round-robin balancing method. See the `upstream <http://nginx.org/en/docs/stream/ngx_stream_upstream_module.html#upstream>`_ section for available methods and their details.
+     - ``string``
      - No
 
 ```

internal/configs/transportserver.go

@@ -192,6 +192,19 @@ func generateStreamUpstream(upstream *conf_v1alpha1.Upstream, upstreamNamer *ups
 	return version2.StreamUpstream{
 		Name:                name,
 		Servers:             upsServers,
-		LoadBalancingMethod: upstream.LoadBalancingMethod,
+		LoadBalancingMethod: generateLoadBalancingMethod(upstream.LoadBalancingMethod),
 	}
 }
+
+func generateLoadBalancingMethod(method string) string {
+	if method == "" {
+		// By default, if unspecified, Nginx uses the 'round_robin' load balancing method.
+		// We override this default which suits the Ingress Controller better.
+		return "random two least_conn"
+	}
+	if method == "round_robin" {
+		// By default, Nginx uses round robin. We select this method by not specifying any method.
+		return ""
+	}
+	return method
+}

internal/configs/transportserver_test.go

@@ -112,6 +112,7 @@ func TestGenerateTransportServerConfigForTCPSnippets(t *testing.T) {
 					ResourceNamespace: "default",
 					Service:           "tcp-app-svc",
 				},
+				LoadBalancingMethod: "random two least_conn",
 			},
 		},
 		Server: version2.StreamServer{
@@ -196,6 +197,7 @@ func TestGenerateTransportServerConfigForTCP(t *testing.T) {
 					ResourceNamespace: "default",
 					Service:           "tcp-app-svc",
 				},
+				LoadBalancingMethod: "random two least_conn",
 			},
 		},
 		Server: version2.StreamServer{
@@ -279,6 +281,7 @@ func TestGenerateTransportServerConfigForTLSPasstrhough(t *testing.T) {
 					ResourceNamespace: "default",
 					Service:           "tcp-app-svc",
 				},
+				LoadBalancingMethod: "random two least_conn",
 			},
 		},
 		Server: version2.StreamServer{
@@ -368,6 +371,7 @@ func TestGenerateTransportServerConfigForUDP(t *testing.T) {
 					ResourceNamespace: "default",
 					Service:           "udp-app-svc",
 				},
+				LoadBalancingMethod: "random two least_conn",
 			},
 		},
 		Server: version2.StreamServer{

internal/configs/version2/nginx-plus.transportserver.tmpl

@@ -3,7 +3,7 @@ upstream {{ $u.Name }} {
     zone {{ $u.Name }} 256k;
 
     {{ if $u.LoadBalancingMethod }}
-    $u.LoadBalancingMethod;
+    {{ $u.LoadBalancingMethod }};
     {{ end }}
 
     {{ range $s := $u.Servers }}

internal/configs/version2/nginx.transportserver.tmpl

@@ -3,7 +3,7 @@ upstream {{ $u.Name }} {
     zone {{ $u.Name }} 256k;
 
     {{ if $u.LoadBalancingMethod }}
-    $u.LoadBalancingMethod;
+    {{ $u.LoadBalancingMethod }};
     {{ end }}
 
     {{ range $s := $u.Servers }}

pkg/apis/configuration/validation/transportserver.go

@@ -208,36 +208,35 @@ func validateLoadBalancingMethod(method string, fieldPath *field.Path, isPlus bo
 }
 
 var nginxStreamLoadBalanceValidInput = map[string]bool{
+	"round_robin":           true,
 	"least_conn":            true,
 	"random":                true,
 	"random two":            true,
 	"random two least_conn": true,
 }
 
 var nginxPlusStreamLoadBalanceValidInput = map[string]bool{
-	"least_conn":                       true,
-	"random":                           true,
-	"random two":                       true,
-	"random two least_conn":            true,
-	"random two least_time":            true,
-	"random two least_time=connect":    true,
-	"random two least_time=first_byte": true,
-	"random two least_time=last_byte":  true,
-	"random least_conn":                true,
-	"random least_time":                true,
-	"random least_time=connect":        true,
-	"random least_time=first_byte":     true,
-	"random least_time=last_byte":      true,
-	"least_time connect":               true,
-	"least_time first_byte":            true,
-	"least_time last_byte":             true,
-	"least_time last_byte inflight":    true,
+	"round_robin":                   true,
+	"least_conn":                    true,
+	"random":                        true,
+	"random two":                    true,
+	"random two least_conn":         true,
+	"random least_conn":             true,
+	"least_time connect":            true,
+	"least_time first_byte":         true,
+	"least_time last_byte":          true,
+	"least_time last_byte inflight": true,
 }
 
 func validateHashLoadBalancingMethod(method string) error {
 	keyWords := strings.Split(method, " ")
 	if len(keyWords) > 0 && keyWords[0] == "hash" {
 		if len(keyWords) == 2 || (len(keyWords) == 3 && keyWords[2] == "consistent") {
+			value := keyWords[1]
+			if !escapedStringsFmtRegexp.MatchString(value) {
+				return fmt.Errorf("invalid value for hash: %v", validation.RegexError(escapedStringsErrMsg, escapedStringsFmt))
+			}
+
 			return nil
 		}
 	}

pkg/apis/configuration/validation/transportserver_test.go

@@ -280,11 +280,11 @@ func TestValidateTransportServerLoadBalancingMethod(t *testing.T) {
 		{
 			method:   "random two least_time",
 			isPlus:   true,
-			hasError: false,
+			hasError: true,
 		}, {
 			method:   "random two least_time=connect",
 			isPlus:   true,
-			hasError: false,
+			hasError: true,
 		},
 	}