Skip to content

RHTAP-1134: Support custom Gitlab instances #240

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.

Sign up for GitHub

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

Merged
chmeliik merged 1 commit into from
Jan 8, 2024
Merged

RHTAP-1134: Support custom Gitlab instances #240

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 3 additions & 3 deletions pkg/git/gitlab/gitlab_client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import (
)

// Allow mocking for tests
var NewGitlabClient func(accessToken string) (*GitlabClient, error) = newGitlabClient
var NewGitlabClient func(accessToken, baseUrl string) (*GitlabClient, error) = newGitlabClient

var _ gp.GitProviderClient = (*GitlabClient)(nil)

Expand Down Expand Up @@ -330,9 +330,9 @@ func (g *GitlabClient) GetConfiguredGitAppName() (string, string, error) {
return "", "", fmt.Errorf("GitLab does not support applications")
}

func newGitlabClient(accessToken string) (*GitlabClient, error) {
func newGitlabClient(accessToken, baseUrl string) (*GitlabClient, error) {
glc := &GitlabClient{}
c, err := gitlab.NewClient(accessToken)
c, err := gitlab.NewClient(accessToken, gitlab.WithBaseURL(baseUrl))
if err != nil {
return nil, err
}
Expand Down
46 changes: 46 additions & 0 deletions pkg/git/gitlab/gitlab_helper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,31 @@ import (
"net/url"
)

type FailedToParseUrlError struct {
url string
err string
}

func (e FailedToParseUrlError) Error() string {
return fmt.Sprintf("Failed to parse url: %s, error: %s", e.url, e.err)
}

type MissingSchemaError struct {
url string
}

func (e MissingSchemaError) Error() string {
return fmt.Sprintf("Failed to detect schema in url %s", e.url)
}

type MissingHostError struct {
url string
}

func (e MissingHostError) Error() string {
return fmt.Sprintf("Failed to detect host in url %s", e.url)
}

func getProjectPathFromRepoUrl(repoUrl string) (string, error) {
url, err := url.Parse(repoUrl)
if err != nil {
Expand All @@ -42,6 +67,24 @@ func getProjectPathFromRepoUrl(repoUrl string) (string, error) {
), nil
}

func GetBaseUrl(repoUrl string) (string, error) {
url, err := url.Parse(repoUrl)
if err != nil {
return "", FailedToParseUrlError{url: repoUrl, err: err.Error()}
Copy link
Copy Markdown
Member

@mmorhun mmorhun Jan 5, 2024

Choose a reason for hiding this comment

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

Custom errors looks redundant here: they are never used / distinguished.

Copy link
Copy Markdown
Member Author

@gbenhaim gbenhaim Jan 7, 2024

Choose a reason for hiding this comment

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

they are used in the tests :)

}

if url.Scheme == "" {
return "", MissingSchemaError{repoUrl}
}

if url.Host == "" {
return "", MissingHostError{repoUrl}
}

// The gitlab client library expects the base url to have a tailing slash
return fmt.Sprintf("%s://%s/", url.Scheme, url.Host), nil
}

// refineGitHostingServiceError generates expected permanent error from GitHub response.
// If no one is detected, the original error will be returned.
// refineGitHostingServiceError should be called just after every GitHub API call.
Expand Down Expand Up @@ -280,6 +323,9 @@ func (g *GitlabClient) getWebhookByTargetUrl(projectPath, webhookTargetUrl strin
opts := &gitlab.ListProjectHooksOptions{PerPage: 100}
webhooks, resp, err := g.client.Projects.ListProjectHooks(projectPath, opts)
if err != nil {
if resp == nil {
return nil, err
}
return nil, refineGitHostingServiceError(resp.Response, err)
}
for _, webhook := range webhooks {
Expand Down
35 changes: 30 additions & 5 deletions pkg/git/gitlab/gitlab_helper_debug_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,12 @@ func TestEnsurePaCMergeRequest(t *testing.T) {
return
}

glclient, err := NewGitlabClient(accessToken)
baseUrl, err := GetBaseUrl(repoUrl)
if err != nil {
t.Fatal(err)
}

glclient, err := NewGitlabClient(accessToken, baseUrl)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -75,7 +80,12 @@ func TestUndoPaCMergeRequest(t *testing.T) {
return
}

glclient, err := NewGitlabClient(accessToken)
baseUrl, err := GetBaseUrl(repoUrl)
if err != nil {
t.Fatal(err)
}

glclient, err := NewGitlabClient(accessToken, baseUrl)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -112,7 +122,12 @@ func TestSetupPaCWebhook(t *testing.T) {
targetWebhookUrl := "https://pac.route.my-cluster.net"
webhookSecretString := "d01b38971dad59514298d763f288392c08221043"

glclient, err := NewGitlabClient(accessToken)
baseUrl, err := GetBaseUrl(repoUrl)
if err != nil {
t.Fatal(err)
}

glclient, err := NewGitlabClient(accessToken, baseUrl)
if err != nil {
t.Fatal(err)
}
Expand All @@ -130,7 +145,12 @@ func TestDeletePaCWebhook(t *testing.T) {

targetWebhookUrl := "https://pac.route.my-cluster.net"

glclient, err := NewGitlabClient(accessToken)
baseUrl, err := GetBaseUrl(repoUrl)
if err != nil {
t.Fatal(err)
}

glclient, err := NewGitlabClient(accessToken, baseUrl)
if err != nil {
t.Fatal(err)
}
Expand All @@ -146,7 +166,12 @@ func TestIsRepositoryPublic(t *testing.T) {
return
}

glclient, err := NewGitlabClient(accessToken)
baseUrl, err := GetBaseUrl(repoUrl)
if err != nil {
t.Fatal(err)
}

glclient, err := NewGitlabClient(accessToken, baseUrl)
if err != nil {
t.Fatal(err)
}
Expand Down
115 changes: 115 additions & 0 deletions pkg/git/gitlab/gitlab_helper_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,3 +56,118 @@ func TestGetProjectPathFromRepoUrl(t *testing.T) {
}
}
}

func TestGetHostFromRepoUrl(t *testing.T) {
var params = []struct {
in string
out string
err error
}{
{
"https://gitlab.host.com/rhtap/tenants-config.git",
"https://gitlab.host.com/",
nil,
},
{
"https://gitlab.com/projectname",
"https://gitlab.com/",
nil,
},
{
"http!!://abc",
"",
FailedToParseUrlError{},
},
}

for _, tt := range params {
t.Run(tt.in, func(t *testing.T) {
actual, err := GetBaseUrl(tt.in)
if err != nil && tt.err == nil {
t.Fatalf("Expected call to succeed, found error %s", err.Error())
}
if err == nil && tt.err != nil {
t.Fatalf("Expected call to end with an error, but it succeed")
}
if actual != tt.out {
t.Fatalf("Expected %s found %s", tt.out, actual)
}
})
}
}

func TestGetHostFromRepoUrlFailToParseUrl(t *testing.T) {
var params = []struct {
in string
out string
}{
{
"http!!://abc",
"",
},
}

for _, tt := range params {
t.Run(tt.in, func(t *testing.T) {
actual, err := GetBaseUrl(tt.in)
if actual != "" {
t.Fatalf("Expected an empty string result")
}
if _, ok := err.(FailedToParseUrlError); !ok {
t.Fatalf("Expected to received error of type FailedToParseUrlError got %T", err)
}
})
}
}

func TestGetHostFromRepoUrlMissingSchema(t *testing.T) {
var params = []struct {
in string
out string
}{
{
"gitlab.com",
"",
},
{
"gitlab.cee.redhat.com",
"",
},
}

for _, tt := range params {
t.Run(tt.in, func(t *testing.T) {
actual, err := GetBaseUrl(tt.in)
if actual != "" {
t.Fatalf("Expected an empty string result")
}
if _, ok := err.(MissingSchemaError); !ok {
t.Fatalf("Expected to received error of type MissingSchemaError got %T", err)
}
})
}
}

func TestGetHostFromRepoUrlMissingHost(t *testing.T) {
var params = []struct {
in string
out string
}{
{
"http:///abc",
"",
},
}

for _, tt := range params {
t.Run(tt.in, func(t *testing.T) {
actual, err := GetBaseUrl(tt.in)
if actual != "" {
t.Fatalf("Expected an empty string result")
}
if _, ok := err.(MissingHostError); !ok {
t.Fatalf("Expected to received error of type MissingHostError got %T", err)
}
})
}
}
6 changes: 5 additions & 1 deletion pkg/git/gitproviderfactory/gitproviderfactory.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,11 @@ func createGitClient(gitClientConfig GitClientConfig) (gitprovider.GitProviderCl
if isAppUsed {
return nil, fmt.Errorf("GitLab does not have applications")
}
return gitlab.NewGitlabClient(accessToken)
baseUrl, err := gitlab.GetBaseUrl(gitClientConfig.RepoUrl)
if err != nil {
return nil, err
}
return gitlab.NewGitlabClient(accessToken, baseUrl)

case "bitbucket":
return nil, boerrors.NewBuildOpError(boerrors.EUnknownGitProvider, fmt.Errorf("git provider %s is not supported", gitProvider))
Expand Down
23 changes: 20 additions & 3 deletions pkg/git/gitproviderfactory/gitproviderfactory_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func TestGetContainerImageRepository(t *testing.T) {
t.Errorf("should not be invoked")
return nil
}
gitlab.NewGitlabClient = func(accessToken string) (*gitlab.GitlabClient, error) {
gitlab.NewGitlabClient = func(accessToken, baseUrl string) (*gitlab.GitlabClient, error) {
t.Errorf("should not be invoked")
return nil, nil
}
Expand Down Expand Up @@ -213,16 +213,33 @@ func TestGetContainerImageRepository(t *testing.T) {
"gitlab_token": []byte("token"),
},
GitProvider: "gitlab",
RepoUrl: repoUrl,
RepoUrl: "https://gitlab.com/my-org/my-repo",
IsAppInstallationExpected: true,
},
allowConstructors: func() {
gitlab.NewGitlabClient = func(accessToken string) (*gitlab.GitlabClient, error) {
gitlab.NewGitlabClient = func(accessToken, baseUrl string) (*gitlab.GitlabClient, error) {
expectedBaseUrl := "https://gitlab.com/"
if baseUrl != expectedBaseUrl {
return nil, fmt.Errorf("Expected to get baseUrl: %s, got %s", expectedBaseUrl, baseUrl)
}
return &gitlab.GitlabClient{}, nil
}
},
expectError: false,
},
{
name: "should fail to create Gitlab client since the base url can't be detected",
gitClientConfig: GitClientConfig{
PacSecretData: map[string][]byte{
"gitlab_token": []byte("token"),
},
GitProvider: "gitlab",
RepoUrl: "https://",
IsAppInstallationExpected: false,
},
allowConstructors: func() {},
expectError: true,
},
{
name: "should not create BitBucket client",
gitClientConfig: GitClientConfig{
Expand Down