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

tester: allow configurable allowlist #753

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -23,8 +23,11 @@
package org.openjdk.skara.bots.tester;

import org.openjdk.skara.forge.PullRequest;
import org.openjdk.skara.host.HostUser;
import org.openjdk.skara.issuetracker.Comment;

import java.util.function.Predicate;

class State {
private final Stage stage;
private final Comment requested;
@@ -77,7 +80,7 @@ Comment finished() {
return finished;
}

static State from(PullRequest pr, String approverGroupId) {
static State from(PullRequest pr, Predicate<HostUser> validate) {
Comment requested = null;
Comment pending = null;
Comment approval = null;
@@ -127,15 +130,15 @@ static State from(PullRequest pr, String approverGroupId) {
}
} else if (body.equals("/test approve")) {
approval = comment;
if (host.isMemberOf(approverGroupId, author)) {
if (validate.test(author)) {
isApproved = true;
}
} else if (body.equals("/test cancel")) {
if (comment.author().equals(requested.author())) {
cancelled = comment;
}
} else if (body.startsWith("/test")) {
if (host.isMemberOf(approverGroupId, author)) {
if (validate.test(author)) {
isApproved = true;
}
}
@@ -47,6 +47,7 @@ private static class Observation {
private final Logger log = Logger.getLogger("org.openjdk.skara.bots");;
private final ContinuousIntegration ci;
private final String approversGroupId;
private final Set<String> allowlist;
private final List<String> availableJobs;
private final List<String> defaultJobs;
private final String name;
@@ -57,13 +58,15 @@ private static class Observation {

TestBot(ContinuousIntegration ci,
String approversGroupId,
Set<String> allowlist,
List<String> availableJobs,
List<String> defaultJobs,
String name,
Path storage,
HostedRepository repo) {
this.ci = ci;
this.approversGroupId = approversGroupId;
this.allowlist = allowlist;
this.availableJobs = availableJobs;
this.defaultJobs = defaultJobs;
this.name = name;
@@ -83,6 +86,7 @@ public List<WorkItem> getPeriodicItems() {
if (cache.needsUpdate(pr)) {
ret.add(new TestWorkItem(ci,
approversGroupId,
allowlist,
availableJobs,
defaultJobs,
name,
@@ -121,6 +125,7 @@ public List<WorkItem> getPeriodicItems() {
if (shouldUpdate) {
ret.add(new TestWorkItem(ci,
approversGroupId,
allowlist,
availableJobs,
defaultJobs,
name,
@@ -52,13 +52,14 @@ public List<Bot> create(BotConfiguration configuration) {
var specific = configuration.specific();

var approvers = specific.get("approvers").asString();
var allowlist = specific.get("allowlist").stream().map(JSONValue::asString).collect(Collectors.toSet());
var availableJobs = specific.get("availableJobs").stream().map(JSONValue::asString).collect(Collectors.toList());
var defaultJobs = specific.get("defaultJobs").stream().map(JSONValue::asString).collect(Collectors.toList());
var name = specific.get("name").asString();
var ci = configuration.continuousIntegration(specific.get("ci").asString());
for (var repo : specific.get("repositories").asArray()) {
var hostedRepo = configuration.repository(repo.asString());
ret.add(new TestBot(ci, approvers, availableJobs, defaultJobs, name, storage, hostedRepo));
ret.add(new TestBot(ci, approvers, allowlist, availableJobs, defaultJobs, name, storage, hostedRepo));
}

return ret;
@@ -26,6 +26,7 @@
import org.openjdk.skara.ci.*;
import org.openjdk.skara.forge.*;
import org.openjdk.skara.vcs.*;
import org.openjdk.skara.host.HostUser;

import java.io.*;
import java.net.*;
@@ -34,22 +35,25 @@
import java.util.*;
import java.util.logging.Logger;
import java.util.stream.*;
import java.util.function.Predicate;

public class TestWorkItem implements WorkItem {
private final Logger log = Logger.getLogger("org.openjdk.skara.bots");;
private final ContinuousIntegration ci;
private final String approversGroupId;
private final Set<String> allowlist;
private final List<String> availableJobs;
private final List<String> defaultJobs;
private final String name;
private final Path storage;
private final HostedRepository repository;
private final PullRequest pr;

TestWorkItem(ContinuousIntegration ci, String approversGroupId, List<String> availableJobs,
TestWorkItem(ContinuousIntegration ci, String approversGroupId, Set<String> allowlist, List<String> availableJobs,
List<String> defaultJobs, String name, Path storage, PullRequest pr) {
this.ci = ci;
this.approversGroupId = approversGroupId;
this.allowlist = allowlist;
this.availableJobs = availableJobs;
this.defaultJobs = defaultJobs;
this.name = name;
@@ -246,9 +250,14 @@ private String display(Job job) {
return sb.toString();
}

private boolean validate(HostUser u) {
var forge = pr.repository().forge();
return forge.isMemberOf(approversGroupId, u) || allowlist.contains(u.id());
}

@Override
public Collection<WorkItem> run(Path scratchPath) {
var state = State.from(pr, approversGroupId);
var state = State.from(pr, this::validate);
var stage = state.stage();
if (stage == Stage.NA || stage == Stage.ERROR || stage == Stage.PENDING || stage == Stage.FINISHED) {
// nothing to do
@@ -48,7 +48,7 @@ void noCommentsShouldEqualNA() {
pr.author = duke;
pr.comments = List.of();

var state = State.from(pr, "0");
var state = State.from(pr, u -> host.isMemberOf("0", u));
assertEquals(Stage.NA, state.stage());
assertEquals(null, state.requested());
assertEquals(null, state.pending());
@@ -78,7 +78,7 @@ void testCommentFromNotApprovedUserShouldEqualRequested() {
var approvers = "0";
host.groups = Map.of(approvers, Set.of());

var state = State.from(pr, approvers);
var state = State.from(pr, u -> host.isMemberOf(approvers, u));
assertEquals(Stage.REQUESTED, state.stage());
assertEquals(comment, state.requested());
assertEquals(null, state.pending());
@@ -108,7 +108,7 @@ void testCommentFromApprovedUserShouldEqualApproved() {
var approvers = "0";
host.groups = Map.of(approvers, Set.of(duke));

var state = State.from(pr, approvers);
var state = State.from(pr, u -> host.isMemberOf(approvers, u));
assertEquals(Stage.APPROVED, state.stage());
assertEquals(comment, state.requested());
assertEquals(null, state.pending());
@@ -143,7 +143,7 @@ void testApprovalNeededCommentShouldResultInPending() {
pr.comments = List.of(testComment, pendingComment);
host.groups = Map.of("0", Set.of());

var state = State.from(pr, "0");
var state = State.from(pr, u -> host.isMemberOf("0", u));
assertEquals(Stage.PENDING, state.stage());
assertEquals(testComment, state.requested());
assertEquals(pendingComment, state.pending());
@@ -191,7 +191,7 @@ void testStartedCommentShouldResultInRunning() {
var approvers = "0";
host.groups = Map.of(approvers, Set.of(member));

var state = State.from(pr, approvers);
var state = State.from(pr, u -> host.isMemberOf(approvers, u));
assertEquals(Stage.STARTED, state.stage());
assertEquals(testComment, state.requested());
assertEquals(pendingComment, state.pending());
@@ -222,7 +222,7 @@ void cancelCommentFromAuthorShouldEqualCancelled() {
var approvers = "0";
host.groups = Map.of(approvers, Set.of());

var state = State.from(pr, approvers);
var state = State.from(pr, u -> host.isMemberOf(approvers, u));
assertEquals(Stage.CANCELLED, state.stage());
assertEquals(testComment, state.requested());
assertEquals(cancelComment, state.cancelled());
@@ -256,7 +256,7 @@ void cancelCommentFromAnotherUserShouldHaveNoEffect() {
var approvers = "0";
host.groups = Map.of(approvers, Set.of());

var state = State.from(pr, approvers);
var state = State.from(pr, u -> host.isMemberOf(approvers, u));
assertEquals(Stage.REQUESTED, state.stage());
assertEquals(testComment, state.requested());
assertEquals(null, state.cancelled());
@@ -289,7 +289,7 @@ void multipleTestCommentsShouldOnlyCareAboutLast() {
var approvers = "0";
host.groups = Map.of(approvers, Set.of());

var state = State.from(pr, approvers);
var state = State.from(pr, u -> host.isMemberOf(approvers, u));
assertEquals(Stage.REQUESTED, state.stage());
assertEquals(test3Comment, state.requested());
assertEquals(null, state.cancelled());
@@ -326,7 +326,7 @@ void errorAfterRequestedShouldBeError() {
var approvers = "0";
host.groups = Map.of(approvers, Set.of());

var state = State.from(pr, approvers);
var state = State.from(pr, u -> host.isMemberOf(approvers, u));
assertEquals(Stage.ERROR, state.stage());
assertEquals(testComment, state.requested());
assertEquals(null, state.pending());
@@ -381,7 +381,7 @@ void testFinishedCommentShouldResultInFinished() {
var approvers = "0";
host.groups = Map.of(approvers, Set.of(member));

var state = State.from(pr, approvers);
var state = State.from(pr, u -> host.isMemberOf(approvers, u));
assertEquals(Stage.FINISHED, state.stage());
assertEquals(testComment, state.requested());
assertEquals(pendingComment, state.pending());
@@ -49,7 +49,7 @@ void noTestCommentShouldDoNothing(TestInfo testInfo) throws IOException {

var storage = tmp.path().resolve("storage");
var ci = new InMemoryContinuousIntegration();
var bot = new TestBot(ci, "0", List.of(), List.of(), "", storage, upstreamHostedRepo);
var bot = new TestBot(ci, "0", Set.of(), List.of(), List.of(), "", storage, upstreamHostedRepo);
var runner = new TestBotRunner();

runner.runPeriodicItems(bot);