From 6d730d24dd783841f6d56b0a4b776da5527954ee Mon Sep 17 00:00:00 2001 From: Dan Luhring Date: Mon, 1 Feb 2021 10:26:48 -0500 Subject: [PATCH 1/4] Lean on built-in URL parsing to enable path prefix Signed-off-by: Dan Luhring --- cmd/root.go | 11 +---------- internal/anchore/client.go | 28 +++++++++++++++++++++------- internal/anchore/import.go | 2 +- 3 files changed, 23 insertions(+), 18 deletions(-) diff --git a/cmd/root.go b/cmd/root.go index 65262c6de..2786a45ea 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -158,19 +158,10 @@ func doImport(src source.Source, s source.Metadata, catalog *pkg.Catalog, d *dis } } - var scheme string - var hostname = appConfig.Anchore.Host - urlFields := strings.Split(hostname, "://") - if len(urlFields) > 1 { - scheme = urlFields[0] - hostname = urlFields[1] - } - c, err := anchore.NewClient(anchore.Configuration{ - Hostname: hostname, + BasePath: appConfig.Anchore.Host, Username: appConfig.Anchore.Username, Password: appConfig.Anchore.Password, - Scheme: scheme, }) if err != nil { return fmt.Errorf("failed to create anchore client: %+v", err) diff --git a/internal/anchore/client.go b/internal/anchore/client.go index 3283baf71..9033d4a26 100644 --- a/internal/anchore/client.go +++ b/internal/anchore/client.go @@ -3,6 +3,7 @@ package anchore import ( "context" "fmt" + "strings" "github.com/anchore/client-go/pkg/external" "github.com/anchore/syft/internal" @@ -10,11 +11,10 @@ import ( ) type Configuration struct { - Hostname string + BasePath string Username string Password string UserAgent string - Scheme string } type Client struct { @@ -29,16 +29,14 @@ func NewClient(cfg Configuration) (*Client, error) { cfg.UserAgent = fmt.Sprintf("%s / %s %s", internal.ApplicationName, versionInfo.Version, versionInfo.Platform) } - if cfg.Scheme == "" { - cfg.Scheme = "https" - } + basePath := ensureURLHasScheme(cfg.BasePath) // we can rely on the built-in URL parsing for the scheme, host, + // port, and path prefix, as long as a scheme is present return &Client{ config: cfg, client: external.NewAPIClient(&external.Configuration{ - Host: cfg.Hostname, + BasePath: basePath, UserAgent: cfg.UserAgent, - Scheme: cfg.Scheme, }), }, nil } @@ -56,3 +54,19 @@ func (c *Client) newRequestContext(parentContext context.Context) context.Contex }, ) } + +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 +} diff --git a/internal/anchore/import.go b/internal/anchore/import.go index a5c32bd38..25bef2297 100644 --- a/internal/anchore/import.go +++ b/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.Hostname) + stage, prog := importProgress(c.config.BasePath) ctxWithTimeout, cancel := context.WithTimeout(ctx, time.Second*30) defer cancel() From 5b5fa7ec90831b0cc7323fbd9dcef18791b28d6d Mon Sep 17 00:00:00 2001 From: Dan Luhring Date: Mon, 1 Feb 2021 13:57:40 -0500 Subject: [PATCH 2/4] Add tests for Anchore client URL intake Signed-off-by: Dan Luhring --- internal/anchore/client_test.go | 71 +++++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) create mode 100644 internal/anchore/client_test.go diff --git a/internal/anchore/client_test.go b/internal/anchore/client_test.go new file mode 100644 index 000000000..cf3d71b25 --- /dev/null +++ b/internal/anchore/client_test.go @@ -0,0 +1,71 @@ +package anchore + +import "testing" + +func TestHasScheme(t *testing.T) { + cases := []struct { + url string + expected bool + }{ + { + url: "http://localhost", + expected: true, + }, + { + url: "https://anchore.com:8443", + expected: true, + }, + { + url: "google.com", + expected: false, + }, + { + url: "", + expected: false, + }, + } + + for _, testCase := range cases { + t.Run(testCase.url, func(t *testing.T) { + result := hasScheme(testCase.url) + + if testCase.expected != result { + t.Errorf("expected %t but got %t", testCase.expected, result) + } + }) + } +} + +func TestEnsureURLHasScheme(t *testing.T) { + cases := []struct { + url string + expected string + }{ + { + url: "http://localhost", + expected: "http://localhost", + }, + { + url: "https://anchore.com:8443", + expected: "https://anchore.com:8443", + }, + { + url: "google.com:1234/v1/", + expected: "http://google.com:1234/v1/", + }, + { + url: "localhost", + expected: "http://localhost", + }, + } + + for _, testCase := range cases { + t.Run(testCase.url, func(t *testing.T) { + result := ensureURLHasScheme(testCase.url) + + if testCase.expected != result { + t.Errorf("expected '%s' but got '%s'", testCase.expected, result) + } + }) + } +} From b207bc8ee2d28abf59dca9bd8ad4fd4c898c80dd Mon Sep 17 00:00:00 2001 From: Dan Luhring Date: Mon, 1 Feb 2021 16:59:23 -0500 Subject: [PATCH 3/4] Ensure upload base path ends in /v1 Signed-off-by: Dan Luhring --- internal/anchore/client.go | 11 ++++++++++ internal/anchore/client_test.go | 38 +++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/internal/anchore/client.go b/internal/anchore/client.go index 9033d4a26..b3d168063 100644 --- a/internal/anchore/client.go +++ b/internal/anchore/client.go @@ -31,6 +31,9 @@ func NewClient(cfg Configuration) (*Client, error) { basePath := ensureURLHasScheme(cfg.BasePath) // we can rely on the built-in URL parsing for the scheme, host, // port, and path prefix, as long as a scheme is present + basePath = strings.TrimSuffix(basePath, "/") + 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, @@ -70,3 +73,11 @@ func ensureURLHasScheme(url string) string { return url } + +func ensureURLHasSuffix(url, suffix string) string { + if !strings.HasSuffix(url, suffix) { + return url + suffix + } + + return url +} diff --git a/internal/anchore/client_test.go b/internal/anchore/client_test.go index cf3d71b25..3aab531a0 100644 --- a/internal/anchore/client_test.go +++ b/internal/anchore/client_test.go @@ -69,3 +69,41 @@ func TestEnsureURLHasScheme(t *testing.T) { }) } } +func TestEnsureURLHasSuffix(t *testing.T) { + cases := []struct { + url string + suffix string + expected string + }{ + { + url: "http://localhost", + suffix: "/v1", + expected: "http://localhost/v1", + }, + { + url: "http://localhost/v1", + suffix: "/v1", + expected: "http://localhost/v1", + }, + { + url: "http://localhost/v1/", + suffix: "/v1", + expected: "http://localhost/v1//v1", + }, + { + url: "http://localhost-v1", + suffix: "/v1", + expected: "http://localhost-v1/v1", + }, + } + + for _, testCase := range cases { + t.Run(testCase.url, func(t *testing.T) { + result := ensureURLHasSuffix(testCase.url, testCase.suffix) + + if testCase.expected != result { + t.Errorf("expected '%s' but got '%s'", testCase.expected, result) + } + }) + } +} From babb09b3a4cb904fe1952617e592886a367f7c7f Mon Sep 17 00:00:00 2001 From: Dan Luhring Date: Tue, 2 Feb 2021 09:54:00 -0500 Subject: [PATCH 4/4] Refactor and improve base URL prep for client Signed-off-by: Dan Luhring --- cmd/root.go | 4 +- internal/anchore/client.go | 82 ++++++++++----- internal/anchore/client_test.go | 171 +++++++++++++++++++++++++------- internal/anchore/import.go | 2 +- 4 files changed, 196 insertions(+), 63 deletions(-) diff --git a/cmd/root.go b/cmd/root.go index 2786a45ea..d89f8e739 100644 --- a/cmd/root.go +++ b/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{ diff --git a/internal/anchore/client.go b/internal/anchore/client.go index b3d168063..b048fcdca 100644 --- a/internal/anchore/client.go +++ b/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, - // port, and path prefix, as long as a scheme is present - basePath = strings.TrimSuffix(basePath, "/") - basePath = ensureURLHasSuffix(basePath, - "/v1") // We need some mechanism to ensure Syft doesn't try to communicate with the wrong API version. + baseURL, err := prepareBaseURLForClient(cfg.BaseURL) + if err != nil { + return nil, fmt.Errorf("unable to create client: %w", err) + } 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 -} diff --git a/internal/anchore/client_test.go b/internal/anchore/client_test.go index 3aab531a0..fc07ce708 100644 --- a/internal/anchore/client_test.go +++ b/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 - expected string + inputURL string + expectedURL string + expectedErr error }{ { - url: "http://localhost", - expected: "http://localhost", + inputURL: "", + expectedURL: "", + expectedErr: ErrInvalidBaseURLInput, }, { - url: "https://anchore.com:8443", - expected: "https://anchore.com:8443", + inputURL: "localhost", + expectedURL: "http://localhost/v1", + expectedErr: nil, }, { - url: "google.com:1234/v1/", - expected: "http://google.com:1234/v1/", + inputURL: "https://localhost", + expectedURL: "https://localhost/v1", + expectedErr: nil, }, { - url: "localhost", - expected: "http://localhost", + inputURL: "https://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) { - result := ensureURLHasScheme(testCase.url) + t.Run(testCase.inputURL, func(t *testing.T) { + 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 { - t.Errorf("expected '%s' but got '%s'", testCase.expected, result) + if resultURL != testCase.expectedURL { + 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 - suffix string - expected string + input string + expected error }{ { - url: "http://localhost", - suffix: "/v1", - expected: "http://localhost/v1", + input: "", + expected: ErrInvalidBaseURLInput, }, { - url: "http://localhost/v1", - suffix: "/v1", - expected: "http://localhost/v1", + input: "x", + expected: nil, }, { - url: "http://localhost/v1/", - suffix: "/v1", - expected: "http://localhost/v1//v1", + input: "localhost:8000", + expected: nil, }, { - url: "http://localhost-v1", - suffix: "/v1", - expected: "http://localhost-v1/v1", + input: ":80", + expected: ErrInvalidBaseURLInput, + }, + { + input: "/v1", + expected: ErrInvalidBaseURLInput, }, } for _, testCase := range cases { - t.Run(testCase.url, func(t *testing.T) { - result := ensureURLHasSuffix(testCase.url, testCase.suffix) + t.Run(testCase.input, func(t *testing.T) { + resultErr := checkBaseURLInput(testCase.input) - if testCase.expected != result { - t.Errorf("expected '%s' but got '%s'", testCase.expected, result) + if testCase.expected != resultErr { + 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) } }) } diff --git a/internal/anchore/import.go b/internal/anchore/import.go index 25bef2297..f6f62b861 100644 --- a/internal/anchore/import.go +++ b/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()