Refactor and improve base URL prep for client

Signed-off-by: Dan Luhring <dan.luhring@anchore.com>

cmd/root.go

@@ -159,12 +159,12 @@ func doImport(src source.Source, s source.Metadata, catalog *pkg.Catalog, d *dis
 	}
 
 	c, err := anchore.NewClient(anchore.Configuration{
-		BasePath: appConfig.Anchore.Host,
+		BaseURL:  appConfig.Anchore.Host,
 		Username: appConfig.Anchore.Username,
 		Password: appConfig.Anchore.Password,
 	})
 	if err != nil {
-		return fmt.Errorf("failed to create anchore client: %+v", err)
+		return fmt.Errorf("unable to upload results: %w", err)
 	}
 
 	importCfg := anchore.ImportConfig{

internal/anchore/client.go

@@ -2,8 +2,11 @@ package anchore
 
 import (
 	"context"
+	"errors"
 	"fmt"
+	"path"
 	"strings"
+	"unicode"
 
 	"github.com/anchore/client-go/pkg/external"
 	"github.com/anchore/syft/internal"
@@ -11,7 +14,7 @@ import (
 )
 
 type Configuration struct {
-	BasePath  string
+	BaseURL   string
 	Username  string
 	Password  string
 	UserAgent string
@@ -29,16 +32,15 @@ func NewClient(cfg Configuration) (*Client, error) {
 		cfg.UserAgent = fmt.Sprintf("%s / %s %s", internal.ApplicationName, versionInfo.Version, versionInfo.Platform)
 	}
 
-	basePath := ensureURLHasScheme(cfg.BasePath) // we can rely on the built-in URL parsing for the scheme, host,
+	baseURL, err := prepareBaseURLForClient(cfg.BaseURL)
-	// port, and path prefix, as long as a scheme is present
+	if err != nil {
-	basePath = strings.TrimSuffix(basePath, "/")
+		return nil, fmt.Errorf("unable to create client: %w", err)
-	basePath = ensureURLHasSuffix(basePath,
+	}
-		"/v1") // We need some mechanism to ensure Syft doesn't try to communicate with the wrong API version.
 
 	return &Client{
 		config: cfg,
 		client: external.NewAPIClient(&external.Configuration{
-			BasePath:  basePath,
+			BasePath:  baseURL,
 			UserAgent: cfg.UserAgent,
 		}),
 	}, nil
@@ -58,26 +60,56 @@ func (c *Client) newRequestContext(parentContext context.Context) context.Contex
 	)
 }
 
+var ErrInvalidBaseURLInput = errors.New("invalid base URL input")
+
+func prepareBaseURLForClient(baseURL string) (string, error) {
+	if err := checkBaseURLInput(baseURL); err != nil {
+		return "", err
+	}
+
+	scheme, urlWithoutScheme := splitSchemeFromURL(baseURL)
+
+	if scheme == "" {
+		scheme = "http"
+	}
+
+	urlWithoutScheme = path.Clean(urlWithoutScheme)
+
+	const requiredSuffix = "v1"
+	if path.Base(urlWithoutScheme) != requiredSuffix {
+		urlWithoutScheme = path.Join(urlWithoutScheme, requiredSuffix)
+	}
+
+	preparedBaseURL := scheme + "://" + urlWithoutScheme
+	return preparedBaseURL, nil
+}
+
+func checkBaseURLInput(url string) error {
+	if url == "" {
+		return ErrInvalidBaseURLInput
+	}
+
+	firstCharacter := rune(url[0])
+	if !(unicode.IsLetter(firstCharacter)) {
+		return ErrInvalidBaseURLInput
+	}
+
+	return nil
+}
+
+func splitSchemeFromURL(url string) (scheme, urlWithoutScheme string) {
+	if hasScheme(url) {
+		urlParts := strings.SplitN(url, "://", 2)
+		scheme = urlParts[0]
+		urlWithoutScheme = urlParts[1]
+		return
+	}
+
+	return "", url
+}
+
 func hasScheme(url string) bool {
 	parts := strings.Split(url, "://")
 
 	return len(parts) > 1
 }
-
-func ensureURLHasScheme(url string) string {
-	const defaultScheme = "http"
-
-	if !hasScheme(url) {
-		return fmt.Sprintf("%s://%s", defaultScheme, url)
-	}
-
-	return url
-}
-
-func ensureURLHasSuffix(url, suffix string) string {
-	if !strings.HasSuffix(url, suffix) {
-		return url + suffix
-	}
-
-	return url
-}

internal/anchore/client_test.go

@@ -36,73 +36,174 @@ func TestHasScheme(t *testing.T) {
 	}
 }
 
-func TestEnsureURLHasScheme(t *testing.T) {
+func TestPrepareBaseURLForClient(t *testing.T) {
 	cases := []struct {
-		url      string
+		inputURL    string
-		expected string
+		expectedURL string
+		expectedErr error
 	}{
 		{
-			url:      "http://localhost",
+			inputURL:    "",
-			expected: "http://localhost",
+			expectedURL: "",
+			expectedErr: ErrInvalidBaseURLInput,
 		},
 		{
-			url:      "https://anchore.com:8443",
+			inputURL:    "localhost",
-			expected: "https://anchore.com:8443",
+			expectedURL: "http://localhost/v1",
+			expectedErr: nil,
 		},
 		{
-			url:      "google.com:1234/v1/",
+			inputURL:    "https://localhost",
-			expected: "http://google.com:1234/v1/",
+			expectedURL: "https://localhost/v1",
+			expectedErr: nil,
 		},
 		{
-			url:      "localhost",
+			inputURL:    "https://localhost/",
-			expected: "http://localhost",
+			expectedURL: "https://localhost/v1",
+			expectedErr: nil,
+		},
+		{
+			inputURL:    "https://localhost/v1/",
+			expectedURL: "https://localhost/v1",
+			expectedErr: nil,
+		},
+		{
+			inputURL:    "https://localhost/v1//",
+			expectedURL: "https://localhost/v1",
+			expectedErr: nil,
+		},
+		{
+			inputURL:    "http://something.com/platform/v1/services/anchore",
+			expectedURL: "http://something.com/platform/v1/services/anchore/v1",
+			expectedErr: nil,
+		},
+		{
+			inputURL:    "my-host:8228",
+			expectedURL: "http://my-host:8228/v1",
+			expectedErr: nil,
+		},
+		{
+			inputURL:    "v1/v1",
+			expectedURL: "http://v1/v1",
+			expectedErr: nil,
+		},
+		{
+			inputURL:    "/v1",
+			expectedURL: "",
+			expectedErr: ErrInvalidBaseURLInput,
+		},
+		{
+			inputURL:    "/imports/images",
+			expectedURL: "",
+			expectedErr: ErrInvalidBaseURLInput,
 		},
 	}
 
 	for _, testCase := range cases {
-		t.Run(testCase.url, func(t *testing.T) {
+		t.Run(testCase.inputURL, func(t *testing.T) {
-			result := ensureURLHasScheme(testCase.url)
+			resultURL, err := prepareBaseURLForClient(testCase.inputURL)
+			if err != testCase.expectedErr {
+				t.Errorf("expected err to be '%v' but got '%v'", testCase.expectedErr, err)
+			}
 
-			if testCase.expected != result {
+			if resultURL != testCase.expectedURL {
-				t.Errorf("expected '%s' but got '%s'", testCase.expected, result)
+				t.Errorf("expected URL to be '%v' but got '%v'", testCase.expectedURL, resultURL)
 			}
 		})
 	}
 }
-func TestEnsureURLHasSuffix(t *testing.T) {
+
+func TestCheckBaseURLInput(t *testing.T) {
 	cases := []struct {
-		url      string
+		input    string
-		suffix   string
+		expected error
-		expected string
 	}{
 		{
-			url:      "http://localhost",
+			input:    "",
-			suffix:   "/v1",
+			expected: ErrInvalidBaseURLInput,
-			expected: "http://localhost/v1",
 		},
 		{
-			url:      "http://localhost/v1",
+			input:    "x",
-			suffix:   "/v1",
+			expected: nil,
-			expected: "http://localhost/v1",
 		},
 		{
-			url:      "http://localhost/v1/",
+			input:    "localhost:8000",
-			suffix:   "/v1",
+			expected: nil,
-			expected: "http://localhost/v1//v1",
 		},
 		{
-			url:      "http://localhost-v1",
+			input:    ":80",
-			suffix:   "/v1",
+			expected: ErrInvalidBaseURLInput,
-			expected: "http://localhost-v1/v1",
+		},
+		{
+			input:    "/v1",
+			expected: ErrInvalidBaseURLInput,
 		},
 	}
 
 	for _, testCase := range cases {
-		t.Run(testCase.url, func(t *testing.T) {
+		t.Run(testCase.input, func(t *testing.T) {
-			result := ensureURLHasSuffix(testCase.url, testCase.suffix)
+			resultErr := checkBaseURLInput(testCase.input)
 
-			if testCase.expected != result {
+			if testCase.expected != resultErr {
-				t.Errorf("expected '%s' but got '%s'", testCase.expected, result)
+				t.Errorf("expected err to be '%v' but got '%v'", testCase.expected, resultErr)
+			}
+		})
+	}
+}
+
+func TestSplitSchemeFromURL(t *testing.T) {
+	cases := []struct {
+		input                    string
+		expectedScheme           string
+		expectedURLWithoutScheme string
+	}{
+		{
+			input:                    "",
+			expectedScheme:           "",
+			expectedURLWithoutScheme: "",
+		},
+		{
+			input:                    "localhost",
+			expectedScheme:           "",
+			expectedURLWithoutScheme: "localhost",
+		},
+		{
+			input:                    "https://anchore.com/path",
+			expectedScheme:           "https",
+			expectedURLWithoutScheme: "anchore.com/path",
+		},
+		{
+			input:                    "tcp://host:1234",
+			expectedScheme:           "tcp",
+			expectedURLWithoutScheme: "host:1234",
+		},
+		{
+			input:                    "/hello",
+			expectedScheme:           "",
+			expectedURLWithoutScheme: "/hello",
+		},
+		{
+			input:                    "://host",
+			expectedScheme:           "",
+			expectedURLWithoutScheme: "host",
+		},
+		{
+			input:                    "http//localhost",
+			expectedScheme:           "",
+			expectedURLWithoutScheme: "http//localhost",
+		},
+	}
+
+	for _, testCase := range cases {
+		t.Run(testCase.input, func(t *testing.T) {
+			resultScheme, resultURLWithoutScheme := splitSchemeFromURL(testCase.input)
+
+			if testCase.expectedScheme != resultScheme {
+				t.Errorf("expected scheme to be '%s' but got '%s'", testCase.expectedScheme, resultScheme)
+			}
+
+			if testCase.expectedURLWithoutScheme != resultURLWithoutScheme {
+				t.Errorf("expected urlWithoutScheme to be '%s' but got '%s'", testCase.expectedURLWithoutScheme, resultURLWithoutScheme)
 			}
 		})
 	}

internal/anchore/import.go

@@ -51,7 +51,7 @@ func importProgress(source string) (*progress.Stage, *progress.Manual) {
 
 // nolint:funlen
 func (c *Client) Import(ctx context.Context, cfg ImportConfig) error {
-	stage, prog := importProgress(c.config.BasePath)
+	stage, prog := importProgress(c.config.BaseURL)
 
 	ctxWithTimeout, cancel := context.WithTimeout(ctx, time.Second*30)
 	defer cancel()