potiuk commented on PR #71650:
URL: https://github.com/apache/airflow/pull/71650#issuecomment-5697201885

   Thank you for this — and I want to be clear up front that closing it is not 
a judgement on the work. I went through it properly before deciding, and it 
does what I asked for in my last review: the dual-base transition follows the 
shape of the previous Debian migrations, `ALLOWED_DEBIAN_VERSIONS` keeps 
Bookworm selectable, the changelog documents both the transition and the 3.5.0 
removal, and there are tests covering both bases. I also verified the Microsoft 
signing-key rotation against the vendor rather than taking it on trust — the 
key you added matches `https://packages.microsoft.com/keys/microsoft-2025.asc` 
exactly (`AA86F75E427A19DD33346403EE4D7792F748182B`), and Microsoft's own key 
README gives the reason, which is precisely our reason: the old `BE1229CF` key 
carries SHA1 signatures that newer Linux releases no longer accept. That was 
the highest-consequence line in the diff and it was right.
   
   ### Why I'm closing it anyway
   
   The base-image story has moved underneath this PR. We are switching the CI 
and PROD images to **Docker Hardened Images** first, and only then revisiting 
the Debian release. That work is in 
https://github.com/apache/airflow/pull/73038, with the background in 
https://github.com/apache/airflow/issues/59625.
   
   That changes the ground this PR is standing on. Once the images are based on 
a hardened Python image, we no longer compile CPython on top of 
`debian:bookworm-slim` at all — `install_python()`, the tarball download and 
the sigstore/GPG verification all go away, along with the fifteen apt packages 
that existed only to build CPython. Several of those are exactly the packages 
this PR is carefully making conditional per Debian release. So the 
`bookworm`/`trixie` branching added here to `Dockerfile`, `Dockerfile.ci` and 
`install_os_dependencies.sh` would have to be rewritten on top of a different 
base rather than merged and then adapted, and the two changes would conflict 
heavily in the meantime.
   
   Doing hardened first and Debian second is the order that costs the least 
total work — but it does mean this PR would be largely redone rather than 
rebased, which is not a fair thing to ask of you.
   
   ### What is worth keeping, and is independent of all of the above
   
   Several pieces here are not coupled to the base-image question at all and 
are useful on their own. If you would like to carry any of them forward as 
separate, much smaller PRs, they would be very welcome:
   
   - **The Microsoft signing-key rotation to the 2025 key.** Needed regardless 
of which base we end up on, for the SHA1 reason above. One request if you do: 
add a comment next to the key ID in `scripts/ci/prek/update_docker_gpg_keys.py` 
citing `https://packages.microsoft.com/keys/microsoft-2025.asc` and the full 
fingerprint. That hook fetches keys **by ID from public keyservers**, which 
accept uploads from anyone with any UID — so the hardcoded ID is the actual 
trust root and cannot validate itself. Recording where it came from makes it 
re-verifiable in seconds.
   - **The two provider dependency floors** — `xmlsec>=1.3.16` in 
`providers/amazon` and `ibm-db>=3.2.8` in `providers/ibm/db2`. Both are 
justified on their own terms and have nothing to do with the Debian base. Worth 
noting if you split them out: provider changelogs are regenerated from `git 
log` at release time rather than from newsfragments, so name the bumps in the 
commit message or they will be missed during release prep.
   - **The MariaDB 10.11 → 11.8 selection**, which will be needed by whichever 
base ships a newer Debian.
   - **The `openjdk-17` fix in the extending example.** One question there 
rather than a request: Trixie ships `openjdk-21-jre-headless`, so is 
`eclipse-temurin` needed because the pinned Spark provider requires 17 
specifically? If 21 works, staying on the distro package keeps our canonical 
example Debian-only; if 17 is genuinely required, a line in the example saying 
so would help, since readers copy these verbatim.
   
   Sorry to close a PR you have already revised once in response to my review. 
If you would like to pick the Debian work back up after the hardened images 
land, please do — and feel free to follow 
https://github.com/apache/airflow/pull/73038 to see when that is.
   
   ---
   Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to