Skip to content
20 changes: 20 additions & 0 deletions apidef/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
&RuleValidateIPList{},
&RuleValidateEnforceTimeout{},
&RuleUpstreamAuth{},
&RuleLoadBalancingTargets{},

Check failure on line 61 in apidef/validator.go

View check run for this annotation

probelabs / Visor: dependency

architecture Issue

The introduction of `RuleLoadBalancingTargets` makes previously acceptable `ApiDefinition` custom resources invalid if they configure load balancing with all target weights set to zero. When `tyk-operator` attempts to reconcile such a resource, the gateway will reject it, causing the operator's reconciliation loop to fail and preventing any further updates.
Raw output
Update `tyk-operator` to incorporate the same validation logic, ideally in a validating admission webhook, to reject invalid `ApiDefinition` custom resources at the time of submission (`kubectl apply`). A corresponding `tyk-operator` issue or PR should be created and linked.

Check failure on line 61 in apidef/validator.go

View check run for this annotation

probelabs / Visor: dependency

architecture Issue

Multi-Data Centre Bridge (MDCB) will fail to synchronize any existing API definition that uses the now-invalid all-zero-weight load balancing configuration. When `tyk-sink` attempts to propagate this API definition to a replica gateway, the replica will reject it, leading to configuration drift and potential outages.
Raw output
Provide a migration path or clear release notes instructing users to identify and correct any existing invalid API definitions in their primary data store before upgrading gateways in replica data centers. The component managing the source-of-truth definitions (e.g., Tyk Dashboard) must be updated first.

Check failure on line 61 in apidef/validator.go

View check run for this annotation

probelabs / Visor: dependency

architecture Issue

The Tyk Developer Portal's UI may permit a user to configure load balancing targets where all weights are zero. An attempt to save such a configuration will now fail with an HTTP 400 error from the gateway, resulting in a poor user experience as the cause of the failure may not be clear from the UI.
Raw output
Update the portal's frontend and backend validation to prevent users from creating this invalid configuration. The UI should provide immediate and clear feedback, ensuring at least one target has a weight greater than zero when load balancing is enabled.
}

func Validate(definition *APIDefinition, ruleSet ValidationRuleSet) ValidationResult {
Expand Down Expand Up @@ -210,6 +211,8 @@
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.
Expand Down Expand Up @@ -250,3 +253,20 @@
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)
}
}
75 changes: 75 additions & 0 deletions apidef/validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
}
68 changes: 68 additions & 0 deletions gateway/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2127,6 +2127,74 @@
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)

Check warning on line 2144 in gateway/api_test.go

View check run for this annotation

probelabs / Visor: quality

style Issue

The test assertion performs an exact match on the full error message string, making it brittle. Any change to the generic error message prefix or suffix will break this test, even if the core validation logic is correct. The accompanying OAS test correctly uses a more robust substring match.
Raw output
To improve test resilience, use `assert.Contains` to check for the specific error substring from `apidef.ErrAllLoadBalancingTargetsZeroWeight.Error()`. This ensures the test validates the important part of the error message without being sensitive to formatting changes.

Example:
```go
assert.Contains(t, errorResponse.Message, apidef.ErrAllLoadBalancingTargetsZeroWeight.Error())
```
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"
Expand Down
Loading