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?
|
|
| }; | ||
|
|
||
| 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? |
One option is https://github.com/github/super-linter |
Probably not |
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 |
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. |
|
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 |
|
Updating the keys is fine, as that might actually be the ones being used by the person cutting the security release |
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? |
| 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
We've actually started doing that in a few repos in |
|
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. |
|
Might be a good idea to update this after #1646 lands |
|
@ttshivers hey, would you be up for continuing this? 🙂 |
| }); | ||
|
|
||
| const fetchText = (url) => new Promise((resolve, reject) => { | ||
| https.get(url, (res) => { |
|
yarn v1 version (which is what we ship) will never change, so we can drop the |
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