Problem/Motivation

At the moment, the process for keeping JavaScript dependencies up to date is informal and dependent on individual contributors ensuring that updates have been applied on time. Ideally we would have processes in place for getting notified of security vulnerabilities in the dependency tree, and a step to make sure that dependencies get updated prior to every release to make (patch, minor and major). This would make sure that if an individual contributor is not available, the team would still be responsible for ensuring that updates have been applied on time.

For context, the total dependency tree at the moment is over 3000 packages meaning that updates are happening at a high frequency. Preparing to a new release should likely include multiple check points where lates updates get applied.

Proposed resolution

There are 2 topics, security warnings and automated dependency MR creation.

Security monitoring

This is fairly easy, a simple gitlabci file such as https://git.drupalcode.org/project/drupal_core_gitlabci_test/-/blob/10.0... will monitor the (JS & PHP) dependencies for security updates and integrates with the gitlab UI : https://git.drupalcode.org/project/drupal_core_gitlabci_test/-/security/.... We can schedule a daily scan for this and I'm sure there are gitlab settings for sending emails or such when a new issues is picked up.

Automated update MR

The principle for the various tools that exist is to make one PR for each package update. The only groupings that would make sense for us are doing all CKEditor 5 packages updates (execpt the @ckeditor/ckeditor5-dev-utils) in the same MR and @bable/core and @babel/preset-env in the same MR since they're released in sync. All other dependencies should have their own MR.

Detect updates

First step is to detect packages updates, this can be done by dependabot or renovate.

This should be run on all supported branches. One package update in 3 branches would create 3 different merge requests.

Prepare the MR

Once we know what there is to update we need to

  1. create a d.o issue (template TBD)
  2. add all relevant core committers as followers of the issue
  3. create the issue fork
  4. create a new branch dep-update-XXX (with XXX the name of the dependency to update)
  5. commit the result of dependabot/renovate to this branch
Update Drupal code
  1. create a new branch git checkout -b dep-update-drupal-XXX dep-update-XXX
  2. run yarn
  3. run yarn build
  4. Depending on the updated package(s) run different commands:
    • if package is used in vendor-update run yarn vendor-update
    • if package is part of CKEditor 5 run yarn run build:ckeditor5-types (yarn build:ckeditor5 is already run by yarn build above
    • if package is cspell run yarn spellcheck:make-drupal-dict
  5. commit the code to the branch
  6. create the merge request against the appropriate core branch
  7. set the issue as Need Review so that testbot is run against the new code (triggering the various linting scripts and build checks)
CKEditor 5 changes review

When the update package is CKEditor 5 or webpack* in the dep-update-cke5 branch run the following commands:

    1. create a new (ckeditor5-build) branch from the core branch
    2. run yarn
    3. run yarn build:ckeditor5-dev
    4. commit the result
    1. create a new branch using the updated deps git checkout -b review-ckeditor5-build dep-update-XXX
    2. run yarn
    3. run yarn build:ckeditor5-dev
    4. commit the result
  3. Create a DRAFT merge request from review-ckeditor5-build to ckeditor5-build to review the unminified changes (so that CI doesn't run for this MR)

The various steps involves creating 4 branches max:

  1. dep-update-XXX
  2. dep-update-drupal-XXX
  3. ckeditor5-build
  4. review-ckeditor5-build

And 2 merge requests:

  1. from dep-update-drupal-XXX to core branch (the MR to commit)
  2. a DRAFT MR from review-ckeditor5-build to ckeditor5-build for review

Remaining tasks

Agree on the proposal

Release notes snippet

CommentFileSizeAuthor
node-modules-meme.jpeg47.51 KBlauriii

Issue fork drupal-3280275

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

  • 3280275-set-up Comparecompare
About issue forks

Comments

lauriii created an issue. See original summary.

nod_’s picture

nod_
Primary language French
Location Lille
commented

We could try to have a bot open merge requests automatically on all supported branches to update the minor/patch version automatically. That would trigger CI and tests without human intervention

xjm’s picture

xjm
she/her
Primary language English
commented

It'd be a bit more complicated than essentially "Dependabot for GitLab" because we'd have to run all our build steps afterward, but we still could add a lot of automation that would make things easier. The CI to generate the MR would:

  1. Always do: rm -rf node_modules; yarn install; yarn upgrade; yarn vendor-update; yarn build; yarn lint:css; yarn lint:core-js-passing
  2. If cspell is among the versions changed in the lockfile, run: yarn spellcheck:make-drupal-dict
  3. Maybe: If the build steps result in changes to CKEditor assets, run: yarn run build:ckeditor5; yarn run build:ckeditor5-type. (Do we need this step, or is only necessary when updating CKE5 itself?)
  4. For extra robot credit, create us a separate review diff MR following the process for reviewing minified CKE5 assets.
  5. Allow subscribing via email as test results do and display the MR somwhere prominent, like auto-follow it for the committers and/or have a dashboard block for it and/or display it in a block on node/3060/qa. Maybe all three.
  6. For extra robot credit, run a yarn-lock-diff of the updated branch against HEAD, and highlight major and minor updates that are happening for the reviewer to look over.

Then the reviewer just has to look over the changes to the built assets, figure out why they are happening, and verify that they are good changes for the branch in question (not breaking changes or supply chain attacks or anything else).

Automating all that would save a ton of time because it takes like 15+ min on my machine when the cspell dictionary gets rebuilt, and that for five branches plus the basic mechanics of patch creation and upload leads to two solid hours just to post the results of automated tools.

nod_’s picture

nod_
Primary language French
Location Lille
commented

to get notified of security issues we can rely on gitlabci as in : https://git.drupalcode.org/project/drupal_core_gitlabci_test/-/security/...

nod_’s picture

nod_
Primary language French
Location Lille
commented
Issue summary: View changes

updated IS with proposed steps

nod_’s picture

nod_
Primary language French
Location Lille
commented
Issue summary: View changes
nod_’s picture

nod_
Primary language French
Location Lille
commented
Issue summary: View changes
Status: Active » Needs review

I'm not sure we need yarn-lock-diff in that situation. The principle of the various auto-updates bots are to make one MR for each package updated and they include the version changes in the MR summary (as well as the changelogs) see the examples MR in the issue summary.

nod_’s picture

nod_
Primary language French
Location Lille
commented
Issue summary: View changes
xjm’s picture

xjm
she/her
Primary language English
commented

Thanks @nod_ for your work on this!
https://git.drupalcode.org/project/drupal_core_gitlabci_test/-/security/vulnerability_report is 404 for me. This is presumably due to one of:

  • There not being any current vulnerabilities (possible -- yarn audit is clean locally atm), or
  • Me not having necessary permissions on the test project.

The proposal looks pretty solid to me. One additional feature I'd request under the "Prepare the MR" section is email notifications beyond what following the issue provides. I don't subscribe to emails for issues I follow nor necessarily find them meaningfully in my tracker; as a committer I comment on too many issues for this to work for me (even though I've tried to develop a habit of habitually unfollowing things). I assume the DA could hook us up with an email subscription option somehow as part of the process.

Everything else makes perfect sense as outlined.

I have no strong opinions on dependabot vs. renovate; I'm familiar with dependabot notifications on GH projects but renovate does look pretty nice.

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented

This overall gets a happy +1 from me. These type of issues get tedious, probably because the majority of the work is literally something a machine could do... so I'm pleased to see a machine doing it.

A few thoughts

  • Based on the issue summary, it sounds like an MR is created, then an issue. I'm not familiar with how an issue associates itself with an existing MR. All my experience has been issue first, generate fork via issue, then create an MR from a branch in that fork. There's a good chance this is functionality I'm unaware of, but if there are docs regarding how that part works I'd like to check them out.
  • Any benefit in including Slack notifications with the creation of the MR/issues? Having it in Drupal Slack could help make these visible somewhere we haven't already hit notification saturation. If it's evident in the audit that an update addresses a security issue those notifications could potentially be suppressed, too (at that point it would be public information, but I could imagine being reluctant to broadcast a newly discovered vulnerability).
  • If there are any parts of this that are harder to iterate on, such as things that require more D.O. integration, I'd like to see those steps mentioned in the IS. Overall I'm of the mind that this is a big improvement and would like it sooner than later, even if it means having to tweak things a few times before it's ideal. If we know which steps may take longer, due to needing consensus, not a priority for the few people that can do it, etc. those should obviously get vetted a little more
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented

Great proposal @nod_! 👏

My main concern is with how we make sure someone takes an action on the issue when update has happened. So far, most of the work has been coordinated by a human pinging people directly for a review. If we are moving that to CI, there needs to be some automation in the sense that some folks get notified that their attention is needed. Getting an email from issue being updated is a good start but maybe we might have to figure out if we could do something like assigning the issue automatically to a person for review and sending them a special email to notify them.

From a technical perspective I'm a bit concerned if we can associate the PR's with a Drupal.org issue. I think that would be something worth testing before putting too much effort to this.

nod_’s picture

nod_
Primary language French
Location Lille
commented

I don't expect renovate/dependabot to create the MR used later in the d.o issue.

We're going a bit much into the implementation part of things but what I had in mind was that we run them periodically "somewhere" and when there are things to update we take the diff from the commit and/or the MR description and commit those to the d.o issue fork. We can create a dummy project on gitlab that's a mirror of core repo. That project is only used to run the bots and there they'll have permission to create branches and MR.

We can set up webhooks to monitor new MR created on that project and trigger the creation of the d.o issue when a new one is created. It'll be a matter of getting the diff and MR description to copy in the d.o issue fork.

nod_’s picture

nod_
Primary language French
Location Lille
commented

A topic still to be defined:

When a dependency release a new version when the previous version is still not committed, what happens? do we update the existing issue/mr, close and create a new one, or something else? (That would be the case if a dependency release a version then the next day releases another for some reason).

hestenet’s picture

hestenet
He/Him
Location Portland, OR 🇺🇸
commented

Per DA Staff and core discussion:

However, we are not close to fully support Core GitLabCI yet and there are several potential challenges:

  • limiting the visibility of the MR
  • showing the MR to security team members
  • notifications of the MR's existence beyond normal notifications
  • k8s cluster for GitLabCI needs to be in good shape (it's still losing track of it's nodes)
  • Security team workflows need to be sorted out

There are some more limited scope considerations:

  • Could start off with just dependency scanning if we had an understanding of controlling visibility.
bbrala’s picture

bbrala
Location Netherlands
commented

Just gonna pitch in here, since i was directed here by @nod_ when posting that we should just run renovate on Gitlab for core.

Seems like automating at least getting new dependencies in is a great QOL improvement. Having updated dependencies ready and tested automatically is a great way to make life easier. We do this (and more) in our company also, feels great to only need to merge to update.

Regarding the open issue in #13.

I'd say that an task like this should only have one MR open. I'd also prefer to have it update the same MR, since that will also generate possible history. Perhaps a few versions have been skipped since last update and something broke along the way, that is information you'd want to have when evaluating a MR like that.

bbrala’s picture

bbrala
Location Netherlands
commented

Regarding #12. Think Gitlab is pretty stable by now because the work of the infra team.

xjm’s picture

xjm
she/her
Primary language English
commented

@bbrala, there are still lots of outstanding things for the GitLab migration. :)

smustgrave’s picture

smustgrave commented
Category: Task » Plan

This seems more like a plan.

Version: 10.0.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

xjm’s picture

xjm
she/her
Primary language English
commented

Amending attribution.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

smustgrave’s picture

smustgrave commented

Does this related to our new dependency coordinator role #3563613: Define core gate for 'dependency coordination' topic ?