Skip to content
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

Add Docker support for sample mcp servers #368

Merged
merged 21 commits into from
Dec 20, 2024

Conversation

slimslenderslacks
Copy link
Contributor

@slimslenderslacks slimslenderslacks commented Dec 17, 2024

Description

Server Details

  • Server: for all 18 servers, we are adding support for running them using docker
  • Changes to: no changes to server functionality or how they are run using either npx or uv. Minor code changes to filesystem, and puppeteer to support containerization

Motivation and Context

This is an additional option for running mcp servers that addresses a few pain points:

  • Environment Conflicts: Installing MCP servers often requires specific versions of Node.js, Python, and other dependencies, which may conflict with existing installations on a user’s machine.
  • Complex Setup: Users need to manually clone the server repository, install dependencies, and configure their environment, which can be time-consuming and error-prone.
  • Cross-Platform Challenges: Running the servers consistently across different architectures (e.g., x86 vs. ARM) or operating systems can introduce additional complexity.
  • Dependency Isolation: Ensuring that server-specific runtime dependencies like Git or SQLite do not interfere with the host environment can be tricky.

How Has This Been Tested?

We are testing with a claude_desktop_config.json containing all 18 servers. We are also adding additional entries to each of the server README.md projects to describe how to make the optional docker-based client configuration. The list of tests we have to describe here is quite long since we have to re-test all 18 servers.

Breaking Changes

No, this will not impact any existing installs. It is an orthogonal, and new, way to install and run the servers.

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)
  • Documentation update

Checklist

  • I have read the MCP Protocol Documentation
  • My changes follows MCP security best practices
  • I have updated the server's README accordingly
  • I have tested this with an LLM client
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have documented all environment variables and configuration options

Additional context

@ColinMcNeil ColinMcNeil force-pushed the docker/servers branch 3 times, most recently from 2e55e9d to de059f1 Compare December 18, 2024 17:51
@slimslenderslacks slimslenderslacks marked this pull request as ready for review December 19, 2024 21:30
Copy link
Member

@dsp-ant dsp-ant left a comment

Choose a reason for hiding this comment

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

Thank you for working on this. Quite excited.

src/everart/README.md Outdated Show resolved Hide resolved
src/fetch/Dockerfile Outdated Show resolved Hide resolved
src/gdrive/README.md Outdated Show resolved Hide resolved
Comment on lines +52 to +53
#### Docker

Copy link
Member

Choose a reason for hiding this comment

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

We likely want ot highlight the difference between the npx and the docker version (mostly just passing credentials (which i think in the explizit form is nicer).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ya, we've added a section on the explicit authentication step (which is also a docker cli command). This is a prerequisite step before adding the gdrive server to mcpServers

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Great thank you.

src/git/README.md Outdated Show resolved Hide resolved
src/puppeteer/Dockerfile Outdated Show resolved Hide resolved
src/puppeteer/README.md Outdated Show resolved Hide resolved
@@ -31,8 +31,9 @@ The server provides access to Google Drive files:
3. [Configure an OAuth consent screen](https://console.cloud.google.com/apis/credentials/consent) ("internal" is fine for testing)
4. Add OAuth scope `https://www.googleapis.com/auth/drive.readonly`
5. [Create an OAuth Client ID](https://console.cloud.google.com/apis/credentials/oauthclient) for application type "Desktop App"
6. Download the JSON file of your client's OAuth keys
7. Rename the key file to `gcp-oauth.keys.json` and place into the root of this repo (i.e. `servers/gcp-oauth.keys.json`)
6. Add http://localhost:3000/oauth2callback as a redirect URI
Copy link
Contributor

Choose a reason for hiding this comment

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

@dsp-ant Hope you don't mind, but I found this extra step was important to get the mcp server to work regardless of Docker/npx

Copy link
Member

Choose a reason for hiding this comment

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

Thank you for noticing. I think is fine.


# Basic script to replace opn(authorizeUrl, { wait: false }).then(cp => cp.unref()); with process.stdout.write(`Open this URL in your browser: ${authorizeUrl}`);

sed -i 's/opn(authorizeUrl, { wait: false }).then(cp => cp.unref());/process.stderr.write(`Open this URL in your browser: ${authorizeUrl}\n`);/' node_modules/@google-cloud/local-auth/build/src/index.js
Copy link
Contributor

Choose a reason for hiding this comment

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

Build-time workaround for trying to open user's browser from inside the container. Instead of triggering xdg-open, this prints a URL to stderr:

Screenshot 2024-12-19 at 6 54 35 PM

dsp-ant
dsp-ant previously approved these changes Dec 20, 2024
Copy link
Member

@dsp-ant dsp-ant left a comment

Choose a reason for hiding this comment

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

Thank you so much for getting this over the finish line. I am super stoked to have this and can't wait for people to work with MCP servers that can take really interesting dependencies because of docker containers.

A few suggestions:

  1. We should probably add a note about the usage of Docker to the top-level readme, including a link to the mcp namespace on docker hub.
  2. I would love to see a section in docs/ on how to build MCP servers with docker.
  3. I'd love if we would say on the README that this is maintained by Docker itself.

A few questions:

  1. How should we deal with CI? Do we need to add anything to our .github/workflows to trigger builds?
  2. Where would users report errors about docker setup. We can help with maintenance but I'd love if there you all could help out. Happy to redirect them to a docker repo or if you want a docker tag in this repo to follow.

@@ -31,8 +31,9 @@ The server provides access to Google Drive files:
3. [Configure an OAuth consent screen](https://console.cloud.google.com/apis/credentials/consent) ("internal" is fine for testing)
4. Add OAuth scope `https://www.googleapis.com/auth/drive.readonly`
5. [Create an OAuth Client ID](https://console.cloud.google.com/apis/credentials/oauthclient) for application type "Desktop App"
6. Download the JSON file of your client's OAuth keys
7. Rename the key file to `gcp-oauth.keys.json` and place into the root of this repo (i.e. `servers/gcp-oauth.keys.json`)
6. Add http://localhost:3000/oauth2callback as a redirect URI
Copy link
Member

Choose a reason for hiding this comment

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

Thank you for noticing. I think is fine.

Comment on lines +52 to +53
#### Docker

Copy link
Member

Choose a reason for hiding this comment

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

Great thank you.

@@ -0,0 +1,5 @@
#! /bin/bash

# Basic script to replace opn(authorizeUrl, { wait: false }).then(cp => cp.unref()); with process.stdout.write(`Open this URL in your browser: ${authorizeUrl}`);
Copy link
Member

Choose a reason for hiding this comment

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

Maybe in a follow up, let's add the reasoning you described. future me will thank if this is inside the bash script instead of a github comment :)

src/git/Dockerfile Show resolved Hide resolved
@dsp-ant dsp-ant merged commit d5b6b4c into modelcontextprotocol:main Dec 20, 2024
@slimslenderslacks slimslenderslacks deleted the docker/servers branch December 20, 2024 20:10
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.

3 participants