kubernetes-sigs/kustomize
1
0
Fork 0
mirror of https://github.com/kubernetes-sigs/kustomize.git synced 2026-06-29 17:41:13 +00:00
Code Issues Packages Projects Releases 1 Wiki Activity

Merge pull request #4342 from natasha41575/moduleSpanDefault

Browse Source 
Remove module span check
This commit is contained in:
Kubernetes Prow Robot
2021-12-15 11:45:51 -08:00
committed by GitHub
parent c0b61b9442 26999664e6
commit ec38bbeb99
6 changed files with 0 additions and 630 deletions
Download Patch File Download Diff File Expand all files Collapse all files

21
Makefile
View File

@@ -40,7 +40,6 @@ verify-kustomize: \
prow-presubmit-check: \
install-tools \
lint-kustomize \
test-multi-module \
test-unit-kustomize-all \
test-unit-cmd-all \
test-go-mod \
@@ -83,11 +82,6 @@ $(MYGOBIN)/pluginator:
cd cmd/pluginator; \
go install .
# Build from local source.
$(MYGOBIN)/prchecker:
cd cmd/prchecker; \
go install .
# Build from local source.
$(MYGOBIN)/kustomize: build-kustomize-api
cd kustomize; \
@@ -102,7 +96,6 @@ install-tools: \
$(MYGOBIN)/k8scopy \
$(MYGOBIN)/mdrip \
$(MYGOBIN)/pluginator \
$(MYGOBIN)/prchecker \
$(MYGOBIN)/stringer
### Begin kustomize plugin rules.
@@ -252,19 +245,6 @@ test-unit-cmd-all:
test-go-mod:
./hack/check-go-mod.sh
# Environment variables are defined at
# https://github.com/kubernetes/test-infra/blob/master/prow/jobs.md#job-environment-variables
.PHONY: test-multi-module
test-multi-module: $(MYGOBIN)/prchecker
( \
export MYGOBIN=$(MYGOBIN); \
export REPO_OWNER=$(REPO_OWNER); \
export REPO_NAME=$(REPO_NAME); \
export PULL_NUMBER=$(PULL_NUMBER); \
export MODULES=$(MODULES); \
./hack/check-multi-module.sh; \
)
.PHONY:
test-examples-e2e-kustomize: $(MYGOBIN)/mdrip $(MYGOBIN)/kind
( \
@@ -358,7 +338,6 @@ clean: clean-kustomize-external-go-plugin
rm -f $(MYGOBIN)/golangci-lint-kustomize
rm -f $(MYGOBIN)/kustomize
rm -f $(MYGOBIN)/mdrip
rm -f $(MYGOBIN)/prchecker
rm -f $(MYGOBIN)/stringer
# Handle pluginator manually.

8
cmd/prchecker/go.mod
View File

@@ -1,8 +0,0 @@
module sigs.k8s.io/kustomize/cmd/prchecker
go 1.16
require (
github.com/google/go-github v17.0.0+incompatible
github.com/google/go-querystring v1.0.0 // indirect
)

4
cmd/prchecker/go.sum
View File

@@ -1,4 +0,0 @@
github.com/google/go-github v17.0.0+incompatible h1:N0LgJ1j65A7kfXrZnUDaYCs/Sf4rEjNlfyDHW9dolSY=
github.com/google/go-github v17.0.0+incompatible/go.mod h1:zLgOLi98H3fifZn+44m+umXrS52loVEgC2AApnigrVQ=
github.com/google/go-querystring v1.0.0 h1:Xkwi/a1rcvNg1PPYe5vI8GbeBY/jrVuDX5ASuANWTrk=
github.com/google/go-querystring v1.0.0/go.mod h1:odCYkC5MyYFN7vkCjXpyrEuKhc/BUO6wN/zVPAxq5ck=

271
cmd/prchecker/main.go
View File

@@ -1,271 +0,0 @@
// prchecker examines pull requests
//
// - When a PR includes files from multiple modules that we'd rather not
// modify at the same time (in an effort to have more self-contained
// release notes), the script will exit with a non-zero exit code.
//
// Usage:
//
// go run . \
// -owner=kubernetes-sigs \
// -repo=kustomize \
// -pr=2997 \
// cmd/config api kustomize kyaml
package main
import (
"context"
"flag"
"fmt"
"os"
"regexp"
"strings"
"github.com/google/go-github/github"
)
// splitCommitsHelp is a help message displayed when a PR has commits which
// span modules.
const splitCommitsHelp = "\nCommits that include multiple Go modules must be split into multiple commits that each touch no more than one module.\n" +
"Splitting instructions: https://git-scm.com/docs/git-rebase#_splitting_commits\n"
// Ignore span pattern defines a regular expression pattern that, when matched,
// causes this check to be ignored.
// Pattern expects "ALLOW_MODULE_SPAN" in CAPS on a line by itself.
// Spaces may be included before or after but no other characters with one
// exception. ">" may be used to quote the exclusion.
// Pattern may be provided at any point in the pull request description.
//
// Ex: "ALLOW_MODULE_SPAN", "> ALLOW_MODULE_SPAN", " ALLOW_MODULE_SPAN "
const ignoreSpanPattern = "(?m)^>?\\s*ALLOW_MODULE_SPAN\\s*$"
// Changeset represents a set of file modifications associated with a commit
type Changeset struct {
files []string
id string
}
// GitHubRepository represents a pairing of the owner and repository to make
// passing around references to specific projects simpler
type GitHubRepository struct {
client *github.Client
owner *string
repo *string
}
// GitHubService is the collection of GitHub API interactions this service consumes
type GitHubService interface {
GetPullRequest(prId int) (*github.PullRequest, *github.Response, error)
GetCommit(commitSha string) (*github.RepositoryCommit, *github.Response, error)
ListCommits(prId int, options *github.ListOptions) ([]*github.RepositoryCommit, *github.Response, error)
}
// GetPullRequest retrieves details about a pull request from GitHub API
func (repository GitHubRepository) GetPullRequest(prId int) (
*github.PullRequest, *github.Response, error) {
return repository.client.PullRequests.Get(
context.Background(),
*repository.owner,
*repository.repo,
prId)
}
// GetCommit retrieves commit details from GitHub API
func (repository GitHubRepository) GetCommit(commitSha string) (
*github.RepositoryCommit, *github.Response, error) {
return repository.client.Repositories.GetCommit(context.Background(),
*repository.owner,
*repository.repo,
commitSha)
}
// ListCommits lists commits in a PR via GitHub API
func (repository GitHubRepository) ListCommits(prId int, options *github.ListOptions) (
[]*github.RepositoryCommit, *github.Response, error) {
return repository.client.PullRequests.ListCommits(context.Background(),
*repository.owner,
*repository.repo,
prId,
options)
}
func main() {
owner := flag.String("owner", "", "the github repository owner name")
repo := flag.String("repo", "", "the github repository name")
pullrequest := flag.Int("pr", -1, "the pull request number")
flag.Parse()
// Treat all following arguments as restricted directories
restrictedPaths := flag.Args()
// Short circuit check if restricted paths is less than 2
// Conflicts won't exist in this scenario so we don't need to call the API
if len(restrictedPaths) <= 1 {
fmt.Println("Check not run. Add at least two restricted paths and run again.")
os.Exit(0)
}
client := github.NewClient(nil)
githubRepo := &GitHubRepository{
client: client,
owner: owner,
repo: repo,
}
// Check if module span is allowed before scanning on commits
isSpanAllowed, err := ModuleSpanAllowed(githubRepo, *pullrequest)
if err != nil {
fmt.Printf("unable to retrieve pull request details: %v", err.Error())
os.Exit(2)
}
if isSpanAllowed {
fmt.Println("Check not run. Module spanning was allowed in this Pull Request.")
os.Exit(0)
}
isSpanningPull, _, err := PullRequestSpanningPathList(githubRepo, *pullrequest, restrictedPaths)
if err != nil {
fmt.Printf("unable to retrieve pull request details: %v", err)
os.Exit(2)
}
// Exit with error if two or more restricted directories where modified
if isSpanningPull {
// Provide a suggestion for potential solution if the check fails.
fmt.Println(splitCommitsHelp)
os.Exit(1)
}
}
// ConstructChangeset creates a changeset from a GitHub Commit object
func ConstructChangeset(commit *github.RepositoryCommit) *Changeset {
id := commit.SHA
fileset := []string{}
for _, file := range commit.Files {
fileset = append(fileset, *file.Filename)
}
return &Changeset{
files: fileset,
id: *id,
}
}
// StringAllowsModuleSpan tests if a string matches the allow span regex
func StringAllowsModuleSpan(body string) (bool, error) {
return regexp.MatchString(ignoreSpanPattern, body)
}
// ModuleSpanAllowed tests a Pull Requests description for a regular
// expression. If the expression matches then spanning modules are allowed.
func ModuleSpanAllowed(repository GitHubService, pullId int) (bool, error) {
// Note: There are multiple ways to pull a github commit object
// we want a RepositoryCommit.
pullRequest, _, err := repository.GetPullRequest(pullId)
if err != nil {
return false, err
}
return StringAllowsModuleSpan(*pullRequest.Body)
}
// GetCommitChanges looks up a github commit by SHA and returns a Changeset
// containing the modified files in the specified commit.
func GetCommitChanges(repository GitHubService, commitSha string) (*Changeset, error) {
commit, _, err := repository.GetCommit(commitSha)
if err != nil {
return nil, err
}
return ConstructChangeset(commit), nil
}
// GetPullRequestCommits constructs a list of all commits and the associated
// files changes in the given pull request
func GetPullRequestCommits(repository GitHubService, pullrequest int) ([]*Changeset, error) {
// foundFiles across all pages from github api
var collectedCommits []*github.RepositoryCommit
// Github only returns a limited set of commits per request and PR's may
// exceed this so loop until all pages have been enumerated.
options := &github.ListOptions{Page: 1}
for options.Page != 0 {
commits, response, err := repository.ListCommits(pullrequest, options)
// If an error has occurred while querying api exit early, report error
if err != nil {
return nil, err
}
collectedCommits = append(collectedCommits, commits...)
// setup next page to continue loop
options = &github.ListOptions{Page: response.NextPage}
}
var changesetResults []*Changeset
for _, commit := range collectedCommits {
// The repository commits from list commits are not hydrated
// We will need to retrieve the complete object:
changeset, err := GetCommitChanges(repository, *commit.SHA)
if err != nil {
return nil, err
}
changesetResults = append(changesetResults, changeset)
}
return changesetResults, nil
}
// PullRequestSpanningPathList tests if a pull request spans multiple
// directory paths
func PullRequestSpanningPathList(repository GitHubService, pullrequest int, paths []string) (bool, []*Changeset, error) {
// Create a buffer for commits
changesets, err := GetPullRequestCommits(repository, pullrequest)
if err != nil {
return false, nil, err
}
spanningChangesExist := false
for _, changeset := range changesets {
if changeset.isSpanningPaths(paths) {
// When detecting the first spanning changeset print a prefix message
if !spanningChangesExist {
fmt.Printf("Spanning changesets detected in the following commits:\n\n")
}
fmt.Printf("\t* %s\n", changeset.id)
// In order provide a full list of outstanding commits, do not shortcircuit this check
spanningChangesExist = true
}
}
return spanningChangesExist, changesets, nil
}
// isSpanningPaths tests if a changeset is spanning
// multiple directory paths.
func (changeset *Changeset) isSpanningPaths(paths []string) bool {
matchedPath := ""
for _, file := range changeset.files {
for _, path := range paths {
if strings.HasPrefix(file, path) {
// If a different path has already matched then the changeset spans multiple restricted paths
if matchedPath != "" && matchedPath != path {
return true
}
matchedPath = path
}
}
}
return false
}

303
cmd/prchecker/main_test.go
View File

@@ -1,303 +0,0 @@
package main
import (
"testing"
"github.com/google/go-github/github"
)
func TestStringAllowsModuleSpan(t *testing.T) {
var tests = []struct {
name string
body string
matchExpected bool
}{
{
"exception not included",
"foo",
false,
},
{
"exception mentioned in sentence does not exempt span check",
"don't ALLOW_MODULE_SPAN",
false,
},
{
"PR body is just exception",
"ALLOW_MODULE_SPAN",
true,
},
{
"support markdown quoting exception",
"> ALLOW_MODULE_SPAN",
true,
},
{
"support whitespace padding",
"\t ALLOW_MODULE_SPAN\t ",
true,
},
{
"module span exemption allowed at start of string",
"ALLOW_MODULE_SPAN\nat start of file",
true,
},
{
"module span exemption allowed at end of string",
"at end of file\nALLOW_MODULE_SPAN",
true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result, err := StringAllowsModuleSpan(tt.body)
if err != nil {
t.Errorf(err.Error())
}
if result != tt.matchExpected {
t.Errorf("got %t, want %t", result, tt.matchExpected)
}
})
}
}
func TestIsModuleSpanAllowed(t *testing.T) {
var tests = []struct {
name string
body string
matchExpected bool
}{
{
"module spanning not allowed",
"don't ALLOW_MODULE_SPAN",
false,
},
{
"module spanning allowed",
"ALLOW_MODULE_SPAN",
true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
client := FakeGitHubService{
pullRequest: &github.PullRequest{
Body: &tt.body,
},
}
result, _ := ModuleSpanAllowed(&client, 1)
if result != tt.matchExpected {
t.Errorf("got %t, want %t", result, tt.matchExpected)
}
})
}
}
func TestGetCommitChanges(t *testing.T) {
client := FakeGitHubService{
commit: &github.RepositoryCommit{
SHA: github.String("abc123"),
Files: []github.CommitFile{{Filename: github.String("foo")}, {Filename: github.String("bar")}},
},
}
result, _ := GetCommitChanges(&client, "abc123")
if result.id != "abc123" {
t.Errorf("got %v, want %v", result.id, "abc123")
}
if len(result.files) != 2 {
t.Errorf("got %v, want %v", len(result.files), "2")
}
expectedFiles := []string{"foo", "bar"}
if !StringSliceAreEqual(result.files, expectedFiles) {
t.Errorf("got %v, want %v", result.files, expectedFiles)
}
}
func TestGetPullRequestCommits(t *testing.T) {
client := FakeGitHubService{
commit: &github.RepositoryCommit{
SHA: github.String("abc123"),
Files: []github.CommitFile{{Filename: github.String("foo")}, {Filename: github.String("bar")}},
},
commitList: []*github.RepositoryCommit{
{
SHA: github.String("abc123"),
},
{
SHA: github.String("abc123"),
},
},
}
result, _ := GetPullRequestCommits(&client, 42)
if len(result) != 2 {
t.Errorf("got %v, want %v", len(result), 2)
}
expectedFiles := []string{"foo", "bar"}
if !StringSliceAreEqual(result[0].files, expectedFiles) {
t.Errorf("[%d] got %v, want %v", 0, result[0].files, expectedFiles)
}
if !StringSliceAreEqual(result[1].files, expectedFiles) {
t.Errorf("[%d] got %v, want %v", 1, result[1].files, expectedFiles)
}
}
func TestContstructingChangeset(t *testing.T) {
var tests = []struct {
name string
sha string
files []github.CommitFile
expected Changeset
}{
{
"construct from single file",
"abc123",
[]github.CommitFile{{Filename: github.String("foo")}},
Changeset{id: "abc123", files: []string{"foo"}},
},
{
"construct from multiple files",
"1234",
[]github.CommitFile{{Filename: github.String("foo")}, {Filename: github.String("bar")}},
Changeset{id: "1234", files: []string{"foo", "bar"}},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
commit := &github.RepositoryCommit{SHA: github.String(tt.sha), Files: tt.files}
result := ConstructChangeset(commit)
if tt.expected.id != result.id {
t.Errorf("got %v, want %v", result.id, tt.expected.id)
}
if !StringSliceAreEqual(tt.expected.files, result.files) {
t.Errorf("got %v, want %v", result.files, tt.expected.files)
}
})
}
}
func TestIsChangesetSpanning(t *testing.T) {
var tests = []struct {
name string
changeset []string
files []string
expected bool
}{
{
"matching sets of 1 element do not span",
[]string{"1"},
[]string{"1"},
false,
},
{
"subdirectories do not match top level directories",
[]string{"a/1"},
[]string{"1"},
false,
},
{
"distinct sets do not span",
[]string{"1", "2"},
[]string{"a", "b"},
false,
},
{
"single matching path does not span",
[]string{"1", "a"},
[]string{"1", "2"},
false,
},
{
"path and subdirectory of same restriction do not span",
[]string{"1", "1/a"},
[]string{"1", "2", "3"},
false,
},
{
"matching sets span",
[]string{"1", "2"},
[]string{"1", "2"},
true,
},
{
"superset of restricted paths spans",
[]string{"1", "2", "3"},
[]string{"1", "2"},
true,
},
{
"subset of restricted paths spans",
[]string{"1", "3"},
[]string{"1", "2", "3"},
true,
},
{
"subdirectories of restricted paths span",
[]string{"1/a", "3/b"},
[]string{"1", "2", "3"},
true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
changeset := &Changeset{files: tt.changeset}
result := changeset.isSpanningPaths(tt.files)
if result != tt.expected {
t.Errorf("got %t, want %t", result, tt.expected)
}
})
}
}
type FakeGitHubService struct {
pullRequest *github.PullRequest
commit *github.RepositoryCommit
commitList []*github.RepositoryCommit
}
func (repository FakeGitHubService) GetPullRequest(prId int) (*github.PullRequest, *github.Response, error) {
return repository.pullRequest, nil, nil
}
func (repository FakeGitHubService) GetCommit(commitSha string) (*github.RepositoryCommit, *github.Response, error) {
return repository.commit, nil, nil
}
func (repository FakeGitHubService) ListCommits(prId int, options *github.ListOptions) ([]*github.RepositoryCommit, *github.Response, error) {
return repository.commitList, &github.Response{NextPage: 0}, nil
}
func StringSliceAreEqual(left []string, right []string) bool {
if len(left) != len(right) {
return false
}
for i, elem := range left {
if elem != right[i] {
return false
}
}
return true
}

23
hack/check-multi-module.sh
View File

@@ -1,23 +0,0 @@
#!/usr/bin/env bash
set -e
if [[ "$PULL_NUMBER" -ne "" ]]; then
cmd="$MYGOBIN/prchecker
-owner=$REPO_OWNER
-repo=$REPO_NAME
-pr=$PULL_NUMBER
$MODULES"
echo $MYGOBIN
echo $REPO_OWNER
echo $REPO_NAME
echo $PULL_NUMBER
echo $MODULES
eval $cmd
else
echo "Multi-module check skipped. No PULL_NUMBER provided.
To run this check locally set PULL_NUMBER to the PR ID from GitHub."
fi