From facf582069a07663b7c0bccf15b5dba7e05649c3 Mon Sep 17 00:00:00 2001 From: z1cheng Date: Thu, 6 Jul 2023 12:29:48 +0000 Subject: [PATCH 01/12] Add golangci github action and replace the deprecated golint Signed-off-by: z1cheng --- .github/workflows/ci.yaml | 19 ------------------- .github/workflows/golangci-lint.yml | 28 ++++++++++++++++++++++++++++ .golangci.yml | 4 ++++ hack/verify-golint.sh | 17 ++--------------- 4 files changed, 34 insertions(+), 34 deletions(-) create mode 100644 .github/workflows/golangci-lint.yml create mode 100644 .golangci.yml diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 150ec29ae6..e54cac37a7 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -78,25 +78,6 @@ jobs: # G307 TODO: Deferring unsafe method "Close" args: -exclude=G109,G601,G104,G204,G304,G306,G307 -tests=false -exclude-dir=test -exclude-dir=images/ -exclude-dir=docs/ ./... - lint: - runs-on: ubuntu-latest - needs: changes - if: | - (needs.changes.outputs.go == 'true') - steps: - - name: Checkout - uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3 - - - name: Set up Go - id: go - uses: actions/setup-go@fac708d6674e30b6ba41289acaab6d4b75aa0753 # v4.0.1 - with: - go-version: '1.20' - check-latest: true - - - name: Run Lint - run: ./hack/verify-golint.sh - gofmt: runs-on: ubuntu-latest needs: changes diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml new file mode 100644 index 0000000000..26c4addbac --- /dev/null +++ b/.github/workflows/golangci-lint.yml @@ -0,0 +1,28 @@ +name: golangci-lint + +on: + pull_request: + types: [opened, edited, synchronize, reopened] + +permissions: + contents: read + +jobs: + golangci: + name: lint + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3 + + - name: Set up Go + id: go + uses: actions/setup-go@fac708d6674e30b6ba41289acaab6d4b75aa0753 # v4.0.1 + with: + go-version: '1.20' + check-latest: true + + - name: golangci-lint + uses: golangci/golangci-lint-action@639cd343e1d3b897ff35927a75193d57cfcba299 # tag=v3.6.0 + with: + version: v1.53 \ No newline at end of file diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000000..5f475378c2 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,4 @@ +run: + timeout: 10m + go: "1.20" + allow-parallel-runners: true \ No newline at end of file diff --git a/hack/verify-golint.sh b/hack/verify-golint.sh index 7c92848809..690fed855a 100755 --- a/hack/verify-golint.sh +++ b/hack/verify-golint.sh @@ -22,19 +22,6 @@ KUBE_ROOT=$(dirname "${BASH_SOURCE}")/.. cd "${KUBE_ROOT}" -GOLINT=${GOLINT:-"golint"} -PACKAGES=($(go list ./internal/... | grep -v /vendor/)) -bad_files=() -for package in "${PACKAGES[@]}"; do - out=$("${GOLINT}" -min_confidence=0.9 "${package}" | grep -v -E '(should not use dot imports|internal/file/bindata.go)' || :) - if [[ -n "${out}" ]]; then - bad_files+=("${out}") - fi -done -if [[ "${#bad_files[@]}" -ne 0 ]]; then - echo "!!! '$GOLINT' problems: " - echo "${bad_files[@]}" - exit 1 -fi +LINT=${LINT:-golangci-lint} -# ex: ts=2 sw=2 et filetype=sh +${LINT} run \ No newline at end of file From 280a7e756581e8486f6f92428ff9752e539b9bfe Mon Sep 17 00:00:00 2001 From: z1cheng Date: Thu, 6 Jul 2023 13:57:54 +0000 Subject: [PATCH 02/12] Install if golangci-lint not exists Signed-off-by: z1cheng --- hack/verify-golint.sh | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/hack/verify-golint.sh b/hack/verify-golint.sh index 690fed855a..0b623c767d 100755 --- a/hack/verify-golint.sh +++ b/hack/verify-golint.sh @@ -24,4 +24,11 @@ cd "${KUBE_ROOT}" LINT=${LINT:-golangci-lint} +if ! command -v ${LINT} >/dev/null 2>&1; then + echo "${LINT} is missing. Installing it now." + # See: https://golangci-lint.run/usage/install/#local-installation + curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.53.3 + LINT=$(go env GOPATH)/bin/golangci-lint +fi + ${LINT} run \ No newline at end of file From cc45cdaf22bc76dc79f20fcb29437c0a890f7b0d Mon Sep 17 00:00:00 2001 From: z1cheng Date: Fri, 7 Jul 2023 07:34:32 +0000 Subject: [PATCH 03/12] Use -z operator Signed-off-by: z1cheng --- hack/verify-golint.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hack/verify-golint.sh b/hack/verify-golint.sh index 0b623c767d..9fd617b8fb 100755 --- a/hack/verify-golint.sh +++ b/hack/verify-golint.sh @@ -24,7 +24,7 @@ cd "${KUBE_ROOT}" LINT=${LINT:-golangci-lint} -if ! command -v ${LINT} >/dev/null 2>&1; then +if [[ -z "$(command -v ${LINT})" ]]; then echo "${LINT} is missing. Installing it now." # See: https://golangci-lint.run/usage/install/#local-installation curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.53.3 From 1b448b0df8bcbc891724429db76e7333ab6af0ea Mon Sep 17 00:00:00 2001 From: z1cheng Date: Fri, 7 Jul 2023 07:51:37 +0000 Subject: [PATCH 04/12] Fix json tag for DatadogSampleRate field in config.go Signed-off-by: z1cheng --- internal/ingress/controller/config/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/ingress/controller/config/config.go b/internal/ingress/controller/config/config.go index 345ce89b46..2ee7923b79 100644 --- a/internal/ingress/controller/config/config.go +++ b/internal/ingress/controller/config/config.go @@ -708,7 +708,7 @@ type Configuration struct { // DatadogSampleRate specifies sample rate for any traces created. // Default: use a dynamic rate instead - DatadogSampleRate *float32 `json:"datadog-sample-rate",omitempty` + DatadogSampleRate *float32 `json:"datadog-sample-rate,omitempty"` // MainSnippet adds custom configuration to the main section of the nginx configuration MainSnippet string `json:"main-snippet"` From 0f4781d1d672f2d944e424cb7027b69e260c17d5 Mon Sep 17 00:00:00 2001 From: z1cheng Date: Fri, 7 Jul 2023 14:21:19 +0000 Subject: [PATCH 05/12] Add golangci linters Signed-off-by: z1cheng --- .golangci.yml | 241 +++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 239 insertions(+), 2 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 5f475378c2..7ff31f8355 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,4 +1,241 @@ run: timeout: 10m - go: "1.20" - allow-parallel-runners: true \ No newline at end of file + allow-parallel-runners: true + + # Maximum issues count per one linter. Set to 0 to disable. Default is 50. + max-issues-per-linter: 0 + + # Maximum count of issues with the same text. Set to 0 to disable. Default is 3. + max-same-issues: 0 +linters: + disable-all: true + enable: + - asasalint + - asciicheck + - bidichk + - bodyclose + - contextcheck + - decorder + - dogsled + - dupl + - durationcheck + - errcheck + - errchkjson + - errname + - execinquery + - ginkgolinter + - gocheckcompilerdirectives + - goconst + - gocritic + - gocyclo + - godox + - gofmt + - gofumpt + - goheader + - goimports + - gomoddirectives + - gomodguard + - goprintffuncname + - gosec + - gosimple + - govet + - grouper + - importas + - ineffassign + - loggercheck + - makezero + - misspell + - musttag + - nakedret + - nolintlint + - nosprintfhostport + - prealloc + - predeclared + - promlinter + - reassign + - revive + - rowserrcheck + - sqlclosecheck + - staticcheck + - stylecheck + - tenv + - testableexamples + - typecheck + - unconvert + - unparam + - unused + - usestdlibvars + - whitespace + # - containedctx + # - cyclop + # - dupword + # - errorlint + # - exhaustive + # - exhaustruct + # - exportloopref + # - forbidigo + # - forcetypeassert + # - funlen + # - gci + # - gochecknoglobals + # - gochecknoinits + # - gocognit + # - godot + # - goerr113 + # - gomnd + # - interfacebloat + # - ireturn + # - lll + # - maintidx + # - nestif + # - nilerr + # - nilnil + # - nlreturn + # - noctx + # - nonamedreturns + # - paralleltest + # - tagliatelle + # - testpackage + # - thelper + # - tparallel + # - varnamelen + # - wastedassign + # - wrapcheck + # - wsl +linters-settings: + gocyclo: + min-complexity: 40 + godox: + keywords: + - BUG + - FIXME + - HACK + errcheck: + check-type-assertions: true + check-blank: true + gocritic: + enabled-checks: + # Diagnostic + - appendAssign + - argOrder + - badCall + - badCond + - badLock + - badRegexp + - badSorting + - builtinShadowDecl + - caseOrder + - codegenComment + - commentedOutCode + - deferInLoop + - deprecatedComment + - dupArg + - dupBranchBody + - dupCase + - dupSubExpr + - dynamicFmtString + - emptyDecl + - evalOrder + - exitAfterDefer + - externalErrorReassign + - filepathJoin + - flagDeref + - flagName + - mapKey + - nilValReturn + - offBy1 + - regexpPattern + - returnAfterHttpError + - sloppyReassign + - sloppyTypeAssert + - sortSlice + - sprintfQuotedString + - sqlQuery + - syncMapLoadAndDelete + - truncateCmp + - unnecessaryDefer + - weakCond + + # Performance + - appendCombine + - equalFold + - hugeParam + - indexAlloc + - preferDecodeRune + - preferFprint + - preferStringWriter + - preferWriteByte + - rangeExprCopy + - rangeValCopy + - sliceClear + - stringXbytes + + # Style + - assignOp + - boolExprSimplify + - captLocal + - commentFormatting + - commentedOutImport + - defaultCaseOrder + - deferUnlambda + - docStub + - dupImport + - elseif + - emptyFallthrough + - emptyStringTest + - exposedSyncMutex + - hexLiteral + - httpNoBody + - ifElseChain + - methodExprCall + - newDeref + - octalLiteral + - preferFilepathJoin + - redundantSprint + - regexpMust + - regexpSimplify + - ruleguard + - singleCaseSwitch + - sloppyLen + - stringConcatSimplify + - stringsCompare + - switchTrue + - timeCmpSimplify + - timeExprSimplify + - todoCommentWithoutDetail + - tooManyResultsChecker + - typeAssertChain + - typeDefFirst + - typeSwitchVar + - underef + - unlabelStmt + - unlambda + - unslice + - valSwap + - whyNoLint + - wrapperFunc + - yodaStyleExpr + + # Opinionated + - builtinShadow + - importShadow + - initClause + - nestingReduce + - paramTypeCombine + - ptrToRefParam + - typeUnparen + - unnamedResult + - unnecessaryBlock + nolintlint: + # Enable to ensure that nolint directives are all used. Default is true. + allow-unused: false + # Disable to ensure that nolint directives don't have a leading space. Default is true. + # TODO(lint): Enforce machine-readable `nolint` directives + allow-leading-space: true + # Exclude following linters from requiring an explanation. Default is []. + allow-no-explanation: [] + # Enable to require an explanation of nonzero length after each nolint directive. Default is false. + # TODO(lint): Enforce explanations for `nolint` directives + require-explanation: false + # Enable to require nolint directives to mention the specific linter being suppressed. Default is false. + require-specific: true \ No newline at end of file From 1fb98df5b7948081ef3fa98cbe6eddcb1f65b14c Mon Sep 17 00:00:00 2001 From: z1cheng Date: Mon, 10 Jul 2023 10:27:48 +0000 Subject: [PATCH 06/12] Revert DatadogSampleRate fix Signed-off-by: z1cheng --- internal/ingress/controller/config/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/ingress/controller/config/config.go b/internal/ingress/controller/config/config.go index 2ee7923b79..345ce89b46 100644 --- a/internal/ingress/controller/config/config.go +++ b/internal/ingress/controller/config/config.go @@ -708,7 +708,7 @@ type Configuration struct { // DatadogSampleRate specifies sample rate for any traces created. // Default: use a dynamic rate instead - DatadogSampleRate *float32 `json:"datadog-sample-rate,omitempty"` + DatadogSampleRate *float32 `json:"datadog-sample-rate",omitempty` // MainSnippet adds custom configuration to the main section of the nginx configuration MainSnippet string `json:"main-snippet"` From 37d88a4909c37234e430c73bc7cb259ea354b4f5 Mon Sep 17 00:00:00 2001 From: z1cheng Date: Mon, 10 Jul 2023 12:50:13 +0000 Subject: [PATCH 07/12] Fix comments Signed-off-by: z1cheng --- .github/workflows/golangci-lint.yml | 6 +----- hack/verify-golint.sh | 2 +- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml index 26c4addbac..be7f6d8763 100644 --- a/.github/workflows/golangci-lint.yml +++ b/.github/workflows/golangci-lint.yml @@ -1,9 +1,5 @@ name: golangci-lint -on: - pull_request: - types: [opened, edited, synchronize, reopened] - permissions: contents: read @@ -23,6 +19,6 @@ jobs: check-latest: true - name: golangci-lint - uses: golangci/golangci-lint-action@639cd343e1d3b897ff35927a75193d57cfcba299 # tag=v3.6.0 + uses: golangci/golangci-lint-action@639cd343e1d3b897ff35927a75193d57cfcba299 # v3.6.0 with: version: v1.53 \ No newline at end of file diff --git a/hack/verify-golint.sh b/hack/verify-golint.sh index 9fd617b8fb..17bcedd9fc 100755 --- a/hack/verify-golint.sh +++ b/hack/verify-golint.sh @@ -31,4 +31,4 @@ if [[ -z "$(command -v ${LINT})" ]]; then LINT=$(go env GOPATH)/bin/golangci-lint fi -${LINT} run \ No newline at end of file +${LINT} run From 7651b18a252e88dece0765d1820683a444d95d2e Mon Sep 17 00:00:00 2001 From: z1cheng Date: Mon, 10 Jul 2023 12:52:35 +0000 Subject: [PATCH 08/12] Add a new line Signed-off-by: z1cheng --- .github/workflows/golangci-lint.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml index be7f6d8763..3d9bbbd64d 100644 --- a/.github/workflows/golangci-lint.yml +++ b/.github/workflows/golangci-lint.yml @@ -21,4 +21,4 @@ jobs: - name: golangci-lint uses: golangci/golangci-lint-action@639cd343e1d3b897ff35927a75193d57cfcba299 # v3.6.0 with: - version: v1.53 \ No newline at end of file + version: v1.53 From f5557a6c2d7239674aa7a80a54b5187224db6c3b Mon Sep 17 00:00:00 2001 From: z1cheng Date: Mon, 10 Jul 2023 12:54:37 +0000 Subject: [PATCH 09/12] fixup! Add a new line Signed-off-by: z1cheng --- .golangci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.golangci.yml b/.golangci.yml index 7ff31f8355..5da8f0399f 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -238,4 +238,4 @@ linters-settings: # TODO(lint): Enforce explanations for `nolint` directives require-explanation: false # Enable to require nolint directives to mention the specific linter being suppressed. Default is false. - require-specific: true \ No newline at end of file + require-specific: true From fa5948c4c4983cfd01e8cbe94b81e82a16172085 Mon Sep 17 00:00:00 2001 From: z1cheng Date: Tue, 11 Jul 2023 12:17:25 +0000 Subject: [PATCH 10/12] Add trigger condition Signed-off-by: z1cheng --- .github/workflows/golangci-lint.yml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml index 3d9bbbd64d..d601d69dd5 100644 --- a/.github/workflows/golangci-lint.yml +++ b/.github/workflows/golangci-lint.yml @@ -1,5 +1,15 @@ name: golangci-lint +on: + pull_request: + push: + branches: + - main + paths-ignore: + - 'docs/**' + - 'deploy/**' + - '**.md' + permissions: contents: read From b26e645a0152259132043df9f06ddcae7a42ac73 Mon Sep 17 00:00:00 2001 From: Chen Chen Date: Tue, 11 Jul 2023 14:07:12 +0000 Subject: [PATCH 11/12] Add golint-check entry in makefile Signed-off-by: Chen Chen --- Makefile | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Makefile b/Makefile index 7b413141a0..fcbbb9f62a 100644 --- a/Makefile +++ b/Makefile @@ -128,6 +128,10 @@ static-check: ## Run verification script for boilerplate, codegen, gofmt, golint MAC_OS=$(MAC_OS) \ hack/verify-all.sh +.PHONY: golint-check +golint-check: + hack/verify-golint.sh + ############################### # Tests for ingress-nginx ############################### From 88be7be3ce772a60868a8aaab4b6d043150d0cd8 Mon Sep 17 00:00:00 2001 From: Chen Chen Date: Tue, 11 Jul 2023 14:12:47 +0000 Subject: [PATCH 12/12] Run golint-check in a container Signed-off-by: Chen Chen --- Makefile | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index fcbbb9f62a..792cd56a76 100644 --- a/Makefile +++ b/Makefile @@ -130,7 +130,9 @@ static-check: ## Run verification script for boilerplate, codegen, gofmt, golint .PHONY: golint-check golint-check: - hack/verify-golint.sh + @build/run-in-docker.sh \ + MAC_OS=$(MAC_OS) \ + hack/verify-golint.sh ############################### # Tests for ingress-nginx