mirror of
https://github.com/sourcegraph/sourcegraph.git
synced 2026-02-06 15:31:48 +00:00
Removed pr-auditor and left redirect notice (#59497)
removed pr-auditor and left redirect notice
This commit is contained in:
James McNamara
GitHub
parent
39eb9f3636
commit
ec24769023
@ -1,43 +0,0 @@
|
||||
load("//dev:go_defs.bzl", "go_test")
|
||||
load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library")
|
||||
|
||||
go_library(
|
||||
name = "pr-auditor_lib",
|
||||
srcs = [
|
||||
"check.go",
|
||||
"issue.go",
|
||||
"main.go",
|
||||
"webhook.go",
|
||||
],
|
||||
importpath = "github.com/sourcegraph/sourcegraph/dev/pr-auditor",
|
||||
visibility = ["//visibility:private"],
|
||||
deps = [
|
||||
"//lib/errors",
|
||||
"@com_github_google_go_github_v55//github",
|
||||
"@com_github_grafana_regexp//:regexp",
|
||||
"@org_golang_x_exp//slices",
|
||||
"@org_golang_x_oauth2//:oauth2",
|
||||
],
|
||||
)
|
||||
|
||||
go_binary(
|
||||
name = "pr-auditor",
|
||||
embed = [":pr-auditor_lib"],
|
||||
visibility = ["//visibility:public"],
|
||||
)
|
||||
|
||||
go_test(
|
||||
name = "pr-auditor_test",
|
||||
timeout = "short",
|
||||
srcs = [
|
||||
"check_test.go",
|
||||
"issue_test.go",
|
||||
],
|
||||
data = glob(["testdata/**"]),
|
||||
embed = [":pr-auditor_lib"],
|
||||
deps = [
|
||||
"@com_github_google_go_cmp//cmp",
|
||||
"@com_github_stretchr_testify//assert",
|
||||
"@com_github_stretchr_testify//require",
|
||||
],
|
||||
)
|
||||
@ -1,30 +1,3 @@
|
||||
# pr-auditor [](https://github.com/sourcegraph/sourcegraph/actions/workflows/pr-auditor.yml)
|
||||
# pr-auditor
|
||||
|
||||
`pr-auditor` is a tool designed to operate on some [GitHub Actions pull request events](https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request) in order to check for SOC2 compliance.
|
||||
Owned by the [DevInfra team](https://handbook.sourcegraph.com/departments/engineering/teams/devinfra/).
|
||||
|
||||
Learn more: [Testing principles and guidelines](https://docs.sourcegraph.com/dev/background-information/testing_principles)
|
||||
|
||||
## Usage
|
||||
|
||||
This action is primarily designed to run on GitHub Actions, and leverages the [pull request event payloads](https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request) extensively.
|
||||
|
||||
The optional `-protected-branch` flag defines a base branch that always opens a PR audit issue to track all pull requests made to it.
|
||||
|
||||
```sh
|
||||
GITHUB_EVENT_PATH="/path/to/json/payload.json"
|
||||
GITHUB_TOKEN="personal-access-token"
|
||||
|
||||
# run directly
|
||||
go run ./dev/pr-auditor/ check \
|
||||
-github.payload-path="$GITHUB_EVENT_PATH" \
|
||||
-github.token="$GITHUB_TOKEN" \
|
||||
-protected-branch="release"
|
||||
|
||||
# run using wrapper script
|
||||
./dev/buildchecker/check-pr.sh
|
||||
```
|
||||
|
||||
## Deployment
|
||||
|
||||
`pr-auditor` can be deployed to repositories using the available [batch changes](./batch-changes/README.md).
|
||||
This action has been moved to https://github.com/sourcegraph/pr-auditor
|
||||
|
||||
@ -1,16 +0,0 @@
|
||||
# pr-auditor batch changes
|
||||
|
||||
## Rollouts
|
||||
|
||||
[`pr-auditor-rollout.yml`](./pr-auditor-rollout.yml) describes a batch change that can be used to roll out `pr-auditor` to repositories that do not have it set up yet.
|
||||
|
||||
To use it:
|
||||
|
||||
1. [Create a batch change on `/batch-changes/create`](https://k8s.sgdev.org/batch-changes/create)
|
||||
2. Paste [`pr-auditor-rollout.yml`](./pr-auditor-rollout.yml) into the spec input and make the appropriate changes to the template (for example, adjust the `on:` parameters to target the desired repositories)
|
||||
3. Preview and apply the changes
|
||||
4. Selectively publish the appropriate changesets for review and merge
|
||||
|
||||
## Updates
|
||||
|
||||
The [`pr-auditor-patch.yml`](./pr-auditor-patch.yml) spec describes a batch change that can be used to roll out `pr-auditor` workflow updates to repositories that already have it set up by synchronizing them with the source-of-truth script in the batch change spec.
|
||||
@ -1,51 +0,0 @@
|
||||
# Describes a batch change for rolling out pr-auditor workflow updates
|
||||
|
||||
name: pr-auditor-patch
|
||||
description: Roll out pr-auditor workflow patch.
|
||||
|
||||
on:
|
||||
- repositoriesMatchingQuery: |
|
||||
repo:github.com/sourcegraph/.*
|
||||
repohasfile:.github/workflows/pr-auditor.yml
|
||||
count:all
|
||||
|
||||
changesetTemplate:
|
||||
title: 'workflows: update pr-auditor workflow'
|
||||
body: |
|
||||
This pull request updates the pr-auditor workflow.
|
||||
|
||||
Test plan: We have tested the code itself in https://github.com/sourcegraph/sourcegraph/tree/main/dev/pr-auditor,
|
||||
and the pr-auditor action should continue working as expected on this pull request.
|
||||
branch: update-pr-auditor
|
||||
commit:
|
||||
message: 'workflows: update pr-auditor workflow'
|
||||
|
||||
steps:
|
||||
# Overwrite the GitHub action
|
||||
- run: |
|
||||
read -r -d '' TEMPLATE << 'EOF'
|
||||
# See https://docs.sourcegraph.com/dev/background-information/ci#pr-auditor
|
||||
name: pr-auditor
|
||||
on:
|
||||
pull_request_target:
|
||||
types: [ closed, edited, opened, synchronize, ready_for_review ]
|
||||
|
||||
jobs:
|
||||
check-pr:
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- uses: actions/checkout@v2
|
||||
with: { repository: 'sourcegraph/sourcegraph' }
|
||||
- uses: actions/setup-go@v2
|
||||
with: { go-version: '1.18' }
|
||||
|
||||
- run: ./dev/pr-auditor/check-pr.sh
|
||||
env:
|
||||
GITHUB_EVENT_PATH: ${{ "${{ env.GITHUB_EVENT_PATH }}" }}
|
||||
GITHUB_TOKEN: ${{ "${{ secrets.CODENOTIFY_GITHUB_TOKEN }}" }}
|
||||
GITHUB_RUN_URL: https://github.com/${{ "${{ github.repository }}" }}/actions/runs/${{ "${{ github.run_id }}" }}
|
||||
EOF
|
||||
|
||||
rm -rf .github/workflows/pr-auditor.yml
|
||||
echo "$TEMPLATE" >> .github/workflows/pr-auditor.yml
|
||||
container: alpine:3
|
||||
@ -1,73 +0,0 @@
|
||||
# Describes a batch change for rolling out pr-auditor workflows
|
||||
|
||||
name: pr-auditor-rollout
|
||||
description: Roll out pr-auditor workflow to more repositories.
|
||||
|
||||
on:
|
||||
- repositoriesMatchingQuery: |
|
||||
-repohasfile:.github/workflows/pr-auditor.yml count:all
|
||||
(
|
||||
repo:^github\.com/sourcegraph/infrastructure$
|
||||
OR repo:^github\.com/sourcegraph/src-cli$
|
||||
OR repo:^github\.com/sourcegraph/deploy-sourcegraph$
|
||||
OR repo:^github\.com/sourcegraph/deploy-sourcegraph-(docker|cloud|dogfood-k8s|managed|helm)$
|
||||
OR repo:^github\.com/sourcegraph/lsif-.*
|
||||
OR repo:^github\.com/sourcegraph/sbt-sourcegraph$
|
||||
OR repo:^github\.com/sourcegraph/terraform-.*-executors
|
||||
OR repo:^github\.com/sourcegraph/sourcegraph-.*
|
||||
)
|
||||
|
||||
changesetTemplate:
|
||||
title: 'workflows: add pr-auditor and test plans to PR templates'
|
||||
body: |
|
||||
This pull request adds test plans to all PR templates, and introduces a pr-auditor workflow to ensure test plans are provided.
|
||||
For more details see https://docs.sourcegraph.com/dev/background-information/testing_principles
|
||||
|
||||
Test plan: created action should correctly set a status on this pull request.
|
||||
branch: add-pr-auditor-and-test-plan
|
||||
commit:
|
||||
message: 'workflows: add pr-auditor and test plans to PR templates'
|
||||
|
||||
steps:
|
||||
# Add test plans to PR templates
|
||||
- run: |
|
||||
read -r -d '' TEMPLATE << 'EOF'
|
||||
## Test plan
|
||||
|
||||
<!-- All pull requests REQUIRE a test plan: https://docs.sourcegraph.com/dev/background-information/testing_principles -->
|
||||
|
||||
EOF
|
||||
|
||||
mkdir -p .github
|
||||
echo "\n\n$TEMPLATE" >> .github/PULL_REQUEST_TEMPLATE.md
|
||||
echo "\n\n$TEMPLATE" | tee -a $(find -name .github/PULL_REQUEST_TEMPLATE/*.md)
|
||||
container: alpine:3
|
||||
# Create a GitHub action
|
||||
- run: |
|
||||
read -r -d '' TEMPLATE << 'EOF'
|
||||
# See https://docs.sourcegraph.com/dev/background-information/ci#pr-auditor
|
||||
name: pr-auditor
|
||||
on:
|
||||
pull_request_target:
|
||||
types: [ closed, edited, opened, synchronize, ready_for_review ]
|
||||
|
||||
jobs:
|
||||
check-pr:
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- uses: actions/checkout@v2
|
||||
with: { repository: 'sourcegraph/sourcegraph' }
|
||||
- uses: actions/setup-go@v2
|
||||
with: { go-version: '1.18' }
|
||||
|
||||
- run: ./dev/pr-auditor/check-pr.sh
|
||||
env:
|
||||
GITHUB_EVENT_PATH: ${{ "${{ env.GITHUB_EVENT_PATH }}" }}
|
||||
GITHUB_TOKEN: ${{ "${{ secrets.CODENOTIFY_GITHUB_TOKEN }}" }}
|
||||
GITHUB_RUN_URL: https://github.com/${{ "${{ github.repository }}" }}/actions/runs/${{ "${{ github.run_id }}" }}
|
||||
EOF
|
||||
|
||||
mkdir -p .github/workflows
|
||||
rm -rf .github/workflows/pr-auditor.yml
|
||||
echo "$TEMPLATE" >> .github/workflows/pr-auditor.yml
|
||||
container: alpine:3
|
||||
@ -1,14 +0,0 @@
|
||||
#! /usr/bin/env bash
|
||||
|
||||
# Make this script independent of where it's called
|
||||
cd "$(dirname "${BASH_SOURCE[0]}")"/../..
|
||||
|
||||
set -eu
|
||||
|
||||
echo "--- Running 'pr-auditor'"
|
||||
go run ./dev/pr-auditor/ \
|
||||
-github.payload-path="$GITHUB_EVENT_PATH" \
|
||||
-github.token="$GITHUB_TOKEN" \
|
||||
-github.run-url="$GITHUB_RUN_URL" \
|
||||
-skip-check-test-plan="${SKIP_CHECK_TEST_PLAN:-False}" \
|
||||
-skip-check-reviews="${SKIP_CHECK_REVIEWS:-False}"
|
||||
@ -1,117 +0,0 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"context"
|
||||
"strings"
|
||||
|
||||
"github.com/google/go-github/v55/github"
|
||||
"github.com/grafana/regexp"
|
||||
"golang.org/x/exp/slices"
|
||||
)
|
||||
|
||||
type checkResult struct {
|
||||
// ReviewSatisfied indicates that *any* review has been made on the PR. It is also set to
|
||||
// true if the test plan indicates that this PR does not need to be review.
|
||||
ReviewSatisfied bool
|
||||
// CanSkipTestPlan indicates that the test plan is not required for audit.
|
||||
CanSkipTestPlan bool
|
||||
// TestPlan is the content provided after the acceptance checklist checkbox.
|
||||
TestPlan string
|
||||
// ProtectedBranch indicates that the base branch for this PR is protected and merges
|
||||
// are considered to be exceptional and should always be justified.
|
||||
ProtectedBranch bool
|
||||
// Error indicating any issue that might have occured during the check.
|
||||
Error error
|
||||
}
|
||||
|
||||
func (r checkResult) IsSatisfied() bool {
|
||||
return r.IsTestPlanSatisfied() && r.ReviewSatisfied && !r.ProtectedBranch
|
||||
}
|
||||
|
||||
func (r checkResult) IsTestPlanSatisfied() bool {
|
||||
return r.CanSkipTestPlan || r.TestPlan != ""
|
||||
}
|
||||
|
||||
var (
|
||||
testPlanDividerRegexp = regexp.MustCompile("(?m)(#+ Test [pP]lan)|(Test [pP]lan:)")
|
||||
noReviewNeededDividerRegexp = regexp.MustCompile("(?m)([nN]o [rR]eview [rR]equired:)")
|
||||
|
||||
markdownCommentRegexp = regexp.MustCompile("<!--((.|\n)*?)-->(\n)*")
|
||||
|
||||
noReviewNeedLabels = []string{"no-review-required", "automerge"}
|
||||
)
|
||||
|
||||
type checkOpts struct {
|
||||
SkipReviews bool
|
||||
SkipTestPlan bool
|
||||
ProtectedBranch string
|
||||
}
|
||||
|
||||
func isProtectedBranch(payload *EventPayload, protectedBranch string) bool {
|
||||
return protectedBranch != "" && payload.PullRequest.Base.Ref == protectedBranch
|
||||
}
|
||||
|
||||
func checkPR(ctx context.Context, ghc *github.Client, payload *EventPayload, opts checkOpts) checkResult {
|
||||
pr := payload.PullRequest
|
||||
|
||||
// Whether or not this PR was reviewed can be inferred from payload, but an approval
|
||||
// might not have any comments so we need to double-check through the GitHub API
|
||||
var err error
|
||||
reviewed := pr.ReviewComments > 0
|
||||
if !reviewed && !opts.SkipReviews {
|
||||
owner, repo := payload.Repository.GetOwnerAndName()
|
||||
var reviews []*github.PullRequestReview
|
||||
// Continue, but return err later
|
||||
reviews, _, err = ghc.PullRequests.ListReviews(ctx, owner, repo, payload.PullRequest.Number, &github.ListOptions{})
|
||||
reviewed = len(reviews) > 0
|
||||
}
|
||||
|
||||
// Parse test plan data from body
|
||||
sections := testPlanDividerRegexp.Split(pr.Body, 2)
|
||||
if len(sections) < 2 {
|
||||
return checkResult{
|
||||
ReviewSatisfied: reviewed,
|
||||
CanSkipTestPlan: opts.SkipTestPlan,
|
||||
Error: err,
|
||||
}
|
||||
}
|
||||
|
||||
testPlan := cleanMarkdown(sections[1])
|
||||
|
||||
// Look for no review required explanation in the test plan
|
||||
if sections := noReviewNeededDividerRegexp.Split(testPlan, 2); len(sections) > 1 {
|
||||
noReviewRequiredExplanation := cleanMarkdown(sections[1])
|
||||
if len(noReviewRequiredExplanation) > 0 {
|
||||
reviewed = true
|
||||
}
|
||||
}
|
||||
|
||||
if testPlan != "" {
|
||||
for _, label := range pr.Labels {
|
||||
if slices.Contains(noReviewNeedLabels, label.Name) {
|
||||
reviewed = true
|
||||
break
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
mergeAgainstProtected := isProtectedBranch(payload, opts.ProtectedBranch)
|
||||
|
||||
return checkResult{
|
||||
ReviewSatisfied: reviewed,
|
||||
CanSkipTestPlan: opts.SkipTestPlan,
|
||||
TestPlan: testPlan,
|
||||
ProtectedBranch: mergeAgainstProtected,
|
||||
Error: err,
|
||||
}
|
||||
}
|
||||
|
||||
func cleanMarkdown(s string) string {
|
||||
content := s
|
||||
// Remove comments
|
||||
content = markdownCommentRegexp.ReplaceAllString(content, "")
|
||||
// Remove whitespace
|
||||
content = strings.TrimSpace(content)
|
||||
|
||||
return content
|
||||
}
|
||||
@ -1,199 +0,0 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"context"
|
||||
"os"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/google/go-cmp/cmp"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
func TestCheckTestPlan(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
bodyFile string
|
||||
labels []string
|
||||
baseBranch string
|
||||
protectedBranch string
|
||||
canSkipTestPlan bool
|
||||
want checkResult
|
||||
}{
|
||||
{
|
||||
name: "has test plan",
|
||||
bodyFile: "testdata/pull_request_body/has-plan.md",
|
||||
want: checkResult{
|
||||
ReviewSatisfied: false,
|
||||
TestPlan: "I have a plan!",
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "protected branch",
|
||||
bodyFile: "testdata/pull_request_body/has-plan.md",
|
||||
baseBranch: "release",
|
||||
protectedBranch: "release",
|
||||
want: checkResult{
|
||||
ReviewSatisfied: false,
|
||||
TestPlan: "I have a plan!",
|
||||
ProtectedBranch: true,
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "non protected branch",
|
||||
bodyFile: "testdata/pull_request_body/has-plan.md",
|
||||
baseBranch: "preprod",
|
||||
protectedBranch: "release",
|
||||
want: checkResult{
|
||||
ReviewSatisfied: false,
|
||||
TestPlan: "I have a plan!",
|
||||
ProtectedBranch: false,
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "no test plan",
|
||||
bodyFile: "testdata/pull_request_body/no-plan.md",
|
||||
want: checkResult{
|
||||
ReviewSatisfied: false,
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "complicated test plan",
|
||||
bodyFile: "testdata/pull_request_body/has-plan-fancy.md",
|
||||
want: checkResult{
|
||||
ReviewSatisfied: false,
|
||||
TestPlan: `This is a plan!
|
||||
Quite lengthy
|
||||
|
||||
And a little complicated; there's also the following reasons:
|
||||
|
||||
1. A
|
||||
2. B
|
||||
3. C`,
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "inline test plan",
|
||||
bodyFile: "testdata/pull_request_body/inline-plan.md",
|
||||
want: checkResult{
|
||||
ReviewSatisfied: false,
|
||||
TestPlan: `This is a plan!
|
||||
Quite lengthy
|
||||
|
||||
And a little complicated; there's also the following reasons:
|
||||
|
||||
1. A
|
||||
2. B
|
||||
3. C`,
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "no review required",
|
||||
bodyFile: "testdata/pull_request_body/no-review-required.md",
|
||||
want: checkResult{
|
||||
ReviewSatisfied: true,
|
||||
TestPlan: "I have a plan! No review required: this is a bot PR",
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "bad markdown still passes",
|
||||
bodyFile: "testdata/pull_request_body/bad-markdown.md",
|
||||
want: checkResult{
|
||||
ReviewSatisfied: true,
|
||||
TestPlan: "This is still a plan! No review required: just trust me",
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "no review required via automerge label",
|
||||
bodyFile: "testdata/pull_request_body/has-plan.md",
|
||||
labels: []string{"automerge"},
|
||||
want: checkResult{
|
||||
ReviewSatisfied: true,
|
||||
TestPlan: "I have a plan!",
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "no review required via no-review-required label",
|
||||
bodyFile: "testdata/pull_request_body/has-plan.md",
|
||||
labels: []string{"no-review-required"},
|
||||
want: checkResult{
|
||||
ReviewSatisfied: true,
|
||||
TestPlan: "I have a plan!",
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "no review required via automerge label but no plan",
|
||||
bodyFile: "testdata/pull_request_body/no-plan.md",
|
||||
labels: []string{"automerge"},
|
||||
want: checkResult{
|
||||
ReviewSatisfied: false,
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "no review required via no-review-required label but no plan",
|
||||
bodyFile: "testdata/pull_request_body/no-plan.md",
|
||||
labels: []string{"no-review-required"},
|
||||
want: checkResult{
|
||||
ReviewSatisfied: false,
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "no review required but with the wrong label",
|
||||
bodyFile: "testdata/pull_request_body/has-plan.md",
|
||||
labels: []string{"random-label"},
|
||||
want: checkResult{
|
||||
ReviewSatisfied: false,
|
||||
TestPlan: "I have a plan!",
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "no test plan but skip-test-plans enabled",
|
||||
bodyFile: "testdata/pull_request_body/no-plan.md",
|
||||
canSkipTestPlan: true,
|
||||
want: checkResult{
|
||||
ReviewSatisfied: false,
|
||||
CanSkipTestPlan: true,
|
||||
},
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
body, err := os.ReadFile(tt.bodyFile)
|
||||
require.NoError(t, err)
|
||||
|
||||
payload := &EventPayload{
|
||||
PullRequest: PullRequestPayload{
|
||||
Body: string(body),
|
||||
},
|
||||
}
|
||||
if tt.labels != nil {
|
||||
payload.PullRequest.Labels = make([]Label, len(tt.labels))
|
||||
for i, label := range tt.labels {
|
||||
payload.PullRequest.Labels[i] = Label{Name: label}
|
||||
}
|
||||
}
|
||||
checkOpts := checkOpts{
|
||||
SkipReviews: true,
|
||||
SkipTestPlan: tt.canSkipTestPlan,
|
||||
}
|
||||
|
||||
if tt.baseBranch != "" && tt.protectedBranch != "" {
|
||||
payload.PullRequest.Base = RefPayload{Ref: tt.baseBranch}
|
||||
checkOpts.ProtectedBranch = tt.protectedBranch
|
||||
}
|
||||
|
||||
got := checkPR(context.Background(), nil, payload, checkOpts)
|
||||
assert.Equal(t, tt.want.IsTestPlanSatisfied(), got.IsTestPlanSatisfied())
|
||||
t.Log("got.TestPlan: ", got.TestPlan)
|
||||
if tt.want.TestPlan == "" {
|
||||
assert.Empty(t, got.TestPlan)
|
||||
} else {
|
||||
assert.True(t, strings.Contains(got.TestPlan, tt.want.TestPlan),
|
||||
cmp.Diff(got.TestPlan, tt.want.TestPlan))
|
||||
}
|
||||
assert.Equal(t, tt.want.ProtectedBranch, got.ProtectedBranch)
|
||||
assert.Equal(t, tt.want.ReviewSatisfied, got.ReviewSatisfied)
|
||||
})
|
||||
}
|
||||
}
|
||||
@ -1,76 +0,0 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
|
||||
"github.com/google/go-github/v55/github"
|
||||
)
|
||||
|
||||
const (
|
||||
testPlanDocs = "https://docs.sourcegraph.com/dev/background-information/testing_principles#test-plans"
|
||||
)
|
||||
|
||||
func generateExceptionIssue(payload *EventPayload, result *checkResult, additionalContext string) *github.IssueRequest {
|
||||
// 🚨 SECURITY: Do not reference other potentially sensitive fields of pull requests
|
||||
prTitle := payload.PullRequest.Title
|
||||
if payload.Repository.Private {
|
||||
prTitle = "<redacted>"
|
||||
}
|
||||
|
||||
var (
|
||||
issueTitle = fmt.Sprintf("%s#%d: %q", payload.Repository.FullName, payload.PullRequest.Number, prTitle)
|
||||
issueBody string
|
||||
exceptionLabels = []string{}
|
||||
issueAssignees = []string{}
|
||||
)
|
||||
|
||||
if !result.ReviewSatisfied {
|
||||
exceptionLabels = append(exceptionLabels, "exception/review")
|
||||
}
|
||||
|
||||
if !result.IsTestPlanSatisfied() {
|
||||
exceptionLabels = append(exceptionLabels, "exception/test-plan")
|
||||
}
|
||||
if result.ProtectedBranch {
|
||||
exceptionLabels = append(exceptionLabels, "exception/protected-branch")
|
||||
}
|
||||
|
||||
if !result.ReviewSatisfied {
|
||||
if result.IsTestPlanSatisfied() {
|
||||
issueBody = fmt.Sprintf("%s %q **has a test plan (or does not require one)** but **was not reviewed**.", payload.PullRequest.URL, prTitle)
|
||||
} else {
|
||||
issueBody = fmt.Sprintf("%s %q **has no test plan** and **was not reviewed**.", payload.PullRequest.URL, prTitle)
|
||||
}
|
||||
} else if !result.IsTestPlanSatisfied() {
|
||||
issueBody = fmt.Sprintf("%s %q **has no test plan**.", payload.PullRequest.URL, prTitle)
|
||||
}
|
||||
|
||||
if !result.IsTestPlanSatisfied() {
|
||||
issueBody += fmt.Sprintf("\n\nLearn more about test plans in our [testing guidelines](%s).", testPlanDocs)
|
||||
}
|
||||
|
||||
if result.ProtectedBranch {
|
||||
issueBody += fmt.Sprintf("\n\nThe base branch %q is protected and should not have direct pull requests to it.", payload.PullRequest.Base.Ref)
|
||||
}
|
||||
|
||||
if additionalContext != "" {
|
||||
issueBody += fmt.Sprintf("\n\n%s", additionalContext)
|
||||
}
|
||||
|
||||
user := payload.PullRequest.MergedBy.Login
|
||||
issueAssignees = append(issueAssignees, user)
|
||||
issueBody += fmt.Sprintf("\n\n@%s please comment in this issue with an explanation for this exception and close this issue.", user)
|
||||
|
||||
if result.Error != nil {
|
||||
// Log the error in the issue
|
||||
issueBody += fmt.Sprintf("\n\nEncountered error when checking PR: %s", result.Error)
|
||||
}
|
||||
|
||||
labels := append(exceptionLabels, payload.Repository.FullName)
|
||||
return &github.IssueRequest{
|
||||
Title: github.String(issueTitle),
|
||||
Body: github.String(issueBody),
|
||||
Assignees: &issueAssignees,
|
||||
Labels: &labels,
|
||||
}
|
||||
}
|
||||
@ -1,114 +0,0 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
func TestGenerateExceptionIssue(t *testing.T) {
|
||||
payload := EventPayload{
|
||||
Repository: RepositoryPayload{FullName: "bobheadxi/robert"},
|
||||
PullRequest: PullRequestPayload{
|
||||
Title: "some pull request",
|
||||
URL: "https://bobheadxi.dev",
|
||||
MergedBy: UserPayload{Login: "robert"},
|
||||
},
|
||||
}
|
||||
privatePayload := payload
|
||||
privatePayload.Repository.Private = true
|
||||
|
||||
protectedPayload := payload
|
||||
protectedPayload.PullRequest.Base = RefPayload{Ref: "release"}
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
payload EventPayload
|
||||
result checkResult
|
||||
additionalContext string
|
||||
|
||||
wantAssignees []string
|
||||
wantLabels []string
|
||||
wantBodyContains []string
|
||||
wantBodyExcludes []string
|
||||
}{{
|
||||
name: "not reviewed, not planned",
|
||||
payload: payload,
|
||||
result: checkResult{
|
||||
ReviewSatisfied: false,
|
||||
},
|
||||
wantAssignees: []string{"robert"},
|
||||
wantLabels: []string{"exception/review", "exception/test-plan", "bobheadxi/robert"},
|
||||
wantBodyContains: []string{"some pull request", "has no test plan", "was not reviewed"},
|
||||
wantBodyExcludes: []string{"protected"},
|
||||
}, {
|
||||
name: "not reviewed, planned",
|
||||
payload: payload,
|
||||
result: checkResult{
|
||||
ReviewSatisfied: false,
|
||||
TestPlan: "A plan!",
|
||||
},
|
||||
wantAssignees: []string{"robert"},
|
||||
wantLabels: []string{"exception/review", "bobheadxi/robert"},
|
||||
wantBodyContains: []string{"some pull request", "has a test plan", "was not reviewed"},
|
||||
wantBodyExcludes: []string{"protected"},
|
||||
}, {
|
||||
name: "not reviewed, not planned, but plan skipping is disabled",
|
||||
payload: payload,
|
||||
result: checkResult{
|
||||
ReviewSatisfied: false,
|
||||
CanSkipTestPlan: true,
|
||||
},
|
||||
wantAssignees: []string{"robert"},
|
||||
wantLabels: []string{"exception/review", "bobheadxi/robert"},
|
||||
wantBodyContains: []string{"some pull request", "has a test plan", "was not reviewed"},
|
||||
wantBodyExcludes: []string{"protected"},
|
||||
}, {
|
||||
name: "not planned, reviewed",
|
||||
payload: payload,
|
||||
result: checkResult{
|
||||
ReviewSatisfied: true,
|
||||
},
|
||||
wantAssignees: []string{"robert"},
|
||||
wantLabels: []string{"exception/test-plan", "bobheadxi/robert"},
|
||||
wantBodyContains: []string{"some pull request", "has no test plan"},
|
||||
wantBodyExcludes: []string{"protected"},
|
||||
}, {
|
||||
name: "private repo, not planned, reviewed",
|
||||
payload: privatePayload,
|
||||
result: checkResult{},
|
||||
wantAssignees: []string{"robert"},
|
||||
wantLabels: []string{"exception/review", "exception/test-plan", "bobheadxi/robert"},
|
||||
wantBodyExcludes: []string{"some pull request", "protected"},
|
||||
}, {
|
||||
name: "reviewed, planned but protected",
|
||||
payload: protectedPayload,
|
||||
result: checkResult{ProtectedBranch: true},
|
||||
wantAssignees: []string{"robert"},
|
||||
wantLabels: []string{"exception/review", "exception/test-plan", "exception/protected-branch", "bobheadxi/robert"},
|
||||
wantBodyContains: []string{"\"release\" is protected"},
|
||||
}, {
|
||||
name: "reviewed, planned but protected",
|
||||
payload: protectedPayload,
|
||||
additionalContext: "please use preprod branch",
|
||||
result: checkResult{ProtectedBranch: true},
|
||||
wantAssignees: []string{"robert"},
|
||||
wantLabels: []string{"exception/review", "exception/test-plan", "exception/protected-branch", "bobheadxi/robert"},
|
||||
wantBodyContains: []string{"please use preprod branch"},
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
got := generateExceptionIssue(&tt.payload, &tt.result, tt.additionalContext)
|
||||
t.Log(got.GetTitle(), "\n", got.GetBody())
|
||||
assert.Equal(t, tt.wantAssignees, got.GetAssignees())
|
||||
assert.Equal(t, tt.wantLabels, got.GetLabels())
|
||||
for _, content := range tt.wantBodyContains {
|
||||
assert.Contains(t, *got.Body, content, "body does not contain expected strings")
|
||||
}
|
||||
for _, content := range tt.wantBodyExcludes {
|
||||
assert.NotContains(t, *got.Body, content, "body contains unexpected strings")
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@ -1,219 +0,0 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"flag"
|
||||
"fmt"
|
||||
"log"
|
||||
"os"
|
||||
|
||||
"github.com/sourcegraph/sourcegraph/lib/errors"
|
||||
|
||||
"github.com/google/go-github/v55/github"
|
||||
"golang.org/x/oauth2"
|
||||
)
|
||||
|
||||
type Flags struct {
|
||||
GitHubPayloadPath string
|
||||
GitHubToken string
|
||||
GitHubRunURL string
|
||||
|
||||
IssuesRepoOwner string
|
||||
IssuesRepoName string
|
||||
|
||||
// ProtectedBranch designates a branch name that should always record an exception when a PR is opened
|
||||
// against it. It's primary use case is to discourage PRs againt the release branch on sourcegraph/deploy-sourcegraph-cloud.
|
||||
ProtectedBranch string
|
||||
|
||||
// AdditionalContext contains a paragraph that will be appended at the end of the created exception. It enables
|
||||
// repositories to further explain why an exception has been recorded.
|
||||
AdditionalContext string
|
||||
|
||||
// SkipStatus if true will skip updating commit status on GitHub and just record exceptions. Useful when crawling through failed runs caused by infrastructure issues.
|
||||
SkipStatus bool
|
||||
|
||||
SkipCheckTestPlan bool
|
||||
SkipCheckReview bool
|
||||
}
|
||||
|
||||
func (f *Flags) Parse() {
|
||||
flag.StringVar(&f.GitHubPayloadPath, "github.payload-path", "", "path to JSON file with GitHub event payload")
|
||||
flag.StringVar(&f.GitHubToken, "github.token", "", "GitHub token")
|
||||
flag.StringVar(&f.GitHubRunURL, "github.run-url", "", "URL to GitHub actions run")
|
||||
flag.StringVar(&f.IssuesRepoOwner, "issues.repo-owner", "sourcegraph", "owner of repo to create issues in")
|
||||
flag.StringVar(&f.IssuesRepoName, "issues.repo-name", "sec-pr-audit-trail", "name of repo to create issues in")
|
||||
flag.StringVar(&f.ProtectedBranch, "protected-branch", "", "name of branch that if set as the base branch in a PR, will always open an exception")
|
||||
flag.StringVar(&f.AdditionalContext, "additional-context", "", "additional information that will be appended to the recorded exception, if any.")
|
||||
flag.BoolVar(&f.SkipStatus, "skip-status", false, "skip updating commit status on GitHub and just record exceptions")
|
||||
flag.BoolVar(&f.SkipCheckTestPlan, "skip-check-test-plan", false, "Allows PRs without a test plan to pass audit")
|
||||
flag.BoolVar(&f.SkipCheckReview, "skip-check-review", false, "Allows PRs without any approving reviews to be merged")
|
||||
flag.Parse()
|
||||
}
|
||||
|
||||
func main() {
|
||||
flags := &Flags{}
|
||||
flags.Parse()
|
||||
|
||||
ctx := context.Background()
|
||||
ghc := github.NewClient(oauth2.NewClient(ctx, oauth2.StaticTokenSource(
|
||||
&oauth2.Token{AccessToken: flags.GitHubToken},
|
||||
)))
|
||||
|
||||
payloadData, err := os.ReadFile(flags.GitHubPayloadPath)
|
||||
if err != nil {
|
||||
log.Fatal("ReadFile: ", err)
|
||||
}
|
||||
var payload *EventPayload
|
||||
if err := json.Unmarshal(payloadData, &payload); err != nil {
|
||||
log.Fatal("Unmarshal: ", err)
|
||||
}
|
||||
log.Printf("handling event for pull request %s, payload: %+v\n", payload.PullRequest.URL, payload.Dump())
|
||||
|
||||
// Discard unwanted events
|
||||
switch ref := payload.PullRequest.Base.Ref; ref {
|
||||
// This is purely an API call usage optimization, so we don't need to be so specific
|
||||
// as to require usage to provide the default branch - we can just rely on a simple
|
||||
// allowlist of commonly used default branches.
|
||||
case "main", "master", "release":
|
||||
log.Printf("performing checks against allow-listed pull request base %q", ref)
|
||||
case flags.ProtectedBranch:
|
||||
if flags.ProtectedBranch == "" {
|
||||
log.Printf("unknown pull request base %q - discarding\n", ref)
|
||||
return
|
||||
}
|
||||
|
||||
log.Printf("performing checks against protected pull request base %q", ref)
|
||||
default:
|
||||
log.Printf("unknown pull request base %q - discarding\n", ref)
|
||||
return
|
||||
}
|
||||
if payload.PullRequest.Draft {
|
||||
log.Println("skipping event on draft PR")
|
||||
return
|
||||
}
|
||||
if payload.Action == "closed" && !payload.PullRequest.Merged {
|
||||
log.Println("ignoring closure of un-merged pull request")
|
||||
return
|
||||
}
|
||||
if payload.Action == "edited" && payload.PullRequest.Merged {
|
||||
log.Println("ignoring edit of already-merged pull request")
|
||||
return
|
||||
}
|
||||
|
||||
// Do checks
|
||||
if payload.PullRequest.Merged {
|
||||
if err := postMergeAudit(ctx, ghc, payload, flags); err != nil {
|
||||
log.Fatalf("postMergeAudit: %s", err)
|
||||
}
|
||||
} else {
|
||||
if err := preMergeAudit(ctx, ghc, payload, flags); err != nil {
|
||||
log.Fatalf("preMergeAudit: %s", err)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const (
|
||||
commitStatusPostMerge = "pr-auditor / post-merge"
|
||||
commitStatusPreMerge = "pr-auditor / pre-merge"
|
||||
)
|
||||
|
||||
func postMergeAudit(ctx context.Context, ghc *github.Client, payload *EventPayload, flags *Flags) error {
|
||||
result := checkPR(ctx, ghc, payload, checkOpts{
|
||||
SkipReviews: flags.SkipCheckReview,
|
||||
SkipTestPlan: flags.SkipCheckTestPlan,
|
||||
ProtectedBranch: flags.ProtectedBranch,
|
||||
})
|
||||
log.Printf("checkPR: %+v\n", result)
|
||||
|
||||
if result.IsSatisfied() {
|
||||
log.Println("Acceptance checked and PR reviewed, done")
|
||||
// Don't create status that likely nobody will check anyway
|
||||
return nil
|
||||
}
|
||||
|
||||
owner, repo := payload.Repository.GetOwnerAndName()
|
||||
if result.Error != nil && !flags.SkipStatus {
|
||||
_, _, statusErr := ghc.Repositories.CreateStatus(ctx, owner, repo, payload.PullRequest.Head.SHA, &github.RepoStatus{
|
||||
Context: github.String(commitStatusPostMerge),
|
||||
State: github.String("error"),
|
||||
Description: github.String(fmt.Sprintf("checkPR: %s", result.Error.Error())),
|
||||
TargetURL: github.String(flags.GitHubRunURL),
|
||||
})
|
||||
if statusErr != nil {
|
||||
return errors.Newf("result.Error != nil (%w), statusErr: %w", result.Error, statusErr)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
issue := generateExceptionIssue(payload, &result, flags.AdditionalContext)
|
||||
|
||||
log.Printf("Ensuring label for repository %q\n", payload.Repository.FullName)
|
||||
_, _, err := ghc.Issues.CreateLabel(ctx, flags.IssuesRepoOwner, flags.IssuesRepoName, &github.Label{
|
||||
Name: github.String(payload.Repository.FullName),
|
||||
})
|
||||
if err != nil {
|
||||
log.Printf("Ignoring error on CreateLabel: %s\n", err)
|
||||
}
|
||||
|
||||
log.Printf("Creating issue for exception: %+v\n", issue)
|
||||
created, _, err := ghc.Issues.Create(ctx, flags.IssuesRepoOwner, flags.IssuesRepoName, issue)
|
||||
if err != nil {
|
||||
// Let run fail, don't include special status
|
||||
return errors.Newf("Issues.Create: %w", err)
|
||||
}
|
||||
|
||||
log.Println("Created issue: ", created.GetHTMLURL())
|
||||
|
||||
// Let run succeed, create separate status indicating an exception was created
|
||||
_, _, err = ghc.Repositories.CreateStatus(ctx, owner, repo, payload.PullRequest.Head.SHA, &github.RepoStatus{
|
||||
Context: github.String(commitStatusPostMerge),
|
||||
State: github.String("failure"),
|
||||
Description: github.String("Exception detected and audit trail issue created"),
|
||||
TargetURL: github.String(created.GetHTMLURL()),
|
||||
})
|
||||
if err != nil {
|
||||
return errors.Newf("CreateStatus: %w", err)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
func preMergeAudit(ctx context.Context, ghc *github.Client, payload *EventPayload, flags *Flags) error {
|
||||
result := checkPR(ctx, ghc, payload, checkOpts{
|
||||
SkipReviews: true, // only validate reviews on post-merge
|
||||
SkipTestPlan: flags.SkipCheckTestPlan,
|
||||
ProtectedBranch: flags.ProtectedBranch,
|
||||
})
|
||||
log.Printf("checkPR: %+v\n", result)
|
||||
|
||||
var prState, stateDescription string
|
||||
stateURL := flags.GitHubRunURL
|
||||
switch {
|
||||
case result.Error != nil:
|
||||
prState = "error"
|
||||
stateDescription = fmt.Sprintf("checkPR: %s", result.Error.Error())
|
||||
case !result.IsTestPlanSatisfied():
|
||||
prState = "failure"
|
||||
stateDescription = "No test plan detected - please provide one!"
|
||||
stateURL = "https://docs.sourcegraph.com/dev/background-information/testing_principles#test-plans"
|
||||
case result.ProtectedBranch:
|
||||
prState = "success"
|
||||
stateDescription = "No action needed, but an exception will be opened post-merge."
|
||||
default:
|
||||
prState = "success"
|
||||
stateDescription = "No action needed, nice!"
|
||||
}
|
||||
|
||||
owner, repo := payload.Repository.GetOwnerAndName()
|
||||
_, _, err := ghc.Repositories.CreateStatus(ctx, owner, repo, payload.PullRequest.Head.SHA, &github.RepoStatus{
|
||||
Context: github.String(commitStatusPreMerge),
|
||||
State: github.String(prState),
|
||||
Description: github.String(stateDescription),
|
||||
TargetURL: github.String(stateURL),
|
||||
})
|
||||
if err != nil {
|
||||
return errors.Newf("CreateStatus: %w", err)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
@ -1,3 +0,0 @@
|
||||
Some poorly formatted markdown test plan
|
||||
## Test plan
|
||||
This is still a plan! No review required: just trust me
|
||||
@ -1,18 +0,0 @@
|
||||
|
||||
|
||||
---
|
||||
|
||||
### Test plan
|
||||
|
||||
<!-- blah blah -->
|
||||
|
||||
This is a plan!
|
||||
Quite lengthy
|
||||
|
||||
And a little complicated; there's also the following reasons:
|
||||
|
||||
<!-- blah blah -->
|
||||
|
||||
1. A
|
||||
2. B
|
||||
3. C
|
||||
@ -1,12 +0,0 @@
|
||||
|
||||
|
||||
---
|
||||
|
||||
### Test Plan
|
||||
|
||||
<!-- blah blah -->
|
||||
|
||||
I have a plan!
|
||||
|
||||
<!-- blah blah -->
|
||||
|
||||
@ -1,12 +0,0 @@
|
||||
Some description
|
||||
|
||||
Test plan: This is a plan!
|
||||
Quite lengthy
|
||||
|
||||
<!-- blah blah -->
|
||||
|
||||
And a little complicated; there's also the following reasons:
|
||||
|
||||
1. A
|
||||
2. B
|
||||
3. C
|
||||
@ -1,13 +0,0 @@
|
||||
|
||||
|
||||
---
|
||||
|
||||
### Acceptance checklist
|
||||
|
||||
<!-- blah blah -->
|
||||
|
||||
<!-- blah blah -->
|
||||
|
||||
<!--
|
||||
comments don't count!
|
||||
-->
|
||||
@ -1 +0,0 @@
|
||||
Test plan: I have a plan! No review required: this is a bot PR
|
||||
@ -1,64 +0,0 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"strings"
|
||||
)
|
||||
|
||||
// EventPayload describes the payload of the pull_request event we subscribe to:
|
||||
// https://docs.github.com/en/developers/webhooks-and-events/webhooks/webhook-events-and-payloads#pull_request
|
||||
type EventPayload struct {
|
||||
Action string `json:"action"`
|
||||
PullRequest PullRequestPayload `json:"pull_request"`
|
||||
Repository RepositoryPayload `json:"repository"`
|
||||
}
|
||||
|
||||
func (p EventPayload) Dump() string {
|
||||
return fmt.Sprintf(`Action: %s, PullRequest: { Merged: %v, MergedBy: %+v, Head: %+v }, Repository: %s`,
|
||||
p.Action, p.PullRequest.Merged, p.PullRequest.MergedBy, p.PullRequest.Head, p.Repository.FullName)
|
||||
}
|
||||
|
||||
type PullRequestPayload struct {
|
||||
Number int `json:"number"`
|
||||
Title string `json:"title"`
|
||||
Body string `json:"body"`
|
||||
Draft bool `json:"draft"`
|
||||
|
||||
Labels []Label `json:"labels"`
|
||||
|
||||
ReviewComments int `json:"review_comments"`
|
||||
|
||||
Merged bool `json:"merged"`
|
||||
MergedBy UserPayload `json:"merged_by"`
|
||||
|
||||
URL string `json:"html_url"`
|
||||
|
||||
Base RefPayload `json:"base"`
|
||||
Head RefPayload `json:"head"`
|
||||
}
|
||||
|
||||
type Label struct {
|
||||
Name string `json:"name"`
|
||||
}
|
||||
|
||||
type UserPayload struct {
|
||||
Login string `json:"login"`
|
||||
URL string `json:"html_url"`
|
||||
}
|
||||
|
||||
type RepositoryPayload struct {
|
||||
FullName string `json:"full_name"`
|
||||
URL string `json:"html_url"`
|
||||
Private bool `json:"private"`
|
||||
}
|
||||
|
||||
func (r *RepositoryPayload) GetOwnerAndName() (string, string) {
|
||||
repoParts := strings.Split(r.FullName, "/")
|
||||
return repoParts[0], repoParts[1]
|
||||
}
|
||||
|
||||
type RefPayload struct {
|
||||
// e.g. 'main'
|
||||
Ref string `json:"ref"`
|
||||
SHA string `json:"sha"`
|
||||
}
|
||||
Loading…
Reference in New Issue
Block a user