Skip to content

[TT-15595] temporarily remove upstream target from load balancing#7524

Merged
andrei-tyk merged 7 commits intomasterfrom
TT-15595-temporarily-remove-upstream-target-from-load-balancing
Nov 10, 2025
Merged

[TT-15595] temporarily remove upstream target from load balancing#7524
andrei-tyk merged 7 commits intomasterfrom
TT-15595-temporarily-remove-upstream-target-from-load-balancing

Conversation

@andrei-tyk
Copy link
Copy Markdown
Contributor

@andrei-tyk andrei-tyk commented Nov 7, 2025

Description

This PR adds validation to prevent creating or updating APIs with load balancing enabled where all targets have weight 0. Previously, this configuration would be accepted but cause runtime errors (index
out of range) when traffic was sent to the API. The new RuleLoadBalancingTargets validation rule is added to the DefaultValidationRuleSet and checks if load balancing is enabled with an empty targets
list (which occurs when all weights are 0). The validation triggers on both Classic and OAS API endpoints (POST/PUT) and returns a clear HTTP 400 error: "all load balancing targets have weight 0, at
least one target must have weight > 0". This catches the edge case at API configuration time rather than failing at request time, improving the developer experience and preventing invalid configurations
from being saved.

Related Issue

Motivation and Context

How This Has Been Tested

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Refactoring or add test (improvements in base code or adds test coverage to functionality)

Checklist

  • I ensured that the documentation is up to date
  • I explained why this PR updates go.mod in detail with reasoning why it's required
  • I would like a code coverage CI quality gate exception and have explained why

Ticket Details

TT-15595
Status In Dev
Summary Temporarily remove upstream target from load balancing

Generated at: 2025-11-07 15:07:19

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Nov 7, 2025

🎯 Recommended Merge Targets

Based on JIRA ticket TT-15595: Temporarily remove upstream target from load balancing

Fix Version: Tyk 5.11.0

⚠️ Warning: Expected release branches not found in repository

Required:

  • master - No matching release branches found. Fix will be included in future releases.

📋 Workflow

  1. Merge this PR to master first

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Nov 7, 2025

API Changes

--- prev.txt	2025-11-07 15:07:50.598875916 +0000
+++ current.txt	2025-11-07 15:07:40.626795263 +0000
@@ -128,6 +128,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")
 )
 var DefaultValidationRuleSet = ValidationRuleSet{
 	&RuleUniqueDataSourceNames{},
@@ -135,6 +137,7 @@
 	&RuleValidateIPList{},
 	&RuleValidateEnforceTimeout{},
 	&RuleUpstreamAuth{},
+	&RuleLoadBalancingTargets{},
 }
 var ErrAllAuthSourcesDisabled = "all auth sources are disabled for %s, at least one of header/cookie/query must be enabled"
 var ErrDuplicateDataSourceName = errors.New("duplicate data source names are not allowed")
@@ -1242,6 +1245,14 @@
 
 func (r *RuleAtLeastEnableOneAuthSource) Validate(apiDef *APIDefinition, validationResult *ValidationResult)
 
+type RuleLoadBalancingTargets struct{}
+    RuleLoadBalancingTargets implements validations for load balancing target
+    configurations.
+
+func (r *RuleLoadBalancingTargets) Validate(apiDef *APIDefinition, validationResult *ValidationResult)
+    Validate validates that when load balancing is enabled, at least one target
+    has weight > 0.
+
 type RuleUniqueDataSourceNames struct{}
 
 func (r *RuleUniqueDataSourceNames) Validate(apiDef *APIDefinition, validationResult *ValidationResult)

@probelabs
Copy link
Copy Markdown
Contributor

probelabs Bot commented Nov 7, 2025

🔍 Code Analysis Results

This PR introduces a validation rule to prevent creating or updating an API with load balancing enabled where all upstream targets have a weight of 0. This configuration previously led to a runtime panic (index out of range) when the API received traffic. The new RuleLoadBalancingTargets validation rule catches this invalid setup during the API definition's submission, returning a clear HTTP 400 error. This shifts the failure from runtime to configuration time, improving system stability and providing immediate feedback to developers.

Files Changed Analysis

  • apidef/validator.go: Introduces the new validation logic (RuleLoadBalancingTargets) and a corresponding error (ErrAllLoadBalancingTargetsZeroWeight). The new rule is added to the DefaultValidationRuleSet, ensuring it is applied automatically during API validation.
  • apidef/validator_test.go: Adds unit tests for the new validation rule. The tests cover cases where load balancing is disabled, enabled with valid targets, and enabled with all-zero-weight targets.
  • gateway/api_test.go: Adds integration tests to confirm that the gateway's admin API endpoints for both Classic (/tyk/apis) and OAS (/tyk/apis/oas) definitions correctly reject the invalid configuration with a StatusBadRequest.

Architecture & Impact Assessment

  • What this PR accomplishes: It enhances system resilience by preventing the deployment of an invalid API configuration that would otherwise cause a runtime panic.
  • Key technical changes introduced: A new validation rule, RuleLoadBalancingTargets, is implemented and added to the default validation set. It checks if apiDef.Proxy.EnableLoadBalancing is true while len(apiDef.Proxy.Targets) is zero. An empty Targets slice is Tyk's internal representation for a load balancing setup where all defined targets have a weight of 0.
  • Affected system components: The change affects the API definition validation process within the Tyk Gateway. Consequently, any system component that creates or updates APIs (e.g., Tyk Dashboard, Tyk Sync, direct API calls) will be subject to this new validation.

The following diagram illustrates how the new validation rule intercepts an invalid API configuration:

sequenceDiagram
    participant Client as (e.g., Dashboard/API)
    participant Gateway Admin API
    participant APIDef Validator
    participant Datastore

    Client->>+Gateway Admin API: POST /tyk/apis (Create API with all target weights=0)
    Gateway Admin API->>+APIDef Validator: Validate(apiDefinition)
    APIDef Validator->>APIDef Validator: Run RuleLoadBalancingTargets
    Note over APIDef Validator: Detects load balancing is enabled<br/>but target list is empty.
    APIDef Validator-->>-Gateway Admin API: Validation Failed: ErrAllLoadBalancingTargetsZeroWeight
    Gateway Admin API-->>-Client: HTTP 400 Bad Request
    Note over Gateway Admin API, Datastore: Invalid API is not saved
Loading

Scope Discovery & Context Expansion

The change is well-contained within the API definition validation module. By adding the rule to the DefaultValidationRuleSet, the PR ensures its broad application across all API management workflows. The integration tests confirm its effect on the two primary API definition formats (Classic and OAS).

The primary impact extends to any tool or process that relies on the gateway's API management endpoints, enforcing a stricter configuration standard for load balancing. This could be a breaking change for any automation (e.g., CI/CD scripts, Tyk Operator) that may have been creating these invalid (albeit non-functional) API definitions.

The provided tests are sufficient, and no further code exploration is necessary to understand the scope and impact of this change.

Metadata
  • Review Effort: 2 / 5
  • Primary Label: enhancement

Powered by Visor from Probelabs

Last updated: 2025-11-07T15:10:40.316Z | Triggered by: synchronize | Commit: eaeede1

💡 TIP: You can chat with Visor using /visor ask <your question>

@probelabs
Copy link
Copy Markdown
Contributor

probelabs Bot commented Nov 7, 2025

🔍 Code Analysis Results

✅ Security Check Passed

No security issues found – changes LGTM.

✅ Architecture Check Passed

No architecture issues found – changes LGTM.

✅ Performance Check Passed

No performance issues found – changes LGTM.

Quality Issues (1)

Severity Location Issue
🟡 Warning gateway/api_test.go:2144
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.
💡 SuggestionTo 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:

assert.Contains(t, errorResponse.Message, apidef.ErrAllLoadBalancingTargetsZeroWeight.Error())

Dependency Issues (3)

Severity Location Issue
🔴 Critical apidef/validator.go:61
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.
💡 SuggestionUpdate `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.
🔴 Critical apidef/validator.go:61
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.
💡 SuggestionProvide 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.
🟠 Error apidef/validator.go:61
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.
💡 SuggestionUpdate 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.

✅ Connectivity Check Passed

No connectivity issues found – changes LGTM.


Powered by Visor from Probelabs

Last updated: 2025-11-07T15:10:41.582Z | Triggered by: synchronize | Commit: eaeede1

💡 TIP: You can chat with Visor using /visor ask <your question>

@andrei-tyk
Copy link
Copy Markdown
Contributor Author

All probelabs issues are invalid as the all 0 targets did not exist before this feature.

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Nov 7, 2025

@andrei-tyk andrei-tyk merged commit d3d522a into master Nov 10, 2025
48 of 49 checks passed
@andrei-tyk andrei-tyk deleted the TT-15595-temporarily-remove-upstream-target-from-load-balancing branch November 10, 2025 09:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants