From 5dfe71c02ec50b11290b18b2f61f2cc881599af0 Mon Sep 17 00:00:00 2001 From: Alan-pad <46325799+Alan-pad@users.noreply.github.com> Date: Mon, 13 Nov 2023 18:09:08 +0100 Subject: [PATCH] fix(url): normalizeURL wasn't working on https with .git suffix (#176) --- .../terraformpullrequest/github/provider.go | 16 ++------- .../terraformpullrequest/gitlab/provider.go | 15 ++------ internal/utils/url/url.go | 34 +++++++++++++++++++ internal/utils/url/url_test.go | 25 ++++++++++++++ internal/webhook/event/common.go | 17 ---------- internal/webhook/event/pullrequest.go | 5 +-- internal/webhook/event/push.go | 5 +-- internal/webhook/github/provider.go | 6 ++-- internal/webhook/gitlab/provider.go | 5 +-- 9 files changed, 76 insertions(+), 52 deletions(-) create mode 100644 internal/utils/url/url.go create mode 100644 internal/utils/url/url_test.go diff --git a/internal/controllers/terraformpullrequest/github/provider.go b/internal/controllers/terraformpullrequest/github/provider.go index 121fd2da..9a6c8847 100644 --- a/internal/controllers/terraformpullrequest/github/provider.go +++ b/internal/controllers/terraformpullrequest/github/provider.go @@ -3,7 +3,6 @@ package github import ( "context" "errors" - "fmt" "strconv" "strings" @@ -12,6 +11,7 @@ import ( "github.com/padok-team/burrito/internal/annotations" "github.com/padok-team/burrito/internal/burrito/config" "github.com/padok-team/burrito/internal/controllers/terraformpullrequest/comment" + utils "github.com/padok-team/burrito/internal/utils/url" log "github.com/sirupsen/logrus" "golang.org/x/oauth2" ) @@ -92,21 +92,9 @@ func (g *Github) Comment(repository *configv1alpha1.TerraformRepository, pr *con } func parseGithubUrl(url string) (string, string) { - normalizedUrl := normalizeUrl(url) + normalizedUrl := utils.NormalizeUrl(url) // nomalized url are "https://padok.github.com/owner/repo" // we remove "https://" then split on "/" split := strings.Split(normalizedUrl[8:], "/") return split[1], split[2] } - -func normalizeUrl(url string) string { - if strings.Contains(url, "https://") { - return url - } - // All SSH URL from GitHub are like "git@padok.github.com:/.git" - // We split on ":" then remove ".git" by removing the last characters - // To handle enterprise GitHub, we dynamically get "padok.github.com" - // By removing "git@" at the beginning of the string - split := strings.Split(url, ":") - return fmt.Sprintf("https://%s/%s", split[0][4:], split[1][:len(split[1])-4]) -} diff --git a/internal/controllers/terraformpullrequest/gitlab/provider.go b/internal/controllers/terraformpullrequest/gitlab/provider.go index 57b6e20b..92dcc682 100644 --- a/internal/controllers/terraformpullrequest/gitlab/provider.go +++ b/internal/controllers/terraformpullrequest/gitlab/provider.go @@ -9,6 +9,7 @@ import ( "github.com/padok-team/burrito/internal/annotations" "github.com/padok-team/burrito/internal/burrito/config" "github.com/padok-team/burrito/internal/controllers/terraformpullrequest/comment" + utils "github.com/padok-team/burrito/internal/utils/url" log "github.com/sirupsen/logrus" "github.com/xanzy/go-gitlab" ) @@ -86,18 +87,6 @@ func (g *Gitlab) Comment(repository *configv1alpha1.TerraformRepository, pr *con } func getGitlabNamespacedName(url string) string { - normalizedUrl := normalizeUrl(url) + normalizedUrl := utils.NormalizeUrl(url) return strings.Join(strings.Split(normalizedUrl[8:], "/")[1:], "/") } - -func normalizeUrl(url string) string { - if strings.Contains(url, "https://") { - return url - } - // All SSH URL from GitLab are like "git@:/.git" - // We split on ":" then remove ".git" by removing the last characters - // To handle enterprise GitLab on premise, we dynamically get "padok.gitlab.com" - // By removing "git@" at the beginning of the string - split := strings.Split(url, ":") - return fmt.Sprintf("https://%s/%s", split[0][4:], split[1][:len(split[1])-4]) -} diff --git a/internal/utils/url/url.go b/internal/utils/url/url.go new file mode 100644 index 00000000..4c1957a5 --- /dev/null +++ b/internal/utils/url/url.go @@ -0,0 +1,34 @@ +package url + +import ( + "fmt" + "strings" +) + +// Normalize a Github/Gitlab URL (SSH or HTTPS) to a HTTPS URL +func NormalizeUrl(url string) string { + if strings.Contains(url, "https://") { + return removeGitExtension(url) + } + if strings.Contains(url, "http://") { + return removeGitExtension("https://" + url[7:]) + } + // All SSH URL from GitHub are like "git@padok.github.com:/.git" + // We split on ":" then remove ".git" by removing the last characters + // To handle enterprise GitHub, we dynamically get "padok.github.com" + // By removing "git@" at the beginning of the string + server, repo := splitSSHUrl(url) + return fmt.Sprintf("https://%s/%s", server, repo) +} + +func splitSSHUrl(url string) (server string, repo string) { + split := strings.Split(url, ":") + return split[0][4:], removeGitExtension(split[1]) +} + +func removeGitExtension(url string) string { + if strings.HasSuffix(url, ".git") { + return url[:len(url)-4] + } + return url +} diff --git a/internal/utils/url/url_test.go b/internal/utils/url/url_test.go new file mode 100644 index 00000000..126a05fb --- /dev/null +++ b/internal/utils/url/url_test.go @@ -0,0 +1,25 @@ +package url_test + +import ( + "testing" + + "github.com/padok-team/burrito/internal/utils/url" +) + +func TestNormalizeURL(t *testing.T) { + urlTypes := []string{ + "git@github.com:padok-team/burrito.git", + "git@github.com:padok-team/burrito", + "https://github.com/padok-team/burrito.git", + "https://github.com/padok-team/burrito", + "http://github.com/padok-team/burrito.git", + "http://github.com/padok-team/burrito", + } + expected := "https://github.com/padok-team/burrito" + for _, u := range urlTypes { + normalized := url.NormalizeUrl(u) + if normalized != expected { + t.Errorf("Passed: %s, Expected %s, got %s", u, expected, normalized) + } + } +} diff --git a/internal/webhook/event/common.go b/internal/webhook/event/common.go index f13ddda3..9696ee68 100644 --- a/internal/webhook/event/common.go +++ b/internal/webhook/event/common.go @@ -1,7 +1,6 @@ package event import ( - "fmt" "strings" configv1alpha1 "github.com/padok-team/burrito/api/v1alpha1" @@ -20,22 +19,6 @@ type Event interface { Handle(client.Client) error } -// Normalize a Github/Gitlab URL (SSH or HTTPS) to a HTTPS URL -func NormalizeUrl(url string) string { - if strings.Contains(url, "https://") { - return url - } - if strings.Contains(url, "http://") { - return "https://" + url[7:] - } - // All SSH URL from GitHub are like "git@padok.github.com:/.git" - // We split on ":" then remove ".git" by removing the last characters - // To handle enterprise GitHub, we dynamically get "padok.github.com" - // By removing "git@" at the beginning of the string - split := strings.Split(url, ":") - return fmt.Sprintf("https://%s/%s", split[0][4:], split[1][:len(split[1])-4]) -} - func ParseRevision(ref string) string { refParts := strings.SplitN(ref, "/", 3) return refParts[len(refParts)-1] diff --git a/internal/webhook/event/pullrequest.go b/internal/webhook/event/pullrequest.go index 58804c84..6e4b5e23 100644 --- a/internal/webhook/event/pullrequest.go +++ b/internal/webhook/event/pullrequest.go @@ -4,6 +4,7 @@ import ( "context" "fmt" + utils "github.com/padok-team/burrito/internal/utils/url" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" configv1alpha1 "github.com/padok-team/burrito/api/v1alpha1" @@ -104,8 +105,8 @@ func (e *PullRequestEvent) getAffectedRepositories(repositories []configv1alpha1 affectedRepositories := []configv1alpha1.TerraformRepository{} for _, repo := range repositories { log.Infof("evaluating terraform repository %s for url %s", repo.Name, repo.Spec.Repository.Url) - log.Infof("comparing normalized url %s with received URL from payload %s", NormalizeUrl(repo.Spec.Repository.Url), e.URL) - if e.URL == NormalizeUrl(repo.Spec.Repository.Url) { + log.Infof("comparing normalized url %s with received URL from payload %s", utils.NormalizeUrl(repo.Spec.Repository.Url), e.URL) + if e.URL == utils.NormalizeUrl(repo.Spec.Repository.Url) { affectedRepositories = append(affectedRepositories, repo) } } diff --git a/internal/webhook/event/push.go b/internal/webhook/event/push.go index 4f7e9216..4245272a 100644 --- a/internal/webhook/event/push.go +++ b/internal/webhook/event/push.go @@ -6,7 +6,9 @@ import ( configv1alpha1 "github.com/padok-team/burrito/api/v1alpha1" "github.com/padok-team/burrito/internal/annotations" controller "github.com/padok-team/burrito/internal/controllers/terraformlayer" + utils "github.com/padok-team/burrito/internal/utils/url" log "github.com/sirupsen/logrus" + "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -84,8 +86,7 @@ func (e *PushEvent) getAffectedRepositories(repositories []configv1alpha1.Terraf affectedRepositories := []configv1alpha1.TerraformRepository{} log.Infof("looking for affected repositories, event url: %s", e.URL) for _, repo := range repositories { - if e.URL == NormalizeUrl(repo.Spec.Repository.Url) { - log.Infof("repository %s is affected by push event", repo.Name) + if e.URL == utils.NormalizeUrl(repo.Spec.Repository.Url) { affectedRepositories = append(affectedRepositories, repo) continue } diff --git a/internal/webhook/github/provider.go b/internal/webhook/github/provider.go index 1c7ec2af..63b3d74b 100644 --- a/internal/webhook/github/provider.go +++ b/internal/webhook/github/provider.go @@ -5,6 +5,8 @@ import ( "net/http" "strconv" + utils "github.com/padok-team/burrito/internal/utils/url" + "github.com/go-playground/webhooks/github" "github.com/padok-team/burrito/internal/burrito/config" "github.com/padok-team/burrito/internal/webhook/event" @@ -51,7 +53,7 @@ func (g *Github) GetEvent(r *http.Request) (event.Event, error) { changedFiles = append(changedFiles, commit.Removed...) } e = &event.PushEvent{ - URL: event.NormalizeUrl(payload.Repository.HTMLURL), + URL: utils.NormalizeUrl(payload.Repository.HTMLURL), Revision: event.ParseRevision(payload.Ref), ChangeInfo: event.ChangeInfo{ ShaBefore: payload.Before, @@ -68,7 +70,7 @@ func (g *Github) GetEvent(r *http.Request) (event.Event, error) { e = &event.PullRequestEvent{ Provider: "github", ID: strconv.FormatInt(payload.PullRequest.Number, 10), - URL: event.NormalizeUrl(payload.Repository.HTMLURL), + URL: utils.NormalizeUrl(payload.Repository.HTMLURL), Revision: payload.PullRequest.Head.Ref, Action: getNormalizedAction(payload.Action), Base: payload.PullRequest.Base.Ref, diff --git a/internal/webhook/gitlab/provider.go b/internal/webhook/gitlab/provider.go index 9004628d..025d02dc 100644 --- a/internal/webhook/gitlab/provider.go +++ b/internal/webhook/gitlab/provider.go @@ -7,6 +7,7 @@ import ( "github.com/go-playground/webhooks/gitlab" "github.com/padok-team/burrito/internal/burrito/config" + utils "github.com/padok-team/burrito/internal/utils/url" "github.com/padok-team/burrito/internal/webhook/event" log "github.com/sirupsen/logrus" ) @@ -49,7 +50,7 @@ func (g *Gitlab) GetEvent(r *http.Request) (event.Event, error) { changedFiles = append(changedFiles, commit.Removed...) } e = &event.PushEvent{ - URL: event.NormalizeUrl(payload.Project.WebURL), + URL: utils.NormalizeUrl(payload.Project.WebURL), Revision: event.ParseRevision(payload.Ref), ChangeInfo: event.ChangeInfo{ ShaBefore: payload.Before, @@ -62,7 +63,7 @@ func (g *Gitlab) GetEvent(r *http.Request) (event.Event, error) { e = &event.PullRequestEvent{ Provider: "gitlab", ID: strconv.Itoa(int(payload.ObjectAttributes.IID)), - URL: event.NormalizeUrl(payload.Project.WebURL), + URL: utils.NormalizeUrl(payload.Project.WebURL), Revision: payload.ObjectAttributes.SourceBranch, Action: getNormalizedAction(payload.ObjectAttributes.Action), Base: payload.ObjectAttributes.TargetBranch,