diff --git a/.golangci.yaml b/.golangci.yaml index 3681a4a28..9fd64dddf 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -1,92 +1,83 @@ +version: '2' run: build-tags: - integration concurrency: 4 issues-exit-code: 1 - skip-dirs: [] tests: true - timeout: 5m - -linters-settings: - errcheck: - check-blank: true - check-type-assertions: true - exhaustive: - default-signifies-exhaustive: true - goconst: - ignore-calls: false - gocritic: - enabled-tags: - - diagnostic - - experimental - - opinionated - - performance - - style - gocyclo: - min-complexity: 15 - gofumpt: - module-path: github.com/snyk/go-application-framework - extra-rules: true - goimports: - local-prefixes: github.com/snyk/go-application-framework - gosimple: - checks: ['all'] - govet: - enable-all: true - disable: - - fieldalignment - lll: - line-length: 160 - misspell: - locale: US - nolintlint: - allow-unused: false - require-explanation: true - require-specific: true - prealloc: - simple: true - range-loops: true - for-loops: true - promlinter: - strict: true - revive: - rules: - - name: blank-imports - disabled: true - - name: context-as-argument - - name: context-keys-type - - name: dot-imports - - name: error-return - - name: error-strings - - name: error-naming - - name: exported - disable-stuttering-check: true - - name: if-return - - name: increment-decrement - - name: var-naming - - name: var-declaration - disabled: true - - name: package-comments - - name: range - - name: receiver-naming - - name: time-naming - - name: unexported-return - - name: indent-error-flow - - name: errorf - - name: empty-block - - name: superfluous-else - - name: unused-parameter - - name: unreachable-code - - name: redefines-builtin-id - staticcheck: - checks: ['all'] - stylecheck: - checks: ['all'] - http-status-code-whitelist: [] - varcheck: - exported-fields: true - linters: + settings: + errcheck: + check-blank: true + check-type-assertions: true + exhaustive: + default-signifies-exhaustive: true + goconst: + ignore-calls: false + gocritic: + enabled-tags: + - diagnostic + - experimental + - opinionated + - performance + - style + gocyclo: + min-complexity: 15 + govet: + enable-all: true + disable: + - fieldalignment + lll: + line-length: 160 + misspell: + locale: US + nolintlint: + allow-unused: false + require-explanation: true + require-specific: true + prealloc: + simple: true + range-loops: true + for-loops: true + promlinter: + strict: true + revive: + rules: + - name: blank-imports + disabled: true + - name: context-as-argument + - name: context-keys-type + - name: dot-imports + - name: error-return + - name: error-strings + - name: error-naming + - name: exported + - name: if-return + - name: increment-decrement + - name: var-naming + - name: var-declaration + disabled: true + - name: package-comments + - name: range + - name: receiver-naming + - name: time-naming + - name: unexported-return + - name: indent-error-flow + - name: errorf + - name: empty-block + - name: superfluous-else + - name: unused-parameter + - name: unreachable-code + - name: redefines-builtin-id + staticcheck: + checks: + - all + - -ST1000 # Disable package comment enforcement + - -ST1003 # Disable naming convention checks (e.g. myId -> myID) + - -SA1019 # Disable deprecated usage checks + http-status-code-whitelist: [] # Flag ALL hard-coded HTTP status codes + unused: + exported-fields-are-used: false # Check unused exported fields enable: - asasalint - asciicheck @@ -111,10 +102,6 @@ linters: - gocyclo # TODO(godot): revisit #- godot - # TODO(gofumpt): revisit - #- gofumpt - # TODO(goimports): revisit - #- goimports - goprintffuncname # TODO(gosec): revisit; consequences of revoking non-owner file permissions? #- gosec @@ -141,8 +128,7 @@ linters: #- revive - rowserrcheck - sqlclosecheck - # TODO(stylecheck): revisit in a followup; some breaking API changes - #- stylecheck + - staticcheck # NOTE: removed tagliatelle as it conflicts too much with existing API wireformats # - TODO(usetesting) # TODO(testpackage): improve open vs closed-box testing in a followup @@ -151,26 +137,33 @@ linters: - tparallel - unconvert - unparam + - unused - usestdlibvars - wastedassign - whitespace # TODO(wrapcheck): wrap errors in a followup #- wrapcheck - -issues: - exclude-rules: - - linters: - - staticcheck - text: "SA1019" - - path: _test\.go - linters: - - bodyclose - - forcetypeassert - - goconst - - ireturn - - path: test/ - linters: - - testpackage - include: - - EXC0012 - - EXC0014 + exclusions: + rules: + - path: _test\.go + linters: + - bodyclose + - forcetypeassert + - goconst + - ireturn + - path: test/ + linters: + - testpackage +formatters: + enable: [] + # TODO(gofumpt): revisit + #- gofumpt + # TODO(goimports): revisit + #- goimports + settings: + gofumpt: + module-path: github.com/snyk/go-application-framework + extra-rules: true + goimports: + local-prefixes: + - github.com/snyk/go-application-framework diff --git a/Makefile b/Makefile index 92a505cf2..309ffbe1f 100644 --- a/Makefile +++ b/Makefile @@ -3,7 +3,10 @@ GOOS = $(shell go env GOOS) GOARCH = $(shell go env GOARCH) GO_BIN := $(shell pwd)/.bin -OVERRIDE_GOCI_LINT_V := v1.64.8 + +GOCI_LINT_V := v2.3.0 +GOCI_LINT_TARGETS := $(GO_BIN)/golangci-lint $(GO_BIN)/.golangci-lint_$(GOCI_LINT_V) + SHELL := env PATH=$(GO_BIN):$(shell go env GOROOT)/bin:$(PATH) $(SHELL) .PHONY: format @@ -12,10 +15,10 @@ format: @gofmt -w -l -e . .PHONY: lint -lint: $(GO_BIN)/golangci-lint +lint: $(GOCI_LINT_TARGETS) @echo "Linting..." @./scripts/lint.sh - $(GO_BIN)/golangci-lint run ./... + $(GO_BIN)/golangci-lint run --timeout=10m ./... .PHONY: build build: @@ -44,10 +47,15 @@ generate: @make format .PHONY: tools -tools: $(GO_BIN)/golangci-lint +tools: $(GOCI_LINT_TARGETS) + +$(GO_BIN): + @mkdir -p $(GO_BIN) -$(GO_BIN)/golangci-lint: - curl -sSfL 'https://raw.githubusercontent.com/golangci/golangci-lint/${OVERRIDE_GOCI_LINT_V}/install.sh' | sh -s -- -b ${GO_BIN} ${OVERRIDE_GOCI_LINT_V} +$(GOCI_LINT_TARGETS): | $(GO_BIN) + @rm -f $(GO_BIN)/.golangci-lint_* + @curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/$(GOCI_LINT_V)/install.sh | sh -s -- -b $(GO_BIN) $(GOCI_LINT_V) + @touch $(GO_BIN)/.golangci-lint_$(GOCI_LINT_V) .PHONY: update-dragonfly update-dragonfly: diff --git a/internal/api/api.go b/internal/api/api.go index 97ddf5e6c..d0ba10c01 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -184,8 +184,7 @@ func (a *snykApiClient) GetFeatureFlag(flagname string, orgId string) (bool, err if err != nil { return defaultResult, fmt.Errorf("unable to retrieve feature flag: %w", err) } - //goland:noinspection GoUnhandledErrorResult - defer res.Body.Close() + defer func() { _ = res.Body.Close() }() //nolint:errcheck // Ignore lack of error handling body, err := io.ReadAll(res.Body) if err != nil { @@ -232,8 +231,7 @@ func (a *snykApiClient) GetSastSettings(orgId string) (*sast_contract.SastRespon if err != nil { return nil, fmt.Errorf("unable to retrieve settings: %w", err) } - //goland:noinspection GoUnhandledErrorResult - defer res.Body.Close() + defer func() { _ = res.Body.Close() }() //nolint:errcheck // Ignore lack of error handling body, err := io.ReadAll(res.Body) if err != nil { @@ -255,8 +253,7 @@ func (a *snykApiClient) GetOrgSettings(orgId string) (*contract.OrgSettingsRespo if err != nil { return nil, fmt.Errorf("unable to retrieve org settings: %w", err) } - //goland:noinspection GoUnhandledErrorResult - defer res.Body.Close() + defer func() { _ = res.Body.Close() }() //nolint:errcheck // Ignore lack of error handling body, err := io.ReadAll(res.Body) if err != nil { @@ -291,7 +288,7 @@ func (a *snykApiClient) GetOrgSettings(orgId string) (*contract.OrgSettingsRespo // apiVersion := "2022-01-12" // response, err := clientGet(myApiClient, "/organizations", &apiVersion, "limit", "50") func clientGet(a *snykApiClient, endpoint string, version *string, queryParams ...string) ([]byte, error) { - var apiVersion string = constants.SNYK_DEFAULT_API_VERSION + apiVersion := constants.SNYK_DEFAULT_API_VERSION if version != nil && *version != "" { apiVersion = *version } @@ -316,7 +313,7 @@ func clientGet(a *snykApiClient, endpoint string, version *string, queryParams . return nil, fmt.Errorf("failed to read response body: %w", err) } - defer res.Body.Close() + defer func() { _ = res.Body.Close() }() //nolint:errcheck // Ignore lack of error handling return body, nil } diff --git a/internal/presenters/funcs.go b/internal/presenters/funcs.go index a218115fd..8a2242c8b 100644 --- a/internal/presenters/funcs.go +++ b/internal/presenters/funcs.go @@ -330,7 +330,7 @@ func mustReverse(v interface{}) ([]interface{}, error) { return nl, nil default: - return nil, fmt.Errorf("Cannot find reverse on type %s", tp) + return nil, fmt.Errorf("cannot find reverse on type %s", tp) } } diff --git a/internal/presenters/html.go b/internal/presenters/html.go index d5682f11f..d5d03b135 100644 --- a/internal/presenters/html.go +++ b/internal/presenters/html.go @@ -70,7 +70,7 @@ func (p *HTMLpresenter) Present(htmlString string) (string, error) { if n.Type == html.ElementNode { tagName := n.Data var cssClass string - var hasClassAttribute bool = false + hasClassAttribute := false for _, attr := range n.Attr { if attr.Key == "class" { diff --git a/internal/presenters/jsonwriter.go b/internal/presenters/jsonwriter.go index c2390649d..64ff57df5 100644 --- a/internal/presenters/jsonwriter.go +++ b/internal/presenters/jsonwriter.go @@ -11,9 +11,7 @@ type JsonWriter struct { stripWhiteSpaces bool } -/* - * This Writer can be used to strip away whitespaces from json content to reduce the final size - */ +// NewJsonWriter creates a Writer that can be used to strip away whitespaces from json content to reduce the final size func NewJsonWriter(next io.Writer, stripWhitespaces bool) io.Writer { return &JsonWriter{ next: next, diff --git a/pkg/analytics/analytics.go b/pkg/analytics/analytics.go index 81e58c953..5956817ed 100644 --- a/pkg/analytics/analytics.go +++ b/pkg/analytics/analytics.go @@ -2,8 +2,6 @@ package analytics import ( "bytes" - "github.com/snyk/go-application-framework/pkg/logging" - //nolint:gosec // insecure sha1 used for legacy identifier "crypto/sha1" "encoding/json" @@ -22,6 +20,7 @@ import ( "github.com/snyk/go-application-framework/internal/api" utils2 "github.com/snyk/go-application-framework/internal/utils" + "github.com/snyk/go-application-framework/pkg/logging" ) // Analytics is an interface for managing analytics. @@ -319,7 +318,7 @@ func (a *AnalyticsImpl) SetInstrumentation(ic InstrumentationCollector) { a.instrumentor = ic } -var DisabledInFedrampErr = errors.New("analytics are disabled in FedRAMP environments") //nolint:errname // breaking API change +var DisabledInFedrampErr = errors.New("analytics are disabled in FedRAMP environments") //nolint:errname,staticcheck // breaking API change // This method sanitizes the given content by searching for key-value mappings. It thereby replaces all keys defined in keysToFilter by the replacement string // Supported patterns are: diff --git a/pkg/apiclients/testapi/mock_server_test.go b/pkg/apiclients/testapi/mock_server_test.go index d8fc38d3b..c98bf531e 100644 --- a/pkg/apiclients/testapi/mock_server_test.go +++ b/pkg/apiclients/testapi/mock_server_test.go @@ -109,7 +109,10 @@ func handleCreateTestRequest(t *testing.T, w http.ResponseWriter, r *http.Reques if config.ExpectedCreateTestBody != nil { bodyBytes, bodyErr := io.ReadAll(r.Body) require.NoError(t, bodyErr) - defer r.Body.Close() + defer func() { + closeErr := r.Body.Close() + assert.NoError(t, closeErr) + }() expectedBodyBytes, err := json.Marshal(config.ExpectedCreateTestBody) require.NoError(t, err) assert.JSONEq(t, string(expectedBodyBytes), string(bodyBytes)) diff --git a/pkg/configuration/configuration_test.go b/pkg/configuration/configuration_test.go index c50893cdf..16a8641ce 100644 --- a/pkg/configuration/configuration_test.go +++ b/pkg/configuration/configuration_test.go @@ -13,6 +13,7 @@ import ( "github.com/spf13/pflag" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) const ( @@ -39,15 +40,17 @@ func cleanupConfigstore(t *testing.T) { assert.NoError(t, err) } -func cleanUpEnvVars() { - os.Unsetenv("SNYK_TOKEN") - os.Unsetenv("SNYK_OAUTH_TOKEN") - os.Unsetenv("SNYK_DOCKER_TOKEN") - os.Unsetenv("SNYK_DISABLE_ANALYTICS") +// tUnsetEnv Unset an environment variable, while ensuring it is cleaned up after the test. +func tUnsetEnv(t *testing.T, envVarName string) { + t.Helper() + t.Setenv(envVarName, "") // Ensures automatic cleanup + err := os.Unsetenv(envVarName) + require.NoError(t, err) } func Test_ConfigurationGet_AUTHENTICATION_TOKEN(t *testing.T) { - os.Unsetenv("SNYK_TOKEN") + tUnsetEnv(t, "SNYK_TOKEN") + expectedValue := "mytoken" expectedValue2 := "123456" assert.Nil(t, prepareConfigstore(`{"api": "mytoken", "somethingElse": 12}`)) @@ -66,10 +69,13 @@ func Test_ConfigurationGet_AUTHENTICATION_TOKEN(t *testing.T) { assert.Equal(t, expectedValue2, actualValue) cleanupConfigstore(t) - cleanUpEnvVars() } func Test_ConfigurationGet_AUTHENTICATION_BEARER_TOKEN(t *testing.T) { + tUnsetEnv(t, "SNYK_TOKEN") + tUnsetEnv(t, "SNYK_OAUTH_TOKEN") + tUnsetEnv(t, "SNYK_DOCKER_TOKEN") + expectedValue := "anotherToken" expectedValueDocker := "dockerToken" assert.Nil(t, prepareConfigstore(`{"api": "mytoken", "somethingElse": 12}`)) @@ -93,7 +99,6 @@ func Test_ConfigurationGet_AUTHENTICATION_BEARER_TOKEN(t *testing.T) { }) cleanupConfigstore(t) - cleanUpEnvVars() } func Test_ConfigurationGet_ANALYTICS_DISABLED(t *testing.T) { @@ -113,7 +118,6 @@ func Test_ConfigurationGet_ANALYTICS_DISABLED(t *testing.T) { assert.False(t, actualValue) cleanupConfigstore(t) - cleanUpEnvVars() } func Test_Configuration_GetE(t *testing.T) { @@ -124,7 +128,7 @@ func Test_Configuration_GetE(t *testing.T) { WithSupportedEnvVarPrefixes("snyk_"), ) - _ = os.Unsetenv(ANALYTICS_DISABLED) + tUnsetEnv(t, "SNYK_DISABLE_ANALYTICS") actualValue, err := config.GetWithError(ANALYTICS_DISABLED) assert.Nil(t, err) @@ -143,10 +147,11 @@ func Test_Configuration_GetE(t *testing.T) { assert.Equal(t, "0", actualValue) cleanupConfigstore(t) - cleanUpEnvVars() } func Test_ConfigurationGet_ALTERNATE_KEYS(t *testing.T) { + tUnsetEnv(t, "SNYK_TOKEN") + key := "snyk_cfg_api" expected := "value" alternateKeys := []string{"snyk_token", "snyk_cfg_api", "api"} @@ -724,8 +729,6 @@ func Test_Configuration_envVarSupport(t *testing.T) { shouldBeNil := config.Get(invalidKey) assert.Nil(t, shouldBeNil) - - cleanUpEnvVars() }) t.Run("supports a list of env vars", func(t *testing.T) { @@ -790,8 +793,6 @@ func Test_Configuration_envVarSupport(t *testing.T) { shouldBeNil := config.Get(invalidKey) assert.Nil(t, shouldBeNil) - - cleanUpEnvVars() }) t.Run("WithAutomaticEnv takes precedence", func(t *testing.T) { @@ -833,8 +834,6 @@ func Test_Configuration_envVarSupport(t *testing.T) { actualAutoEnvValue := config.Get(autoEnv) assert.Equal(t, autoEnvValue, actualAutoEnvValue) - - cleanUpEnvVars() }) } diff --git a/pkg/envvars/environment.go b/pkg/envvars/environment.go index dfa8ff9ae..df3caea84 100644 --- a/pkg/envvars/environment.go +++ b/pkg/envvars/environment.go @@ -123,7 +123,7 @@ func UpdatePath(pathExtension string, prepend bool) string { } if currentPath == "" { - _ = os.Setenv(PathEnvVarName, pathExtension) + _ = os.Setenv(PathEnvVarName, pathExtension) //nolint:errcheck // Ignore lack of error handling return pathExtension } @@ -140,6 +140,6 @@ func UpdatePath(pathExtension string, prepend bool) string { newPathSlice := utils.Dedupe(combinedSliceWithDuplicates) newPath := strings.Join(newPathSlice, string(os.PathListSeparator)) - _ = os.Setenv(PathEnvVarName, newPath) + _ = os.Setenv(PathEnvVarName, newPath) //nolint:errcheck // Ignore lack of error handling return newPath } diff --git a/pkg/instrumentation/get-target-id.go b/pkg/instrumentation/get-target-id.go index f5220d69e..f4e62cd4d 100644 --- a/pkg/instrumentation/get-target-id.go +++ b/pkg/instrumentation/get-target-id.go @@ -144,9 +144,9 @@ func emptyTargetId() *url.URL { func gitBaseIdFromRemote(repoUrl string) (string, error) { if strings.HasPrefix(repoUrl, "git@") { - formattedString := strings.Replace(repoUrl, "@", "/", -1) - formattedString = strings.Replace(formattedString, ":", "/", -1) - formattedString = strings.Replace(formattedString, ".git", "", -1) + formattedString := strings.ReplaceAll(repoUrl, "@", "/") + formattedString = strings.ReplaceAll(formattedString, ":", "/") + formattedString = strings.ReplaceAll(formattedString, ".git", "") return formattedString, nil } diff --git a/pkg/instrumentation/get-target-id_test.go b/pkg/instrumentation/get-target-id_test.go index dfaebcab3..85363c494 100644 --- a/pkg/instrumentation/get-target-id_test.go +++ b/pkg/instrumentation/get-target-id_test.go @@ -196,7 +196,10 @@ func updateFile(t *testing.T, filePath, target, replacement string) error { t.Helper() file, err := os.Open(filePath) assert.NoError(t, err) - defer func() { _ = file.Close() }() + defer func() { + closeErr := file.Close() + assert.NoError(t, closeErr) + }() scanner := bufio.NewScanner(file) var updatedLines []string @@ -204,7 +207,7 @@ func updateFile(t *testing.T, filePath, target, replacement string) error { for scanner.Scan() { line := scanner.Text() if strings.Contains(line, target) { - newLine := strings.Replace(line, target, replacement, -1) + newLine := strings.ReplaceAll(line, target, replacement) updatedLines = append(updatedLines, newLine) } else { updatedLines = append(updatedLines, line) @@ -221,7 +224,10 @@ func updateFile(t *testing.T, filePath, target, replacement string) error { if err != nil { return err } - defer newFile.Close() + defer func() { + closeErr := newFile.Close() + assert.NoError(t, closeErr) + }() writer := bufio.NewWriter(newFile) for _, line := range updatedLines { diff --git a/pkg/local_workflows/auth_workflow.go b/pkg/local_workflows/auth_workflow.go index b49abdfb5..49a2e68bd 100644 --- a/pkg/local_workflows/auth_workflow.go +++ b/pkg/local_workflows/auth_workflow.go @@ -32,7 +32,7 @@ If you can't wait use this url: %s ` -// define a new workflow identifier for this workflow +// WORKFLOWID_AUTH defines a new workflow identifier for this workflow var WORKFLOWID_AUTH workflow.Identifier = workflow.NewWorkflowIdentifier(workflowNameAuth) var ConfigurationNewAuthenticationToken = "internal_new_snyk_token" diff --git a/pkg/local_workflows/code_workflow/code_client_helper.go b/pkg/local_workflows/code_workflow/code_client_helper.go index aaf59e7e8..d3f043a7b 100644 --- a/pkg/local_workflows/code_workflow/code_client_helper.go +++ b/pkg/local_workflows/code_workflow/code_client_helper.go @@ -23,7 +23,7 @@ func (c *codeClientConfig) IsFedramp() bool { } func (c *codeClientConfig) SnykCodeApi() string { - return strings.Replace(c.localConfiguration.GetString(configuration.API_URL), "api", "deeproxy", -1) + return strings.ReplaceAll(c.localConfiguration.GetString(configuration.API_URL), "api", "deeproxy") } func (c *codeClientConfig) SnykApi() string { diff --git a/pkg/local_workflows/code_workflow/native_workflow.go b/pkg/local_workflows/code_workflow/native_workflow.go index 3e0173724..d861af200 100644 --- a/pkg/local_workflows/code_workflow/native_workflow.go +++ b/pkg/local_workflows/code_workflow/native_workflow.go @@ -102,7 +102,7 @@ func trackUsage(network networking.NetworkAccess, config configuration.Configura return } - resp.Body.Close() + _ = resp.Body.Close() //nolint:errcheck // Ignore lack of error handling } func EntryPointNative(invocationCtx workflow.InvocationContext, opts ...OptionalAnalysisFunctions) ([]workflow.Data, error) { diff --git a/pkg/local_workflows/code_workflow_test.go b/pkg/local_workflows/code_workflow_test.go index c84a03f6a..e5c3bf499 100644 --- a/pkg/local_workflows/code_workflow_test.go +++ b/pkg/local_workflows/code_workflow_test.go @@ -364,7 +364,8 @@ func Test_Code_FF_CODE_CONSISTENT_IGNORES(t *testing.T) { } assert.NoError(t, err) - fmt.Fprintln(w, string(data)) + _, err = fmt.Fprintln(w, string(data)) + assert.NoError(t, err) })) defer ts.Close() diff --git a/pkg/local_workflows/connectivity_check_extension/connectivity/checker.go b/pkg/local_workflows/connectivity_check_extension/connectivity/checker.go index 0e0e52a0d..bad66a6bb 100644 --- a/pkg/local_workflows/connectivity_check_extension/connectivity/checker.go +++ b/pkg/local_workflows/connectivity_check_extension/connectivity/checker.go @@ -287,7 +287,7 @@ func (c *Checker) checkHost(host string) HostResult { result.Status = c.categorizeError(err) return result } - defer resp.Body.Close() + defer func() { _ = resp.Body.Close() }() //nolint:errcheck // Ignore lack of error handling result.StatusCode = resp.StatusCode diff --git a/pkg/local_workflows/connectivity_check_extension/connectivity/formatter.go b/pkg/local_workflows/connectivity_check_extension/connectivity/formatter.go index a98f52138..abafd2cdb 100644 --- a/pkg/local_workflows/connectivity_check_extension/connectivity/formatter.go +++ b/pkg/local_workflows/connectivity_check_extension/connectivity/formatter.go @@ -19,7 +19,7 @@ type Formatter struct { // output is a helper method that writes to the writer func (f *Formatter) output(str string) { - fmt.Fprintln(f.writer, str) + _, _ = fmt.Fprintln(f.writer, str) //nolint:errcheck // Ignore lack of error handling } // NewFormatter creates a new formatter using an io.Writer diff --git a/pkg/local_workflows/connectivity_check_extension/connectivity/types.go b/pkg/local_workflows/connectivity_check_extension/connectivity/types.go index 67db9de1c..4556befc6 100644 --- a/pkg/local_workflows/connectivity_check_extension/connectivity/types.go +++ b/pkg/local_workflows/connectivity_check_extension/connectivity/types.go @@ -107,7 +107,6 @@ type ConnectivityCheckResult struct { OrgCheckError error `json:"orgCheckError,omitempty"` } -// AddTODO adds a new TODO item to the result // AddTODOf adds a TODO item to the result with printf-style formatting func (r *ConnectivityCheckResult) AddTODOf(level TodoLevel, format string, args ...interface{}) { r.TODOs = append(r.TODOs, TODO{ diff --git a/pkg/local_workflows/connectivity_check_extension/connectivity_check_workflow.go b/pkg/local_workflows/connectivity_check_extension/connectivity_check_workflow.go index 3ce13b24b..9816caadd 100644 --- a/pkg/local_workflows/connectivity_check_extension/connectivity_check_workflow.go +++ b/pkg/local_workflows/connectivity_check_extension/connectivity_check_workflow.go @@ -24,7 +24,7 @@ const ( maxOrgCountFlag = "max-org-count" ) -// Define workflow identifier +// WORKFLOWID_CONNECTIVITY_CHECK defines workflow identifier var WORKFLOWID_CONNECTIVITY_CHECK workflow.Identifier = workflow.NewWorkflowIdentifier(connectivityCheckWorkflowName) // InitConnectivityCheckWorkflow initializes the connectivity check workflow diff --git a/pkg/local_workflows/filter_workflow_test.go b/pkg/local_workflows/filter_workflow_test.go index 5416c9ba8..97691b5f9 100644 --- a/pkg/local_workflows/filter_workflow_test.go +++ b/pkg/local_workflows/filter_workflow_test.go @@ -99,7 +99,7 @@ func TestFilterFindingsEntryPoint(t *testing.T) { t.Run("with invalid severity threshold", func(t *testing.T) { ctx := setupMockFilterContext(t, "invalid") - var findings local_models.LocalFinding = local_models.LocalFinding{} + findings := local_models.LocalFinding{} findingsBytes, err := json.Marshal(findings) assert.NoError(t, err) input := []workflow.Data{workflow.NewData( diff --git a/pkg/local_workflows/json_schemas/analytics_v2.go b/pkg/local_workflows/json_schemas/analytics_v2.go index 0b40c7689..16050736c 100644 --- a/pkg/local_workflows/json_schemas/analytics_v2.go +++ b/pkg/local_workflows/json_schemas/analytics_v2.go @@ -1,6 +1,7 @@ package json_schemas // TODO: this should be dynamically fetched if possible; it is owned by Analytics API not by GAF + const AnalyticsV2EventSchema = `{ "$schema": "http://json-schema.org/draft-04/schema#", "type": "object", diff --git a/pkg/local_workflows/local_models/transform.go b/pkg/local_workflows/local_models/transform.go index fce6514e6..c04a20a47 100644 --- a/pkg/local_workflows/local_models/transform.go +++ b/pkg/local_workflows/local_models/transform.go @@ -247,7 +247,7 @@ func mapLocations(res sarif.Result) []IoSnykReactiveFindingLocation { return locations } -// updateFindingsSummary updates the summary of the findings based on their severity levels +// UpdateFindingSummary updates the summary of the findings based on their severity levels func UpdateFindingSummary(findingsModel *LocalFinding) { updatedFindingCounts := NewFindingsCounts() updatedFindingCounts.CountKeyOrderAsc = findingsModel.Summary.Counts.CountKeyOrderAsc diff --git a/pkg/local_workflows/local_models/type.go b/pkg/local_workflows/local_models/type.go index 61f893dee..046216a7d 100644 --- a/pkg/local_workflows/local_models/type.go +++ b/pkg/local_workflows/local_models/type.go @@ -4,6 +4,7 @@ type FindingResource TypesFindingResource type TestOutcome TypesTestOutcome // TODO: This schema should be imported from Dragonfly + type LocalFinding struct { Findings []FindingResource `json:"findings"` Outcome TestOutcome `json:"outcome"` diff --git a/pkg/local_workflows/output_workflow.go b/pkg/local_workflows/output_workflow.go index 006f8c8bf..5fb10d4c8 100644 --- a/pkg/local_workflows/output_workflow.go +++ b/pkg/local_workflows/output_workflow.go @@ -144,8 +144,8 @@ func handleContentTypeOthers(input []workflow.Data, i int, mimeType string, outp singleDataAsString = string(singleData) } - outputDestination.Println(singleDataAsString) - return nil + _, err := outputDestination.Println(singleDataAsString) + return err } func handleContentTypeJson(config configuration.Configuration, input []workflow.Data, i int, outputDestination iUtils.OutputDestination, debugLogger *zerolog.Logger) error { @@ -168,7 +168,10 @@ func handleContentTypeJson(config configuration.Configuration, input []workflow. } if printJsonToCmd { - outputDestination.Println(string(singleData)) + _, err := outputDestination.Println(string(singleData)) + if err != nil { + return err + } } if writeToFile { diff --git a/pkg/local_workflows/report_analytics_workflow.go b/pkg/local_workflows/report_analytics_workflow.go index d1f3642fd..ab9ededa4 100644 --- a/pkg/local_workflows/report_analytics_workflow.go +++ b/pkg/local_workflows/report_analytics_workflow.go @@ -162,7 +162,7 @@ func callEndpoint(invocationCtx workflow.InvocationContext, input workflow.Data, if err != nil { return fmt.Errorf("error sending request: %w", err) } - defer resp.Body.Close() + defer func() { _ = resp.Body.Close() }() //nolint:errcheck // Ignore lack of error handling if resp.StatusCode != http.StatusCreated { return fmt.Errorf("error sending request: %v", resp.Status) diff --git a/pkg/local_workflows/whoami_workflow.go b/pkg/local_workflows/whoami_workflow.go index 03bb98a4b..0684ab054 100644 --- a/pkg/local_workflows/whoami_workflow.go +++ b/pkg/local_workflows/whoami_workflow.go @@ -20,7 +20,7 @@ const ( jsonFlag = "json" ) -// define a new workflow identifier for this workflow +// WORKFLOWID_WHOAMI defines a new workflow identifier for this workflow var WORKFLOWID_WHOAMI workflow.Identifier = workflow.NewWorkflowIdentifier(whoAmIworkflowName) // InitWhoAmIWorkflow initializes the whoAmI workflow before registering it with the engine. diff --git a/pkg/local_workflows/whoami_workflow_test.go b/pkg/local_workflows/whoami_workflow_test.go index 3239b1f3a..d11153fbe 100644 --- a/pkg/local_workflows/whoami_workflow_test.go +++ b/pkg/local_workflows/whoami_workflow_test.go @@ -89,7 +89,7 @@ func setupMockContext(t *testing.T, payload string, experimental bool, json bool networkAccessMock := mocks.NewMockNetworkAccess(ctrl) invocationContextMock := mocks.NewMockInvocationContext(ctrl) - var httpClient *http.Client = http.DefaultClient + httpClient := http.DefaultClient if mockClient { httpClient = NewTestClient(func(req *http.Request) *http.Response { diff --git a/pkg/logging/scrubbingLogWriter.go b/pkg/logging/scrubbingLogWriter.go index 1fbfd4602..0a5f1f122 100644 --- a/pkg/logging/scrubbingLogWriter.go +++ b/pkg/logging/scrubbingLogWriter.go @@ -338,7 +338,7 @@ func scrub(p []byte, scrubDict ScrubbingDict) []byte { if entry.groupToRedact >= len(match) || match[entry.groupToRedact] == "" { continue } - s = strings.Replace(s, match[entry.groupToRedact], SANITIZE_REPLACEMENT_STRING, -1) + s = strings.ReplaceAll(s, match[entry.groupToRedact], SANITIZE_REPLACEMENT_STRING) } } return []byte(s) diff --git a/pkg/networking/certs/certs.go b/pkg/networking/certs/certs.go index d8a712b0f..396d3ebfd 100644 --- a/pkg/networking/certs/certs.go +++ b/pkg/networking/certs/certs.go @@ -71,7 +71,7 @@ func MakeSelfSignedCert(certName string, dnsNames []string, debugLogger *log.Log return certPEMBlockBytes, keyPEMBlockBytes, nil } -// Append all certificates specified via extraCertificateLocation to the given PEM formatted byte slice +// AppendExtraCaCert appends all certificates specified via extraCertificateLocation to the given PEM formatted byte slice func AppendExtraCaCert(extraCertificateLocation string, certPem []byte) []byte { // try to access certificate file provided via the environment variable extraCertificateLocation extraCertificateBytes, _, extraCertificateError := GetExtraCaCert(extraCertificateLocation) @@ -82,7 +82,7 @@ func AppendExtraCaCert(extraCertificateLocation string, certPem []byte) []byte { return certPem } -// Returns the Certificates specified via extraCertificateLocation as a PEM formatted byte slice and as a list of certificates. +// GetExtraCaCert returns the Certificates specified via extraCertificateLocation as a PEM formatted byte slice and as a list of certificates. func GetExtraCaCert(extraCertificateLocation string) ([]byte, []*x509.Certificate, error) { var resultAsByte []byte var err error @@ -107,7 +107,7 @@ func GetExtraCaCert(extraCertificateLocation string) ([]byte, []*x509.Certificat return resultAsByte, resultAsCert, err } -// Decode all Certifactes given in the PEM formatted input slice and return the Certificates as a list. It returns an error if any of the content is not a Certificate. +// GetAllCerts decodes all Certifactes given in the PEM formatted input slice and returns the Certificates as a list. It returns an error if any of the content is not a Certificate. func GetAllCerts(pemData []byte) ([]*x509.Certificate, error) { var result []*x509.Certificate for len(pemData) > 0 { @@ -135,7 +135,7 @@ func GetAllCerts(pemData []byte) ([]*x509.Certificate, error) { return result, nil } -// Get global Certificate pool including x509.SystemCertPool() + extraCertificateLocation +// AddCertificatesToPool gets global Certificate pool including x509.SystemCertPool() + extraCertificateLocation func AddCertificatesToPool(pool *x509.CertPool, extraCertificateLocation string) error { _, extracCertList, err := GetExtraCaCert(extraCertificateLocation) if err != nil { diff --git a/pkg/networking/certs/certs_test.go b/pkg/networking/certs/certs_test.go index 579588a75..13126d2da 100644 --- a/pkg/networking/certs/certs_test.go +++ b/pkg/networking/certs/certs_test.go @@ -25,7 +25,10 @@ func Test_GetExtraCaCert_InvalidPathSpecified(t *testing.T) { func Test_GetExtraCaCert_InvalidCertSpecified(t *testing.T) { file, err := os.CreateTemp(t.TempDir(), "") - defer func() { _ = file.Close() }() + defer func() { + closeErr := file.Close() + assert.NoError(t, closeErr) + }() assert.NoError(t, err) _, err = file.Write([]byte{'h', 'e', 'l', 'l', 'o'}) assert.NoError(t, err) @@ -42,7 +45,10 @@ func Test_GetExtraCaCert_CertSpecified(t *testing.T) { certPem, _, err := MakeSelfSignedCert("mycert", []string{"dns"}, logger) assert.NoError(t, err) file, err := os.CreateTemp(t.TempDir(), "") - defer func() { _ = file.Close() }() + defer func() { + closeErr := file.Close() + assert.NoError(t, closeErr) + }() assert.NoError(t, err) _, err = file.Write(certPem) assert.NoError(t, err) @@ -60,7 +66,10 @@ func Test_AppendExtraCaCert_AddOneCert(t *testing.T) { certPem, _, err := MakeSelfSignedCert("mycert", []string{"dns"}, logger) assert.NoError(t, err) file, err := os.CreateTemp(t.TempDir(), "") - defer func() { _ = file.Close() }() + defer func() { + closeErr := file.Close() + assert.NoError(t, closeErr) + }() assert.NoError(t, err) _, err = file.Write(extraCertPem) assert.NoError(t, err) @@ -78,7 +87,10 @@ func Test_AppendExtraCaCert_AddNoCert(t *testing.T) { certPem, _, err := MakeSelfSignedCert("mycert", []string{"dns"}, logger) assert.NoError(t, err) file, err := os.CreateTemp(t.TempDir(), "") - defer func() { _ = file.Close() }() + defer func() { + closeErr := file.Close() + assert.NoError(t, closeErr) + }() assert.NoError(t, err) _, err = file.Write([]byte(extraCertPem)) assert.NoError(t, err) diff --git a/pkg/networking/fips_enable/non_fips_init.go b/pkg/networking/fips_enable/non_fips_init.go index 5742f5e04..a919a2ee1 100644 --- a/pkg/networking/fips_enable/non_fips_init.go +++ b/pkg/networking/fips_enable/non_fips_init.go @@ -8,5 +8,5 @@ import "os" func init() { existingValue := os.Getenv(godebugEnvVarName) existingValue = setFipState(existingValue, false) - os.Setenv(godebugEnvVarName, existingValue) + _ = os.Setenv(godebugEnvVarName, existingValue) //nolint:errcheck // Ignore lack of error handling } diff --git a/pkg/networking/logging.go b/pkg/networking/logging.go index 5564c84bf..620671240 100644 --- a/pkg/networking/logging.go +++ b/pkg/networking/logging.go @@ -40,7 +40,7 @@ func getResponseBody(response *http.Response) io.ReadCloser { if response.Body != nil { bodyBytes, bodyErr := io.ReadAll(response.Body) if bodyErr == nil { - response.Body.Close() + _ = response.Body.Close() //nolint:errcheck // Ignore lack of error handling bodyReader := io.NopCloser(bytes.NewBuffer(bodyBytes)) response.Body = io.NopCloser(bytes.NewBuffer(bodyBytes)) return bodyReader @@ -61,7 +61,7 @@ func getRequestBody(request *http.Request) io.ReadCloser { if request.Body != nil { bodyBytes, bodyErr := io.ReadAll(request.Body) if bodyErr == nil { - request.Body.Close() + _ = request.Body.Close() //nolint:errcheck // Ignore lack of error handling bodyReader := io.NopCloser(bytes.NewBuffer(bodyBytes)) request.Body = io.NopCloser(bytes.NewBuffer(bodyBytes)) return bodyReader @@ -77,7 +77,7 @@ func decodeBody(bodyBytes []byte, contentEncoding string) (string, error) { if err != nil { return "", errors.Wrap(err, "failed to create gzip reader") } - defer reader.Close() + defer func() { _ = reader.Close() }() //nolint:errcheck // Ignore lack of error handling decodedBytes, err := io.ReadAll(reader) if err != nil { return "", errors.Wrap(err, "failed to read decoded body") @@ -163,7 +163,7 @@ func LogResponse(response *http.Response, logger *zerolog.Logger) { logger.WithLevel(defaultNetworkLogLevel).Msgf("%s header: %v", logPrefixResponse, response.Header) // additional logs for trace level logging and error responses - if !(response.StatusCode >= 400 || !shouldNotLog(logger.GetLevel(), extendedNetworkLogLevel)) { + if response.StatusCode < 400 && shouldNotLog(logger.GetLevel(), extendedNetworkLogLevel) { return } diff --git a/pkg/networking/networking_test.go b/pkg/networking/networking_test.go index 08eefa090..4d7f84591 100644 --- a/pkg/networking/networking_test.go +++ b/pkg/networking/networking_test.go @@ -231,7 +231,8 @@ func Test_GetHTTPClient_EmptyCAs(t *testing.T) { listener, err := net.Listen("tcp", "127.0.0.1:0") assert.NoError(t, err) t.Cleanup(func() { - listener.Close() + closeErr := listener.Close() + assert.NoError(t, closeErr) }) serverURL := fmt.Sprintf("https://localhost:%d/", listener.Addr().(*net.TCPAddr).Port) //nolint:errcheck //in this test, the type is clear diff --git a/pkg/networking/useragent.go b/pkg/networking/useragent.go index cd87af946..2e26fafc3 100644 --- a/pkg/networking/useragent.go +++ b/pkg/networking/useragent.go @@ -72,7 +72,7 @@ func UserAgent(opts ...UserAgentOptions) UserAgentInfo { return ua } -// ToUserAgentHeader returns a string that can be used as a User-Agent header. +// String returns a string that can be used as a User-Agent header. // The string is following this format: // / (;;) / (/) // Everything other than the app, app version and system information (os/arch/process name) is optional. diff --git a/pkg/ui/consoleui_test.go b/pkg/ui/consoleui_test.go index 082f79c25..893a425a1 100644 --- a/pkg/ui/consoleui_test.go +++ b/pkg/ui/consoleui_test.go @@ -32,7 +32,7 @@ func Test_ProgressBar_Spinner(t *testing.T) { err = bar.UpdateProgress(1.5) assert.Error(t, err) - expected := "\r\\ 0% Hello\r| 30% Hello\r/ 100% Hello\r\u001B[K" + expected := "\r\x1b[K\\ 0% Hello\r\x1b[K| 30% Hello\r\x1b[K/ 100% Hello\r\u001B[K" assert.Equal(t, expected, writer.String()) } @@ -56,7 +56,7 @@ func Test_ProgressBar_Spinner_Infinite(t *testing.T) { err = bar.UpdateProgress(1.5) assert.Error(t, err) - expected := "\r\\ Hello\r| Hello\r/ Hello\r\u001B[K" + expected := "\r\x1b[K\\ Hello\r\x1b[K| Hello\r\x1b[K/ Hello\r\u001B[K" assert.Equal(t, expected, writer.String()) } diff --git a/pkg/ui/progressbar.go b/pkg/ui/progressbar.go index 2f9cd1205..7bf47f657 100644 --- a/pkg/ui/progressbar.go +++ b/pkg/ui/progressbar.go @@ -127,11 +127,7 @@ func (p *consoleProgressBar) update() { } var err error - for { - if !p.active.Load() { - break - } - + for p.active.Load() { var progressString string progress := *p.progress.Load() if progress >= 0 { @@ -145,11 +141,12 @@ func (p *consoleProgressBar) update() { p.state++ - if p.progressType == SpinnerType { + switch p.progressType { + case SpinnerType: err = p.renderSpinner(progressString) - } else if p.progressType == BarType { + case BarType: err = p.renderBar(progressString) - } else { + default: err = p.renderText(progressString) } diff --git a/pkg/utils/array.go b/pkg/utils/array.go index 7d54b0b8f..24fb768cd 100644 --- a/pkg/utils/array.go +++ b/pkg/utils/array.go @@ -138,7 +138,7 @@ func ToSlice(input map[string]string, combineBy string) []string { return result } -// Removes a given key from the input map and uses FindKeyCaseInsensitive() for this. The resulting map is being returned. +// Remove removes a given key from the input map and uses FindKeyCaseInsensitive() for this. The resulting map is being returned. // If the key was not found, the input map will be returned. // // Example: @@ -155,7 +155,7 @@ func Remove(input map[string]string, key string) map[string]string { return input } -// This method tries to find the given key is in the map. It searches different cases of the key: +// FindKeyCaseInsensitive tries to find the given key is in the map. It searches different cases of the key: // // 1. the exact match // 2. all lower case letters @@ -187,7 +187,7 @@ func FindKeyCaseInsensitive(input map[string]string, key string) (string, bool) return key, found } -// This method tries to find the given key is in the map and return its value. It searches different cases of the key: +// FindValueCaseInsensitive tries to find the given key is in the map and return its value. It searches different cases of the key: // // 1. the exact match // 2. all lower case letters diff --git a/pkg/utils/directories.go b/pkg/utils/directories.go index 3a367e0a4..96c857228 100644 --- a/pkg/utils/directories.go +++ b/pkg/utils/directories.go @@ -8,7 +8,7 @@ import ( "github.com/pkg/errors" ) -// 0755 is the default permission for directories, it means the owner can read, write, and execute, +// DIR_PERMISSION is 0755 which is the default permission for directories, it means the owner can read, write, and execute, // and everyone else can read and execute but not write. const DIR_PERMISSION = 0755 diff --git a/pkg/utils/sarif/sarif.go b/pkg/utils/sarif/sarif.go index f46576f38..d208f40e1 100644 --- a/pkg/utils/sarif/sarif.go +++ b/pkg/utils/sarif/sarif.go @@ -10,34 +10,31 @@ const ( summaryType = "sast" ) -// Convert Sarif Level to internal Severity +// SarifLevelToSeverity converts Sarif Level to internal Severity func SarifLevelToSeverity(level string) string { - var severity string - if level == "note" { - severity = "low" - } else if level == "warning" { - severity = "medium" - } else if level == "error" { - severity = "high" - } else { - severity = "unmapped" + switch level { + case "note": + return "low" + case "warning": + return "medium" + case "error": + return "high" + default: + return "unmapped" } - - return severity } func SeverityToSarifLevel(severity string) string { - var level string - if severity == "low" { - level = "note" - } else if severity == "medium" { - level = "warning" - } else if severity == "high" || severity == "critical" { - level = "error" - } else { - level = "unmapped" + switch severity { + case "low": + return "note" + case "medium": + return "warning" + case "high", "critical": + return "error" + default: + return "unmapped" } - return level } // CreateCodeSummary Iterates through the sarif data and create a summary out of it. diff --git a/pkg/workflow/types.go b/pkg/workflow/types.go index 5a70fe17d..42129ea34 100644 --- a/pkg/workflow/types.go +++ b/pkg/workflow/types.go @@ -5,6 +5,7 @@ import ( "net/url" "github.com/rs/zerolog" + "github.com/snyk/error-catalog-golang-public/snyk_errors" "github.com/snyk/go-application-framework/pkg/analytics" @@ -17,6 +18,7 @@ import ( //go:generate go tool github.com/golang/mock/mockgen -source=types.go -destination ../mocks/workflow.go -package mocks -self_package github.com/snyk/go-application-framework/pkg/workflow/ // typedefs + type Identifier = *url.URL type Callback func(invocation InvocationContext, input []Data) ([]Data, error) type ExtensionInit func(engine Engine) error