Closed rhyskoedijk closed 2 months ago
This has got to be the most detailed contribution I have seen in this repository. Maybe the first from someone with proper Ruby knowledge. Thank you so much.
Here's my view:
update_script_new.rb
to update_script_vnext.rb
, add a Boolean input to the extension task named useVNext
, which in turn changes the command sent to docker.Overall, thanks so much for the contribution!
- The renaming of files towards using the lib makes a lot of sense. However, I'd prefer we did that in a separate PR before to be merged before this one.
Makes sense, I'll push a new PR for these.
- The files I copied from the core repository are unused so you're right in starting to create a new client. I think we need to delete those files too.
There are a few files from the core repo that will be really useful still I think, but I agree there are a few that could be removed. I will see if I can clean them up a little.
- To allow for others to test, id rename
update_script_new.rb
toupdate_script_vnext.rb
, add a Boolean input to the extension task nameduseVNext
, which in turn changes the command sent to docker.
Sounds good to me, I'll update this.
- What feature set are we looking at in this new/vnext workflow? Groups is one, are there others that I didn't see or come as a consequence? Also, I wonder if this will affect things like conflict resolution and closure. Though I think that's why we are keeping the other one around.
The main features I am personally interested in are:
I was hoping that if the updater piggybacks off the core repository classes as much as possible, then any new Dependabot features should more easily "just work" (overly optimistic maybe).
Overall, thanks so much for the contribution!
I should be thanking you for making this project in the first place :)
After merging the other PRs, the focus here is now more specific.
If we can piggyback on the updated core logic then it simplifies the maintenance works. This is why I had started to keep the files up-to date but got too busy to keep track, yet the updater is not published as a consumable. They don't accept contributions for it either.
Gathered some extra thoughts:
If we can piggyback on the updated core logic then it simplifies the maintenance works. This is why I had started to keep the files up-to date but got too busy to keep track, yet the updater is not published as a consumable. They don't accept contributions for it either.
Yeh, I think we can use what is there to do the majority of the heavy lifting though. The dry_run.rb
script hasn't been updated to support things like dependency groups (unless I'm blind), but the updater code has. So to get the most "authentic" experience, updater seems like the best choice.
I might submit a PR soon to update the /updater/lib/dependabot
files to latest version as there have been a number of changes since these were committed.
We have an empty client were the actual PRs opened? It may have done so if we are relying on the internal Azure client but I'm unsure.
The API client still needs to be finished, it will be a combination of your "azure_helper" and the built-in Azure client. When I put this up for draft, I only did the minimum required to get the grouped PRs to work on the local repo within the Dependabot container.
Do signature annotations require adding of types to the whole runtime code? (Assuming you have more knowledge on Ruby than me which is likely)
Honestly, not sure. I only just picked up Ruby a couple of weeks ago when I started contributing to this and dependabot-core. I will look in to it though before publishing the PR.
Maybe we can indicate what features are not supported at the start. For example: vulnerability checks, auto approve, etc Unless you already have solutions for this.
Good idea. I'm hoping to implement all the features in to the new script, but if that doesn't happen for some reason I'll look in to this.
Heads up! Made some updates that may cause trouble with lint/specs.
https://github.com/tinglesoftware/dependabot-azure-devops/issues/1202
@mburumaxwell this is [mostly] feature complete now. I still need to fully test all config options, fix a few small bugs, and clean/optimise the code. However, I would be interested to know if you have any concerns or suggestions on the high-level implementation of this, especially regarding structure of the code and the environment variable limitations (see PR description section on limitations).
I've added a new column to the updater environment variables that lists which command scripts each variable is supported by. If you can think of a better way to communicate this, let me know.
I also decided not update the DevOps extension in this PR, just focus on the updater and environment variables. I'll update the extension in a separate PR if/when this is approved.
I think this is generally okay. I cannot see anything of major concern yet in the updater. Issues may show up when we make it available for others to test. I see some commits since you asked for review, I will keep the assumption that so long as the PR is in draft mode, you are still working on stuff, and then I can review again when you make it ready to review.
On a side note, I do not know if other people use the server component that will be affected when we make this mainstream as it would require the fetch_files.rb
and update_files.rb
. Maybe we can worry about it when it happens.
I think this is generally okay. I cannot see anything of major concern yet in the updater. Issues may show up when we make it available for others to test. I see some commits since you asked for review, I will keep the assumption that so long as the PR is in draft mode, you are still working on stuff, and then I can review again when you make it ready to review.
Will do. There are two bugs that I know about which I'm still working though, but shouldn't be too far away from publishing now.
On a side note, I do not know if other people use the server component that will be affected when we make this mainstream as it would require the
fetch_files.rb
andupdate_files.rb
. Maybe we can worry about it when it happens.
I didn't realise the server component used these files, my mistake. It is probably a good idea that I restore them then if that is the case.
Hey guys! I've been following this.. we are just starting to implements dependabot on our Azure organization. I am tempted to test this, since we have a monorepo and want to use groups. Any tips on how to test this vnext update script? Do I just run the ecosystem image and use the new name as the command? Thank you!!
Any tips on how to test this vnext update script?
Unfortunately you can't test this new script in DevOps until the extension is updated (see: https://github.com/tinglesoftware/dependabot-azure-devops/pull/1216). However, you can run the updater manually using Docker if that is something you are comfortable with. The image tag would need to be 1.29.8-ci0001
and the command would be update_script_vnext
, for example:
docker run --rm -t \
<your_environment_variables_here> \
ghcr.io/tinglesoftware/dependabot-updater-<your_ecosystem_name_here>:1.29.8-ci0001 update_script_vnext
Hi @rhyskoedijk appreciate it will give it a try and I'll report back in the form of new issue/discussion with my findings. Thanks! Have a great day Leo.
What are you trying to accomplish?
groups:
,directories:
, etc)New update script 'vNext'
To preserve the existing behavior, a new update script (
update_script_vnext.rb
) was added. This new script leverages as much existing functionality from the dependabot-core "updater" project as possible; as opposed toupdate_script.rb
which is based ondry-run.rb
and does not handle many of the newer Dependabot features such as dependency groups and multiple directories.High-level update sequence diagram
Dependency state metadata now stored in pull request properties
The vNext script will use Pull Request Properties to store metadata related to Dependabot updates. This is done to mimic how
Dependabot::ApiClient
works and is primarily used to accurately identify which dependencies were modified by a PR without needing to interept the PR title. The stored PR property names are:dependabot.base_commit_sha
dependabot.updated_dependencies
If the "updated_dependencies" property is not present in a PR, the vNext script will not recognise it when checking for existing PRs during the update process. The property list for a grouped dependency update PR would look like:
Limitations
The following environment variables and features are not currently supported by the vNext script.
Dependabot::Updater
to run the core updater logic, there is no out-of-the-box option to cleanly break out of the update process. The behaviour is that the updater will process all updates and then aggregate errors at the end; Interrupting this process by throwing exceptions to break out of the updater results in error details and stack trace info being lost, making diagnosing issues very difficultDependabot::Updater
cannot be cleanly interrupted. This means that once the pull request limit is reached, dependency updates will continue to be processed but they will not be committed to DevOps. This may result in overall longer task run times due to the updater processing updates that may not end up being committed.New environment variables
The following new environments variables have been added and are supported by the vNext script only.
['/', '/src']
. When specified, it overridesDEPENDABOT_DIRECTORY
. When not specified,DEPENDABOT_DIRECTORY
is used instead. See official docs for more.true
, only security updates will be processed. Can be used in combinationDEPENDABOT_OPEN_PULL_REQUESTS_LIMIT
to exclusively perform security updates whilst also limiting the total number of security PRs opened at once.gomod
as Dependabot automatically detects vendoring. See official docs for more.{"microsoft":{"applies-to":"version-updates","dependency-type":"production","patterns":["microsoft*"],"exclude-patterns":["*azure*"],"update-types":["minor","patch"]}}
. See official docs for more.dependabot
.none
,angular
,eslint
,gitmoji
. IfDEPENDABOT_COMMIT_MESSAGE_OPTIONS
prefixes are also defined, this option does nothing. Defaults tonone
.false
. See official docs for more.false
.false
.Dependency groups
Dependency groups are supported if
DEPENDABOT_DEPENDENCY_GROUPS
is set with the group rules, in JSON format. See the official docs for more.Multiple directories per package ecosystem
Multiple directories per ecosystem are supported if
DEPENDABOT_DIRECTORIES
is set with the directory paths, in JSON format. See the official docs for more.Comment on pull requests
When
DEPENDABOT_COMMENT_PULL_REQUESTS
is set, a comment will be added before closing pull requests explaining why it was closed. The comment closely (but not exactly) match those used by the GitHub Dependabot service.Git branch prefixes
The Git branch prefix can be set using
DEPENDABOT_BRANCH_NAME_PREFIX
.Pull request name prefix styles
The (hidden?) name prefix style options can be forced on using
DEPENDABOT_PR_NAME_PREFIX_STYLE
. These styles are similar to "commit options" prefixes, but slightly more dynamic? (e.g.gitmoji
uses "⬆️" for regular updates, or "⬆️🔒" for a security updates). Not sure if this is a new experimental feature, or an old one that they are phasing out. Either way, it can now be configured. Supported options are:Default
Gitmoji
Angular
Eslint
Compatibility score badges
When
DEPENDABOT_COMPATIBILITY_SCORE_BADGES
is set, compatibility score badges are shown in the pull request description for single dependency updates (but not group updates). This feature uses public information from GitHub and enabling it does not send any private information about your repository to GitHub other than the dependency name and version number(s) required to calculate to the compatibility score. Defaults tofalse
. See official docs for more.Pull request description header/footer text
Extra header/footer text can be added to pull request descriptions with
DEPENDABOT_MESSAGE_HEADER
andDEPENDABOT_MESSAGE_FOOTER
respectively.Pull request description "Vulnerabilities fixed" info
When
GITHUB_ACCESS_TOKEN
is set, pull requests containing security related dependency updates will now have:Pull request author signed commits (signature key config)
Commits made by Dependabot will be signed when
DEPENDABOT_SIGNATURE_KEY
is set with an appropriate GPG key. See official docs for more.