Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for GitOps comments on Push commits #1638

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/content/docs/guide/authoringprs.md
Original file line number Diff line number Diff line change
Expand Up @@ -184,9 +184,9 @@ You can find more information about the CEL language spec here :

### Matching a PipelineRun on a regexp in a comment

{{< tech_preview "Matching PipelineRun on regexp in comments" >}}
{{< tech_preview "Matching PipelineRun with regexp on comments" >}}

You can match a PipelineRun on a comment on a Pull Request with the annotation
You can match a PipelineRun on a comment on a Pull Request or a [Pushed Commit]({{< relref "/docs/guide/running.md#gitops-commands-on-pushed-commits" >}}) with the annotation
`pipelinesascode.tekton.dev/on-comment`.

The comment is a regexp and if a newly created comment has this regexp it will
Expand Down
1 change: 0 additions & 1 deletion pkg/pipelineascode/match.go
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,6 @@ func (p *PacRun) getPipelineRunsFromRepo(ctx context.Context, repo *v1alpha1.Rep
p.eventEmitter.EmitMessage(nil, zap.InfoLevel, "RepositoryCannotLocatePipelineRun", msg)
return nil, nil
}

pipelineRuns, err = resolve.MetadataResolve(pipelineRuns)
if err != nil && len(pipelineRuns) == 0 {
p.eventEmitter.EmitMessage(repo, zap.ErrorLevel, "FailedToResolvePipelineRunMetadata", err.Error())
Expand Down
2 changes: 1 addition & 1 deletion pkg/provider/github/detect.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func detectTriggerTypeFromPayload(ghEventType string, eventInt any) (triggertype
return triggertype.Cancel, ""
}
}
return "", fmt.Sprintf("commit_comment: unsupported action \"%s\"", event.GetAction())
return triggertype.Comment, ""
}
return "", fmt.Sprintf("github: event \"%v\" is not supported", ghEventType)
}
4 changes: 2 additions & 2 deletions pkg/provider/github/detect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,13 @@ func TestProvider_Detect(t *testing.T) {
processReq: false,
},
{
name: "invalid commit_comment Event",
name: "non standard commit_comment Event",
event: github.CommitCommentEvent{
Action: github.String("something"),
},
eventType: "commit_comment",
isGH: true,
processReq: false,
processReq: true,
},
{
name: "invalid check run Event",
Expand Down
5 changes: 2 additions & 3 deletions pkg/provider/github/parse_payload.go
Original file line number Diff line number Diff line change
Expand Up @@ -400,9 +400,8 @@ func (v *Provider) handleCommitCommentEvent(ctx context.Context, event *github.C
runevent.SHA = event.GetComment().GetCommitID()
runevent.HeadURL = runevent.URL
runevent.BaseURL = runevent.HeadURL
runevent.EventType = "push"
runevent.TriggerTarget = "push"
runevent.TriggerComment = event.GetComment().GetBody()
runevent.TriggerTarget = triggertype.Push
opscomments.SetEventTypeAndTargetPR(runevent, event.GetComment().GetBody())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to verify
/hello-world works fine on latest pushed commits
but when i try to do on different branches

/hello-world branch:newbranch

Its not working as expected where as /test branch:newbranch works fine

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's fine i guess to say that's not working properly since my feeling is that it's a bit of a edge case


// Set main as default branch to runevent.HeadBranch, runevent.BaseBranch
runevent.HeadBranch, runevent.BaseBranch = "main", "main"
Expand Down
65 changes: 63 additions & 2 deletions test/github_push_retest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,25 +5,86 @@ package test

import (
"context"
"fmt"
"regexp"
"testing"

"github.com/google/go-github/v66/github"
"github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/keys"
"github.com/openshift-pipelines/pipelines-as-code/test/pkg/cctx"
tgithub "github.com/openshift-pipelines/pipelines-as-code/test/pkg/github"
"github.com/openshift-pipelines/pipelines-as-code/test/pkg/options"
twait "github.com/openshift-pipelines/pipelines-as-code/test/pkg/wait"
tektonv1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
"gotest.tools/v3/assert"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

func TestGithubPushRequestGitOpsCommentOnComment(t *testing.T) {
opsComment := "/hello-world"
ctx := context.Background()
g := &tgithub.PRTest{
Label: "Github GitOps push/retest request",
YamlFiles: []string{"testdata/pipelinerun-on-comment-annotation.yaml"},
NoStatusCheck: true,
TargetRefName: options.MainBranch,
}
g.RunPushRequest(ctx, t)
defer g.TearDown(ctx, t)

// let's make sure we didn't create any PipelineRuns since we only match on-comment here
pruns, err := g.Cnx.Clients.Tekton.TektonV1().PipelineRuns(g.TargetNamespace).List(ctx, metav1.ListOptions{})
assert.NilError(t, err)
assert.Equal(t, len(pruns.Items), 0)

g.Cnx.Clients.Log.Infof("Running ops comment %s as Push comment", opsComment)
_, _, err = g.Provider.Client.Repositories.CreateComment(ctx,
g.Options.Organization,
g.Options.Repo, g.SHA,
&github.RepositoryComment{Body: github.String(opsComment)})
assert.NilError(t, err)

waitOpts := twait.Opts{
RepoName: g.TargetNamespace,
Namespace: g.TargetNamespace,
MinNumberStatus: len(g.YamlFiles),
PollTimeout: twait.DefaultTimeout,
TargetSHA: g.SHA,
}
g.Cnx.Clients.Log.Info("Waiting for Repository to be updated")
_, err = twait.UntilRepositoryUpdated(ctx, g.Cnx.Clients, waitOpts)
assert.NilError(t, err)

g.Cnx.Clients.Log.Infof("Check if we have the repository set as succeeded")
repo, err := g.Cnx.Clients.PipelineAsCode.PipelinesascodeV1alpha1().Repositories(g.TargetNamespace).Get(ctx, g.TargetNamespace, metav1.GetOptions{})
assert.NilError(t, err)
assert.Equal(t, repo.Status[len(repo.Status)-1].Conditions[0].Status, corev1.ConditionTrue)

pruns, err = g.Cnx.Clients.Tekton.TektonV1().PipelineRuns(g.TargetNamespace).List(ctx, metav1.ListOptions{})
assert.NilError(t, err)
assert.Equal(t, len(pruns.Items), len(g.YamlFiles))
lastPrName := pruns.Items[0].GetName()
err = twait.RegexpMatchingInPodLog(
context.Background(),
g.Cnx,
g.TargetNamespace,
fmt.Sprintf("tekton.dev/pipelineRun=%s", lastPrName),
"step-task",
*regexp.MustCompile(opsComment),
"",
2)

assert.NilError(t, err)
}

func TestGithubPushRequestGitOpsCommentRetest(t *testing.T) {
ctx := context.Background()
g := &tgithub.PRTest{
Label: "Github GitOps push/retest request",
YamlFiles: []string{"testdata/pipelinerun-on-push.yaml", "testdata/pipelinerun.yaml"},
Label: "Github GitOps push/retest request",
YamlFiles: []string{
"testdata/pipelinerun-on-push.yaml", "testdata/pipelinerun.yaml",
},
}
g.RunPushRequest(ctx, t)
defer g.TearDown(ctx, t)
Expand Down
61 changes: 42 additions & 19 deletions test/pkg/github/pr.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/openshift-pipelines/pipelines-as-code/test/pkg/options"
"github.com/openshift-pipelines/pipelines-as-code/test/pkg/payload"
"github.com/openshift-pipelines/pipelines-as-code/test/pkg/repository"
"github.com/openshift-pipelines/pipelines-as-code/test/pkg/scm"
"github.com/openshift-pipelines/pipelines-as-code/test/pkg/wait"
"github.com/tektoncd/pipeline/pkg/names"
"go.uber.org/zap"
Expand Down Expand Up @@ -72,7 +73,7 @@ func PushFilesToRef(ctx context.Context, client *ghlib.Client, commitMessage, ba
},
}, &ghlib.CreateCommitOptions{})
if err != nil {
return "", nil, err
return "", nil, fmt.Errorf("error creating commit: %w", err)
}

ref := &ghlib.Reference{
Expand All @@ -83,9 +84,8 @@ func PushFilesToRef(ctx context.Context, client *ghlib.Client, commitMessage, ba
}
vref, _, err := client.Git.CreateRef(ctx, owner, repo, ref)
if err != nil {
return "", nil, err
return "", nil, fmt.Errorf("error creating ref: %w", err)
}

return commit.GetSHA(), vref, nil
}

Expand Down Expand Up @@ -151,9 +151,10 @@ func (g *PRTest) RunPullRequest(ctx context.Context, t *testing.T) {
targetRefName := fmt.Sprintf("refs/heads/%s",
names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("pac-e2e-test"))

sha, vref, err := PushFilesToRef(ctx, ghcnx.Client, logmsg, repoinfo.GetDefaultBranch(), targetRefName,
opts.Organization, opts.Repo, entries)
// TODO(chmouel): Use scm.PushFilesToRefGit instead of API like push
sha, vref, err := PushFilesToRef(ctx, ghcnx.Client, logmsg, repoinfo.GetDefaultBranch(), targetRefName, opts.Organization, opts.Repo, entries)
assert.NilError(t, err)

g.Logger.Infof("Commit %s has been created and pushed to %s", sha, vref.GetURL())
number, err := PRCreate(ctx, runcnx, ghcnx, opts.Organization,
opts.Repo, targetRefName, repoinfo.GetDefaultBranch(), logmsg)
Expand Down Expand Up @@ -196,14 +197,21 @@ func (g *PRTest) TearDown(ctx context.Context, t *testing.T) {
if g.TargetNamespace != "" {
repository.NSTearDown(ctx, t, g.Cnx, g.TargetNamespace)
}
g.Logger.Infof("Deleting Ref %s", g.TargetRefName)
_, err := g.Provider.Client.Git.DeleteRef(ctx, g.Options.Organization, g.Options.Repo, g.TargetRefName)
assert.NilError(t, err)
if g.TargetRefName != options.MainBranch {
branch := fmt.Sprintf("heads/%s", filepath.Base(g.TargetRefName))
g.Logger.Infof("Deleting Ref %s", branch)
_, err := g.Provider.Client.Git.DeleteRef(ctx, g.Options.Organization, g.Options.Repo, branch)
assert.NilError(t, err)
}
}

func (g *PRTest) RunPushRequest(ctx context.Context, t *testing.T) {
targetNS := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("pac-e2e-push")
targetBranch := targetNS

targetBranch := g.TargetRefName
if targetBranch == "" {
targetBranch = targetNS
}
targetEvent := "push"
ctx, runcnx, opts, ghcnx, err := Setup(ctx, g.SecondController, g.Webhook)
assert.NilError(t, err)
Expand Down Expand Up @@ -234,19 +242,34 @@ func (g *PRTest) RunPushRequest(ctx context.Context, t *testing.T) {
targetNS, targetBranch, targetEvent, map[string]string{})
assert.NilError(t, err)

targetRefName := fmt.Sprintf("refs/heads/%s", targetBranch)
sha, vref, err := PushFilesToRef(ctx, ghcnx.Client, logmsg, repoinfo.GetDefaultBranch(), targetRefName, opts.Organization, opts.Repo, entries)
g.Logger.Infof("Commit %s has been created and pushed to %s", sha, vref.GetURL())
targetRefName := targetBranch
cloneURL, err := scm.MakeGitCloneURL(repoinfo.GetCloneURL(), "git", *ghcnx.Token)
assert.NilError(t, err)
scmOpts := scm.Opts{
GitURL: cloneURL,
TargetRefName: targetRefName,
BaseRefName: repoinfo.GetDefaultBranch(),
WebURL: repoinfo.GetHTMLURL(),
Log: runcnx.Clients.Log,
CommitTitle: logmsg,
}
scm.PushFilesToRefGit(t, &scmOpts, entries)
branch, _, err := ghcnx.Client.Repositories.GetBranch(ctx, opts.Organization, opts.Repo, targetBranch, 1)
assert.NilError(t, err)
sha := branch.GetCommit().GetSHA()
g.Logger.Infof("Commit %s has been created and pushed to %s in branch %s", sha, branch.GetCommit().GetHTMLURL(), branch.GetName())
assert.NilError(t, err)

sopt := wait.SuccessOpt{
Title: logmsg,
OnEvent: triggertype.Push.String(),
TargetNS: targetNS,
NumberofPRMatch: len(g.YamlFiles),
SHA: sha,
if !g.NoStatusCheck {
sopt := wait.SuccessOpt{
Title: logmsg,
OnEvent: triggertype.Push.String(),
TargetNS: targetNS,
NumberofPRMatch: len(g.YamlFiles),
SHA: sha,
}
wait.Succeeded(ctx, t, runcnx, opts, sopt)
}
wait.Succeeded(ctx, t, runcnx, opts, sopt)

g.Cnx = runcnx
g.Options = opts
Expand Down
1 change: 1 addition & 0 deletions test/pkg/github/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ func Setup(ctx context.Context, onSecondController, viaDirectWebhook bool) (cont
Token: githubToken,
URL: githubURL,
}
gprovider.Token = &githubToken
// TODO: before PR
if err := gprovider.SetClient(ctx, nil, event, nil, nil); err != nil {
return ctx, nil, options.E2E{}, github.New(), err
Expand Down
Loading