Support weights 0 and 100 in traffic splitting

docs/content/configuration/virtualserver-and-virtualserverroute-resources.md

@@ -731,7 +731,7 @@ splits:
 {{% table %}}
 |Field | Description | Type | Required |
 | ---| ---| ---| --- |
-|``weight`` | The weight of an action. Must fall into the range ``1..99``. The sum of the weights of all splits must be equal to ``100``. | ``int`` | Yes |
+|``weight`` | The weight of an action. Must fall into the range ``0..100``. The sum of the weights of all splits must be equal to ``100``. | ``int`` | Yes |
 |``action`` | The action to perform for a request. | [action](#action) | Yes |
 {{% /table %}}
 

internal/configs/virtualserver.go

@@ -1985,6 +1985,9 @@ func generateSplits(
 	var distributions []version2.Distribution
 
 	for i, s := range splits {
+		if s.Weight == 0 {
+			continue
+		}
 		d := version2.Distribution{
 			Weight: fmt.Sprintf("%d%%", s.Weight),
 			Value:  fmt.Sprintf("/%vsplits_%d_split_%d", internalLocationPrefix, scIndex, i),

internal/configs/virtualserver_test.go

@@ -7400,32 +7400,101 @@ func TestCreateUpstreamServersConfigForPlusNoUpstreams(t *testing.T) {
 
 func TestGenerateSplits(t *testing.T) {
 	t.Parallel()
-	originalPath := "/path"
-	splits := []conf_v1.Split{
+	tests := []struct {
+		splits              []conf_v1.Split
+		expectedSplitClient version2.SplitClient
+		msg                 string
+	}{
 		{
-			Weight: 90,
-			Action: &conf_v1.Action{
-				Proxy: &conf_v1.ActionProxy{
-					Upstream:    "coffee-v1",
-					RewritePath: "/rewrite",
+			splits: []conf_v1.Split{
+				{
+					Weight: 90,
+					Action: &conf_v1.Action{
+						Proxy: &conf_v1.ActionProxy{
+							Upstream:    "coffee-v1",
+							RewritePath: "/rewrite",
+						},
+					},
+				},
+				{
+					Weight: 9,
+					Action: &conf_v1.Action{
+						Pass: "coffee-v2",
+					},
+				},
+				{
+					Weight: 1,
+					Action: &conf_v1.Action{
+						Return: &conf_v1.ActionReturn{
+							Body: "hello",
+						},
+					},
 				},
 			},
-		},
-		{
-			Weight: 9,
-			Action: &conf_v1.Action{
-				Pass: "coffee-v2",
+			expectedSplitClient: version2.SplitClient{
+				Source:   "$request_id",
+				Variable: "$vs_default_cafe_splits_1",
+				Distributions: []version2.Distribution{
+					{
+						Weight: "90%",
+						Value:  "/internal_location_splits_1_split_0",
+					},
+					{
+						Weight: "9%",
+						Value:  "/internal_location_splits_1_split_1",
+					},
+					{
+						Weight: "1%",
+						Value:  "/internal_location_splits_1_split_2",
+					},
+				},
 			},
+			msg: "Normal Split",
 		},
 		{
-			Weight: 1,
-			Action: &conf_v1.Action{
-				Return: &conf_v1.ActionReturn{
-					Body: "hello",
+			splits: []conf_v1.Split{
+				{
+					Weight: 90,
+					Action: &conf_v1.Action{
+						Proxy: &conf_v1.ActionProxy{
+							Upstream:    "coffee-v1",
+							RewritePath: "/rewrite",
+						},
+					},
+				},
+				{
+					Weight: 0,
+					Action: &conf_v1.Action{
+						Pass: "coffee-v2",
+					},
+				},
+				{
+					Weight: 10,
+					Action: &conf_v1.Action{
+						Return: &conf_v1.ActionReturn{
+							Body: "hello",
+						},
+					},
+				},
+			},
+			expectedSplitClient: version2.SplitClient{
+				Source:   "$request_id",
+				Variable: "$vs_default_cafe_splits_1",
+				Distributions: []version2.Distribution{
+					{
+						Weight: "90%",
+						Value:  "/internal_location_splits_1_split_0",
+					},
+					{
+						Weight: "10%",
+						Value:  "/internal_location_splits_1_split_2",
+					},
 				},
 			},
+			msg: "Split with 0 weight",
 		},
 	}
+	originalPath := "/path"
 
 	virtualServer := conf_v1.VirtualServer{
 		ObjectMeta: meta_v1.ObjectMeta{
@@ -7476,24 +7545,6 @@ func TestGenerateSplits(t *testing.T) {
 			},
 		},
 	}
-	expectedSplitClient := version2.SplitClient{
-		Source:   "$request_id",
-		Variable: "$vs_default_cafe_splits_1",
-		Distributions: []version2.Distribution{
-			{
-				Weight: "90%",
-				Value:  "/internal_location_splits_1_split_0",
-			},
-			{
-				Weight: "9%",
-				Value:  "/internal_location_splits_1_split_1",
-			},
-			{
-				Weight: "1%",
-				Value:  "/internal_location_splits_1_split_2",
-			},
-		},
-	}
 	expectedLocations := []version2.Location{
 		{
 			Path:      "/internal_location_splits_1_split_0",
@@ -7589,33 +7640,36 @@ func TestGenerateSplits(t *testing.T) {
 	}
 
 	vsc := newVirtualServerConfigurator(&cfgParams, false, false, &StaticConfigParams{}, false)
+	for _, test := range tests {
+		t.Run(test.msg, func(t *testing.T) {
+			resultSplitClient, resultLocations, resultReturnLocations := generateSplits(
+				test.splits,
+				upstreamNamer,
+				crUpstreams,
+				variableNamer,
+				scIndex,
+				&cfgParams,
+				errorPageDetails,
+				originalPath,
+				locSnippet,
+				enableSnippets,
+				returnLocationIndex,
+				true,
+				"coffee",
+				"default",
+				vsc.warnings,
+			)
 
-	resultSplitClient, resultLocations, resultReturnLocations := generateSplits(
-		splits,
-		upstreamNamer,
-		crUpstreams,
-		variableNamer,
-		scIndex,
-		&cfgParams,
-		errorPageDetails,
-		originalPath,
-		locSnippet,
-		enableSnippets,
-		returnLocationIndex,
-		true,
-		"coffee",
-		"default",
-		vsc.warnings,
-	)
-
-	if diff := cmp.Diff(expectedSplitClient, resultSplitClient); diff != "" {
-		t.Errorf("generateSplits() resultSplitClient mismatch (-want +got):\n%s", diff)
-	}
-	if diff := cmp.Diff(expectedLocations, resultLocations); diff != "" {
-		t.Errorf("generateSplits() resultLocations mismatch (-want +got):\n%s", diff)
-	}
-	if diff := cmp.Diff(expectedReturnLocations, resultReturnLocations); diff != "" {
-		t.Errorf("generateSplits() resultReturnLocations mismatch (-want +got):\n%s", diff)
+			if !cmp.Equal(test.expectedSplitClient, resultSplitClient) {
+				t.Errorf("generateSplits() resultSplitClient mismatch (-want +got):\n%s", cmp.Diff(test.expectedSplitClient, resultSplitClient))
+			}
+			if !cmp.Equal(expectedLocations, resultLocations) {
+				t.Errorf("generateSplits() resultLocations mismatch (-want +got):\n%s", cmp.Diff(expectedLocations, resultLocations))
+			}
+			if !cmp.Equal(expectedReturnLocations, resultReturnLocations) {
+				t.Errorf("generateSplits() resultReturnLocations mismatch (-want +got):\n%s", cmp.Diff(expectedReturnLocations, resultReturnLocations))
+			}
+		})
 	}
 }
 

pkg/apis/configuration/validation/virtualserver.go

@@ -1194,7 +1194,7 @@ func (vsv *VirtualServerValidator) validateSplits(splits []v1.Split, fieldPath *
 	for i, s := range splits {
 		idxPath := fieldPath.Index(i)
 
-		for _, msg := range validation.IsInRange(s.Weight, 1, 99) {
+		for _, msg := range validation.IsInRange(s.Weight, 0, 100) {
 			allErrs = append(allErrs, field.Invalid(idxPath.Child("weight"), s.Weight, msg))
 		}
 

pkg/apis/configuration/validation/virtualserver_test.go

@@ -1583,32 +1583,89 @@ func TestValidatePath(t *testing.T) {
 
 func TestValidateSplits(t *testing.T) {
 	t.Parallel()
-	splits := []v1.Split{
+	tests := []struct {
+		splits        []v1.Split
+		upstreamNames sets.Set[string]
+		msg           string
+	}{
 		{
-			Weight: 90,
-			Action: &v1.Action{
-				Pass: "test-1",
+			splits: []v1.Split{
+				{
+					Weight: 90,
+					Action: &v1.Action{
+						Pass: "test-1",
+					},
+				},
+				{
+					Weight: 10,
+					Action: &v1.Action{
+						Pass: "test-2",
+					},
+				},
+			},
+			upstreamNames: map[string]sets.Empty{
+				"test-1": {},
+				"test-2": {},
 			},
+			msg: "valid weights",
 		},
 		{
-			Weight: 10,
-			Action: &v1.Action{
-				Proxy: &v1.ActionProxy{
-					Upstream: "test-2",
+			splits: []v1.Split{
+				{
+					Weight: 0,
+					Action: &v1.Action{
+						Pass: "test-1",
+					},
+				},
+				{
+					Weight: 100,
+					Action: &v1.Action{
+						Pass: "test-2",
+					},
 				},
 			},
+			upstreamNames: map[string]sets.Empty{
+				"test-1": {},
+				"test-2": {},
+			},
+			msg: "valid weights with 0 and 100",
+		},
+		{
+			splits: []v1.Split{
+				{
+					Weight: 0,
+					Action: &v1.Action{
+						Pass: "test-1",
+					},
+				},
+				{
+					Weight: 50,
+					Action: &v1.Action{
+						Pass: "test-2",
+					},
+				},
+				{
+					Weight: 50,
+					Action: &v1.Action{
+						Pass: "test-2",
+					},
+				},
+			},
+			upstreamNames: map[string]sets.Empty{
+				"test-1": {},
+				"test-2": {},
+			},
+			msg: "valid weights with 0",
 		},
-	}
-	upstreamNames := map[string]sets.Empty{
-		"test-1": {},
-		"test-2": {},
 	}
 
 	vsv := &VirtualServerValidator{isPlus: false}
 
-	allErrs := vsv.validateSplits(splits, field.NewPath("splits"), upstreamNames, "")
-	if len(allErrs) > 0 {
-		t.Errorf("validateSplits() returned errors %v for valid input", allErrs)
+	for _, test := range tests {
+		allErrs := vsv.validateSplits(test.splits, field.NewPath("splits"), test.upstreamNames, "")
+		if len(allErrs) > 0 {
+			t.Errorf("validateSplits() returned errors %v for valid input", allErrs)
+		}
 	}
 }
 
@@ -1717,6 +1774,27 @@ func TestValidateSplitsFails(t *testing.T) {
 			},
 			msg: "invalid action with non-existing upstream",
 		},
+		{
+			splits: []v1.Split{
+				{
+					Weight: 100,
+					Action: &v1.Action{
+						Pass: "test-1",
+					},
+				},
+				{
+					Weight: -2,
+					Action: &v1.Action{
+						Pass: "test-2",
+					},
+				},
+			},
+			upstreamNames: map[string]sets.Empty{
+				"test-1": {},
+				"test-2": {},
+			},
+			msg: "invalid negative weight",
+		},
 	}
 
 	vsv := &VirtualServerValidator{isPlus: false}