RFR: 2137: Add integrity check to integrate and sponsor commands

Erik Joelsson erikj at openjdk.org
Wed Jan 10 20:48:07 UTC 2024


On Wed, 10 Jan 2024 09:12:50 GMT, Erik Duveblad <ehelin at openjdk.org> wrote:

> Hi all,
> 
> please review this patch that adds that makes it possible to enable an "integrity check" for the `/integrate` and `/sponsor` commands. The integrity check will be run before a commit is pushed and ensures that the most recent commit on the target branch was integrated by the bots.
> 
> The integrity check is implemented by utilizing an additional repository that for each repository and branch combination for a forge stores the two most recent commits pushed by the bots. This is information is stored in a text file called `heads.txt`, the first list line being the full hash of the most recent commit and the second line being the full hash of the next to most recent commit. The hashes of the two most recent commits are needed for the scenario when a bot throws an exception _after_ the file `heads.txt` in the integrity repo has been updated but _before_ the actual commit has been pushed to the target branch of the pull request. The `IntegrityVerifier` will recognize this case the next time a commit is verified (via a call to `verifyBranch`) and automatically correct the data in the integrity repository.
> 
> I also added a couple of new unit tests as well as two larger unit tests for the `/integrate` and `/sponsor` commands.
> 
> Thanks,
> Erik

We discussed this offline already but summarizing here for the record.

I think the integrity repo should be handled more similar to how we handle for example the notifier tracking repos. In those repos, the default is just a top level file in the master branch for each tracked repository. It's also possible to override the base filename. Having similar patterns makes it easier for administrators when the data needs to be inspected or possibly manually modified. In this case we would still need a separate file for each repo/branch combination. If you want to make part of that file path a directory, I'm fine with that.

I also think that we should use the `HostedRepository.writeFileContents` API instead of cloning/fetching/pushing through a local repository. It will be way more efficient in the long run as validating and fetching a book keeping repository has been shown to be extremely costly as it grows over time.

-------------

PR Review: https://git.openjdk.org/skara/pull/1596#pullrequestreview-1814144141


More information about the skara-dev mailing list