feat: implement high-priority platform engineering improvements#8
Conversation
tukue
commented
May 29, 2026
tukue
commented
- Fix make deploy to use GitOps (remove imperative helm template | kubectl apply)
- Move legacy/ to archive/ and update all references
- Add PodDisruptionBudget template with Helm unit tests
- Add ResourceQuota and RBAC for stage and prod namespaces
- Add CI guard rejecting 'latest' image tags and 'HEAD' revisions
- Parametrize AWS account ID across all values files
- Fix make deploy to use GitOps (remove imperative helm template | kubectl apply) - Move legacy/ to archive/ and update all references - Add PodDisruptionBudget template with Helm unit tests - Add ResourceQuota and RBAC for stage and prod namespaces - Add CI guard rejecting 'latest' image tags and 'HEAD' revisions - Parametrize AWS account ID across all values files
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary
This PR implements important platform engineering improvements including GitOps adoption, PodDisruptionBudget support, ResourceQuota/RBAC for environments, and immutable tag enforcement. However, there are critical defects that will prevent deployments from working.
Critical Issues (Must Fix Before Merge)
-
Missing Image Repository Configuration - The
image.repositoryfield was removed from all 6 environment-specific values files (dev/stage/prod for both simple-app and app-b), but not replaced. This will cause Helm template rendering to produce invalid image references like:1.0.0instead of944684220857.dkr.ecr.eu-north-1.amazonaws.com/app:1.0.0, breaking all deployments. -
PodDisruptionBudget Validation Error - The new PDB template allows both
minAvailableandmaxUnavailableto be undefined, which violates Kubernetes API requirements. The API server will reject PDBs with neither field set.
Positive Changes
The PR successfully implements:
- GitOps workflow by removing imperative kubectl commands from Makefile
- CI validation for immutable tags and revisions
- ResourceQuota and RBAC configurations for stage and prod namespaces
- PodDisruptionBudget template with Helm unit tests
- Legacy code archival
Action Required: Address the critical image repository and PDB validation issues before merging to prevent production deployment failures.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
| image: | ||
| repository: 944684220857.dkr.ecr.eu-north-1.amazonaws.com/app | ||
| tag: "1.0.0-dev.1" |
There was a problem hiding this comment.
🛑 Deployment Failure Risk: Removing image.repository without replacement will cause Helm template rendering to fail. The deployment template (line 45) expects .Values.image.repository to construct the full image reference. With this field removed and the base values.yaml using an empty string, the rendered image will be invalid (e.g., :1.0.0-dev.1 instead of <account>.dkr.ecr.<region>.amazonaws.com/app:1.0.0-dev.1).
| image: | |
| repository: 944684220857.dkr.ecr.eu-north-1.amazonaws.com/app | |
| tag: "1.0.0-dev.1" | |
| image: | |
| repository: 944684220857.dkr.ecr.eu-north-1.amazonaws.com/app | |
| tag: "1.0.0-dev.1" |
| image: | ||
| repository: 944684220857.dkr.ecr.eu-north-1.amazonaws.com/app | ||
| tag: "1.0.0-rc1" |
There was a problem hiding this comment.
🛑 Deployment Failure Risk: Missing image.repository field will cause deployment failures. Add the repository configuration to ensure proper image reference.
| image: | |
| repository: 944684220857.dkr.ecr.eu-north-1.amazonaws.com/app | |
| tag: "1.0.0-rc1" | |
| image: | |
| repository: 944684220857.dkr.ecr.eu-north-1.amazonaws.com/app | |
| tag: "1.0.0-rc1" |
| image: | ||
| repository: 944684220857.dkr.ecr.eu-north-1.amazonaws.com/app | ||
| tag: "1.0.0" |
There was a problem hiding this comment.
🛑 Deployment Failure Risk: Missing image.repository field will cause deployment failures. Add the repository configuration to ensure proper image reference.
| image: | |
| repository: 944684220857.dkr.ecr.eu-north-1.amazonaws.com/app | |
| tag: "1.0.0" | |
| image: | |
| repository: 944684220857.dkr.ecr.eu-north-1.amazonaws.com/app | |
| tag: "1.0.0" |
| spec: | ||
| {{- with .Values.podDisruptionBudget.minAvailable }} | ||
| minAvailable: {{ . }} | ||
| {{- end }} | ||
| {{- with .Values.podDisruptionBudget.maxUnavailable }} | ||
| maxUnavailable: {{ . }} | ||
| {{- end }} | ||
| selector: |
There was a problem hiding this comment.
🛑 Logic Error: PodDisruptionBudget allows both minAvailable and maxUnavailable to be undefined simultaneously. According to Kubernetes API validation, at least one of these fields must be specified. When both are empty, the PDB will be rejected by the API server with an error.
| spec: | |
| {{- with .Values.podDisruptionBudget.minAvailable }} | |
| minAvailable: {{ . }} | |
| {{- end }} | |
| {{- with .Values.podDisruptionBudget.maxUnavailable }} | |
| maxUnavailable: {{ . }} | |
| {{- end }} | |
| selector: | |
| spec: | |
| {{- if .Values.podDisruptionBudget.minAvailable }} | |
| minAvailable: {{ .Values.podDisruptionBudget.minAvailable }} | |
| {{- else if .Values.podDisruptionBudget.maxUnavailable }} | |
| maxUnavailable: {{ .Values.podDisruptionBudget.maxUnavailable }} | |
| {{- else }} | |
| {{- fail "podDisruptionBudget.minAvailable or podDisruptionBudget.maxUnavailable must be set when podDisruptionBudget.enabled is true" }} | |
| {{- end }} | |
| selector: |
| image: | ||
| repository: 944684220857.dkr.ecr.eu-north-1.amazonaws.com/app | ||
| tag: "2.0.0-dev.1" |
There was a problem hiding this comment.
🛑 Deployment Failure Risk: Missing image.repository field will cause deployment failures. Add the repository configuration to ensure proper image reference.
| image: | |
| repository: 944684220857.dkr.ecr.eu-north-1.amazonaws.com/app | |
| tag: "2.0.0-dev.1" | |
| image: | |
| repository: 944684220857.dkr.ecr.eu-north-1.amazonaws.com/app | |
| tag: "2.0.0-dev.1" |
| image: | ||
| repository: 944684220857.dkr.ecr.eu-north-1.amazonaws.com/app | ||
| tag: "2.0.0-rc1" |
There was a problem hiding this comment.
🛑 Deployment Failure Risk: Missing image.repository field will cause deployment failures. Add the repository configuration to ensure proper image reference.
| image: | |
| repository: 944684220857.dkr.ecr.eu-north-1.amazonaws.com/app | |
| tag: "2.0.0-rc1" | |
| image: | |
| repository: 944684220857.dkr.ecr.eu-north-1.amazonaws.com/app | |
| tag: "2.0.0-rc1" |
| image: | ||
| repository: 944684220857.dkr.ecr.eu-north-1.amazonaws.com/app | ||
| tag: "2.0.0" |
There was a problem hiding this comment.
🛑 Deployment Failure Risk: Missing image.repository field will cause deployment failures. Add the repository configuration to ensure proper image reference.
| image: | |
| repository: 944684220857.dkr.ecr.eu-north-1.amazonaws.com/app | |
| tag: "2.0.0" | |
| image: | |
| repository: 944684220857.dkr.ecr.eu-north-1.amazonaws.com/app | |
| tag: "2.0.0" |
- Restore working ECR URL as chart default image.repository (configurable via Makefile AWS_ACCOUNT_ID and AWS_REGION variables) - Add Helm fail guard to PDB template when both minAvailable and maxUnavailable are unset (required by Kubernetes API validation)
GitHub API rate limiting causes the dynamic version fetch to return null, resulting in a 404 download. Pin to v1.18.0 as fallback.
- Rename all template include names (my-app.xxx -> app.xxx) - Change test-connection image from busybox to alpine:3.19 - Remove hardcoded image.repository from chart default (now set via --set) - Add --set image.repository to all helm template commands in Makefile and CI
1 participant
