[TT-881] fix issue where upstream targets have been duplicated

[pvormste]

User description

TT-881

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

---

This PR fixes an issue where upstream targets are being duplicated therefore resulting in increased weights for load balancing.

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

Bug fix, Tests

---

Description

• Fixed duplication of upstream targets in load balancing.

• Simplified initialization of proxyConfTargets in ExtractTo.

• Added test case to validate target deduplication logic.

• Ensured existing targets are handled correctly in tests.

---

Changes walkthrough 📝

	Relevant files
Bug fix	upstream.go Fix and simplify upstream target handling logic                    apidef/oas/upstream.go Removed unnecessary initialization of proxyConfTargets. Simplified logic for handling upstream targets. Ensured no duplication of targets during extraction. +1/-5
Tests	upstream_test.go Add test for upstream target deduplication                              apidef/oas/upstream_test.go Added test case to verify deduplication of upstream targets. Initialized existing targets in test setup. Validated expected targets and load balancing state. +5/-0

---

Need help?

Type /help how to ... in the comments thread for any questions about PR-Agent usage.

Check out the documentation for more information.

[buger]

Knock Knock! 🔍

Just thought I'd let you know that your PR title and story title look quite different. PR titles that closely resemble the story title make it easier for reviewers to understand the context of the PR.

An easy-to-understand PR title a day makes the reviewer review away! 😛⚡️

Story Title	[OAS] Upstream load balancing
PR Title	fix issue where upstream targets have been duplicated (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: 3 🔵🔵🔵⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Possible Logic Issue The new implementation initializes proxyConfTargets as an empty slice, potentially overwriting existing targets in api.Proxy.Targets. This change should be reviewed to ensure it aligns with the intended behavior and does not introduce regressions. proxyConfTargets := make([]string, 0) api.Proxy.EnableLoadBalancing = l.Enabled for _, target := range l.Targets { for i := 0; i < target.Weight; i++ { proxyConfTargets = append(proxyConfTargets, target.URL) Test Coverage Validation The test case initializes apiDef.Proxy.Targets with predefined values. It should be validated whether this test sufficiently covers scenarios where apiDef.Proxy.Targets is nil or already populated. apiDef.Proxy.Targets = []string{ "http://old1.upstream.test", "http://old2.upstream.test", "http://old3.upstream.test", }

[github-actions]

API Changes

no api changes detected

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Possible issue	Update api.Proxy.Targets after modification Ensure that proxyConfTargets is assigned back to api.Proxy.Targets after appending the target URLs to avoid potential issues with the api.Proxy.Targets field not being updated. apidef/oas/upstream.go [900-902] for _, target := range l.Targets { for i := 0; i < target.Weight; i++ { proxyConfTargets = append(proxyConfTargets, target.URL) } } +api.Proxy.Targets = proxyConfTargets Suggestion importance[1-10]: 9 Why: The suggestion ensures that proxyConfTargets is assigned back to api.Proxy.Targets, which is crucial for maintaining the integrity of the api.Proxy.Targets field. Without this, the updates to proxyConfTargets would not reflect in api.Proxy.Targets, potentially causing functional issues. This is a critical fix for correctness.	9
General	Verify old targets are replaced Add assertions to verify that the initial apiDef.Proxy.Targets values are correctly replaced by the new targets generated by the ExtractTo function. apidef/oas/upstream_test.go [665-666] assert.Equal(t, tc.expectedEnabled, apiDef.Proxy.EnableLoadBalancing) assert.Equal(t, tc.expectedTargets, apiDef.Proxy.Targets) +assert.NotContains(t, apiDef.Proxy.Targets, "http://old1.upstream.test") +assert.NotContains(t, apiDef.Proxy.Targets, "http://old2.upstream.test") +assert.NotContains(t, apiDef.Proxy.Targets, "http://old3.upstream.test") Suggestion importance[1-10]: 7 Why: Adding assertions to confirm that old targets are replaced by the new ones enhances test coverage and ensures correctness of the ExtractTo function. While not critical, it improves the robustness of the test suite by verifying expected behavior.	7

[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