Skip to content

Conversation

@rakdutta
Copy link
Collaborator

@rakdutta rakdutta commented Aug 11, 2025

Fix issue #649

This pull request introduces a new function to normalize gateway URLs by resolving hostnames to their corresponding IP addresses. The main goal is to ensure that gateway URLs referencing the same endpoint—whether by hostname or IP—are treated as identical rather than distinct.

Key changes:

  • Added a function to normalize URLs by resolving hostnames to IP addresses.
  • Updated the register_gateway and update_gateway methods to use the normalization function, ensuring consistent handling of gateway URLs.
  • Modified related unit tests to validate the new normalization logic and ensure correct behavior when using both hostname and IP address forms.

These changes help prevent duplicate gateway registrations and updates caused by different URL representations, improving consistency and reliability.


Test Steps:

  1. Added a new gateway server with name mcp-server and URL http://localhost:8080/sse, verified that the URL was normalized to http://127.0.0.1:8080/sse and reflected correctly in both the database and UI.

  2. Added a new gateway server with name mcp-server1 and URL http://localhost:8000/sse, verified that the URL was normalized to http://127.0.0.1:8000/sse and reflected correctly in both the database and UI.

  3. Attempted to add a new gateway server with name mcp-server2 and URL http://127.0.0.1:8080/sse (or http://localhost:8080/sse), blocked with the message "A gateway with this URL already exists".

  4. Attempted to update the URL for mcp-server1 to http://127.0.0.1:8080/sse (or http://localhost:8080/sse), blocked with the message "A gateway with this URL already exists".


DB Output Screenshot: Attached below is the database view showing the gateways table entries corresponding to the above test cases, confirming stored id, name, and normalized url values.

image

UI Output Screenshot: Attached below is the UI view showing the gateway entries for the above test cases, confirming correct display of names, normalized URLs, and statuses.

image

@rakdutta rakdutta added this to the Release 0.6.0 milestone Aug 11, 2025
@rakdutta rakdutta marked this pull request as ready for review August 11, 2025 07:02
@MohanLaksh
Copy link
Collaborator

PR Test summary

make serve - pass
make test - pass (83%, === 1552 passed, 10 skipped, 55 warnings in 70.44s (0:01:10) ===)
make autoflake isort black flake8 - pass
make pylint - pass (Your code has been rated at 10.00/10 )
make smoketest - ✅ Smoketest passed!
make doctest - pass (55%, 445 passed, 8 skipped, 1 warning in 14.07s)

Testing:

image

Good to Merge

rakdutta1 and others added 2 commits August 11, 2025 11:31
Signed-off-by: RAKHI DUTTA <[email protected]>
Signed-off-by: Mihai Criveti <[email protected]>
@crivetimihai crivetimihai merged commit 6ccc204 into IBM:main Aug 11, 2025
37 checks passed
crivetimihai added a commit that referenced this pull request Aug 14, 2025
The normalize_url method was resolving domain names to IP addresses,
which breaks for services behind CDNs, load balancers, or reverse
proxies. Removed DNS resolution to preserve original domain names.

This issue was introduced in PR #712.

- Removed socket.gethostbyname() DNS resolution
- Updated normalize_url to preserve original domains
- Added regression test to verify domains are preserved
- Removed unused socket import

Signed-off-by: Mihai Criveti <[email protected]>
crivetimihai added a commit that referenced this pull request Aug 14, 2025
The normalize_url method was resolving ALL domain names to IP addresses,
which breaks for services behind CDNs, load balancers, or reverse
proxies. This fix:

- Preserves domain names for external services (CDN/load balancer support)
- Only normalizes 127.0.0.1 to localhost to prevent local duplicates
- Maintains fix for issue #649 for localhost/127.0.0.1 duplicates
- Keeps compatibility with PR #712's duplicate prevention for local services

Changes:
- Removed socket.gethostbyname() for external domains
- Added special case to convert 127.0.0.1 to localhost
- Updated tests to verify both behaviors
- Added regression test for duplicate prevention

This balances the needs of both issues:
- #649: Prevent localhost/127.0.0.1 duplicates (still fixed)
- #744: Support services behind CDNs/load balancers (now fixed)

Signed-off-by: Mihai Criveti <[email protected]>
crivetimihai added a commit that referenced this pull request Aug 14, 2025
* Fix #744: Preserve domain names while preventing localhost duplicates

The normalize_url method was resolving ALL domain names to IP addresses,
which breaks for services behind CDNs, load balancers, or reverse
proxies. This fix:

- Preserves domain names for external services (CDN/load balancer support)
- Only normalizes 127.0.0.1 to localhost to prevent local duplicates
- Maintains fix for issue #649 for localhost/127.0.0.1 duplicates
- Keeps compatibility with PR #712's duplicate prevention for local services

Changes:
- Removed socket.gethostbyname() for external domains
- Added special case to convert 127.0.0.1 to localhost
- Updated tests to verify both behaviors
- Added regression test for duplicate prevention

This balances the needs of both issues:
- #649: Prevent localhost/127.0.0.1 duplicates (still fixed)
- #744: Support services behind CDNs/load balancers (now fixed)

Signed-off-by: Mihai Criveti <[email protected]>

* Fix localhost resolution

Signed-off-by: Mihai Criveti <[email protected]>

* fix pylint

Signed-off-by: RAKHI DUTTA <[email protected]>

---------

Signed-off-by: Mihai Criveti <[email protected]>
Signed-off-by: RAKHI DUTTA <[email protected]>
Co-authored-by: RAKHI DUTTA <[email protected]>
vk-playground pushed a commit to vk-playground/mcp-context-forge that referenced this pull request Sep 14, 2025
…ss Check (IBM#712)

* issue 649

Signed-off-by: RAKHI DUTTA <[email protected]>

* Rebased and linted

Signed-off-by: Mihai Criveti <[email protected]>

---------

Signed-off-by: RAKHI DUTTA <[email protected]>
Signed-off-by: Mihai Criveti <[email protected]>
Co-authored-by: RAKHI DUTTA <[email protected]>
Co-authored-by: Mihai Criveti <[email protected]>
vk-playground pushed a commit to vk-playground/mcp-context-forge that referenced this pull request Sep 14, 2025
* Fix IBM#744: Preserve domain names while preventing localhost duplicates

The normalize_url method was resolving ALL domain names to IP addresses,
which breaks for services behind CDNs, load balancers, or reverse
proxies. This fix:

- Preserves domain names for external services (CDN/load balancer support)
- Only normalizes 127.0.0.1 to localhost to prevent local duplicates
- Maintains fix for issue IBM#649 for localhost/127.0.0.1 duplicates
- Keeps compatibility with PR IBM#712's duplicate prevention for local services

Changes:
- Removed socket.gethostbyname() for external domains
- Added special case to convert 127.0.0.1 to localhost
- Updated tests to verify both behaviors
- Added regression test for duplicate prevention

This balances the needs of both issues:
- IBM#649: Prevent localhost/127.0.0.1 duplicates (still fixed)
- IBM#744: Support services behind CDNs/load balancers (now fixed)

Signed-off-by: Mihai Criveti <[email protected]>

* Fix localhost resolution

Signed-off-by: Mihai Criveti <[email protected]>

* fix pylint

Signed-off-by: RAKHI DUTTA <[email protected]>

---------

Signed-off-by: Mihai Criveti <[email protected]>
Signed-off-by: RAKHI DUTTA <[email protected]>
Co-authored-by: RAKHI DUTTA <[email protected]>
vk-playground pushed a commit to vk-playground/mcp-context-forge that referenced this pull request Sep 14, 2025
…ss Check (IBM#712)

* issue 649

Signed-off-by: RAKHI DUTTA <[email protected]>

* Rebased and linted

Signed-off-by: Mihai Criveti <[email protected]>

---------

Signed-off-by: RAKHI DUTTA <[email protected]>
Signed-off-by: Mihai Criveti <[email protected]>
Co-authored-by: RAKHI DUTTA <[email protected]>
Co-authored-by: Mihai Criveti <[email protected]>
vk-playground pushed a commit to vk-playground/mcp-context-forge that referenced this pull request Sep 14, 2025
* Fix IBM#744: Preserve domain names while preventing localhost duplicates

The normalize_url method was resolving ALL domain names to IP addresses,
which breaks for services behind CDNs, load balancers, or reverse
proxies. This fix:

- Preserves domain names for external services (CDN/load balancer support)
- Only normalizes 127.0.0.1 to localhost to prevent local duplicates
- Maintains fix for issue IBM#649 for localhost/127.0.0.1 duplicates
- Keeps compatibility with PR IBM#712's duplicate prevention for local services

Changes:
- Removed socket.gethostbyname() for external domains
- Added special case to convert 127.0.0.1 to localhost
- Updated tests to verify both behaviors
- Added regression test for duplicate prevention

This balances the needs of both issues:
- IBM#649: Prevent localhost/127.0.0.1 duplicates (still fixed)
- IBM#744: Support services behind CDNs/load balancers (now fixed)

Signed-off-by: Mihai Criveti <[email protected]>

* Fix localhost resolution

Signed-off-by: Mihai Criveti <[email protected]>

* fix pylint

Signed-off-by: RAKHI DUTTA <[email protected]>

---------

Signed-off-by: Mihai Criveti <[email protected]>
Signed-off-by: RAKHI DUTTA <[email protected]>
Co-authored-by: RAKHI DUTTA <[email protected]>
vk-playground pushed a commit to vk-playground/mcp-context-forge that referenced this pull request Sep 16, 2025
…ss Check (IBM#712)

* issue 649

Signed-off-by: RAKHI DUTTA <[email protected]>

* Rebased and linted

Signed-off-by: Mihai Criveti <[email protected]>

---------

Signed-off-by: RAKHI DUTTA <[email protected]>
Signed-off-by: Mihai Criveti <[email protected]>
Co-authored-by: RAKHI DUTTA <[email protected]>
Co-authored-by: Mihai Criveti <[email protected]>
vk-playground pushed a commit to vk-playground/mcp-context-forge that referenced this pull request Sep 16, 2025
* Fix IBM#744: Preserve domain names while preventing localhost duplicates

The normalize_url method was resolving ALL domain names to IP addresses,
which breaks for services behind CDNs, load balancers, or reverse
proxies. This fix:

- Preserves domain names for external services (CDN/load balancer support)
- Only normalizes 127.0.0.1 to localhost to prevent local duplicates
- Maintains fix for issue IBM#649 for localhost/127.0.0.1 duplicates
- Keeps compatibility with PR IBM#712's duplicate prevention for local services

Changes:
- Removed socket.gethostbyname() for external domains
- Added special case to convert 127.0.0.1 to localhost
- Updated tests to verify both behaviors
- Added regression test for duplicate prevention

This balances the needs of both issues:
- IBM#649: Prevent localhost/127.0.0.1 duplicates (still fixed)
- IBM#744: Support services behind CDNs/load balancers (now fixed)

Signed-off-by: Mihai Criveti <[email protected]>

* Fix localhost resolution

Signed-off-by: Mihai Criveti <[email protected]>

* fix pylint

Signed-off-by: RAKHI DUTTA <[email protected]>

---------

Signed-off-by: Mihai Criveti <[email protected]>
Signed-off-by: RAKHI DUTTA <[email protected]>
Co-authored-by: RAKHI DUTTA <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants