Ap 226: Update Framework's Dockerfile to use the most recent Node LTS #15
Conversation
awilfox
left a comment
awilfox
left a comment
There was a problem hiding this comment.
This is a refreshingly small change to be able to upgrade this far up - I'm quite happy to see that!
I do have a few questions about the changes before approving it.
Dockerfile
Outdated
| curl \ | ||
| python3 | ||
|
|
||
| RUN apt-mark manual python3.13-minimal \ |
There was a problem hiding this comment.
Do we want to hardcode the version? (Is that what the new Node depends on instead of just '3'?)
And is it intentional that we aren't auto removing python3(.13) any more?
There was a problem hiding this comment.
There are multiple Python versions installed. I kept python3.13-minimal, which works correctly. Autoremoving python3 triggers removal of Node.js dependencies, which causes failures in the production stage when yarn install runs
There was a problem hiding this comment.
If we aren't auto-removing Python, then IMO we don't really need to mark anything manual.
But I think it'd be better to mark the real dependencies as dependencies. This isn't just a size concern; having extraneous dependencies can allow attackers "chain" attacks (i.e. if they can execute limited code, but it includes Python code, and there's a further CVE in Python, they can use that) so it is a security best-practice to only include what we need.
If we keep python3.13-minimal and auto remove python3, what other dependencies does Node need? If it's just a few, we might be able to figure out something out.
If you like, I can toy around with this on my computer after I finish my work with AP-270.
There was a problem hiding this comment.
Sure, Node runtime does not need Python, only the yarn installation needs Python. Let me take a look again too.
There was a problem hiding this comment.
Does yarn need Python anymore if it's being installed with corepack?
There was a problem hiding this comment.
Yes, the yarn install in production stage still need it.
There was a problem hiding this comment.
When python3 is removed, node.js becomes unavailable.
There was a problem hiding this comment.
For Node.js v24.13.0 (NodeSource/Debian slim), Python3 is a required dependency, which is why removing Python3 also remove Node.js. This differs from Node.js 16, where Python3 was not required. Considering the security concern raised by Anna, we should keep Python 3 installed here, so I removes the code to set 'python3.13-minimal' as manual one.
Also I would suggest to make a follow-up decision on how we want to manage those dependencies in long term:
Option A: switch to the official Node.js binary installation to avoid apt related dependencies like Python.
Option B: moving Node.js/Yarn installation from base stage to development or a build stage and copy related libraries into production stage. My testing show Node.js and Yarn still existed in production image even when yarn install is removed from production stage, we may revisit the need of yarn install in the production stage.
anarchivist
left a comment
anarchivist
left a comment
There was a problem hiding this comment.
my comments/questions are basically the same as @awilfox's - if we can resolve those, then i think this might be ready to go.
3 participants
Update Node and Yarn to the latest:
Node: v24.13.0
Yarn: 1.22.22