ci(update): port update.sh to nodejs#1368
Conversation
| @@ -0,0 +1,62 @@ | |||
| #!/usr/bin/env node | |||
There was a problem hiding this comment.
does this work on windows? requiring node update.js is probably better?
PeterDaveHello left a comment
PeterDaveHello left a comment There was a problem hiding this comment.
Let's also add some basic linter for it as we did for shell scripts before!?
SimenB commented Oct 15, 2020
|
nschonni left a comment
nschonni left a comment There was a problem hiding this comment.
Maybe I missed it, but I'm not seeing the a way to do the old -s security updates where only the Node version gets updated
| }; | ||
| const fetchMuslChecksum = async (nodeVersion) =>{ | ||
| const checksums = await fetchText( |
There was a problem hiding this comment.
Is there a way to handle/wait for the checksum to be available? Just thinking more in a CI way, for the auto-update PR flow
There was a problem hiding this comment.
If the checksum isn't available, an action using this should fail since fetchText will reject on non 2XX status codes. Is that sufficient? I could instead have it periodically poll that site but that would run into the action time limit of 6 hours. Any thoughts?
I didn't implement that (yet). I could add that if desired. I just have it regenerate the entire template when the node version changes by default (no args). Is there a case where you would only want to update the node version and not also update the yarn version and keys? Are there other situations like that where I should add more customization. For example, I didn't port the logic to specify certain versions or variants to only update. Should I also add this back in? |
ttshivers commented Oct 15, 2020
One option is https://github.com/github/super-linter |
SimenB commented Oct 15, 2020
Probably not |
nschonni commented Oct 15, 2020
Haven't used it before, but I think this is one repo were it actually makes sense 😄 because we don't pin/track versions of those tools |
ttshivers commented Oct 15, 2020
Okay, just so I understand: There should be an Should that be the default behavior? To only update the node version, and if there is a node version update, only update the node keys. |
nschonni commented Oct 16, 2020
I don't think it needs to be the default behaviour, we can just keep it for the cases where there is a security release. I believe they have the |
At the moment, it appears that the only thing the Lines 68 to 71 in 0e87209 Lines 138 to 142 in 0e87209 Note, setting the Line 158 in 0e87209 So, it appears that the current behavior of the |
nschonni commented Oct 16, 2020
Updating the keys is fine, as that might actually be the ones being used by the person cutting the security release |
ttshivers commented Oct 16, 2020
That does make sense. I am trying to think of the best way to handle that. Currently, my script takes the same approach as Any advice or other strategies? |
nschonni left a comment
nschonni left a comment There was a problem hiding this comment.
Sorry, not a solution, but took a quick read down the files and commented where the start of the Yarn/Security-only bubbling would need to start
| console.log(usage); | ||
| }; | ||
| const runUpdate = async (updateAll) =>{ |
There was a problem hiding this comment.
I think the Yarn part would be a separate parameter here, defaulting to true
| }; | ||
| const updateDockerfiles = async (outdated) =>{ | ||
| const yarnVersion = await fetchText(yarnVersionUrl); |
There was a problem hiding this comment.
I think here, if it is a security update, it would read the existing Dockerfile, instead of doing the fetch
yosifkit commented Oct 16, 2020
We've actually started doing that in a few repos in |
ttshivers commented Nov 15, 2020
Coming back around to work on this. I think I'm going to try out the json metadata approach that yousifkit mentioned. I know my current scripts lacked a security only mode, so I'm going to add that. I am trying to see if there is a good way to see if a given nodejs update is a security release or not.
I don't see a good way of determining if a yarn release is a security update or not. |
SimenB commented Mar 10, 2022
Might be a good idea to update this after #1646 lands |
SimenB commented May 15, 2022
@ttshivers hey, would you be up for continuing this? 🙂 |
| }); | ||
| const fetchText = (url) => new Promise((resolve, reject) =>{ | ||
| https.get(url, (res) =>{ |
SimenB commented Feb 17, 2023
yarn v1 version (which is what we ship) will never change, so we can drop the |
5 participants
Ported the update.sh script to nodejs.
Changes:
-aor--allcauses all Dockerfiles to be regenerated from the template.updateLib.jscan be used in the auto-pr cronjob to also get what Dockerfiles changed. By default, it will only update a Dockerfile when node updates, so it would work well in an cronjob action.Refs: #1314
diff