docs: Add nico-mcp operator guide#3498
Conversation
Signed-off-by: Kyle Felter <kfelter@nvidia.com>
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Summary by CodeRabbit
WalkthroughAdds operational documentation for deploying, configuring, securing, and debugging the NICo MCP server, with Helm chart guidance and documentation navigation updates. ChangesNICo MCP Operations
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
🔍 Container Scan Summary
Per-CVE detail lives in the per-service |
|
🌿 Preview your docs: https://nvidia-preview-pull-request-3498.docs.buildwithfern.com/infra-controller |
🔐 TruffleHog Secret Scan✅ No secrets or credentials found! Your code has been scanned for 700+ types of secrets and credentials. All clear! 🎉 🕐 Last updated: 2026-07-16 18:30:55 UTC | Commit: 67ce2b3 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 67ce2b39c8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/operations/nico-mcp.md`:
- Around line 21-23: Update docs/operations/nico-mcp.md lines 21-23 to describe
ClusterIP and port 8080 as defaults, while stating that production deployments
must retain those settings. Update helm/rest/nico-mcp/README.md line 3 to say
that, by default, the chart deploys a ClusterIP Service.
- Around line 132-144: Update the startup-settings documentation around the
table to clarify that the environment-variable fallback describes standalone
binary behavior only. State that Helm renders explicit --listen, --path, and
--shutdown-timeout arguments, so those Helm values take precedence over the
corresponding NICO_MCP_* environment variables.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: cfc20088-c31d-4501-a139-174b0856e401
📒 Files selected for processing (3)
docs/index.ymldocs/operations/nico-mcp.mdhelm/rest/nico-mcp/README.md
|
@kfelternv the PR description is too long since it winds up as the git commit message, so basically should just be the what and why. The how it was tested & how to reproduce testing should probably go into a comment after the PR description so it doesn't wind up in the git commit. |
How we verified itVerified the exact current merge result on macOS 26.5.1 arm64. The docs head was signed commit Hands-on verification built the Go 1.26.4
Supporting checks:
The optional Fern broken-link scan reported existing errors in files outside this three-file diff; it reported no broken link in the new guide or chart README. How to reproduce the verificationPrerequisites: Git, Go with toolchain download enabled, Helm 3, Node.js/npm, Python 3, curl, and Start from a clean checkout and construct the exact verified tree: git clone https://github.com/NVIDIA/infra-controller.git
cd infra-controller
git fetch origin refs/pull/3498/head:pr-3498
git checkout --detach d83e8c6296c7e932b1f639472994fc900082b0ba
git merge --no-ff --no-commit pr-3498
git write-treeThe merge must be conflict-free and Build and run the focused checks: VERIFY_ROOT="$(cd .. && pwd)/pr3498-verification"
mkdir -p "$VERIFY_ROOT/gopath/bin"
cd rest-api
GOPATH="$VERIFY_ROOT/gopath" make nico-mcp
GOPATH="$VERIFY_ROOT/gopath" go test ./mcp/... -count=1
GOPATH="$VERIFY_ROOT/gopath" go vet ./mcp/...
cd ..
helm lint helm/rest/nico-mcp \
--set global.image.repository=registry.example.com/nico \
--set global.image.tag=2.0.0 \
--set config.baseURL=http://nico-rest-api:8388 \
--set config.org=example-org \
--set config.apiName=nico
npx --yes fern-api@5.57.0 check --warnings
uvx rumdl check docs/operations/nico-mcp.md helm/rest/nico-mcp/README.md
git diff --cached --checkFor the real credential-routing path, create a standard-library receiver: cat >"$VERIFY_ROOT/receiver.py" <<'PY'
import json
import sys
from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer
class Handler(BaseHTTPRequestHandler):
def do_GET(self):
print(json.dumps({
"port": self.server.server_port,
"path": self.path,
"authorization": self.headers.get("Authorization", ""),
}), flush=True)
self.send_response(200)
self.send_header("Content-Type", "application/json")
self.end_headers()
self.wfile.write(b"[]")
def log_message(self, _format, *_args):
pass
ThreadingHTTPServer(("127.0.0.1", int(sys.argv[1])), Handler).serve_forever()
PY
python3 -u "$VERIFY_ROOT/receiver.py" 19110 >"$VERIFY_ROOT/caller.log" 2>&1 &
CALLER_PID=$!
python3 -u "$VERIFY_ROOT/receiver.py" 19111 >"$VERIFY_ROOT/configured.log" 2>&1 &
CONFIGURED_PID=$!
"$VERIFY_ROOT/gopath/bin/nico-mcp" \
--listen 127.0.0.1:19112 \
--path /mcp \
--base-url http://127.0.0.1:19111 \
--org test-org >"$VERIFY_ROOT/nico-mcp.log" 2>&1 &
MCP_PID=$!Send one mismatched and one matching call: curl --retry 10 --retry-connrefused --retry-delay 1 -sS \
-H 'Content-Type: application/json' \
-H 'Accept: application/json, text/event-stream' \
-H 'Authorization: Bearer mismatch-inherited-sentinel' \
--data '{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"nico_get_all_site","arguments":{"base_url":"http://127.0.0.1:19110","org":"test-org"}}}' \
http://127.0.0.1:19112/mcp
curl -sS \
-H 'Content-Type: application/json' \
-H 'Accept: application/json, text/event-stream' \
-H 'Authorization: Bearer matching-inherited-sentinel' \
--data '{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"nico_get_all_site","arguments":{"base_url":"http://127.0.0.1:19111/","org":"test-org"}}}' \
http://127.0.0.1:19112/mcp
test ! -s "$VERIFY_ROOT/caller.log"
rg 'matching-inherited-sentinel' "$VERIFY_ROOT/configured.log"
kill "$MCP_PID" "$CONFIGURED_PID" "$CALLER_PID"The mismatched call must return Finally, render the guide: npx --yes fern-api@5.57.0 docs dev --port 19120 &
FERN_PID=$!
curl --retry 20 --retry-connrefused --retry-delay 1 -fsS \
http://127.0.0.1:19120/infra-controller/documentation/operations-day-2/ni-co-mcp-server \
| rg 'Supported Deployment Topologies|Runtime Configuration|Security Requirements|Debug Logging'
kill "$FERN_PID"The page must return HTTP 200 and contain all four section headings. |
shayan1995
left a comment
shayan1995
left a comment
There was a problem hiding this comment.
LGTM — I cross-checked the guide against the chart templates/values and the server source (ServeFlags, FromCallConfig, the debug-header redaction in cli/pkg/client.go), and every default, env var, precedence rule, and security claim matches the code, including the subtle ones (no env var for --debug, trailing-slash tolerance on base_url, empty-token-allowed). Nice work.
A few non-blocking nits inline. One optional thought with no good anchor line: the guide doesn't mention that the chart's probes are TCP-only and the endpoint only answers JSON-RPC POSTs (both noted in the deployment template comments) — a one-liner under the production topology section could save an operator wiring an HTTP-health-checking gateway some confusion.
4 participants
Adds a Day 2 NICo MCP operator guide, links it from the Helm chart, and includes it in documentation navigation. The guide explains supported deployment topologies, configuration precedence, and the credential-to-REST-destination behavior implemented by #3499 so operators can configure the service without forwarding credentials to an unintended destination.
Related issues
Resolves #2801.
Type of Change
Breaking Changes
Testing
Additional Notes
Detailed verification and reproduction steps are in the existing top-level verification comment.