Linter fixes and cleanup (#1424)

* Linter fixes and cleanup

- all linter fixes
- provide godoc strings to packages
- minor cleanup of testing
- get rid of pkg/testutil (used in single test file)
- get rid of pkg/wait (replace use with pkg/sleep)
- move pkg/rnd under pkg/testing
- use testify/mock in plg/testing/bulk.go - assert on expected calls
  that use the mock
- provide testing/log to stop log output from appearing when tests
  are ran for unit tests (integration test have a race failure)

.gitignore

@@ -14,4 +14,9 @@ fleet-server.dev.yml
 
 dev-tools/integration/.env.bck
 
-.service_token
+# editor swap files
+*.swp
+*.swo
+*.swn
+
+.service_token

.golangci.yml

@@ -2,6 +2,7 @@
 run:
   # timeout for analysis, e.g. 30s, 5m, default is 1m
   timeout: 1m
+  build-tags: integration
 
 issues:
   # Maximum count of issues with the same text.
@@ -33,7 +34,7 @@ linters:
     - goconst # finds repeated strings that could be replaced by a constant
     - dupl # tool for code clone detection
     - forbidigo # forbids identifiers	matched by reg exps
-    - gomoddirectives # manage the use of 'replace', 'retract', and 'excludes' directives in go.mod.
+    #- gomoddirectives # manage the use of 'replace', 'retract', and 'excludes' directives in go.mod.
     - gosimple # linter for Go source code that specializes in simplifying a code
     - misspell # finds commonly misspelled English words in comments
     - nakedret # finds naked returns in functions greater than a specified function length

NOTICE.txt

@@ -2344,11 +2344,11 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 --------------------------------------------------------------------------------
 Dependency : github.com/rs/xid
-Version: v1.2.1
+Version: v1.3.0
 Licence type (autodetected): MIT
 --------------------------------------------------------------------------------
 
-Contents of probable licence file $GOMODCACHE/github.com/rs/xid@v1.2.1/LICENSE:
+Contents of probable licence file $GOMODCACHE/github.com/rs/xid@v1.3.0/LICENSE:
 
 Copyright (c) 2015 Olivier Poitrey <rs@dailymotion.com>
 
@@ -2373,11 +2373,11 @@ THE SOFTWARE.
 
 --------------------------------------------------------------------------------
 Dependency : github.com/rs/zerolog
-Version: v1.22.0
+Version: v1.26.1
 Licence type (autodetected): MIT
 --------------------------------------------------------------------------------
 
-Contents of probable licence file $GOMODCACHE/github.com/rs/zerolog@v1.22.0/LICENSE:
+Contents of probable licence file $GOMODCACHE/github.com/rs/zerolog@v1.26.1/LICENSE:
 
 MIT License
 
@@ -41763,11 +41763,11 @@ THE SOFTWARE.
 
 --------------------------------------------------------------------------------
 Dependency : golang.org/x/crypto
-Version: v0.0.0-20211202192323-5770296d904e
+Version: v0.0.0-20211215165025-cf75a172585e
 Licence type (autodetected): BSD-3-Clause
 --------------------------------------------------------------------------------
 
-Contents of probable licence file $GOMODCACHE/golang.org/x/crypto@v0.0.0-20211202192323-5770296d904e/LICENSE:
+Contents of probable licence file $GOMODCACHE/golang.org/x/crypto@v0.0.0-20211215165025-cf75a172585e/LICENSE:
 
 Copyright (c) 2009 The Go Authors. All rights reserved.
 

cmd/fleet/main.go

@@ -87,8 +87,8 @@ func makeCacheConfig(cfg *config.Config) cache.Config {
 		ActionTTL:    ccfg.ActionTTL,
 		EnrollKeyTTL: ccfg.EnrollKeyTTL,
 		ArtifactTTL:  ccfg.ArtifactTTL,
-		ApiKeyTTL:    ccfg.APIKeyTTL,
-		ApiKeyJitter: ccfg.APIKeyJitter,
+		APIKeyTTL:    ccfg.APIKeyTTL,
+		APIKeyJitter: ccfg.APIKeyJitter,
 	}
 }
 

cmd/fleet/main_integration_test.go

@@ -5,6 +5,7 @@
 //go:build integration
 // +build integration
 
+//nolint:unused // some unused code may be added to more tests
 package fleet
 
 import (
@@ -53,7 +54,7 @@ output:
     service_token: '${ELASTICSEARCH_SERVICE_TOKEN}'
 `
 
-var agentIdCfgData = `
+var agentIDCfgData = `
 output:
   elasticsearch:
     hosts: '${ELASTICSEARCH_HOSTS:localhost:9200}'
@@ -84,9 +85,9 @@ func (s *agentSuite) TestAgentMode(t *testing.T) {
 	bulker := ftesting.SetupBulk(ctx, t)
 
 	// add a real default fleet server policy
-	policyId := uuid.Must(uuid.NewV4()).String()
+	policyID := uuid.Must(uuid.NewV4()).String()
 	_, err := dl.CreatePolicy(ctx, bulker, model.Policy{
-		PolicyID:           policyId,
+		PolicyID:           policyID,
 		RevisionIdx:        1,
 		DefaultFleetServer: true,
 		Data:               policyData,
@@ -98,7 +99,7 @@ func (s *agentSuite) TestAgentMode(t *testing.T) {
 		Name:     "Default",
 		APIKey:   "keyvalue",
 		APIKeyID: "keyid",
-		PolicyID: policyId,
+		PolicyID: policyID,
 		Active:   true,
 	})
 	require.NoError(t, err)
@@ -134,7 +135,7 @@ func (s *agentSuite) TestAgentMode(t *testing.T) {
 	}, ftesting.RetrySleep(100*time.Millisecond), ftesting.RetryCount(120))
 
 	// reconfigure with agent ID set
-	err = appState.UpdateConfig(agentIdCfgData)
+	err = appState.UpdateConfig(agentIDCfgData)
 	require.NoError(t, err)
 
 	// wait for fleet-server to report as healthy
@@ -160,7 +161,7 @@ func (s *agentSuite) TestAgentMode(t *testing.T) {
 	}, ftesting.RetrySleep(100*time.Millisecond), ftesting.RetryCount(120))
 
 	// reconfigure to good config
-	err = appState.UpdateConfig(agentIdCfgData)
+	err = appState.UpdateConfig(agentIDCfgData)
 	require.NoError(t, err)
 
 	// wait for fleet-server to report as healthy

cmd/fleet/server_integration_test.go

@@ -48,7 +48,7 @@ type tserver struct {
 	srv *FleetServer
 }
 
-func (s *tserver) baseUrl() string {
+func (s *tserver) baseURL() string {
 	input, _ := s.cfg.GetFleetInput()
 	tls := input.Server.TLS
 	schema := "http"
@@ -65,14 +65,14 @@ func (s *tserver) waitExit() error {
 func startTestServer(ctx context.Context) (*tserver, error) {
 	cfg, err := config.LoadFile("../../fleet-server.yml")
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("config load error: %w", err)
 	}
 
-	logger.Init(cfg, "fleet-server")
+	logger.Init(cfg, "fleet-server") //nolint:errcheck // test logging setup
 
 	port, err := ftesting.FreePort()
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("unable to find port: %w", err)
 	}
 
 	srvcfg := &config.Server{}
@@ -84,7 +84,7 @@ func startTestServer(ctx context.Context) (*tserver, error) {
 
 	srv, err := NewFleetServer(cfg, build.Info{Version: serverVersion}, status.NewLog())
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("unable to create server: %w", err)
 	}
 
 	g, ctx := errgroup.WithContext(ctx)
@@ -96,7 +96,7 @@ func startTestServer(ctx context.Context) (*tserver, error) {
 	tsrv := &tserver{cfg: cfg, g: g, srv: srv}
 	err = tsrv.waitServerUp(ctx, testWaitServerUp)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("unable to start server: %w", err)
 	}
 	return tsrv, nil
 }
@@ -105,7 +105,7 @@ func (s *tserver) waitServerUp(ctx context.Context, dur time.Duration) error {
 	start := time.Now()
 	cli := cleanhttp.DefaultClient()
 	for {
-		res, err := cli.Get(s.baseUrl() + "/api/status")
+		res, err := cli.Get(s.baseURL() + "/api/status") //nolint:noctx // test setup
 		if err != nil {
 			if time.Since(start) > dur {
 				return err
@@ -123,7 +123,7 @@ func (s *tserver) waitServerUp(ctx context.Context, dur time.Duration) error {
 
 }
 
-func (s *tserver) buildUrl(id string, cmd string) string {
+func (s *tserver) buildURL(id string, cmd string) string {
 	ur := "/api/fleet/agents"
 	if id != "" {
 		ur = path.Join(ur, id)
@@ -132,7 +132,7 @@ func (s *tserver) buildUrl(id string, cmd string) string {
 		ur = path.Join(ur, cmd)
 	}
 
-	return s.baseUrl() + ur
+	return s.baseURL() + ur
 }
 
 func TestServerUnauthorized(t *testing.T) {
@@ -143,16 +143,16 @@ func TestServerUnauthorized(t *testing.T) {
 	srv, err := startTestServer(ctx)
 	require.NoError(t, err)
 
-	agentId := uuid.Must(uuid.NewV4()).String()
+	agentID := uuid.Must(uuid.NewV4()).String()
 	cli := cleanhttp.DefaultClient()
 
 	agenturls := []string{
-		srv.buildUrl(agentId, "checkin"),
-		srv.buildUrl(agentId, "acks"),
+		srv.buildURL(agentID, "checkin"),
+		srv.buildURL(agentID, "acks"),
 	}
 
 	allurls := []string{
-		srv.buildUrl("", "enroll"),
+		srv.buildURL("", "enroll"),
 	}
 	allurls = append(allurls, agenturls...)
 
@@ -161,7 +161,7 @@ func TestServerUnauthorized(t *testing.T) {
 	// TODO: revisit error response format
 	t.Run("no auth header", func(t *testing.T) {
 		for _, u := range allurls {
-			res, err := cli.Post(u, "application/json", bytes.NewBuffer([]byte("{}")))
+			res, err := cli.Post(u, "application/json", bytes.NewBuffer([]byte("{}"))) //nolint:noctx // test case
 			if err != nil {
 				t.Fatal(err)
 			}
@@ -190,9 +190,8 @@ func TestServerUnauthorized(t *testing.T) {
 
 	// Unauthorized, expecting error from /_security/_authenticate
 	t.Run("unauthorized", func(t *testing.T) {
-
 		for _, u := range agenturls {
-			req, err := http.NewRequest("POST", u, bytes.NewBuffer([]byte("{}")))
+			req, err := http.NewRequest("POST", u, bytes.NewBuffer([]byte("{}"))) //nolint:noctx // test case
 			require.NoError(t, err)
 			req.Header.Set("Content-Type", "application/json")
 			req.Header.Set("Authorization", "ApiKey ZExqY1hYWUJJUVVxWDVia2JvVGM6M05XaUt5aHBRYk9YSTRQWDg4YWp0UQ==")
@@ -225,7 +224,7 @@ func TestServerUnauthorized(t *testing.T) {
 
 	// Stop test server
 	cancel()
-	srv.waitExit()
+	srv.waitExit() //nolint:errcheck // test case
 }
 
 func TestServerInstrumentation(t *testing.T) {
@@ -239,7 +238,7 @@ func TestServerInstrumentation(t *testing.T) {
 			return
 		}
 		tracerConnected <- struct{}{}
-		io.Copy(io.Discard, req.Body)
+		io.Copy(io.Discard, req.Body) //nolint:errcheck // test case
 		tracerDisconnected <- struct{}{}
 	}))
 	defer server.Close()
@@ -248,8 +247,7 @@ func TestServerInstrumentation(t *testing.T) {
 	srv, err := startTestServer(ctx)
 	require.NoError(t, err)
 
-	newInstrumentationCfg := func(cfg config.Config, instr config.Instrumentation) {
-		cfg.Inputs[0] = cfg.Inputs[0]
+	newInstrumentationCfg := func(cfg config.Config, instr config.Instrumentation) { //nolint:govet // mutex should not be copied in operation (hopefully)
 		cfg.Inputs[0].Server.Instrumentation = instr
 
 		newCfg, err := srv.cfg.Merge(&cfg)
@@ -259,18 +257,23 @@ func TestServerInstrumentation(t *testing.T) {
 	}
 
 	// Enable instrumentation
-	newInstrumentationCfg(*srv.cfg, config.Instrumentation{
+	newInstrumentationCfg(*srv.cfg, config.Instrumentation{ //nolint:govet // mutex should not be copied in operation (hopefully)
 		Enabled: true,
 		Hosts:   []string{server.URL},
 	})
 
 	stopClient := make(chan struct{})
 	cli := cleanhttp.DefaultClient()
 	callCheckinFunc := func() {
+		var Err error
+		defer require.NoError(t, Err)
 		for {
-			agentId := "1e4954ce-af37-4731-9f4a-407b08e69e42"
-			cli.Post(srv.buildUrl(agentId, "checkin"), "application/json", bytes.NewBuffer([]byte("{}")))
-			require.NoError(t, err)
+			agentID := "1e4954ce-af37-4731-9f4a-407b08e69e42"
+			res, err := cli.Post(srv.buildURL(agentID, "checkin"), "application/json", bytes.NewBuffer([]byte("{}"))) //nolint:noctx,staticcheck // test case
+			if res != nil && res.Body != nil {
+				res.Body.Close()
+			}
+			Err = err //nolint:ineffassign,staticcheck,wastedassign // ugly work around for error checking
 			select {
 			case <-ctx.Done():
 				return
@@ -292,7 +295,7 @@ func TestServerInstrumentation(t *testing.T) {
 	}
 
 	// Turn instrumentation off
-	newInstrumentationCfg(*srv.cfg, config.Instrumentation{
+	newInstrumentationCfg(*srv.cfg, config.Instrumentation{ //nolint:govet // mutex should not be copied in operation (hopefully)
 		Enabled: false,
 		Hosts:   []string{server.URL},
 	})

go.mod

@@ -19,8 +19,8 @@ require (
 	github.com/miolini/datacounter v1.0.2
 	github.com/pbnjay/memory v0.0.0-20210728143218-7b4eea64cf58
 	github.com/pkg/errors v0.9.1
-	github.com/rs/xid v1.2.1
-	github.com/rs/zerolog v1.22.0
+	github.com/rs/xid v1.3.0
+	github.com/rs/zerolog v1.26.1
 	github.com/spf13/cobra v1.3.0
 	github.com/stretchr/testify v1.7.0
 	go.elastic.co/apm v1.15.0
@@ -56,6 +56,7 @@ require (
 	github.com/pmezard/go-difflib v1.0.0 // indirect
 	github.com/santhosh-tekuri/jsonschema v1.2.4 // indirect
 	github.com/spf13/pflag v1.0.5 // indirect
+	github.com/stretchr/objx v0.2.0 // indirect
 	go.elastic.co/apm/module/apmgrpc v1.15.0 // indirect
 	go.elastic.co/apm/module/apmhttp v1.15.0 // indirect
 	go.elastic.co/fastjson v1.1.0 // indirect
@@ -82,7 +83,7 @@ require (
 	go.elastic.co/ecszap v1.0.0 // indirect
 	go.uber.org/atomic v1.9.0 // indirect
 	go.uber.org/multierr v1.7.0 // indirect
-	golang.org/x/crypto v0.0.0-20211202192323-5770296d904e // indirect
+	golang.org/x/crypto v0.0.0-20211215165025-cf75a172585e // indirect
 	golang.org/x/net v0.0.0-20211216030914-fe4d6282115f // indirect
 	golang.org/x/sys v0.0.0-20220209214540-3681064d5158 // indirect
 	golang.org/x/tools v0.1.8 // indirect

go.sum

@@ -1762,10 +1762,12 @@ github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFR
 github.com/rogpeppe/go-internal v1.5.2/go.mod h1:xXDCJY+GAPziupqXw64V24skbSoqbTEfhy4qGm1nDQc=
 github.com/rs/cors v1.6.0/go.mod h1:gFx+x8UowdsKA9AchylcLynDq+nNFfI8FkUZdN/jGCU=
 github.com/rs/cors v1.7.0/go.mod h1:gFx+x8UowdsKA9AchylcLynDq+nNFfI8FkUZdN/jGCU=
-github.com/rs/xid v1.2.1 h1:mhH9Nq+C1fY2l1XIpgxIiUOfNpRBYH1kKcr+qfKgjRc=
 github.com/rs/xid v1.2.1/go.mod h1:+uKXf+4Djp6Md1KODXJxgGQPKngRmWyn10oCKFzNHOQ=
-github.com/rs/zerolog v1.22.0 h1:XrVUjV4K+izZpKXZHlPrYQiDtmdGiCylnT4i43AAWxg=
+github.com/rs/xid v1.3.0 h1:6NjYksEUlhurdVehpc7S7dk6DAmcKv8V9gG0FsVN2U4=
+github.com/rs/xid v1.3.0/go.mod h1:trrq9SKmegXys3aeAKXMUTdJsYXVwGY3RLcfgqegfbg=
 github.com/rs/zerolog v1.22.0/go.mod h1:ZPhntP/xmq1nnND05hhpAh2QMhSsA4UN3MGZ6O2J3hM=
+github.com/rs/zerolog v1.26.1 h1:/ihwxqH+4z8UxyI70wM1z9yCvkWcfz/a3mj48k/Zngc=
+github.com/rs/zerolog v1.26.1/go.mod h1:/wSSJWX7lVrsOwlbyTRSOJvqRlc+WjWlfes+CiJ+tmc=
 github.com/rubenv/sql-migrate v0.0.0-20210614095031-55d5740dbbcc/go.mod h1:HFLT6i9iR4QBOF5rdCyjddC9t59ArqWJV2xx+jwcCMo=
 github.com/russross/blackfriday v1.5.2/go.mod h1:JO/DiYxRf+HjHt06OyowR9PTA263kcR/rfWxYHBV53g=
 github.com/russross/blackfriday/v2 v2.0.1/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
@@ -2163,8 +2165,8 @@ golang.org/x/crypto v0.0.0-20210421170649-83a5a9bb288b/go.mod h1:T9bdIzuCu7OtxOm
 golang.org/x/crypto v0.0.0-20210513164829-c07d793c2f9a/go.mod h1:P+XmwS30IXTQdn5tA2iutPOUgjI07+tq3H3K9MVA1s8=
 golang.org/x/crypto v0.0.0-20210616213533-5ff15b29337e/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc=
 golang.org/x/crypto v0.0.0-20210817164053-32db794688a5/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc=
-golang.org/x/crypto v0.0.0-20211202192323-5770296d904e h1:MUP6MR3rJ7Gk9LEia0LP2ytiH6MuCfs7qYz+47jGdD8=
-golang.org/x/crypto v0.0.0-20211202192323-5770296d904e/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4=
+golang.org/x/crypto v0.0.0-20211215165025-cf75a172585e h1:1SzTfNOXwIS2oWiMF+6qu0OUDKb0dauo6MoDUQyu+yU=
+golang.org/x/crypto v0.0.0-20211215165025-cf75a172585e/go.mod h1:P+XmwS30IXTQdn5tA2iutPOUgjI07+tq3H3K9MVA1s8=
 golang.org/x/exp v0.0.0-20180321215751-8460e604b9de/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
 golang.org/x/exp v0.0.0-20180807140117-3d87b88a115f/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
 golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
@@ -2290,7 +2292,6 @@ golang.org/x/net v0.0.0-20210813160813-60bc85c4be6d/go.mod h1:9nx3DQGgdP8bBQD5qx
 golang.org/x/net v0.0.0-20210825183410-e898025ed96a/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y=
 golang.org/x/net v0.0.0-20211015210444-4f30a5c0130f/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y=
 golang.org/x/net v0.0.0-20211020060615-d418f374d309/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y=
-golang.org/x/net v0.0.0-20211112202133-69e39bad7dc2/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y=
 golang.org/x/net v0.0.0-20211209124913-491a49abca63/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y=
 golang.org/x/net v0.0.0-20211216030914-fe4d6282115f h1:hEYJvxw1lSnWIl8X9ofsYMklzaDs90JI2az5YMd4fPM=
 golang.org/x/net v0.0.0-20211216030914-fe4d6282115f/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y=