Skip to content

examples(airflow): fix SonarCloud alerts in airflow-k8s-pipeline manifests and Dockerfiles - #52

Open
devin-ai-integration[bot] wants to merge 2 commits into
developfrom
devin/1794421911-airflow-k8s-sonar-fixes
Open

devin-ai-integration[bot] wants to merge 2 commits into
developfrom
devin/1794421911-airflow-k8s-sonar-fixes

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor

Summary

This PR fixes the 11 SonarCloud code-scanning alerts on examples/airflow-k8s-pipeline/k8s/** (scan of develop at ca85348). Every alert gets a real fix; nothing is suppressed and none needs dismissing.

Alert Rule Fix
#45 S6865 minio automountServiceAccountToken: false on the pod spec
#50 S6865 rabbitmq same
#51 S6865 listener same. The listener only talks AMQP and the Airflow REST API, never the k8s API
#54 S6865 seed job same
#53 S6864 seed job resources added: requests 64Mi/50m, limits 128Mi/200m
#46 S6870 minio ephemeral-storage request 1Gi, limit 10Gi, plus emptyDir.sizeLimit: 10Gi on /data. A run leaves about 258 MiB, so the limit holds about 38 runs and uses half of minikube's default 20 GB disk
#52 S6506 worker curl --proto '=https' --tlsv1.2 for the AWS CLI bundle
#68 S6471 worker USER root is now only used for the install step. The image then useradds tig and ends with USER 1000:1000. The wrappers only write under /tmp/work and the calibration mount is read-only, so VICAR runs fine as non-root
#67 S6471 listener Adds a listener user and ends with USER 1000:1000
#48 S8544 listener New requirements.in (pika, requests) plus a requirements.txt locked with pip-compile --generate-hashes on Python 3.11; installed with --require-hashes
#49 S8541 listener pip install --only-binary :all: (every locked dependency has a cp311 or py3 wheel)

Two other changes:

  • RabbitMQ readiness probe: added timeoutSeconds: 5. Under the 500m CPU limit, rabbitmq-diagnostics check_port_connectivity takes about 1.3 s, longer than the 1 s default. Unmodified develop never became Ready on minikube either (tested), so this fix is needed to deploy at all.
  • README: the repo-tree listing now includes the two requirements files.

Verification (minikube, 4 CPU, README steps 1–12)

  • Both images build. id inside them gives uid 1000 (listener and tig), aws --version works in the worker, and pika 1.4.4 and requests 2.34.2 import.
  • In the listener, MinIO and RabbitMQ pods, /var/run/secrets/kubernetes.io/serviceaccount is absent, and spec.automountServiceAccountToken=false on all four pods.
  • The seed job completed and published 2 events. The listener matched the stereo pair and triggered ids_terrain_ncam.
  • All 8 tasks succeeded (rad, correlate, xyz, mesh for each eye) using the real sol-1835 FDR pair, with the worker running as non-root. All 10 outputs (.obj, .iv, .mtl, .png, .lbl for each eye) are under output/sol/01835/ids/rdr/ncam/.

Heads-up (not changed here, out of scope)

The pinned images minio/minio:RELEASE.2024-12-18T13-15-44Z and minio/mc:RELEASE.2024-11-21T17-21-54Z can no longer be pulled. Docker Hub says "repository does not exist", quay.io returns 401, and dl.min.io binaries return 410. To run the test above, I tagged Chainguard minio / minio-client (mc on a busybox base) under those names and loaded them into minikube. That test-only workaround is not committed. On a clean machine the example will not deploy until those images are replaced, which should be its own PR.

Link to Devin session: https://nasa-jpl-demo.devinenterprise.com/sessions/3946939957d5462eaa81505e727ac747
Open in Devin Desktop: https://nasa-jpl-demo.devinenterprise.com/desktop/session/3946939957d5462eaa81505e727ac747?variant=devin
Requested by: @HuiJun


🟢 Devin fixed the finding · Follow Devin's work

Devin Review

…ockerfiles

- Disable service-account token automount on listener, MinIO, RabbitMQ and seed pods
- Add seed job resource limits and MinIO ephemeral-storage limit (+ emptyDir sizeLimit)
- Run listener and tig-worker images as uid 1000
- Restrict the AWS CLI download to HTTPS/TLS1.2
- Install listener deps from a hash-locked requirements.txt, wheels only
- Give the RabbitMQ readiness probe a 5s timeout (CLI exceeds the 1s default)

Co-Authored-By: jason.han <hanhuijun@gmail.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

  • Disable automatic comment, CI, and merge conflict monitoring

… fit

Co-Authored-By: jason.han <hanhuijun@gmail.com>

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 finding. Devin automatically fixed it.

Fixed by Devin (1)
  • Repeated runs erase the demo bucket — Raised the MinIO ephemeral-storage limit and emptyDir sizeLimit from 4Gi to 10Gi (about 38 runs, half of minikube's default 20 GB disk). A limit is still needed for S6870, and the data in an emptyDir is already lost on any pod restart.

View findings in Devin Review · Follow Devin's work

Devin Review

@github-actions

Copy link
Copy Markdown

Project Requirements: PASS

PR #52 hardens the examples/airflow-k8s-pipeline example: hash-locked listener deps (requirements.in/.txt, --require-hashes --only-binary), non-root listener and worker images (uid 1000), automountServiceAccountToken: false on all pods, a longer RabbitMQ probe timeout, resource requests/limits on the seed job, and a 10Gi ephemeral-storage limit/emptyDir sizeLimit on MinIO. All changes stay inside examples/. Documentation parity: concern, because the README file tree lists the new requirements files but does not mention the new MinIO 10Gi storage cap, the disk this assumes, or the eviction/data-loss behavior when the cap is exceeded. Broad deployability: pass, since nothing is site-specific and the core CLI and image are untouched. Test coverage: pass, since this is example-only hardening covered by the documented manual end-to-end procedure (README steps plus .agents/skills/testing-airflow-k8s-pipeline). No blocking failures.

Requirement Verdict Notes
1. Documentation parity Concern These are example changes, so they count as user-facing. The README tree was updated for requirements.in/.txt. The new MinIO ephemeral-storage limit and emptyDir sizeLimit (10Gi), which assume minikube's default 20 GB disk, are not documented, and neither is what users see when the limit is exceeded. The non-root worker/listener change is not described either, though it changes no documented command.
2. Broad deployability Pass All changes are confined to examples/airflow-k8s-pipeline and are portable hardening: public PyPI wheels with hashes, generic non-root users, standard Kubernetes fields. No site-specific hosts, credentials, or runtimes are added, and the core CLI and image are untouched. The x86_64-only AWS CLI download in the worker Dockerfile already existed and is unchanged apart from added curl TLS flags. The 10Gi storage default is a sensible minikube default that users can override by editing the manifest.
3. Test coverage Pass No new tests are needed. This is example-only hardening (dependency locking, non-root users, probe timeout, resource limits) with no new capability or bug-fix logic in TIG code. The criteria's minimum for examples/** is met by the existing documented, reproducible manual end-to-end procedure (README setup/verify steps and .agents/skills/testing-airflow-k8s-pipeline/SKILL.md), which builds both images and runs the full DAG, so it exercises the non-root worker, hash-locked listener install, and MinIO limits. No CI workflow builds the example images; that is a pre-existing gap and not introduced here.

1. Documentation parity

  • examples/airflow-k8s-pipeline/k8s/minio/deployment.yaml:298: MinIO's /data emptyDir now has a 10Gi sizeLimit and ephemeral-storage limit. When the bucket goes over this (about 38 runs at ~260 MiB each), the kubelet evicts the MinIO pod and every object in the emptyDir is lost. The README does not mention the cap, the disk it assumes (the YAML comment cites minikube's default 20 GB disk, but step 1's minikube start does not set --disk-size), or the eviction symptom. Users who run the pipeline repeatedly will hit unexplained data loss. Fix: In examples/airflow-k8s-pipeline/README.md, state the 10Gi MinIO storage cap and the minikube disk it assumes in Implementation Notes (or Prerequisites/step 1). Add a Troubleshooting entry: 'MinIO pod Evicted / bucket emptied -> ephemeral-storage limit exceeded; delete old output/processque objects or raise sizeLimit/limits.ephemeral-storage (and minikube --disk-size)'.
  • examples/airflow-k8s-pipeline/k8s/worker/Dockerfile:368: The tig-worker and ids-listener images now run as uid 1000 instead of root. README step 4 still describes the worker only as 'TIG base + AWS CLI', so anyone who customizes the wrappers to write outside /tmp, or mounts a writable hostPath, will get permission errors with no documentation to explain them. Fix: Add a short Implementation Notes bullet to examples/airflow-k8s-pipeline/README.md: worker and listener containers run as unprivileged uid 1000, wrappers must write only under /tmp, and writable mounts need matching ownership or an fsGroup. Optionally mention how to regenerate requirements.txt (pip-compile --generate-hashes --strip-extras requirements.in on Python 3.11) next to step 3.

Evaluated at d6786b5 · Devin session · workflow run

Criteria: .github/project-requirements/criteria.md. A fail blocks the merge; a maintainer can waive it by adding the requirements-waived label.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant