Skip to content

chore: Support for proxies for Atlas tools. MCP-87 #407

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Jul 30, 2025
Merged

Conversation

kmruiz
Copy link
Collaborator

@kmruiz kmruiz commented Jul 28, 2025

Proposed changes

To add proxy support I had to do a few more changes than expected.

  1. Had to swap simple-oauth2 with oauth4web, as simple-oauth2 does not support custom proxies.
  2. Tweaked ApiClient and openapi/fetch to use node-fetch Request, so it can be proxied as expected.
  3. Switched openapi/fetch to use a devtools-proxy-support createFetch instance
  4. Added also the custom createFetch to oauth4web.
  5. Added some integration tests to verify that the proxy support works. It can be manually tested with mitmproxy easily too.

Also, some relevant info:

  1. Added a fake certificate and key files for testing purposes.
  2. httpsServerProxyTest is a simplified version of the devtools-proxy-support proxy server for testing.

Checklist

kmruiz added 2 commits July 28, 2025 20:33
Right now the code is broken because there is some issue with the
promise returned by the agent. I'm still investigating it.
This adds support for a custom fetch that can use proxies
configured with environment variables.
@kmruiz kmruiz changed the title chore: [broken] Add a test to verify that the http-proxy is called. chore: Add a test to verify that the http-proxy is called. MCP-87 Jul 29, 2025
@kmruiz kmruiz changed the title chore: Add a test to verify that the http-proxy is called. MCP-87 chore: Support for proxies for Atlas tools. MCP-87 Jul 29, 2025
Uses a variant devtools-shared/devtools-proxy-support http proxy
server for testing.
kmruiz added 4 commits July 29, 2025 19:10
node-fetch requires an internal symbol to identify if a
request is an object or a string. Because openapi-fetch
does not provide this symbol, node-fetch misunderstands
the request.
@coveralls
Copy link
Collaborator

coveralls commented Jul 30, 2025

Pull Request Test Coverage Report for Build 16626320664

Details

  • 83 of 86 (96.51%) changed or added relevant lines in 1 file are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.2%) to 80.494%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/common/atlas/apiClient.ts 83 86 96.51%
Files with Coverage Reduction New Missed Lines %
src/common/atlas/apiClient.ts 1 77.6%
Totals Coverage Status
Change from base Build 16594557749: 0.2%
Covered Lines: 3157
Relevant Lines: 3884

💛 - Coveralls

@kmruiz kmruiz marked this pull request as ready for review July 30, 2025 10:24
@kmruiz kmruiz requested a review from a team as a code owner July 30, 2025 10:24
@kmruiz kmruiz enabled auto-merge (squash) July 30, 2025 10:26
@@ -22,9 +23,10 @@
"mongodb-log-writer": "^2.4.1",
"mongodb-redact": "^1.1.8",
"mongodb-schema": "^12.6.2",
"node-fetch": "^3.3.2",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need node-fetch here? my understanding is that we should be using native fetch from v8

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

devtools-proxy-support uses node-fetch right now, so sadly we will need to use node-fetch for now. I'm already in conversations with Anna to see when we can drop support for node-fetch in favour of the native fetch from Node.js.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will stay with node-fetch for a while, because the native fetch doesn't support agents (necessary for proxying) and it doesn't support natively proxies either (support for HTTP and HTTPs proxies is still experimental and doesn't support socks5).

Copy link
Collaborator

@himanshusinghs himanshusinghs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM :)

@kmruiz kmruiz merged commit b24fe5e into main Jul 30, 2025
17 checks passed
@kmruiz kmruiz deleted the chore/mcp-87 branch July 30, 2025 15:14
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