fix: quote secret in kubectl create secret commands#8
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThis pull request: quotes GitHub Actions secret references (docker-username, docker-password, openai-api-key) in production and staging workflows and removes the push-on-main trigger from the staging workflow; changes startup/liveness/readiness probes from HTTP GET to TCP socket on port 1234; adds Service-level sessionAffinity and sessionAffinityTimeoutSeconds (values.yaml); upgrades Gateway and HTTPRoute apiVersion from gateway.networking.k8s.io/v1beta1 to v1; adds an early-return guard in scripts/init-vector-store.ts for empty chunks; changes default MCPServer responseMode in src/index.ts from "stream" to "batch". No public API signatures were altered. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
deploy/mcp-for-docs/templates/mcp-for-docs.yaml (1)
48-67: Switching all probes totcpSocketreduces health-check fidelity.TCP socket probes only confirm the port is accepting connections—they won't detect application-level issues (e.g., the process is listening but stuck or misconfigured). This is fine for
startupProbe, but forlivenessProbeandreadinessProbe, an HTTP endpoint (or gRPC health check) gives stronger guarantees that the service is actually functional.If the application doesn't expose an HTTP health endpoint (e.g., it's a pure SSE/WebSocket/MCP server), then
tcpSocketis a pragmatic choice—just flagging the trade-off.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/staging.yml (1)
59-64: Other secrets in shell commands are also unquoted.The
--docker-usernameand--docker-passwordvalues on lines 61–62 are equally susceptible to the same trailing whitespace/newline issue that broke the OpenAI secret. Consider quoting them for consistency and resilience.Proposed fix
kubectl -n mcp-for-docs create secret docker-registry ghcr \ --docker-server=ghcr.io \ - --docker-username=${{ secrets.GHCR_USERNAME }} \ - --docker-password=${{ secrets.GHCR_TOKEN }} \ + --docker-username="${{ secrets.GHCR_USERNAME }}" \ + --docker-password="${{ secrets.GHCR_TOKEN }}" \ --docker-email=ci@appwrite.io \ --dry-run=client -o yaml | kubectl apply -f -
There was a problem hiding this comment.
🧹 Nitpick comments (1)
deploy/mcp-for-docs/templates/mcp-for-docs.yaml (1)
48-67: TCP probes are a weaker health signal than HTTP probes — consider adding a dedicated health endpoint long-term.TCP socket probes only confirm the port is accepting connections, not that the application can serve requests. This is particularly relevant for
readinessProbe, where a pod could be listening but unable to process MCP requests (e.g., vector store not loaded). If the MCP framework or your app can expose a lightweight HTTP health check (even on a separate port), that would be a stronger readiness signal.Acceptable for now given the protocol constraints, but worth tracking as a follow-up.
d091243 to
e7aa0de
Compare
fix: quote secret in kubectl create secret commands
Merge pull request #8 from appwrite/fix/quote-secret-in-workflows
Summary
${{ secrets.OPENAI_API_KEY }}in thekubectl create secretcommands in both staging and production workflows\line continuation, causing--dry-run=clientto be interpreted as a standalone commandTest plan
Summary by CodeRabbit
Bug Fixes
Chores