Feat: Initial Setup & Kubernetes Infrastructure for Mariana Deep (Internal Docs) - #3582
Merged
Conversation
Introduce a containerized local environment for the internal "The Mariana" documentation using BookStack. Adds docker-compose.yml (BookStack + MariaDB), .env.example with recommended defaults (APP_URL, DB creds, APP_PUBLIC=false), .gitignore to protect .env and persisted data (config/, db_data/), and a README with setup, access, security guidance, IA conventions, and persistence notes. Intended for local development on port 6875; remember to copy .env.example -> .env and update default credentials and secrets before use.
Introduce a new Helm chart for AirQo internal docs (The Mariana). Adds Chart.yaml, template helpers, deployments for app (BookStack) and DB (MariaDB), services, and PVCs. Includes environment-specific values files for production and staging with image defaults (linuxserver/bookstack, linuxserver/mariadb), resource and autoscaling settings, nodeSelector/affinity, unique NodePorts, configmap references, DB env vars, and 5Gi persistent storage for both app and DB.
Rebrand internal docs from "The Mariana" to "Mariana Deep" across .env.example, README, docker-compose.yml, and Chart.yaml. Standardize MariaDB usage by switching to the official mariadb:10.6 image, changing the DB data mount to /var/lib/mysql, switching to a named Docker volume (bookstack_db_data) and adding the volume block in docker-compose. Mirror the image/tag change in Helm values and adjust the k8s deployment volume mount. Also add db_data_v2/ to .gitignore.
Delete k8s/values-stage.yaml and update src/internal-docs/README.md to remove staging references and the staging Helm command. The README now states Helm charts deploy to the production cluster and retains the production deploy command (-f k8s/values-prod.yaml -n production). This cleans up stale staging configuration and documentation.
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 33762647 | Triggered | Laravel APP_KEY | 0fe4bcd | src/internal-docs/.env.example | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
Contributor
|
Wondering what really moved? Review this PR in Change Stack to inspect semantic changes, definitions, and references. 📝 WalkthroughWalkthroughThis PR introduces a complete BookStack-based internal documentation platform for "Mariana Deep." The changes include local development setup via Docker Compose, comprehensive documentation, and production-grade Kubernetes/Helm infrastructure supporting scalable, persistent deployments with integrated MariaDB database. ChangesBookStack Internal Documentation Platform
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (5)
src/internal-docs/.gitignore (1)
7-9: 💤 Low valueClarify db_data directory ignore patterns.
The docker-compose.yml uses a named Docker volume (
bookstack_db_data) rather than bind-mounting todb_data/ordb_data_v2/. These .gitignore entries won't match anything created by the current setup. If these are legacy patterns from an earlier implementation, consider removing them. If they're intentional for future bind-mount scenarios, that's fine—just noting the inconsistency.🤖 Prompt for 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. In `@src/internal-docs/.gitignore` around lines 7 - 9, The .gitignore entries db_data/ and db_data_v2/ appear to be legacy bind‑mount patterns that don't match the current Docker setup which uses the named volume bookstack_db_data in docker-compose.yml; either remove the unused db_data/ and db_data_v2/ lines if they’re legacy, or update the comment to explicitly state they’re for optional future bind‑mount scenarios (or add bookstack_db_data to docs if you intend to ignore a host directory created for that volume). Locate the .gitignore entries (db_data and db_data_v2) and either delete them or revise the comment to reflect the current named volume (bookstack_db_data) and intended usage so the ignore file matches the actual docker-compose.yml configuration.src/internal-docs/k8s/values-prod.yaml (1)
54-63: 💤 Low valueRemove redundant node affinity preference.
The
nodeSelectoron line 50 already requiresrole: high-memnodes (hard constraint), making thepreferredDuringSchedulingIgnoredDuringExecutionaffinity (soft preference) for the same label redundant.♻️ Simplify by removing redundant affinity
nodeSelector: role: high-mem tolerations: [] - -affinity: - nodeAffinity: - preferredDuringSchedulingIgnoredDuringExecution: - - preference: - matchExpressions: - - key: role - operator: In - values: - - high-mem - weight: 1🤖 Prompt for 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. In `@src/internal-docs/k8s/values-prod.yaml` around lines 54 - 63, Remove the redundant soft node affinity: the nodeAffinity block using preferredDuringSchedulingIgnoredDuringExecution that prefers matchExpressions key "role" operator "In" values ["high-mem"] duplicates the hard constraint already enforced by nodeSelector (role: high-mem); delete the entire preferredDuringSchedulingIgnoredDuringExecution entry (or the enclosing nodeAffinity block if nothing else remains) so only the nodeSelector enforces the node role.src/internal-docs/k8s/templates/deployment-db.yaml (1)
9-9: ⚡ Quick winDocument why database replicas are hardcoded to 1.
The single replica is correct for a standalone MariaDB instance using ReadWriteOnce storage, but this constraint should be documented in comments to prevent accidental scaling attempts.
📝 Suggested documentation addition
spec: + # Replica count is fixed at 1 because MariaDB is not configured for replication + # and the PVC uses ReadWriteOnce access mode (single-node attachment only). + # For HA, consider StatefulSet with replication or a managed database service. replicas: 1 selector:🤖 Prompt for 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. In `@src/internal-docs/k8s/templates/deployment-db.yaml` at line 9, The deployment template hardcodes replicas: 1 in deployment-db.yaml without explanation; add a concise comment above the replicas: 1 line explaining this is intentional because the MariaDB StatefulSet/Deployment uses ReadWriteOnce (RWO) persistent volume which only supports a single writer, so scaling replicas would break storage access, and mention that HA requires a clustered DB or shared storage; update the comment near the replicas: 1 entry to state the RWO constraint, that this is for standalone MariaDB, and any instructions for how to proceed if someone needs HA (e.g., use Galera/cluster or shared storage).src/internal-docs/k8s/templates/deployment-app.yaml (1)
22-42: ⚖️ Poor tradeoffAdd liveness and readiness probes for production reliability.
The deployment lacks health checks, which prevents Kubernetes from detecting and recovering from application failures. This can lead to traffic being routed to unhealthy pods.
🏥 Suggested health check configuration
volumeMounts: - name: bookstack-config mountPath: /config + livenessProbe: + httpGet: + path: /status + port: http + initialDelaySeconds: 30 + periodSeconds: 10 + timeoutSeconds: 5 + failureThreshold: 3 + readinessProbe: + httpGet: + path: /status + port: http + initialDelaySeconds: 10 + periodSeconds: 5 + timeoutSeconds: 3 + failureThreshold: 2 resources: {{- toYaml .Values.resources | nindent 12 }}Note: Verify that BookStack exposes a
/statusendpoint or adjust the path accordingly.🤖 Prompt for 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. In `@src/internal-docs/k8s/templates/deployment-app.yaml` around lines 22 - 42, Add HTTP livenessProbe and readinessProbe entries to the container spec for {{ .Values.app.label }} so Kubernetes can detect and recover unhealthy pods: under the container with image and ports, add livenessProbe and readinessProbe using httpGet against path (e.g. /status) and port {{ .Values.service.targetPort }}, and set sensible timeouts and thresholds (initialDelaySeconds, periodSeconds, timeoutSeconds, failureThreshold) — make these values configurable via .Values (e.g. .Values.probes.liveness/readiness) so they can be tuned per-environment and default to conservative production settings.src/internal-docs/k8s/templates/service-app.yaml (1)
8-19: 💤 Low valueClarify how NodePort maps to the production URL.
The service uses
NodePort(31197) but the README states production is accessed athttps://platform.airqo.net/mariana. This suggests external infrastructure (reverse proxy or load balancer) routes traffic to the NodePort, but this isn't documented in the chart.Consider adding a comment or documentation explaining the routing path from the external URL to the NodePort service.
🤖 Prompt for 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. In `@src/internal-docs/k8s/templates/service-app.yaml` around lines 8 - 19, The template exposes a NodePort via .Values.service.type/.nodePort (e.g., NodePort 31197) but the chart lacks documentation tying that to the public URL (https://platform.airqo.net/mariana); add a brief comment in service-app.yaml and/or the chart README explaining the routing path: external reverse proxy or load balancer (Ingress/NGINX/HAProxy or cloud LB) listens on the public hostname/path and forwards traffic to the NodePort (31197) on the worker nodes (mapping from .Values.service.port/.targetPort to .Values.service.nodePort), or alternatively recommend switching to a LoadBalancer/Ingress and show the required annotations/config for that mapping; reference the symbols .Values.service.type, .Values.service.nodePort, .Values.service.port, .Values.service.targetPort and .Values.app.label so reviewers can find and update the template or docs.
🤖 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 `@src/internal-docs/.env.example`:
- Line 21: Remove the hardcoded value for APP_KEY in the example env file and
replace it with a clear placeholder that forces generating a unique key (e.g., a
descriptive token like "GENERATE_YOUR_OWN_KEY_HERE" or "<base64_key_here>") so
contributors cannot accidentally copy a real key; update the APP_KEY entry in
src/internal-docs/.env.example (look for the APP_KEY line) to use that
placeholder and add a short inline comment prompting generation of a new key
before use.
In `@src/internal-docs/docker-compose.yml`:
- Around line 37-38: Remove the unsupported PUID and PGID environment variables
from the MariaDB service definition (the block that uses image "mariadb:10.6");
delete the lines setting PUID and PGID so the compose file only contains valid
MariaDB env vars (e.g., MYSQL_ROOT_PASSWORD, MYSQL_DATABASE, etc.) and does not
include LinuxServer.io-specific keys PUID or PGID which the official image
ignores.
- Around line 16-20: Remove the redundant DB_USERNAME and DB_PASSWORD lines and
consolidate on the LinuxServer-expected variables DB_USER and DB_PASS; keep
DB_DATABASE. Specifically, delete the environment entries for DB_USERNAME and
DB_PASSWORD from the service block and ensure DB_USER and DB_PASS retain their
current fallbacks to ${DB_USER:-bookstack} and
${DB_PASSWORD:-bookstack_secure_password} so the container reads the expected
variable names.
In `@src/internal-docs/k8s/templates/deployment-app.yaml`:
- Line 24: The deployment template uses image: "{{ .Values.image.repository
}}:{{ .Values.image.tag }}"; ensure production values do not set image.tag to
"latest" — update values-prod.yaml to pin an explicit version or digest (e.g.,
"24.05.2" or "sha256:...") so .Values.image.tag (or switch to
.Values.image.digest) provides an immutable image reference for the Deployment;
adjust values-prod.yaml accordingly and, if necessary, update Helm template
usage to prefer a digest field when present.
- Around line 36-37: The Deployment references a ConfigMap via
.Values.app.configmap (used in
src/internal-docs/k8s/templates/deployment-app.yaml) but that ConfigMap (e.g.
env-internal-docs-prod) is missing; create/provision a ConfigMap with the name
matching .Values.app.configmap in the target namespace and populate it with all
required BookStack environment keys (DB_* credentials, APP_KEY, etc.) so the
envFrom in the Pod spec can resolve them at startup.
In `@src/internal-docs/k8s/templates/deployment-db.yaml`:
- Around line 22-37: Add Kubernetes livenessProbe and readinessProbe to the
MariaDB container (the container defined by name: {{ .Values.dbApp.label }} and
port {{ .Values.dbService.targetPort }}). For readinessProbe use a tcpSocket
(port: {{ .Values.dbService.targetPort }}) with a short initialDelaySeconds
(e.g. 10–30), periodSeconds (e.g. 10) and timeoutSeconds (e.g. 5); for
livenessProbe use either tcpSocket or an exec that runs a lightweight mysql
client command to test responsiveness, with a larger initialDelaySeconds (e.g.
30), failureThreshold (e.g. 3) and periodSeconds (e.g. 10). Place these probes
under the container spec alongside ports, envFrom and volumeMounts so Kubernetes
can detect unhealthy/unready DB pods and restart or avoid routing traffic
accordingly.
In `@src/internal-docs/k8s/templates/pvc.yaml`:
- Around line 13-23: The PVC spec for the PersistentVolumeClaim named via {{
.Values.dbApp.name }}-pvc currently hardcodes resources.requests.storage to 5Gi
and omits storageClassName; change the template to use configurable values
(e.g., .Values.dbApp.storage and .Values.dbApp.storageClassName), set a larger
sensible default than 5Gi in values (for production), and add storageClassName
under spec so the PVC uses an explicit class rather than the cluster default;
ensure values.yaml documents the new keys and validation for production
overrides.
In `@src/internal-docs/k8s/values-prod.yaml`:
- Around line 44-47: The autoscaling settings in values-prod.yaml have no effect
because there is no HorizontalPodAutoscaler manifest to consume them; add a new
Helm template (e.g., create src/internal-docs/k8s/templates/hpa.yaml) that
conditionally renders an autoscaling/v2 HorizontalPodAutoscaler when
.Values.autoscaling is present, using .Values.app.name for metadata.name and
spec.scaleTargetRef.name, and wiring .Values.autoscaling.minReplicas,
.autoscaling.maxReplicas and .autoscaling.targetCPUUtilizationPercentage into
spec.minReplicas, spec.maxReplicas and the CPU metric averageUtilization
respectively; ensure the template is wrapped in the conditional (if
.Values.autoscaling) so it only deploys when autoscaling is configured.
In `@src/internal-docs/README.md`:
- Line 98: Update the persistence description to reflect that the database uses
a named Docker volume rather than a bind-mounted ./db_data: change the sentence
referencing "the git-ignored ./config and ./db_data folders" to state that
./config is a bind mount while the database is persisted to the named Docker
volume bookstack_db_data (and that in Kubernetes PVCs are used for pod
restarts); remove or replace any mention of a local ./db_data bind mount and
keep the note about PVCs for Kubernetes.
- Around line 35-37: Update the "Access the Application" section in
src/internal-docs/README.md (the lines that show "Open your browser and navigate
to `http://localhost:6875`" and the default credentials) to add a note telling
users to wait ~20s after running `docker-compose up -d` for database migrations
to complete before accessing the app, and optionally suggest how to verify
progress (e.g., check docker-compose logs or wait for migrations to finish) so
users don't hit migration-related errors when loading the site.
---
Nitpick comments:
In `@src/internal-docs/.gitignore`:
- Around line 7-9: The .gitignore entries db_data/ and db_data_v2/ appear to be
legacy bind‑mount patterns that don't match the current Docker setup which uses
the named volume bookstack_db_data in docker-compose.yml; either remove the
unused db_data/ and db_data_v2/ lines if they’re legacy, or update the comment
to explicitly state they’re for optional future bind‑mount scenarios (or add
bookstack_db_data to docs if you intend to ignore a host directory created for
that volume). Locate the .gitignore entries (db_data and db_data_v2) and either
delete them or revise the comment to reflect the current named volume
(bookstack_db_data) and intended usage so the ignore file matches the actual
docker-compose.yml configuration.
In `@src/internal-docs/k8s/templates/deployment-app.yaml`:
- Around line 22-42: Add HTTP livenessProbe and readinessProbe entries to the
container spec for {{ .Values.app.label }} so Kubernetes can detect and recover
unhealthy pods: under the container with image and ports, add livenessProbe and
readinessProbe using httpGet against path (e.g. /status) and port {{
.Values.service.targetPort }}, and set sensible timeouts and thresholds
(initialDelaySeconds, periodSeconds, timeoutSeconds, failureThreshold) — make
these values configurable via .Values (e.g. .Values.probes.liveness/readiness)
so they can be tuned per-environment and default to conservative production
settings.
In `@src/internal-docs/k8s/templates/deployment-db.yaml`:
- Line 9: The deployment template hardcodes replicas: 1 in deployment-db.yaml
without explanation; add a concise comment above the replicas: 1 line explaining
this is intentional because the MariaDB StatefulSet/Deployment uses
ReadWriteOnce (RWO) persistent volume which only supports a single writer, so
scaling replicas would break storage access, and mention that HA requires a
clustered DB or shared storage; update the comment near the replicas: 1 entry to
state the RWO constraint, that this is for standalone MariaDB, and any
instructions for how to proceed if someone needs HA (e.g., use Galera/cluster or
shared storage).
In `@src/internal-docs/k8s/templates/service-app.yaml`:
- Around line 8-19: The template exposes a NodePort via
.Values.service.type/.nodePort (e.g., NodePort 31197) but the chart lacks
documentation tying that to the public URL (https://platform.airqo.net/mariana);
add a brief comment in service-app.yaml and/or the chart README explaining the
routing path: external reverse proxy or load balancer (Ingress/NGINX/HAProxy or
cloud LB) listens on the public hostname/path and forwards traffic to the
NodePort (31197) on the worker nodes (mapping from
.Values.service.port/.targetPort to .Values.service.nodePort), or alternatively
recommend switching to a LoadBalancer/Ingress and show the required
annotations/config for that mapping; reference the symbols .Values.service.type,
.Values.service.nodePort, .Values.service.port, .Values.service.targetPort and
.Values.app.label so reviewers can find and update the template or docs.
In `@src/internal-docs/k8s/values-prod.yaml`:
- Around line 54-63: Remove the redundant soft node affinity: the nodeAffinity
block using preferredDuringSchedulingIgnoredDuringExecution that prefers
matchExpressions key "role" operator "In" values ["high-mem"] duplicates the
hard constraint already enforced by nodeSelector (role: high-mem); delete the
entire preferredDuringSchedulingIgnoredDuringExecution entry (or the enclosing
nodeAffinity block if nothing else remains) so only the nodeSelector enforces
the node role.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 88dbb812-b651-41ad-a739-fdd7131bd3b6
📒 Files selected for processing (12)
src/internal-docs/.env.examplesrc/internal-docs/.gitignoresrc/internal-docs/README.mdsrc/internal-docs/docker-compose.ymlsrc/internal-docs/k8s/Chart.yamlsrc/internal-docs/k8s/templates/_helpers.tplsrc/internal-docs/k8s/templates/deployment-app.yamlsrc/internal-docs/k8s/templates/deployment-db.yamlsrc/internal-docs/k8s/templates/pvc.yamlsrc/internal-docs/k8s/templates/service-app.yamlsrc/internal-docs/k8s/templates/service-db.yamlsrc/internal-docs/k8s/values-prod.yaml
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds an initial “internal-docs” module that provides (1) a local Docker Compose environment and (2) a production-targeted Helm chart to deploy Mariana Deep (BookStack + MariaDB) into Kubernetes.
Changes:
- Added local developer/test environment for BookStack using Docker Compose plus an
.env.exampleand.gitignore. - Introduced a Helm chart under
src/internal-docs/k8s/with production values, deployments, services, PVCs, HPA, and a ConfigMap. - Added onboarding/usage documentation for local usage, content creation, and production deployment.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/internal-docs/README.md | Onboarding docs for local usage, content authoring, and prod deployment commands. |
| src/internal-docs/docker-compose.yml | Local BookStack + MariaDB compose stack with volumes/networking. |
| src/internal-docs/.env.example | Example environment variable template for local setup. |
| src/internal-docs/.gitignore | Ignores local runtime state (.env, config/, etc.). |
| src/internal-docs/k8s/Chart.yaml | Helm chart metadata for internal-docs deployment. |
| src/internal-docs/k8s/values-prod.yaml | Production values for BookStack/MariaDB images, resources, service, persistence, and env vars. |
| src/internal-docs/k8s/templates/_helpers.tpl | Helm helper templates for naming/labels. |
| src/internal-docs/k8s/templates/configmap.yaml | ConfigMap template providing BookStack + DB environment variables. |
| src/internal-docs/k8s/templates/deployment-app.yaml | Kubernetes Deployment for the BookStack app container with PVC mount. |
| src/internal-docs/k8s/templates/deployment-db.yaml | Kubernetes Deployment for MariaDB with PVC mount and probes. |
| src/internal-docs/k8s/templates/service-app.yaml | Kubernetes Service for BookStack (supports NodePort). |
| src/internal-docs/k8s/templates/service-db.yaml | Kubernetes Service for MariaDB. |
| src/internal-docs/k8s/templates/pvc.yaml | PVCs for app config/uploads and MariaDB data. |
| src/internal-docs/k8s/templates/hpa.yaml | HPA for the BookStack app deployment (CPU-based autoscaling). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+10
to
+12
| APP_NAME: {{ .Values.env.APP_NAME | quote }} | ||
| APP_KEY: {{ .Values.env.APP_KEY | quote }} | ||
| APP_PUBLIC: {{ .Values.env.APP_PUBLIC | quote }} |
Comment on lines
+29
to
+33
| 2. **Start the Containers** | ||
| Use Docker Compose to build and start the environment in the background: | ||
| ```bash | ||
| docker-compose up -d | ||
| ``` |
| command: | ||
| - sh | ||
| - -c | ||
| - mysqladmin ping -h localhost |
| command: | ||
| - sh | ||
| - -c | ||
| - mysql -h localhost -e 'SELECT 1' |
Comment on lines
+15
to
+17
| DB_USER: {{ .Values.env.DB_USER | quote }} | ||
| DB_PASS: {{ .Values.env.DB_PASSWORD | quote }} | ||
| DB_DATABASE: {{ .Values.env.DB_DATABASE | quote }} |
Comment on lines
+20
to
+23
| MYSQL_USER: {{ .Values.env.DB_USER | quote }} | ||
| MYSQL_PASSWORD: {{ .Values.env.DB_PASSWORD | quote }} | ||
| MYSQL_DATABASE: {{ .Values.env.DB_DATABASE | quote }} | ||
| MYSQL_ROOT_PASSWORD: {{ .Values.env.MYSQL_ROOT_PASSWORD | quote }} |
Comment on lines
+3
to
+6
| # ------------------------------------------------------------------------- | ||
| # Copy this file to `.env` before running `docker-compose up -d`. | ||
| # Do not commit the `.env` file to version control. | ||
| # ------------------------------------------------------------------------- |
Comment on lines
+1
to
+3
| apiVersion: v2 | ||
| appVersion: "1.0.0" | ||
| description: AirQo Internal Docs (Mariana Deep) Helm Chart |
Introduce a Kubernetes Secret for sensitive BookStack environment variables and stop storing secrets in the ConfigMap. Added templates/secret.yaml and removed APP_KEY, DB_PASS, MYSQL_PASSWORD and MYSQL_ROOT_PASSWORD from the ConfigMap; deployments for app and db now load secrets via secretRef. Update MariaDB readiness/liveness checks to use 127.0.0.1 and authenticate with MYSQL_ROOT_PASSWORD. Bumped Helm Chart appVersion and updated docs/.env example to use the newer `docker compose up -d` command. This improves security by requiring secret injection via CI/CD or cluster secrets.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/internal-docs/k8s/templates/deployment-app.yaml (1)
22-42:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd liveness and readiness probes for production availability.
The container lacks health probes, which means Kubernetes cannot detect unhealthy pods or restart hung containers. Without a readiness probe, traffic will be sent to pods before BookStack is ready (during startup migrations). Without a liveness probe, crashed or deadlocked processes won't be auto-restarted.
🏥 Proposed fix to add HTTP health probes
volumeMounts: - name: bookstack-config mountPath: /config + livenessProbe: + httpGet: + path: /status + port: http + initialDelaySeconds: 60 + periodSeconds: 10 + timeoutSeconds: 5 + failureThreshold: 3 + readinessProbe: + httpGet: + path: /status + port: http + initialDelaySeconds: 30 + periodSeconds: 5 + timeoutSeconds: 3 + failureThreshold: 3 resources:Note: Verify that BookStack exposes
/statusor use/loginas the probe path if/statusis not available.🤖 Prompt for 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. In `@src/internal-docs/k8s/templates/deployment-app.yaml` around lines 22 - 42, Add HTTP liveness and readiness probes to the container spec for production availability: inside the container block for {{ .Values.app.label }} in deployment-app.yaml, add a readinessProbe and livenessProbe that use httpGet on the app path (prefer /status, fall back to /login if /status isn't available) and set sensible timings (e.g., initialDelaySeconds, periodSeconds, timeoutSeconds, successThreshold/failureThreshold) so Kubernetes waits for startup migrations and restarts hung processes; ensure the probes reference the same containerPort ({{ .Values.service.targetPort }}) and are configurable via values if desired.src/internal-docs/k8s/values-prod.yaml (1)
14-23:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDocument or make the fixed NodePort configurable.
The service uses a hardcoded
nodePort: 31197, which will cause deployment failures if that port is already allocated in your cluster or falls outside the configured NodePort range. Consider documenting this requirement in your deployment guide or making the port configurable via a separate value.🔧 Recommended approach
Option 1: Document the port requirement in README.md.
Option 2: Make it configurable:
service: type: NodePort port: 6875 targetPort: 80 - nodePort: 31197 # Unique NodePort for production + nodePort: {{ .Values.service.nodePort | default 31197 }}Then allow override at install time:
--set service.nodePort=31198🤖 Prompt for 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. In `@src/internal-docs/k8s/values-prod.yaml` around lines 14 - 23, Replace the hardcoded nodePort by making it a configurable value (service.nodePort) in values-prod.yaml and updating the Helm chart/service template to use .Values.service.nodePort when present (omit nodePort to let Kubernetes pick one if unset); also add a short note in the README explaining the NodePort range requirement and how to override at install time (e.g., --set service.nodePort=31198). Ensure the symbol names referenced are service.nodePort in values and the service template's nodePort field so reviewers can locate and update the template and documentation accordingly.
🧹 Nitpick comments (3)
src/internal-docs/k8s/templates/secret.yaml (1)
10-14: ⚡ Quick winRemove redundant
DB_PASSsecret entry.Both
DB_PASSandDB_PASSWORDreference the same value (.Values.env.DB_PASSWORD), but BookStack expects the standardDB_PASSWORDenvironment variable. TheDB_PASSentry appears unused and creates inconsistency: it hasrequiredvalidation whileDB_PASSWORDdoesn't.♻️ Proposed cleanup
stringData: APP_KEY: {{ required "env.APP_KEY must be set (inject via CI/CD/Secrets)" .Values.env.APP_KEY | quote }} - DB_PASS: {{ required "env.DB_PASSWORD must be set" .Values.env.DB_PASSWORD | quote }} - DB_PASSWORD: {{ .Values.env.DB_PASSWORD | quote }} + DB_PASSWORD: {{ required "env.DB_PASSWORD must be set" .Values.env.DB_PASSWORD | quote }} MYSQL_PASSWORD: {{ .Values.env.DB_PASSWORD | quote }} MYSQL_ROOT_PASSWORD: {{ required "env.MYSQL_ROOT_PASSWORD must be set" .Values.env.MYSQL_ROOT_PASSWORD | quote }}This consolidates to a single
DB_PASSWORDentry with proper validation.🤖 Prompt for 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. In `@src/internal-docs/k8s/templates/secret.yaml` around lines 10 - 14, Remove the redundant DB_PASS secret entry and consolidate to a single DB_PASSWORD entry: delete the DB_PASS line that references .Values.env.DB_PASSWORD, and update the DB_PASSWORD entry to use the same required validation used by DB_PASS (i.e., call required with the same error message or an appropriate one) so only DB_PASSWORD (not DB_PASS) is present and validated; keep other related keys (MYSQL_PASSWORD, MYSQL_ROOT_PASSWORD) unchanged.src/internal-docs/k8s/templates/deployment-app.yaml (1)
17-60: ⚡ Quick winConsider adding a security context for defense in depth.
The pod spec lacks security context constraints, which means containers run with default privileges. For production workloads, enforcing
runAsNonRoot, dropping capabilities, and setting read-only root filesystem improves your security posture.🔒 Proposed security context
spec: {{- with .Values.imagePullSecrets }} imagePullSecrets: {{- toYaml . | nindent 8 }} {{- end }} + securityContext: + runAsNonRoot: true + runAsUser: 1000 + fsGroup: 1000 containers: - name: {{ .Values.app.label }} image: "{{ .Values.image.repository }}:{{ .Values.image.tag }}" imagePullPolicy: {{ .Values.image.pullPolicy }} + securityContext: + allowPrivilegeEscalation: false + capabilities: + drop: + - ALL + readOnlyRootFilesystem: false # BookStack needs write access to /config ports:Note: Verify that the BookStack image supports running as non-root user 1000.
🤖 Prompt for 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. In `@src/internal-docs/k8s/templates/deployment-app.yaml` around lines 17 - 60, Add a security context at the Pod and Container levels in the Deployment template to enforce non-root execution and drop privileges: in the spec (near containers:) add pod-level securityContext with runAsNonRoot: true, runAsUser: 1000 (and fsGroup if needed), and at the container (the one named via {{ .Values.app.label }}) add securityContext with allowPrivilegeEscalation: false, readOnlyRootFilesystem: true, and capabilities: drop: ["ALL"]; ensure any writable mount (e.g., the bookstack-config mount or other app-specific directories) are listed in a writable emptyDir/PVC or excluded from readOnlyRootFilesystem, and confirm the BookStack image supports UID 1000 before enabling runAsUser.src/internal-docs/k8s/templates/hpa.yaml (1)
1-24: HPA looks good, but verify metrics-server is installed.The HorizontalPodAutoscaler configuration is correct. Note that HPA requires the Kubernetes metrics-server to be deployed in your cluster. Without it, the HPA won't collect CPU metrics and autoscaling will not function.
✅ Verify metrics-server
Run this to check if metrics-server is available:
#!/bin/bash # Check if metrics-server is running in the cluster kubectl get deployment metrics-server -n kube-system🤖 Prompt for 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. In `@src/internal-docs/k8s/templates/hpa.yaml` around lines 1 - 24, The HPA template (src/internal-docs/k8s/templates/hpa.yaml) requires Kubernetes metrics-server to function; add a clear user-facing note in the chart documentation or Helm NOTES (e.g., create/update templates/NOTES.txt or README) that states metrics-server must be installed and include the verification command (kubectl get deployment metrics-server -n kube-system) and suggested install link/command; reference the HorizontalPodAutoscaler resource and .Values.autoscaling usage so consumers know to check metrics-server before enabling autoscaling.
🤖 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.
Outside diff comments:
In `@src/internal-docs/k8s/templates/deployment-app.yaml`:
- Around line 22-42: Add HTTP liveness and readiness probes to the container
spec for production availability: inside the container block for {{
.Values.app.label }} in deployment-app.yaml, add a readinessProbe and
livenessProbe that use httpGet on the app path (prefer /status, fall back to
/login if /status isn't available) and set sensible timings (e.g.,
initialDelaySeconds, periodSeconds, timeoutSeconds,
successThreshold/failureThreshold) so Kubernetes waits for startup migrations
and restarts hung processes; ensure the probes reference the same containerPort
({{ .Values.service.targetPort }}) and are configurable via values if desired.
In `@src/internal-docs/k8s/values-prod.yaml`:
- Around line 14-23: Replace the hardcoded nodePort by making it a configurable
value (service.nodePort) in values-prod.yaml and updating the Helm chart/service
template to use .Values.service.nodePort when present (omit nodePort to let
Kubernetes pick one if unset); also add a short note in the README explaining
the NodePort range requirement and how to override at install time (e.g., --set
service.nodePort=31198). Ensure the symbol names referenced are service.nodePort
in values and the service template's nodePort field so reviewers can locate and
update the template and documentation accordingly.
---
Nitpick comments:
In `@src/internal-docs/k8s/templates/deployment-app.yaml`:
- Around line 17-60: Add a security context at the Pod and Container levels in
the Deployment template to enforce non-root execution and drop privileges: in
the spec (near containers:) add pod-level securityContext with runAsNonRoot:
true, runAsUser: 1000 (and fsGroup if needed), and at the container (the one
named via {{ .Values.app.label }}) add securityContext with
allowPrivilegeEscalation: false, readOnlyRootFilesystem: true, and capabilities:
drop: ["ALL"]; ensure any writable mount (e.g., the bookstack-config mount or
other app-specific directories) are listed in a writable emptyDir/PVC or
excluded from readOnlyRootFilesystem, and confirm the BookStack image supports
UID 1000 before enabling runAsUser.
In `@src/internal-docs/k8s/templates/hpa.yaml`:
- Around line 1-24: The HPA template (src/internal-docs/k8s/templates/hpa.yaml)
requires Kubernetes metrics-server to function; add a clear user-facing note in
the chart documentation or Helm NOTES (e.g., create/update templates/NOTES.txt
or README) that states metrics-server must be installed and include the
verification command (kubectl get deployment metrics-server -n kube-system) and
suggested install link/command; reference the HorizontalPodAutoscaler resource
and .Values.autoscaling usage so consumers know to check metrics-server before
enabling autoscaling.
In `@src/internal-docs/k8s/templates/secret.yaml`:
- Around line 10-14: Remove the redundant DB_PASS secret entry and consolidate
to a single DB_PASSWORD entry: delete the DB_PASS line that references
.Values.env.DB_PASSWORD, and update the DB_PASSWORD entry to use the same
required validation used by DB_PASS (i.e., call required with the same error
message or an appropriate one) so only DB_PASSWORD (not DB_PASS) is present and
validated; keep other related keys (MYSQL_PASSWORD, MYSQL_ROOT_PASSWORD)
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6614fb02-e4f6-487b-a29c-bebef4afddc0
📒 Files selected for processing (11)
src/internal-docs/.env.examplesrc/internal-docs/README.mdsrc/internal-docs/docker-compose.ymlsrc/internal-docs/k8s/Chart.yamlsrc/internal-docs/k8s/templates/configmap.yamlsrc/internal-docs/k8s/templates/deployment-app.yamlsrc/internal-docs/k8s/templates/deployment-db.yamlsrc/internal-docs/k8s/templates/hpa.yamlsrc/internal-docs/k8s/templates/pvc.yamlsrc/internal-docs/k8s/templates/secret.yamlsrc/internal-docs/k8s/values-prod.yaml
✅ Files skipped from review due to trivial changes (3)
- src/internal-docs/k8s/Chart.yaml
- src/internal-docs/README.md
- src/internal-docs/.env.example
🚧 Files skipped from review as they are similar to previous changes (2)
- src/internal-docs/docker-compose.yml
- src/internal-docs/k8s/templates/deployment-db.yaml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📖 Description
This PR introduces the foundational containerized environment and Kubernetes Helm charts required to deploy Mariana Deep (our internal documentation platform powered by BookStack) strictly to our production environment.
This sets up a robust, scalable architecture for our engineering documentation, allowing seamless contributions from the team without the maintenance overhead of redundant staging environments.
🛠️ Changes Made
Chart.yaml,deployment-app,deployment-db,service,pvc) undersrc/internal-docs/k8s/mirroring our existing platform architecture.values-prod.yamlfor a dedicated, optimized production deployment.docker-compose.ymlwith the officialmariadb:10.6to ensure stable compatibility with BookStack's database schema.bookstack_db_data) to provide native performance and prevent cross-platform file locking issues.README.mdincluding a Table of Contents, Product-focused Information Architecture, and clear Contribution Guidelines for the team.🚦 Testing Instructions
src/internal-docs.cp .env.example .env.docker compose up -d.http://localhost:6875and log in with the default credentials (admin@admin.com/password) to verify the environment starts from a clean slate.✅ Checklist
README.md).Summary by CodeRabbit
Chores
Documentation