Skip to content

Commit 6a17c4f

Browse files
committed
fix: GitLab provider getting a URL on same host
Refactored the GitLab provider to correctly handle project IDs and fetch raw files with our token. We were fetching the URL as is when there are URLs don't work like GitHub which allows passing a token and getting a private raw URL directly. Jira: https://issues.redhat.com/browse/SRVKP-7357 AI-assisted-by: Google Gemini 3 Signed-off-by: Chmouel Boudjnah <[email protected]>
1 parent 3d87a1b commit 6a17c4f

File tree

2 files changed

+223
-8
lines changed

2 files changed

+223
-8
lines changed

pkg/provider/gitlab/task.go

Lines changed: 39 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,18 @@ package gitlab
22

33
import (
44
"context"
5+
"errors"
56
"fmt"
67
"net/url"
78
"regexp"
89

910
"github.com/openshift-pipelines/pipelines-as-code/pkg/params/info"
1011
"github.com/openshift-pipelines/pipelines-as-code/pkg/provider"
12+
gl "gitlab.com/gitlab-org/api/client-go"
1113
)
1214

1315
type gitLabInfo struct {
16+
Scheme string
1417
Host string
1518
GroupOrUser string
1619
Repository string
@@ -55,6 +58,7 @@ func extractGitLabInfo(gitlabURL string) (*gitLabInfo, error) {
5558
}
5659

5760
return &gitLabInfo{
61+
Scheme: parsedURL.Scheme,
5862
Host: parsedURL.Host,
5963
GroupOrUser: groupOrUser,
6064
Repository: repoName,
@@ -65,7 +69,7 @@ func extractGitLabInfo(gitlabURL string) (*gitLabInfo, error) {
6569

6670
// GetTaskURI if we are getting a URL from the same URL where the provider is,
6771
// it means we can try to get the file with the provider token.
68-
func (v *Provider) GetTaskURI(ctx context.Context, event *info.Event, uri string) (bool, string, error) {
72+
func (v *Provider) GetTaskURI(_ context.Context, event *info.Event, uri string) (bool, string, error) {
6973
if ret := provider.CompareHostOfURLS(uri, event.URL); !ret {
7074
return false, "", nil
7175
}
@@ -74,13 +78,40 @@ func (v *Provider) GetTaskURI(ctx context.Context, event *info.Event, uri string
7478
return false, "", err
7579
}
7680

77-
nEvent := info.NewEvent()
78-
nEvent.Organization = extracted.GroupOrUser
79-
nEvent.Repository = extracted.Repository
80-
nEvent.BaseBranch = extracted.Revision
81-
ret, err := v.GetFileInsideRepo(ctx, nEvent, extracted.FilePath, extracted.Revision)
81+
// Use the existing client if available, otherwise create a temporary one.
82+
// We avoid storing it to prevent side effects on the provider's state.
83+
client := v.gitlabClient
84+
if client == nil {
85+
baseURL := fmt.Sprintf("%s://%s", extracted.Scheme, extracted.Host)
86+
var clientErr error
87+
client, clientErr = gl.NewClient(event.Provider.Token, gl.WithBaseURL(baseURL))
88+
if clientErr != nil {
89+
return false, "", fmt.Errorf("failed to create gitlab client: %w", clientErr)
90+
}
91+
}
92+
93+
// Construct the project slug for the remote repository
94+
projectSlug := extracted.GroupOrUser + "/" + extracted.Repository
95+
96+
// Get the project ID for the remote repository
97+
project, _, err := client.Projects.GetProject(projectSlug, &gl.GetProjectOptions{})
8298
if err != nil {
83-
return false, "", err
99+
if errors.Is(err, gl.ErrNotFound) {
100+
return false, "", nil
101+
}
102+
return false, "", fmt.Errorf("failed to get project ID for %s: %w", projectSlug, err)
103+
}
104+
105+
// Fetch the file from the remote repository
106+
opt := &gl.GetRawFileOptions{
107+
Ref: gl.Ptr(extracted.Revision),
108+
}
109+
file, _, err := client.RepositoryFiles.GetRawFile(project.ID, extracted.FilePath, opt)
110+
if err != nil {
111+
if errors.Is(err, gl.ErrNotFound) {
112+
return false, "", nil
113+
}
114+
return false, "", fmt.Errorf("failed to get file %s from remote repository: %w", extracted.FilePath, err)
84115
}
85-
return true, ret, nil
116+
return true, string(file), nil
86117
}

pkg/provider/gitlab/task_test.go

Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,14 @@
11
package gitlab
22

33
import (
4+
"context"
5+
"fmt"
6+
"net/http"
7+
"net/http/httptest"
48
"testing"
59

10+
"github.com/openshift-pipelines/pipelines-as-code/pkg/params/info"
11+
gl "gitlab.com/gitlab-org/api/client-go"
612
"gotest.tools/v3/assert"
713
)
814

@@ -16,6 +22,7 @@ func TestExtractGitLabInfo(t *testing.T) {
1622
name: "custom host",
1723
url: "https://gitlab.chmouel.com/group/subgroup/repo/-/blob/main/README.md?ref_type=heads",
1824
expected: &gitLabInfo{
25+
Scheme: "https",
1926
Host: "gitlab.chmouel.com",
2027
GroupOrUser: "group/subgroup",
2128
Repository: "repo",
@@ -27,6 +34,7 @@ func TestExtractGitLabInfo(t *testing.T) {
2734
name: "org repo",
2835
url: "https://gitlab.com/org/repo/-/blob/main/README.md",
2936
expected: &gitLabInfo{
37+
Scheme: "https",
3038
Host: "gitlab.com",
3139
GroupOrUser: "org",
3240
Repository: "repo",
@@ -38,6 +46,7 @@ func TestExtractGitLabInfo(t *testing.T) {
3846
name: "long group and subgroups",
3947
url: "https://gitlab.com/gitlab-com/partners/alliance/corp/sandbox/another/foo-foo/-/raw/main/hello.txt?ref_type=heads",
4048
expected: &gitLabInfo{
49+
Scheme: "https",
4150
Host: "gitlab.com",
4251
GroupOrUser: "gitlab-com/partners/alliance/corp/sandbox/another",
4352
Repository: "foo-foo",
@@ -55,3 +64,178 @@ func TestExtractGitLabInfo(t *testing.T) {
5564
})
5665
}
5766
}
67+
68+
func TestGetTaskURI(t *testing.T) {
69+
ctx := context.Background()
70+
expectedContent := "apiVersion: tekton.dev/v1beta1\nkind: Pipeline\nmetadata:\n name: test-pipeline\nspec:\n tasks:\n - name: echo-task\n taskSpec:\n steps:\n - name: echo\n image: ubuntu\n script: |\n echo \"Hello from remote pipeline!\"\n"
71+
72+
tests := []struct {
73+
name string
74+
setup func(t *testing.T) (*httptest.Server, string, string)
75+
wantFound bool
76+
wantErr bool
77+
wantErrContains string
78+
wantContent string
79+
}{
80+
{
81+
name: "success",
82+
setup: func(t *testing.T) (*httptest.Server, string, string) {
83+
projectID := 12345
84+
projectSlug := "chmouel/dazgo"
85+
revision := "main"
86+
filePath := "task.yaml"
87+
88+
mux := http.NewServeMux()
89+
mux.HandleFunc(fmt.Sprintf("/api/v4/projects/%s", gl.PathEscape(projectSlug)), func(w http.ResponseWriter, _ *http.Request) {
90+
fmt.Fprintf(w, `{"id": %d, "path_with_namespace": "%s"}`, projectID, projectSlug)
91+
})
92+
93+
mux.HandleFunc(fmt.Sprintf("/api/v4/projects/%d/repository/files/%s/raw", projectID, gl.PathEscape(filePath)), func(w http.ResponseWriter, r *http.Request) {
94+
assert.Equal(t, "token", r.Header.Get("Private-Token"), "Expected Private-Token header to be 'token'")
95+
assert.Equal(t, revision, r.URL.Query().Get("ref"), "Expected 'ref' query parameter to be 'main'")
96+
fmt.Fprint(w, expectedContent)
97+
})
98+
99+
server := httptest.NewServer(mux)
100+
remotePipelineURL := fmt.Sprintf("%s/%s/-/raw/%s/%s", server.URL, projectSlug, revision, filePath)
101+
return server, server.URL, remotePipelineURL
102+
},
103+
wantFound: true,
104+
wantErr: false,
105+
wantContent: expectedContent,
106+
},
107+
{
108+
name: "file not found (404)",
109+
setup: func(_ *testing.T) (*httptest.Server, string, string) {
110+
mux := http.NewServeMux()
111+
projectID := 12345
112+
projectSlug := "chmouel/dazgo"
113+
filePath := "nonexistent.yaml"
114+
115+
mux.HandleFunc(fmt.Sprintf("/api/v4/projects/%s", gl.PathEscape(projectSlug)), func(w http.ResponseWriter, _ *http.Request) {
116+
fmt.Fprintf(w, `{"id": %d, "path_with_namespace": "%s"}`, projectID, projectSlug)
117+
})
118+
119+
mux.HandleFunc(fmt.Sprintf("/api/v4/projects/%d/repository/files/%s/raw", projectID, gl.PathEscape(filePath)), func(w http.ResponseWriter, _ *http.Request) {
120+
w.WriteHeader(http.StatusNotFound)
121+
fmt.Fprint(w, `{"message": "404 File Not Found"}`)
122+
})
123+
124+
server := httptest.NewServer(mux)
125+
remotePipelineURL := fmt.Sprintf("%s/%s/-/raw/main/%s", server.URL, projectSlug, filePath)
126+
return server, server.URL, remotePipelineURL
127+
},
128+
wantFound: false,
129+
wantErr: false,
130+
},
131+
{
132+
name: "project not found (404)",
133+
setup: func(_ *testing.T) (*httptest.Server, string, string) {
134+
mux := http.NewServeMux()
135+
projectSlug := "nonexistent/project"
136+
137+
mux.HandleFunc(fmt.Sprintf("/api/v4/projects/%s", gl.PathEscape(projectSlug)), func(w http.ResponseWriter, _ *http.Request) {
138+
w.WriteHeader(http.StatusNotFound)
139+
fmt.Fprint(w, `{"message": "404 Project Not Found"}`)
140+
})
141+
142+
server := httptest.NewServer(mux)
143+
remotePipelineURL := fmt.Sprintf("%s/%s/-/raw/main/task.yaml", server.URL, projectSlug)
144+
return server, server.URL, remotePipelineURL
145+
},
146+
wantFound: false,
147+
wantErr: false,
148+
},
149+
{
150+
name: "invalid gitlab URL format",
151+
setup: func(_ *testing.T) (*httptest.Server, string, string) {
152+
return nil, "https://example.com", "https://example.com/invalid/url/format"
153+
},
154+
wantFound: false,
155+
wantErr: true,
156+
wantErrContains: "URL does not match the expected GitLab pattern",
157+
},
158+
{
159+
name: "different host - should return not found",
160+
setup: func(_ *testing.T) (*httptest.Server, string, string) {
161+
return nil, "https://gitlab.com", "https://different-host.com/chmouel/dazgo/-/raw/main/task.yaml"
162+
},
163+
wantFound: false,
164+
wantErr: false,
165+
},
166+
{
167+
name: "API error on GetProject",
168+
setup: func(_ *testing.T) (*httptest.Server, string, string) {
169+
mux := http.NewServeMux()
170+
projectSlug := "chmouel/dazgo"
171+
172+
mux.HandleFunc(fmt.Sprintf("/api/v4/projects/%s", gl.PathEscape(projectSlug)), func(w http.ResponseWriter, _ *http.Request) {
173+
w.WriteHeader(http.StatusInternalServerError)
174+
fmt.Fprint(w, `{"message": "Internal Server Error"}`)
175+
})
176+
177+
server := httptest.NewServer(mux)
178+
remotePipelineURL := fmt.Sprintf("%s/%s/-/raw/main/task.yaml", server.URL, projectSlug)
179+
return server, server.URL, remotePipelineURL
180+
},
181+
wantFound: false,
182+
wantErr: true,
183+
wantErrContains: "failed to get project ID",
184+
},
185+
{
186+
name: "API error on GetRawFile",
187+
setup: func(_ *testing.T) (*httptest.Server, string, string) {
188+
mux := http.NewServeMux()
189+
projectID := 12345
190+
projectSlug := "chmouel/dazgo"
191+
filePath := "task.yaml"
192+
193+
mux.HandleFunc(fmt.Sprintf("/api/v4/projects/%s", gl.PathEscape(projectSlug)), func(w http.ResponseWriter, _ *http.Request) {
194+
fmt.Fprintf(w, `{"id": %d, "path_with_namespace": "%s"}`, projectID, projectSlug)
195+
})
196+
197+
mux.HandleFunc(fmt.Sprintf("/api/v4/projects/%d/repository/files/%s/raw", projectID, gl.PathEscape(filePath)), func(w http.ResponseWriter, _ *http.Request) {
198+
w.WriteHeader(http.StatusInternalServerError)
199+
fmt.Fprint(w, `{"message": "Internal Server Error"}`)
200+
})
201+
202+
server := httptest.NewServer(mux)
203+
remotePipelineURL := fmt.Sprintf("%s/%s/-/raw/main/%s", server.URL, projectSlug, filePath)
204+
return server, server.URL, remotePipelineURL
205+
},
206+
wantFound: false,
207+
wantErr: true,
208+
wantErrContains: "failed to get file",
209+
},
210+
}
211+
212+
for _, tt := range tests {
213+
t.Run(tt.name, func(t *testing.T) {
214+
server, eventURL, remotePipelineURL := tt.setup(t)
215+
if server != nil {
216+
defer server.Close()
217+
}
218+
219+
v := &Provider{}
220+
event := info.NewEvent()
221+
event.URL = eventURL
222+
event.Provider = &info.Provider{
223+
Token: "token",
224+
}
225+
226+
found, content, err := v.GetTaskURI(ctx, event, remotePipelineURL)
227+
228+
if tt.wantErr {
229+
assert.Assert(t, err != nil, "expected error but got nil")
230+
if tt.wantErrContains != "" {
231+
assert.ErrorContains(t, err, tt.wantErrContains)
232+
}
233+
} else {
234+
assert.NilError(t, err)
235+
}
236+
237+
assert.Equal(t, tt.wantFound, found)
238+
assert.Equal(t, tt.wantContent, content)
239+
})
240+
}
241+
}

0 commit comments

Comments
 (0)