Hubops tests (#3393)

cmd/crowdsec-cli/require/require.go

@@ -89,7 +89,6 @@ func HubDownloader(ctx context.Context, c *csconfig.Config) *cwhub.Downloader {
 	remote := &cwhub.Downloader{
 		Branch:      branch,
 		URLTemplate: urlTemplate,
-		IndexPath:   ".index.json",
 	}
 
 	return remote
@@ -115,7 +114,7 @@ func Hub(c *csconfig.Config, logger *logrus.Logger) (*cwhub.Hub, error) {
 	}
 
 	if err := hub.Load(); err != nil {
-		return nil, fmt.Errorf("failed to read hub index: %w. Run 'sudo cscli hub update' to download the index again", err)
+		return nil, err
 	}
 
 	return hub, nil

pkg/cwhub/cwhub_test.go

@@ -29,10 +29,9 @@ const mockURLTemplate = "https://cdn-hub.crowdsec.net/crowdsecurity/%s/%s"
 
 var responseByPath map[string]string
 
-// testHub initializes a temporary hub with an empty json file, optionally updating it.
-func testHub(t *testing.T, update bool) *Hub {
-	tmpDir, err := os.MkdirTemp("", "testhub")
-	require.NoError(t, err)
+// testHubOld initializes a temporary hub with an empty json file, optionally updating it.
+func testHubOld(t *testing.T, update bool) *Hub {
+	tmpDir := t.TempDir()
 
 	local := &csconfig.LocalHubCfg{
 		HubDir:         filepath.Join(tmpDir, "crowdsec", "hub"),
@@ -41,7 +40,7 @@ func testHub(t *testing.T, update bool) *Hub {
 		InstallDataDir: filepath.Join(tmpDir, "installed-data"),
 	}
 
-	err = os.MkdirAll(local.HubDir, 0o700)
+	err := os.MkdirAll(local.HubDir, 0o700)
 	require.NoError(t, err)
 
 	err = os.MkdirAll(local.InstallDir, 0o700)
@@ -53,22 +52,17 @@ func testHub(t *testing.T, update bool) *Hub {
 	err = os.WriteFile(local.HubIndexFile, []byte("{}"), 0o644)
 	require.NoError(t, err)
 
-	t.Cleanup(func() {
-		os.RemoveAll(tmpDir)
-	})
-
 	hub, err := NewHub(local, log.StandardLogger())
 	require.NoError(t, err)
 
 	if update {
 		indexProvider := &Downloader{
 			Branch:      "master",
 			URLTemplate: mockURLTemplate,
-			IndexPath:   ".index.json",
 		}
 
 		ctx := context.Background()
-		err := hub.Update(ctx, indexProvider, false)
+		err = hub.Update(ctx, indexProvider, false)
 		require.NoError(t, err)
 	}
 
@@ -92,7 +86,7 @@ func envSetup(t *testing.T) *Hub {
 	// Mock the http client
 	HubClient.Transport = newMockTransport()
 
-	hub := testHub(t, true)
+	hub := testHubOld(t, true)
 
 	return hub
 }

pkg/cwhub/doc.go

@@ -90,7 +90,6 @@
 //	indexProvider := cwhub.Downloader{
 //		URLTemplate: "https://cdn-hub.crowdsec.net/crowdsecurity/%s/%s",
 //		Branch: "master",
-//		IndexPath: ".index.json",
 //	}
 //
 // The URLTemplate is a string that will be used to build the URL of the remote hub. It must contain two

pkg/cwhub/download.go

@@ -12,11 +12,13 @@ import (
 	"github.com/crowdsecurity/go-cs-lib/downloader"
 )
 
+// no need to import the lib package to use this
+type NotFoundError = downloader.NotFoundError
+
 // Downloader is used to retrieve index and items from a remote hub, with cache control.
 type Downloader struct {
 	Branch      string
 	URLTemplate string
-	IndexPath   string
 }
 
 // IndexProvider retrieves and writes .index.json
@@ -61,7 +63,7 @@ func addURLParam(rawURL string, param string, value string) (string, error) {
 // It uses a temporary file to avoid partial downloads, and won't overwrite the original
 // if it has not changed.
 func (d *Downloader) FetchIndex(ctx context.Context, destPath string, withContent bool, logger *logrus.Logger) (bool, error) {
-	url, err := d.urlTo(d.IndexPath)
+	url, err := d.urlTo(".index.json")
 	if err != nil {
 		return false, fmt.Errorf("failed to build hub index request: %w", err)
 	}

pkg/cwhub/download_test.go

@@ -10,43 +10,173 @@ import (
 	"testing"
 
 	"github.com/sirupsen/logrus"
+	logtest "github.com/sirupsen/logrus/hooks/test"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+
+	"github.com/crowdsecurity/go-cs-lib/cstest"
 )
 
 func TestFetchIndex(t *testing.T) {
 	ctx := context.Background()
 
 	mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path != "/main/.index.json" {
+			w.WriteHeader(http.StatusNotFound)
+		}
+
 		if r.URL.Query().Get("with_content") == "true" {
-			w.WriteHeader(http.StatusOK)
 			_, err := w.Write([]byte(`Hi I'm an index with content`))
 			assert.NoError(t, err)
 		} else {
-			w.WriteHeader(http.StatusOK)
-			_, err := w.Write([]byte(`Hi I'm a regular index`))
+			_, err := w.Write([]byte(`Hi I'm a minified index`))
 			assert.NoError(t, err)
 		}
 	}))
 	defer mockServer.Close()
 
+	discard := logrus.New()
+	discard.Out = io.Discard
+
 	downloader := &Downloader{
-		Branch:      "main",
 		URLTemplate: mockServer.URL + "/%s/%s",
-		IndexPath:   "index.txt",
 	}
 
-	logger := logrus.New()
-	logger.Out = io.Discard
-
-	destPath := filepath.Join(t.TempDir(), "index.txt")
+	destPath := filepath.Join(t.TempDir(), "index-here")
 	withContent := true
 
-	downloaded, err := downloader.FetchIndex(ctx, destPath, withContent, logger)
+	var notFoundError NotFoundError
+
+	// bad branch
+
+	downloader.Branch = "dev"
+
+	downloaded, err := downloader.FetchIndex(ctx, destPath, withContent, discard)
+	require.ErrorAs(t, err, &notFoundError)
+	assert.False(t, downloaded)
+
+	// ok
+
+	downloader.Branch = "main"
+
+	downloaded, err = downloader.FetchIndex(ctx, destPath, withContent, discard)
 	require.NoError(t, err)
 	assert.True(t, downloaded)
 
 	content, err := os.ReadFile(destPath)
 	require.NoError(t, err)
 	assert.Equal(t, "Hi I'm an index with content", string(content))
+
+	// not "downloading" a second time
+	// since we don't have cache control in the mockServer,
+	// the file is downloaded to a temporary location but not replaced
+
+	downloaded, err = downloader.FetchIndex(ctx, destPath, withContent, discard)
+	require.NoError(t, err)
+	assert.False(t, downloaded)
+
+	// download without item content
+
+	downloaded, err = downloader.FetchIndex(ctx, destPath, !withContent, discard)
+	require.NoError(t, err)
+	assert.True(t, downloaded)
+
+	content, err = os.ReadFile(destPath)
+	require.NoError(t, err)
+	assert.Equal(t, "Hi I'm a minified index", string(content))
+
+	// bad domain name
+
+	downloader.URLTemplate = "x/%s/%s"
+	downloaded, err = downloader.FetchIndex(ctx, destPath, !withContent, discard)
+	cstest.AssertErrorContains(t, err, `Get "x/main/.index.json": unsupported protocol scheme ""`)
+	assert.False(t, downloaded)
+
+	downloader.URLTemplate = "http://x/%s/%s"
+	downloaded, err = downloader.FetchIndex(ctx, destPath, !withContent, discard)
+	// can be no such host, server misbehaving, etc
+	cstest.AssertErrorContains(t, err, `Get "http://x/main/.index.json": dial tcp: lookup x`)
+	assert.False(t, downloaded)
+}
+
+func TestFetchContent(t *testing.T) {
+	ctx := context.Background()
+
+	wantContent := "{'description':'linux'}"
+	wantHash := "e557cb9e1cb051bc3b6a695e4396c5f8e0eff4b7b0d2cc09f7684e1d52ea2224"
+	remotePath := "collections/crowdsecurity/linux.yaml"
+
+	mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path != "/main/"+remotePath {
+			w.WriteHeader(http.StatusNotFound)
+		}
+
+		_, err := w.Write([]byte(wantContent))
+		assert.NoError(t, err)
+	}))
+	defer mockServer.Close()
+
+	wantURL := mockServer.URL + "/main/collections/crowdsecurity/linux.yaml"
+
+	// bad branch
+
+	hubDownloader := &Downloader{
+		URLTemplate: mockServer.URL + "/%s/%s",
+	}
+
+	discard := logrus.New()
+	discard.Out = io.Discard
+
+	destPath := filepath.Join(t.TempDir(), "content-here")
+
+	var notFoundError NotFoundError
+
+	// bad branch
+
+	hubDownloader.Branch = "dev"
+
+	downloaded, url, err := hubDownloader.FetchContent(ctx, remotePath, destPath, wantHash, discard)
+	assert.Empty(t, url)
+	require.ErrorAs(t, err, &notFoundError)
+	assert.False(t, downloaded)
+
+	// bad path
+
+	hubDownloader.Branch = "main"
+
+	downloaded, url, err = hubDownloader.FetchContent(ctx, "collections/linux.yaml", destPath, wantHash, discard)
+	assert.Empty(t, url)
+	require.ErrorAs(t, err, &notFoundError)
+	assert.False(t, downloaded)
+
+	// hash mismatch: the file is not reported as downloaded because it's not replaced
+
+	capture, hook := logtest.NewNullLogger()
+	capture.SetLevel(logrus.WarnLevel)
+
+	downloaded, url, err = hubDownloader.FetchContent(ctx, remotePath, destPath, "1234", capture)
+	assert.Equal(t, wantURL, url)
+	require.NoError(t, err)
+	assert.False(t, downloaded)
+	cstest.RequireLogContains(t, hook, "hash mismatch: expected 1234, got "+wantHash)
+
+	// ok
+
+	downloaded, url, err = hubDownloader.FetchContent(ctx, remotePath, destPath, wantHash, discard)
+	assert.Equal(t, wantURL, url)
+	require.NoError(t, err)
+	assert.True(t, downloaded)
+
+	content, err := os.ReadFile(destPath)
+	require.NoError(t, err)
+	assert.Equal(t, wantContent, string(content))
+
+	// not "downloading" a second time
+	// since we don't have cache control in the mockServer,
+	// the file is downloaded to a temporary location but not replaced
+
+	downloaded, url, err = hubDownloader.FetchContent(ctx, remotePath, destPath, wantHash, discard)
+	assert.Equal(t, wantURL, url)
+	require.NoError(t, err)
+	assert.False(t, downloaded)
 }

pkg/cwhub/fetch.go

@@ -11,8 +11,8 @@ import (
 )
 
 // writeEmbeddedContentTo writes the embedded content to the specified path and checks the hash.
-// If the content is base64 encoded, it will be decoded before writing. Check for item.Content
-// before calling this method.
+// If the content is base64 encoded, it will be decoded before writing. Call this method only
+// if item.Content if not empty.
 func (i *Item) writeEmbeddedContentTo(destPath, wantHash string) error {
 	if i.Content == "" {
 		return fmt.Errorf("no embedded content for %s", i.Name)
@@ -48,7 +48,9 @@ func (i *Item) writeEmbeddedContentTo(destPath, wantHash string) error {
 }
 
 // FetchContentTo writes the last version of the item's YAML file to the specified path.
-// Returns whether the file was downloaded, and the remote url for feedback purposes.
+// If the file is embedded in the index file, it will be written directly without downloads.
+// Returns whether the file was downloaded (to inform if the security engine needs reloading)
+// and the remote url for feedback purposes.
 func (i *Item) FetchContentTo(ctx context.Context, contentProvider ContentProvider, destPath string) (bool, string, error) {
 	wantHash := i.latestHash()
 	if wantHash == "" {

pkg/cwhub/hub.go

@@ -36,7 +36,7 @@ func (h *Hub) GetDataDir() string {
 // and check for unmanaged items.
 func NewHub(local *csconfig.LocalHubCfg, logger *logrus.Logger) (*Hub, error) {
 	if local == nil {
-		return nil, errors.New("no hub configuration found")
+		return nil, errors.New("no hub configuration provided")
 	}
 
 	if logger == nil {
@@ -58,14 +58,10 @@ func (h *Hub) Load() error {
 	h.logger.Debugf("loading hub idx %s", h.local.HubIndexFile)
 
 	if err := h.parseIndex(); err != nil {
-		return err
-	}
-
-	if err := h.localSync(); err != nil {
-		return fmt.Errorf("failed to sync hub items: %w", err)
+		return fmt.Errorf("invalid hub index: %w. Run 'sudo cscli hub update' to download the index again", err)
 	}
 
-	return nil
+	return h.localSync()
 }
 
 // parseIndex takes the content of an index file and fills the map of associated parsers/scenarios/collections.
@@ -153,12 +149,14 @@ func (h *Hub) ItemStats() []string {
 	return ret
 }
 
+var ErrUpdateAfterSync = errors.New("cannot update hub index after load/sync")
+
 // Update downloads the latest version of the index and writes it to disk if it changed.
-// It cannot be called after Load() unless the hub is completely empty.
+// It cannot be called after Load() unless the index was completely empty.
 func (h *Hub) Update(ctx context.Context, indexProvider IndexProvider, withContent bool) error {
-	if len(h.pathIndex) > 0 {
+	if len(h.items) > 0 {
 		// if this happens, it's a bug.
-		return errors.New("cannot update hub after items have been loaded")
+		return ErrUpdateAfterSync
 	}
 
 	downloaded, err := indexProvider.FetchIndex(ctx, h.local.HubIndexFile, withContent, h.logger)