Merge pull request #875 from gabemontero/bz1971332

Bug 1971332: revert incorrect allowance of ssh:// prefix with scp styled URLs
openshift · Jul 26, 2021 · 1557476 · 1557476
2 parents 4df50be + 9298da9
commit 1557476
Show file tree

Hide file tree

Showing 3 changed files with 61 additions and 31 deletions.
diff --git a/pkg/helpers/newapp/cmd/newapp_test.go b/pkg/helpers/newapp/cmd/newapp_test.go
@@ -457,6 +457,51 @@ func TestBuildPipelinesWithUnresolvedImage(t *testing.T) {
 	}
 }
 
+func TestBuildPipelinesWithSSHGitURL(t *testing.T) {
+	sshURL := "ssh://git@github.com:22/user/repo.git"
+	sourceRepo, err := app.NewSourceRepository(sshURL, newapp.StrategyDocker)
+	if err != nil {
+		t.Fatal(err)
+	}
+	refs := app.ComponentReferences{
+		app.ComponentReference(&app.ComponentInput{
+			Value:         "ruby",
+			Uses:          sourceRepo,
+			ExpectToBuild: true,
+			ResolvedMatch: &app.ComponentMatch{
+				Value: "ruby",
+			},
+		}),
+	}
+
+	a := AppConfig{}
+	a.Out = &bytes.Buffer{}
+	group, err := a.buildPipelines(refs, app.Environment{}, app.Environment{})
+	if err != nil {
+		t.Error(err)
+	}
+	gitLocationOK := false
+	for _, p := range group {
+		if p.Build != nil {
+			bc, err := p.Build.BuildConfig()
+			if err != nil {
+				t.Error(err)
+			}
+			if bc.Spec.Source.Git == nil {
+				continue
+			}
+			if bc.Spec.Source.Git.URI == sshURL {
+				gitLocationOK = true
+				break
+			}
+		}
+	}
+	if !gitLocationOK {
+		t.Error("ssh git location not preserved")
+	}
+
+}
+
 func TestBuildOutputCycleResilience(t *testing.T) {
 
 	config := &AppConfig{}

diff --git a/pkg/helpers/source-to-image/git/url.go b/pkg/helpers/source-to-image/git/url.go
@@ -86,8 +86,6 @@ func splitOnByte(s string, c byte) (string, string) {
 // Parse parses a "Git URL"
 func Parse(rawurl string) (*URL, error) {
 	if urlSchemeRegexp.MatchString(rawurl) &&
-		// at least through golang 1.14.9, url.Parse cannot handle ssh://git@github.com:sclorg/nodejs-ex
-		!strings.HasPrefix(rawurl, "ssh://") &&
 		(runtime.GOOS != "windows" || !dosDriveRegexp.MatchString(rawurl)) {
 		u, err := url.Parse(rawurl)
 		if err != nil {
@@ -108,12 +106,6 @@ func Parse(rawurl string) (*URL, error) {
 		}, nil
 	}
 
-	// if ssh://git@github.com:sclorg/nodejs-ex then strip ssh:// a la what we
-	// see in other upstream git parsing handling of scp styled git URLs;
-	if strings.HasPrefix(rawurl, "ssh://") {
-		rawurl = strings.Trim(rawurl, "ssh://")
-	}
-
 	s, fragment := splitOnByte(rawurl, '#')
 
 	if m := scpRegexp.FindStringSubmatch(s); m != nil &&

diff --git a/pkg/helpers/source-to-image/git/url_test.go b/pkg/helpers/source-to-image/git/url_test.go
@@ -10,7 +10,6 @@ import (
 
 type parseTest struct {
 	rawurl         string
-	ammendedRawURL string
 	expectedGitURL *URL
 	expectedError  bool
 }
@@ -186,15 +185,19 @@ func TestParse(t *testing.T) {
 			},
 		},
 		parseTest{
-			rawurl:         "ssh://git@github.com:sclorg/nodejs-ex",
-			ammendedRawURL: "git@github.com:sclorg/nodejs-ex",
+			rawurl:        "ssh://git@github.com:sclorg/nodejs-ex",
+			expectedError: true,
+		},
+		parseTest{
+			rawurl: "ssh://git@github.com:22/user/repo.git",
 			expectedGitURL: &URL{
 				URL: url.URL{
-					User: url.User("git"),
-					Host: "github.com",
-					Path: "sclorg/nodejs-ex",
+					Scheme: "ssh",
+					User:   url.User("git"),
+					Host:   "github.com:22",
+					Path:   "/user/repo.git",
 				},
-				Type: URLTypeSCP,
+				Type: URLTypeURL,
 			},
 			expectedError: false,
 		},
@@ -235,24 +238,14 @@ func TestParse(t *testing.T) {
 			t.Errorf("%s: Parse() returned\n\t%#v\nWanted\n\t%#v", test.rawurl, parsedURL, test.expectedGitURL)
 		}
 
-		if len(test.ammendedRawURL) > 0 {
-			if parsedURL.String() != test.ammendedRawURL {
-				t.Errorf("%s: String() returned %s", test.ammendedRawURL, parsedURL.String())
-			}
-
-			if parsedURL.StringNoFragment() != strings.SplitN(test.ammendedRawURL, "#", 2)[0] {
-				t.Errorf("%s: StringNoFragment() returned %s", test.ammendedRawURL, parsedURL.StringNoFragment())
-			}
-
-		} else {
-			if parsedURL.String() != test.rawurl {
-				t.Errorf("%s: String() returned %s", test.rawurl, parsedURL.String())
-			}
+		if parsedURL.String() != test.rawurl {
+			t.Errorf("%s: String() returned %s", test.rawurl, parsedURL.String())
+		}
 
-			if parsedURL.StringNoFragment() != strings.SplitN(test.rawurl, "#", 2)[0] {
-				t.Errorf("%s: StringNoFragment() returned %s", test.rawurl, parsedURL.StringNoFragment())
-			}
+		if parsedURL.StringNoFragment() != strings.SplitN(test.rawurl, "#", 2)[0] {
+			t.Errorf("%s: StringNoFragment() returned %s", test.rawurl, parsedURL.StringNoFragment())
 		}
+
 	}
 }