diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ad4d6b6d6..b8ae301b8 100644 --- 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,7 +16,7 @@ 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 @@ -24,95 +24,8 @@ jobs: 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 index c9e76d0e5..bc6faf2d5 100644 --- 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 \ No newline at end of file diff --git a/istio/install.go b/istio/install.go index 31ef6faad..275b83a9c 100644 --- 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 index 31db16b62..69831ce1b 100644 --- a/istio/istio.go +++ b/istio/istio.go @@ -303,12 +303,11 @@ func (istio *Istio) ProcessOAM(ctx context.Context, oamReq adapter.OAMRequest) ( kubeconfigs := oamReq.K8sConfigs var comps []v1alpha1.Component for _, acomp := range oamReq.OamComps { - comp, err := oam.ParseApplicationComponent(acomp) - if err != nil { + comp, configErr := oam.ParseApplicationComponent(acomp) + if configErr != nil { istio.Log.Error(ErrParseOAMComponent) continue } - comps = append(comps, comp) } @@ -320,15 +319,15 @@ func (istio *Istio) ProcessOAM(ctx context.Context, oamReq adapter.OAMRequest) ( // If operation is delete then first HandleConfiguration and then handle the deployment if oamReq.DeleteOp { // Process configuration - msg2, err := istio.HandleApplicationConfiguration(config, oamReq.DeleteOp, kubeconfigs) - if err != nil { - return msg2, ErrProcessOAM(err) + msg2, appConfiguration := istio.HandleApplicationConfiguration(config, oamReq.DeleteOp, kubeconfigs) + if appConfiguration != nil { + return msg2, ErrProcessOAM(appConfiguration) } // Process components - msg1, err := istio.HandleComponents(comps, oamReq.DeleteOp, kubeconfigs) - if err != nil { - return msg1 + "\n" + msg2, ErrProcessOAM(err) + msg1, componentsErr := istio.HandleComponents(comps, oamReq.DeleteOp, kubeconfigs) + if componentsErr != nil { + return msg1 + "\n" + msg2, ErrProcessOAM(componentsErr) } return msg1 + "\n" + msg2, nil