From d22ea3432ee3c4a3b9c3d7e96868c94c84ec763e Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Wed, 22 Jan 2025 12:52:51 +0000 Subject: [PATCH 1/4] Add custom KAL golangci-lint in-place of golangci-lint --- .golangci.yml | 36 ++++++++++++++++++++++++++++++++++++ Makefile | 15 +++++++++++---- hack/tools/.custom-gcl.yaml | 6 ++++++ 3 files changed, 53 insertions(+), 4 deletions(-) create mode 100644 hack/tools/.custom-gcl.yaml diff --git a/.golangci.yml b/.golangci.yml index 09b15826482d..78d0116a32ab 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -33,6 +33,7 @@ linters: - importas # consistent import aliases - ineffassign # ineffectual assignments - intrange # suggest using integer range in for loops + - kal # linter for Kube API conventions - loggercheck # check for even key/value pairs in logger calls - misspell # spelling - nakedret # naked returns (named return parameters and an empty return) @@ -226,6 +227,37 @@ linters-settings: - name: constant-logical-expr goconst: ignore-tests: true + custom: + kal: + type: "module" + description: KAL is the Kube-API-Linter and lints Kube like APIs based on API conventions and best practices. + settings: + linters: + enable: + - "conditions" # Ensure conditions have the correct json tags and markers. + - "commentstart" # Ensure comments start with the serialized version of the field name. + - "integers" # Ensure only int32 and int64 are used for integers. + - "jsontags" # Ensure every field has a json tag. + - "maxlength" # Ensure all strings and arrays have maximum lengths/maximum items. + - "nobools" # Bools do not evolve over time, should use enums instead. + - "nophase" # Phase fields are discouraged by the Kube API conventions, use conditions instead. + - "optionalorrequired" # Every field should be marked as `+optional` or `+required`. + - "requiredfields" # Required fields should not be pointers, and should not have `omitempty`. + - "statussubresource" # All root objects that have a `status` field should have a status subresource. + disable: + - "*" # We will manually enable new linters after understanding the impact. Disable all by default. + lintersConfig: + conditions: + isFirstField: Warn # Require conditions to be the first field in the status struct. + useProtobuf: Ignore # We don't use protobuf, so missing protobuf tags are ignored. + # jsonTags: + # jsonTagRegex: "^[a-z][a-z0-9]*(?:[A-Z][a-z0-9]*)*$" # The default regex is appropriate for our use case. + # optionalOrRequired: + # preferredOptionalMarker: optional | kubebuilder:validation:Optional # The preferred optional marker to use, fixes will suggest to use this marker. Defaults to `optional`. + # preferredRequiredMarker: required | kubebuilder:validation:Required # The preferred required marker to use, fixes will suggest to use this marker. Defaults to `required`. + # requiredFields: + # pointerPolicy: Warn | SuggestFix # Defaults to `SuggestFix`. We want our required fields to not be pointers. + issues: exclude-files: - "zz_generated.*\\.go$" @@ -369,3 +401,7 @@ issues: - linters: - govet text: "non-constant format string in call to sigs\\.k8s\\.io\\/cluster-api\\/util\\/conditions\\." + # KAL should only run on API folders. + - path-except: "api/v1beta2/*" + linters: + - kal diff --git a/Makefile b/Makefile index ea4f170f15d6..698d4a76a076 100644 --- a/Makefile +++ b/Makefile @@ -167,6 +167,10 @@ GOLANGCI_LINT_VER := $(shell cat .github/workflows/pr-golangci-lint.yaml | grep GOLANGCI_LINT := $(abspath $(TOOLS_BIN_DIR)/$(GOLANGCI_LINT_BIN)-$(GOLANGCI_LINT_VER)) GOLANGCI_LINT_PKG := github.com/golangci/golangci-lint/cmd/golangci-lint +GOLANGCI_LINT_KAL_BIN := golangci-lint-kal +GOLANGCI_LINT_KAL_VER := $(shell cat ./hack/tools/.custom-gcl.yaml | grep name: | sed 's/name: golangci-lint-kal-//') +GOLANGCI_LINT_KAL := $(abspath $(TOOLS_BIN_DIR)/$(GOLANGCI_LINT_KAL_BIN)-$(GOLANGCI_LINT_KAL_VER)) + GOVULNCHECK_BIN := govulncheck GOVULNCHECK_VER := v1.1.4 GOVULNCHECK := $(abspath $(TOOLS_BIN_DIR)/$(GOVULNCHECK_BIN)-$(GOVULNCHECK_VER)) @@ -660,10 +664,10 @@ generate-test-infra-prowjobs: $(PROWJOB_GEN) ## Generates the prowjob configurat ##@ lint and verify: .PHONY: lint -lint: $(GOLANGCI_LINT) ## Lint the codebase - $(GOLANGCI_LINT) run -v $(GOLANGCI_LINT_EXTRA_ARGS) - cd $(TEST_DIR); $(GOLANGCI_LINT) run --path-prefix $(TEST_DIR) --config $(ROOT_DIR)/.golangci.yml -v $(GOLANGCI_LINT_EXTRA_ARGS) - cd $(TOOLS_DIR); $(GOLANGCI_LINT) run --path-prefix $(TOOLS_DIR) --config $(ROOT_DIR)/.golangci.yml -v $(GOLANGCI_LINT_EXTRA_ARGS) +lint: $(GOLANGCI_LINT) $(GOLANGCI_LINT_KAL) ## Lint the codebase + $(GOLANGCI_LINT_KAL) run -v $(GOLANGCI_LINT_EXTRA_ARGS) + cd $(TEST_DIR); $(GOLANGCI_LINT_KAL) run --path-prefix $(TEST_DIR) --config $(ROOT_DIR)/.golangci.yml -v $(GOLANGCI_LINT_EXTRA_ARGS) + cd $(TOOLS_DIR); $(GOLANGCI_LINT_KAL) run --path-prefix $(TOOLS_DIR) --config $(ROOT_DIR)/.golangci.yml -v $(GOLANGCI_LINT_EXTRA_ARGS) ./scripts/lint-dockerfiles.sh $(HADOLINT_VER) $(HADOLINT_FAILURE_THRESHOLD) .PHONY: lint-dockerfiles @@ -1501,6 +1505,9 @@ $(GINKGO): # Build ginkgo from tools folder. $(GOLANGCI_LINT): # Build golangci-lint from tools folder. GOBIN=$(TOOLS_BIN_DIR) $(GO_INSTALL) $(GOLANGCI_LINT_PKG) $(GOLANGCI_LINT_BIN) $(GOLANGCI_LINT_VER) +$(GOLANGCI_LINT_KAL): $(GOLANGCI_LINT) # Build golangci-lint-kal from custom configuration. + cd $(TOOLS_DIR); $(GOLANGCI_LINT) custom + $(GOVULNCHECK): # Build govulncheck. GOBIN=$(TOOLS_BIN_DIR) $(GO_INSTALL) $(GOVULNCHECK_PKG) $(GOVULNCHECK_BIN) $(GOVULNCHECK_VER) diff --git a/hack/tools/.custom-gcl.yaml b/hack/tools/.custom-gcl.yaml new file mode 100644 index 000000000000..212d92f197fd --- /dev/null +++ b/hack/tools/.custom-gcl.yaml @@ -0,0 +1,6 @@ +version: v1.63.4 +name: golangci-lint-kal-v1.63.4 +destination: ./bin +plugins: +- module: 'github.com/JoelSpeed/kal' + version: v0.0.0-20250121143106-304c215e474e From 000d87f43341da75582db36e00921d1c56e1c13e Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Wed, 22 Jan 2025 13:01:13 +0000 Subject: [PATCH 2/4] Move KAL to separate golangci-lint run This will make it easier to integrate into the GitHub actions workflow as the regular linters can be run completely separately using the upstream golangci-lint, and not the custom build --- .golangci-kal.yml | 55 +++++++++++++++++++++++++++++++++++++++++++++++ .golangci.yml | 35 ------------------------------ Makefile | 15 ++++++++++--- 3 files changed, 67 insertions(+), 38 deletions(-) create mode 100644 .golangci-kal.yml diff --git a/.golangci-kal.yml b/.golangci-kal.yml new file mode 100644 index 000000000000..cdd612b9faee --- /dev/null +++ b/.golangci-kal.yml @@ -0,0 +1,55 @@ +run: + timeout: 10m + go: "1.23" + allow-parallel-runners: true + +linters: + disable-all: true + enable: + - kal # linter for Kube API conventions + +linters-settings: + custom: + kal: + type: "module" + description: KAL is the Kube-API-Linter and lints Kube like APIs based on API conventions and best practices. + settings: + linters: + enable: + - "conditions" # Ensure conditions have the correct json tags and markers. + - "commentstart" # Ensure comments start with the serialized version of the field name. + - "integers" # Ensure only int32 and int64 are used for integers. + - "jsontags" # Ensure every field has a json tag. + - "maxlength" # Ensure all strings and arrays have maximum lengths/maximum items. + - "nobools" # Bools do not evolve over time, should use enums instead. + - "nophase" # Phase fields are discouraged by the Kube API conventions, use conditions instead. + - "optionalorrequired" # Every field should be marked as `+optional` or `+required`. + - "requiredfields" # Required fields should not be pointers, and should not have `omitempty`. + - "statussubresource" # All root objects that have a `status` field should have a status subresource. + disable: + - "*" # We will manually enable new linters after understanding the impact. Disable all by default. + lintersConfig: + conditions: + isFirstField: Warn # Require conditions to be the first field in the status struct. + useProtobuf: Ignore # We don't use protobuf, so missing protobuf tags are ignored. + # jsonTags: + # jsonTagRegex: "^[a-z][a-z0-9]*(?:[A-Z][a-z0-9]*)*$" # The default regex is appropriate for our use case. + # optionalOrRequired: + # preferredOptionalMarker: optional | kubebuilder:validation:Optional # The preferred optional marker to use, fixes will suggest to use this marker. Defaults to `optional`. + # preferredRequiredMarker: required | kubebuilder:validation:Required # The preferred required marker to use, fixes will suggest to use this marker. Defaults to `required`. + # requiredFields: + # pointerPolicy: Warn | SuggestFix # Defaults to `SuggestFix`. We want our required fields to not be pointers. + +issues: + exclude-files: + - "zz_generated.*\\.go$" + - "vendored_openapi\\.go$" + # We don't want to invest time to fix new linter findings in old API types. + - "internal/apis/.*" + max-same-issues: 0 + max-issues-per-linter: 0 + exclude-rules: + # KAL should only run on API folders. + - path-except: "api/v1beta2/*" + linters: + - kal diff --git a/.golangci.yml b/.golangci.yml index 78d0116a32ab..072f3f32752b 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -33,7 +33,6 @@ linters: - importas # consistent import aliases - ineffassign # ineffectual assignments - intrange # suggest using integer range in for loops - - kal # linter for Kube API conventions - loggercheck # check for even key/value pairs in logger calls - misspell # spelling - nakedret # naked returns (named return parameters and an empty return) @@ -227,36 +226,6 @@ linters-settings: - name: constant-logical-expr goconst: ignore-tests: true - custom: - kal: - type: "module" - description: KAL is the Kube-API-Linter and lints Kube like APIs based on API conventions and best practices. - settings: - linters: - enable: - - "conditions" # Ensure conditions have the correct json tags and markers. - - "commentstart" # Ensure comments start with the serialized version of the field name. - - "integers" # Ensure only int32 and int64 are used for integers. - - "jsontags" # Ensure every field has a json tag. - - "maxlength" # Ensure all strings and arrays have maximum lengths/maximum items. - - "nobools" # Bools do not evolve over time, should use enums instead. - - "nophase" # Phase fields are discouraged by the Kube API conventions, use conditions instead. - - "optionalorrequired" # Every field should be marked as `+optional` or `+required`. - - "requiredfields" # Required fields should not be pointers, and should not have `omitempty`. - - "statussubresource" # All root objects that have a `status` field should have a status subresource. - disable: - - "*" # We will manually enable new linters after understanding the impact. Disable all by default. - lintersConfig: - conditions: - isFirstField: Warn # Require conditions to be the first field in the status struct. - useProtobuf: Ignore # We don't use protobuf, so missing protobuf tags are ignored. - # jsonTags: - # jsonTagRegex: "^[a-z][a-z0-9]*(?:[A-Z][a-z0-9]*)*$" # The default regex is appropriate for our use case. - # optionalOrRequired: - # preferredOptionalMarker: optional | kubebuilder:validation:Optional # The preferred optional marker to use, fixes will suggest to use this marker. Defaults to `optional`. - # preferredRequiredMarker: required | kubebuilder:validation:Required # The preferred required marker to use, fixes will suggest to use this marker. Defaults to `required`. - # requiredFields: - # pointerPolicy: Warn | SuggestFix # Defaults to `SuggestFix`. We want our required fields to not be pointers. issues: exclude-files: @@ -401,7 +370,3 @@ issues: - linters: - govet text: "non-constant format string in call to sigs\\.k8s\\.io\\/cluster-api\\/util\\/conditions\\." - # KAL should only run on API folders. - - path-except: "api/v1beta2/*" - linters: - - kal diff --git a/Makefile b/Makefile index 698d4a76a076..097d4d276a5d 100644 --- a/Makefile +++ b/Makefile @@ -665,10 +665,11 @@ generate-test-infra-prowjobs: $(PROWJOB_GEN) ## Generates the prowjob configurat .PHONY: lint lint: $(GOLANGCI_LINT) $(GOLANGCI_LINT_KAL) ## Lint the codebase - $(GOLANGCI_LINT_KAL) run -v $(GOLANGCI_LINT_EXTRA_ARGS) - cd $(TEST_DIR); $(GOLANGCI_LINT_KAL) run --path-prefix $(TEST_DIR) --config $(ROOT_DIR)/.golangci.yml -v $(GOLANGCI_LINT_EXTRA_ARGS) - cd $(TOOLS_DIR); $(GOLANGCI_LINT_KAL) run --path-prefix $(TOOLS_DIR) --config $(ROOT_DIR)/.golangci.yml -v $(GOLANGCI_LINT_EXTRA_ARGS) + $(GOLANGCI_LINT) run -v $(GOLANGCI_LINT_EXTRA_ARGS) + cd $(TEST_DIR); $(GOLANGCI_LINT) run --path-prefix $(TEST_DIR) --config $(ROOT_DIR)/.golangci.yml -v $(GOLANGCI_LINT_EXTRA_ARGS) + cd $(TOOLS_DIR); $(GOLANGCI_LINT) run --path-prefix $(TOOLS_DIR) --config $(ROOT_DIR)/.golangci.yml -v $(GOLANGCI_LINT_EXTRA_ARGS) ./scripts/lint-dockerfiles.sh $(HADOLINT_VER) $(HADOLINT_FAILURE_THRESHOLD) + $(GOLANGCI_LINT_KAL) run -v --config $(ROOT_DIR)/.golangci-kal.yml $(GOLANGCI_LINT_EXTRA_ARGS) .PHONY: lint-dockerfiles lint-dockerfiles: @@ -678,6 +679,14 @@ lint-dockerfiles: lint-fix: $(GOLANGCI_LINT) ## Lint the codebase and run auto-fixers if supported by the linter GOLANGCI_LINT_EXTRA_ARGS=--fix $(MAKE) lint +.PHONY: lint-api +lint-api: $(GOLANGCI_LINT_KAL) + $(GOLANGCI_LINT_KAL) run -v --config $(ROOT_DIR)/.golangci-kal.yml $(GOLANGCI_LINT_EXTRA_ARGS) + +.PHONY: lint-api-fix +lint-api-fix: $(GOLANGCI_LINT_KAL) + GOLANGCI_LINT_EXTRA_ARGS=--fix $(MAKE) lint-api + .PHONY: tiltfile-fix tiltfile-fix: ## Format the Tiltfile TRACE=$(TRACE) ./hack/verify-starlark.sh fix From 1dce912487c21e4214fd69bd033cd9d2593486c8 Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Wed, 22 Jan 2025 13:03:35 +0000 Subject: [PATCH 3/4] Add KAL to lint workflow --- .github/workflows/pr-golangci-lint.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/pr-golangci-lint.yaml b/.github/workflows/pr-golangci-lint.yaml index 6cd9c43fbeb0..7b1c0326e54f 100644 --- a/.github/workflows/pr-golangci-lint.yaml +++ b/.github/workflows/pr-golangci-lint.yaml @@ -33,3 +33,5 @@ jobs: version: v1.63.4 args: --out-format=colored-line-number working-directory: ${{matrix.working-directory}} + - name: Lint API + run: GOLANGCI_LINT_EXTRA_ARGS=--out-format=colored-line-number make lint-api From ab11d8e9da482ae371e5c0c087e95e2b6e448d73 Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Wed, 22 Jan 2025 13:04:50 +0000 Subject: [PATCH 4/4] DNM: Lint v1beta1 folder --- .golangci-kal.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.golangci-kal.yml b/.golangci-kal.yml index cdd612b9faee..7488841e93bf 100644 --- a/.golangci-kal.yml +++ b/.golangci-kal.yml @@ -50,6 +50,6 @@ issues: max-issues-per-linter: 0 exclude-rules: # KAL should only run on API folders. - - path-except: "api/v1beta2/*" + - path-except: "api/v1beta1/*" linters: - kal