add load balancing to oas configuration (TT-881)

[pvormste]

User description

TT-881

Summary	[OAS] Upstream load balancing
Type	Story
Status	In Dev
Points	N/A
Labels	A, CSE, EMEA, Gold, customer_request, jira_escalated, updated

---

This PR adds weighted load balancing to OAS configuration.

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)

---

PR Type

Enhancement, Tests

---

Description

• Added load balancing configuration to the OAS upstream definition.

• Implemented LoadBalancing and LoadBalancingTarget structures with related methods.

• Updated schema to include load balancing configuration.

• Added comprehensive unit tests for load balancing functionality.

---

Changes walkthrough 📝

	Relevant files
Tests	oas_test.go Remove deprecated load balancing references in tests          apidef/oas/oas_test.go Removed references to deprecated load balancing fields. Cleaned up test cases to align with new load balancing implementation. +0/-2      upstream_test.go Add unit tests for load balancing functionality                    apidef/oas/upstream_test.go Added unit tests for LoadBalancing and LoadBalancingTarget. Tested fill and extractTo methods for load balancing. Verified behavior with various load balancing configurations. +192/-0
Enhancement	upstream.go Add load balancing logic to upstream handling                        apidef/oas/upstream.go Added LoadBalancing and LoadBalancingTarget structures. Implemented methods to populate and extract load balancing configurations. Integrated load balancing logic into upstream handling. +104/-0  x-tyk-api-gateway.json Update schema to support load balancing configuration        apidef/oas/schema/x-tyk-api-gateway.json Updated schema to include loadBalancing configuration. Added definitions for X-Tyk-LoadBalancing and X-Tyk-LoadBalancingTarget. +38/-0

---

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

[buger]

Let's make that PR title a 💯 shall we? 💪

Your PR title and story title look slightly different. Just checking in to know if it was intentional!

Story Title	[OAS] Upstream load balancing
PR Title	add load balancing to oas configuration (TT-881)

Check out this guide to learn more about PR best-practices.

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Possible Issue The Fill method in LoadBalancing uses a map to count targets and then converts it to a slice. Ensure that the logic correctly handles cases where targets have duplicate URLs with different weights or other edge cases. func (l *LoadBalancing) Fill(api apidef.APIDefinition) { if len(api.Proxy.Targets) == 0 { api.Proxy.EnableLoadBalancing = false api.Proxy.Targets = nil return } l.Enabled = api.Proxy.EnableLoadBalancing targetCounter := make(map[string]*LoadBalancingTarget) for _, target := range api.Proxy.Targets { if _, ok := targetCounter[target]; !ok { targetCounter[target] = &LoadBalancingTarget{ URL: target, Weight: 0, } } targetCounter[target].Weight++ } targets := make([]LoadBalancingTarget, len(targetCounter)) i := 0 for _, target := range targetCounter { targets[i] = *target i++ } targetsSorter := func(i, j int) bool { return targets[i].URL < targets[j].URL } sort.Slice(targets, targetsSorter) l.Targets = targets } Test Coverage The test cases for LoadBalancing do not seem to cover scenarios where the Targets array contains invalid or malformed URLs. Consider adding such test cases to ensure robustness. func TestLoadBalancing(t *testing.T) { t.Parallel() t.Run("fill", func(t *testing.T) { t.Parallel() testcases := []struct { title string input apidef.APIDefinition expected *LoadBalancing }{ { title: "disable load balancing when targets list is empty", input: apidef.APIDefinition{ Proxy: apidef.ProxyConfig{ EnableLoadBalancing: true, Targets: []string{}, }, }, expected: nil, }, { title: "load balancing disabled with filled target list", input: apidef.APIDefinition{ Proxy: apidef.ProxyConfig{ EnableLoadBalancing: false, Targets: []string{ "http://upstream-one", "http://upstream-one", "http://upstream-one", "http://upstream-one", "http://upstream-one", "http://upstream-three", "http://upstream-three", }, }, }, expected: &LoadBalancing{ Enabled: false, Targets: []LoadBalancingTarget{ { URL: "http://upstream-one", Weight: 5, }, { URL: "http://upstream-three", Weight: 2, }, }, }, }, { title: "load balancing enabled with filled target list", input: apidef.APIDefinition{ Proxy: apidef.ProxyConfig{ EnableLoadBalancing: true, Targets: []string{ "http://upstream-one", "http://upstream-one", "http://upstream-one", "http://upstream-one", "http://upstream-one", "http://upstream-three", "http://upstream-three", }, }, }, expected: &LoadBalancing{ Enabled: true, Targets: []LoadBalancingTarget{ { URL: "http://upstream-one", Weight: 5, }, { URL: "http://upstream-three", Weight: 2, }, }, }, }, } for _, tc := range testcases { tc := tc t.Run(tc.title, func(t *testing.T) { t.Parallel() g := new(Upstream) g.Fill(tc.input) assert.Equal(t, tc.expected, g.LoadBalancing) }) } }) t.Run("extractTo", func(t *testing.T) { t.Parallel() testcases := []struct { title string input *LoadBalancing expectedEnabled bool expectedTargets []string }{ { title: "disable load balancing when targets list is empty", input: &LoadBalancing{ Enabled: false, Targets: nil, }, expectedEnabled: false, expectedTargets: nil, }, { title: "load balancing disabled with filled target list", input: &LoadBalancing{ Enabled: false, Targets: []LoadBalancingTarget{ { URL: "http://upstream-one", Weight: 5, }, { URL: "http://upstream-two", Weight: 0, }, { URL: "http://upstream-three", Weight: 2, }, }, }, expectedEnabled: false, expectedTargets: []string{ "http://upstream-one", "http://upstream-one", "http://upstream-one", "http://upstream-one", "http://upstream-one", "http://upstream-three", "http://upstream-three", }, }, { title: "load balancing enabled with filled target list", input: &LoadBalancing{ Enabled: true, Targets: []LoadBalancingTarget{ { URL: "http://upstream-one", Weight: 5, }, { URL: "http://upstream-two", Weight: 0, }, { URL: "http://upstream-three", Weight: 2, }, }, }, expectedEnabled: true, expectedTargets: []string{ "http://upstream-one", "http://upstream-one", "http://upstream-one", "http://upstream-one", "http://upstream-one", "http://upstream-three", "http://upstream-three", }, }, } for _, tc := range testcases { tc := tc // Creating a new 'tc' scoped to the loop t.Run(tc.title, func(t *testing.T) { t.Parallel() g := new(Upstream) g.LoadBalancing = tc.input var apiDef apidef.APIDefinition g.ExtractTo(&apiDef) assert.Equal(t, tc.expectedEnabled, apiDef.Proxy.EnableLoadBalancing) assert.Equal(t, tc.expectedTargets, apiDef.Proxy.Targets) }) } }) }

[github-actions]

API Changes

--- prev.txt	2025-01-15 12:56:47.271413586 +0000
+++ current.txt	2025-01-15 12:56:42.662355967 +0000
@@ -3086,6 +3086,32 @@
 func (lp *ListenPath) Fill(api apidef.APIDefinition)
     Fill fills *ListenPath from apidef.APIDefinition.
 
+type LoadBalancing struct {
+	// Enabled determines if load balancing is active.
+	Enabled bool `json:"enabled,omitempty" bson:"enabled,omitempty"`
+	// Targets defines the list of targets with their respective weights for load balancing.
+	Targets []LoadBalancingTarget `json:"targets,omitempty" bson:"targets,omitempty"`
+}
+    LoadBalancing represents the configuration for load balancing between
+    multiple upstream targets.
+
+func (l *LoadBalancing) ExtractTo(api *apidef.APIDefinition)
+    ExtractTo populates an APIDefinition's proxy load balancing configuration
+    with data from the LoadBalancing instance.
+
+func (l *LoadBalancing) Fill(api apidef.APIDefinition)
+    Fill populates the LoadBalancing structure based on the provided
+    APIDefinition, including targets and their weights.
+
+type LoadBalancingTarget struct {
+	// URL specifies the upstream target URL for load balancing, represented as a string.
+	URL string `json:"url,omitempty" bson:"url,omitempty"`
+	// Weight specifies the relative distribution factor for load balancing, determining the importance of this target.
+	Weight int `json:"weight,omitempty" bson:"weight,omitempty"`
+}
+    LoadBalancingTarget represents a single upstream target for load balancing
+    with a URL and an associated weight.
+
 type Middleware struct {
 	// Global contains configuration for middleware that affects the whole API (all endpoints).
 	Global *Global `bson:"global,omitempty" json:"global,omitempty"`
@@ -4221,6 +4247,9 @@
 
 	// Authentication contains the configuration related to upstream authentication.
 	Authentication *UpstreamAuth `bson:"authentication,omitempty" json:"authentication,omitempty"`
+
+	// LoadBalancing contains configuration for load balancing between multiple upstream targets.
+	LoadBalancing *LoadBalancing `bson:"loadBalancing,omitempty" json:"loadBalancing,omitempty"`
 }
     Upstream holds configuration for the upstream server to which Tyk should
     proxy requests.

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
General	Restrict the weight property to non-negative integers in the JSON schema Ensure that the weight property in X-Tyk-LoadBalancingTarget is defined as a non-negative integer to prevent invalid configurations. apidef/oas/schema/x-tyk-api-gateway.json [2249-2250] "weight": { - "type": "number" + "type": "integer", + "minimum": 0 } Suggestion importance[1-10]: 9 Why: This change ensures that the weight property is restricted to non-negative integers, which aligns with the intended use and prevents invalid configurations at the schema level.	9
	Add test cases to validate handling of invalid URLs and weights in load balancing targets Include test cases for scenarios where LoadBalancing.Targets contains invalid URLs or weights to ensure robust validation. apidef/oas/upstream_test.go [525-553] { - title: "load balancing enabled with filled target list", + title: "load balancing with invalid target URLs", input: apidef.APIDefinition{ Proxy: apidef.ProxyConfig{ EnableLoadBalancing: true, Targets: []string{ - "http://upstream-one", - "http://upstream-one", - "http://upstream-one", + "invalid-url", + "http://valid-url", }, }, }, - expected: &LoadBalancing{ - Enabled: true, - Targets: []LoadBalancingTarget{ - { - URL: "http://upstream-one", - Weight: 3, - }, - }, - }, + expected: nil, // Expect validation to fail }, Suggestion importance[1-10]: 7 Why: Adding test cases for invalid URLs and weights enhances the robustness of the test suite, ensuring that edge cases are handled correctly. However, the suggestion does not address weights explicitly in the example provided.	7
Possible issue	Add a check to handle empty slices before sorting to prevent runtime errors Ensure that the sort.Slice function in the Fill method of LoadBalancing properly handles cases where targets is empty to avoid potential runtime panics. apidef/oas/upstream.go [886-887] -sort.Slice(targets, targetsSorter) +if len(targets) > 0 { + sort.Slice(targets, targetsSorter) +} l.Targets = targets Suggestion importance[1-10]: 8 Why: The suggestion adds a safeguard to prevent runtime errors when attempting to sort an empty slice, which is a valid and practical improvement to ensure robustness.	8

[sonarqubecloud]

Quality Gate passed

Issues
7 New issues
0 Accepted issues

Measures
0 Security Hotspots
83.5% Coverage on New Code
0.2% Duplication on New Code

See analysis details on SonarQube Cloud