Conversation
Replace the CI workflow with staging and production deployment workflows that build to GHCR and deploy to Kubernetes via Helm.
WalkthroughThe PR removes the existing CI workflow (.github/workflows/ci.yml), adds staging and production GitHub Actions workflows (.github/workflows/staging.yml and production.yml) that build/push images to GHCR and deploy via Helm to multiple regions, and adds a Helm chart under deploy/mcp-for-docs (Chart.yaml, values, .helmignore, environment values, and templates including Deployment, HPA, Service, PDB, Gateway, ReferenceGrant, and HTTPRoute). The Dockerfile is modified to read OPENAI_API_KEY via a Docker secret mount during build steps instead of exposing it as a build ARG/ENV. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In @.github/workflows/production.yml:
- Around line 32-34: The build-args block currently includes surrounding double
quotes which will make Docker receive the literal quotes in the key/value;
update the build-args entry so the argument is passed without surrounding quotes
(i.e. use OPENAI_API_KEY=${{ secrets.OPENAI_API_KEY }} instead of
"OPENAI_API_KEY=${{ secrets.OPENAI_API_KEY }}") under the build-args key so
Docker gets the correct arg name/value; ensure the change is applied where the
workflow defines build-args.
- Around line 26-34: The workflow currently passes OPENAI_API_KEY via build-args
in the "Build and push" docker/build-push-action step which leaks the secret
into image metadata; change the action config to use the action's secrets mount
instead (set secrets: OPENAI_API_KEY=${{ secrets.OPENAI_API_KEY }} and remove
the build-args entry), and update the Dockerfile's init-vector-store build step
to use BuildKit secret mounts (replace any RUN that reads the build-arg with RUN
--mount=type=secret,id=OPENAI_API_KEY and read the key from
/run/secrets/OPENAI_API_KEY). Ensure BuildKit is enabled for the build (so
--mount=type=secret works) when running docker/build-push-action.
In @.github/workflows/staging.yml:
- Around line 32-34: The build-args entry is quoted which injects literal double
quotes into the Docker build arg; update the workflow so the build-args block
uses unquoted key/value (remove the surrounding quotes around
"OPENAI_API_KEY=${{ secrets.OPENAI_API_KEY }}") so the build arg name
OPENAI_API_KEY and value from secrets.OPENAI_API_KEY are passed correctly; keep
the tags line unchanged (ghcr.io/appwrite/mcp-for-docs:${{ env.TAG }}).
In `@deploy/mcp-for-docs/templates/gateway.yaml`:
- Around line 24-28: The Gateway in namespace mcp-for-docs uses
tls.certificateRefs to reference the Secret named "gateway" in namespace
"traefik", which requires a ReferenceGrant in the "traefik" namespace to
authorize cross-namespace Secret access; add a ReferenceGrant resource (in the
traefik namespace) that grants from kind: Gateway, namespace: mcp-for-docs (the
Gateway that uses tls.certificateRefs) to allow access to kind: Secret in
namespace: traefik (specifying the "gateway" secret or allowing Secrets) so
Traefik can use the TLS certificate.
In `@deploy/mcp-for-docs/templates/mcp-for-docs.yaml`:
- Around line 20-23: The nodeSelector templating uses {{ toYaml . }} which can
produce malformed YAML when the map has multiple keys because it doesn't indent
subsequent lines; update the template so the rendered map is properly indented
by replacing the raw toYaml use with nindent (e.g., use toYaml . | nindent 8 or
the appropriate indent level) inside the nodeSelector block so all lines of the
toYaml output are indented correctly; target the nodeSelector block in the
mcp-for-docs.yaml template and ensure the nindent value matches the current
block indentation.
- Line 6: The Deployment template currently sets spec.replicas: {{
.Values.minReplicas }} which conflicts with an HPA and causes Helm to reset
scaled counts on every upgrade; remove the replicas field entirely or guard it
so it only applies on first install by wrapping it in a Helm lookup check for
the "apps/v1" Deployment named "mcp-for-docs" in the release namespace (i.e.,
only emit replicas: {{ .Values.minReplicas }} when lookup returns nil/not
found), keeping the .Values.minReplicas symbol as the default initial count.
🧹 Nitpick comments (5)
deploy/mcp-for-docs/values.yaml (1)
1-8: Default values look reasonable.Consider documenting the required values (
domain,version,imagePullSecret) that must be provided at install time — either via comments in this file or aREADME.md— to help anyone runninghelm templateorhelm installoutside of CI.deploy/mcp-for-docs/templates/gateway.yaml (2)
1-29: Inconsistent indentation between Gateway and HTTPRoute resources.The Gateway block uses 4-space indentation while the HTTPRoute block (lines 33–48) uses 2-space indentation. This won't break rendering but is inconsistent within the same template file.
Also, the
$domainsvariable assignment is duplicated (lines 8 and 38). You could hoist it once at the top of the file using a{{- $domains := ... }}and reuse it in both resources.
43-48: HTTPRoute applies to all listeners including plain HTTP.The
parentRefsdon't specify asectionName, so this route matches both the HTTP (port 8000) and HTTPS (port 8443) listeners. If you intend HTTPS-only traffic (with HTTP→HTTPS redirect handled elsewhere), consider scoping the route to only the HTTPS listeners, or confirm that Traefik handles the redirect at the controller level..github/workflows/staging.yml (1)
1-80: Consider extracting a reusable workflow to reduce duplication.The staging and production workflows are nearly identical (~95% shared logic), differing only in trigger, TAG computation, cluster suffix (
-stgvs-prod), and values path (staging/vsproduction/). A reusable workflow with inputs for environment, cluster suffix, and values path would eliminate this duplication and make future changes less error-prone.deploy/mcp-for-docs/templates/mcp-for-docs.yaml (1)
7-11: Over-indentedstrategyblock.
typeandrollingUpdateare indented 6 spaces understrategywhile the rest of the file uses 2-space increments. This works but is inconsistent.Proposed fix
strategy: - type: RollingUpdate - rollingUpdate: - maxSurge: 1 - maxUnavailable: 1 + type: RollingUpdate + rollingUpdate: + maxSurge: 1 + maxUnavailable: 1
ChiragAgg5k
force-pushed
the
chore/add-helm-chart-and-deploy-workflows
branch
from
4284528 to
69f49f7
Compare
ChiragAgg5k
force-pushed
the
chore/add-helm-chart-and-deploy-workflows
branch
from
69f49f7 to
842be6e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@deploy/mcp-for-docs/templates/mcp-for-docs.yaml`:
- Around line 40-45: The Deployment references a Kubernetes Secret named
"mcp-for-docs" (key "openai-api-key") as the source for the environment variable
OPENAI_API_KEY, but the chart does not create this Secret so pods will fail with
CreateContainerConfigError if it is missing; update the chart README and
templates to document this prerequisite and ensure the secret is provisioned
out-of-band (for example add instructions for creating the "mcp-for-docs" secret
or using sealed-secrets/CI pipeline to create it prior to deployment), and
optionally add a validation hook or pre-install note check that fails the
release with a clear message if "mcp-for-docs" is not present.
🧹 Nitpick comments (2)
deploy/mcp-for-docs/templates/gateway.yaml (2)
10-16: Consider adding an HTTP-to-HTTPS redirect on the HTTP listener.The HTTP listener on port 8000 will serve unencrypted traffic. If this isn't handled upstream (e.g., by a load balancer or Traefik entrypoint configuration), clients may inadvertently use plain HTTP in production. If Traefik or the LB already enforces the redirect, this is fine as-is.
1-1: Consider upgrading Gateway API resources fromv1beta1tov1.Gateway API
v1(stable channel) has been available since the v1.0.0 release (October 31, 2023). BothGatewayandHTTPRoutewere promoted tov1at that time, andReferenceGrantwas promoted tov1in v1.2.0. If your cluster's Gateway API CRDs supportv1, upgrading avoids future deprecation.Also applies to: 33-33, 50-50
1 participant
Summary
deploy/mcp-for-docs/) for Kubernetes deployment with Deployment, HPA, Service, PDB, and Traefik Gatewaymain) and production workflow (triggers on release publish)ci.ymlworkflow, replaced by the new staging/production pipelinesfra1region (staging and production)Test plan
helm templateOPENAI_API_KEY,DIGITALOCEAN_ACCESS_TOKEN,GHCR_USERNAME,GHCR_TOKEN) are configuredmainSummary by CodeRabbit
Chores
New Features