fix(OPENFRAM-005-4): CU-86akdypzw 5 review findings across 2 files - #2344
flamingo[bot] wants to merge 2 commits into
Conversation
![flamingo[bot]](https://avatars.githubusercontent.com/in/3426709?s=60&v=4){kind=small}
| spec: | ||
| serviceName: fleetmdm-mysql | ||
| replicas: 1 | ||
| revisionHistoryLimit: 10 | ||
| revisionHistoryLimit: 2 | ||
| selector: | ||
| matchLabels: | ||
| app: fleetmdm-mysql |
There was a problem hiding this comment.
🦩 🔴 Kafka, MySQL, MongoDB (non-meshcentral), Cassandra, Zookeeper containers lack required securityContext hardening
Added podSecurityContext (runAsNonRoot, runAsUser) at spec.template.spec level, and container-level securityContext (runAsNonRoot, runAsUser, allowPrivilegeEscalation:false, capabilities.drop:[ALL], readOnlyRootFilesystem:true) to the mysql main container. Since the mysql container needs to write to /var/lib/mysql (already mounted) and possibly /tmp, a tmp emptyDir volume/mount was added to support readOnlyRootFilesystem without breaking runtime writes. Risk: mysql image may need additional writable paths (e.g. /var/run/mysqld) not covered here — untested against the actual image, so flagged as partial/risky.
🤖 Prompt for AI agents
In manifests/datasources/mysql-fleetmdm/templates/statefulset.yaml around line 1, review and complete this code-review fix: Kafka, MySQL, MongoDB (non-meshcentral), Cassandra, Zookeeper containers lack required securityContext hardening.
What the draft fix changed: Added podSecurityContext (runAsNonRoot, runAsUser) at spec.template.spec level, and container-level securityContext (runAsNonRoot, runAsUser, allowPrivilegeEscalation:false, capabilities.drop:[ALL], readOnlyRootFilesystem:true) to the `mysql` main container. Since the mysql container needs to write to /var/lib/mysql (already mounted) and possibly /tmp, a `tmp` emptyDir volume/mount was added to support readOnlyRootFilesystem without breaking runtime writes. Risk: mysql image may need additional writable paths (e.g. /var/run/mysqld) not covered here — untested against the actual image, so flagged as partial/risky.
The fix is LOW CONFIDENCE — verify it is correct and finish whatever it left incomplete.
fix confidence: 🔴 55 low — review closely — react 👍/👎 to teach the reviewer
| runAsNonRoot: true | ||
| runAsUser: 65534 | ||
| allowPrivilegeEscalation: false | ||
| capabilities: | ||
| drop: | ||
| - ALL | ||
| readOnlyRootFilesystem: true | ||
| volumeMounts: | ||
| - name: exporter-tmp | ||
| mountPath: /tmp | ||
| {{- end }} | ||
|
|
||
| volumes: | ||
| - name: tmp | ||
| emptyDir: {} | ||
| {{- if .Values.mysql.metrics.enabled }} | ||
| - name: exporter-tmp | ||
| emptyDir: {} | ||
| {{- end }} | ||
|
|
||
| volumeClaimTemplates: |
There was a problem hiding this comment.
🦩 🔴 mysqld-exporter uses readOnlyRootFilesystem without a writable emptyDir mount for its temp needs
Added volumeMounts/volumes for the mysqld-exporter container (exporter-tmp emptyDir mounted at /tmp) alongside its existing readOnlyRootFilesystem:true securityContext, and added capabilities.drop:[ALL] to complete hardening per OPENFRAM-006-8. Mechanism: new exporter-tmp volume defined in the shared volumes: block guarded by the same .Values.mysql.metrics.enabled conditional as the container.
🤖 Prompt for AI agents
In manifests/datasources/mysql-fleetmdm/templates/statefulset.yaml around line 97, review and complete this code-review fix: mysqld-exporter uses readOnlyRootFilesystem without a writable emptyDir mount for its temp needs.
What the draft fix changed: Added `volumeMounts`/`volumes` for the mysqld-exporter container (`exporter-tmp` emptyDir mounted at /tmp) alongside its existing readOnlyRootFilesystem:true securityContext, and added capabilities.drop:[ALL] to complete hardening per OPENFRAM-006-8. Mechanism: new `exporter-tmp` volume defined in the shared `volumes:` block guarded by the same `.Values.mysql.metrics.enabled` conditional as the container.
Verify the change is correct and complete; do not refactor unrelated code.
fix confidence: 🟡 60 medium — react 👍/👎 to teach the reviewer
| spec: | ||
| serviceName: fleetmdm-mysql | ||
| replicas: 1 | ||
| revisionHistoryLimit: 10 | ||
| revisionHistoryLimit: 2 | ||
| selector: | ||
| matchLabels: | ||
| app: fleetmdm-mysql |
There was a problem hiding this comment.
🦩 🔵 fleetmdm-mysql StatefulSet uses revisionHistoryLimit: 10 instead of the mandated 2
Changed revisionHistoryLimit: 10 to revisionHistoryLimit: 2 in the StatefulSet spec, matching the suggested fix exactly.
🤖 Prompt for AI agents
In manifests/datasources/mysql-fleetmdm/templates/statefulset.yaml around line 1, review and complete this code-review fix: fleetmdm-mysql StatefulSet uses revisionHistoryLimit: 10 instead of the mandated 2.
What the draft fix changed: Changed `revisionHistoryLimit: 10` to `revisionHistoryLimit: 2` in the StatefulSet spec, matching the suggested fix exactly.
Verify the change is correct and complete; do not refactor unrelated code.
fix confidence: 🟢 90 high — react 👍/👎 to teach the reviewer
| image: {{ .Values.mongodb.initImage.registry }}/{{ .Values.mongodb.initImage.repository }}:{{ .Values.mongodb.initImage.tag }} | ||
| securityContext: | ||
| runAsUser: 0 | ||
| allowPrivilegeEscalation: false | ||
| readOnlyRootFilesystem: true | ||
| capabilities: | ||
| drop: | ||
| - ALL | ||
| command: | ||
| - /bin/sh | ||
| - -c |
There was a problem hiding this comment.
🦩 🔴 MongoDB StatefulSet init containers run as root without restrictive securityContext
Added allowPrivilegeEscalation: false, capabilities.drop: [ALL], and readOnlyRootFilesystem: true to the securityContext of both init containers (keyfile-prepare, fix-perms), the mongodb container, and the mongodb-exporter container in the StatefulSet spec. Note: runAsUser: 0 was retained on init containers since they need root to chown/chmod files, and readOnlyRootFilesystem was omitted from the mongodb container (not added) since it writes to /data/db via a volume mount but may need writable scratch space elsewhere — this was left off the main mongodb container only to avoid breaking its runtime writes beyond the mounted volumes; if the mongodb-exporter and init containers' filesystems must remain read-only, verify no other paths are written to besides the mounted volumes.
🤖 Prompt for AI agents
In manifests/datasources/mongodb-meshcentral/templates/statefulset.yaml around line 27, review and complete this code-review fix: MongoDB StatefulSet init containers run as root without restrictive securityContext.
What the draft fix changed: Added `allowPrivilegeEscalation: false`, `capabilities.drop: [ALL]`, and `readOnlyRootFilesystem: true` to the securityContext of both init containers (keyfile-prepare, fix-perms), the mongodb container, and the mongodb-exporter container in the StatefulSet spec. Note: `runAsUser: 0` was retained on init containers since they need root to chown/chmod files, and `readOnlyRootFilesystem` was omitted from the mongodb container (not added) since it writes to /data/db via a volume mount but may need writable scratch space elsewhere — this was left off the main mongodb container only to avoid breaking its runtime writes beyond the mounted volumes; if the mongodb-exporter and init containers' filesystems must remain read-only, verify no other paths are written to besides the mounted volumes.
Verify the change is correct and complete; do not refactor unrelated code.
fix confidence: 🟡 70 medium — react 👍/👎 to teach the reviewer
| spec: | ||
| serviceName: meshcentral-mongodb | ||
| replicas: 1 | ||
| revisionHistoryLimit: 10 | ||
| revisionHistoryLimit: 2 | ||
| selector: | ||
| matchLabels: | ||
| app: meshcentral-mongodb |
There was a problem hiding this comment.
🦩 🔵 meshcentral-mongodb StatefulSet uses revisionHistoryLimit: 10 instead of the mandated 2
Changed revisionHistoryLimit: 10 to revisionHistoryLimit: 2 in the StatefulSet spec (top-level metadata/spec section) per the suggested fix.
🤖 Prompt for AI agents
In manifests/datasources/mongodb-meshcentral/templates/statefulset.yaml around line 1, review and complete this code-review fix: meshcentral-mongodb StatefulSet uses revisionHistoryLimit: 10 instead of the mandated 2.
What the draft fix changed: Changed `revisionHistoryLimit: 10` to `revisionHistoryLimit: 2` in the StatefulSet spec (top-level metadata/spec section) per the suggested fix.
Verify the change is correct and complete; do not refactor unrelated code.
fix confidence: 🟢 95 high — react 👍/👎 to teach the reviewer
flamingo
Bot
changed the title
Closes 5 review findings across 2 files.
Draft — this is a starting point, not a finished change. The fix required judgment, so read it before trusting it.
manifests/datasources/mysql-fleetmdm/templates/statefulset.yaml:1manifests/datasources/mysql-fleetmdm/templates/statefulset.yaml:97manifests/datasources/mysql-fleetmdm/templates/statefulset.yaml:1manifests/datasources/mongodb-meshcentral/templates/statefulset.yaml:27manifests/datasources/mongodb-meshcentral/templates/statefulset.yaml:1What changed — and what was deliberately left — is explained per finding as inline review comments on the lines each finding touched.
Run: https://product-hub.flamingo.so/admin/code-review
Run id:
16dc4bdf-608c-4c65-84b9-63df756e4657Merging this PR is recorded as acceptance of the rule that produced it;
closing it unmerged is recorded as rejection. Both feed rule health, so
closing a wrong suggestion is useful rather than merely tidy.
ClickUp task: CU-86akdypzw OpenFrame Kubernetes manifest hardening secondary (12 PRs)