Merge pull request #6034 from thaJeztah/connhelper_cleanups_step2

cli/connhelper/ssh: add NewSpec utility to prevent parsing URL twice

Author: thaJeztah

cli/connhelper/connhelper.go

@@ -41,7 +41,7 @@ func getConnectionHelper(daemonURL string, sshFlags []string) (*ConnectionHelper
 		return nil, err
 	}
 	if u.Scheme == "ssh" {
-		sp, err := ssh.ParseURL(daemonURL)
+		sp, err := ssh.NewSpec(u)
 		if err != nil {
 			return nil, fmt.Errorf("ssh host connection is not valid: %w", err)
 		}

cli/connhelper/ssh/ssh.go

@@ -17,16 +17,26 @@ func ParseURL(daemonURL string) (*Spec, error) {
 		if errors.As(err, &urlErr) {
 			err = urlErr.Unwrap()
 		}
-		return nil, fmt.Errorf("invalid ssh URL: %w", err)
+		return nil, fmt.Errorf("invalid SSH URL: %w", err)
 	}
-	s, err := newSpec(u)
+	return NewSpec(u)
+}
+
+// NewSpec creates a [Spec] from the given ssh URL's properties. It returns
+// an error if the URL is using the wrong scheme, contains fragments,
+// query-parameters, or contains a password.
+func NewSpec(sshURL *url.URL) (*Spec, error) {
+	s, err := newSpec(sshURL)
 	if err != nil {
-		return nil, fmt.Errorf("invalid ssh URL: %w", err)
+		return nil, fmt.Errorf("invalid SSH URL: %w", err)
 	}
 	return s, nil
 }
 
 func newSpec(u *url.URL) (*Spec, error) {
+	if u == nil {
+		return nil, errors.New("URL is nil")
+	}
 	if u.Scheme == "" {
 		return nil, errors.New("no scheme provided")
 	}

cli/connhelper/ssh/ssh_test.go

@@ -87,41 +87,41 @@ func TestParseURL(t *testing.T) {
 		{
 			doc:           "malformed URL",
 			url:           "malformed %%url",
-			expectedError: `invalid ssh URL: invalid URL escape "%%u"`,
+			expectedError: `invalid SSH URL: invalid URL escape "%%u"`,
 		},
 		{
 			doc:           "URL missing scheme",
 			url:           "no-scheme.example.com",
-			expectedError: "invalid ssh URL: no scheme provided",
+			expectedError: "invalid SSH URL: no scheme provided",
 		},
 		{
 			doc:           "invalid URL with password",
 			url:           "ssh://me:[email protected]",
-			expectedError: "invalid ssh URL: plain-text password is not supported",
+			expectedError: "invalid SSH URL: plain-text password is not supported",
 		},
 		{
 			doc:           "invalid URL with query parameter",
 			url:           "ssh://example.com?foo=bar&bar=baz",
-			expectedError: `invalid ssh URL: query parameters are not allowed: "foo=bar&bar=baz"`,
+			expectedError: `invalid SSH URL: query parameters are not allowed: "foo=bar&bar=baz"`,
 		},
 		{
 			doc:           "invalid URL with fragment",
 			url:           "ssh://example.com#bar",
-			expectedError: `invalid ssh URL: fragments are not allowed: "bar"`,
+			expectedError: `invalid SSH URL: fragments are not allowed: "bar"`,
 		},
 		{
 			doc:           "invalid URL without hostname",
 			url:           "ssh://",
-			expectedError: "invalid ssh URL: hostname is empty",
+			expectedError: "invalid SSH URL: hostname is empty",
 		},
 		{
 			url:           "ssh:///no-hostname",
-			expectedError: "invalid ssh URL: hostname is empty",
+			expectedError: "invalid SSH URL: hostname is empty",
 		},
 		{
 			doc:           "invalid URL with unsupported scheme",
 			url:           "https://example.com",
-			expectedError: `invalid ssh URL: incorrect scheme: https`,
+			expectedError: `invalid SSH URL: incorrect scheme: https`,
 		},
 	}
 	for _, tc := range testCases {