Closed janbrasna closed 1 month ago
Should node-version
be 22
instead of 20
?
Otherwise LGTM. Should the current manual process be documented as having been supplanted by this github action?
Merging to facilitate deployments. Thanks, @janbrasna! My test system has node 20, and that is what I tested, so I am merging this as-is.
Should node-version
be 22
instead of 20
?
Should docs/
be removed from the repo? git rm -r docs
@gene1wood This will also need changing publishing source in repo settings (GitHub Pages › Build and deployment › Source › GitHub Action) to allow writing to the existing deployment environment instead of the current folder source. (When you're there, just check the custom domain is still set there correctly, as the new deployment doesn't use CNAME but API access for the value. Other than that we're using the same setup so environment protections should be empty or default which would work with this as well so they shouldn't need any tweaking…) — Thanks!
@gstrauss The NodeJS version used in automation shouldn't be 22 for now as it currently takes 3× longer to build compared to 16/18/20 (might be just a particular version/patch glitch that will get better over time but at the time of testing the current default npm version within node22 cached env in the action runners takes several times longer to resolve dependencies than the previous LTS). The stack comes from node12~ish epoch and only recently got upgraded for node16+ compatibility that luckily helps staying compatible with newer LTS versions too, so we have the sweet spot of conservative and recommended environment at Node 20 at this point. (I will be looking into node<18 bc-breaking upgrades in the coming months, as that might help fixing the npm resolve performance for the future versions.)
Removal of the committed artifacts in /docs will be part of some of the next steps, along with updates to readme et al., once the pipeline works to our liking for some time — it needs more clickops to enable so it's not entirely resolved upon merging this — hence why I plan to tackle that separately once it all works. (With more tweaks in configs, plugins etc. related to the change as well, simplifying some unnecessary things at that point to mere defaults for smaller config surface where not needed…)
https://mozilla.github.io/ssl-config-generator redirects to https://ssl-config.mozilla.org/
https://ssl-config.mozilla.org/ does not have recently committed changes, even though the comment on the pages appears to regenerate daily.
@gene1wood is there a job somewhere which builds and deploys to https://mozilla.github.io/ssl-config-generator on a daily basis? I did not see any existing .github/workflows or .git/hooks which would do this. Also, is there a repository setting which redirects github pages for ssl-config-generator to https://mozilla.github.io/ssl-config-generator?
@gstrauss That is the same deployment, the alias/redirection is a gh.io-internal feature [docs], you shouldn't ever see two different things. (If "comment on the page" is the build timestamp in footer, I only see the last d05284a hash built on 1/24, since then, only this same build is being pushed over and over even in recent directory deployments [log] which is intentional for now not messing with build output conflicts, it still deploys the /docs snapshot from that time until the publishing source is changed to the actions workflow.)
Which BTW may need manually enabling first by repo admin, and a version hotfix in the meantime too #254 >:§ (thought we're on deprecated upload-artifact
, not upload-pages-artifact
🤦 …)
FYI: this might be one cause of the slowdown:
npm warn deprecated core-js@2.6.12: core-js@<3.23.3 is no longer maintained and not recommended for usage due to the number of issues. Because of the V8 engine whims, feature detection in old core-js versions could cause a slowdown up to 100x even if nothing is polyfilled. Some versions have web compatibility issues. Please, upgrade your dependencies to the actual version of core-js.
@gstrauss This is around for a good part of the last decade of Node versions, and we're probably not hitting that transpilation issue. The build step seem ok, it's the CI installation that hangs (for basically the same result as the lower versions) — I will fiddle with that if it doesn't resolve itself over time, but it's not too big of a priority, as there's a lot of of the toolchain that needs upgrading with breaking changes first, to be more on the happy update path, to then look into ironing out the wrinkles with up and coming env versions. For now the 20 branch seems very stable for us, so the 22 performance is something to keep an eye on, but doesn't need much focus in the near future. — FYI we'll be moved from ubuntu-22 to ubuntu-24 runner within a week or two so the cached binaries will be a bit different over there, let's see how the matrix checks perform over there.
FYI: For now both deployments are active, the pre-built docs is faster so it deploys earlier, the fresh build follows it overwriting the old content only thanks to the duration of the build step (as these are disjunct triggers not sharing the same concurrency group), so as long as the old publishing source is still active in the admin (I'm not comfortable dropping the docs dir as that may lead to a whole bunch of other errors before disabling it in settings), we're sort of having this alternating deploys publishing both the old static and fresh push ref at the same time ;D but this should not cause any major trouble for the short term — just if you see anything weird, that might be the culprit;D
🚀 Changes are being published live, and the production environment is set there correctly (so we're not missing prod-level enforced CSP rules, live analytics, build date/revision stamp etc.) 🎉 Lovely.
(In case the old build gets stuck there from time to time, feel free to dispatch the workflow manually, it is set to allow manual runs for this exact purpose, if you need to deploy default branch HEAD — or any ref for that matter if you need to — without any push-based trigger per se.)
This will also need changing publishing source in repo settings (GitHub Pages › Build and deployment › Source › GitHub Action)
Done
@janbrasna Does it look correct now? Is there anything else needed in the repo settings that I should change?
Does it look correct now? Is there anything else needed in the repo settings that I should change?
There must a setting somewhere which requires Approval, which is separate from the number of approvals required. I can merge requests only after I assign a reviewer (which could be myself) and the reviewer approves it, even though 0 Approvals are required. This may be a privileging issue as @janbrasna and I are contributors and not admins of the repo.
@gstrauss I've unchecked
Require approval of the most recent reviewable push Whether the most recent reviewable push must be approved by someone other than the person who pushed it.
Does that change things?
So just on the off chance in that 5 minute window between when I posted my comment above and now, you tested, I hadn't saved the change that I described (it was waiting for my 2FA).
I've now confirmed the that the Require approval of the most recent reviewable push
is set. Does that change things?
Thanks, @gene1wood. I won't be able to test that specific things until one of us makes another push, but that sounds like the right setting!
...I think I was able to workaround that restriction by rebasing the branch on a different commit, but did not check carefully if that was a github bug which allowed the policy bypass.
Resolves #161
Publishes ssl-config-generator to production from master. No underlying git history and/or keeping a separate branch needed, artifacts are built and published straight to the deployment environment.
Compiles under 10sec, whole npm ci takes about ~30sec, all in all builds and deploys under 1 minute.