-
Notifications
You must be signed in to change notification settings - Fork 3
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Added bot command to enforce changelog requirements
- Loading branch information
Showing
5 changed files
with
335 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,123 @@ | ||
/* | ||
Copyright 2022 Gravitational, Inc. | ||
Licensed under the Apache License, Version 2.0 (the "License"); | ||
you may not use this file except in compliance with the License. | ||
You may obtain a copy of the License at | ||
http://www.apache.org/licenses/LICENSE-2.0 | ||
Unless required by applicable law or agreed to in writing, software | ||
distributed under the License is distributed on an "AS IS" BASIS, | ||
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
See the License for the specific language governing permissions and | ||
limitations under the License. | ||
*/ | ||
|
||
package bot | ||
|
||
import ( | ||
"context" | ||
"fmt" | ||
"log" | ||
"regexp" | ||
"strings" | ||
"unicode" | ||
|
||
"github.com/gravitational/trace" | ||
"k8s.io/utils/strings/slices" | ||
) | ||
|
||
const NoChangelogLabel string = "no-changelog" | ||
const ChangelogPrefix string = "changelog: " | ||
const ChangelogRegex string = "(?mi)^changelog: .*" | ||
|
||
// Checks if the PR contains a changelog entry in the PR body, or a "no-changelog" label. | ||
// | ||
// A few tests are performed on the extracted changelog entry to ensure it conforms to a | ||
// common standard. | ||
func (b *Bot) CheckChangelog(ctx context.Context) error { | ||
pull, err := b.c.GitHub.GetPullRequest(ctx, | ||
b.c.Environment.Organization, | ||
b.c.Environment.Repository, | ||
b.c.Environment.Number, | ||
) | ||
if err != nil { | ||
return trace.Wrap(err, "failed to retrieve pull request for https://github.com/%s/%s/pull/%d", b.c.Environment.Organization, b.c.Environment.Repository, b.c.Environment.Number) | ||
} | ||
|
||
if slices.Contains(pull.UnsafeLabels, NoChangelogLabel) { | ||
log.Printf("PR contains %q label, skipping changelog check", NoChangelogLabel) | ||
return nil | ||
} | ||
|
||
changelogEntry, err := b.getChangelogEntry(ctx, pull.UnsafeBody) | ||
if err != nil { | ||
return trace.Wrap(err, "failed to get changelog entry") | ||
} | ||
|
||
err = b.validateChangelogEntry(ctx, changelogEntry) | ||
if err != nil { | ||
return trace.Wrap(err, "failed to validate changelog entry") | ||
} | ||
|
||
return nil | ||
} | ||
|
||
func (b *Bot) getChangelogEntry(ctx context.Context, prBody string) (string, error) { | ||
changelogRegex := regexp.MustCompile(ChangelogRegex) | ||
|
||
changelogMatches := changelogRegex.FindAllString(prBody, 2) // Two or more changelog entries is invalid, so no need to search for more than two. | ||
if len(changelogMatches) == 0 { | ||
return "", b.logFailedCheck(ctx, "Changelog entry not found in the PR body. Please add a %q label to the PR, or a changelog line starting with `%s` followed by the changelog entry for the PR.", NoChangelogLabel, ChangelogPrefix) | ||
} | ||
|
||
if len(changelogMatches) > 1 { | ||
return "", b.logFailedCheck(ctx, "Found two or more changelog entries in the PR body: %v", changelogMatches) | ||
} | ||
|
||
changelogEntry := changelogMatches[0][len(ChangelogPrefix):] // Case insensitive prefix removal | ||
log.Printf("Found changelog entry %q", changelogEntry) | ||
return changelogEntry, nil | ||
} | ||
|
||
// Checks for common issues with the changelog entry. | ||
// This is not intended to be comprehensive, rather, it is intended to cover the majority of problems. | ||
func (b *Bot) validateChangelogEntry(ctx context.Context, changelogEntry string) error { | ||
if strings.TrimSpace(changelogEntry) == "" { | ||
return b.logFailedCheck(ctx, "The changelog entry must contain one or more non-whitespace characters") | ||
} | ||
|
||
if !unicode.IsLetter([]rune(changelogEntry)[0]) { | ||
return b.logFailedCheck(ctx, "The changelog entry must start with a letter") | ||
} | ||
|
||
if strings.HasPrefix(strings.ToLower(changelogEntry), "backport of") || | ||
strings.HasPrefix(strings.ToLower(changelogEntry), "backports") { | ||
return b.logFailedCheck(ctx, "The changelog entry must contain the actual change, not a reference to the source PR of the backport.") | ||
} | ||
|
||
if strings.Contains(changelogEntry, "](") { | ||
return b.logFailedCheck(ctx, "The changelog entry must not contain a Markdown link or image") | ||
} | ||
|
||
if strings.Contains(changelogEntry, "```") { | ||
return b.logFailedCheck(ctx, "The changelog entry must not contain a multiline code block") | ||
} | ||
|
||
return nil | ||
} | ||
|
||
func (b *Bot) logFailedCheck(ctx context.Context, format string, args ...interface{}) error { | ||
err := b.c.GitHub.CreateComment(ctx, | ||
b.c.Environment.Organization, | ||
b.c.Environment.Repository, | ||
b.c.Environment.Number, | ||
fmt.Sprintf(format, args...), | ||
) | ||
if err != nil { | ||
return trace.Wrap(err, "failed to create or update the changelog comment") | ||
} | ||
|
||
return trace.Errorf(format, args...) | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,207 @@ | ||
/* | ||
Copyright 2021 Gravitational, Inc. | ||
Licensed under the Apache License, Version 2.0 (the "License"); | ||
you may not use this file except in compliance with the License. | ||
You may obtain a copy of the License at | ||
http://www.apache.org/licenses/LICENSE-2.0 | ||
Unless required by applicable law or agreed to in writing, software | ||
distributed under the License is distributed on an "AS IS" BASIS, | ||
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
See the License for the specific language governing permissions and | ||
limitations under the License. | ||
*/ | ||
|
||
package bot | ||
|
||
import ( | ||
"context" | ||
"fmt" | ||
"strings" | ||
"testing" | ||
|
||
"github.com/gravitational/shared-workflows/bot/internal/env" | ||
"github.com/gravitational/shared-workflows/bot/internal/github" | ||
"github.com/stretchr/testify/require" | ||
) | ||
|
||
func TestChangelog(t *testing.T) { | ||
t.Run("pass-no-changelog-label", func(t *testing.T) { | ||
b, ctx := buildTestingFixtures() | ||
b.c.GitHub.(*fakeGithub).pull.UnsafeLabels = []string{NoChangelogLabel} | ||
|
||
err := b.CheckChangelog(ctx) | ||
require.True(t, err == nil) | ||
}) | ||
} | ||
|
||
func TestGetChangelogEntry(t *testing.T) { | ||
tests := []struct { | ||
desc string | ||
body string | ||
shouldError bool | ||
expected string | ||
}{ | ||
{ | ||
desc: "pass-simple", | ||
body: strings.Join([]string{"some typical PR entry", fmt.Sprintf("%schangelog entry", ChangelogPrefix)}, "\n"), | ||
shouldError: false, | ||
expected: "changelog entry", | ||
}, | ||
{ | ||
desc: "pass-case-invariant", | ||
body: strings.Join([]string{"some typical PR entry", fmt.Sprintf("%schangelog entry", strings.ToUpper(ChangelogPrefix))}, "\n"), | ||
shouldError: false, | ||
expected: "changelog entry", | ||
}, | ||
{ | ||
desc: "pass-prefix-in-changelog-entry", | ||
body: strings.Join([]string{"some typical PR entry", strings.Repeat(ChangelogPrefix, 5)}, "\n"), | ||
shouldError: false, | ||
expected: strings.Repeat(ChangelogPrefix, 4), | ||
}, | ||
{ | ||
desc: "pass-only-changelog-in-body", | ||
body: fmt.Sprintf("%schangelog entry", ChangelogPrefix), | ||
shouldError: false, | ||
expected: "changelog entry", | ||
}, | ||
{ | ||
desc: "fail-if-no-body", | ||
body: "", | ||
shouldError: true, | ||
}, | ||
{ | ||
desc: "fail-if-no-entry", | ||
body: "some typical PR entry", | ||
shouldError: true, | ||
}, | ||
{ | ||
desc: "fail-if-multiple-entries", | ||
body: strings.Join([]string{ChangelogPrefix, ChangelogPrefix, ChangelogPrefix}, "\n"), | ||
shouldError: true, | ||
}, | ||
} | ||
for _, test := range tests { | ||
t.Run(test.desc, func(t *testing.T) { | ||
b, ctx := buildTestingFixtures() | ||
|
||
changelogEntry, err := b.getChangelogEntry(ctx, test.body) | ||
require.Equal(t, test.shouldError, err != nil) | ||
if !test.shouldError { | ||
require.Equal(t, test.expected, changelogEntry) | ||
} | ||
}) | ||
} | ||
} | ||
|
||
func TestValidateGetChangelogEntry(t *testing.T) { | ||
tests := []struct { | ||
desc string | ||
entry string | ||
shouldError bool | ||
}{ | ||
{ | ||
desc: "pass-simple", | ||
entry: "Changelog entry", | ||
shouldError: false, | ||
}, | ||
{ | ||
desc: "pass-markdown-single-line-code-block", | ||
entry: "Changelog `entry`", | ||
shouldError: false, | ||
}, | ||
{ | ||
desc: "fail-empty", | ||
entry: "", | ||
shouldError: true, | ||
}, | ||
{ | ||
desc: "fail-whitespace", | ||
entry: " \t ", | ||
shouldError: true, | ||
}, | ||
{ | ||
desc: "fail-non-alphabetical-starting-character", | ||
entry: "1234Changelog entry", | ||
shouldError: true, | ||
}, | ||
{ | ||
desc: "fail-refers-to-backport", | ||
entry: "Backport of #1234", | ||
shouldError: true, | ||
}, | ||
{ | ||
desc: "fail-ends-with-markdown-link", | ||
entry: "Changelog [entry](https://some-link.com).", | ||
shouldError: true, | ||
}, | ||
{ | ||
desc: "fail-ends-with-markdown-image", | ||
entry: "Changelog ![entry](https://some-link.com).", | ||
shouldError: true, | ||
}, | ||
{ | ||
desc: "fail-ends-with-markdown-header", | ||
entry: "## Changelog entry", | ||
shouldError: true, | ||
}, | ||
{ | ||
desc: "fail-ends-with-markdown-ordered-list", | ||
entry: "1. Changelog entry", | ||
shouldError: true, | ||
}, | ||
{ | ||
desc: "fail-ends-with-markdown-unordered-list", | ||
entry: "- Changelog entry", | ||
shouldError: true, | ||
}, | ||
{ | ||
desc: "fail-ends-with-markdown-multiline-code-block", | ||
entry: "Changelog entry ```", | ||
shouldError: true, | ||
}, | ||
} | ||
for _, test := range tests { | ||
t.Run(test.desc, func(t *testing.T) { | ||
b, ctx := buildTestingFixtures() | ||
|
||
err := b.validateChangelogEntry(ctx, test.entry) | ||
require.Equal(t, test.shouldError, err != nil) | ||
}) | ||
} | ||
} | ||
|
||
func TestLogFailedCheck(t *testing.T) { | ||
t.Run("fail-contains-passed-message", func(t *testing.T) { | ||
b, ctx := buildTestingFixtures() | ||
|
||
err := b.logFailedCheck(ctx, "error %s", "message") | ||
require.ErrorContains(t, err, "error message") | ||
}) | ||
} | ||
|
||
func buildTestingFixtures() (*Bot, context.Context) { | ||
return &Bot{ | ||
c: &Config{ | ||
Environment: &env.Environment{ | ||
Organization: "foo", | ||
Author: "9", | ||
Repository: "bar", | ||
Number: 0, | ||
UnsafeBase: "branch/v8", | ||
UnsafeHead: "fix", | ||
}, | ||
GitHub: &fakeGithub{ | ||
comments: []github.Comment{ | ||
{ | ||
Author: "[email protected]", | ||
Body: "PR comment body", | ||
}, | ||
}, | ||
}, | ||
}, | ||
}, context.Background() | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters