Merge pull request #627 from meshery/fixed/wrongUseErr

Aisuko · web-flow · commit 7acc183754e1 · 2023-10-09T18:48:31.000+11:00
Make the configuration of ci more simpler and fix the error
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -2,7 +2,7 @@ name: Default Meshery Istio Workflow
 on:
   push:
     branches:
-      - "*"
+      - "master"
     tags:
       - "v*"
     paths-ignore:
@@ -16,103 +16,16 @@ jobs:
     name: golangci-lint
     runs-on: ubuntu-22.04
     steps:
-      - uses: actions/setup-go@v3
+      - uses: actions/setup-go@v4
         with:
           go-version: 1.19
       - uses: actions/checkout@v3
       - name: golangci-lint
         uses: golangci/golangci-lint-action@v3
         with:
           # Required: the version of golangci-lint is required and must be specified without patch version: we always use the latest patch version.
-          version: v1.49
-
-          # Optional: working directory, useful for monorepos
-          # working-directory: somedir
-
-          # Optional: golangci-lint command line arguments.
-          # args: --issues-exit-code=0
-
-          # Optional: show only new issues if it's a pull request. The default value is `false`.
-          # only-new-issues: true
-  error_check:
-    name: Error check
-    runs-on: ubuntu-22.04
-    steps:
-      - name: Check out code
-        uses: actions/checkout@v3
-        with:
-          fetch-depth: 1
-      - name: Setup Go
-        uses: actions/setup-go@v3
-        with:
-          go-version: 1.19
-      - run: GOPROXY=https://proxy.golang.org,direct GOSUMDB=off GO111MODULE=on go install github.com/kisielk/errcheck@latest; /home/runner/go/bin/errcheck -tags draft ./...
-  error_code_check:
-    name: Error code utility check
-    runs-on: ubuntu-22.04
-    steps:
-      - name: Check out code
-        uses: actions/checkout@v3
-        with:
-          fetch-depth: 1
-      - name: Setup Go
-        uses: actions/setup-go@v3
-        with:
-          go-version: 1.19
-      - run: |
-            errWillHave="level=error"
-            GOPROXY=https://proxy.golang.org,direct GOSUMDB=off GO111MODULE=on go install github.com/layer5io/meshkit/cmd/errorutil;
-            err=$(/home/runner/go/bin/errorutil -d . update --skip-dirs meshery -i ./helpers -o ./helpers);
-            echo "ERR: $err";
-            if [[ $err == *"$errWillHave"* ]];
-            then
-              echo "$err";
-              return 1;
-            fi
-            
-  static_check:
-    name: Static check
-    runs-on: ubuntu-22.04
-    steps:
-      - name: Check out code
-        uses: actions/checkout@v3
-        with:
-          fetch-depth: 1
-      - name: Setup Go
-        uses: actions/setup-go@v3
-        with:
-          go-version: 1.19
-      - uses: dominikh/staticcheck-action@v1.2.0
-        with:
-          install-go: false
-          version: "2022.1"
-  vet:
-    name: Vet
-    runs-on: ubuntu-22.04
-    steps:
-      - name: Check out code
-        uses: actions/checkout@v3
-        with:
-          fetch-depth: 1
-      - name: Setup Go
-        uses: actions/setup-go@v3
-        with:
-          go-version: 1.19
-      - run: GOPROXY=https://proxy.golang.org,direct GOSUMDB=off GO111MODULE=on go vet -tags draft ./...
-  sec_check:
-    name: Security check
-    runs-on: ubuntu-22.04
-    env:
-      GO111MODULE: on
-    steps:
-      - name: Check out code
-        uses: actions/checkout@v3
-        with:
-          fetch-depth: 1
-      - name: Run Gosec Security Scanner
-        uses: securego/gosec@master
-        with:
-          args: -exclude=G301,G304,G107,G101,G110,G204,G409,G305,G302 ./... 
+          version: latest
+          args: --timeout=5m
   tests:
     #  needs: [lint, error_check, static_check, vet, sec_check]
      name: Tests
diff --git a/.golangci.yml b/.golangci.yml
@@ -1,144 +1,63 @@
 linters-settings:
-  depguard:
-    list-type: blacklist
-    packages:
-      # logging is allowed only by logutils.Log, logrus
-      # is allowed to use only in logutils package
-      - github.com/sirupsen/logrus
-    packages-with-error-message:
-      - github.com/sirupsen/logrus: "logging is allowed only by logutils.Log"
-  dupl:
-    threshold: 100
-  exhaustive:
-    default-signifies-exhaustive: false
-  funlen:
-    lines: 100
-    statements: 50
   gci:
-    local-prefixes: github.com/golangci/golangci-lint
+    enabled: true
+    max-len: 120
+    line-length: 120
   goconst:
-    min-len: 2
-    min-occurrences: 2
+    enabled: true
   gocritic:
-    enabled-tags:
-      - diagnostic
-      - experimental
-      - opinionated
-      - performance
-      - style
-    disabled-checks:
-      - dupImport # https://github.com/go-critic/go-critic/issues/845
-      - ifElseChain
-      - octalLiteral
-      - whyNoLint
-      - wrapperFunc
-  gocyclo:
-    min-complexity: 15
-  goimports:
-    local-prefixes: github.com/golangci/golangci-lint
-  golint:
-    min-confidence: 0
-  gomnd:
-    settings:
-      mnd:
-        # don't include the "operation" and "assign"
-        checks: argument,case,condition,return
-  gosec:
-    settings:
-      exclude: -G204
+    enabled: true
+    disable:
+      - parallelize
+      - nesting
+      - hugeParam
+      - hugeStruct
+      - nestParam
+      - prealloc
   govet:
-    check-shadowing: false
-    settings:
-      printf:
-        funcs:
-          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Infof
-          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Warnf
-          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Errorf
-          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Fatalf
-  lll:
-    line-length: 950
-  maligned:
-    suggest-new: true
-  misspell:
-    locale: US
-  nolintlint:
-    allow-leading-space: true # don't require machine-readable nolint directives (i.e. with no leading space)
-    allow-unused: false # report any unused nolint directives
-    require-explanation: false # don't require an explanation for nolint directives
-    require-specific: false # don't require nolint directives to be specific about which linter is being skipped
+    enabled: true
+    check-shadowing: true
+    tests: true
+  golint:
+    enabled: true
+    min-confidence: 0.8
+  unused:
+    enabled: true
+    check-exported: true
+    check-packages: true
+    check-generated: true
+    tests: true
+    allow-unused-type-export: true
+  cyclop:
+    enabled: true
+    average-strictness: 7
+  scopelint:
+    enabled: true
+    tests: true
+
 
+# Configuration for golangci-lint that is suitable for a Kubernetes operator project built with Golang
 linters:
-  # please, do not use `enable-all`: it's deprecated and will be removed soon.
-  # inverted configuration with `enable-all` and `disable` is not scalable during updates of golangci-lint
-  # https://golangci-lint.run/usage/linters/
-  disable-all: true
-  enable:
-    # TODO: consider continuously if more should be enabled.
-    # Can also be useful to run with more strict settings before commit locally, i.e. to test for TODOs (godox)
-    # - bodyclose
-    # - deadcode
-    - dogsled
-    # - dupl
-    - errcheck
-    # - exhaustive
-    # - funlen
-    # - goconst
-    # - gocritic
-    # - gocyclo
-    - gofmt
-    - goimports
-    # - golint
-    - gomodguard
-    - gosec
-    # - gomnd
-    # - goprintffuncname
-    - gosimple
+  enable-all: false
+  disable-all: false
+  linters:
+    - gci
+    - goconst
+    - gocritic
     - govet
-    - ineffassign
-    # - interfacer
-    - lll
-    - misspell
-    # - nakedret
-    # - nolintlint
-    # - rowserrcheck
-    # - scopelint
-    - staticcheck
-    # - structcheck
-    - stylecheck
-    - typecheck
-    # - unconvert
-    # - unparam
+    - golint
     - unused
-    # - varcheck
-    - whitespace
-    - asciicheck
-#    - gochecknoglobals
-#    - gocognit
-#    - godot
-#    - godox
-#    - goerr113
-#    - maligned
-#    - nestif
-#    - prealloc
-#    - testpackage
-#    - wsl
-
-issues:
-  # Excluding configuration per-path, per-linter, per-text and per-source
+    - cyclop
+    - scopelint
   exclude-rules:
-    - path: _test\.go
-      linters:
-        - gomnd
-
-    # https://github.com/go-critic/go-critic/issues/926
-    - linters:
-        - gocritic
-      text: "unnecessaryDefer:"
+    - testpackage
 
 run:
+  enable-cache: true
   skip-dirs:
-    - test/testdata_etc
-    - internal/cache
-    - internal/renameio
-    - internal/robustio
-  timeout: 5m
+    - vendor
+    - bundle
+    - config
+    - hack
+    - helpers
+    - img
diff --git a/istio/install.go b/istio/install.go
@@ -409,6 +409,7 @@ func (istio *Istio) getExecutable(release, dirName string) (string, error) {
 
 func tarxzf(location string, stream io.Reader) error {
 	uncompressedStream, err := gzip.NewReader(stream)
+
 	if err != nil {
 		return err
 	}
@@ -429,34 +430,30 @@ func tarxzf(location string, stream io.Reader) error {
 		switch header.Typeflag {
 		case tar.TypeDir:
 			// File traversal is required to store the extracted manifests at the right place
-			// #nosec
-			if err := os.MkdirAll(path.Join(location, header.Name), 0750); err != nil {
+			if os.MkdirAll(path.Join(location, header.Name), 0750) != nil {
 				return ErrTarXZF(err)
 			}
 		case tar.TypeReg:
 			// File traversal is required to store the extracted manifests at the right place
-			// #nosec
-			outFile, err := os.Create(path.Join(location, header.Name))
-			if err != nil {
-				return ErrTarXZF(err)
+			outFile, createFileErr := os.Create(path.Join(location, header.Name))
+			if createFileErr != nil {
+				return ErrTarXZF(createFileErr)
 			}
-			// Trust istioctl tar
-			// #nosec
-			if _, err := io.Copy(outFile, tarReader); err != nil {
-				return ErrTarXZF(err)
+			// Trust istioctl tar hence
+			if _, copyErr := io.Copy(outFile, tarReader); copyErr != nil {
+				return ErrTarXZF(copyErr)
 			}
+
 			if err = outFile.Close(); err != nil {
 				return ErrTarXZF(err)
 			}
 
 			if header.FileInfo().Name() == "istioctl" {
 				// istioctl binary needs to be executable
-				// #nosec
-				if err = os.Chmod(outFile.Name(), 0750); err != nil {
+				if os.Chmod(outFile.Name(), 0750) != nil {
 					return ErrMakingBinExecutable(err)
 				}
 			}
-
 		default:
 			return ErrTarXZF(err)
 		}
@@ -465,6 +462,7 @@ func tarxzf(location string, stream io.Reader) error {
 	return nil
 }
 
+// TODO: The defer function is the last change to close the file, if it fails, it will be left open. We should return this error rather than logging it
 func unzip(location string, zippedContent io.Reader) error {
 	// Keep file in memory: Approx size ~ 50MB
 	// TODO: Find a better approach
@@ -490,7 +488,6 @@ func unzip(location string, zippedContent io.Reader) error {
 		}()
 
 		// need file traversal to place the extracted files at the right place, hence
-		// #nosec
 		extractedFilePath := path.Join(location, file.Name)
 
 		if file.FileInfo().IsDir() {
@@ -508,11 +505,8 @@ func unzip(location string, zippedContent io.Reader) error {
 			if err != nil {
 				return ErrUnzipFile(err)
 			}
-
-			/* #nosec G307 */
-
 			defer func() {
-				if err := outputFile.Close(); err != nil {
+				if outputFile.Close() != nil {
 					fmt.Println(err)
 				}
 			}()
diff --git a/istio/istio.go b/istio/istio.go
