Improve code quality based on linter hints (#563)

efd6 · web-flow · commit d27d3b4bd047 · 2021-11-03T08:02:37.000+01:00
* all: gardening clean up

Changes to remove magic constants, handle errors and clean up. Some changes
requested by opinionated editor/linter.

Makefile fixed to ensure that build directory exists before starting work.

* address pr comments
diff --git a/internal/fields/dependency_manager.go b/internal/fields/dependency_manager.go
@@ -86,9 +86,9 @@ func loadECSFieldsSchema(dep buildmanifest.ECSDependency) ([]FieldDefinition, er
 			return nil, errors.Wrapf(err, "can't download the online schema (URL: %s)", url)
 		}
 		defer resp.Body.Close()
-		if resp.StatusCode == 404 {
-			return nil, fmt.Errorf("unsatisfied ECS dependency, reference defined in build manifest doesn't exist (HTTP 404, URL: %s)", url)
-		} else if resp.StatusCode != 200 {
+		if resp.StatusCode == http.StatusNotFound {
+			return nil, fmt.Errorf("unsatisfied ECS dependency, reference defined in build manifest doesn't exist (HTTP StatusNotFound, URL: %s)", url)
+		} else if resp.StatusCode != http.StatusOK {
 			return nil, fmt.Errorf("unexpected HTTP status code: %d", resp.StatusCode)
 		}
 
diff --git a/internal/files/compress.go b/internal/files/compress.go
@@ -36,7 +36,10 @@ func Zip(sourcePath, destinationFile string) error {
 	}
 	defer os.RemoveAll(tempDir)
 	workDir := filepath.Join(tempDir, folderNameFromFileName(destinationFile))
-	os.MkdirAll(workDir, 0755)
+	err = os.MkdirAll(workDir, 0755)
+	if err != nil {
+		return errors.Wrapf(err, "can't prepare work directory: %s", workDir)
+	}
 
 	logger.Debugf("Create work directory for archiving: %s", workDir)
 	err = CopyAll(sourcePath, workDir)
diff --git a/internal/kibana/agents.go b/internal/kibana/agents.go
@@ -7,6 +7,7 @@ package kibana
 import (
 	"encoding/json"
 	"fmt"
+	"net/http"
 	"time"
 
 	"github.com/pkg/errors"
@@ -45,8 +46,8 @@ func (c *Client) ListAgents() ([]Agent, error) {
 		return nil, errors.Wrap(err, "could not list agents")
 	}
 
-	if statusCode != 200 {
-		return nil, fmt.Errorf("could not list agents; API status code = %d", statusCode)
+	if statusCode != http.StatusOK {
+		return nil, fmt.Errorf("could not list agents; API status code = %d; response body = %s", statusCode, respBody)
 	}
 
 	var resp struct {
@@ -70,8 +71,8 @@ func (c *Client) AssignPolicyToAgent(a Agent, p Policy) error {
 		return errors.Wrap(err, "could not assign policy to agent")
 	}
 
-	if statusCode != 200 {
-		return fmt.Errorf("could not assign policy to agent; API status code = %d; response body = %s", statusCode, string(respBody))
+	if statusCode != http.StatusOK {
+		return fmt.Errorf("could not assign policy to agent; API status code = %d; response body = %s", statusCode, respBody)
 	}
 
 	err = c.waitUntilPolicyAssigned(a, p)
@@ -115,8 +116,8 @@ func (c *Client) getAgent(agentID string) (*Agent, error) {
 		return nil, errors.Wrap(err, "could not list agents")
 	}
 
-	if statusCode != 200 {
-		return nil, fmt.Errorf("could not list agents; API status code = %d", statusCode)
+	if statusCode != http.StatusOK {
+		return nil, fmt.Errorf("could not list agents; API status code = %d; response body = %s", statusCode, respBody)
 	}
 
 	var resp struct {
diff --git a/internal/kibana/dashboards.go b/internal/kibana/dashboards.go
@@ -35,13 +35,13 @@ func (c *Client) Export(dashboardIDs []string) ([]common.MapStr, error) {
 	path := fmt.Sprintf("%s/dashboards/export%s", CoreAPI, query.String())
 	statusCode, respBody, err := c.get(path)
 	if err != nil {
-		return nil, errors.Wrapf(err, "could not export dashboards; API status code = %d; response body = %s", statusCode, string(respBody))
+		return nil, errors.Wrapf(err, "could not export dashboards; API status code = %d; response body = %s", statusCode, respBody)
 	}
 
 	var exported exportedType
 	err = json.Unmarshal(respBody, &exported)
 	if err != nil {
-		return nil, errors.Wrapf(err, "unmarshalling response failed (body: \n%s)", string(respBody))
+		return nil, errors.Wrapf(err, "unmarshalling response failed (body: \n%s)", respBody)
 	}
 
 	var multiErr multierror.Error
diff --git a/internal/kibana/packages.go b/internal/kibana/packages.go
@@ -7,6 +7,7 @@ package kibana
 import (
 	"encoding/json"
 	"fmt"
+	"net/http"
 
 	"github.com/pkg/errors"
 
@@ -38,7 +39,7 @@ func (c *Client) RemovePackage(pkg packages.PackageManifest) ([]packages.Asset,
 }
 
 func processResults(action string, statusCode int, respBody []byte) ([]packages.Asset, error) {
-	if statusCode != 200 {
+	if statusCode != http.StatusOK {
 		return nil, fmt.Errorf("could not %s package; API status code = %d; response body = %s", action, statusCode, respBody)
 	}
 
diff --git a/internal/kibana/policies.go b/internal/kibana/policies.go
@@ -7,6 +7,7 @@ package kibana
 import (
 	"encoding/json"
 	"fmt"
+	"net/http"
 
 	"github.com/pkg/errors"
 
@@ -34,7 +35,7 @@ func (c *Client) CreatePolicy(p Policy) (*Policy, error) {
 		return nil, errors.Wrap(err, "could not create policy")
 	}
 
-	if statusCode != 200 {
+	if statusCode != http.StatusOK {
 		return nil, fmt.Errorf("could not create policy; API status code = %d; response body = %s", statusCode, respBody)
 	}
 
@@ -56,7 +57,7 @@ func (c *Client) GetPolicy(policyID string) (*Policy, error) {
 		return nil, errors.Wrap(err, "could not get policy")
 	}
 
-	if statusCode != 200 {
+	if statusCode != http.StatusOK {
 		return nil, fmt.Errorf("could not get policy; API status code = %d; response body = %s", statusCode, respBody)
 	}
 
@@ -80,7 +81,7 @@ func (c *Client) DeletePolicy(p Policy) error {
 		return errors.Wrap(err, "could not delete policy")
 	}
 
-	if statusCode != 200 {
+	if statusCode != http.StatusOK {
 		return fmt.Errorf("could not delete policy; API status code = %d; response body = %s", statusCode, respBody)
 	}
 
@@ -150,7 +151,7 @@ func (c *Client) AddPackageDataStreamToPolicy(r PackageDataStream) error {
 		return errors.Wrap(err, "could not add package to policy")
 	}
 
-	if statusCode != 200 {
+	if statusCode != http.StatusOK {
 		return fmt.Errorf("could not add package to policy; API status code = %d; response body = %s", statusCode, respBody)
 	}
 
diff --git a/internal/kibana/saved_objects.go b/internal/kibana/saved_objects.go
@@ -60,7 +60,7 @@ func (c *Client) FindDashboards() (DashboardSavedObjects, error) {
 	logger.Debug("Find dashboards using the Saved Objects API")
 
 	var foundObjects DashboardSavedObjects
-	var page = 1
+	page := 1
 
 	for {
 		r, err := c.findDashboardsNextPage(page)
@@ -94,7 +94,7 @@ func (c *Client) findDashboardsNextPage(page int) (*savedObjectsResponse, error)
 	path := fmt.Sprintf("%s/_find?type=dashboard&fields=title&per_page=%d&page=%d", SavedObjectsAPI, findDashboardsPerPage, page)
 	statusCode, respBody, err := c.get(path)
 	if err != nil {
-		return nil, errors.Wrapf(err, "could not find dashboards; API status code = %d; response body = %s", statusCode, string(respBody))
+		return nil, errors.Wrapf(err, "could not find dashboards; API status code = %d; response body = %s", statusCode, respBody)
 	}
 
 	var r savedObjectsResponse
diff --git a/internal/packages/packages.go b/internal/packages/packages.go
@@ -43,12 +43,10 @@ func (vv *VarValue) Unpack(value interface{}) error {
 	switch u := value.(type) {
 	case []interface{}:
 		vv.list = u
-		return nil
 	default:
 		vv.scalar = u
-		return nil
 	}
-	return errors.New("unknown variable value")
+	return nil
 }
 
 // MarshalJSON knows how to serialize a VarValue into the appropriate
diff --git a/internal/registry/revisions.go b/internal/registry/revisions.go
@@ -7,6 +7,7 @@ package registry
 import (
 	"encoding/json"
 	"fmt"
+	"net/http"
 	"sort"
 
 	"github.com/Masterminds/semver"
@@ -44,7 +45,7 @@ func (c *Client) Revisions(packageName string, options SearchOptions) ([]package
 	if err != nil {
 		return nil, errors.Wrap(err, "could not retrieve package")
 	}
-	if statusCode != 200 {
+	if statusCode != http.StatusOK {
 		return nil, fmt.Errorf("could not retrieve package; API status code = %d; response body = %s", statusCode, respBody)
 	}
 
diff --git a/internal/testrunner/runners/pipeline/ingest_pipeline.go b/internal/testrunner/runners/pipeline/ingest_pipeline.go
@@ -10,6 +10,7 @@ import (
 	"fmt"
 	"io"
 	"log"
+	"net/http"
 	"os"
 	"path/filepath"
 	"regexp"
@@ -169,7 +170,7 @@ func putIngestPipeline(esClient *elasticsearch.Client, pipeline pipelineResource
 		return errors.Wrapf(err, "failed to read PutPipeline API response body (pipelineName: %s)", pipeline.name)
 	}
 
-	if r.StatusCode != 200 {
+	if r.StatusCode != http.StatusOK {
 		return errors.Wrapf(es.NewError(body), "unexpected response status for PutPipeline (%d): %s (pipelineName: %s)",
 			r.StatusCode, r.Status(), pipeline.name)
 	}
@@ -190,7 +191,7 @@ func getIngestPipeline(esClient *elasticsearch.Client, pipelineName string) erro
 		return errors.Wrapf(err, "failed to read GetPipeline API response body (pipelineName: %s)", pipelineName)
 	}
 
-	if r.StatusCode != 200 {
+	if r.StatusCode != http.StatusOK {
 		return errors.Wrapf(es.NewError(body), "unexpected response status for GetPipeline (%d): %s (pipelineName: %s)",
 			r.StatusCode, r.Status(), pipelineName)
 	}
@@ -237,7 +238,7 @@ func simulatePipelineProcessing(esClient *elasticsearch.Client, pipelineName str
 		return nil, errors.Wrap(err, "failed to read Simulate API response body")
 	}
 
-	if r.StatusCode != 200 {
+	if r.StatusCode != http.StatusOK {
 		return nil, errors.Wrapf(es.NewError(body), "unexpected response status for Simulate (%d): %s", r.StatusCode, r.Status())
 	}
 

Original file line number	Diff line number	Diff line change
`@@ -86,9 +86,9 @@ func loadECSFieldsSchema(dep buildmanifest.ECSDependency) ([]FieldDefinition, er`
`86`	`86`	`return nil, errors.Wrapf(err, "can't download the online schema (URL: %s)", url)`
`87`	`87`	`}`
`88`	`88`	`defer resp.Body.Close()`
`89`		`- if resp.StatusCode == 404 {`
`90`		`- return nil, fmt.Errorf("unsatisfied ECS dependency, reference defined in build manifest doesn't exist (HTTP 404, URL: %s)", url)`
`91`		`- } else if resp.StatusCode != 200 {`
	`89`	`+ if resp.StatusCode == http.StatusNotFound {`
	`90`	`+ return nil, fmt.Errorf("unsatisfied ECS dependency, reference defined in build manifest doesn't exist (HTTP StatusNotFound, URL: %s)", url)`
	`91`	`+ } else if resp.StatusCode != http.StatusOK {`
`92`	`92`	`return nil, fmt.Errorf("unexpected HTTP status code: %d", resp.StatusCode)`
`93`	`93`	`}`
`94`	`94`
Original file line number	Diff line number	Diff line change
`@@ -7,6 +7,7 @@ package kibana`
`7`	`7`	`import (`
`8`	`8`	`"encoding/json"`
`9`	`9`	`"fmt"`
	`10`	`+ "net/http"`
`10`	`11`	`"time"`
`11`	`12`
`12`	`13`	`"github.com/pkg/errors"`
`@@ -45,8 +46,8 @@ func (c *Client) ListAgents() ([]Agent, error) {`
`45`	`46`	`return nil, errors.Wrap(err, "could not list agents")`
`46`	`47`	`}`
`47`	`48`
`48`		`- if statusCode != 200 {`
`49`		`- return nil, fmt.Errorf("could not list agents; API status code = %d", statusCode)`
	`49`	`+ if statusCode != http.StatusOK {`
	`50`	`+ return nil, fmt.Errorf("could not list agents; API status code = %d; response body = %s", statusCode, respBody)`
`50`	`51`	`}`
`51`	`52`
`52`	`53`	`var resp struct {`
`@@ -70,8 +71,8 @@ func (c *Client) AssignPolicyToAgent(a Agent, p Policy) error {`
`70`	`71`	`return errors.Wrap(err, "could not assign policy to agent")`
`71`	`72`	`}`
`72`	`73`
`73`		`- if statusCode != 200 {`
`74`		`- return fmt.Errorf("could not assign policy to agent; API status code = %d; response body = %s", statusCode, string(respBody))`
	`74`	`+ if statusCode != http.StatusOK {`
	`75`	`+ return fmt.Errorf("could not assign policy to agent; API status code = %d; response body = %s", statusCode, respBody)`
`75`	`76`	`}`
`76`	`77`
`77`	`78`	`err = c.waitUntilPolicyAssigned(a, p)`
`@@ -115,8 +116,8 @@ func (c Client) getAgent(agentID string) (Agent, error) {`
`115`	`116`	`return nil, errors.Wrap(err, "could not list agents")`
`116`	`117`	`}`
`117`	`118`
`118`		`- if statusCode != 200 {`
`119`		`- return nil, fmt.Errorf("could not list agents; API status code = %d", statusCode)`
	`119`	`+ if statusCode != http.StatusOK {`
	`120`	`+ return nil, fmt.Errorf("could not list agents; API status code = %d; response body = %s", statusCode, respBody)`
`120`	`121`	`}`
`121`	`122`
`122`	`123`	`var resp struct {`
Original file line number	Diff line number	Diff line change
`@@ -35,13 +35,13 @@ func (c *Client) Export(dashboardIDs []string) ([]common.MapStr, error) {`
`35`	`35`	`path := fmt.Sprintf("%s/dashboards/export%s", CoreAPI, query.String())`
`36`	`36`	`statusCode, respBody, err := c.get(path)`
`37`	`37`	`if err != nil {`
`38`		`- return nil, errors.Wrapf(err, "could not export dashboards; API status code = %d; response body = %s", statusCode, string(respBody))`
	`38`	`+ return nil, errors.Wrapf(err, "could not export dashboards; API status code = %d; response body = %s", statusCode, respBody)`
`39`	`39`	`}`
`40`	`40`
`41`	`41`	`var exported exportedType`
`42`	`42`	`err = json.Unmarshal(respBody, &exported)`
`43`	`43`	`if err != nil {`
`44`		`- return nil, errors.Wrapf(err, "unmarshalling response failed (body: \n%s)", string(respBody))`
	`44`	`+ return nil, errors.Wrapf(err, "unmarshalling response failed (body: \n%s)", respBody)`
`45`	`45`	`}`
`46`	`46`
`47`	`47`	`var multiErr multierror.Error`
Original file line number	Diff line number	Diff line change
`@@ -43,12 +43,10 @@ func (vv *VarValue) Unpack(value interface{}) error {`
`43`	`43`	`switch u := value.(type) {`
`44`	`44`	`case []interface{}:`
`45`	`45`	`vv.list = u`
`46`		`- return nil`
`47`	`46`	`default:`
`48`	`47`	`vv.scalar = u`
`49`		`- return nil`
`50`	`48`	`}`
`51`		`- return errors.New("unknown variable value")`
	`49`	`+ return nil`
`52`	`50`	`}`
`53`	`51`
`54`	`52`	`// MarshalJSON knows how to serialize a VarValue into the appropriate`
Original file line number	Diff line number	Diff line change
`@@ -7,6 +7,7 @@ package registry`
`7`	`7`	`import (`
`8`	`8`	`"encoding/json"`
`9`	`9`	`"fmt"`
	`10`	`+ "net/http"`
`10`	`11`	`"sort"`
`11`	`12`
`12`	`13`	`"github.com/Masterminds/semver"`
`@@ -44,7 +45,7 @@ func (c *Client) Revisions(packageName string, options SearchOptions) ([]package`
`44`	`45`	`if err != nil {`
`45`	`46`	`return nil, errors.Wrap(err, "could not retrieve package")`
`46`	`47`	`}`
`47`		`- if statusCode != 200 {`
	`48`	`+ if statusCode != http.StatusOK {`
`48`	`49`	`return nil, fmt.Errorf("could not retrieve package; API status code = %d; response body = %s", statusCode, respBody)`
`49`	`50`	`}`
`50`	`51`
Original file line number	Diff line number	Diff line change
`@@ -10,6 +10,7 @@ import (`
`10`	`10`	`"fmt"`
`11`	`11`	`"io"`
`12`	`12`	`"log"`
	`13`	`+ "net/http"`
`13`	`14`	`"os"`
`14`	`15`	`"path/filepath"`
`15`	`16`	`"regexp"`
`@@ -169,7 +170,7 @@ func putIngestPipeline(esClient *elasticsearch.Client, pipeline pipelineResource`
`169`	`170`	`return errors.Wrapf(err, "failed to read PutPipeline API response body (pipelineName: %s)", pipeline.name)`
`170`	`171`	`}`
`171`	`172`
`172`		`- if r.StatusCode != 200 {`
	`173`	`+ if r.StatusCode != http.StatusOK {`
`173`	`174`	`return errors.Wrapf(es.NewError(body), "unexpected response status for PutPipeline (%d): %s (pipelineName: %s)",`
`174`	`175`	`r.StatusCode, r.Status(), pipeline.name)`
`175`	`176`	`}`
`@@ -190,7 +191,7 @@ func getIngestPipeline(esClient *elasticsearch.Client, pipelineName string) erro`
`190`	`191`	`return errors.Wrapf(err, "failed to read GetPipeline API response body (pipelineName: %s)", pipelineName)`
`191`	`192`	`}`
`192`	`193`
`193`		`- if r.StatusCode != 200 {`
	`194`	`+ if r.StatusCode != http.StatusOK {`
`194`	`195`	`return errors.Wrapf(es.NewError(body), "unexpected response status for GetPipeline (%d): %s (pipelineName: %s)",`
`195`	`196`	`r.StatusCode, r.Status(), pipelineName)`
`196`	`197`	`}`
`@@ -237,7 +238,7 @@ func simulatePipelineProcessing(esClient *elasticsearch.Client, pipelineName str`
`237`	`238`	`return nil, errors.Wrap(err, "failed to read Simulate API response body")`
`238`	`239`	`}`
`239`	`240`
`240`		`- if r.StatusCode != 200 {`
	`241`	`+ if r.StatusCode != http.StatusOK {`
`241`	`242`	`return nil, errors.Wrapf(es.NewError(body), "unexpected response status for Simulate (%d): %s", r.StatusCode, r.Status())`
`242`	`243`	`}`
`243`	`244`