Skip to content

Commit 531449f

Browse files
enekofbenekofb
authored
bootstrapping normalises ssh urls without scheme (#3712)
* support ssh urls with scheme (shorter scp-like) * acceptance test using ssh short url * update docs * review suggestion
1 parent 1172c1d commit 531449f

File tree

6 files changed

+131
-52
lines changed

6 files changed

+131
-52
lines changed

‎cmd/gitops/app/bootstrap/cmd_acceptance_test.go

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -73,41 +73,51 @@ func TestBootstrapCmd(t *testing.T) {
7373
g.SetDefaultEventuallyPollingInterval(defaultInterval)
7474
testLog := testr.New(t)
7575

76+
// ssh git repo configuration
7677
privateKeyFile := os.Getenv("GIT_PRIVATEKEY_PATH")
7778
g.Expect(privateKeyFile).NotTo(BeEmpty())
7879

80+
privateKeyPassword := os.Getenv("GIT_PRIVATEKEY_PASSWORD")
81+
g.Expect(privateKeyPassword).NotTo(BeEmpty())
82+
7983
repoURLSSH := os.Getenv("GIT_REPO_URL_SSH")
8084
g.Expect(repoURLSSH).NotTo(BeEmpty())
85+
86+
repoURLSSHNoScheme := os.Getenv("GIT_REPO_URL_SSH_NO_SCHEME")
87+
g.Expect(repoURLSSHNoScheme).NotTo(BeEmpty())
88+
89+
privateKeyFlag := fmt.Sprintf("--private-key=%s", privateKeyFile)
90+
privateKeyPasswordFlag := fmt.Sprintf("--private-key-password=%s", privateKeyPassword)
91+
gitRepoUrlSshNoSchemeFlag := fmt.Sprintf("--repo-url=%s", repoURLSSHNoScheme)
92+
93+
// https git repo configuration
8194
repoURLHTTPS := os.Getenv("GIT_REPO_URL_HTTPS")
8295
g.Expect(repoURLHTTPS).NotTo(BeEmpty())
96+
8397
gitUsername := os.Getenv("GIT_USERNAME")
8498
g.Expect(gitUsername).NotTo(BeEmpty())
99+
85100
gitPassword := os.Getenv("GIT_PASSWORD")
86101
g.Expect(gitPassword).NotTo(BeEmpty())
102+
103+
// git repo configuration
87104
gitBranch := os.Getenv("GIT_BRANCH")
88105
g.Expect(gitBranch).NotTo(BeEmpty())
106+
89107
gitRepoPath := os.Getenv("GIT_REPO_PATH")
90108
g.Expect(gitRepoPath).NotTo(BeEmpty())
91-
privateKeyPassword := os.Getenv("GIT_PRIVATEKEY_PASSWORD")
92-
g.Expect(privateKeyPassword).NotTo(BeEmpty())
93-
oidcClientSecret := os.Getenv("OIDC_CLIENT_SECRET")
94-
g.Expect(oidcClientSecret).NotTo(BeEmpty())
95-
96-
privateKeyFlag := fmt.Sprintf("--private-key=%s", privateKeyFile)
97-
privateKeyPasswordFlag := fmt.Sprintf("--private-key-password=%s", privateKeyPassword)
98-
99-
kubeconfigFlag := fmt.Sprintf("--kubeconfig=%s", kubeconfigPath)
100-
101-
repoHTTPSURLFlag := fmt.Sprintf("--repo-url=%s", repoURLHTTPS)
102-
103-
gitUsernameFlag := fmt.Sprintf("--git-username=%s", gitUsername)
104-
gitPasswordFlag := fmt.Sprintf("--git-password=%s", gitPassword)
105109

106110
gitBranchFlag := fmt.Sprintf("--branch=%s", gitBranch)
107111
gitRepoPathFlag := fmt.Sprintf("--repo-path=%s", gitRepoPath)
108112

113+
// oidc configuration
114+
oidcClientSecret := os.Getenv("OIDC_CLIENT_SECRET")
115+
g.Expect(oidcClientSecret).NotTo(BeEmpty())
116+
109117
oidcClientSecretFlag := fmt.Sprintf("--client-secret=%s", oidcClientSecret)
110118

119+
kubeconfigFlag := fmt.Sprintf("--kubeconfig=%s", kubeconfigPath)
120+
111121
_ = k8sClient.Create(context.Background(), &fluxSystemNamespace)
112122

113123
tests := []struct {
@@ -147,8 +157,8 @@ func TestBootstrapCmd(t *testing.T) {
147157
"--password=admin123",
148158
"--discovery-url=https://dex-01.wge.dev.weave.works/.well-known/openid-configuration",
149159
"--client-id=weave-gitops-enterprise",
150-
gitUsernameFlag, gitPasswordFlag, gitBranchFlag, gitRepoPathFlag,
151-
repoHTTPSURLFlag,
160+
gitRepoUrlSshNoSchemeFlag, gitBranchFlag, gitRepoPathFlag,
161+
privateKeyFlag, privateKeyPasswordFlag,
152162
oidcClientSecretFlag, "-s",
153163
"--components-extra=policy-agent,tf-controller",
154164
"--bootstrap-flux",

‎docs/cli/bootstrap.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -375,6 +375,7 @@ Entitlement stage
375375
- `GIT_PRIVATEKEY_PATH`: path to the private key to do the git operations.
376376
- `GIT_PRIVATEKEY_PASSWORD`: password protecting access to private key
377377
- `GIT_REPO_URL_SSH`: git ssh url for the repo wge configuration repo.
378+
- `GIT_REPO_URL_SSH_NO_SCHEME`: git ssh url for the repo wge configuration repo without scheme like `[email protected]:weaveworks/cli-dev.git`
378379
- `GIT_REPO_URL_HTTPS`: git https url for the repo wge configuration repo.
379380
- `GIT_USERNAME`: git username for testing https auth
380381
- `GIT_PASSWORD`: git password for testing https auth

‎pkg/bootstrap/steps/bootstrap_flux_repo_config_test.go

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ func TestCreateGitRepositoryConfig(t *testing.T) {
8989
},
9090
wantErr: func(t assert.TestingT, err error, i ...interface{}) bool {
9191
assert.Error(t, err)
92-
assert.Contains(t, err.Error(), "unsupported repository scheme: ssl")
92+
assert.Contains(t, err.Error(), "invalid repository scheme")
9393
return true
9494
},
9595
},
@@ -110,13 +110,14 @@ func TestCreateGitRepositoryConfig(t *testing.T) {
110110
},
111111
},
112112
config: &Config{
113-
GitRepository: GitRepositoryConfig{},
114-
},
115-
wantErr: func(t assert.TestingT, err error, i ...interface{}) bool {
116-
assert.Error(t, err)
117-
assert.Contains(t, err.Error(), "repository scheme cannot be empty")
118-
return true
113+
GitRepository: GitRepositoryConfig{
114+
Url: "ssh://[email protected]/my-org-name/my-repo-name",
115+
Path: "test/test",
116+
Branch: "main",
117+
Scheme: sshScheme,
118+
},
119119
},
120+
wantErr: assert.NoError,
120121
},
121122
}
122123
for _, tt := range tests {

‎pkg/bootstrap/steps/bootstrap_git_repo_config.go

Lines changed: 30 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package steps
33
import (
44
"fmt"
55
"net/url"
6+
"strings"
67
)
78

89
const (
@@ -54,27 +55,53 @@ type GitRepositoryConfig struct {
5455
Scheme string
5556
}
5657

57-
// NewGitRepositoryConfig creates new configuration out of the user input and discovered state
58+
// NewGitRepositoryConfig creates new Git repository configuration from valid input parameters.
5859
func NewGitRepositoryConfig(url string, branch string, path string) (GitRepositoryConfig, error) {
5960
var scheme string
6061
var err error
62+
var normalisedUrl string
6163

6264
if url != "" {
63-
scheme, err = parseRepoScheme(url)
65+
normalisedUrl, scheme, err = normaliseUrl(url)
6466
if err != nil {
6567
return GitRepositoryConfig{}, fmt.Errorf("error parsing repo scheme: %v", err)
6668
}
6769
}
6870

6971
return GitRepositoryConfig{
70-
Url: url,
72+
Url: normalisedUrl,
7173
Branch: branch,
7274
Path: path,
7375
Scheme: scheme,
7476
}, nil
7577

7678
}
7779

80+
// normaliseUrl normalises the given url to meet standard URL syntax. The main motivation to have this function
81+
// is to support Git server URLs in "shorter scp-like syntax for the SSH protocol" as described in https://git-scm.com/book/en/v2/Git-on-the-Server-The-Protocols
82+
// and followed by popular Git server providers like GitHub ([email protected]:weaveworks/weave-gitops.git) and GitLab (i.e. [email protected]:gitlab-org/gitlab-foss.git).
83+
// Returns the normalisedUrl, as well the scheme and an error if any.
84+
func normaliseUrl(repoURL string) (normalisedUrl string, scheme string, err error) {
85+
// transform in case of ssh like [email protected]:username/repository.git
86+
if strings.Contains(repoURL, "@") && !strings.Contains(repoURL, "://") {
87+
repoURL = "ssh://" + strings.Replace(repoURL, ":", "/", 1)
88+
}
89+
90+
repositoryURL, err := url.Parse(repoURL)
91+
if err != nil {
92+
return "", "", fmt.Errorf("error parsing repository URL: %v", err)
93+
}
94+
95+
switch repositoryURL.Scheme {
96+
case sshScheme:
97+
return repositoryURL.String(), sshScheme, nil
98+
case httpsScheme:
99+
return repositoryURL.String(), httpsScheme, nil
100+
default:
101+
return "", "", fmt.Errorf("invalid repository scheme: %s", repositoryURL.Scheme)
102+
}
103+
}
104+
78105
// NewGitRepositoryConfig step to configure the flux git repository
79106
func NewGitRepositoryConfigStep(config GitRepositoryConfig) BootstrapStep {
80107
// create steps
@@ -138,22 +165,3 @@ func createGitRepositoryConfig(input []StepInput, c *Config) ([]StepOutput, erro
138165
c.Logger.Actionf("configured repo: %s", c.GitRepository.Url)
139166
return []StepOutput{}, nil
140167
}
141-
142-
func parseRepoScheme(repoURL string) (string, error) {
143-
repositoryURL, err := url.Parse(repoURL)
144-
if err != nil {
145-
return "", fmt.Errorf("incorrect repository url %s:%v", repoURL, err)
146-
}
147-
var scheme string
148-
switch repositoryURL.Scheme {
149-
case "":
150-
return "", fmt.Errorf("repository scheme cannot be empty")
151-
case sshScheme:
152-
scheme = sshScheme
153-
case httpsScheme:
154-
scheme = httpsScheme
155-
default:
156-
return "", fmt.Errorf("unsupported repository scheme: %s", repositoryURL.Scheme)
157-
}
158-
return scheme, nil
159-
}

‎pkg/bootstrap/steps/bootstrap_git_repo_config_test.go

Lines changed: 64 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,12 +42,13 @@ func TestNewGitRepositoryConfig(t *testing.T) {
4242
branch: "main",
4343
path: "clusters/management",
4444
},
45-
want: GitRepositoryConfig{},
46-
wantErr: func(t assert.TestingT, err error, i ...interface{}) bool {
47-
assert.Error(t, err)
48-
assert.Contains(t, err.Error(), "repository scheme cannot be empty")
49-
return true
45+
want: GitRepositoryConfig{
46+
Url: "ssh://[email protected]/example/cli-dev",
47+
Branch: "main",
48+
Path: "clusters/management",
49+
Scheme: sshScheme,
5050
},
51+
wantErr: assert.NoError,
5152
},
5253
{
5354
name: "should create config for valid https url ",
@@ -77,3 +78,61 @@ func TestNewGitRepositoryConfig(t *testing.T) {
7778
})
7879
}
7980
}
81+
82+
func Test_normaliseUrl(t *testing.T) {
83+
tests := []struct {
84+
name string
85+
url string
86+
wantNormalisedUrl string
87+
wantScheme string
88+
wantErr assert.ErrorAssertionFunc
89+
}{
90+
{
91+
name: "should normalise https url with .git",
92+
url: "https://github.com/username/repository.git",
93+
wantNormalisedUrl: "https://github.com/username/repository.git",
94+
wantScheme: httpsScheme,
95+
wantErr: assert.NoError,
96+
},
97+
{
98+
name: "should normalise https url",
99+
url: "https://github.com/username/repository",
100+
wantNormalisedUrl: "https://github.com/username/repository",
101+
wantScheme: httpsScheme,
102+
wantErr: assert.NoError,
103+
},
104+
{
105+
name: "should normalise ssh url without scheme",
106+
url: "[email protected]:weaveworks/weave-gitops.git",
107+
wantNormalisedUrl: "ssh://[email protected]/weaveworks/weave-gitops.git",
108+
wantScheme: sshScheme,
109+
wantErr: assert.NoError,
110+
},
111+
{
112+
name: "should normalise ssh url with scheme",
113+
url: "ssh://[email protected]/username/repository.git",
114+
wantNormalisedUrl: "ssh://[email protected]/username/repository.git",
115+
wantScheme: sshScheme,
116+
wantErr: assert.NoError,
117+
},
118+
{
119+
name: "should fail for invalid url",
120+
url: "invalid_url",
121+
wantErr: func(t assert.TestingT, err error, i ...interface{}) bool {
122+
assert.Error(t, err)
123+
assert.Contains(t, err.Error(), "invalid repository scheme")
124+
return true
125+
},
126+
},
127+
}
128+
for _, tt := range tests {
129+
t.Run(tt.name, func(t *testing.T) {
130+
gotNormalisedUrl, gotScheme, err := normaliseUrl(tt.url)
131+
if !tt.wantErr(t, err, fmt.Sprintf("normaliseUrl(%v)", tt.url)) {
132+
return
133+
}
134+
assert.Equalf(t, tt.wantNormalisedUrl, gotNormalisedUrl, "normaliseUrl(%v)", tt.url)
135+
assert.Equalf(t, tt.wantScheme, gotScheme, "normaliseUrl(%v)", tt.url)
136+
})
137+
}
138+
}

‎pkg/bootstrap/steps/flux.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ func verifyFluxInstallation(input []StepInput, c *Config) ([]StepOutput, error)
3434
}
3535
c.Logger.Successf(fluxExistingInstallMsg)
3636

37-
c.Logger.Actionf("verifying flux reconcillation")
37+
c.Logger.Actionf("verifying flux reconciliation")
3838
out, err = runner.Run("flux", "reconcile", "kustomization", "flux-system")
3939
if err != nil {
4040
return []StepOutput{}, fmt.Errorf("flux bootstrapped error: %v. %s", string(out), fluxFatalErrorMsg)
@@ -45,7 +45,7 @@ func verifyFluxInstallation(input []StepInput, c *Config) ([]StepOutput, error)
4545
if err != nil {
4646
return []StepOutput{}, fmt.Errorf("failed to get flux repository: %v", err)
4747
}
48-
scheme, err := parseRepoScheme(repo.Spec.URL)
48+
_, scheme, err := normaliseUrl(repo.Spec.URL)
4949
if err != nil {
5050
return []StepOutput{}, fmt.Errorf("failed to parse flux repository: %v", err)
5151
}

0 commit comments

Comments
 (0)