12 Commits

Author SHA256 Message Date
877e93c9bf pr: always require review, if configured
Some checks failed
go-generate-check / go-generate-check (pull_request) Failing after 23s
Implement ReviewRequired option to workflow.config. This will
always require a review by maintainer, unless no other maintainers
are available.

By default, ReviewRequired is false
2026-01-20 19:18:56 +01:00
cc69a9348c Merge commit 'refs/pull/109/head' of src.opensuse.org:git-workflow/autogits 2026-01-20 18:39:23 +01:00
5b5bb9a5bc group-review: fix test name
Some checks failed
go-generate-check / go-generate-check (push) Failing after 22s
2026-01-19 13:41:05 +01:00
Antonello Tartamo
2f39fc9836 initial documentation review 2026-01-14 15:43:27 +01:00
38f4c44fd0 group-review: add more unit tests
Some checks failed
go-generate-check / go-generate-check (pull_request) Failing after 28s
2026-01-07 11:56:04 +01:00
605d3dee06 group-review: add notification unit tests 2026-01-07 11:32:04 +01:00
6f26bcdccc group-review: add mocked unit test 2026-01-07 10:42:39 +01:00
fffdf4fad3 group-review: add additional unit tests 2026-01-07 10:29:54 +01:00
f6d2239f4d group-review: move globals to struct
This enables easier testing
2026-01-07 10:27:12 +01:00
913fb7c046 group-review: add systemd file 2026-01-05 17:07:27 +01:00
79318dc169 group-review: add env variables
instead of using command-line parameters, we can use env variables
Very helpful for services.
2026-01-05 16:53:30 +01:00
377ed1c37f group-review: set correct comment text on negative review 2026-01-05 14:13:53 +01:00
18 changed files with 1142 additions and 443 deletions

View File

@@ -169,6 +169,7 @@ install -D -m0755 gitea-events-rabbitmq-publisher/gitea-events-rabbitmq-publishe
install -D -m0644 systemd/gitea-events-rabbitmq-publisher.service %{buildroot}%{_unitdir}/gitea-events-rabbitmq-publisher.service
install -D -m0755 gitea_status_proxy/gitea_status_proxy %{buildroot}%{_bindir}/gitea_status_proxy
install -D -m0755 group-review/group-review %{buildroot}%{_bindir}/group-review
install -D -m0644 systemd/group-review@.service %{buildroot}%{_unitdir}/group-review@.service
install -D -m0755 obs-forward-bot/obs-forward-bot %{buildroot}%{_bindir}/obs-forward-bot
install -D -m0755 obs-staging-bot/obs-staging-bot %{buildroot}%{_bindir}/obs-staging-bot
install -D -m0644 systemd/obs-staging-bot.service %{buildroot}%{_unitdir}/obs-staging-bot.service
@@ -191,6 +192,18 @@ install -D -m0755 utils/hujson/hujson
%postun gitea-events-rabbitmq-publisher
%service_del_postun gitea-events-rabbitmq-publisher.service
%pre group-review
%service_add_pre group-review@.service
%post group-review
%service_add_post group-review@.service
%preun group-review
%service_del_preun group-review@.service
%postun group-review
%service_del_postun group-review@.service
%pre obs-staging-bot
%service_add_pre obs-staging-bot.service
@@ -251,6 +264,7 @@ install -D -m0755 utils/hujson/hujson
%license COPYING
%doc group-review/README.md
%{_bindir}/group-review
%{_unitdir}/group-review@.service
%files obs-forward-bot
%license COPYING

View File

@@ -91,6 +91,7 @@ type AutogitConfig struct {
NoProjectGitPR bool // do not automatically create project git PRs, just assign reviewers and assume somethign else creates the ProjectGit PR
ManualMergeOnly bool // only merge with "Merge OK" comment by Project Maintainers and/or Package Maintainers and/or reviewers
ManualMergeProject bool // require merge of ProjectGit PRs with "Merge OK" by ProjectMaintainers and/or reviewers
ReviewRequired bool // always require a maintainer review, even if maintainer submits it. Only ignored if no other package or project reviewers
}
type AutogitConfigs []*AutogitConfig
@@ -292,9 +293,9 @@ func (config *AutogitConfig) GetRemoteBranch() string {
}
func (config *AutogitConfig) Label(label string) string {
if t, found := config.Labels[LabelKey(label)]; found {
return t
}
if t, found := config.Labels[LabelKey(label)]; found {
return t
}
return label
}

View File

@@ -391,17 +391,12 @@ func (e *GitHandlerImpl) GitExecQuietOrPanic(cwd string, params ...string) {
}
type ChanIO struct {
ch chan byte
done chan struct{}
ch chan byte
}
func (c *ChanIO) Write(p []byte) (int, error) {
for _, b := range p {
select {
case c.ch <- b:
case <-c.done:
return 0, io.EOF
}
c.ch <- b
}
return len(p), nil
}
@@ -410,32 +405,21 @@ func (c *ChanIO) Write(p []byte) (int, error) {
func (c *ChanIO) Read(data []byte) (idx int, err error) {
var ok bool
select {
case data[idx], ok = <-c.ch:
data[idx], ok = <-c.ch
if !ok {
err = io.EOF
return
}
idx++
for len(c.ch) > 0 && idx < len(data) {
data[idx], ok = <-c.ch
if !ok {
err = io.EOF
return
}
idx++
case <-c.done:
err = io.EOF
return
}
for len(c.ch) > 0 && idx < len(data) {
select {
case data[idx], ok = <-c.ch:
if !ok {
err = io.EOF
return
}
idx++
case <-c.done:
err = io.EOF
return
default:
return
}
idx++
}
return
@@ -482,14 +466,7 @@ func parseGitMsg(data <-chan byte) (GitMsg, error) {
var size int
pos := 0
for {
c, ok := <-data
if !ok {
return GitMsg{}, io.EOF
}
if c == ' ' {
break
}
for c := <-data; c != ' '; c = <-data {
if (c >= '0' && c <= '9') || (c >= 'a' && c <= 'f') {
id[pos] = c
pos++
@@ -501,15 +478,7 @@ func parseGitMsg(data <-chan byte) (GitMsg, error) {
pos = 0
var c byte
for {
var ok bool
c, ok = <-data
if !ok {
return GitMsg{}, io.EOF
}
if c == ' ' || c == '\x00' {
break
}
for c = <-data; c != ' ' && c != '\x00'; c = <-data {
if c >= 'a' && c <= 'z' {
msgType[pos] = c
pos++
@@ -535,14 +504,7 @@ func parseGitMsg(data <-chan byte) (GitMsg, error) {
return GitMsg{}, fmt.Errorf("Invalid object type: '%s'", string(msgType))
}
for {
c, ok := <-data
if !ok {
return GitMsg{}, io.EOF
}
if c == '\x00' {
break
}
for c = <-data; c != '\000'; c = <-data {
if c >= '0' && c <= '9' {
size = size*10 + (int(c) - '0')
} else {
@@ -561,37 +523,18 @@ func parseGitCommitHdr(oldHdr [2]string, data <-chan byte) ([2]string, int, erro
hdr := make([]byte, 0, 60)
val := make([]byte, 0, 1000)
c, ok := <-data
if !ok {
return [2]string{}, 0, io.EOF
}
c := <-data
size := 1
if c != '\n' { // end of header marker
for {
if c == ' ' {
break
}
for ; c != ' '; c = <-data {
hdr = append(hdr, c)
size++
var ok bool
c, ok = <-data
if !ok {
return [2]string{}, size, io.EOF
}
}
if size == 1 { // continuation header here
hdr = []byte(oldHdr[0])
val = append([]byte(oldHdr[1]), '\n')
}
for {
var ok bool
c, ok = <-data
if !ok {
return [2]string{}, size, io.EOF
}
if c == '\n' {
break
}
for c := <-data; c != '\n'; c = <-data {
val = append(val, c)
size++
}
@@ -604,14 +547,7 @@ func parseGitCommitHdr(oldHdr [2]string, data <-chan byte) ([2]string, int, erro
func parseGitCommitMsg(data <-chan byte, l int) (string, error) {
msg := make([]byte, 0, l)
for {
c, ok := <-data
if !ok {
return string(msg), io.EOF
}
if c == '\x00' {
break
}
for c := <-data; c != '\x00'; c = <-data {
msg = append(msg, c)
l--
}
@@ -637,7 +573,7 @@ func parseGitCommit(data <-chan byte) (GitCommit, error) {
var hdr [2]string
hdr, size, err := parseGitCommitHdr(hdr, data)
if err != nil {
return GitCommit{}, err
return GitCommit{}, nil
}
l -= size
@@ -658,28 +594,14 @@ func parseGitCommit(data <-chan byte) (GitCommit, error) {
func parseTreeEntry(data <-chan byte, hashLen int) (GitTreeEntry, error) {
var e GitTreeEntry
for {
c, ok := <-data
if !ok {
return e, io.EOF
}
if c == ' ' {
break
}
for c := <-data; c != ' '; c = <-data {
e.mode = e.mode*8 + int(c-'0')
e.size++
}
e.size++
name := make([]byte, 0, 128)
for {
c, ok := <-data
if !ok {
return e, io.EOF
}
if c == '\x00' {
break
}
for c := <-data; c != '\x00'; c = <-data {
name = append(name, c)
e.size++
}
@@ -690,10 +612,7 @@ func parseTreeEntry(data <-chan byte, hashLen int) (GitTreeEntry, error) {
hash := make([]byte, 0, hashLen*2)
for range hashLen {
c, ok := <-data
if !ok {
return e, io.EOF
}
c := <-data
hash = append(hash, hexBinToAscii[((c&0xF0)>>4)], hexBinToAscii[c&0xF])
}
e.hash = string(hash)
@@ -714,16 +633,13 @@ func parseGitTree(data <-chan byte) (GitTree, error) {
for parsedLen < hdr.size {
entry, err := parseTreeEntry(data, len(hdr.hash)/2)
if err != nil {
return GitTree{}, err
return GitTree{}, nil
}
t.items = append(t.items, entry)
parsedLen += entry.size
}
c, ok := <-data // \0 read
if !ok {
return t, io.EOF
}
c := <-data // \0 read
if c != '\x00' {
return t, fmt.Errorf("Unexpected character during git tree data read")
@@ -744,16 +660,9 @@ func parseGitBlob(data <-chan byte) ([]byte, error) {
d := make([]byte, hdr.size)
for l := 0; l < hdr.size; l++ {
var ok bool
d[l], ok = <-data
if !ok {
return d, io.EOF
}
}
eob, ok := <-data
if !ok {
return d, io.EOF
d[l] = <-data
}
eob := <-data
if eob != '\x00' {
return d, fmt.Errorf("invalid byte read in parseGitBlob")
}
@@ -765,25 +674,16 @@ func (e *GitHandlerImpl) GitParseCommits(cwd string, commitIDs []string) (parsed
var done sync.Mutex
done.Lock()
done_signal := make(chan struct{})
var once sync.Once
close_done := func() {
once.Do(func() {
close(done_signal)
})
}
data_in, data_out := ChanIO{make(chan byte), done_signal}, ChanIO{make(chan byte), done_signal}
data_in, data_out := ChanIO{make(chan byte)}, ChanIO{make(chan byte)}
parsedCommits = make([]GitCommit, 0, len(commitIDs))
go func() {
defer done.Unlock()
defer close_done()
defer close(data_out.ch)
for _, id := range commitIDs {
data_out.Write([]byte(id))
data_out.Write([]byte{0})
data_out.ch <- '\x00'
c, e := parseGitCommit(data_in.ch)
if e != nil {
err = fmt.Errorf("Error parsing git commit: %w", e)
@@ -810,14 +710,12 @@ func (e *GitHandlerImpl) GitParseCommits(cwd string, commitIDs []string) (parsed
LogDebug("command run:", cmd.Args)
if e := cmd.Run(); e != nil {
LogError(e)
close_done()
close(data_in.ch)
close(data_out.ch)
return nil, e
}
done.Lock()
close_done()
close(data_in.ch)
return
}
@@ -826,21 +724,15 @@ func (e *GitHandlerImpl) GitCatFile(cwd, commitId, filename string) (data []byte
var done sync.Mutex
done.Lock()
done_signal := make(chan struct{})
var once sync.Once
close_done := func() {
once.Do(func() {
close(done_signal)
})
}
data_in, data_out := ChanIO{make(chan byte), done_signal}, ChanIO{make(chan byte), done_signal}
data_in, data_out := ChanIO{make(chan byte)}, ChanIO{make(chan byte)}
go func() {
defer done.Unlock()
defer close_done()
defer close(data_out.ch)
data_out.Write([]byte(commitId))
data_out.Write([]byte{0})
data_out.ch <- '\x00'
var c GitCommit
c, err = parseGitCommit(data_in.ch)
if err != nil {
@@ -848,9 +740,11 @@ func (e *GitHandlerImpl) GitCatFile(cwd, commitId, filename string) (data []byte
return
}
data_out.Write([]byte(c.Tree))
data_out.Write([]byte{0})
data_out.ch <- '\x00'
var tree GitTree
tree, err = parseGitTree(data_in.ch)
if err != nil {
LogError("Error parsing git tree:", err)
return
@@ -860,7 +754,7 @@ func (e *GitHandlerImpl) GitCatFile(cwd, commitId, filename string) (data []byte
if te.isBlob() && te.name == filename {
LogInfo("blob", te.hash)
data_out.Write([]byte(te.hash))
data_out.Write([]byte{0})
data_out.ch <- '\x00'
data, err = parseGitBlob(data_in.ch)
return
}
@@ -885,13 +779,11 @@ func (e *GitHandlerImpl) GitCatFile(cwd, commitId, filename string) (data []byte
LogDebug("command run:", cmd.Args)
if e := cmd.Run(); e != nil {
LogError(e)
close_done()
close(data_in.ch)
close(data_out.ch)
return nil, e
}
done.Lock()
close_done()
close(data_in.ch)
return
}
@@ -901,24 +793,16 @@ func (e *GitHandlerImpl) GitDirectoryList(gitPath, commitId string) (directoryLi
directoryList = make(map[string]string)
done.Lock()
done_signal := make(chan struct{})
var once sync.Once
close_done := func() {
once.Do(func() {
close(done_signal)
})
}
data_in, data_out := ChanIO{make(chan byte), done_signal}, ChanIO{make(chan byte), done_signal}
data_in, data_out := ChanIO{make(chan byte)}, ChanIO{make(chan byte)}
LogDebug("Getting directory for:", commitId)
go func() {
defer done.Unlock()
defer close_done()
defer close(data_out.ch)
data_out.Write([]byte(commitId))
data_out.Write([]byte{0})
data_out.ch <- '\x00'
var c GitCommit
c, err = parseGitCommit(data_in.ch)
if err != nil {
@@ -934,7 +818,7 @@ func (e *GitHandlerImpl) GitDirectoryList(gitPath, commitId string) (directoryLi
delete(trees, p)
data_out.Write([]byte(tree))
data_out.Write([]byte{0})
data_out.ch <- '\x00'
var tree GitTree
tree, err = parseGitTree(data_in.ch)
@@ -968,14 +852,12 @@ func (e *GitHandlerImpl) GitDirectoryList(gitPath, commitId string) (directoryLi
LogDebug("command run:", cmd.Args)
if e := cmd.Run(); e != nil {
LogError(e)
close_done()
close(data_in.ch)
close(data_out.ch)
return directoryList, e
}
done.Lock()
close_done()
close(data_in.ch)
return directoryList, err
}
@@ -985,24 +867,16 @@ func (e *GitHandlerImpl) GitSubmoduleList(gitPath, commitId string) (submoduleLi
submoduleList = make(map[string]string)
done.Lock()
done_signal := make(chan struct{})
var once sync.Once
close_done := func() {
once.Do(func() {
close(done_signal)
})
}
data_in, data_out := ChanIO{make(chan byte), done_signal}, ChanIO{make(chan byte), done_signal}
data_in, data_out := ChanIO{make(chan byte)}, ChanIO{make(chan byte)}
LogDebug("Getting submodules for:", commitId)
go func() {
defer done.Unlock()
defer close_done()
defer close(data_out.ch)
data_out.Write([]byte(commitId))
data_out.Write([]byte{0})
data_out.ch <- '\x00'
var c GitCommit
c, err = parseGitCommit(data_in.ch)
if err != nil {
@@ -1018,7 +892,7 @@ func (e *GitHandlerImpl) GitSubmoduleList(gitPath, commitId string) (submoduleLi
delete(trees, p)
data_out.Write([]byte(tree))
data_out.Write([]byte{0})
data_out.ch <- '\x00'
var tree GitTree
tree, err = parseGitTree(data_in.ch)
@@ -1055,26 +929,17 @@ func (e *GitHandlerImpl) GitSubmoduleList(gitPath, commitId string) (submoduleLi
LogDebug("command run:", cmd.Args)
if e := cmd.Run(); e != nil {
LogError(e)
close_done()
close(data_in.ch)
close(data_out.ch)
return submoduleList, e
}
done.Lock()
close_done()
close(data_in.ch)
return submoduleList, err
}
func (e *GitHandlerImpl) GitSubmoduleCommitId(cwd, packageName, commitId string) (subCommitId string, valid bool) {
done_signal := make(chan struct{})
var once sync.Once
close_done := func() {
once.Do(func() {
close(done_signal)
})
}
data_in, data_out := ChanIO{make(chan byte), done_signal}, ChanIO{make(chan byte), done_signal}
data_in, data_out := ChanIO{make(chan byte)}, ChanIO{make(chan byte)}
var wg sync.WaitGroup
wg.Add(1)
@@ -1090,18 +955,17 @@ func (e *GitHandlerImpl) GitSubmoduleCommitId(cwd, packageName, commitId string)
}()
defer wg.Done()
defer close_done()
defer close(data_out.ch)
data_out.Write([]byte(commitId))
data_out.Write([]byte{0})
data_out.ch <- '\x00'
c, err := parseGitCommit(data_in.ch)
if err != nil {
LogError("Error parsing git commit:", err)
panic(err)
}
data_out.Write([]byte(c.Tree))
data_out.Write([]byte{0})
data_out.ch <- '\x00'
tree, err := parseGitTree(data_in.ch)
if err != nil {
@@ -1133,14 +997,12 @@ func (e *GitHandlerImpl) GitSubmoduleCommitId(cwd, packageName, commitId string)
LogDebug("command run:", cmd.Args)
if e := cmd.Run(); e != nil {
LogError(e)
close_done()
close(data_in.ch)
close(data_out.ch)
return subCommitId, false
}
wg.Wait()
close_done()
close(data_in.ch)
return subCommitId, len(subCommitId) > 0
}

View File

@@ -27,7 +27,6 @@ import (
"slices"
"strings"
"testing"
"time"
)
func TestGitClone(t *testing.T) {
@@ -597,47 +596,3 @@ func TestGitStatusParse(t *testing.T) {
})
}
}
func TestGitDeadlockFix(t *testing.T) {
gitDir := t.TempDir()
testDir, _ := os.Getwd()
cmd := exec.Command("/usr/bin/bash", path.Join(testDir, "tsetup.sh"))
cmd.Dir = gitDir
_, err := cmd.CombinedOutput()
if err != nil {
t.Fatal(err)
}
gh, err := AllocateGitWorkTree(gitDir, "Test", "test@example.com")
if err != nil {
t.Fatal(err)
}
h, err := gh.ReadExistingPath(".")
if err != nil {
t.Fatal(err)
}
defer h.Close()
// Use a blob ID to trigger error in GitParseCommits
// This ensures that the function returns error immediately and doesn't deadlock
blobId := "81aba862107f1e2f5312e165453955485f424612f313d6c2fb1b31fef9f82a14"
done := make(chan error)
go func() {
_, err := h.GitParseCommits("", []string{blobId})
done <- err
}()
select {
case err := <-done:
if err == nil {
t.Error("Expected error from GitParseCommits with blob ID, got nil")
} else {
// This is expected
t.Logf("Got expected error: %v", err)
}
case <-time.After(2 * time.Second):
t.Fatal("GitParseCommits deadlocked! Fix is NOT working.")
}
}

View File

@@ -25,6 +25,7 @@ const ProjectFileKey = "_project"
type MaintainershipMap struct {
Data map[string][]string
IsDir bool
Config *AutogitConfig
FetchPackage func(string) ([]byte, error)
}
@@ -39,7 +40,9 @@ func parseMaintainershipData(data []byte) (*MaintainershipMap, error) {
return maintainers, nil
}
func FetchProjectMaintainershipData(gitea GiteaMaintainershipReader, org, prjGit, branch string) (*MaintainershipMap, error) {
func FetchProjectMaintainershipData(gitea GiteaMaintainershipReader, config *AutogitConfig) (*MaintainershipMap, error) {
org, prjGit, branch := config.GetPrjGit()
data, _, err := gitea.FetchMaintainershipDirFile(org, prjGit, branch, ProjectFileKey)
dir := true
if err != nil || data == nil {
@@ -61,6 +64,7 @@ func FetchProjectMaintainershipData(gitea GiteaMaintainershipReader, org, prjGit
m, err := parseMaintainershipData(data)
if m != nil {
m.Config = config
m.IsDir = dir
m.FetchPackage = func(pkg string) ([]byte, error) {
data, _, err := gitea.FetchMaintainershipDirFile(org, prjGit, branch, pkg)
@@ -149,7 +153,10 @@ func (data *MaintainershipMap) IsApproved(pkg string, reviews []*models.PullRevi
}
LogDebug("Looking for review by:", reviewers)
if slices.Contains(reviewers, submitter) {
slices.Sort(reviewers)
reviewers = slices.Compact(reviewers)
SubmitterIdxInReviewers := slices.Index(reviewers, submitter)
if SubmitterIdxInReviewers > -1 && (!data.Config.ReviewRequired || len(reviewers) == 1) {
LogDebug("Submitter is maintainer. Approving.")
return true
}

View File

@@ -13,10 +13,10 @@ import (
)
func TestMaintainership(t *testing.T) {
config := common.AutogitConfig{
config := &common.AutogitConfig{
Branch: "bar",
Organization: "foo",
GitProjectName: common.DefaultGitPrj,
GitProjectName: common.DefaultGitPrj + "#bar",
}
packageTests := []struct {
@@ -141,7 +141,7 @@ func TestMaintainership(t *testing.T) {
notFoundError := repository.NewRepoGetContentsNotFound()
for _, test := range packageTests {
runTests := func(t *testing.T, mi common.GiteaMaintainershipReader) {
maintainers, err := common.FetchProjectMaintainershipData(mi, config.Organization, config.GitProjectName, config.Branch)
maintainers, err := common.FetchProjectMaintainershipData(mi, config)
if err != nil && !test.otherError {
if test.maintainersFileErr == nil {
t.Fatal("Unexpected error recieved", err)
@@ -253,3 +253,43 @@ func TestMaintainershipFileWrite(t *testing.T) {
})
}
}
func TestReviewRequired(t *testing.T) {
tests := []struct {
name string
maintainers []string
config *common.AutogitConfig
is_approved bool
}{
{
name: "ReviewRequired=false",
maintainers: []string{"maintainer1", "maintainer2"},
config: &common.AutogitConfig{ReviewRequired: false},
is_approved: true,
},
{
name: "ReviewRequired=true",
maintainers: []string{"maintainer1", "maintainer2"},
config: &common.AutogitConfig{ReviewRequired: true},
is_approved: false,
},
{
name: "ReviewRequired=true",
maintainers: []string{"maintainer1"},
config: &common.AutogitConfig{ReviewRequired: true},
is_approved: true,
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
m := &common.MaintainershipMap{
Data: map[string][]string{"": test.maintainers},
}
m.Config = test.config
if approved := m.IsApproved("", nil, "maintainer1", nil); approved != test.is_approved {
t.Error("Expected m.IsApproved()->", test.is_approved, "but didn't get it")
}
})
}
}

View File

@@ -316,7 +316,10 @@ func (rs *PRSet) FindMissingAndExtraReviewers(maintainers MaintainershipData, id
// only need project maintainer reviews if:
// * not created by a bot and has other PRs, or
// * not created by maintainer
noReviewPRCreators := prjMaintainers
noReviewPRCreators := []string{}
if !rs.Config.ReviewRequired {
noReviewPRCreators = prjMaintainers
}
if len(rs.PRs) > 1 {
noReviewPRCreators = append(noReviewPRCreators, rs.BotUser)
}
@@ -339,7 +342,10 @@ func (rs *PRSet) FindMissingAndExtraReviewers(maintainers MaintainershipData, id
pkg := pr.PR.Base.Repo.Name
pkgMaintainers := maintainers.ListPackageMaintainers(pkg, nil)
Maintainers := slices.Concat(prjMaintainers, pkgMaintainers)
noReviewPkgPRCreators := pkgMaintainers
noReviewPkgPRCreators := []string{}
if !rs.Config.ReviewRequired {
noReviewPkgPRCreators = pkgMaintainers
}
LogDebug("packakge maintainers:", Maintainers)

View File

@@ -833,7 +833,6 @@ func TestFindMissingAndExtraReviewers(t *testing.T) {
[]string{"autogits_obs_staging_bot", "user1"},
},
},
{
name: "Add reviewer if also maintainer where review by maintainer is not needed",
prset: &common.PRSet{
@@ -1095,8 +1094,67 @@ func TestFindMissingAndExtraReviewers(t *testing.T) {
expected_missing_reviewers: [][]string{{"pkgm2", "prj2"}},
expected_extra_reviewers: [][]string{{}, {"prj1"}},
},
{
name: "Package maintainer submitter, AlwaysRequireReview=false",
prset: &common.PRSet{
PRs: []*common.PRInfo{
{
PR: &models.PullRequest{
User: &models.User{UserName: "pkgmaintainer"},
Base: &models.PRBranchInfo{Name: "main", Repo: &models.Repository{Name: "pkg", Owner: &models.User{UserName: "org"}}},
},
Reviews: &common.PRReviews{},
},
},
Config: &common.AutogitConfig{
GitProjectName: "prg/repo#main",
Organization: "org",
Branch: "main",
Reviewers: []string{},
ReviewRequired: false,
},
},
maintainers: &common.MaintainershipMap{
Data: map[string][]string{
"pkg": {"pkgmaintainer", "pkgm1"},
},
},
noAutoStaging: true,
expected_missing_reviewers: [][]string{
{},
},
},
{
name: "Package maintainer submitter, AlwaysRequireReview=true",
prset: &common.PRSet{
PRs: []*common.PRInfo{
{
PR: &models.PullRequest{
User: &models.User{UserName: "pkgmaintainer"},
Base: &models.PRBranchInfo{Name: "main", Repo: &models.Repository{Name: "pkg", Owner: &models.User{UserName: "org"}}},
},
Reviews: &common.PRReviews{},
},
},
Config: &common.AutogitConfig{
GitProjectName: "prg/repo#main",
Organization: "org",
Branch: "main",
Reviewers: []string{},
ReviewRequired: true,
},
},
maintainers: &common.MaintainershipMap{
Data: map[string][]string{
"pkg": {"pkgmaintainer", "pkgm1"},
},
},
noAutoStaging: true,
expected_missing_reviewers: [][]string{
{"pkgm1"},
},
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
test.prset.HasAutoStaging = !test.noAutoStaging

View File

@@ -1,41 +1,65 @@
Group Review Bot
================
Areas of responsibility
-----------------------
This workaround is mainly needed because Gitea does not track which team member performed a review on behalf of a team.
1. Is used to handle reviews associated with groups defined in the
ProjectGit.
Main Tasks
----------
2. Assumes: workflow-pr needs to associate and define the PR set from
which the groups.json is read (Base of the PrjGit PR)
Awaits a comment in the format “@groupreviewbot-name: approve”, then approves the PR with the comment “<user> approved a review on behalf of <groupreviewbot-name>.”
Target Usage
------------
Projects where policy reviews are required.
Configiuration
Configuration
--------------
Groups are defined in the workflow.config inside the project git. They take following options,
The bot is configured via the `ReviewGroups` field in the `workflow.config` file, located in the ProjectGit repository.
See `ReviewGroups` in the [workflow-pr configuration](../workflow-pr/README.md#config-file).
```json
{
...
ReviewGroups: [
{
"Name": "name of the group user",
"Reviewers": ["members", "of", "group"],
"Silent": (true, false) -- if true, do not explicitly require review requests of group members
},
],
...
...
"ReviewGroups": [
{
"Name": "name of the group user",
"Reviewers": ["members", "of", "group"],
"Silent": "(true, false) -- if true, do not explicitly require review requests of group members"
}
],
...
}
```
Server configuration
--------------------------
**Configuration file:**
| Field | Type | Notes |
| ----- | ----- | ----- |
| root | Array of string | Format **org/repo\#branch** |
Requirements
------------
* Gitea token to:
+ R/W PullRequest
+ R/W Notification
+ R User
Gitea token with following permissions:
- R/W PullRequest
- R/W Notification
- R User
Env Variables
-------------
The following variables can be used (and override) command line parameters.
* `AUTOGITS_CONFIG` - config file location
* `AUTOGITS_URL` - Gitea URL
* `AUTOGITS_RABBITURL` - RabbitMQ url
* `AUTOGITS_DEBUG` - when set, debug level logging enabled
Authentication env variables
* `GITEA_TOKEN` - Gitea user token
* `AMQP_USERNAME`, `AMQP_PASSWORD` - username and password for rabbitmq

View File

@@ -5,6 +5,7 @@ import (
"fmt"
"log"
"net/url"
"os"
"regexp"
"runtime/debug"
"slices"
@@ -17,20 +18,23 @@ import (
"src.opensuse.org/autogits/common/gitea-generated/models"
)
var configs common.AutogitConfigs
var acceptRx *regexp.Regexp
var rejectRx *regexp.Regexp
var groupName string
func InitRegex(newGroupName string) {
groupName = newGroupName
acceptRx = regexp.MustCompile("^:\\s*(LGTM|approved?)")
rejectRx = regexp.MustCompile("^:\\s*")
type ReviewBot struct {
configs common.AutogitConfigs
acceptRx *regexp.Regexp
rejectRx *regexp.Regexp
groupName string
gitea common.Gitea
}
func ParseReviewLine(reviewText string) (bool, string) {
func (bot *ReviewBot) InitRegex(newGroupName string) {
bot.groupName = newGroupName
bot.acceptRx = regexp.MustCompile("^:\\s*(LGTM|approved?)")
bot.rejectRx = regexp.MustCompile("^:\\s*")
}
func (bot *ReviewBot) ParseReviewLine(reviewText string) (bool, string) {
line := strings.TrimSpace(reviewText)
groupTextName := "@" + groupName
groupTextName := "@" + bot.groupName
glen := len(groupTextName)
if len(line) < glen || line[0:glen] != groupTextName {
return false, line
@@ -50,20 +54,20 @@ func ParseReviewLine(reviewText string) (bool, string) {
return false, line
}
func ReviewAccepted(reviewText string) bool {
func (bot *ReviewBot) ReviewAccepted(reviewText string) bool {
for _, line := range common.SplitStringNoEmpty(reviewText, "\n") {
if matched, reviewLine := ParseReviewLine(line); matched {
return acceptRx.MatchString(reviewLine)
if matched, reviewLine := bot.ParseReviewLine(line); matched {
return bot.acceptRx.MatchString(reviewLine)
}
}
return false
}
func ReviewRejected(reviewText string) bool {
func (bot *ReviewBot) ReviewRejected(reviewText string) bool {
for _, line := range common.SplitStringNoEmpty(reviewText, "\n") {
if matched, reviewLine := ParseReviewLine(line); matched {
if rejectRx.MatchString(reviewLine) {
return !acceptRx.MatchString(reviewLine)
if matched, reviewLine := bot.ParseReviewLine(line); matched {
if bot.rejectRx.MatchString(reviewLine) {
return !bot.acceptRx.MatchString(reviewLine)
}
}
}
@@ -113,10 +117,10 @@ var commentStrings = []string{
"change_time_estimate",
}*/
func FindAcceptableReviewInTimeline(user string, timeline []*models.TimelineComment, reviews []*models.PullReview) *models.TimelineComment {
func (bot *ReviewBot) FindAcceptableReviewInTimeline(user string, timeline []*models.TimelineComment, reviews []*models.PullReview) *models.TimelineComment {
for _, t := range timeline {
if t.Type == common.TimelineCommentType_Comment && t.User.UserName == user && t.Created == t.Updated {
if ReviewAccepted(t.Body) || ReviewRejected(t.Body) {
if bot.ReviewAccepted(t.Body) || bot.ReviewRejected(t.Body) {
return t
}
}
@@ -125,9 +129,9 @@ func FindAcceptableReviewInTimeline(user string, timeline []*models.TimelineComm
return nil
}
func FindOurLastReviewInTimeline(timeline []*models.TimelineComment) *models.TimelineComment {
func (bot *ReviewBot) FindOurLastReviewInTimeline(timeline []*models.TimelineComment) *models.TimelineComment {
for _, t := range timeline {
if t.Type == common.TimelineCommentType_Review && t.User.UserName == groupName && t.Created == t.Updated {
if t.Type == common.TimelineCommentType_Review && t.User.UserName == bot.groupName && t.Created == t.Updated {
return t
}
}
@@ -135,13 +139,13 @@ func FindOurLastReviewInTimeline(timeline []*models.TimelineComment) *models.Tim
return nil
}
func UnrequestReviews(gitea common.Gitea, org, repo string, id int64, users []string) {
if err := gitea.UnrequestReview(org, repo, id, users...); err != nil {
func (bot *ReviewBot) UnrequestReviews(org, repo string, id int64, users []string) {
if err := bot.gitea.UnrequestReview(org, repo, id, users...); err != nil {
common.LogError("Can't remove reviewrs after a review:", err)
}
}
func ProcessNotifications(notification *models.NotificationThread, gitea common.Gitea) {
func (bot *ReviewBot) ProcessNotifications(notification *models.NotificationThread) {
defer func() {
if r := recover(); r != nil {
common.LogInfo("panic cought --- recovered")
@@ -168,14 +172,14 @@ func ProcessNotifications(notification *models.NotificationThread, gitea common.
id, _ := strconv.ParseInt(match[3], 10, 64)
common.LogInfo("processing:", fmt.Sprintf("%s/%s!%d", org, repo, id))
pr, err := gitea.GetPullRequest(org, repo, id)
pr, err := bot.gitea.GetPullRequest(org, repo, id)
if err != nil {
common.LogError(" ** Cannot fetch PR associated with review:", subject.URL, "Error:", err)
return
}
if err := ProcessPR(pr); err == nil && !common.IsDryRun {
if err := gitea.SetNotificationRead(notification.ID); err != nil {
if err := bot.ProcessPR(pr); err == nil && !common.IsDryRun {
if err := bot.gitea.SetNotificationRead(notification.ID); err != nil {
common.LogDebug(" Cannot set notification as read", err)
}
} else if err != nil && err != ReviewNotFinished {
@@ -185,24 +189,24 @@ func ProcessNotifications(notification *models.NotificationThread, gitea common.
var ReviewNotFinished = fmt.Errorf("Review is not finished")
func ProcessPR(pr *models.PullRequest) error {
func (bot *ReviewBot) ProcessPR(pr *models.PullRequest) error {
org := pr.Base.Repo.Owner.UserName
repo := pr.Base.Repo.Name
id := pr.Index
found := false
for _, reviewer := range pr.RequestedReviewers {
if reviewer != nil && reviewer.UserName == groupName {
if reviewer != nil && reviewer.UserName == bot.groupName {
found = true
break
}
}
if !found {
common.LogInfo(" review is not requested for", groupName)
common.LogInfo(" review is not requested for", bot.groupName)
return nil
}
config := configs.GetPrjGitConfig(org, repo, pr.Base.Name)
config := bot.configs.GetPrjGitConfig(org, repo, pr.Base.Name)
if config == nil {
return fmt.Errorf("Cannot find config for: %s", pr.URL)
}
@@ -212,17 +216,17 @@ func ProcessPR(pr *models.PullRequest) error {
return nil
}
reviews, err := gitea.GetPullRequestReviews(org, repo, id)
reviews, err := bot.gitea.GetPullRequestReviews(org, repo, id)
if err != nil {
return fmt.Errorf("Failed to fetch reviews for: %v: %w", pr.URL, err)
}
timeline, err := common.FetchTimelineSinceReviewRequestOrPush(gitea, groupName, pr.Head.Sha, org, repo, id)
timeline, err := common.FetchTimelineSinceReviewRequestOrPush(bot.gitea, bot.groupName, pr.Head.Sha, org, repo, id)
if err != nil {
return fmt.Errorf("Failed to fetch timeline to review. %w", err)
}
groupConfig, err := config.GetReviewGroup(groupName)
groupConfig, err := config.GetReviewGroup(bot.groupName)
if err != nil {
return fmt.Errorf("Failed to fetch review group. %w", err)
}
@@ -233,30 +237,30 @@ func ProcessPR(pr *models.PullRequest) error {
// pr.Head.Sha
for _, reviewer := range requestReviewers {
if review := FindAcceptableReviewInTimeline(reviewer, timeline, reviews); review != nil {
if ReviewAccepted(review.Body) {
if review := bot.FindAcceptableReviewInTimeline(reviewer, timeline, reviews); review != nil {
if bot.ReviewAccepted(review.Body) {
if !common.IsDryRun {
text := reviewer + " approved a review on behalf of " + groupName
if review := FindOurLastReviewInTimeline(timeline); review == nil || review.Body != text {
_, err := gitea.AddReviewComment(pr, common.ReviewStateApproved, text)
text := reviewer + " approved a review on behalf of " + bot.groupName
if review := bot.FindOurLastReviewInTimeline(timeline); review == nil || review.Body != text {
_, err := bot.gitea.AddReviewComment(pr, common.ReviewStateApproved, text)
if err != nil {
common.LogError(" -> failed to write approval comment", err)
}
UnrequestReviews(gitea, org, repo, id, requestReviewers)
bot.UnrequestReviews(org, repo, id, requestReviewers)
}
}
common.LogInfo(" -> approved by", reviewer)
common.LogInfo(" review at", review.Created)
return nil
} else if ReviewRejected(review.Body) {
} else if bot.ReviewRejected(review.Body) {
if !common.IsDryRun {
text := reviewer + " requested changes on behalf of " + groupName + ". See " + review.HTMLURL
if review := FindOurLastReviewInTimeline(timeline); review == nil || review.Body != text {
_, err := gitea.AddReviewComment(pr, common.ReviewStateRequestChanges, "Changes requested. See review by: "+reviewer)
text := reviewer + " requested changes on behalf of " + bot.groupName + ". See " + review.HTMLURL
if review := bot.FindOurLastReviewInTimeline(timeline); review == nil || review.Body != text {
_, err := bot.gitea.AddReviewComment(pr, common.ReviewStateRequestChanges, text)
if err != nil {
common.LogError(" -> failed to write rejecting comment", err)
}
UnrequestReviews(gitea, org, repo, id, requestReviewers)
bot.UnrequestReviews(org, repo, id, requestReviewers)
}
}
common.LogInfo(" -> declined by", reviewer)
@@ -270,7 +274,7 @@ func ProcessPR(pr *models.PullRequest) error {
if !groupConfig.Silent && len(requestReviewers) > 0 {
common.LogDebug(" Requesting reviews for:", requestReviewers)
if !common.IsDryRun {
if _, err := gitea.RequestReviews(pr, requestReviewers...); err != nil {
if _, err := bot.gitea.RequestReviews(pr, requestReviewers...); err != nil {
common.LogDebug(" -> err:", err)
}
} else {
@@ -283,42 +287,40 @@ func ProcessPR(pr *models.PullRequest) error {
// add a helpful comment, if not yet added
found_help_comment := false
for _, t := range timeline {
if t.Type == common.TimelineCommentType_Comment && t.User != nil && t.User.UserName == groupName {
if t.Type == common.TimelineCommentType_Comment && t.User != nil && t.User.UserName == bot.groupName {
found_help_comment = true
break
}
}
if !found_help_comment && !common.IsDryRun {
helpComment := fmt.Sprintln("Review by", groupName, "represents a group of reviewers:", strings.Join(requestReviewers, ", "), ".\n\n"+
helpComment := fmt.Sprintln("Review by", bot.groupName, "represents a group of reviewers:", strings.Join(requestReviewers, ", "), ".\n\n"+
"Do **not** use standard review interface to review on behalf of the group.\n"+
"To accept the review on behalf of the group, create the following comment: `@"+groupName+": approve`.\n"+
"To request changes on behalf of the group, create the following comment: `@"+groupName+": decline` followed with lines justifying the decision.\n"+
"To accept the review on behalf of the group, create the following comment: `@"+bot.groupName+": approve`.\n"+
"To request changes on behalf of the group, create the following comment: `@"+bot.groupName+": decline` followed with lines justifying the decision.\n"+
"Future edits of the comments are ignored, a new comment is required to change the review state.")
if slices.Contains(groupConfig.Reviewers, pr.User.UserName) {
helpComment = helpComment + "\n\n" +
"Submitter is member of this review group, hence they are excluded from being one of the reviewers here"
}
gitea.AddComment(pr, helpComment)
bot.gitea.AddComment(pr, helpComment)
}
return ReviewNotFinished
}
func PeriodReviewCheck() {
notifications, err := gitea.GetNotifications(common.GiteaNotificationType_Pull, nil)
func (bot *ReviewBot) PeriodReviewCheck() {
notifications, err := bot.gitea.GetNotifications(common.GiteaNotificationType_Pull, nil)
if err != nil {
common.LogError(" Error fetching unread notifications: %w", err)
return
}
for _, notification := range notifications {
ProcessNotifications(notification, gitea)
bot.ProcessNotifications(notification)
}
}
var gitea common.Gitea
func main() {
giteaUrl := flag.String("gitea-url", "https://src.opensuse.org", "Gitea instance used for reviews")
rabbitMqHost := flag.String("rabbit-url", "amqps://rabbit.opensuse.org", "RabbitMQ instance where Gitea webhook notifications are sent")
@@ -328,6 +330,24 @@ func main() {
flag.BoolVar(&common.IsDryRun, "dry", false, "Dry run, no effect. For debugging")
flag.Parse()
if err := common.SetLoggingLevelFromString(*logging); err != nil {
common.LogError(err.Error())
return
}
if cf := os.Getenv("AUTOGITS_CONFIG"); len(cf) > 0 {
*configFile = cf
}
if url := os.Getenv("AUTOGITS_URL"); len(url) > 0 {
*giteaUrl = url
}
if url := os.Getenv("AUTOGITS_RABBITURL"); len(url) > 0 {
*rabbitMqHost = url
}
if debug := os.Getenv("AUTOGITS_DEBUG"); len(debug) > 0 {
common.SetLoggingLevel(common.LogLevelDebug)
}
args := flag.Args()
if len(args) != 1 {
log.Println(" syntax:")
@@ -336,7 +356,7 @@ func main() {
flag.Usage()
return
}
groupName = args[0]
targetGroupName := args[0]
if *configFile == "" {
common.LogError("Missing config file")
@@ -359,36 +379,35 @@ func main() {
return
}
gitea = common.AllocateGiteaTransport(*giteaUrl)
configs, err = common.ResolveWorkflowConfigs(gitea, configData)
giteaTransport := common.AllocateGiteaTransport(*giteaUrl)
configs, err := common.ResolveWorkflowConfigs(giteaTransport, configData)
if err != nil {
common.LogError("Cannot parse workflow configs:", err)
return
}
reviewer, err := gitea.GetCurrentUser()
reviewer, err := giteaTransport.GetCurrentUser()
if err != nil {
common.LogError("Cannot fetch review user:", err)
return
}
if err := common.SetLoggingLevelFromString(*logging); err != nil {
common.LogError(err.Error())
return
}
if *interval < 1 {
*interval = 1
}
InitRegex(groupName)
bot := &ReviewBot{
gitea: giteaTransport,
configs: configs,
}
bot.InitRegex(targetGroupName)
common.LogInfo(" ** processing group reviews for group:", groupName)
common.LogInfo(" ** processing group reviews for group:", bot.groupName)
common.LogInfo(" ** username in Gitea:", reviewer.UserName)
common.LogInfo(" ** polling interval:", *interval, "min")
common.LogInfo(" ** connecting to RabbitMQ:", *rabbitMqHost)
if groupName != reviewer.UserName {
if bot.groupName != reviewer.UserName {
common.LogError(" ***** Reviewer does not match group name. Aborting. *****")
return
}
@@ -400,10 +419,13 @@ func main() {
}
config_update := ConfigUpdatePush{
bot: bot,
config_modified: make(chan *common.AutogitConfig),
}
process_issue_pr := IssueCommentProcessor{}
process_issue_pr := IssueCommentProcessor{
bot: bot,
}
configUpdates := &common.RabbitMQGiteaEventsProcessor{
Orgs: []string{},
@@ -413,7 +435,7 @@ func main() {
},
}
configUpdates.Connection().RabbitURL = u
for _, c := range configs {
for _, c := range bot.configs {
if org, _, _ := c.GetPrjGit(); !slices.Contains(configUpdates.Orgs, org) {
configUpdates.Orgs = append(configUpdates.Orgs, org)
}
@@ -426,17 +448,17 @@ func main() {
select {
case configTouched, ok := <-config_update.config_modified:
if ok {
for idx, c := range configs {
for idx, c := range bot.configs {
if c == configTouched {
org, repo, branch := c.GetPrjGit()
prj := fmt.Sprintf("%s/%s#%s", org, repo, branch)
common.LogInfo("Detected config update for", prj)
new_config, err := common.ReadWorkflowConfig(gitea, prj)
new_config, err := common.ReadWorkflowConfig(bot.gitea, prj)
if err != nil {
common.LogError("Failed parsing Project config for", prj, err)
} else {
configs[idx] = new_config
bot.configs[idx] = new_config
}
}
}
@@ -446,7 +468,7 @@ func main() {
}
}
PeriodReviewCheck()
bot.PeriodReviewCheck()
time.Sleep(time.Duration(*interval * int64(time.Minute)))
}
}

View File

@@ -1,6 +1,359 @@
package main
import "testing"
import (
"fmt"
"testing"
"go.uber.org/mock/gomock"
"src.opensuse.org/autogits/common"
"src.opensuse.org/autogits/common/gitea-generated/models"
mock_common "src.opensuse.org/autogits/common/mock"
)
func TestProcessPR(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
mockGitea := mock_common.NewMockGitea(ctrl)
groupName := "testgroup"
bot := &ReviewBot{
gitea: mockGitea,
groupName: groupName,
}
bot.InitRegex(groupName)
org := "myorg"
repo := "myrepo"
prIndex := int64(1)
headSha := "abcdef123456"
pr := &models.PullRequest{
Index: prIndex,
URL: "http://gitea/pr/1",
State: "open",
Base: &models.PRBranchInfo{
Name: "main",
Repo: &models.Repository{
Name: repo,
Owner: &models.User{
UserName: org,
},
},
},
Head: &models.PRBranchInfo{
Sha: headSha,
},
User: &models.User{
UserName: "submitter",
},
RequestedReviewers: []*models.User{
{UserName: groupName},
},
}
prjConfig := &common.AutogitConfig{
GitProjectName: org + "/" + repo + "#main",
ReviewGroups: []*common.ReviewGroup{
{
Name: groupName,
Reviewers: []string{"reviewer1", "reviewer2"},
},
},
}
bot.configs = common.AutogitConfigs{prjConfig}
t.Run("Review not requested for group", func(t *testing.T) {
prNoRequest := *pr
prNoRequest.RequestedReviewers = nil
err := bot.ProcessPR(&prNoRequest)
if err != nil {
t.Errorf("Expected no error, got %v", err)
}
})
t.Run("PR is closed", func(t *testing.T) {
prClosed := *pr
prClosed.State = "closed"
err := bot.ProcessPR(&prClosed)
if err != nil {
t.Errorf("Expected no error, got %v", err)
}
})
t.Run("Successful Approval", func(t *testing.T) {
common.IsDryRun = false
mockGitea.EXPECT().GetPullRequestReviews(org, repo, prIndex).Return(nil, nil)
// reviewer1 approved in timeline
timeline := []*models.TimelineComment{
{
Type: common.TimelineCommentType_Comment,
User: &models.User{UserName: "reviewer1"},
Body: "@" + groupName + ": approve",
},
}
mockGitea.EXPECT().GetTimeline(org, repo, prIndex).Return(timeline, nil)
expectedText := "reviewer1 approved a review on behalf of " + groupName
mockGitea.EXPECT().AddReviewComment(pr, common.ReviewStateApproved, expectedText).Return(nil, nil)
mockGitea.EXPECT().UnrequestReview(org, repo, prIndex, gomock.Any()).Return(nil)
err := bot.ProcessPR(pr)
if err != nil {
t.Errorf("Expected nil error, got %v", err)
}
})
t.Run("Dry Run - No actions taken", func(t *testing.T) {
common.IsDryRun = true
mockGitea.EXPECT().GetPullRequestReviews(org, repo, prIndex).Return(nil, nil)
timeline := []*models.TimelineComment{
{
Type: common.TimelineCommentType_Comment,
User: &models.User{UserName: "reviewer1"},
Body: "@" + groupName + ": approve",
},
}
mockGitea.EXPECT().GetTimeline(org, repo, prIndex).Return(timeline, nil)
// No AddReviewComment or UnrequestReview should be called
err := bot.ProcessPR(pr)
if err != nil {
t.Errorf("Expected nil error, got %v", err)
}
})
t.Run("Approval already exists - No new comment", func(t *testing.T) {
common.IsDryRun = false
mockGitea.EXPECT().GetPullRequestReviews(org, repo, prIndex).Return(nil, nil)
approvalText := "reviewer1 approved a review on behalf of " + groupName
timeline := []*models.TimelineComment{
{
Type: common.TimelineCommentType_Review,
User: &models.User{UserName: groupName},
Body: approvalText,
},
{
Type: common.TimelineCommentType_Comment,
User: &models.User{UserName: "reviewer1"},
Body: "@" + groupName + ": approve",
},
{
Type: common.TimelineCommentType_Comment,
User: &models.User{UserName: groupName},
Body: "Help comment",
},
}
mockGitea.EXPECT().GetTimeline(org, repo, prIndex).Return(timeline, nil)
// No AddReviewComment, UnrequestReview, or AddComment should be called
err := bot.ProcessPR(pr)
if err != nil {
t.Errorf("Expected nil error, got %v", err)
}
})
t.Run("Rejection already exists - No new comment", func(t *testing.T) {
common.IsDryRun = false
mockGitea.EXPECT().GetPullRequestReviews(org, repo, prIndex).Return(nil, nil)
rejectionText := "reviewer1 requested changes on behalf of " + groupName + ". See http://gitea/comment/123"
timeline := []*models.TimelineComment{
{
Type: common.TimelineCommentType_Review,
User: &models.User{UserName: groupName},
Body: rejectionText,
},
{
Type: common.TimelineCommentType_Comment,
User: &models.User{UserName: "reviewer1"},
Body: "@" + groupName + ": decline",
HTMLURL: "http://gitea/comment/123",
},
{
Type: common.TimelineCommentType_Comment,
User: &models.User{UserName: groupName},
Body: "Help comment",
},
}
mockGitea.EXPECT().GetTimeline(org, repo, prIndex).Return(timeline, nil)
err := bot.ProcessPR(pr)
if err != nil {
t.Errorf("Expected nil error, got %v", err)
}
})
t.Run("Pending review - Help comment already exists", func(t *testing.T) {
common.IsDryRun = false
mockGitea.EXPECT().GetPullRequestReviews(org, repo, prIndex).Return(nil, nil)
timeline := []*models.TimelineComment{
{
Type: common.TimelineCommentType_Comment,
User: &models.User{UserName: groupName},
Body: "Some help comment",
},
}
mockGitea.EXPECT().GetTimeline(org, repo, prIndex).Return(timeline, nil)
// It will try to request reviews
mockGitea.EXPECT().RequestReviews(pr, "reviewer1", "reviewer2").Return(nil, nil)
// AddComment should NOT be called because bot already has a comment in timeline
err := bot.ProcessPR(pr)
if err != ReviewNotFinished {
t.Errorf("Expected ReviewNotFinished error, got %v", err)
}
})
t.Run("Submitter is group member - Excluded from review request", func(t *testing.T) {
common.IsDryRun = false
prSubmitterMember := *pr
prSubmitterMember.User = &models.User{UserName: "reviewer1"}
mockGitea.EXPECT().GetPullRequestReviews(org, repo, prIndex).Return(nil, nil)
mockGitea.EXPECT().GetTimeline(org, repo, prIndex).Return(nil, nil)
mockGitea.EXPECT().RequestReviews(&prSubmitterMember, "reviewer2").Return(nil, nil)
mockGitea.EXPECT().AddComment(&prSubmitterMember, gomock.Any()).Return(nil)
err := bot.ProcessPR(&prSubmitterMember)
if err != ReviewNotFinished {
t.Errorf("Expected ReviewNotFinished error, got %v", err)
}
})
t.Run("Successful Rejection", func(t *testing.T) {
common.IsDryRun = false
mockGitea.EXPECT().GetPullRequestReviews(org, repo, prIndex).Return(nil, nil)
timeline := []*models.TimelineComment{
{
Type: common.TimelineCommentType_Comment,
User: &models.User{UserName: "reviewer2"},
Body: "@" + groupName + ": decline",
HTMLURL: "http://gitea/comment/999",
},
}
mockGitea.EXPECT().GetTimeline(org, repo, prIndex).Return(timeline, nil)
expectedText := "reviewer2 requested changes on behalf of " + groupName + ". See http://gitea/comment/999"
mockGitea.EXPECT().AddReviewComment(pr, common.ReviewStateRequestChanges, expectedText).Return(nil, nil)
mockGitea.EXPECT().UnrequestReview(org, repo, prIndex, gomock.Any()).Return(nil)
err := bot.ProcessPR(pr)
if err != nil {
t.Errorf("Expected nil error, got %v", err)
}
})
t.Run("Config not found", func(t *testing.T) {
bot.configs = common.AutogitConfigs{}
err := bot.ProcessPR(pr)
if err == nil {
t.Error("Expected error when config is missing, got nil")
}
})
t.Run("Gitea error in GetPullRequestReviews", func(t *testing.T) {
bot.configs = common.AutogitConfigs{prjConfig}
mockGitea.EXPECT().GetPullRequestReviews(org, repo, prIndex).Return(nil, fmt.Errorf("gitea error"))
err := bot.ProcessPR(pr)
if err == nil {
t.Error("Expected error from gitea, got nil")
}
})
}
func TestProcessNotifications(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
mockGitea := mock_common.NewMockGitea(ctrl)
groupName := "testgroup"
bot := &ReviewBot{
gitea: mockGitea,
groupName: groupName,
}
bot.InitRegex(groupName)
org := "myorg"
repo := "myrepo"
prIndex := int64(123)
notificationID := int64(456)
notification := &models.NotificationThread{
ID: notificationID,
Subject: &models.NotificationSubject{
URL: fmt.Sprintf("http://gitea/api/v1/repos/%s/%s/pulls/%d", org, repo, prIndex),
},
}
t.Run("Notification Success", func(t *testing.T) {
common.IsDryRun = false
pr := &models.PullRequest{
Index: prIndex,
Base: &models.PRBranchInfo{
Name: "main",
Repo: &models.Repository{
Name: repo,
Owner: &models.User{UserName: org},
},
},
Head: &models.PRBranchInfo{
Sha: "headsha",
Repo: &models.Repository{
Name: repo,
Owner: &models.User{UserName: org},
},
},
User: &models.User{UserName: "submitter"},
RequestedReviewers: []*models.User{{UserName: groupName}},
}
mockGitea.EXPECT().GetPullRequest(org, repo, prIndex).Return(pr, nil)
prjConfig := &common.AutogitConfig{
GitProjectName: org + "/" + repo + "#main",
ReviewGroups: []*common.ReviewGroup{{Name: groupName, Reviewers: []string{"r1"}}},
}
bot.configs = common.AutogitConfigs{prjConfig}
mockGitea.EXPECT().GetPullRequestReviews(org, repo, prIndex).Return(nil, nil)
timeline := []*models.TimelineComment{
{
Type: common.TimelineCommentType_Comment,
User: &models.User{UserName: "r1"},
Body: "@" + groupName + ": approve",
},
}
mockGitea.EXPECT().GetTimeline(org, repo, prIndex).Return(timeline, nil)
expectedText := "r1 approved a review on behalf of " + groupName
mockGitea.EXPECT().AddReviewComment(pr, common.ReviewStateApproved, expectedText).Return(nil, nil)
mockGitea.EXPECT().UnrequestReview(org, repo, prIndex, gomock.Any()).Return(nil)
mockGitea.EXPECT().SetNotificationRead(notificationID).Return(nil)
bot.ProcessNotifications(notification)
})
t.Run("Invalid Notification URL", func(t *testing.T) {
badNotification := &models.NotificationThread{
Subject: &models.NotificationSubject{
URL: "http://gitea/invalid/url",
},
}
bot.ProcessNotifications(badNotification)
})
t.Run("Gitea error in GetPullRequest", func(t *testing.T) {
mockGitea.EXPECT().GetPullRequest(org, repo, prIndex).Return(nil, fmt.Errorf("gitea error"))
bot.ProcessNotifications(notification)
})
}
func TestReviewApprovalCheck(t *testing.T) {
tests := []struct {
@@ -60,16 +413,78 @@ func TestReviewApprovalCheck(t *testing.T) {
InString: "@group2: disapprove",
Rejected: true,
},
{
Name: "Whitespace before colon",
GroupName: "group",
InString: "@group : LGTM",
Approved: true,
},
{
Name: "No whitespace after colon",
GroupName: "group",
InString: "@group:LGTM",
Approved: true,
},
{
Name: "Leading and trailing whitespace on line",
GroupName: "group",
InString: " @group: LGTM ",
Approved: true,
},
{
Name: "Multiline: Approved on second line",
GroupName: "group",
InString: "Random noise\n@group: approved",
Approved: true,
},
{
Name: "Multiline: Multiple group mentions, first wins",
GroupName: "group",
InString: "@group: decline\n@group: approve",
Rejected: true,
},
{
Name: "Multiline: Approved on second line",
GroupName: "group",
InString: "noise\n@group: approve\nmore noise",
Approved: true,
},
{
Name: "Not at start of line (even with whitespace)",
GroupName: "group",
InString: "Hello @group: approve",
Approved: false,
},
{
Name: "Rejecting with reason",
GroupName: "group",
InString: "@group: decline because of X, Y and Z",
Rejected: true,
},
{
Name: "No colon after group",
GroupName: "group",
InString: "@group LGTM",
Approved: false,
Rejected: false,
},
{
Name: "Invalid char after group",
GroupName: "group",
InString: "@group! LGTM",
Approved: false,
Rejected: false,
},
}
for _, test := range tests {
t.Run(test.Name, func(t *testing.T) {
InitRegex(test.GroupName)
bot := &ReviewBot{}
bot.InitRegex(test.GroupName)
if r := ReviewAccepted(test.InString); r != test.Approved {
if r := bot.ReviewAccepted(test.InString); r != test.Approved {
t.Error("ReviewAccepted() returned", r, "expecting", test.Approved)
}
if r := ReviewRejected(test.InString); r != test.Rejected {
if r := bot.ReviewRejected(test.InString); r != test.Rejected {
t.Error("ReviewRejected() returned", r, "expecting", test.Rejected)
}
})

View File

@@ -7,7 +7,9 @@ import (
"src.opensuse.org/autogits/common"
)
type IssueCommentProcessor struct{}
type IssueCommentProcessor struct {
bot *ReviewBot
}
func (s *IssueCommentProcessor) ProcessFunc(req *common.Request) error {
if req.Type != common.RequestType_IssueComment {
@@ -19,14 +21,15 @@ func (s *IssueCommentProcessor) ProcessFunc(req *common.Request) error {
repo := data.Repository.Name
index := int64(data.Issue.Number)
pr, err := gitea.GetPullRequest(org, repo, index)
pr, err := s.bot.gitea.GetPullRequest(org, repo, index)
if err != nil {
return fmt.Errorf("Failed to fetch PullRequest from event: %s/%s!%d Error: %w", org, repo, index, err)
}
return ProcessPR(pr)
return s.bot.ProcessPR(pr)
}
type ConfigUpdatePush struct {
bot *ReviewBot
config_modified chan *common.AutogitConfig
}
@@ -46,7 +49,7 @@ func (s *ConfigUpdatePush) ProcessFunc(req *common.Request) error {
}
branch := data.Ref[len(branch_ref):]
c := configs.GetPrjGitConfig(org, repo, branch)
c := s.bot.configs.GetPrjGitConfig(org, repo, branch)
if c == nil {
return nil
}
@@ -64,7 +67,7 @@ func (s *ConfigUpdatePush) ProcessFunc(req *common.Request) error {
}
if modified_config {
for _, config := range configs {
for _, config := range s.bot.configs {
if o, r, _ := config.GetPrjGit(); o == org && r == repo {
s.config_modified <- config
}

203
group-review/rabbit_test.go Normal file
View File

@@ -0,0 +1,203 @@
package main
import (
"fmt"
"testing"
"go.uber.org/mock/gomock"
"src.opensuse.org/autogits/common"
"src.opensuse.org/autogits/common/gitea-generated/models"
mock_common "src.opensuse.org/autogits/common/mock"
)
func TestIssueCommentProcessor(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
mockGitea := mock_common.NewMockGitea(ctrl)
groupName := "testgroup"
bot := &ReviewBot{
gitea: mockGitea,
groupName: groupName,
}
bot.InitRegex(groupName)
processor := &IssueCommentProcessor{bot: bot}
org := "myorg"
repo := "myrepo"
index := 123
event := &common.IssueCommentWebhookEvent{
Repository: &common.Repository{
Name: repo,
Owner: &common.Organization{
Username: org,
},
},
Issue: &common.IssueDetail{
Number: index,
},
}
req := &common.Request{
Type: common.RequestType_IssueComment,
Data: event,
}
t.Run("Successful Processing", func(t *testing.T) {
pr := &models.PullRequest{
Index: int64(index),
Base: &models.PRBranchInfo{
Name: "main",
Repo: &models.Repository{
Name: repo,
Owner: &models.User{UserName: org},
},
},
Head: &models.PRBranchInfo{
Sha: "headsha",
Repo: &models.Repository{
Name: repo,
Owner: &models.User{UserName: org},
},
},
User: &models.User{UserName: "submitter"},
RequestedReviewers: []*models.User{{UserName: groupName}},
}
mockGitea.EXPECT().GetPullRequest(org, repo, int64(index)).Return(pr, nil)
prjConfig := &common.AutogitConfig{
GitProjectName: org + "/" + repo + "#main",
ReviewGroups: []*common.ReviewGroup{{Name: groupName, Reviewers: []string{"r1"}}},
}
bot.configs = common.AutogitConfigs{prjConfig}
mockGitea.EXPECT().GetPullRequestReviews(org, repo, int64(index)).Return(nil, nil)
mockGitea.EXPECT().GetTimeline(org, repo, int64(index)).Return(nil, nil)
mockGitea.EXPECT().RequestReviews(pr, "r1").Return(nil, nil)
mockGitea.EXPECT().AddComment(pr, gomock.Any()).Return(nil)
err := processor.ProcessFunc(req)
if err != ReviewNotFinished {
t.Errorf("Expected ReviewNotFinished, got %v", err)
}
})
t.Run("Gitea error in GetPullRequest", func(t *testing.T) {
mockGitea.EXPECT().GetPullRequest(org, repo, int64(index)).Return(nil, fmt.Errorf("gitea error"))
err := processor.ProcessFunc(req)
if err == nil {
t.Error("Expected error, got nil")
}
})
t.Run("Wrong Request Type", func(t *testing.T) {
wrongReq := &common.Request{Type: common.RequestType_Push}
err := processor.ProcessFunc(wrongReq)
if err == nil {
t.Error("Expected error for wrong request type, got nil")
}
})
}
func TestConfigUpdatePush(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
groupName := "testgroup"
bot := &ReviewBot{
groupName: groupName,
}
bot.InitRegex(groupName)
configChan := make(chan *common.AutogitConfig, 1)
processor := &ConfigUpdatePush{
bot: bot,
config_modified: configChan,
}
org := "myorg"
repo := "myrepo"
branch := "main"
prjConfig := &common.AutogitConfig{
GitProjectName: org + "/" + repo + "#" + branch,
Organization: org,
Branch: branch,
}
bot.configs = common.AutogitConfigs{prjConfig}
event := &common.PushWebhookEvent{
Ref: "refs/heads/" + branch,
Repository: &common.Repository{
Name: repo,
Owner: &common.Organization{
Username: org,
},
},
Commits: []common.Commit{
{
Modified: []string{common.ProjectConfigFile},
},
},
}
req := &common.Request{
Type: common.RequestType_Push,
Data: event,
}
t.Run("Config Modified", func(t *testing.T) {
err := processor.ProcessFunc(req)
if err != nil {
t.Errorf("Expected nil error, got %v", err)
}
select {
case modified := <-configChan:
if modified != prjConfig {
t.Errorf("Expected modified config to be %v, got %v", prjConfig, modified)
}
default:
t.Error("Expected config modification signal, but none received")
}
})
t.Run("No Config Modified", func(t *testing.T) {
noConfigEvent := *event
noConfigEvent.Commits = []common.Commit{{Modified: []string{"README.md"}}}
noConfigReq := &common.Request{Type: common.RequestType_Push, Data: &noConfigEvent}
err := processor.ProcessFunc(noConfigReq)
if err != nil {
t.Errorf("Expected nil error, got %v", err)
}
select {
case <-configChan:
t.Error("Did not expect config modification signal")
default:
// Success
}
})
t.Run("Wrong Branch Ref", func(t *testing.T) {
wrongBranchEvent := *event
wrongBranchEvent.Ref = "refs/tags/v1.0"
wrongBranchReq := &common.Request{Type: common.RequestType_Push, Data: &wrongBranchEvent}
err := processor.ProcessFunc(wrongBranchReq)
if err == nil {
t.Error("Expected error for wrong branch ref, got nil")
}
})
t.Run("Config Not Found", func(t *testing.T) {
bot.configs = common.AutogitConfigs{}
err := processor.ProcessFunc(req)
if err != nil {
t.Errorf("Expected nil error even if config not found, got %v", err)
}
})
}

View File

@@ -4,11 +4,15 @@ OBS Staging Bot
Build a PR against a ProjectGit, if review is requested.
Areas of Responsibility
-----------------------
Main Tasks
----------
* Monitors Notification API in Gitea for review requests
* Reviews Package build results in OBS for all changed packages in ProjectGit PR
* A build in OBS is initiated when a review for this bot is requested.
* The overall build status is reported:
* Build successful
* Build failed
* It checks the build status only for the involved packages compared to the last state of the project for all architectures and all flavors.
* It adds an svg with detailed building status.
Target Usage
@@ -21,22 +25,46 @@ Configuration File
------------------
Bot reads `staging.config` from the project git or the PR to the project git.
It's a JSON file with following syntax
It's a JSON file with following syntax:
```
```json
{
"ObsProject": "home:foo:project",
"StagingProject": "home:foo:project:staging",
"QA": [
{
"Name": "ProjectBuild",
"Origin": "home:foo:product:images"
}
]
"ObsProject": "SUSE:SLFO:1.2",
"StagingProject": "SUSE:SLFO:1.2:PullRequest",
"QA": [
{
"Name": "SLES",
"Origin": "SUSE:SLFO:Products:SLES:16.0"
}
]
}
```
* ObsProject: (**required**) Project where the base project is built. Builds in this project will be used to compare to builds based on sources from the PR
* StagingProject: template project that will be used as template for the staging project. Omitting this will use the ObsProject repositories to create the staging. Staging project will be created under the template, or in the bot's home directory if not specified.
* QA: set of projects to build ontop of the binaries built in staging.
| Field name | Details | Mandatory | Type | Allowed Values | Default |
| ----- | ----- | ----- | ----- | ----- | ----- |
| *ObsProject* | Product OBS project. Builds in this project will be used to compare to builds based on sources from the PR. | yes | string | `[a-zA-Z0-9-_:]+` | |
| *StagingProject* | Used both as base project and prefix for all OBS staging projects. Upon being added as a reviewer to a PrjGit PR, this bot automatically generates an OBS project named *StagingProject:<PR_Number>*. It must be a sub-project of the *ObsProject*. | yes | string | `[a-zA-Z0-9-_:]+` | |
| *QA* | Crucial for generating a product build (such as an ISO or FTP tree) that incorporates the packages. | no | array of objects | | |
| *QA > Name* | Suffix for the QA OBS staging project. The project is named *StagingProject:<PR_Number>:Name*. | no | string | | |
| *QA > Origin* | OBS reference project | no | string | | |
Details
-------
* **OBS staging projects are deleted** when the relative PrjGit PR is closed or merged.
* **PrjGit PR - staging project**
* The OBS staging project utilizes an **scmsync** tag, configured with the `onlybuild` flag, to exclusively build packages associated with this specific PrjGit PR.
* The **build config** is inherited from the PrjGit PR config file (even if unchanged).
* The **project meta** creates a standard repository following the StagingProject as a project path.
* The base *StagingProject* has the macro **FromScratch:** set in its config, which prevents inheriting the configuration from the included project paths.
* The bot copies the project maintainers from *StagingProject* to the specific staging project (*StagingProject:<PR_Number>*).
* The bot reports “Build successful” only if the build is successful for all repositories and all architectures.
* **PrjGit PR - QA staging project**
* The QA staging project is meant for building the product; the relative build config is inherited from the `QA > Origin` project.
* In this case, the **scmsync** tag is inherited from the `QA > Origin` project.

View File

@@ -0,0 +1,15 @@
[Unit]
Description=Group Review bot for %i
After=network-online.target
[Service]
Type=exec
ExecStart=/usr/bin/group-review %i
EnvironmentFile=-/etc/default/group-review/%i.env
DynamicUser=yes
NoNewPrivileges=yes
ProtectSystem=strict
[Install]
WantedBy=multi-user.target

View File

@@ -1,29 +1,41 @@
Direct Workflow bot
===================
The project submodule is automatically updated by the direct bot whenever a branch is updated in a package repository.
This bot can coexist with the Workflow PR bot, which is instead triggered by a new package PR.
Target Usage
------------
Devel project, where direct pushes to package git are possible.
Areas of responsibility
-----------------------
1. Keep ProjectGit in sync with packages in the organization
* on pushes to package, updates the submodule commit id
to the default branch HEAD (as configured in Gitea)
* on repository adds, creates a new submodule (if non empty)
* on repository removal, removes the submodule
* **On pushes to package**: updates the submodule commit ID to the default branch HEAD (as configured in Gitea).
* **On repository adds**: creates a new submodule (if non-empty).
* **On repository removal**: removes the submodule.
**Note:** If you want to revert a change in a package, you need to do that manually in the project git.
Configuration
-------------
Uses `workflow.config` for configuration. Parameters
Uses `workflow.config` for configuration.
* _Workflows_: ["direct"] -- direct entry enables direct workflow. **Mandatory**
* _Organization_: organization that holds all the packages. **Mandatory**
* _Branch_: branch updated in repo's, or blank for default package branch
* _GitProjectName_: package in above org, or `org/package#branch` for PrjGit. By default assumes `_ObsPrj` with default branch and in the `Organization`
| Field name | Details | Mandatory | Type | Allowed Values | Default |
| ----- | ----- | ----- | ----- | ----- | ----- |
| *Workflows* | Type of workflow | yes | string | “direct” | |
| *Organization* | The organization that holds all the packages | yes | string | | |
| *Branch* | The designated branch for packages | no | string | | blank (default package branch) |
| *GitProjectName* | Repository and branch where the ProjectGit lives. | no | string | **Format**: `org/project_repo#branch` | By default assumes `_ObsPrj` with default branch in the *Organization* |
NOTE: `-rm`, `-removed`, `-deleted` are all removed suffixes used to indicate current branch is a placeholder for previously existing package. These branches will be ignored by the bot, and if default, the package will be removed and will not be added to the project.
Running
Environment Variables
-------
* `GITEA_TOKEN` (required)
@@ -37,8 +49,3 @@ Running
* `AUTOGITS_REPO_PATH` - default is temporary directory
* `AUTOGITS_IDENTITY_FILE` - in case where we need explicit identify path for ssh specified
Target Usage
------------
Devel project, where direct pushes to package git are possible

View File

@@ -1,55 +1,65 @@
Workflow-PR bot
===============
Keeps ProjectGit PR in-sync with a PackageGit PR
Areas of Responsibility
-----------------------
* Detects a PackageGit PR creation against a package and creates a coresponsing PR against the ProjectGit
* When a PackageGit PR is updated, the corresponding PR against the ProjectGit is updated
* Stores reference to the PackageGit PR in the headers of the ProjectGit PR comments, for later reference
* this allows ProjectGit PR to be merged to seperated later (via another tool, for example)
* Initiates all staging workflows via review requests
Keeps ProjectGit PRs in-sync with the relative PackageGit PRs.
Target Usage
------------
Any project (devel, etc) that accepts PR
Any project (devel, codestream, product, etc.) that accepts PRs.
Main Tasks
----------
* **Synchronization**:
* When a **PackageGit PR** is created for a package on a specific project branch, a corresponding PR is automatically generated in **ProjectGit**.
* When a PackageGit PR is updated, the corresponding PR against the ProjectGit is updated.
* A link to the PackageGit PR is stored in the body of the ProjectGit PR comments in the following format:
* `PR: organization/package_name!pull_request_number`
* Example: `PR: pool/curl!4`
* It closes an empty ProjectGit PR (e.g., if a PR was initially created for a single package but later integrated into a larger ProjectGit PR).
* It forwards the Work In Progress (WIP) flag to the ProjectGit PR. If the ProjectGit PR references multiple Package PRs, triggering the WIP flag on the ProjectGit PR side only requires a single WIP package PR.
* **Reviewer Management**:
* It adds required reviewers in the ProjectGit PR.
* It adds required reviewers in the PackageGit PR.
* If new commits are added to a PackageGit PR, reviewers who have already approved it will be re-added.
* **Merge Management**:
* Manages PR merges based on configuration flags (`ManualMergeOnly`, `ManualMergeProject`).
* In general, merge only happens if all mandatory reviews are completed.
* **ManualMergeProject** is stricter than **ManualMergeOnly** and has higher priority.
| Flag | Value | Behavior |
| ----- | ----- | ----- |
| ManualMergeProject | true | Both ProjectGit and PackageGit PRs are merged upon an allowed project maintainer commenting "merge ok” in the ProjectGit PR. |
| ManualMergeOnly | true | Both PackageGit PR and ProjectGit PR are merged upon an allowed package maintainer or project maintainer commenting “merge ok” in the PackageGit PR. |
| ManualMergeOnly and ManualMergeProject | false | Both ProjectGit and PackageGit PRs are merged as soon as all reviews are completed in both PrjGit and PkgGit PRs. |
Config file
-----------
JSON
* _Workflows_: ["pr"] -- pr entry enables pr workflow. **Mandatory**
* _Organization_: organization that holds all the packages **Mandatory**
* _Branch_: branch updated in repo's **Mandatory**
* _GitProjectName_: package in above org, or `org/package#branch` for PrjGit. By default assumes `_ObsPrj` with default branch and in the `Organization`
* _Reviewers_: accounts associated with mandatory reviews for PrjGit. Can trigger additional
review requests for PrjGit or associated PkgGit repos. Only when all reviews are
satisfied, will the PrjGit PR be merged. See Reviewers below.
* _ManualMergeOnly_: (true, false) only merge if "merge ok" comment/review by package or project maintainers or reviewers
* _ManualMergeProject_: (true, false) only merge if "merge ok" by project maintainers or reviewers
* _ReviewRequired_: (true, false) ignores that submitter is a maintainer and require a review from other maintainer IFF available
* _NoProjectGitPR_: (true, false) do not create PrjGit PRs, but still process reviews, etc.
* _Permissions_: permissions and associated accounts/groups. See below.
* _Labels_: (string, string) Labels for PRs. See below.
NOTE: `-rm`, `-removed`, `-deleted` are all removed suffixes used to indicate current branch is a placeholder for previously existing package. These branches will be ignored by the bot, and if default, the package will be removed and will not be added to the project.
example:
* Filename: `workflow.config`
* Location: ProjectGit
* Format: non-standard JSON (comments allowed)
| Field name | Details | Mandatory | Type | Allowed Values | Default |
| ----- | ----- | ----- | ----- | ----- | ----- |
| *Workflows* | Type of workflow | yes | string | “pr” | |
| *Organization* | The organization where PackageGit PRs are expected to occur | yes | string | | |
| *Branch* | The designated branch for PackageGit PRs | yes | string | | |
| *GitProjectName* | Repository and branch where the ProjectGit lives. | no | string | **Format**: `org/project_repo#branch` | By default assumes `_ObsPrj` with default branch in the *Organization* |
| *ManualMergeOnly* | Merges are permitted only upon receiving a "merge ok" comment from designated maintainers in the PkgGit PR. | no | bool | true, false | false |
| *ManualMergeProject* | Merges are permitted only upon receiving a "merge ok" comment in the ProjectGit PR from project maintainers. | no | bool | true, false | false |
| *ReviewRequired* | If submitter is a maintainer, require review from another maintainer if available. | no | bool | true, false | false |
| *NoProjectGitPR* | Do not create PrjGit PR, but still perform other tasks. | no | bool | true, false | false |
| *Reviewers* | PrjGit reviewers. Additional review requests are triggered for associated PkgGit PRs. PrjGit PR is merged only when all reviews are complete. | no | array of strings | | `[]` |
| *ReviewGroups* | If a group is specified in Reviewers, its members are listed here. | no | array of objects | | `[]` |
| *ReviewGroups > Name* | Name of the group | no | string | | |
| *ReviewGroups > Reviewers* | Members of the group | no | array of strings | | |
| *ReviewGroups > Silent* | Add members for notifications. If true, members are not explicitly requested to review. If one member approves, others are removed. | no | bool | true, false | false |
[
{
"Workflows": ["pr", "direct"],
"Organization": "autogits",
"GitProjectName": "HiddenPrj",
"Branch": "hidden",
"Reviewers": []
},
...
]
Reviewers
---------
@@ -58,21 +68,30 @@ Reviews is a list of accounts that need to review package and/or project. They h
[~][*|-|+]username
General prefix of ~ indicates advisory reviewer. They will be requested, but ignored otherwise.
A tilde (`~`) before a prefix signifies an advisory reviewer. Their input is requested, but their review status will not otherwise affect the process.
Other prefixes indicate project or package association of the reviewer:
* `*` indicates project *and* package
* `-` indicates project-only reviewer
* `+` indicates package-only reviewer
`+` is implied. For example
`+` is implied.
`[foo, -bar, ~*moo]`
For example: `[foo, -bar, ~*moo]` results in:
* foo: package reviews
* bar: project reviews
* moo: package and project reviews, but ignored
results in
* foo -> package reviews
* bar -> project reviews
* moo -> package and project reviews, but ignored
Package Deletion Requests
-------------------------
(NOT YET IMPLEMENTED)
* **Removing a Package:**
To remove a package from a project, submit a ProjectGit Pull Request (PR) that removes the corresponding submodule. The bot will then rename the project branch in the pool by appending "-removed" to its name.
* **Adding a Package Again:**
If you wish to re-add a package, create a new PrjGit PR which adds again the submodule on the branch that has the "-removed" suffix. The bot will automatically remove this suffix from the project branch in the pool.
Labels
@@ -83,26 +102,38 @@ The following labels are used, when defined in Repo/Org.
| Label Config Entry | Default label | Description
|--------------------|----------------|----------------------------------------
| StagingAuto | staging/Auto | Assigned to Project Git PRs when first staged
| ReviewPending | review/Pending | Assigned to PR when reviews are still pending
| ReviewDone | review/Done | Assigned to PR when reviews are complete on this particular PR
| ReviewPending | review/Pending | Assigned to Project Git PR when package reviews are still pending
| ReviewDone | review/Done | Assigned to Project Git PR when reviews are complete on all package PRs
Maintainership
--------------
Maintainership information is defined per project. For reviews, package maintainers are coalesced
with project maintainers. A review by any of the maintainers is acceptable.
Filename: \_maintainership.json
Location: ProjectGit
Format: JSON
Fields:
example:
| Key | Value | Notes |
| ----- | ----- | ----- |
| package name | array of strings representing the package maintainers | List of package maintainers |
| “” (empty string) | array of strings representing the project maintainers | List of project maintainers |
{
"package1": [ "reviewer", "reviewer2"],
"package2": [],
Maintainership information is defined per project. For PackageGit PR reviews, package maintainers are combined with project maintainers. A review by any of these maintainers is acceptable.
// "project" maintainer
"": ["reviewer3", "reviewer4"]
}
If the submitter is a maintainer it will not get a review requested.
Example:
```
{
"package1": [ "reviewer", "reviewer2"],
"package2": [],
// "project" maintainer
"": ["reviewer3", "reviewer4"]
}
```
Permissions
-----------
@@ -122,3 +153,11 @@ the `workflow.config`.
NOTE: Project Maintainers have these permissions automatically.
Server configuration
--------------------------
**Configuration file:**
| Field | Type | Notes |
| ----- | ----- | ----- |
| root | Array of string | Format **org/repo\#branch** |

View File

@@ -500,7 +500,7 @@ func (pr *PRProcessor) Process(req *models.PullRequest) error {
// make sure that prjgit is consistent and only submodules that are to be *updated*
// reset anything that changed that is not part of the prset
// package removals/additions are *not* counted here
org, repo, branch := config.GetPrjGit()
// TODO: this is broken...
if pr, err := prset.GetPrjGitPR(); err == nil && false {
common.LogDebug("Submodule parse begin")
@@ -549,7 +549,7 @@ func (pr *PRProcessor) Process(req *models.PullRequest) error {
} else {
common.LogInfo("* No prjgit")
}
maintainers, err := common.FetchProjectMaintainershipData(Gitea, org, repo, branch)
maintainers, err := common.FetchProjectMaintainershipData(Gitea, config)
if err != nil {
return err
}