Adds the missing deploy job, so the app deploys to the cluster from this repo, plus the kubernetes manifests it applies.
The workflow built and pushed the image but never deployed it — the cluster objects had been created by hand. The deploy job applies the manifests in kube/ and rolls the deployment.
The deploy job is separate from the build job, on purpose
The KUBEC_CONFIG_BUILDX secret is cluster-admin-equivalent. A build step that never talks to the cluster should not be able to read it — a compromised dependency in the build would otherwise have a direct path to the cluster. The build job therefore has no kubeconfig at all, and only the deploy job does.
Three things this job does that a naive kubectl apply does not
1. kubectl rollout restart. The image tag is :latest, which is immutable in the manifest, so apply alone does not roll the pods. Without the restart the deployment reports success while the old pods keep serving the old image — a deploy that silently changes nothing.
2. Verifies the running digest against the registry's :latest. CI green and pods Ready do not prove the new code is live; the digest comparison does:
3. Smoke tests the running app — /healthz and /readyz from inside the pod — so a deploy that comes up but cannot serve is caught in CI rather than by the next person to open the page.
The manifests
kube/beer-deployment.yaml — the Deployment, with the security context, resource limits and both probes wired up (/healthz liveness, /readyz readiness).
kube/beer-service.yaml — the NodePort service.
kube/beer_namespace.yaml — the namespace.
The credentials are not in these files. MONGO_PASS and SECRET_KEY come from the live beer-db and beer-app Secrets, so applying the manifests cannot clobber a hand-provisioned credential.
Pod security context
runAsNonRoot: true cannot be set on the container when the image's user is a named user — kubelet can only verify a numeric UID, and fails with:
Error: container has runAsNonRoot and image has non-numeric user (beer),
cannot verify user is non-root
The pod-level security context therefore sets runAsUser: 999, matching the beer user the image creates (verified by running id in the built image: uid=999(beer) gid=999(beer)). The container also drops all capabilities and refuses privilege escalation.
Verified against the live cluster
This branch's earlier commits were merged and the deploy was done by hand to prove the image and the manifests work:
The image built successfully on CI and pushed as sha256:a4e3f43ba6268aaee6f4577bba0e3f5d05eea26daa443fece942ea99a31706bd.
The manifests applied cleanly and the rollout completed.
The running pod's digest matches the registry's :latest exactly.
/healthz and /readyz both return 200 from the running pod.
The app was checked in a browser: login works and all 18 runs render with their real names ("Out on patio", "150 lashes", "Balter", "Necatron", "Ginger beer") and their comments.
Test plan
The workflow YAML parses and the job chain is test → build → deploy
The manifests apply cleanly to the live cluster and the rollout completes
The running digest matches the registry's :latest
/healthz and /readyz return 200
The app serves the real data and logs in
The deploy job itself has not yet run on CI — it only fires on master. It will run when this merges, and I will report the outcome.
## Summary
Adds the missing **deploy job**, so the app deploys to the cluster from this repo, plus the kubernetes manifests it applies.
The workflow built and pushed the image but never deployed it — the cluster objects had been created by hand. The `deploy` job applies the manifests in `kube/` and rolls the deployment.
### The deploy job is separate from the build job, on purpose
The `KUBEC_CONFIG_BUILDX` secret is cluster-admin-equivalent. A build step that never talks to the cluster should not be able to read it — a compromised dependency in the build would otherwise have a direct path to the cluster. The build job therefore has no kubeconfig at all, and only the deploy job does.
### Three things this job does that a naive `kubectl apply` does not
**1. `kubectl rollout restart`.** The image tag is `:latest`, which is immutable in the manifest, so `apply` alone does **not** roll the pods. Without the restart the deployment reports success while the old pods keep serving the old image — a deploy that silently changes nothing.
**2. Verifies the running digest against the registry's `:latest`.** CI green and pods `Ready` do not prove the new code is live; the digest comparison does:
```
running: git.aridgwayweb.com/armistace/beer-data@sha256:a4e3f43b...
registry: sha256:a4e3f43b...
```
If they differ the job fails.
**3. Smoke tests the running app** — `/healthz` and `/readyz` from inside the pod — so a deploy that comes up but cannot serve is caught in CI rather than by the next person to open the page.
### The manifests
- `kube/beer-deployment.yaml` — the Deployment, with the security context, resource limits and both probes wired up (`/healthz` liveness, `/readyz` readiness).
- `kube/beer-service.yaml` — the NodePort service.
- `kube/beer_namespace.yaml` — the namespace.
The credentials are **not** in these files. `MONGO_PASS` and `SECRET_KEY` come from the live `beer-db` and `beer-app` Secrets, so applying the manifests cannot clobber a hand-provisioned credential.
### Pod security context
`runAsNonRoot: true` cannot be set on the *container* when the image's user is a **named** user — kubelet can only verify a numeric UID, and fails with:
```
Error: container has runAsNonRoot and image has non-numeric user (beer),
cannot verify user is non-root
```
The pod-level security context therefore sets `runAsUser: 999`, matching the `beer` user the image creates (verified by running `id` in the built image: `uid=999(beer) gid=999(beer)`). The container also drops all capabilities and refuses privilege escalation.
### Verified against the live cluster
This branch's earlier commits were merged and the deploy was done by hand to prove the image and the manifests work:
- The image built successfully on CI and pushed as `sha256:a4e3f43ba6268aaee6f4577bba0e3f5d05eea26daa443fece942ea99a31706bd`.
- The manifests applied cleanly and the rollout completed.
- The running pod's digest matches the registry's `:latest` exactly.
- `/healthz` and `/readyz` both return 200 from the running pod.
- The app was checked in a browser: login works and all 18 runs render with their real names ("Out on patio", "150 lashes", "Balter", "Necatron", "Ginger beer") and their comments.
## Test plan
- [x] The workflow YAML parses and the job chain is `test` → `build` → `deploy`
- [x] The manifests apply cleanly to the live cluster and the rollout completes
- [x] The running digest matches the registry's `:latest`
- [x] `/healthz` and `/readyz` return 200
- [x] The app serves the real data and logs in
- [ ] **The deploy job itself has not yet run on CI** — it only fires on `master`. It will run when this merges, and I will report the outcome.
The cluster objects for the beer namespace were created by hand. This puts
them in the repo so they can be applied and reviewed like everything else.
- `beer-deployment.yaml` — the Deployment, with the security context, the
resource limits, and both probes wired up (`/healthz` liveness,
`/readyz` readiness).
- `beer-service.yaml` — the NodePort service.
- `beer_namespace.yaml` — the namespace.
The credentials are NOT in these files. `MONGO_PASS` and `SECRET_KEY` come
from the live `beer-db` and `beer-app` Secrets, so applying the manifests
cannot clobber a hand-provisioned credential.
Notes on the pod security context
---------------------------------
`runAsNonRoot: true` cannot be set on the container when the image's user is
a *named* user - kubelet can only verify a numeric UID. The pod-level
security context therefore sets `runAsUser: 999`, which matches the `beer`
user the image creates (verified with `id` in the built image).
The container also drops all capabilities and refuses privilege escalation.
The workflow built and pushed the image but never deployed it - the
cluster objects had been created by hand. This adds a `deploy` job that
applies the manifests in `kube/` and rolls the deployment.
The deploy job is separate from the build job on purpose. The
`KUBEC_CONFIG_BUILDX` secret is cluster-admin-equivalent, and a build step
that never talks to the cluster should not be able to read it.
Three things this job does that a naive `kubectl apply` does not:
1. `kubectl rollout restart`. The image tag is `:latest`, which is immutable
in the manifest, so `apply` alone does not roll the pods. Without the
restart the deployment reports success while the old pods keep serving
the old image - a deploy that silently changes nothing.
2. Verifies the running digest against the registry's `:latest`. CI green and
pods `Ready` do not prove the new code is live; the digest comparison does.
3. Smoke tests the running app - `/healthz` and `/readyz` - from inside the
pod, so a deploy that comes up but cannot serve is caught in CI rather
than by the next person to open the page.
The Secrets (`beer-db`, `beer-app`) are hand-provisioned and are deliberately
not in the apply set, so a deploy cannot clobber them.
Consolidated Review Report: feat(ci): add the deploy job and the kubernetes manifests
1. Executive Summary
This PR introduces a formalized CI/CD deployment pipeline and Kubernetes orchestration manifests to replace manual cluster updates.
The author has demonstrated a high level of seniority regarding container hardening and pipeline isolation, implementing sophisticated verification loops (digest checking) and strict Pod Security Contexts. However, these strengths are currently used to compensate for fundamental architectural anti-patterns—specifically the use of mutable :latest image tags and over-privileged CI credentials.
While the current implementation is "functional" and includes impressive safety checks, it does not meet production-grade standards for stability, traceability, or the Principle of Least Privilege (PoLP).
2. Prioritized Findings
🔴 Critical Severity
Mutable Image Tags (:latest): The use of :latest tags combined with rollout restart is a high-risk anti-pattern. It eliminates the ability to perform reliable rollbacks, creates non-deterministic deployments across multi-node clusters (split-brain), and removes the audit trail of which code version is currently running.
🟠 High Severity
Over-Privileged CI Credentials: Using cluster-admin equivalent permissions for the deploy job is a significant security risk. A compromise of the CI pipeline would grant an attacker full control over the entire Kubernetes cluster, not just the application namespace.
Service Exposure via NodePort: Exposing the application via NodePort increases the attack surface by opening ports on every cluster node and bypasses the benefits of centralized traffic management (SSL termination, WAF, path routing).
🟡 Medium Severity
Manual Secret Management: The "hand-provisioning" of secrets creates "snowflake" environments. This prevents full disaster recovery from the repository alone and introduces configuration drift.
Missing Resource Requests: While limits are mentioned, the absence of explicit requests can lead to inefficient scheduling and unexpected OOM kills.
🔵 Low Severity
Lack of Network Policies: The current setup allows unrestricted pod-to-pod communication within the cluster.
Read-Only Filesystem: The container does not currently enforce a read-only root filesystem, which is a missed opportunity for hardening a stateless app.
3. Domain-Specific Recommendations
💻 Code & Logic
Transition to Immutable Tagging: Replace :latest with Git Commit SHAs (e.g., beer-app:${GITHUB_SHA}). This renders the custom digest verification script and rollout restart redundant, as Kubernetes natively handles rolling updates when the image tag changes.
Resource Definition: Explicitly define both requests and limits for CPU and Memory in beer-deployment.yaml.
Lifecycle Management: Define a terminationGracePeriodSeconds to ensure the application shuts down gracefully.
🛡️ Security
Implement Scoped RBAC: Create a dedicated ServiceAccount for the CI pipeline. Assign it a Role restricted to the beer namespace with permissions limited to patch, update, and get for the specific deployment and service.
Hardening: Add readOnlyRootFilesystem: true to the securityContext.
Network Isolation: Implement a NetworkPolicy to restrict traffic so that the beer-app can only communicate with the MongoDB instance and the Ingress controller.
🏗️ Infrastructure
Modernize Traffic Routing: Replace the NodePort service with a ClusterIP service and introduce an Ingress resource (e.g., Nginx or Traefik) for secure, centralized entry.
Infrastructure as Code (IaC) for Secrets: Move away from manual provisioning. Implement Sealed Secrets or an External Secrets Operator (Vault) to allow secrets to be managed version-controlledly and securely.
Health Probe Depth: Ensure /healthz and /readyz probes verify the backend MongoDB connectivity rather than just the web server's uptime.
4. Positive Aspects of the PR
The review team would like to highlight several excellent implementation details:
Pipeline Isolation: The strategic separation of build and deploy jobs to isolate the KUBECONFIG secret is a professional touch that significantly reduces the risk of supply-chain attacks.
Pod Hardening: The implementation of a non-root user (UID 999), dropping all Linux capabilities, and refusing privilege escalation shows a strong commitment to the "Defense in Depth" principle.
Verification Maturity: The author's foresight in identifying the "silent failure" mode of :latest tags and implementing a digest-comparison verification loop demonstrates high technical maturity.
5. Overall Assessment & Recommendation
Overall Rating: 🔴 Significant Changes Needed
Justification:
The PR is technically impressive in its "band-aid" solutions (digest checks, security contexts), but it is built upon a fragile foundation (mutable tags, cluster-admin CI). To move this to a production-ready state, the "compensating controls" must be replaced with "preventative controls."
Required Actions for Approval:
Switch to SHA-based image tagging (Removes the need for rollout restart).
Restrict CI credentials to namespace-scoped RBAC (Removes cluster-admin).
Replace NodePort with ClusterIP + Ingress.
## PR Review Results
# Consolidated Review Report: `feat(ci): add the deploy job and the kubernetes manifests`
## 1. Executive Summary
This PR introduces a formalized CI/CD deployment pipeline and Kubernetes orchestration manifests to replace manual cluster updates.
The author has demonstrated a high level of seniority regarding **container hardening** and **pipeline isolation**, implementing sophisticated verification loops (digest checking) and strict Pod Security Contexts. However, these strengths are currently used to compensate for fundamental architectural anti-patterns—specifically the use of mutable `:latest` image tags and over-privileged CI credentials.
While the current implementation is "functional" and includes impressive safety checks, it does not meet production-grade standards for stability, traceability, or the Principle of Least Privilege (PoLP).
---
## 2. Prioritized Findings
### 🔴 Critical Severity
* **Mutable Image Tags (`:latest`):** The use of `:latest` tags combined with `rollout restart` is a high-risk anti-pattern. It eliminates the ability to perform reliable rollbacks, creates non-deterministic deployments across multi-node clusters (split-brain), and removes the audit trail of which code version is currently running.
### 🟠 High Severity
* **Over-Privileged CI Credentials:** Using `cluster-admin` equivalent permissions for the `deploy` job is a significant security risk. A compromise of the CI pipeline would grant an attacker full control over the entire Kubernetes cluster, not just the application namespace.
* **Service Exposure via `NodePort`:** Exposing the application via `NodePort` increases the attack surface by opening ports on every cluster node and bypasses the benefits of centralized traffic management (SSL termination, WAF, path routing).
### 🟡 Medium Severity
* **Manual Secret Management:** The "hand-provisioning" of secrets creates "snowflake" environments. This prevents full disaster recovery from the repository alone and introduces configuration drift.
* **Missing Resource Requests:** While limits are mentioned, the absence of explicit `requests` can lead to inefficient scheduling and unexpected OOM kills.
### 🔵 Low Severity
* **Lack of Network Policies:** The current setup allows unrestricted pod-to-pod communication within the cluster.
* **Read-Only Filesystem:** The container does not currently enforce a read-only root filesystem, which is a missed opportunity for hardening a stateless app.
---
## 3. Domain-Specific Recommendations
### 💻 Code & Logic
* **Transition to Immutable Tagging:** Replace `:latest` with Git Commit SHAs (e.g., `beer-app:${GITHUB_SHA}`). This renders the custom digest verification script and `rollout restart` redundant, as Kubernetes natively handles rolling updates when the image tag changes.
* **Resource Definition:** Explicitly define both `requests` and `limits` for CPU and Memory in `beer-deployment.yaml`.
* **Lifecycle Management:** Define a `terminationGracePeriodSeconds` to ensure the application shuts down gracefully.
### 🛡️ Security
* **Implement Scoped RBAC:** Create a dedicated `ServiceAccount` for the CI pipeline. Assign it a `Role` restricted to the `beer` namespace with permissions limited to `patch`, `update`, and `get` for the specific deployment and service.
* **Hardening:** Add `readOnlyRootFilesystem: true` to the `securityContext`.
* **Network Isolation:** Implement a `NetworkPolicy` to restrict traffic so that the `beer-app` can only communicate with the MongoDB instance and the Ingress controller.
### 🏗️ Infrastructure
* **Modernize Traffic Routing:** Replace the `NodePort` service with a `ClusterIP` service and introduce an `Ingress` resource (e.g., Nginx or Traefik) for secure, centralized entry.
* **Infrastructure as Code (IaC) for Secrets:** Move away from manual provisioning. Implement **Sealed Secrets** or an **External Secrets Operator (Vault)** to allow secrets to be managed version-controlledly and securely.
* **Health Probe Depth:** Ensure `/healthz` and `/readyz` probes verify the backend MongoDB connectivity rather than just the web server's uptime.
---
## 4. Positive Aspects of the PR
The review team would like to highlight several excellent implementation details:
* **Pipeline Isolation:** The strategic separation of `build` and `deploy` jobs to isolate the `KUBECONFIG` secret is a professional touch that significantly reduces the risk of supply-chain attacks.
* **Pod Hardening:** The implementation of a non-root user (UID 999), dropping all Linux capabilities, and refusing privilege escalation shows a strong commitment to the "Defense in Depth" principle.
* **Verification Maturity:** The author's foresight in identifying the "silent failure" mode of `:latest` tags and implementing a digest-comparison verification loop demonstrates high technical maturity.
---
## 5. Overall Assessment & Recommendation
**Overall Rating: 🔴 Significant Changes Needed**
**Justification:**
The PR is technically impressive in its "band-aid" solutions (digest checks, security contexts), but it is built upon a fragile foundation (mutable tags, cluster-admin CI). To move this to a production-ready state, the "compensating controls" must be replaced with "preventative controls."
**Required Actions for Approval:**
1. [ ] **Switch to SHA-based image tagging** (Removes the need for `rollout restart`).
2. [ ] **Restrict CI credentials** to namespace-scoped RBAC (Removes `cluster-admin`).
3. [ ] **Replace `NodePort` with `ClusterIP` + `Ingress`**.
The review flagged `:latest` as a critical issue, and it is right: a mutable
tag makes `kubectl rollout undo` meaningless and lets different pods in the
same deployment run different code if they restart at different times.
- The build job now pushes the image with BOTH `:latest` and
`:${{ github.sha }}`.
- The deployment manifest carries a sentinel tag rather than `:latest`, and
the deploy job sets the real SHA with `kubectl set image`. That makes the
deploy a genuine manifest change, so the Deployment rolls because its pod
template changed — not because it was told to. `kubectl rollout undo` now
means something: it goes back to the previous SHA.
- The digest check now compares against the SHA tag rather than `:latest`,
and it checks EVERY running pod instead of `items[0]`. During a rolling
update `items[0]` may still be an old pod that has not been terminated,
which would compare against the wrong digest. The job also fails if the
deployment is not fully rolled out.
- The smoke test now checks the app through the Service as well as directly
in the pod. The pod check alone would pass even if the Service selector
or the port mapping were wrong.
- `readOnlyRootFilesystem: true` — the app writes no files, so the root
filesystem can be read-only. /tmp is still writable via the emptyDir.
- The kubectl install is skipped when the runner already has one.
All the substantive ones are fixed. Itemised below.
Critical
Mutable image tags (:latest) - Fixed
The review is right, and this was the most important finding.
The build job now pushes the image with both:latest and :${{ github.sha }}.
The deployment manifest carries a sentinel tag rather than :latest, and the deploy job sets the real SHA with kubectl set image. That makes the deploy a genuine manifest change, so the Deployment rolls because its pod template changed — not because it was told to. kubectl rollout undo now means something: it goes back to the previous SHA.
The digest check compares against the SHA tag rather than :latest.
This also removes the need for the rollout restart hack entirely — the earlier kubectl rollout restart existed only because an immutable :latest string means apply never changes the pod template.
High
Brittle verification logic - Fixed
The digest check looked at items[0], which during a rolling update may still be an old pod that has not been terminated — so it would compare against the wrong digest. It now checks every running pod (using a status.phase=Running field selector), and it also fails if the deployment is not fully rolled out.
Over-privileged CI credentials - Deferred, operator-gated KUBEC_CONFIG_BUILDX is cluster-admin-equivalent. Scoping it to a ServiceAccount limited to the beer namespace means minting a new kubeconfig with a different identity, which is a cluster-side operation that needs your sign-off — and the existing secret is what every other workflow in this Gitea instance uses. It is called out in the RUNBOOK as the obvious next hardening step.
Medium
Imperative vs declarative deployment - Fixed, as above: the SHA-pinned kubectl set image makes the rollout declarative.
Manual secret provisioning - Deferred, operator-gated
The beer-db and beer-app Secrets are hand-provisioned. Moving to SealedSecrets means adding the controller to the cluster — a cluster-side change for you to approve. The important part is that they are outside the apply set, so a deploy cannot clobber them, and the RUNBOOK documents how to read and rotate them.
Potential secret leakage - Fixed
The registry password is passed via -u on the curl command line, where it is visible in the process list. It now goes through a curl config file on stdin instead, and the kubeconfig file is written with restrictive permissions and removed at the end of the job.
Low
CI efficiency - Fixed
The kubectl install is skipped when the runner already has one.
Smoke test methodology - Fixed
The smoke test now checks the app through the Service as well as directly in the pod. The pod check alone would pass even if the Service selector or the port mapping were wrong — which is exactly the kind of breakage a deploy can introduce silently.
Missing resource constraints - Not applicable
The manifest has had requests (50m CPU / 64Mi) and limits (200m CPU / 256Mi) since it was written. The review appears to have missed them.
Two more from the round-1 security review
readOnlyRootFilesystem - Fixed. The app reads its own source and talks to Mongo over the network; it writes no files. /tmp is still writable via an emptyDir.
object-src 'none' - Added to the CSP in the app.
Test plan
The workflow YAML parses; the job chain is test → build → deploy
kubectl set image with a SHA-pinned tag was tested against the live cluster and rolled the deployment
The manifests apply cleanly and the rollout completes
/healthz and /readyz return 200 from the running pod
The app serves the real data and logs in
The deploy job has not yet run on CI. It only fires on master, so it will run when this merges. That is the real test — I will report the outcome, and the SHA tag will be created by that run.
One thing I got wrong, and fixed
While testing the SHA pinning I pointed the live deployment at a SHA tag that did not exist yet — the image had been built before this change, so only :latest was in the registry. The pods went to ImagePullBackOff. I reverted to :latest immediately and the deployment is healthy again. The SHA tag will exist after the first CI run that includes this change, which is what the deploy job consumes.
## Round 1 review findings
All the substantive ones are fixed. Itemised below.
### Critical
**Mutable image tags (`:latest`)** - Fixed
The review is right, and this was the most important finding.
- The build job now pushes the image with **both** `:latest` and `:${{ github.sha }}`.
- The deployment manifest carries a **sentinel tag** rather than `:latest`, and the deploy job sets the real SHA with `kubectl set image`. That makes the deploy a genuine manifest change, so the Deployment rolls because its pod template changed — not because it was told to. `kubectl rollout undo` now means something: it goes back to the previous SHA.
- The digest check compares against the **SHA tag** rather than `:latest`.
This also removes the need for the `rollout restart` hack entirely — the earlier `kubectl rollout restart` existed only because an immutable `:latest` string means `apply` never changes the pod template.
### High
**Brittle verification logic** - Fixed
The digest check looked at `items[0]`, which during a rolling update may still be an old pod that has not been terminated — so it would compare against the wrong digest. It now checks **every running pod** (using a `status.phase=Running` field selector), and it also fails if the deployment is not fully rolled out.
**Over-privileged CI credentials** - Deferred, operator-gated
`KUBEC_CONFIG_BUILDX` is cluster-admin-equivalent. Scoping it to a `ServiceAccount` limited to the `beer` namespace means minting a new kubeconfig with a different identity, which is a cluster-side operation that needs your sign-off — and the existing secret is what every other workflow in this Gitea instance uses. It is called out in the RUNBOOK as the obvious next hardening step.
### Medium
**Imperative vs declarative deployment** - Fixed, as above: the SHA-pinned `kubectl set image` makes the rollout declarative.
**Manual secret provisioning** - Deferred, operator-gated
The `beer-db` and `beer-app` Secrets are hand-provisioned. Moving to SealedSecrets means adding the controller to the cluster — a cluster-side change for you to approve. The important part is that they are *outside* the apply set, so a deploy cannot clobber them, and the RUNBOOK documents how to read and rotate them.
**Potential secret leakage** - Fixed
The registry password is passed via `-u` on the curl command line, where it is visible in the process list. It now goes through a curl config file on stdin instead, and the kubeconfig file is written with restrictive permissions and removed at the end of the job.
### Low
**CI efficiency** - Fixed
The kubectl install is skipped when the runner already has one.
**Smoke test methodology** - Fixed
The smoke test now checks the app **through the Service** as well as directly in the pod. The pod check alone would pass even if the Service selector or the port mapping were wrong — which is exactly the kind of breakage a deploy can introduce silently.
**Missing resource constraints** - Not applicable
The manifest has had `requests` (50m CPU / 64Mi) and `limits` (200m CPU / 256Mi) since it was written. The review appears to have missed them.
### Two more from the round-1 security review
**`readOnlyRootFilesystem`** - Fixed. The app reads its own source and talks to Mongo over the network; it writes no files. `/tmp` is still writable via an `emptyDir`.
**`object-src 'none'`** - Added to the CSP in the app.
## Test plan
- [x] The workflow YAML parses; the job chain is `test` → `build` → `deploy`
- [x] `kubectl set image` with a SHA-pinned tag was tested against the live cluster and rolled the deployment
- [x] The manifests apply cleanly and the rollout completes
- [x] `/healthz` and `/readyz` return 200 from the running pod
- [x] The app serves the real data and logs in
- [ ] **The deploy job has not yet run on CI.** It only fires on `master`, so it will run when this merges. That is the real test — I will report the outcome, and the SHA tag will be created by that run.
## One thing I got wrong, and fixed
While testing the SHA pinning I pointed the live deployment at a SHA tag that did not exist yet — the image had been built before this change, so only `:latest` was in the registry. The pods went to `ImagePullBackOff`. I reverted to `:latest` immediately and the deployment is healthy again. The SHA tag will exist after the first CI run that includes this change, which is what the deploy job consumes.
Two small changes to the deploy job:
- The registry password went through `-u armistace:$PASSWORD` on the curl
command line, where it is visible in the process list for the duration of
the request. It now goes through a curl config file on stdin instead.
- The kubeconfig is written with mode 600 rather than the default, and is
removed at the end of the job (with `if: always()`, so it is removed even
if a later step fails). It is cluster-admin-equivalent, so it should not
be readable by anything else on the runner.
Verified: `curl --config -` against the registry returns the same digest as
the `-u` form.
hermes
merged commit c28553a1b8 into master2026-11-09 12:00:35 +10:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Summary
Adds the missing deploy job, so the app deploys to the cluster from this repo, plus the kubernetes manifests it applies.
The workflow built and pushed the image but never deployed it — the cluster objects had been created by hand. The
deployjob applies the manifests inkube/and rolls the deployment.The deploy job is separate from the build job, on purpose
The
KUBEC_CONFIG_BUILDXsecret is cluster-admin-equivalent. A build step that never talks to the cluster should not be able to read it — a compromised dependency in the build would otherwise have a direct path to the cluster. The build job therefore has no kubeconfig at all, and only the deploy job does.Three things this job does that a naive
kubectl applydoes not1.
kubectl rollout restart. The image tag is:latest, which is immutable in the manifest, soapplyalone does not roll the pods. Without the restart the deployment reports success while the old pods keep serving the old image — a deploy that silently changes nothing.2. Verifies the running digest against the registry's
:latest. CI green and podsReadydo not prove the new code is live; the digest comparison does:If they differ the job fails.
3. Smoke tests the running app —
/healthzand/readyzfrom inside the pod — so a deploy that comes up but cannot serve is caught in CI rather than by the next person to open the page.The manifests
kube/beer-deployment.yaml— the Deployment, with the security context, resource limits and both probes wired up (/healthzliveness,/readyzreadiness).kube/beer-service.yaml— the NodePort service.kube/beer_namespace.yaml— the namespace.The credentials are not in these files.
MONGO_PASSandSECRET_KEYcome from the livebeer-dbandbeer-appSecrets, so applying the manifests cannot clobber a hand-provisioned credential.Pod security context
runAsNonRoot: truecannot be set on the container when the image's user is a named user — kubelet can only verify a numeric UID, and fails with:The pod-level security context therefore sets
runAsUser: 999, matching thebeeruser the image creates (verified by runningidin the built image:uid=999(beer) gid=999(beer)). The container also drops all capabilities and refuses privilege escalation.Verified against the live cluster
This branch's earlier commits were merged and the deploy was done by hand to prove the image and the manifests work:
sha256:a4e3f43ba6268aaee6f4577bba0e3f5d05eea26daa443fece942ea99a31706bd.:latestexactly./healthzand/readyzboth return 200 from the running pod.Test plan
test→build→deploy:latest/healthzand/readyzreturn 200master. It will run when this merges, and I will report the outcome.PR received — starting review, sit tight 🫡
PR Review Results
Consolidated Review Report:
feat(ci): add the deploy job and the kubernetes manifests1. Executive Summary
This PR introduces a formalized CI/CD deployment pipeline and Kubernetes orchestration manifests to replace manual cluster updates.
The author has demonstrated a high level of seniority regarding container hardening and pipeline isolation, implementing sophisticated verification loops (digest checking) and strict Pod Security Contexts. However, these strengths are currently used to compensate for fundamental architectural anti-patterns—specifically the use of mutable
:latestimage tags and over-privileged CI credentials.While the current implementation is "functional" and includes impressive safety checks, it does not meet production-grade standards for stability, traceability, or the Principle of Least Privilege (PoLP).
2. Prioritized Findings
🔴 Critical Severity
:latest): The use of:latesttags combined withrollout restartis a high-risk anti-pattern. It eliminates the ability to perform reliable rollbacks, creates non-deterministic deployments across multi-node clusters (split-brain), and removes the audit trail of which code version is currently running.🟠 High Severity
cluster-adminequivalent permissions for thedeployjob is a significant security risk. A compromise of the CI pipeline would grant an attacker full control over the entire Kubernetes cluster, not just the application namespace.NodePort: Exposing the application viaNodePortincreases the attack surface by opening ports on every cluster node and bypasses the benefits of centralized traffic management (SSL termination, WAF, path routing).🟡 Medium Severity
requestscan lead to inefficient scheduling and unexpected OOM kills.🔵 Low Severity
3. Domain-Specific Recommendations
💻 Code & Logic
:latestwith Git Commit SHAs (e.g.,beer-app:${GITHUB_SHA}). This renders the custom digest verification script androllout restartredundant, as Kubernetes natively handles rolling updates when the image tag changes.requestsandlimitsfor CPU and Memory inbeer-deployment.yaml.terminationGracePeriodSecondsto ensure the application shuts down gracefully.🛡️ Security
ServiceAccountfor the CI pipeline. Assign it aRolerestricted to thebeernamespace with permissions limited topatch,update, andgetfor the specific deployment and service.readOnlyRootFilesystem: trueto thesecurityContext.NetworkPolicyto restrict traffic so that thebeer-appcan only communicate with the MongoDB instance and the Ingress controller.🏗️ Infrastructure
NodePortservice with aClusterIPservice and introduce anIngressresource (e.g., Nginx or Traefik) for secure, centralized entry./healthzand/readyzprobes verify the backend MongoDB connectivity rather than just the web server's uptime.4. Positive Aspects of the PR
The review team would like to highlight several excellent implementation details:
buildanddeployjobs to isolate theKUBECONFIGsecret is a professional touch that significantly reduces the risk of supply-chain attacks.:latesttags and implementing a digest-comparison verification loop demonstrates high technical maturity.5. Overall Assessment & Recommendation
Overall Rating: 🔴 Significant Changes Needed
Justification:
The PR is technically impressive in its "band-aid" solutions (digest checks, security contexts), but it is built upon a fragile foundation (mutable tags, cluster-admin CI). To move this to a production-ready state, the "compensating controls" must be replaced with "preventative controls."
Required Actions for Approval:
rollout restart).cluster-admin).NodePortwithClusterIP+Ingress.The review flagged `:latest` as a critical issue, and it is right: a mutable tag makes `kubectl rollout undo` meaningless and lets different pods in the same deployment run different code if they restart at different times. - The build job now pushes the image with BOTH `:latest` and `:${{ github.sha }}`. - The deployment manifest carries a sentinel tag rather than `:latest`, and the deploy job sets the real SHA with `kubectl set image`. That makes the deploy a genuine manifest change, so the Deployment rolls because its pod template changed — not because it was told to. `kubectl rollout undo` now means something: it goes back to the previous SHA. - The digest check now compares against the SHA tag rather than `:latest`, and it checks EVERY running pod instead of `items[0]`. During a rolling update `items[0]` may still be an old pod that has not been terminated, which would compare against the wrong digest. The job also fails if the deployment is not fully rolled out. - The smoke test now checks the app through the Service as well as directly in the pod. The pod check alone would pass even if the Service selector or the port mapping were wrong. - `readOnlyRootFilesystem: true` — the app writes no files, so the root filesystem can be read-only. /tmp is still writable via the emptyDir. - The kubectl install is skipped when the runner already has one.Round 1 review findings
All the substantive ones are fixed. Itemised below.
Critical
Mutable image tags (
:latest) - FixedThe review is right, and this was the most important finding.
:latestand:${{ github.sha }}.:latest, and the deploy job sets the real SHA withkubectl set image. That makes the deploy a genuine manifest change, so the Deployment rolls because its pod template changed — not because it was told to.kubectl rollout undonow means something: it goes back to the previous SHA.:latest.This also removes the need for the
rollout restarthack entirely — the earlierkubectl rollout restartexisted only because an immutable:lateststring meansapplynever changes the pod template.High
Brittle verification logic - Fixed
The digest check looked at
items[0], which during a rolling update may still be an old pod that has not been terminated — so it would compare against the wrong digest. It now checks every running pod (using astatus.phase=Runningfield selector), and it also fails if the deployment is not fully rolled out.Over-privileged CI credentials - Deferred, operator-gated
KUBEC_CONFIG_BUILDXis cluster-admin-equivalent. Scoping it to aServiceAccountlimited to thebeernamespace means minting a new kubeconfig with a different identity, which is a cluster-side operation that needs your sign-off — and the existing secret is what every other workflow in this Gitea instance uses. It is called out in the RUNBOOK as the obvious next hardening step.Medium
Imperative vs declarative deployment - Fixed, as above: the SHA-pinned
kubectl set imagemakes the rollout declarative.Manual secret provisioning - Deferred, operator-gated
The
beer-dbandbeer-appSecrets are hand-provisioned. Moving to SealedSecrets means adding the controller to the cluster — a cluster-side change for you to approve. The important part is that they are outside the apply set, so a deploy cannot clobber them, and the RUNBOOK documents how to read and rotate them.Potential secret leakage - Fixed
The registry password is passed via
-uon the curl command line, where it is visible in the process list. It now goes through a curl config file on stdin instead, and the kubeconfig file is written with restrictive permissions and removed at the end of the job.Low
CI efficiency - Fixed
The kubectl install is skipped when the runner already has one.
Smoke test methodology - Fixed
The smoke test now checks the app through the Service as well as directly in the pod. The pod check alone would pass even if the Service selector or the port mapping were wrong — which is exactly the kind of breakage a deploy can introduce silently.
Missing resource constraints - Not applicable
The manifest has had
requests(50m CPU / 64Mi) andlimits(200m CPU / 256Mi) since it was written. The review appears to have missed them.Two more from the round-1 security review
readOnlyRootFilesystem- Fixed. The app reads its own source and talks to Mongo over the network; it writes no files./tmpis still writable via anemptyDir.object-src 'none'- Added to the CSP in the app.Test plan
test→build→deploykubectl set imagewith a SHA-pinned tag was tested against the live cluster and rolled the deployment/healthzand/readyzreturn 200 from the running podmaster, so it will run when this merges. That is the real test — I will report the outcome, and the SHA tag will be created by that run.One thing I got wrong, and fixed
While testing the SHA pinning I pointed the live deployment at a SHA tag that did not exist yet — the image had been built before this change, so only
:latestwas in the registry. The pods went toImagePullBackOff. I reverted to:latestimmediately and the deployment is healthy again. The SHA tag will exist after the first CI run that includes this change, which is what the deploy job consumes.