Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Investigate consolidating renoavte and dependabot #2948

Closed
jcscottiii opened this issue Aug 29, 2022 · 8 comments
Closed

Investigate consolidating renoavte and dependabot #2948

jcscottiii opened this issue Aug 29, 2022 · 8 comments

Comments

@jcscottiii
Copy link
Collaborator

Why

We currently leverage two systems for dependency upgrades: renovate and dependabot. Recently, there have been some problems where dependabot PRs cannot successfully finish their gates. (#2926, #2925, #2924) This is due to the fact that GitHub has changed the behavior of Dependabot to be more secure. Dependabot no longer has access to the same secrets as a normal pipeline. That begs the question that why doesn't renovate enforce something like this too? Instead of having to wonder about the security of two systems doing the same thing, why not just consolidate to one and keep up to date with that single one.

@jcscottiii
Copy link
Collaborator Author

jcscottiii commented Aug 29, 2022

What Happens If We Switch To Only Using Dependabot

Pros of using dependabot

  • Other projects use just dependabot. (Ability to stay consistent across projects)
  • Dependabot is a first party bot

Cons of using dependabot

  • More configuration needs to be changed from renovate to dependabot instead of the other way around.
    • Impact: Very low. One time change
  • Lose the ability to run npm dedupe to keep the node package.json clean. Tracking issue
    • Impact: Medium. Periodically will have to manually run npm dedupe and make a PR to keep package.json clean
  • Lose the ability to group updates together. Tracking discussion
    • Impact: Very low. Only packages @browser-logos/* are configured to be grouped under one PR.

Migration Path & Other Suggestions

  1. Add docker, go, node to the existing dependabot config which only has python.
    • Currently, renovate takes care of go and node. Neither upgrade docker.
  2. Switch all scanners to scanning at least weekly. Currently, renovate is configured to scan weekly. Dependabot is configured to scan monthly.
  3. Add conditional gates to the deploy.yml for PRs raised by dependabot.
    • Options:
      1. Disable deploying completely for dependabot PRs. (We currently disable deploying for PRs originating from a fork)
      2. Investigate if there's a way to leverage review deployments specifically for dependabot builds.

Outstanding work

  • Need to look into the Options under Migration Path & Other Suggestions, Bullet Number 3.

@jcscottiii
Copy link
Collaborator Author

jcscottiii commented Aug 29, 2022

What Happens If We Switch To Only Using Renovate

Pros of using renovate

  • Renovate is feature rich. Has some features that dependabot does not have. (Check the cons of the previous post)
  • Only need to migrate python over. Go and node already configured.

Cons of using renovate

  • Renovate is a third party bot. What is the support model for it?
    • Impact: Very Low - Low. If renovate was deprecated, it would not impact the continuity of the site. Renovate is a separate company that we have to depend on.
    • Risk: Low - Medium. If renovate was compromised, it would be the same as if dependabot was compromised. But what are the odds of it happening to renovate vs dependabot? Independently, it may be the same odds. But overall it is one more company to depend on. Makes it inherently more risky.

Migration Path & Other Suggestions

  1. Add docker, python to the existing renovate config which only has go, node.
    • Currently, dependabot takes care of python. Neither upgrade docker.
  2. Switch all scanners to scanning at least weekly. Currently, renovate is configured to scan weekly. Dependabot is configured to scan monthly.
  3. Disable auto merging of renovate PRs
  4. Add conditional gates to the deploy.yml for PRs raised by renovate.
    • Options:
      1. Disable deploying completely for renovate PRs. (We currently disable deploying for PRs originating from a fork)
      2. Investigate if there's a way to leverage review deployments specifically for renovate builds.

Outstanding work

  • Need to look into the Options under Migration Path & Other Suggestions, Bullet Number 4.

@jcscottiii
Copy link
Collaborator Author

Regarding the outstanding work section, here are some data points

a. Disable deploying completely for renovate/dependabot PRs

It is common to see this done. Examples:

  1. google-github-actions/deploy-cloud-functions - Explicitly skips deploying for dependabot builds
  2. worker-place/worker-place - explicitly skips deploying for dependabot builds
  3. DFE-Digital/apply-for-qualified-teacher-status - only deploys if a reviewer adds a tag to it.

b. Investigate if there's a way to leverage review deployments specifically for dependabot/renovate builds

A way to accomplish this option:

  1. Create a new environment in the settings. (e.g. dependabot-review). Add required reviewers to the environment
  2. Add a section to the deploy.yml that looks specifically for dependabot. Add the environment to that section.
  3. Create credentials for dependabot to deploy with. Ensure that the service account credentials only have the roles needed to deploy. Not project wide admin. (This may already be done). Add the credentials under the dependabot section of the repo's secrets

@jcscottiii
Copy link
Collaborator Author

Conclusion

I think dependencies with major version upgrades should be deployed. Maybe even some minor version upgrades too. Patch version upgrades are okay to be skipped. If we want this selective deployment, using 1) the example that adds a tag added by reviewers could be useful or 2) examining the metadata (via steps.dependabot-metadata.outputs.update-type) of the dependabot PR could help.

To keep things simple, we could just deploy on all dependency updates.

In whichever case we do decide to do, we should add reviewed deployments. This will prevent a bad dependency from automatically being built and deployed. A human has to OK the dependency version.

@jcscottiii
Copy link
Collaborator Author

Last week it was discussed whether we should keep both renovate and dependabot or consolidate to one. I added my notes to this thread here. FYI for @DanielRyanSmith @foolip @KyleJu @past

@past
Copy link
Member

past commented Sep 8, 2022

Thank you for the very thorough analysis and investigation, James! I agree with your conclusion, although it doesn't look like you have a recommendation regarding consolidating on renovate or dependabot?

I feel slightly more inclined to move towards dependabot as it is a first-party bot, but I want to know what the rest of the team thinks.

@KyleJu
Copy link
Collaborator

KyleJu commented Sep 9, 2022

+1 to @past's comment. I have gone through the third-party application approval process for pyup (another automatic dep update service), and the official reply is I'm hesitant to add any third-party applications unless they've been vetted by the OSPO team. Dependabot is recommended by the GoogleChrome GitHub organization.

Moreover, third-party applications add another layer of complexity to store credentials, since GitHub has to indirectly access to our GCP service account secret from another party. Overall, my gut feeling here is that the simpler, the better.

@jcscottiii
Copy link
Collaborator Author

Thanks for the feedback! I created #2959 to track the work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants