Skip to content

Commit

Permalink
Add externalId in CheckRun
Browse files Browse the repository at this point in the history
  • Loading branch information
blacelle committed Oct 30, 2021
1 parent 1028492 commit 8a906c4
Show file tree
Hide file tree
Showing 8 changed files with 69 additions and 82 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,11 @@
import eu.solven.cleanthat.code_provider.github.event.pojo.IExternalWebhookRelevancyResult;

/**
* Holds the logic to clean a code poointer (e.g. local folder, or Git ref, ...)
* Holds the logic to clean a code pointer (e.g. local folder, or Git ref, ...)
*
* @author Benoit Lacelle
*/
// TODO Related to IGithubRefCleaner
public interface ICodePointerCleaner {

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,8 @@ public WebhookRelevancyResult filterWebhookEventTargetRelevantBranch(ICodeCleane
throw new IllegalStateException("Should not happen");
}
if (optSha1.isPresent()) {
createCheckRun(githubAuthAsInst, baseRepo, optSha1.get());
String eventKey = githubEvent.getxGithubDelivery();
createCheckRun(githubAuthAsInst, baseRepo, optSha1.get(), eventKey);
}
IGithubRefCleaner cleaner = cleanerFactory.makeCleaner(githubAuthAsInst);
// BEWARE this branch may not exist: either it is a cleanthat branch yet to create. Or it may be deleted in the
Expand All @@ -379,16 +380,17 @@ public WebhookRelevancyResult filterWebhookEventTargetRelevantBranch(ICodeCleane
return WebhookRelevancyResult.relevant(refToClean.get());
}

public void createCheckRun(GithubAndToken githubAuthAsInst, GHRepository baseRepo, String sha1) {
public void createCheckRun(GithubAndToken githubAuthAsInst, GHRepository baseRepo, String sha1, String eventKey) {
if (GHPermissionType.WRITE == githubAuthAsInst.getPermissions().get(PERMISSION_CHECKS)) {
// https://docs.github.com/en/developers/webhooks-and-events/webhooks/webhook-events-and-payloads#check_run
// https://docs.github.com/en/rest/reference/checks#runs
// https://docs.github.com/en/rest/reference/permissions-required-for-github-apps#permission-on-checks
GHCheckRunBuilder checkRunBuilder = baseRepo.createCheckRun("CleanThat", sha1);
GHCheckRunBuilder checkRunBuilder = baseRepo.createCheckRun("CleanThat", sha1).withExternalID(eventKey);
try {
// baseRepo.getCheckRuns("master").asList();
GHCheckRun checkRun = checkRunBuilder.create();
checkRun.update().withConclusion(Conclusion.SUCCESS).withStatus(Status.COMPLETED);
GHCheckRun checkRun = checkRunBuilder.withStatus(Status.IN_PROGRESS).create();

// We complete right now, until we are able to complete this properly
checkRun.update().withConclusion(Conclusion.SUCCESS).withStatus(Status.COMPLETED).create();
} catch (IOException e) {
// https://github.community/t/resource-not-accessible-when-trying-to-read-write-checkrun/193493
if (LOGGER.isDebugEnabled()) {
Expand Down Expand Up @@ -512,14 +514,16 @@ public void doExecuteWebhookEvent(ICodeCleanerFactory cleanerFactory, IWebhookEv
LOGGER.debug("The changes would have been committed directly in the head branch");
}

// This is useful to investigate unexpected rateLimitHit
{
try {
GHRateLimit rateLimit = github.getRateLimit();
LOGGER.info("After process, rateLimit={} for installationId={}", rateLimit, installationId);
} catch (IOException e) {
LOGGER.warn("Issue with RateLimit", e);
}
logAfterCleaning(installationId, github);
}

public void logAfterCleaning(long installationId, GitHub github) {
try {
// This is useful to investigate unexpected rateLimitHit
GHRateLimit rateLimit = github.getRateLimit();
LOGGER.info("After process, rateLimit={} for installationId={}", rateLimit, installationId);
} catch (IOException e) {
LOGGER.warn("Issue with RateLimit", e);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,14 @@ public static GithubWebhookEvent fromCleanThatEvent(IWebhookEvent githubAccepted
throw new IllegalArgumentException("This does not hold a github event");
}

return new GithubWebhookEvent(PepperMapHelper.getRequiredMap(body, "github", "body"));
Map<String, ?> headers = PepperMapHelper.getRequiredMap(body, "github", "headers");
String xGithubEvent = PepperMapHelper.getOptionalString(headers, "X-GitHub-Event").orElse("");
String xGithubDelivery = PepperMapHelper.getOptionalString(headers, "X-GitHub-Delivery").orElse("");
String xGithubSignature256 = PepperMapHelper.getOptionalString(headers, "X-GitHub-Signature-256").orElse("");

return new GithubWebhookEvent(xGithubEvent,
xGithubDelivery,
xGithubSignature256,
PepperMapHelper.getRequiredMap(body, "github", "body"));
}
}
Original file line number Diff line number Diff line change
@@ -1,17 +1,12 @@
package eu.solven.cleanthat.code_provider.github.refs;

import java.io.IOException;
import java.io.UncheckedIOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.function.Consumer;

import org.eclipse.jgit.api.Git;
import org.eclipse.jgit.api.errors.GitAPIException;
import org.kohsuke.github.GHFileNotFoundException;
import org.kohsuke.github.GHPullRequest;
import org.kohsuke.github.GHPullRequestFileDetail;
Expand Down Expand Up @@ -54,11 +49,6 @@ public void listFiles(Consumer<ICodeProviderFile> consumer) throws IOException {
});
}

// @Override
// public boolean deprecatedFileIsRemoved(Object file) {
// return "removed".equals(((GHPullRequestFileDetail) file).getStatus());
// }

public static String loadContent(GHPullRequest pr, String filename) throws IOException {
GHRepository repository = pr.getRepository();
String sha1 = pr.getHead().getSha();
Expand All @@ -78,14 +68,7 @@ public String getTitle() {
@Override
public void persistChanges(Map<String, String> pathToMutatedContent,
List<String> prComments,
Collection<String> prLabels
// ,
// Optional<String> targetBranch
) {
// if (targetBranch.isPresent()) {
// throw new UnsupportedOperationException("TODO");
// }

Collection<String> prLabels) {
String refName = pr.getHead().getRef();
GHRepository repo = pr.getRepository();

Expand Down Expand Up @@ -119,27 +102,4 @@ public Optional<String> loadContentForPath(String path) throws IOException {
public String getRepoUri() {
return pr.getRepository().getGitTransportUrl();
}

// @Override
public Git makeGitRepo() {
Path tmpDir;
try {
tmpDir = Files.createTempDirectory("cleanthat-clone");
} catch (IOException e) {
throw new UncheckedIOException(e);
}

try {
GHRepository repo = pr.getRepository();
return Git.cloneRepository()
.setURI(repo.getGitTransportUrl())
.setDirectory(tmpDir.toFile())
.setBranch(pr.getHead().getRef())
.setCloneAllBranches(false)
.setCloneSubmodules(false)
.call();
} catch (GitAPIException e) {
throw new IllegalArgumentException(e);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,10 @@
*/
public class GithubRefDiffCodeProvider extends AGithubCodeProvider
implements IListOnlyModifiedFiles, ICodeProviderWriter {
private static final Logger LOGGER = LoggerFactory.getLogger(GithubRefDiffCodeProvider.class);

private static final int LIMIT_COMMIT_IN_COMPARE = 250;

private static final Logger LOGGER = LoggerFactory.getLogger(GithubRefDiffCodeProvider.class);

final String token;
final GHRepository baseRepository;
final GHRef base;
Expand Down Expand Up @@ -82,11 +81,6 @@ public void listFiles(Consumer<ICodeProviderFile> consumer) throws IOException {
});
}

// @Override
// public boolean deprecatedFileIsRemoved(Object file) {
// return "removed".equals(((GHPullRequestFileDetail) file).getStatus());
// }

@Override
public String getHtmlUrl() {
return diffSupplier.get().getHtmlUrl().toExternalForm();
Expand All @@ -100,14 +94,7 @@ public String getTitle() {
@Override
public void persistChanges(Map<String, String> pathToMutatedContent,
List<String> prComments,
Collection<String> prLabels
// ,
// Optional<String> targetBranch
) {
// if (targetBranch.isPresent()) {
// throw new UnsupportedOperationException("TODO");
// }

Collection<String> prLabels) {
String refName = head.getRef();
GHRepository repo = baseRepository;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import java.util.Collection;
import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
Expand Down Expand Up @@ -126,8 +127,30 @@ public IWebhookEvent wrapAsEvent(Map<String, ?> input) {
if (input.containsKey(KEY_BODY) && input.containsKey("headers")) {
// see CheckWebhooksLambdaFunction.saveToDynamoDb(String, IWebhookEvent, AmazonDynamoDB)
// event = SaveToDynamoDb.NONE;
event = new CleanThatWebhookEvent((Map<String, ?>) input.get("headers"),
(Map<String, ?>) input.get(KEY_BODY));

Map<String, Object> rootBody = PepperMapHelper.getRequiredMap(input, KEY_BODY);

if (rootBody.containsKey("github")) {
Map<String, Object> github = PepperMapHelper.getRequiredMap(rootBody, "github");

Map<String, Object> githubHeaders = PepperMapHelper.getRequiredMap(github, "headers");

// Headers is typically empty as we fails fetching headers from API Gateway
if (!githubHeaders.containsKey("X-GitHub-Delivery")) {
githubHeaders = new LinkedHashMap<>(githubHeaders);

// TODO We should push this to the headers next to the actual github body, which may be deeper
// We should also push it when the initial event is received
String eventKey = PepperMapHelper.getRequiredString(input, "X-GitHub-Delivery");
githubHeaders.put("X-GitHub-Delivery", eventKey);

// Install the updated headers
github.put("headers", githubHeaders);
}
}

Map<String, Object> headers = PepperMapHelper.getRequiredMap(input, "headers");
event = new CleanThatWebhookEvent(headers, rootBody);
} else {
event = new GithubWebhookEvent(input);
}
Expand All @@ -144,12 +167,11 @@ public IWebhookEvent wrapAsEvent(Map<String, ?> input) {

String eventName = PepperMapHelper.getRequiredString(r, "eventName");

if (!"INSERT".equals(eventName) && !"MODIFY".equals(eventName)) {
// https://docs.aws.amazon.com/amazondynamodb/latest/APIReference/API_streams_Record.html
// We are in a REMOVE event
LOGGER.info("We discard eventName={}", eventName);
asMap = Collections.emptyMap();
} else {
if (
// INSERT event: this is something new process
"INSERT".equals(eventName)
// MODIFY event: this is typically an admin which modify an event manually
|| "MODIFY".equals(eventName)) {
Map<String, ?> dynamoDbMap = PepperMapHelper.getRequiredMap(r, "dynamodb", "NewImage");

// We receive from DynamoDb a json in a special format
Expand All @@ -164,6 +186,11 @@ public IWebhookEvent wrapAsEvent(Map<String, ?> input) {

LOGGER.info("Processing X-GitHub-Delivery={}",
PepperMapHelper.getRequiredString(asMap, "X-GitHub-Delivery"));
} else {
// https://docs.aws.amazon.com/amazondynamodb/latest/APIReference/API_streams_Record.html
// We are in a REMOVE event
LOGGER.info("We discard eventName={}", eventName);
asMap = Collections.emptyMap();
}
return asMap;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@

@RunWith(SpringRunner.class)
@ContextConfiguration(classes = { ExecuteCleaningWebhooksLambdaFunction.class })
public class ITCheckRepoLocallyInDynamoDb {
public class ITCheckDynamoDbEventLocally {
GithubRefCleaner cleaner;

@Autowired
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@

@RunWith(SpringRunner.class)
@ContextConfiguration(classes = { ExecuteCleaningWebhooksLambdaFunction.class })
public class ITCleanEventLocallyInDynamoDb {
public class ITCleanDynamoDbEventLocally {
GithubRefCleaner cleaner;

@Autowired
Expand Down

0 comments on commit 8a906c4

Please sign in to comment.