Add explicit config for Dependabot #2062

benthorner · 2020-05-19T14:15:40Z

Previously we used the Dependabot UI to get top-level updates for
all gems in the Gemfile, which in turn leads to a lot of daily,
manual effort to review and deploy.

However, most of the gems we have in our repos contribute very little
to the codebase, and the risk of them being out-of-date is minimal,
provided we still get any security updates.

This adds an explicit config for Dependabot (to override the UI),
which specified the updates we actually want:

Security updates. We want to protect are apps the best we can.
Dependabot PRs can help to notify us about vulernabilities, and
make it easy for us to incorporate these into our apps.
Internal gem updates. We should always be using the latest version
of our internal gems, because they exist to provide consistency among
our apps, which we don't want to drift away from.
Framework gem updates. We write a lot of code using the APIs and
DSLs of a few frameworks. If we don't keep up with them, it could
become prohibitively hard to update them in future.

Many of the remaining gems would be easy to update manually. There are
a few reasons why we might want to do this:

The gem interacts with an external service or system package, which
no longer supports the old version of the gem.
We want (e.g. for consistency with other apps), or need to use a
feature that's only available in a newer version of the gem.

At the time of writing, the following gems are not included in the
config for Dependabot to update automatically:

"aws-sdk-s3" - this is only used in a small part of the codebase,
and we expect AWS APIs to be backwards compatible.
"interactor" - although we have a lot of code that follows the
Interactor pattern, the API/DSL coupling is low.
"pg" - although we do a lot of DB queries, most of these are done
through ActiveRecord, and is not coupled to this gem
"webmock" - most of our use of this gem is through gds-api-adapters,
so we would expect any major changes to be applied there
"bootsnap" - limited footprint
"hashdiff" - limited footprint
"image_processing" - limited footprint
"kaminari" - limited footprint
"mail-notify" - limited footprint
"pdf-reader" - limited footprint
"rinku" - limited footprint
"rubyzip" - limited footprint
"sanitize" - limited footprint
"sidekiq-scheduler" - limited footprint
"uglifier" - limited footprint
"with_advisory_lock" - limited footprint
"listen" - limited footprint
"simplecov" - limited footprint
"byebug" - limited footprint
"climate_control" - limited footprint
"jasmine_selenium_runner" - limited footprint
"json_matchers" - limited footprint

benthorner · 2020-05-19T14:25:55Z

Test if this config also works if one of the gems isn't used in the app

(This will require a minor tweak to this config to add such a gem - e.g. mongo.)

thomasleese · 2020-05-19T14:39:43Z

.dependabot/config.yml

+      - match: { dependency_name: factory_bot_rails }
+      - match: { dependency_name: jasmine }
+      - match: { dependency_name: rails }
+      - match: { dependency_name: rspec-rails }


Should we include rspec if we're including this? (It's likely they have to be updated in tandem)

I don't think we get these at the moment, as many of our repos are set to "Only top-level dependencies". Looks like rspec-rails is pretty strict on the underlying version, so I think we will still get these implicitly. factory_bot_rails isn't quite so up-to-date, but I think we'd get any major changes to the underlying gem(s) through a version bump at the top-level.

Previously we used the Dependabot UI to get top-level updates for all gems in the Gemfile, which in turn leads to a lot of daily, manual effort to review and deploy. However, most of the gems we have in our repos contribute very little to the codebase, and the risk of them being out-of-date is minimal, provided we still get any security updates. This adds an explicit config for Dependabot (to override the UI), which specified the updates we actually want: - Security updates. We want to protect are apps the best we can. Dependabot PRs can help to notify us about vulernabilities, and make it easy for us to incorporate these into our apps. - Internal gem updates. We should always be using the latest version of our internal gems, because they exist to provide consistency among our apps, which we don't want to drift away from. - Framework gem updates. We write a lot of code using the APIs and DSLs of a few frameworks. If we don't keep up with them, it could become prohibitively hard to update them in future. Many of the remaining gems would be easy to update manually. There are a few reasons why we might want to do this: - The gem interacts with an external service or system package, which no longer supports the old version of the gem. - We want (e.g. for consistency with other apps), or need to use a feature that's only available in a newer version of the gem. At the time of writing, the following gems are not included in the config for Dependabot to update automatically: - "aws-sdk-s3" - this is only used in a small part of the codebase, and we expect AWS APIs to be backwards compatible. - "interactor" - although we have a lot of code that follows the Interactor pattern, the API/DSL coupling is low. - "pg" - although we do a lot of DB queries, most of these are done through ActiveRecord, and is not coupled to this gem - "webmock" - most of our use of this gem is through gds-api-adapters, so we would expect any major changes to be applied there - "bootsnap" - limited footprint - "hashdiff" - limited footprint - "image_processing" - limited footprint - "kaminari" - limited footprint - "mail-notify" - limited footprint - "pdf-reader" - limited footprint - "rinku" - limited footprint - "rubyzip" - limited footprint - "sanitize" - limited footprint - "sidekiq-scheduler" - limited footprint - "uglifier" - limited footprint - "with_advisory_lock" - limited footprint - "listen" - limited footprint - "simplecov" - limited footprint - "byebug" - limited footprint - "climate_control" - limited footprint - "jasmine_selenium_runner" - limited footprint - "json_matchers" - limited footprint

benthorner · 2020-05-19T14:55:35Z

@alex-ju @kevindew what do you think about the following addition for package.json?

  - package_manager: javascript
    directory: /
    update_schedule: daily
    allowed_updates:
      # Security updates - we should always do these
      - match: { update_type: security }
      # Internal gems - we should always update these
      - match: { dependency_name: accessible-autocomplete }
      - match: { dependency_name: markdown-toolbar-element }
      - match: { dependency_name: miller-columns-element }
      - match: { dependency_name: paste-html-to-govspeak }

MuriloDalRi · 2020-05-19T16:04:01Z

.dependabot/config.yml

+    update_schedule: daily
+    allowed_updates:
+      # Security updates - we should always do these
+      - match: { update_type: security }


Would this catch every security updates? This PR for instance includes security updates but doesn't have the security label.

That's interesting - good spot!

Fortunately, this would be covered by https://github.com/alphagov/content-publisher/pull/2062/files#diff-b44e3cec799fb1f7871015e85dba6eb3R25. Arguably this is a problem with Dependabot not marking the update correctly, which would already be a problem for any of our lower-level dependencies. For other gems, I guess we have to accept the risk.

alex-ju · 2020-05-19T16:56:12Z

@alex-ju @kevindew what do you think about the following addition for package.json?

  - package_manager: javascript
    directory: /
    update_schedule: daily
    allowed_updates:
      # Security updates - we should always do these
      - match: { update_type: security }
      # Internal gems - we should always update these
      - match: { dependency_name: accessible-autocomplete }
      - match: { dependency_name: markdown-toolbar-element }
      - match: { dependency_name: miller-columns-element }
      - match: { dependency_name: paste-html-to-govspeak }

Sounds good to me @benthorner, but I'm not sure how would it work for the accessible-autocomplete which is a fork and not picked up via npm.

benthorner · 2020-05-20T08:09:47Z

@alex-ju I'm not sure either. Other repos using the original could at least benefit from Dependabot on the original repo. This is at least no worse than the current behaviour for this dependency, as it shouldn't change whether Dependabot can bump it or not.

kevindew · 2020-05-20T11:00:39Z

Yup, this looks good to me 👍

I take it this will automatically update the UI? Do we have any idea what happens if someone changes the UI when we have a config file? Probably not the end of the world if it allows inconsistency though.

I take it we'll keep gemfiles and package.json files as they are, which seems good for preserving bundle update and yarn upgrade.

Any thoughts/ideas about how to apply this across apps? I guess there's no real alternative than writing this out manually (which would have been the same as Gemfile changes) for each app. I did also wonder if there is anything govuk-saas-config could help us with but both ideas I have seemed unusual for the repo (one was that for gov.uk projects with a Gemfile or package.json add a basic dependabot config that turns on security, but that would require govuk-saas-config to open a PR which I believe is unprecedented, other was to alert if apps lack this but again govuk-saas-config doesn't have precedent for alerting).

thomasleese · 2020-05-20T11:03:51Z

Any thoughts/ideas about how to apply this across apps? I guess there's no real alternative than writing this out manually (which would have been the same as Gemfile changes) for each app. I did also wonder if there is anything govuk-saas-config could help us with but both ideas I have seemed unusual for the repo (one was that for gov.uk projects with a Gemfile or package.json add a basic dependabot config that turns on security, but that would require govuk-saas-config to open a PR which I believe is unprecedented, other was to alert if apps lack this but again govuk-saas-config doesn't have precedent for alerting).

Yeah, I've been thinking about this a bit because we've been doing something very similar to this with Platform Health apps. My current thinking is that we're going to get govuk-saas-config to raise a PR as you mentioned (which is unprecedented, but not dissimilar to how we upgrade all our apps to the next version of Ruby) - can you think of any reasons not to do this?

kevindew · 2020-05-20T11:22:02Z

Any thoughts/ideas about how to apply this across apps? I guess there's no real alternative than writing this out manually (which would have been the same as Gemfile changes) for each app. I did also wonder if there is anything govuk-saas-config could help us with but both ideas I have seemed unusual for the repo (one was that for gov.uk projects with a Gemfile or package.json add a basic dependabot config that turns on security, but that would require govuk-saas-config to open a PR which I believe is unprecedented, other was to alert if apps lack this but again govuk-saas-config doesn't have precedent for alerting).

Yeah, I've been thinking about this a bit because we've been doing something very similar to this with Platform Health apps. My current thinking is that we're going to get govuk-saas-config to raise a PR as you mentioned (which is unprecedented, but not dissimilar to how we upgrade all our apps to the next version of Ruby) - can you think of any reasons not to do this?

If we can do it it sounds good. I guess the tricky bit is how clever we want it to be which may set precedences for consistency.

A simple approach would be to just detect a Gemfile and drop in:

version: 1
update_configs:
  - package_manager: ruby:bundler
    directory: /
    update_schedule: daily
    allowed_updates:
      # Security updates - we should always do these
      - match: { update_type: security }

Or we could go all out with all the gems we care about whether the app has them or not and drop in all gov.uk gems and all the common deps. Though that feels a bit of a bummer to have things marked that apps don't have installed, and I guess it could cause confusing results if some of the gems referenced are sub dependencies.

Then I guess we've got an approach that involves parsing a Gemfile, which I guess isn't too complex but just another thing to maintain.

thomasleese · 2020-05-20T11:25:46Z

Or we could go all out with all the gems we care about whether the app has them or not and drop in all gov.uk gems and all the common deps. Though that feels a bit of a bummer to have things marked that apps don't have installed, and I guess it could cause confusing results if some of the gems referenced are sub dependencies.

This is the approach I was leaning towards. It doesn't feel worth maintaining something complicated when we can just make sure the config file is always up to date with our "reference" config file.

kevindew · 2020-05-20T11:48:41Z

Or we could go all out with all the gems we care about whether the app has them or not and drop in all gov.uk gems and all the common deps. Though that feels a bit of a bummer to have things marked that apps don't have installed, and I guess it could cause confusing results if some of the gems referenced are sub dependencies.

This is the approach I was leaning towards. It doesn't feel worth maintaining something complicated when we can just make sure the config file is always up to date with our "reference" config file.

Ok sure.

So would the expectation be that we never manually edit .dependabot/config.yml as doing so would mean the bulk applier may clobber those changes? or would the bulk applier just make sure there is an initial file and it's up to the repo from there?

I expected that we want some control over an individual repos management. As an example the Coronavirus teams added Dynamo DB support which would be consistent to be managed, but we'd probably not want 100 PRs to add that in for one project.

thomasleese · 2020-05-20T11:54:40Z

So would the expectation be that we never manually edit .dependabot/config.yml as doing so would mean the bulk applier may clobber those changes? or would the bulk applier just make sure there is an initial file and it's up to the repo from there?

I haven't thought about it too much yet, but in my mind we would be expected not to edit the .dependabot/config.yml inside the repo. Although, thinking about it now, we could probably reasonably easily have some logic that parses the file in the repo, and checks that it has the minimum dependencies specified in our common config, but that allows extras to be customised.

The plan at the moment is to open a RFC to discuss this process, so I imagine a lot of these concerns will appear then!

benthorner · 2020-05-21T13:55:20Z

@kevindew looking at the Dependabot UI for Content Tagger, I can't see anything to indicate the config file is override the settings for the repo. @thomasleese are we confident it's working?

thomasleese · 2020-05-21T13:57:53Z

@kevindew looking at the Dependabot UI for Content Tagger, I can't see anything to indicate the config file is override the settings for the repo. @thomasleese are we confident it's working?

I believe the .config/dependabot.yml in the top right corner is the indication, and when I click through on the cog it presents me with the contents of the file.

benthorner · 2020-05-21T14:01:22Z

@thomasleese thanks for checking - I was looking at the wrong repo, sorry!

@kevindew @thomasleese do you think this is something we can add to Content Publisher, or should we just close it and point to it in the RFC?

kevindew · 2020-05-21T14:52:51Z

Seems good to add, I feel like that will give us more to learn from and may reveal some surprises.

Do we have any data on the amount of dependabot PR's that are raised currently for this repo as a basis for comparison?

Changes I'd suggest before going live is to put Node in there too so we can learn from that as well (I feel like that could give us more surprises than Ruby) and maybe add a little disclaimer comment at the start to say we're trialling this approach.

benthorner · 2020-05-21T15:34:35Z

@kevindew sure, I've added both of those things.

My understanding is Platform Health are currently tracking open Dependabot PRs for each repo every two weeks, but not for this repo specifically.

@thomasleese perhaps we can start keeping track of how many open+closed PRs there are?

kevindew

Looks good to me, just a minor fix needed

kevindew · 2020-05-22T09:46:12Z

.dependabot/config.yml

+    allowed_updates:
+      # Security updates - we should always do these
+      - match: { update_type: security }
+      # Internal gems - we should always update these


Suggested change

# Internal gems - we should always update these

# Internal packages - we should always update these

Oops - fixed 👍

Currently Dependabot is enabled for all Gems and packages on this repo. We'd like to trial limiting the number of PRs opened by Dependabot to libraries that we care about explicitly. See alphagov/content-publisher#2062 for more context on this configuration file (and a similar config file for Content Publisher). There's a few reasons I've picked this repo to try out this config (as well as the existing configuration in Content Publisher). - It's a Gem, allowing us to learn more about if there's anything special about Gems which means going down this route would be problematic. - It includes a framework library (`govuk-frontend`) which we'd want to have enabled for this repo, but perhaps not for any others. - It allows us to test the idea of having a global config file containing rules for libraries not used by this repo, and check that Dependabot is okay with that.

benthorner mentioned this pull request May 19, 2020

Explicitly specify which gems can update #2054

Closed

benthorner requested review from thomasleese and kevindew May 19, 2020 14:26

thomasleese reviewed May 19, 2020

View reviewed changes

benthorner force-pushed the dependabot-rules branch from 6719a22 to 6a7095d Compare May 19, 2020 14:53

benthorner force-pushed the dependabot-rules branch from 6a7095d to ee2022a Compare May 19, 2020 14:55

MuriloDalRi reviewed May 19, 2020

View reviewed changes

kevindew approved these changes May 22, 2020

View reviewed changes

Add additional config for JS packages

ecf27f8

benthorner force-pushed the dependabot-rules branch from a63bd3d to ecf27f8 Compare May 22, 2020 10:23

benthorner merged commit f30d69f into master May 22, 2020

benthorner deleted the dependabot-rules branch May 22, 2020 10:39

thomasleese mentioned this pull request Jun 9, 2020

Add explicit config for Dependabot alphagov/govuk_publishing_components#1564

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add explicit config for Dependabot #2062

Add explicit config for Dependabot #2062

benthorner commented May 19, 2020

benthorner commented May 19, 2020

thomasleese May 19, 2020

benthorner May 19, 2020

benthorner commented May 19, 2020

MuriloDalRi May 19, 2020

benthorner May 19, 2020

alex-ju commented May 19, 2020

benthorner commented May 20, 2020

kevindew commented May 20, 2020

thomasleese commented May 20, 2020

kevindew commented May 20, 2020

thomasleese commented May 20, 2020

kevindew commented May 20, 2020

thomasleese commented May 20, 2020

benthorner commented May 21, 2020

thomasleese commented May 21, 2020

benthorner commented May 21, 2020

kevindew commented May 21, 2020

benthorner commented May 21, 2020

kevindew left a comment

kevindew May 22, 2020

benthorner May 22, 2020

	# Internal gems - we should always update these
	# Internal packages - we should always update these

Add explicit config for Dependabot #2062

Add explicit config for Dependabot #2062

Conversation

benthorner commented May 19, 2020

benthorner commented May 19, 2020

thomasleese May 19, 2020

Choose a reason for hiding this comment

benthorner May 19, 2020

Choose a reason for hiding this comment

benthorner commented May 19, 2020

MuriloDalRi May 19, 2020

Choose a reason for hiding this comment

benthorner May 19, 2020

Choose a reason for hiding this comment

alex-ju commented May 19, 2020

benthorner commented May 20, 2020

kevindew commented May 20, 2020

thomasleese commented May 20, 2020

kevindew commented May 20, 2020

thomasleese commented May 20, 2020

kevindew commented May 20, 2020

thomasleese commented May 20, 2020

benthorner commented May 21, 2020

thomasleese commented May 21, 2020

benthorner commented May 21, 2020

kevindew commented May 21, 2020

benthorner commented May 21, 2020

kevindew left a comment

Choose a reason for hiding this comment

kevindew May 22, 2020

Choose a reason for hiding this comment

benthorner May 22, 2020

Choose a reason for hiding this comment