Conversation
There was a problem hiding this comment.
Pull request overview
Adds Lab 11 secret-management work on top of the existing Helm charts, including Kubernetes Secrets and optional HashiCorp Vault Agent Injector integration, plus documentation for Labs 10/11.
Changes:
- Introduces Helm-managed Secret + ServiceAccount templates and wires Secret injection into the main Deployment.
- Adds optional Vault Agent Injector annotations/config (role, secret path, template rendering to file).
- Adds/updates lab documentation and introduces a small Helm library chart (
common-lib) reused by two app charts.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| k8s/SECRETS.md | Lab 11 write-up covering K8s Secrets + Vault integration steps and chart usage |
| k8s/HELM.md | Lab 10 write-up documenting Helm chart structure and operations |
| k8s/devops-python-app/values.yaml | Adds secret/serviceAccount/vault configuration and keeps resource/probe defaults |
| k8s/devops-python-app/values-dev.yaml | Dev overrides for replicas/resources/probes/service |
| k8s/devops-python-app/values-prod.yaml | Prod overrides for image tag/resources/probes/service |
| k8s/devops-python-app/templates/serviceaccount.yaml | Adds ServiceAccount templating |
| k8s/devops-python-app/templates/secrets.yaml | Adds Helm-managed Secret templating |
| k8s/devops-python-app/templates/deployment.yaml | Adds secret envFrom and optional Vault injector annotations + common env helper |
| k8s/devops-python-app/templates/service.yaml | Helm Service template (noted labeling issue in review comments) |
| k8s/devops-python-app/templates/hooks/pre-install-job.yaml | Pre-install hook job template |
| k8s/devops-python-app/templates/hooks/post-install-job.yaml | Post-install hook job template |
| k8s/devops-python-app/templates/NOTES.txt | Helm post-install notes |
| k8s/devops-python-app/templates/_helpers.tpl | Adds wrapper helpers and a shared env snippet |
| k8s/devops-python-app/Chart.yaml | Chart metadata + dependency on common-lib |
| k8s/devops-python-app-v2/Chart.yaml | Second app chart metadata + dependency on common-lib |
| k8s/devops-python-app-v2/values.yaml | v2 chart values including explicit env list |
| k8s/devops-python-app-v2/templates/deployment.yaml | v2 deployment template (noted env rendering edge case) |
| k8s/devops-python-app-v2/templates/service.yaml | v2 service template (noted labeling issue in review comments) |
| k8s/devops-python-app-v2/templates/_helpers.tpl | v2 wrapper helpers |
| k8s/common-lib/Chart.yaml | Library chart metadata |
| k8s/common-lib/values.yaml | Minimal library defaults |
| k8s/common-lib/templates/_helpers.tpl | Shared naming/label helpers for both charts |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| labels: | ||
| {{- include "devops-python-app.labels" . | nindent 4 }} | ||
| app.kubernetes.io/component: service |
There was a problem hiding this comment.
Service metadata.labels renders app.kubernetes.io/component twice: once via devops-python-app.labels helper (set to api) and again as service. Duplicate keys can produce invalid/undefined YAML and inconsistent labeling. Consider removing the component label from the helper, or stop adding it here and set component labels per-resource without duplication.
| labels: | ||
| {{- include "devops-python-app-v2.labels" . | nindent 4 }} | ||
| app.kubernetes.io/component: service |
There was a problem hiding this comment.
Same duplicate-label issue as the primary chart: devops-python-app-v2.labels already emits app.kubernetes.io/component: api, and this Service adds app.kubernetes.io/component: service again. This yields duplicate YAML keys and unreliable labeling. Prefer setting component labels per resource (and keep the shared labels helper free of resource-specific component values).
| env: | ||
| {{- range .Values.env }} | ||
| - name: {{ .name }} | ||
| value: {{ .value | quote }} | ||
| {{- end }} |
There was a problem hiding this comment.
env: is always emitted, but the list items are generated via range. If a user overrides Values.env to an empty list, this renders as env: null (no items), which can fail Kubernetes schema validation. Consider rendering env via toYaml (so empty lists become []) or guarding the entire env: block with an if and providing a safe default.
| ```powershell | ||
| helm upgrade --install myapp-lab11 k8s/devops-python-app ` | ||
| --set-string secret.data.username="admin" ` | ||
| --set-string secret.data.password="secret123" ` | ||
| --set-string secret.data.api_key="demo-api-key" | ||
| ``` |
There was a problem hiding this comment.
The Helm example passes secrets via --set-string. In real clusters, Helm stores rendered manifests (including Secret data) in the release record (Secret/ConfigMap), so values can be retrievable by anyone with access to that release metadata. Consider adding a short note here clarifying this limitation and recommending Vault/ExternalSecrets (or similar) for production secret handling.
No description provided.