Skip to content

fix: ignore data directory in docker build #44

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

Closed
wants to merge 49 commits into from

Conversation

pboyd
Copy link

@pboyd pboyd commented May 15, 2025

Motivation and Context

I get this error when building the docker image after running the server in docker compose:

ERROR: failed to solve: error from sender: open data/diagnostic.data: permission denied

The issue is that docker-compose.yaml mounts the data directory in the mongo container which creates files that my user doesn't have access too. Except for seed.json, there doesn't appear to be a reason for those files to be in the docker build layer anyway.

This adds a .dockeringore file to exclude everything in data except seed.json.

How Has This Been Tested?

docker build

Breaking Changes

No

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 Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

jspahrsummers and others added 30 commits February 5, 2025 17:58
Work in progress .
This commit adds the core registry service implementation with methods to:
- Retrieve all registry entries
- List entries with cursor-based pagination
- Fetch specific server details by ID
…pi_v0

Init world. Basic registry API server <WIP>
…eadme-cleanup

Update README to reflect binary name change
…ridharavinash/seed

Add count to server response and fix Makefile paths
…pi-docs

Add Swagger API documentation and handlers for v0
Contains about 300+ initial servers that can be used to seed a mongoDB
We need to work in multiple orgs with different private repos, this is a
temporary solution until we make the main repo public.
…blishing

- Added authentication service and GitHub OAuth integration.
- Introduced new authentication methods and structures in the model.
- Updated the API to handle authentication during the publishing process.
- Created handlers for starting and checking the status of the OAuth flow.
- Enhanced the publish handler to validate authentication credentials.
- Updated configuration to include GitHub OAuth settings.
- Added tests for the OAuth flow and publishing with authentication.
- Updated documentation to reflect changes in the API and authentication process.
…ecks and introducing dynamic auth method determination
toby and others added 19 commits May 5, 2025 15:25
First (incomplete) cut of OpenAPI spec for registry API. To be continued in separate issues.
Refs modelcontextprotocol#28. After some thinking, this tries for a 'best of both worlds'
approach. Simple arguments can be present essentially as the original
spec recommended. As discussed, we don't specify the args for `npx` and
friends directly, so a simple package is merely...

```yaml
- name: my-api-mcp
  # ...
  packages:
    - type: npm
      name: "@c4312/my-api-mcp"
      version: "0.0.1"
      arguments:
        # Named inputs in 'arguments' generate a `--name=<value>`
        - type: named
          name: "api-key"
          is_required: true
          is_secret: true
```

However, I think we need to be able to specify runtime args in some
cases, otherwise the registry eventually mirrors all docker configs and
options which is not a great situation to be in. So, I suggest a
`runtime_args` in the same format. Say I wanted that for npm, I could:

```yaml
- name: my-api-mcp
  # ...
  packages:
    - type: npm
      name: "@c4312/my-api-mcp"
      version: "0.0.1"
      runtime_arguments:
        # Arguments with a pre-specified `value` don't get prompted for input
        - type: positional
          value: "--experimental-eventsource"
```

But Docker mounts are... complex, and ultimately we may deal with cases
that need multiple inputs in an argument like that. So for that I suggest
the third kind of "templated" input that does basic replacements and has
`properties` that follow the same general input schema:

```yaml
- name: server-filesystem
  # ...
  packages:
    - type: docker
      name: "mcp/filesystem"
      version: "0.0.1"
      runtime_arguments:
        - type: template
          description: Mount paths to access.
          required: true
          repated: true
          template: "--mount=type=bind,src={host_path},dst={container_path}"
          values:
            host_path:
              description: Path to access on the local machine
              required: true
              format: filepath
            container_path:
              description: Path to mount in the container. It should be rooted in `/project` directory.
              required: true
              format: string
              default: "/project"

      arguments:
        - type: positional
          value: "/project"
```

And then client config ends up being pretty simple, for the above case
for example a client could have this config:

```
{
  "filesystem": {
    "server": "@modelcontextprotocol/servers/src/[email protected]",
    "package": "docker",
    "settings": {
      "mount_config": [
        { "source_path": "/Users/username/Desktop", "target_path": "/project/desktop" },
        { "source_path": "/path/to/other/allowed/dir", "target_path": "//projects/other/allowed/dir,ro" },
      ]
    }
  }
}
```

Which might generate a CLI:

```
docker run \
  --mount=type=bind,src=/Users/username/Desktop,dst=/project/desktop \
  --mount=type=bind,src=/path/to/other/allowed/dir,dst=/project/other/allowed/dir,ro \
  mcp/filesystem:1.0.2 \
  /project
```

I think this does a good job of accomplishing the goals we care about:

- We get nicely defined inputs that are easy to extend, easy to process,
  and easy for clients to configure.
- Versioning is enforced as we don't require the package to specify the
  identifier to npx/uvx/etc.
- We _usually_ can break away from a dependency on a specific runtime.
  This promise is broken a bit with `runtime_arguments`, most packages
  should not need this and those that do are those inherently more
  tightly coupled to a specific runtime like Docker (e.g. they would be
  pretty uncommon for Node or Python.)
- We don't have a suple complex template system with loops and fallbacks
  and conditionals, like we would if we tried to have a single template
  string for a command.
I get this error when building the docker image after running the server
in docker compose:

  ERROR: failed to solve: error from sender: open data/diagnostic.data: permission denied

This adds a .dockeringore file to exclude everything in data except
seed.json.
@domdomegg domdomegg self-requested a review August 6, 2025 10:50
Copy link
Member

@domdomegg domdomegg left a comment

Choose a reason for hiding this comment

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

I'm not able to reproduce this problem where these files get added - are you able to provide more context about how/when this happened?

I think there have been quite a few changes since May, so it's possible this is no longer happening but I'm not sure.

@domdomegg
Copy link
Member

I sincerely apologize for the disruption. This PR was accidentally closed due to a git history rewrite mistake. The PR has been recreated as #246 with all the original content preserved.

Please continue any discussions or reviews on the new PR: #246

Again, I apologize for the inconvenience this has caused.

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.

10 participants