TT-15595, fixed all load targets 0 case

Author: andrei-tyk

apidef/validator.go

@@ -58,6 +58,7 @@ var DefaultValidationRuleSet = ValidationRuleSet{
 	&RuleValidateIPList{},
 	&RuleValidateEnforceTimeout{},
 	&RuleUpstreamAuth{},
+	&RuleLoadBalancingTargets{},
 }
 
 func Validate(definition *APIDefinition, ruleSet ValidationRuleSet) ValidationResult {
@@ -210,6 +211,8 @@ var (
 	ErrUpstreamOAuthAuthorizationTypeRequired = errors.New("upstream OAuth authorization type is required")
 	// ErrInvalidUpstreamOAuthAuthorizationType is the error to return when configured OAuth authorization type is invalid.
 	ErrInvalidUpstreamOAuthAuthorizationType = errors.New("invalid OAuth authorization type")
+	// ErrAllLoadBalancingTargetsZeroWeight is the error to return when all load balancing targets have weight 0.
+	ErrAllLoadBalancingTargetsZeroWeight = errors.New("all load balancing targets have weight 0, at least one target must have weight > 0")
 )
 
 // RuleUpstreamAuth implements validations for upstream authentication configurations.
@@ -250,3 +253,20 @@ func (r *RuleUpstreamAuth) Validate(apiDef *APIDefinition, validationResult *Val
 		validationResult.AppendError(ErrInvalidUpstreamOAuthAuthorizationType)
 	}
 }
+
+// RuleLoadBalancingTargets implements validations for load balancing target configurations.
+type RuleLoadBalancingTargets struct{}
+
+// Validate validates that when load balancing is enabled, at least one target has weight > 0.
+func (r *RuleLoadBalancingTargets) Validate(apiDef *APIDefinition, validationResult *ValidationResult) {
+	if !apiDef.Proxy.EnableLoadBalancing {
+		return
+	}
+
+	// In Tyk's internal representation, targets with weight N are repeated N times in Proxy.Targets
+	// If all weights are 0, the targets list will be empty, which is invalid for load balancing
+	if len(apiDef.Proxy.Targets) == 0 {
+		validationResult.IsValid = false
+		validationResult.AppendError(ErrAllLoadBalancingTargetsZeroWeight)
+	}
+}

apidef/validator_test.go

@@ -508,3 +508,78 @@ func TestRuleUpstreamAuth_Validate(t *testing.T) {
 		t.Run(tc.name, runValidationTest(apiDef, ruleSet, tc.result))
 	}
 }
+
+func TestRuleLoadBalancingTargets_Validate(t *testing.T) {
+	ruleSet := ValidationRuleSet{
+		&RuleLoadBalancingTargets{},
+	}
+
+	testCases := []struct {
+		name   string
+		apiDef *APIDefinition
+		result ValidationResult
+	}{
+		{
+			name: "load balancing disabled",
+			apiDef: &APIDefinition{
+				Proxy: ProxyConfig{
+					EnableLoadBalancing: false,
+					Targets:             []string{},
+				},
+			},
+			result: ValidationResult{
+				IsValid: true,
+				Errors:  nil,
+			},
+		},
+		{
+			name: "load balancing enabled with valid targets",
+			apiDef: &APIDefinition{
+				Proxy: ProxyConfig{
+					EnableLoadBalancing: true,
+					Targets: []string{
+						"http://target-1",
+						"http://target-1",
+						"http://target-2",
+					},
+				},
+			},
+			result: ValidationResult{
+				IsValid: true,
+				Errors:  nil,
+			},
+		},
+		{
+			name: "load balancing enabled with all targets weight 0",
+			apiDef: &APIDefinition{
+				Proxy: ProxyConfig{
+					EnableLoadBalancing: true,
+					Targets:             []string{},
+				},
+			},
+			result: ValidationResult{
+				IsValid: false,
+				Errors: []error{
+					ErrAllLoadBalancingTargetsZeroWeight,
+				},
+			},
+		},
+		{
+			name: "load balancing disabled with empty targets",
+			apiDef: &APIDefinition{
+				Proxy: ProxyConfig{
+					EnableLoadBalancing: false,
+					Targets:             []string{},
+				},
+			},
+			result: ValidationResult{
+				IsValid: true,
+				Errors:  nil,
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, runValidationTest(tc.apiDef, ruleSet, tc.result))
+	}
+}

gateway/api_test.go

@@ -2127,6 +2127,74 @@ func TestHandleAddApi(t *testing.T) {
 		assert.Equal(t, http.StatusBadRequest, statusCode)
 	})
 
+	t.Run("should return error when load balancing enabled with all targets weight 0", func(t *testing.T) {
+		apiDef := apidef.DummyAPI()
+		apiDef.APIID = "124"
+		apiDef.Proxy.EnableLoadBalancing = true
+		apiDef.Proxy.Targets = []string{} // Empty targets list means all weights are 0
+		apiDefJson, err := json.Marshal(apiDef)
+		require.NoError(t, err)
+
+		req, err := http.NewRequest(http.MethodPost, "http://gateway", bytes.NewBuffer(apiDefJson))
+		require.NoError(t, err)
+
+		response, statusCode := ts.Gw.handleAddApi(req, testFs, false)
+		errorResponse, ok := response.(apiStatusMessage)
+		require.True(t, ok)
+
+		assert.Equal(t, "Validation of API Definition failed. Reason: all load balancing targets have weight 0, at least one target must have weight > 0.", errorResponse.Message)
+		assert.Equal(t, http.StatusBadRequest, statusCode)
+	})
+
+	t.Run("should return error when OAS load balancing enabled with all targets weight 0", func(t *testing.T) {
+		tykExt := oas.XTykAPIGateway{
+			Info: oas.Info{
+				Name: "test api",
+				State: oas.State{
+					Active: true,
+				},
+			},
+			Upstream: oas.Upstream{
+				URL: "http://example.com",
+				LoadBalancing: &oas.LoadBalancing{
+					Enabled: true,
+					Targets: []oas.LoadBalancingTarget{
+						{URL: "http://target-1", Weight: 0},
+						{URL: "http://target-2", Weight: 0},
+					},
+				},
+			},
+			Server: oas.Server{
+				ListenPath: oas.ListenPath{
+					Value: "/test-lb-zero/",
+					Strip: false,
+				},
+			},
+		}
+
+		oasAPI := openapi3.T{
+			OpenAPI: "3.0.3",
+			Info: &openapi3.Info{
+				Title:   "test api",
+				Version: "1",
+			},
+			Paths: openapi3.NewPaths(),
+		}
+
+		oasAPI.Extensions = map[string]interface{}{
+			oas.ExtensionTykAPIGateway: tykExt,
+		}
+
+		_, _ = ts.Run(t, test.TestCase{
+			AdminAuth: true,
+			Method:    http.MethodPost,
+			Path:      "/tyk/apis/oas",
+			Data:      &oasAPI,
+			BodyMatch: `all load balancing targets have weight 0`,
+			Code:      http.StatusBadRequest,
+		})
+	})
+
 	t.Run("should return success when no error occurs", func(t *testing.T) {
 		apiDef := apidef.DummyAPI()
 		apiDef.APIID = "123"