feat: integrate user's ip address into debugger tool COMPASS-11046 - #8397
feat: integrate user's ip address into debugger tool COMPASS-11046#8397dudaschar wants to merge 5 commits into
Conversation
|
Assigned |
There was a problem hiding this comment.
Pull request overview
Integrates the user’s IP address from Atlas system status into the connection debugger.
Changes:
- Adds validated system-status API support and authentication allowlisting.
- Uses the retrieved IP with graceful fallback behavior.
- Adds API, authentication, and debugger tests.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
packages/compass-generative-ai/src/tools/debug-connection.ts |
Uses the resolved IP for access checks. |
packages/compass-generative-ai/src/tools/debug-connection.spec.ts |
Tests IP resolution and fallback behavior. |
packages/atlas-service/src/main.spec.ts |
Tests system-status authentication. |
packages/atlas-service/src/atlas-admin-api-auth-endpoints.ts |
Allowlists the system-status endpoint. |
packages/atlas-admin-api/src/system-status-types.ts |
Defines and validates system-status responses. |
packages/atlas-admin-api/src/index.ts |
Exports the new response type. |
packages/atlas-admin-api/src/atlas-admin-api-service.ts |
Implements system-status retrieval. |
packages/atlas-admin-api/src/atlas-admin-api-service.spec.ts |
Tests system-status API behavior. |
Suppressed comments (1)
packages/atlas-admin-api/src/index.ts:14
AtlasAdminApiServiceis exposed and consumed through the@mongodb-js/atlas-admin-api/providerentrypoint, whose type exports inprovider.tsxmirror this barrel for the existing response types. ExportingAtlasSystemStatusonly fromsrc/index.tsleaves the return type of the newgetSystemStatusAPI unavailable from the entrypoint consumers already use; please re-export it fromprovider.tsxas well.
export { type AtlasSystemStatus } from './system-status-types';
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| `Bearer ${accessToken}` | ||
| ); | ||
| }); | ||
|
|
There was a problem hiding this comment.
I don't think we need to test every endpoint that is on the list, but it won't hurt I suppose
| ipAccessStatus: | ||
| ipAccessList && userIp && isUserIpIncluded(ipAccessList, userIp) | ||
| ? 'Client IP Allowed' | ||
| : 'Could not confirm', |
There was a problem hiding this comment.
Surprisingly, many customers have problems with ip whitelisting. To give more information, I would separate:
- Atlas is unavailable and we can't confirm
- Atlas is available and my ip is not in the range.
There was a problem hiding this comment.
There was a problem hiding this comment.
We left the "can't confirm" intentionally vague for now as we realised we can't always tell (for example we don't know how to check awsSecurityGroup), but distinguishing that the request failed would make sense if we allow partial results. For now the plan is to not go with partial results though, if some of the requests fail we just let the user to retry the tool.
There was a problem hiding this comment.
We might want to add a clear 'not match' for when we can tell for sure though, will probably be a follow up improvement
paula-stacho
left a comment
There was a problem hiding this comment.
Left minor comment, otherwise looks good 🎉
Description
This PR is using the system status endpoint integration made in #8396.
Replacing hard coded ipAddress with retrieved value from atlas admin endpoint.
Checklist
Motivation and Context
Open Questions
Dependents
Types of changes