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

Add new workflow approval logic #261

Merged
merged 10 commits into from Sep 9, 2022
50 changes: 45 additions & 5 deletions README.adoc
Expand Up @@ -31,13 +31,13 @@ triage:
- labels: [area/amazon-lambda]
title: "lambda"
notify: [patriot1burke, matejvasek]
directories:
files:
- extensions/amazon-lambda
- integration-tests/amazon-lambda
- labels: [area/persistence]
title: "db2"
notify: [aguibert]
directories:
files:
- extensions/reactive-db2-client/
- extensions/jdbc/jdbc-db2/
----
Expand Down Expand Up @@ -88,9 +88,9 @@ There are a few differences though as it doesn't behave in the exact same way.

For pull requests, each rule can be triggered by:

* `directories` - if any file in the commits of the pull requests match, trigger the rule. This is not a regexp (it uses `startsWith`) but glob type expression are supported too `extensions/test/**`.
* `files` - if any file in the commits of the pull requests match, trigger the rule. This is not a regexp (it uses `startsWith`) but glob type expression are supported too `extensions/test/**`.

If no rule is triggered based on directories, or if rules are triggered but they all specify `allowSecondPass: true`,
If no rule is triggered based on files, or if rules are triggered but they all specify `allowSecondPass: true`,
a second pass will be executed; in that second pass, rules can be triggered by:

* `title` - if the title matches this regular expression (case insensitively), trigger the rule
Expand All @@ -113,7 +113,7 @@ triage:
title: "lambda"
notify: [patriot1burke, matejvasek]
notifyInPullRequest: true
directories:
files:
- extensions/amazon-lambda
- integration-tests/amazon-lambda
----
Expand Down Expand Up @@ -193,6 +193,46 @@ When a workflow run associated to a pull request is completed, a report is gener

> image::documentation/screenshots/workflow-run-report.png[]

=== Approve workflow runs

This rule applies more fine-grained protections to workflow runs
than is provided by the basic GitHub settings. If a repository
is https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/enabling-features-for-your-repository/managing-github-actions-settings-for-a-repository[set up to only allow workflow runs from committers],
the bot can automatically approve some workflows which meet a set of rules.

Syntax of the `.github/quarkus-github-bot.yml` file is as follows:

[source, yaml]
----
features: [ APPROVE_WORKFLOWS ]
workflows:
rules:
- allow:
files:
- ./src
- ./doc*
- "**/README.md"
users:
minContributions: 5
unless:
files:
- ./.github
- "**/pom.xml"
----

Workflows will be allowed if they meet one of the rules in the `allow` section,
unless one of the rules in the `unless` section is triggered.

In the example above, any file called `README.md` would be allowed, except for `./github/README.md`.
Users who had made at least 5 commits to the repository would be allowed to make any changes,
except to a `pom.xml` or any files in `.github`. Other users could make changes to `./src` or directories whose name started with `./doc`.

If the rule is triggered, the following actions will be executed:

* `approve` - will approve the workflow which needs approval

If the workflow is not approved, it will be left untouched, for a human approver to look at.

=== Mark closed pull requests as invalid

If a pull request is closed without being merged, we automatically add the `triage/invalid` label to the pull request.
Expand Down
215 changes: 215 additions & 0 deletions src/main/java/io/quarkus/bot/ApproveWorkflow.java
@@ -0,0 +1,215 @@
package io.quarkus.bot;

import io.quarkiverse.githubapp.ConfigFile;
import io.quarkiverse.githubapp.event.WorkflowRun;
import io.quarkus.bot.config.Feature;
import io.quarkus.bot.config.QuarkusGitHubBotConfig;
import io.quarkus.bot.config.QuarkusGitHubBotConfigFile;
import io.quarkus.bot.util.PullRequestFilesMatcher;
import io.quarkus.cache.CacheKey;
import io.quarkus.cache.CacheResult;
import org.jboss.logging.Logger;
import org.kohsuke.github.GHEventPayload;
import org.kohsuke.github.GHPullRequest;
import org.kohsuke.github.GHRepository;
import org.kohsuke.github.GHRepositoryStatistics;
import org.kohsuke.github.GHWorkflowRun;
import org.kohsuke.github.PagedIterable;

import javax.inject.Inject;
import java.io.IOException;
import java.util.List;
import java.util.Map;
import java.util.function.Function;
import java.util.stream.Collectors;

class ApproveWorkflow {

private static final Logger LOG = Logger.getLogger(ApproveWorkflow.class);

@Inject
QuarkusGitHubBotConfig quarkusBotConfig;

void evaluatePullRequest(
@WorkflowRun.Requested GHEventPayload.WorkflowRun workflowPayload,
@ConfigFile("quarkus-github-bot.yml") QuarkusGitHubBotConfigFile quarkusBotConfigFile) throws IOException {
if (!Feature.APPROVE_WORKFLOWS.isEnabled(quarkusBotConfigFile)) {
return;
}

// Don't bother checking if there are no rules
if (quarkusBotConfigFile.workflows.rules != null && quarkusBotConfigFile.workflows.rules.isEmpty()) {
return;
}
GHWorkflowRun workflowRun = workflowPayload.getWorkflowRun();

// Only check workflows which need action
if (!GHWorkflowRun.Conclusion.ACTION_REQUIRED.equals(workflowRun.getConclusion())) {
return;
}

ApprovalStatus approval = new ApprovalStatus();

checkUser(workflowPayload, quarkusBotConfigFile, approval);

// Don't bother checking more if we have a red flag
// (but don't return because we need to do stuff with the answer)
if (!approval.hasRedFlag()) {
checkFiles(quarkusBotConfigFile, workflowRun, approval);
}

if (approval.isApproved()) {
processApproval(workflowRun);
}
}

private void processApproval(GHWorkflowRun workflowRun) throws IOException {
// We could also do things here like adding comments, subject to config
if (!quarkusBotConfig.isDryRun()) {
workflowRun.approve();
}
}

private void checkUser(GHEventPayload.WorkflowRun workflowPayload, QuarkusGitHubBotConfigFile quarkusBotConfigFile,
ApprovalStatus approval) {
for (QuarkusGitHubBotConfigFile.WorkflowApprovalRule rule : quarkusBotConfigFile.workflows.rules) {
// We allow if the files or directories match the allow rule ...
if ((rule.allow != null && rule.allow.users != null) || (rule.unless != null && rule.unless.users != null)) {
GHRepositoryStatistics.ContributorStats stats = getStatsForUser(workflowPayload);
if (matchRuleForUser(stats, rule.allow)) {
approval.shouldApprove = true;
}

if (matchRuleForUser(stats, rule.unless)) {
approval.shouldNotApprove = true;
}
}
}
}

private void checkFiles(QuarkusGitHubBotConfigFile quarkusBotConfigFile, GHWorkflowRun workflowRun,
ApprovalStatus approval) {
String sha = workflowRun.getHeadSha();

// Now we want to get the pull request we're supposed to be checking.
// It would be nice to use commit.listPullRequests() but that only returns something if the
// base and head of the PR are from the same repository, which rules out most scenarios where we would want to do an approval

String fullyQualifiedBranchName = workflowRun.getHeadRepository().getOwnerName() + ":" + workflowRun.getHeadBranch();

PagedIterable<GHPullRequest> pullRequestsForThisBranch = workflowRun.getRepository().queryPullRequests()
.head(fullyQualifiedBranchName)
.list();

// The number of PRs with matching branch name should be exactly one, but if the PR
// has been closed it sometimes disappears from the list; also, if two branch names
// start with the same string, both will turn up in the query.
for (GHPullRequest pullRequest : pullRequestsForThisBranch) {

// Only look at PRs whose commit sha matches
if (sha.equals(pullRequest.getHead().getSha())) {

for (QuarkusGitHubBotConfigFile.WorkflowApprovalRule rule : quarkusBotConfigFile.workflows.rules) {
// We allow if the files or directories match the allow rule ...
if (matchRuleFromChangedFiles(pullRequest, rule.allow)) {
approval.shouldApprove = true;
}
// ... unless we also match the unless rule
if (matchRuleFromChangedFiles(pullRequest, rule.unless)) {
approval.shouldNotApprove = true;
}
}
}
}
}

public static boolean matchRuleFromChangedFiles(GHPullRequest pullRequest,
QuarkusGitHubBotConfigFile.WorkflowApprovalCondition rule) {
// for now, we only use the files but we could also use the other rules at some point
if (rule == null) {
return false;
}

if (rule.files == null || rule.files.isEmpty()) {
return false;
}

PullRequestFilesMatcher prMatcher = new PullRequestFilesMatcher(pullRequest);
return prMatcher.changedFilesMatch(rule.files);
}

private boolean matchRuleForUser(GHRepositoryStatistics.ContributorStats stats,
QuarkusGitHubBotConfigFile.WorkflowApprovalCondition rule) {
if (rule == null || stats == null) {
return false;
}

if (rule.users == null) {
return false;
}

if (rule.users.minContributions != null && stats.getTotal() >= rule.users.minContributions) {
return true;
}

// We can add more rules here, for example how long the user has been contributing

return false;
}

private GHRepositoryStatistics.ContributorStats getStatsForUser(GHEventPayload.WorkflowRun workflowPayload) {

String login = workflowPayload.getSender().getLogin();
if (login != null) {
return getStatsForUser(workflowPayload.getRepository(), login);
}
return null;
}

@CacheResult(cacheName = "contributor-cache")
GHRepositoryStatistics.ContributorStats getStatsForUser(GHRepository repository, @CacheKey String login) {
try {
Map<String, GHRepositoryStatistics.ContributorStats> contributorStats = getContributorStats(repository);
return contributorStats.get(login);
} catch (IOException | InterruptedException | NullPointerException e) {
// We sometimes see an NPE from PagedIterator, if a fetch does not complete properly and leaves the object in an inconsistent state
// Catching these errors allows the null result for this contributor to be cached, which is ok
LOG.error("Could not get repository contributor statistics", e);
}

return null;
}

// We throw errors at this level to force the cache to retry and populate itself on the next request
@CacheResult(cacheName = "stats-cache")
Map<String, GHRepositoryStatistics.ContributorStats> getContributorStats(GHRepository repository)
throws IOException, InterruptedException {
GHRepositoryStatistics statistics = repository.getStatistics();
if (statistics != null) {
PagedIterable<GHRepositoryStatistics.ContributorStats> contributors = statistics.getContributorStats();
// Pull the iterable into a list object to force the traversal of the entire list,
// since then we get a fully-warmed cache on our first request
// Convert to a map for convenience of retrieval
List<GHRepositoryStatistics.ContributorStats> statsList = contributors.toList();
return statsList.stream()
.collect(
Collectors.toMap(contributorStats -> contributorStats.getAuthor().getLogin(), Function.identity()));
}
return null;
}

private static class ApprovalStatus {
// There are two variables here because we check a number of indicators and a number of counter-indicators
// (ie green flags and red flags)
boolean shouldApprove = false;
boolean shouldNotApprove = false;

boolean isApproved() {
return shouldApprove && !shouldNotApprove;
}

public boolean hasRedFlag() {
return shouldNotApprove;
}
}
}
3 changes: 2 additions & 1 deletion src/main/java/io/quarkus/bot/config/Feature.java
Expand Up @@ -10,7 +10,8 @@ public enum Feature {
SET_AREA_LABEL_COLOR,
TRIAGE_ISSUES_AND_PULL_REQUESTS,
TRIAGE_DISCUSSIONS,
PUSH_TO_PROJECTS;
PUSH_TO_PROJECTS,
APPROVE_WORKFLOWS;

public boolean isEnabled(QuarkusGitHubBotConfigFile quarkusBotConfigFile) {
if (quarkusBotConfigFile == null) {
Expand Down
@@ -1,13 +1,13 @@
package io.quarkus.bot.config;

import com.fasterxml.jackson.databind.annotation.JsonDeserialize;

import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.TreeSet;

import com.fasterxml.jackson.databind.annotation.JsonDeserialize;

public class QuarkusGitHubBotConfigFile {

@JsonDeserialize(as = HashSet.class)
Expand All @@ -21,6 +21,8 @@ public class QuarkusGitHubBotConfigFile {

public ProjectsClassic projectsClassic = new ProjectsClassic();

public Workflows workflows = new Workflows();

public static class TriageConfig {

public List<TriageRule> rules = new ArrayList<>();
Expand All @@ -40,9 +42,16 @@ public static class TriageRule {

public String expression;

/**
* @deprecated use files instead
*/
@JsonDeserialize(as = TreeSet.class)
@Deprecated(forRemoval = true)
public Set<String> directories = new TreeSet<>();

@JsonDeserialize(as = TreeSet.class)
public Set<String> files = new TreeSet<>();

@JsonDeserialize(as = TreeSet.class)
public Set<String> labels = new TreeSet<>();

Expand Down Expand Up @@ -81,6 +90,11 @@ public static class WorkflowRunAnalysisConfig {
public Set<String> workflows = new HashSet<>();
}

public static class Workflows {

public List<WorkflowApprovalRule> rules = new ArrayList<>();
}

public static class Projects {

public List<ProjectTriageRule> rules = new ArrayList<>();
Expand All @@ -105,6 +119,25 @@ public static class ProjectTriageRule {
public String status;
}

public static class WorkflowApprovalRule {

public WorkflowApprovalCondition allow;
public WorkflowApprovalCondition unless;

}

public static class WorkflowApprovalCondition {
@JsonDeserialize(as = TreeSet.class)
public Set<String> files = new TreeSet<>();

public UserRule users;

}

public static class UserRule {
public Integer minContributions;
}

boolean isFeatureEnabled(Feature feature) {
return features.contains(Feature.ALL) || features.contains(feature);
}
Expand Down