Merge pull request #984 from fredbi/chore/introduce-linting

Chore/introduce linting

.github/workflows/01-golang-lint.yaml

@@ -0,0 +1,54 @@
+name: golangci-lint
+on:
+  push:
+    branches:
+      - dev
+  pull_request:
+    types: [ edited, opened, synchronize, reopened ]
+    branches: [ master, dev ]
+    paths-ignore:
+      - '**.yaml'
+      - '**.md'
+permissions:
+  contents: read
+  # Optional: allow read access to pull request. Use with `only-new-issues` option.
+  pull-requests: read
+jobs:
+  golangci:
+    name: lint
+    runs-on: ubuntu-20.04
+    steps:
+      - uses: actions/setup-go@v3
+        with:
+          go-version: 1.18
+      - uses: actions/checkout@v3
+        with:
+          submodules: recursive
+      - name: Install libgit2
+        run: make libgit2
+      - name: golangci-lint
+        uses: golangci/golangci-lint-action@v3
+        with:
+          # Optional: version of golangci-lint to use in form of v1.2 or v1.2.3 or `latest` to use the latest version
+          version: latest
+
+          # Optional: working directory, useful for monorepos
+          # working-directory: somedir
+
+          # Optional: golangci-lint command line arguments.
+          # args: --issues-exit-code=0
+          args: --timeout 10m --build-tags=static
+          #--new-from-rev dev
+
+          # Optional: show only new issues if it's a pull request. The default value is `false`.
+          only-new-issues: true
+
+          # Optional: if set to true then the all caching functionality will be complete disabled,
+          #           takes precedence over all other caching options.
+          # skip-cache: true
+
+          # Optional: if set to true then the action don't cache or restore ~/go/pkg.
+          # skip-pkg-cache: true
+
+          # Optional: if set to true then the action don't cache or restore ~/.cache/go-build.
+          # skip-build-cache: true

.gitignore

@@ -5,4 +5,5 @@
 *.pyc*
 .idea
 .history
-ca.srl
+ca.srl
+*.out

.golangci.yml

@@ -0,0 +1,58 @@
+linters-settings:
+  govet:
+    check-shadowing: true
+  dupl:
+    threshold: 200
+  goconst:
+    min-len: 3
+    min-occurrences: 2
+  gocognit:
+    min-complexity: 65
+
+linters:
+  enable:
+    - gosec
+    - staticcheck
+    - nolintlint
+  disable:
+    # temporarily disabled
+    - varcheck
+    - ineffassign
+    - unused
+    - typecheck
+    - errcheck
+    - govet
+    - gosimple
+    - deadcode
+    - gofmt
+    - goimports
+    - bodyclose
+    - dupl
+    - gocognit
+    - gocritic
+    - goimports
+    - nakedret
+    - revive
+    - stylecheck
+    - unconvert
+    - unparam
+    #- forbidigo # <- see later
+    # should remain disabled
+    - maligned
+    - lll
+    - gochecknoinits
+    - gochecknoglobals
+issues:
+  exclude-rules:
+    - linters:
+      - revive
+      text: "var-naming"
+    - linters:
+      - revive
+      text: "type name will be used as (.+?) by other packages, and that stutters"
+    - linters:
+      - stylecheck
+      text: "ST1003"
+run:
+  skip-dirs:
+    - git2go

cmd/completion/completion.go

@@ -9,11 +9,11 @@ import (
 
 var completionCmdExamples = `
 
-  # Enable BASH shell autocompletion 
-  $ source <(kubescape completion bash) 
+  # Enable BASH shell autocompletion
+  $ source <(kubescape completion bash)
   $ echo 'source <(kubescape completion bash)' >> ~/.bashrc
 
-  # Enable ZSH shell autocompletion 
+  # Enable ZSH shell autocompletion
   $ source <(kubectl completion zsh)
   $ echo 'source <(kubectl completion zsh)' >> "${fpath[1]}/_kubectl"
 
@@ -27,7 +27,7 @@ func GetCompletionCmd() *cobra.Command {
 		Example:               completionCmdExamples,
 		DisableFlagsInUseLine: true,
 		ValidArgs:             []string{"bash", "zsh", "fish", "powershell"},
-		Args:                  cobra.ExactValidArgs(1),
+		Args:                  cobra.MatchAll(cobra.ExactArgs(1), cobra.OnlyValidArgs),
 		Run: func(cmd *cobra.Command, args []string) {
 			switch strings.ToLower(args[0]) {
 			case "bash":

core/cautils/customerloader.go

@@ -470,10 +470,7 @@ func (c *ClusterConfig) updateConfigMap() error {
 }
 
 func updateConfigFile(configObj *ConfigObj) error {
-	if err := os.WriteFile(ConfigFileFullPath(), configObj.Config(), 0664); err != nil {
-		return err
-	}
-	return nil
+	return os.WriteFile(ConfigFileFullPath(), configObj.Config(), 0664) //nolint:gosec
 }
 
 func (c *ClusterConfig) updateConfigData(configMap *corev1.ConfigMap) {

core/cautils/getter/getpoliciesutils.go

@@ -21,18 +21,19 @@ func SaveInFile(policy interface{}, pathStr string) error {
 	if err != nil {
 		return err
 	}
-	err = os.WriteFile(pathStr, []byte(fmt.Sprintf("%v", string(encodedData))), 0644)
+	err = os.WriteFile(pathStr, encodedData, 0644) //nolint:gosec
 	if err != nil {
 		if os.IsNotExist(err) {
 			pathDir := path.Dir(pathStr)
-			if err := os.Mkdir(pathDir, 0744); err != nil {
+			// pathDir could contain subdirectories
+			if err := os.MkdirAll(pathDir, 0755); err != nil {
 				return err
 			}
 		} else {
 			return err
 
 		}
-		err = os.WriteFile(pathStr, []byte(fmt.Sprintf("%v", string(encodedData))), 0644)
+		err = os.WriteFile(pathStr, encodedData, 0644) //nolint:gosec
 		if err != nil {
 			return err
 		}

core/cautils/getter/loadpolicy.go

@@ -36,11 +36,11 @@ func NewLoadPolicy(filePaths []string) *LoadPolicy {
 	}
 }
 
-// Return control from file
+// GetControl returns a control from the policy file.
 func (lp *LoadPolicy) GetControl(controlID string) (*reporthandling.Control, error) {
-
 	control := &reporthandling.Control{}
 	filePath := lp.filePath()
+
 	f, err := os.ReadFile(filePath)
 	if err != nil {
 		return nil, err
@@ -49,20 +49,26 @@ func (lp *LoadPolicy) GetControl(controlID string) (*reporthandling.Control, err
 	if err = json.Unmarshal(f, control); err != nil {
 		return control, err
 	}
-	if controlID != "" && !strings.EqualFold(controlID, control.ControlID) && !strings.EqualFold(controlID, control.ControlID) {
-		framework, err := lp.GetFramework(control.Name)
-		if err != nil {
-			return nil, fmt.Errorf("control from file not matching")
-		} else {
-			for _, ctrl := range framework.Controls {
-				if strings.EqualFold(ctrl.ControlID, controlID) || strings.EqualFold(ctrl.ControlID, controlID) {
-					control = &ctrl
-					break
-				}
-			}
+
+	if controlID == "" || strings.EqualFold(controlID, control.ControlID) {
+		return control, nil
+	}
+
+	framework, err := lp.GetFramework(control.Name)
+	if err != nil {
+		return nil, fmt.Errorf("control from file not matching")
+	}
+
+	for _, toPin := range framework.Controls {
+		ctrl := toPin
+		if strings.EqualFold(ctrl.ControlID, controlID) {
+			control = &ctrl
+
+			break
 		}
 	}
-	return control, err
+
+	return control, nil
 }
 
 func (lp *LoadPolicy) GetFramework(frameworkName string) (*reporthandling.Framework, error) {

core/cautils/localgitrepository_test.go

@@ -27,7 +27,7 @@ func unzipFile(zipPath, destinationFolder string) (*zip.ReadCloser, error) {
 		return nil, err
 	}
 	for _, f := range archive.File {
-		filePath := filepath.Join(destinationFolder, f.Name)
+		filePath := filepath.Join(destinationFolder, f.Name) //nolint:gosec
 		if !strings.HasPrefix(filePath, filepath.Clean(destinationFolder)+string(os.PathSeparator)) {
 			return nil, fmt.Errorf("invalid file path")
 		}
@@ -50,7 +50,7 @@ func unzipFile(zipPath, destinationFolder string) (*zip.ReadCloser, error) {
 			return nil, err
 		}
 
-		if _, err := io.Copy(dstFile, fileInArchive); err != nil {
+		if _, err := io.Copy(dstFile, fileInArchive); err != nil { //nolint:gosec
 			return nil, err
 		}
 

core/core/download.go

@@ -6,7 +6,6 @@ import (
 	"path/filepath"
 	"strings"
 
-	"github.com/armosec/armoapi-go/armotypes"
 	logger "github.com/kubescape/go-logger"
 	"github.com/kubescape/go-logger/helpers"
 	"github.com/kubescape/kubescape/v2/core/cautils/getter"
@@ -114,13 +113,10 @@ func downloadConfigInputs(downloadInfo *metav1.DownloadInfo) error {
 }
 
 func downloadExceptions(downloadInfo *metav1.DownloadInfo) error {
-	var err error
 	tenant := getTenantConfig(&downloadInfo.Credentials, "", "", getKubernetesApi())
-
 	exceptionsGetter := getExceptionsGetter("", tenant.GetAccountID(), nil)
-	exceptions := []armotypes.PostureExceptionPolicy{}
 
-	exceptions, err = exceptionsGetter.GetExceptions(tenant.GetContextName())
+	exceptions, err := exceptionsGetter.GetExceptions(tenant.GetContextName())
 	if err != nil {
 		return err
 	}

core/pkg/containerscan/containerscan_mock.go

@@ -50,7 +50,7 @@ func randSeq(n int, bank []rune) string {
 
 	b := make([]rune, n)
 	for i := range b {
-		b[i] = bank[rand.Intn(len(bank))]
+		b[i] = bank[rand.Intn(len(bank))] //nolint:gosec
 	}
 	return string(b)
 }
@@ -60,7 +60,7 @@ func GenerateContainerScanLayer(layer *ScanResultLayer) {
 	layer.LayerHash = randSeq(32, hash)
 	layer.Vulnerabilities = make(VulnerabilitiesList, 0)
 	layer.Packages = make(LinuxPkgs, 0)
-	vuls := rand.Intn(10) + 1
+	vuls := rand.Intn(10) + 1 //nolint:gosec
 
 	for i := 0; i < vuls; i++ {
 		v := Vulnerability{}

core/pkg/opaprocessor/processorhandler.go

@@ -69,23 +69,26 @@ func (opap *OPAProcessor) Process(policies *cautils.Policies) error {
 
 	cautils.StartSpinner()
 
-	var errs error
-	for _, control := range policies.Controls {
+	for _, toPin := range policies.Controls {
+		control := toPin
 
 		resourcesAssociatedControl, err := opap.processControl(&control)
 		if err != nil {
 			logger.L().Error(err.Error())
 		}
+
+		if len(resourcesAssociatedControl) == 0 {
+			continue
+		}
+
 		// update resources with latest results
-		if len(resourcesAssociatedControl) != 0 {
-			for resourceID, controlResult := range resourcesAssociatedControl {
-				if _, ok := opap.ResourcesResult[resourceID]; !ok {
-					opap.ResourcesResult[resourceID] = resourcesresults.Result{ResourceID: resourceID}
-				}
-				t := opap.ResourcesResult[resourceID]
-				t.AssociatedControls = append(t.AssociatedControls, controlResult)
-				opap.ResourcesResult[resourceID] = t
+		for resourceID, controlResult := range resourcesAssociatedControl {
+			if _, ok := opap.ResourcesResult[resourceID]; !ok {
+				opap.ResourcesResult[resourceID] = resourcesresults.Result{ResourceID: resourceID}
 			}
+			t := opap.ResourcesResult[resourceID]
+			t.AssociatedControls = append(t.AssociatedControls, controlResult)
+			opap.ResourcesResult[resourceID] = t
 		}
 	}
 
@@ -95,7 +98,7 @@ func (opap *OPAProcessor) Process(policies *cautils.Policies) error {
 
 	opap.loggerDoneScanning()
 
-	return errs
+	return nil
 }
 
 func (opap *OPAProcessor) loggerStartScanning() {

core/pkg/opaprocessor/processorhandlerutils.go

@@ -15,9 +15,9 @@ import (
 // updateResults updates the results objects and report objects. This is a critical function - DO NOT CHANGE
 //
 // The function:
-//  - removes sensible data
-//  - adds exceptions
-//  - summarizes results
+//   - removes sensible data
+//   - adds exceptions
+//   - summarizes results
 func (opap *OPAProcessor) updateResults() {
 
 	// remove data from all objects
@@ -117,9 +117,11 @@ func getKubernetesObjects(k8sResources *cautils.K8SResources, allResources map[s
 					groupResources := k8sinterface.ResourceGroupToString(groups, version, resource)
 					for _, groupResource := range groupResources {
 						if k8sObj, ok := (*k8sResources)[groupResource]; ok {
-							if k8sObj == nil {
-								// logger.L().Debug("skipping", helpers.String("resource", groupResource))
-							}
+							/*
+								if k8sObj == nil {
+									// logger.L().Debug("skipping", helpers.String("resource", groupResource))
+								}
+							*/
 							for i := range k8sObj {
 								k8sObjects = append(k8sObjects, allResources[k8sObj[i]])
 							}

core/pkg/registryadaptors/armosec/v1/civksadaptor.go

@@ -25,14 +25,17 @@ func (ksCivAdaptor *KSCivAdaptor) Login() error {
 }
 func (ksCivAdaptor *KSCivAdaptor) GetImagesVulnerabilities(imageIDs []registryvulnerabilities.ContainerImageIdentifier) ([]registryvulnerabilities.ContainerImageVulnerabilityReport, error) {
 	resultList := make([]registryvulnerabilities.ContainerImageVulnerabilityReport, 0)
-	for _, imageID := range imageIDs {
+	for _, toPin := range imageIDs {
+		imageID := toPin
 		result, err := ksCivAdaptor.GetImageVulnerability(&imageID)
-		if err == nil {
-			resultList = append(resultList, *result)
-		} else {
+		if err != nil {
 			logger.L().Debug("failed to get image vulnerabilities", helpers.String("image", imageID.Tag), helpers.Error(err))
+			continue
 		}
+
+		resultList = append(resultList, *result)
 	}
+
 	return resultList, nil
 }
 

core/pkg/registryadaptors/gcp/v1/gcpadaptor.go

@@ -30,14 +30,17 @@ func (GCPAdaptor *GCPAdaptor) Login() error {
 
 func (GCPAdaptor *GCPAdaptor) GetImagesVulnerabilities(imageIDs []registryvulnerabilities.ContainerImageIdentifier) ([]registryvulnerabilities.ContainerImageVulnerabilityReport, error) {
 	resultList := make([]registryvulnerabilities.ContainerImageVulnerabilityReport, 0)
-	for _, imageID := range imageIDs {
+	for _, toPin := range imageIDs {
+		imageID := toPin
 		result, err := GCPAdaptor.GetImageVulnerability(&imageID)
-		if err == nil {
-			resultList = append(resultList, *result)
-		} else {
+		if err != nil {
 			logger.L().Debug("failed to get image vulnerabilities", helpers.String("image", imageID.Tag), helpers.Error(err))
+			continue
 		}
+
+		resultList = append(resultList, *result)
 	}
+
 	return resultList, nil
 }
 

core/pkg/registryadaptors/gcp/v1/gcpadaptormock.go

@@ -20,15 +20,16 @@ func (GCPAdaptorMock *GCPAdaptorMock) Login() error {
 
 func (GCPAdaptorMock *GCPAdaptorMock) GetImagesVulnerabilities(imageIDs []registryvulnerabilities.ContainerImageIdentifier) ([]registryvulnerabilities.ContainerImageVulnerabilityReport, error) {
 	resultList := make([]registryvulnerabilities.ContainerImageVulnerabilityReport, 0)
-	for _, imageID := range imageIDs {
+	for _, toPin := range imageIDs {
+		imageID := toPin
 		result, err := GCPAdaptorMock.GetImageVulnerability(&imageID)
-		if err == nil {
-			resultList = append(resultList, *result)
-		} else {
+		if err != nil {
 			return nil, err
 		}
 
-		return resultList, nil
+		resultList = append(resultList, *result)
+
+		return resultList, nil //nolint:staticcheck // we return at once and shorten the mocked result
 	}
 
 	GCPAdaptorMock.resultList = resultList

core/pkg/resourcehandler/k8sresources.go

@@ -247,7 +247,7 @@ func (k8sHandler *K8sResourceHandler) pullSingleResource(resource *schema.GroupV
 			clientResource = k8sHandler.k8s.DynamicClient.Resource(*resource)
 		} else if k8sinterface.IsNamespaceScope(resource) {
 			clientResource = k8sHandler.k8s.DynamicClient.Resource(*resource).Namespace(namespace)
-		} else if k8sHandler.fieldSelector.GetClusterScope(*&resource) {
+		} else if k8sHandler.fieldSelector.GetClusterScope(resource) {
 			clientResource = k8sHandler.k8s.DynamicClient.Resource(*resource)
 		} else {
 			continue

core/pkg/resourcehandler/remotegitutils.go

@@ -15,12 +15,12 @@ import (
 
 // To Check if the given repository is Public(No Authentication needed), send a HTTP GET request to the URL
 // If response code is 200, the repository is Public.
-func isGitRepoPublic(URL string) bool {
-	resp, err := nethttp.Get(URL)
-
+func isGitRepoPublic(u string) bool {
+	resp, err := nethttp.Get(u) //nolint:gosec
 	if err != nil {
 		return false
 	}
+
 	// if the status code is 200, our get request is successful.
 	// It only happens when the repository is public.
 	if resp.StatusCode == 200 {

core/pkg/resultshandling/printer/v2/prometheusutils.go

@@ -319,11 +319,13 @@ func (m *Metrics) setResourcesCounters(
 	resources map[string]workloadinterface.IMetadata,
 	results map[string]resourcesresults.Result) {
 
-	for resourceID, result := range results {
+	for resourceID, toPin := range results {
 		r, ok := resources[resourceID]
 		if !ok {
 			continue
 		}
+		result := toPin
+
 		passed, excluded, failed := resourceControlStatusCounters(&result)
 
 		mrc := mResources{}
@@ -339,5 +341,4 @@ func (m *Metrics) setResourcesCounters(
 
 		m.listResources = append(m.listResources, mrc)
 	}
-
 }

core/pkg/resultshandling/printer/v2/sarifprinter.go

@@ -129,7 +129,9 @@ func (sp *SARIFPrinter) ActionPrint(opaSessionObj *cautils.OPASessionObj) {
 				logger.L().Debug("failed to create location resolver", helpers.Error(err))
 			}
 
-			for _, ac := range result.AssociatedControls {
+			for _, toPin := range result.AssociatedControls {
+				ac := toPin
+
 				if ac.GetStatus(nil).IsFailed() {
 					ctl := opaSessionObj.Report.SummaryDetails.Controls.GetControl(reportsummary.EControlCriteriaID, ac.GetID())
 					location := sp.resolveFixLocation(opaSessionObj, locationResolver, &ac, resourceID)