-
Notifications
You must be signed in to change notification settings - Fork 22
feat: user-defined major Node.js version #1727
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces a new node_major_version variable that enables automatic installation of the latest patch version for a specified major Node.js release. The change simplifies Node.js version management by eliminating the need to manually specify full version numbers and ensures teams always use the latest security patches.
- Added
node_major_versionvariable to fetch and install the latest patch version for a given major release - Updated default
node_versionfrom4.xto24.x - Reorganized documentation to clearly distinguish deprecated approaches from the new recommended method
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| roles/node/tasks/main.yml | Added tasks to fetch the latest version from nodejs.org when node_major_version is specified and updated conditions to handle the new variable |
| roles/node/defaults/main.yml | Updated default node_version from 4.x to 24.x |
| roles/node/README.md | Documented the new node_major_version variable and reorganized documentation to mark node_full_version and node_version as deprecated |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
0a99a41 to
3b0484f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
3ec9898 to
38eacc4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| src: "/tmp/node-v{{ node_full_version }}-linux-{{ architecture }}.tar.gz" | ||
| dest: /usr/local | ||
| remote_src: yes | ||
| remote_src: no |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The remote_src parameter is set to no, but the source is a local file /tmp/node-v{{ node_full_version }}-linux-{{ architecture }}.tar.gz on the target machine. For unarchive module with local files on the remote machine, remote_src should be yes. Setting it to no means Ansible will look for the file on the control machine.
| remote_src: no | |
| remote_src: yes |
| **DEPRECATED**: Use `node_major_version` instead to automatically receive security patches within a major version. | ||
|
|
||
| If you need a specific full version of Node.js (e.g., for reproducible builds), you can specify the exact version. This uses the same installation method as `node_major_version` but skips the automatic version lookup. |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The documentation marks node_full_version as DEPRECATED, but the code still fully supports it as an alternative to node_major_version. This creates confusion about whether users should avoid using it. Consider marking it as 'NOT RECOMMENDED' or 'Alternative approach' rather than DEPRECATED, or clarify that it remains supported for users who need version pinning.
| **DEPRECATED**: Use `node_major_version` instead to automatically receive security patches within a major version. | |
| If you need a specific full version of Node.js (e.g., for reproducible builds), you can specify the exact version. This uses the same installation method as `node_major_version` but skips the automatic version lookup. | |
| **NOT RECOMMENDED**: Use `node_major_version` instead to automatically receive security patches within a major version. | |
| If you need to pin a specific full version of Node.js (e.g., for reproducible builds), you can specify the exact version using `node_full_version`. This variable remains fully supported, but is not recommended for most users as it will not receive automatic updates or security patches. |
| - name: Store node path | ||
| set_fact: | ||
| node_location: /usr/local/{{archive_contents.files[0]}}bin | ||
| node_location: "/usr/local/node-v{{ node_full_version }}-linux-{{ architecture }}/bin" |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The node_location is now hardcoded instead of using archive_contents.files[0]. While this makes the path predictable, it assumes the tarball structure matches this exact pattern. If nodejs.org changes their tarball naming or structure, this will break. Consider adding a comment explaining this assumption or validating that the extracted directory matches the expected name.
What does this change?
node_major_versionto automatically fetch and install the latest patch version for a given major release.node_full_versionbased on the latest available patch version whennode_major_versionis provided.node_version) were removed fromtasks/main.yml, and the default variable was deleted fromdefaults/main.yml.roles/node/README.mdto document the newnode_major_versionvariable and clarify deprecated/removed variable options.node_major_versionfrom nodejs.org, checks for the existence of the requested tarball, and provides clear error messages if the version or architecture is unavailable.How to test
Ran
./multipass/run.shwithin
playbook-custom.yamland observed the tests pass. Also ran withnode_full_version: 20.11.0, no version variable, andnode_version=20.xsuccessfully.Tested the x64 architecture using a docker container with various permutations as above.
What is the value of this?
This should make it much easier for teams to update the Node.js version in their recipes, and also ensures that they are always using the latest patch version so security is improved.
Have we considered potential risks?
We are losing specificity of full versions in favour of ensuring that security patches within a major version are applied to a bake. Users can still, however, specify the full version if required for their use case.