- Notifications
You must be signed in to change notification settings - Fork962
fix: use ENTRYPOINT and CMD for proper argument handling#454
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
fix: use ENTRYPOINT and CMD for proper argument handling#454
Uh oh!
There was an error while loading.Please reload this page.
Conversation
- Change from CMD to ENTRYPOINT + CMD pattern for better Docker practices- ENTRYPOINT sets the executable that always runs- CMD provides default arguments that can be overridden- This allows container runtimes to properly append additional arguments- Fixes issues with argument passing in container orchestration toolsBefore: CMD ["./github-mcp-server", "stdio"]After: ENTRYPOINT ["./github-mcp-server"] + CMD ["stdio"]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Pull Request Overview
Switch from a singleCMD to usingENTRYPOINT plus a defaultCMD in the Dockerfile to enable proper argument appending.
- Introduce
ENTRYPOINTfor the server binary - Keep default
stdioargument inCMD - Update comments to reflect the change
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
This is great, would you mind updating the README.me so that any suggested commands are correct for this change? |
@SamMorrowDrums can you explain a little more? I'm not sure anything would need to change from the README |
I haven't double checked, if we don't suggest anything that would need to change that's no problem. |
@SamMorrowDrums right! the idea is that nothing changes. But this enables for orchestration platforms to actually be able to pass parameters in an easier way. This wasn't part of the README before, so there was nothing to change. |
The kind of things I was think were additional arguments in the docs like Maybe we don't explicitly spell any of them out. I've definitely provided such commands via issues, but that's all I wanted to ensure. I think fortunately the docs are lacking enough that you don't have anything to update. |
Problem
The current Dockerfile uses
CMD ["./github-mcp-server", "stdio"]which causes issues when container orchestration tools try to append additional arguments. This results in the entire CMD being replaced rather than arguments being appended.Root Cause
When using only
CMD, container runtimes replace the entire command when additional arguments are provided. This breaks argument passing in tools like ToolHive, Kubernetes, and other container orchestrators.Solution
Implement Docker best practices by using:
ENTRYPOINTfor the executable that should always runCMDfor default arguments that can be overridden/appended toChanges
Before:
After:
Benefits
stdioargument instead of replacing the entire commandTesting
This change maintains backward compatibility:
docker run github-mcp-server→ runs./github-mcp-server stdio(same as before)docker run github-mcp-server --toolsets all→ runs./github-mcp-server --toolsets all(new capability)Related Issues
This fixes argument passing issues reported in container orchestration tools where additional arguments couldn't be properly passed to the MCP server.