fix(url): normalizeURL wasn't working on https with .git suffix (#176)

padok-team · Nov 13, 2023 · 5dfe71c · 5dfe71c
1 parent dec34ef
commit 5dfe71c
Show file tree

Hide file tree

Showing 9 changed files with 76 additions and 52 deletions.
diff --git 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:<owner>/<repo>.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
@@ -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@<URL>:<owner>/<repo>.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
@@ -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:<owner>/<repo>.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
@@ -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
@@ -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:<owner>/<repo>.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
@@ -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
@@ -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
@@ -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
@@ -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,