Skip to content

Commit 2274e06

Browse files
zakiskzakisk
committed
fix(github): support lightweight tags for GitOps commands
When GitOps command `/test` or `/retest` is triggered on a lightweight tag, it was causing an error in ParsePayload because GitHub treats annotated and lightweight tags differently: - Annotated tags: ref object type is "tag", requires GetTag API call to resolve the commit SHA - Lightweight tags: ref object type is "commit", the ref already contains the commit SHA directly The fix uses a switch statement to handle both cases and returns an error for unexpected object types. Also adds: - Unit tests for lightweight tag and invalid object type scenarios - E2E test coverage for both annotated and lightweight tags - LightweightTag option in test framework https://issues.redhat.com/browse/SRVKP-10467 Signed-off-by: Zaki Shaikh <zashaikh@redhat.com>
1 parent 8eb79d5 commit 2274e06

5 files changed

Lines changed: 169 additions & 117 deletions

File tree

‎pkg/provider/github/parse_payload.go‎

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -662,14 +662,28 @@ func (v *Provider) handleCommitCommentEvent(ctx context.Context, event *github.C
662662
if err != nil {
663663
return runevent, fmt.Errorf("error getting ref for tag %s: %w", tagName, err)
664664
}
665-
// get the tag object to get the SHA
666-
tag, _, err := wrapAPI(v, "get_tag", func() (*github.Tag, *github.Response, error) {
667-
return v.Client().Git.GetTag(ctx, runevent.Organization, runevent.Repository, ref.GetObject().GetSHA())
668-
})
669-
if err != nil {
670-
return runevent, fmt.Errorf("error getting tag %s: %w", tagName, err)
665+
666+
tagSha := ""
667+
668+
switch ref.GetObject().GetType() {
669+
case "tag":
670+
// annotated tag - get the tag object to resolve the commit SHA
671+
tag, _, err := wrapAPI(v, "get_tag", func() (*github.Tag, *github.Response, error) {
672+
return v.Client().Git.GetTag(ctx, runevent.Organization, runevent.Repository, ref.GetObject().GetSHA())
673+
})
674+
if err != nil {
675+
return runevent, fmt.Errorf("error getting tag %s: %w", tagName, err)
676+
}
677+
tagSha = tag.GetObject().GetSHA()
678+
case "commit":
679+
// lightweight tag - ref contains the commit SHA directly.
680+
// trying to get the tag object would return an error.
681+
tagSha = ref.GetObject().GetSHA()
682+
default:
683+
return runevent, fmt.Errorf("invalid object type for tag %s: %s", tagName, ref.GetObject().GetType())
671684
}
672-
if tag.GetObject().GetSHA() != runevent.SHA {
685+
686+
if tagSha != runevent.SHA {
673687
return runevent, fmt.Errorf("provided SHA %s is not the tagged commit for the tag %s", runevent.SHA, tagName)
674688
}
675689
runevent.HeadBranch = tagPath

‎pkg/provider/github/parse_payload_test.go‎

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -409,6 +409,7 @@ func TestParsePayLoad(t *testing.T) {
409409
wantedTagName string
410410
isCancelPipelineRunEnabled bool
411411
skipPushEventForPRCommits bool
412+
objectType string
412413
}{
413414
{
414415
name: "bad/unknown event",
@@ -766,6 +767,7 @@ func TestParsePayLoad(t *testing.T) {
766767
},
767768
},
768769
shaRet: "samplePRsha",
770+
objectType: "tag",
769771
wantedBranchName: "refs/tags/v1.0.0",
770772
},
771773
{
@@ -783,8 +785,46 @@ func TestParsePayLoad(t *testing.T) {
783785
},
784786
shaRet: "samplePRsha",
785787
targetPipelinerun: "dummy",
788+
objectType: "tag",
786789
wantedBranchName: "refs/tags/v1.0.0",
787790
},
791+
{
792+
name: "good/commit comment for test tag with object type commit",
793+
eventType: "commit_comment",
794+
triggerTarget: "push",
795+
githubClient: true,
796+
payloadEventStruct: github.CommitCommentEvent{
797+
Repo: sampleRepo,
798+
Comment: &github.RepositoryComment{
799+
CommitID: github.Ptr("samplePRsha"),
800+
HTMLURL: github.Ptr("/777"),
801+
Body: github.Ptr("/test dummy tag:v1.0.0"),
802+
},
803+
},
804+
shaRet: "samplePRsha",
805+
targetPipelinerun: "dummy",
806+
objectType: "commit",
807+
wantedBranchName: "refs/tags/v1.0.0",
808+
},
809+
{
810+
name: "bad/commit comment for test tag invalid object type",
811+
eventType: "commit_comment",
812+
triggerTarget: "push",
813+
githubClient: true,
814+
payloadEventStruct: github.CommitCommentEvent{
815+
Repo: sampleRepo,
816+
Comment: &github.RepositoryComment{
817+
CommitID: github.Ptr("samplePRsha"),
818+
HTMLURL: github.Ptr("/777"),
819+
Body: github.Ptr("/test dummy tag:v1.0.0"),
820+
},
821+
},
822+
shaRet: "samplePRsha",
823+
targetPipelinerun: "dummy",
824+
wantedBranchName: "refs/tags/v1.0.0",
825+
objectType: "blob",
826+
wantErrString: "invalid object type for tag v1.0.0: blob",
827+
},
788828
{
789829
name: "bad/commit comment for test with pipelinerun name and wrong tag keyword",
790830
eventType: "commit_comment",
@@ -1020,7 +1060,8 @@ func TestParsePayLoad(t *testing.T) {
10201060
mux.HandleFunc(fmt.Sprintf("/repos/%s/%s/git/ref/tags/v1.0.0", "owner", "reponame"), func(rw http.ResponseWriter, _ *http.Request) {
10211061
ref := &github.Reference{
10221062
Object: &github.GitObject{
1023-
SHA: github.Ptr("samplePRsha"),
1063+
SHA: github.Ptr("samplePRsha"),
1064+
Type: github.Ptr(tt.objectType),
10241065
},
10251066
}
10261067
bjeez, _ := json.Marshal(ref)

‎test/github_tag_gitops_test.go‎

Lines changed: 84 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -4,132 +4,122 @@ package test
44

55
import (
66
"context"
7-
"fmt"
87
"net/http"
9-
"path/filepath"
108
"testing"
119

1210
"github.com/google/go-github/v81/github"
13-
"github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/keys"
1411
tgithub "github.com/openshift-pipelines/pipelines-as-code/test/pkg/github"
1512
"github.com/openshift-pipelines/pipelines-as-code/test/pkg/payload"
1613
"github.com/openshift-pipelines/pipelines-as-code/test/pkg/scm"
1714
twait "github.com/openshift-pipelines/pipelines-as-code/test/pkg/wait"
1815
"github.com/tektoncd/pipeline/pkg/names"
1916
"gotest.tools/v3/assert"
20-
corev1 "k8s.io/api/core/v1"
21-
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2217
)
2318

2419
func TestGithubGitOpsCommentOnTag(t *testing.T) {
2520
ctx := context.Background()
2621
ctx, runcnx, opts, ghcnx, err := tgithub.Setup(ctx, false, false)
2722
assert.NilError(t, err)
28-
tagName := "v1.0.0"
29-
comment := "/test tag:" + tagName
30-
sha := ""
31-
targetBranch := "release-" + tagName
32-
label := "Testing GitHub commit comment on tag with GitHub apps integration"
23+
var g *tgithub.PRTest
3324
targetNS := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("pac-e2e-tag")
34-
numberOfPRs := 1
3525
repoinfo, resp, err := ghcnx.Client().Repositories.Get(ctx, opts.Organization, opts.Repo)
3626
assert.NilError(t, err)
3727
if resp != nil && resp.StatusCode == http.StatusNotFound {
38-
t.Errorf("Repository %s not found in %s", opts.Organization, opts.Repo)
28+
t.Errorf("Repository %s not found in %s", opts.Repo, opts.Organization)
3929
}
4030
err = tgithub.CreateCRD(ctx, t, repoinfo, runcnx, opts, targetNS)
4131
assert.NilError(t, err)
42-
4332
runcnx.Clients.Log.Infof("Repository %s has been created successfully", targetNS)
4433

45-
ref, resp, err := ghcnx.Client().Git.GetRef(ctx, opts.Organization, opts.Repo, "refs/tags/"+tagName)
46-
if err != nil && resp.StatusCode == http.StatusNotFound {
47-
runcnx.Clients.Log.Infof("Tag %s is not found on the repository", tagName)
48-
runcnx.Clients.Log.Infof("Creating tag %s on the repository", tagName)
49-
yamlFiles := []string{"testdata/pipelinerun-on-tag.yaml"}
50-
yamlEntries := map[string]string{}
51-
for _, v := range yamlFiles {
52-
yamlEntries[filepath.Join(".tekton", filepath.Base(v))] = v
34+
tags := []string{"v1.0.0", "v1.0.0-lightweight"}
35+
for _, tag := range tags {
36+
comment := "/test tag:" + tag
37+
sha := ""
38+
targetRefName := "release-" + tag
39+
label := ""
40+
if tag == "v1.0.0" {
41+
label = "Testing GitHub commit comment on tag with GitHub apps integration"
42+
} else {
43+
label = "Testing GitHub commit comment on lightweight tag with GitHub apps integration"
5344
}
5445

55-
entries, err := payload.GetEntries(yamlEntries,
56-
targetNS, "refs/tags/*", "push", map[string]string{})
57-
assert.NilError(t, err)
58-
59-
targetRefName := targetBranch
60-
cloneURL, err := scm.MakeGitCloneURL(repoinfo.GetCloneURL(), "git", *ghcnx.Token)
61-
assert.NilError(t, err)
62-
scmOpts := scm.Opts{
63-
GitURL: cloneURL,
64-
TargetRefName: targetRefName,
65-
BaseRefName: repoinfo.GetDefaultBranch(),
66-
WebURL: repoinfo.GetHTMLURL(),
67-
Log: runcnx.Clients.Log,
68-
CommitTitle: label,
46+
numberOfPRs := 1
47+
48+
ref, resp, err := ghcnx.Client().Git.GetRef(ctx, opts.Organization, opts.Repo, "refs/tags/"+tag)
49+
if err != nil && resp.StatusCode == http.StatusNotFound {
50+
runcnx.Clients.Log.Infof("Tag %s is not found on the repository", tag)
51+
runcnx.Clients.Log.Infof("Creating tag %s on the repository", tag)
52+
entries, err := payload.GetEntries(map[string]string{".tekton/pipelinerun-on-tag.yaml": "testdata/pipelinerun-on-tag.yaml"},
53+
targetNS, "refs/tags/*", "push", map[string]string{})
54+
assert.NilError(t, err)
55+
56+
cloneURL, err := scm.MakeGitCloneURL(repoinfo.GetCloneURL(), "git", *ghcnx.Token)
57+
assert.NilError(t, err)
58+
scmOpts := scm.Opts{
59+
GitURL: cloneURL,
60+
TargetRefName: targetRefName,
61+
BaseRefName: repoinfo.GetDefaultBranch(),
62+
WebURL: repoinfo.GetHTMLURL(),
63+
Log: runcnx.Clients.Log,
64+
CommitTitle: label,
65+
}
66+
scm.PushFilesToRefGit(t, &scmOpts, entries)
67+
branch, _, err := ghcnx.Client().Repositories.GetBranch(ctx, opts.Organization, opts.Repo, targetRefName, 1)
68+
assert.NilError(t, err)
69+
70+
sha = branch.GetCommit().GetSHA()
71+
72+
// when we're testing a lightweight tag, we need to set the opts.LightweightTag to true
73+
// so that we don't create an annotated tag. on creating lightweight tag, ref contains the commit SHA directly.
74+
opts.LightweightTag = tag == "v1.0.0-lightweight"
75+
runcnx.Clients.Log.Infof("is Lightweight tag: %t", opts.LightweightTag)
76+
_, err = tgithub.CreateTag(ctx, t, runcnx, ghcnx, opts, sha, tag)
77+
assert.NilError(t, err)
78+
numberOfPRs++
79+
} else {
80+
// else if tag is already created, we need to get the tag object to get the commit SHA
81+
if tag == "v1.0.0" {
82+
// if tag is annotated tag, we need to get the tag object to get the commit SHA
83+
runcnx.Clients.Log.Infof("Tag %s is already created on the repository", tag)
84+
runcnx.Clients.Log.Infof("Getting tag %s", tag)
85+
tagRef, _, err := ghcnx.Client().Git.GetTag(ctx, opts.Organization, opts.Repo, ref.GetObject().GetSHA())
86+
assert.NilError(t, err)
87+
sha = tagRef.GetObject().GetSHA()
88+
} else {
89+
// else if tag is lightweight tag, we need to get the commit SHA from the ref
90+
runcnx.Clients.Log.Infof("Lightweight tag %s is already created on the repository", tag)
91+
sha = ref.GetObject().GetSHA()
92+
}
6993
}
70-
scm.PushFilesToRefGit(t, &scmOpts, entries)
71-
branch, _, err := ghcnx.Client().Repositories.GetBranch(ctx, opts.Organization, opts.Repo, targetBranch, 1)
72-
assert.NilError(t, err)
7394

74-
sha = branch.GetCommit().GetSHA()
95+
g = &tgithub.PRTest{
96+
Label: label,
97+
Cnx: runcnx,
98+
Logger: runcnx.Clients.Log,
99+
Options: opts,
100+
Provider: ghcnx,
101+
TargetNamespace: targetNS,
102+
PRNumber: -1,
103+
SHA: sha,
104+
}
105+
defer g.TearDown(ctx, t)
75106

76-
_, err = tgithub.CreateTag(ctx, t, runcnx, ghcnx, opts, sha, tagName)
77-
assert.NilError(t, err)
78-
numberOfPRs++
79-
} else {
80-
runcnx.Clients.Log.Infof("Tag %s is already created on the repository", tagName)
81-
runcnx.Clients.Log.Infof("Getting tag %s", tagName)
82-
tagRef, _, err := ghcnx.Client().Git.GetTag(ctx, opts.Organization, opts.Repo, ref.GetObject().GetSHA())
107+
runcnx.Clients.Log.Infof("%s on tag commit", comment)
108+
_, _, err = ghcnx.Client().Repositories.CreateComment(ctx,
109+
opts.Organization,
110+
opts.Repo, sha, // this is the commit sha of the tag v1.0.0
111+
&github.RepositoryComment{Body: github.Ptr(comment)})
83112
assert.NilError(t, err)
84-
sha = tagRef.GetObject().GetSHA()
85-
}
86-
87-
g := &tgithub.PRTest{
88-
Label: label,
89-
Cnx: runcnx,
90-
Logger: runcnx.Clients.Log,
91-
Options: opts,
92-
Provider: ghcnx,
93-
TargetNamespace: targetNS,
94-
PRNumber: -1,
95-
SHA: sha,
96-
}
97-
defer g.TearDown(ctx, t)
98113

99-
runcnx.Clients.Log.Infof("%s on tag commit", comment)
100-
_, _, err = ghcnx.Client().Repositories.CreateComment(ctx,
101-
opts.Organization,
102-
opts.Repo, sha, // this is the commit sha of the tag v1.0.0
103-
&github.RepositoryComment{Body: github.Ptr(comment)})
104-
assert.NilError(t, err)
105-
106-
waitOpts := twait.Opts{
107-
RepoName: targetNS,
108-
Namespace: targetNS,
109-
MinNumberStatus: numberOfPRs,
110-
PollTimeout: twait.DefaultTimeout,
111-
TargetSHA: sha, // this is the commit sha of the tag v1.0.0
112-
}
113-
runcnx.Clients.Log.Info("Waiting for Repository to be updated")
114-
_, err = twait.UntilRepositoryUpdated(ctx, runcnx.Clients, waitOpts)
115-
assert.NilError(t, err)
116-
117-
runcnx.Clients.Log.Infof("Check if we have the repository set as succeeded")
118-
repo, err := runcnx.Clients.PipelineAsCode.PipelinesascodeV1alpha1().Repositories(targetNS).Get(ctx, targetNS, metav1.GetOptions{})
119-
assert.NilError(t, err)
120-
assert.Equal(t, repo.Status[len(repo.Status)-1].Conditions[0].Status, corev1.ConditionTrue)
121-
122-
pruns, err := runcnx.Clients.Tekton.TektonV1().PipelineRuns(targetNS).List(ctx, metav1.ListOptions{
123-
LabelSelector: fmt.Sprintf("%s=%s", keys.SHA, sha),
124-
})
125-
assert.NilError(t, err)
126-
assert.Equal(t, len(pruns.Items), numberOfPRs)
127-
128-
for i := range pruns.Items {
129-
sData, err := runcnx.Clients.Kube.CoreV1().Secrets(targetNS).Get(ctx, pruns.Items[i].GetAnnotations()[keys.GitAuthSecret], metav1.GetOptions{})
114+
waitOpts := twait.Opts{
115+
RepoName: targetNS,
116+
Namespace: targetNS,
117+
MinNumberStatus: numberOfPRs,
118+
PollTimeout: twait.DefaultTimeout,
119+
TargetSHA: sha, // this is the commit sha of the tag v1.0.0
120+
}
121+
runcnx.Clients.Log.Info("Waiting for PipelineRun to be created")
122+
err = twait.UntilPipelineRunCreated(ctx, runcnx.Clients, waitOpts)
130123
assert.NilError(t, err)
131-
assert.Assert(t, string(sData.Data["git-provider-token"]) != "")
132-
assert.Assert(t, string(sData.Data[".git-credentials"]) != "")
133-
assert.Assert(t, string(sData.Data[".gitconfig"]) != "")
134124
}
135125
}

‎test/pkg/github/tag.go‎

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -13,29 +13,35 @@ import (
1313
)
1414

1515
func CreateTag(ctx context.Context, t *testing.T, runcnx *params.Run, ghcnx *pacgithub.Provider, opts options.E2E, sha, tagName string) (*github.Tag, error) {
16-
createTag := github.CreateTag{
17-
Tag: tagName,
18-
Message: "Release " + tagName,
19-
Object: sha,
20-
Type: "commit",
21-
Tagger: &github.CommitAuthor{
22-
Name: github.Ptr("OpenShift Pipelines E2E test"),
23-
Email: github.Ptr("e2e-pipeline@redhat.com"),
24-
Date: &github.Timestamp{Time: time.Now()},
25-
},
16+
var tag *github.Tag
17+
var err error
18+
if !opts.LightweightTag {
19+
createTag := github.CreateTag{
20+
Tag: tagName,
21+
Message: "Release " + tagName,
22+
Object: sha,
23+
Type: "commit",
24+
Tagger: &github.CommitAuthor{
25+
Name: github.Ptr("OpenShift Pipelines E2E test"),
26+
Email: github.Ptr("e2e-pipeline@redhat.com"),
27+
Date: &github.Timestamp{Time: time.Now()},
28+
},
29+
}
30+
tag, _, err = ghcnx.Client().Git.CreateTag(ctx, opts.Organization, opts.Repo, createTag)
31+
assert.NilError(t, err)
32+
// if its annotated tag, SHA is different from the commit SHA
33+
sha = tag.GetSHA()
2634
}
27-
tag, _, err := ghcnx.Client().Git.CreateTag(ctx, opts.Organization, opts.Repo, createTag)
28-
assert.NilError(t, err)
2935

3036
createRef := github.CreateRef{
3137
Ref: "refs/tags/" + tagName,
32-
SHA: *tag.SHA,
38+
SHA: sha,
3339
}
3440
_, _, err = ghcnx.Client().Git.CreateRef(ctx, opts.Organization, opts.Repo, createRef)
3541
assert.NilError(t, err)
36-
runcnx.Clients.Log.Infof("Tag %s has been created successfully", tag.GetTag())
42+
runcnx.Clients.Log.Infof("Tag %s has been created successfully", tagName)
3743

38-
return tag, nil
44+
return tag, err
3945
}
4046

4147
func DeleteTag(ctx context.Context, ghcnx *pacgithub.Provider, opts options.E2E, tagName string) error {

‎test/pkg/options/options.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ type E2E struct {
1010
Concurrency int
1111
UserName string
1212
Password string
13+
LightweightTag bool
1314
Settings v1alpha1.Settings
1415
}
1516

0 commit comments

Comments
 (0)