[TT-13507][TT-12873] Fix for custom domains with substring listen path

andrei-tyk commented 2 weeks ago

User description

Actual fix for domain matching edge case related to substring listen paths. Fixed the sorting function for the listen paths and added some unit tests.

Extra: httpbin service not working on macos with apple silicon; now it does

Description

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

PR Type

Bug fix, Configuration changes

Description

Fixed the domain sorting logic in the API loader by changing the comparison from domain length to lexicographical order to ensure correct domain matching.
Updated the Docker configuration for the httpbin service to specify the platform as linux/amd64, addressing compatibility issues on macOS with Apple Silicon.

Changes walkthrough 📝

Relevant files

Bug fix

api_loader.go

Fix domain sorting logic in API loader

gateway/api_loader.go

Modified sorting logic for API specs by domain.

Changed comparison from domain length to lexicographical order.

+1/-1

Configuration changes

httpbin.yml

Specify platform for httpbin service in Docker configuration

docker/services/httpbin.yml - Added platform specification for httpbin service.

+1/-0

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

buger commented 2 weeks ago

:broken_heart: The detected issue is not in one of the allowed statuses :broken_heart:

Detected Status	DoD Check	:x:
Allowed Statuses	In Dev,In Code Review,Ready for Testing,In Test,In Progress,In Review	:heavy_check_mark:

Please ensure your jira story is in one of the allowed statuses

github-actions[bot] commented 2 weeks ago

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

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

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review

Domain Sorting Logic
The change from length-based to lexicographical domain sorting may introduce issues with domain matching where subdomains are involved. This needs thorough testing to ensure no regression or unexpected behavior in domain matching.

github-actions[bot] commented 2 weeks ago

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Best practice	Ensure domain comparison is case-insensitive to avoid inconsistencies ___ Replace the string comparison for domains with a case-insensitive comparison to ensure consistent behavior regardless of case variations. [gateway/api_loader.go [938]](https://github.com/TykTechnologies/tyk/pull/6705/files#diff-cdf0b7f176c9d18e1a314b78ddefc2cb3a94b3de66f1f360174692c915734c68R938-R938) ```diff -return specs[i].Domain > specs[j].Domain +return strings.ToLower(specs[i].Domain) > strings.ToLower(specs[j].Domain) ``` Suggestion importance[1-10]: 7 Why: The suggestion to make domain comparison case-insensitive is valid and improves the robustness of the sorting logic by ensuring consistent behavior across different case variations.	7
Possible issue	Check compatibility of the specified platform with deployment environments ___ Verify the compatibility of the specified platform 'linux/amd64' with all potential deployment environments to ensure there are no runtime issues on different architectures. [docker/services/httpbin.yml [5]](https://github.com/TykTechnologies/tyk/pull/6705/files#diff-1a1b1d8bf4798076a41ca5f3bba0d465dc387e58b6593adbb28a4d5fe60eb810R5-R5) ```diff -platform: linux/amd64 +platform: linux/amd64 # Ensure compatibility with deployment environments ``` Suggestion importance[1-10]: 1 Why: The suggestion is a general reminder to check compatibility, which is a good practice but does not directly improve or modify the code. It's more of a cautionary note than a code improvement.	1