From a462ecbe7956f1bd4edd838cdf476dba35b49667 Mon Sep 17 00:00:00 2001
From: Tingluo Huang <tingluohuang@github.com>
Date: Thu, 9 Mar 2023 09:02:05 -0500
Subject: [PATCH] Trim slash for configure URL. (#2381)

---
 .../templates/autoscalingrunnerset.yaml       |  2 +-
 .../tests/template_test.go                    | 28 ++++++++++++
 github/actions/config.go                      |  4 +-
 github/actions/config_test.go                 | 43 ++++++++++++++++++-
 4 files changed, 73 insertions(+), 4 deletions(-)

diff --git a/charts/gha-runner-scale-set/templates/autoscalingrunnerset.yaml b/charts/gha-runner-scale-set/templates/autoscalingrunnerset.yaml
index 6ad3c6c8..e7f66156 100644
--- a/charts/gha-runner-scale-set/templates/autoscalingrunnerset.yaml
+++ b/charts/gha-runner-scale-set/templates/autoscalingrunnerset.yaml
@@ -12,7 +12,7 @@ metadata:
   labels:
     {{- include "gha-runner-scale-set.labels" . | nindent 4 }}
 spec:
-  githubConfigUrl: {{ required ".Values.githubConfigUrl is required" .Values.githubConfigUrl }}
+  githubConfigUrl: {{ required ".Values.githubConfigUrl is required" (trimSuffix "/" .Values.githubConfigUrl) }}
   githubConfigSecret: {{ include "gha-runner-scale-set.githubsecret" . }}
   {{- with .Values.runnerGroup }}
   runnerGroup: {{ . }}
diff --git a/charts/gha-runner-scale-set/tests/template_test.go b/charts/gha-runner-scale-set/tests/template_test.go
index e6995dff..01b522c6 100644
--- a/charts/gha-runner-scale-set/tests/template_test.go
+++ b/charts/gha-runner-scale-set/tests/template_test.go
@@ -869,3 +869,31 @@ func TestTemplateNamingConstraints(t *testing.T) {
 		})
 	}
 }
+
+func TestTemplateRenderedGitHubConfigUrlEndsWIthSlash(t *testing.T) {
+	t.Parallel()
+
+	// Path to the helm chart we will test
+	helmChartPath, err := filepath.Abs("../../gha-runner-scale-set")
+	require.NoError(t, err)
+
+	releaseName := "test-runners"
+	namespaceName := "test-" + strings.ToLower(random.UniqueId())
+
+	options := &helm.Options{
+		SetValues: map[string]string{
+			"githubConfigUrl":                 "https://github.com/actions/",
+			"githubConfigSecret.github_token": "gh_token12345",
+		},
+		KubectlOptions: k8s.NewKubectlOptions("", "", namespaceName),
+	}
+
+	output := helm.RenderTemplate(t, options, helmChartPath, releaseName, []string{"templates/autoscalingrunnerset.yaml"})
+
+	var ars v1alpha1.AutoscalingRunnerSet
+	helm.UnmarshalK8SYaml(t, output, &ars)
+
+	assert.Equal(t, namespaceName, ars.Namespace)
+	assert.Equal(t, "test-runners", ars.Name)
+	assert.Equal(t, "https://github.com/actions", ars.Spec.GitHubConfigUrl)
+}
diff --git a/github/actions/config.go b/github/actions/config.go
index 204fa0a4..f19cb17f 100644
--- a/github/actions/config.go
+++ b/github/actions/config.go
@@ -29,7 +29,7 @@ type GitHubConfig struct {
 }
 
 func ParseGitHubConfigFromURL(in string) (*GitHubConfig, error) {
-	u, err := url.Parse(in)
+	u, err := url.Parse(strings.Trim(in, "/"))
 	if err != nil {
 		return nil, err
 	}
@@ -45,7 +45,7 @@ func ParseGitHubConfigFromURL(in string) (*GitHubConfig, error) {
 
 	invalidURLError := fmt.Errorf("%q: %w", u.String(), ErrInvalidGitHubConfigURL)
 
-	pathParts := strings.Split(strings.TrimPrefix(u.Path, "/"), "/")
+	pathParts := strings.Split(strings.Trim(u.Path, "/"), "/")
 
 	switch len(pathParts) {
 	case 1: // Organization
diff --git a/github/actions/config_test.go b/github/actions/config_test.go
index e64928e2..e21f7e9e 100644
--- a/github/actions/config_test.go
+++ b/github/actions/config_test.go
@@ -3,6 +3,7 @@ package actions_test
 import (
 	"errors"
 	"net/url"
+	"strings"
 	"testing"
 
 	"github.com/actions/actions-runner-controller/github/actions"
@@ -26,6 +27,16 @@ func TestGitHubConfig(t *testing.T) {
 					IsHosted:     true,
 				},
 			},
+			{
+				configURL: "https://github.com/org/repo/",
+				expected: &actions.GitHubConfig{
+					Scope:        actions.GitHubScopeRepository,
+					Enterprise:   "",
+					Organization: "org",
+					Repository:   "repo",
+					IsHosted:     true,
+				},
+			},
 			{
 				configURL: "https://github.com/org",
 				expected: &actions.GitHubConfig{
@@ -46,6 +57,16 @@ func TestGitHubConfig(t *testing.T) {
 					IsHosted:     true,
 				},
 			},
+			{
+				configURL: "https://github.com/enterprises/my-enterprise/",
+				expected: &actions.GitHubConfig{
+					Scope:        actions.GitHubScopeEnterprise,
+					Enterprise:   "my-enterprise",
+					Organization: "",
+					Repository:   "",
+					IsHosted:     true,
+				},
+			},
 			{
 				configURL: "https://www.github.com/org",
 				expected: &actions.GitHubConfig{
@@ -56,6 +77,16 @@ func TestGitHubConfig(t *testing.T) {
 					IsHosted:     true,
 				},
 			},
+			{
+				configURL: "https://www.github.com/org/",
+				expected: &actions.GitHubConfig{
+					Scope:        actions.GitHubScopeOrganization,
+					Enterprise:   "",
+					Organization: "org",
+					Repository:   "",
+					IsHosted:     true,
+				},
+			},
 			{
 				configURL: "https://github.localhost/org",
 				expected: &actions.GitHubConfig{
@@ -76,11 +107,21 @@ func TestGitHubConfig(t *testing.T) {
 					IsHosted:     false,
 				},
 			},
+			{
+				configURL: "https://my-ghes.com/org/",
+				expected: &actions.GitHubConfig{
+					Scope:        actions.GitHubScopeOrganization,
+					Enterprise:   "",
+					Organization: "org",
+					Repository:   "",
+					IsHosted:     false,
+				},
+			},
 		}
 
 		for _, test := range tests {
 			t.Run(test.configURL, func(t *testing.T) {
-				parsedURL, err := url.Parse(test.configURL)
+				parsedURL, err := url.Parse(strings.Trim(test.configURL, "/"))
 				require.NoError(t, err)
 				test.expected.ConfigURL = parsedURL