From 2fc974d9e0fb27f97e0f47f85b1668412a3b8335 Mon Sep 17 00:00:00 2001 From: Andrea Frittoli Date: Fri, 10 Jan 2025 09:38:23 +0000 Subject: [PATCH] Switch to golangci-lint GH action Start using the golangci-lint github action and stop using the check as part of the build jobs. This way the version of the actions can be maintained by dependabot, the version of golangci-lint can be updated in the YAML and code in PRs can be annotated with issues. It also reduces the load in the prow job slighlty, saving a little infra cost. Removing the failing golang lint surfaced some issues in the yamllint. Issues in workflow YAMLs have been fixed. All testdata YAML files have been excluded from linting. Signed-off-by: Andrea Frittoli --- .github/workflows/bump-payload-on-main.yaml | 2 +- .../workflows/bump-payload-on-releases.yaml | 2 +- .github/workflows/golangci-lint.yaml | 25 +++++++++++++++++++ .github/workflows/helm-release.yaml | 2 +- pkg/reconciler/common/common.go | 10 ++++---- pkg/reconciler/common/deadlockbreaker.go | 2 +- .../tektoninstallerset/client/main_set.go | 3 ++- pkg/reconciler/platform/config.go | 3 ++- test/presubmit-tests.sh | 5 ++-- 9 files changed, 41 insertions(+), 13 deletions(-) create mode 100644 .github/workflows/golangci-lint.yaml diff --git a/.github/workflows/bump-payload-on-main.yaml b/.github/workflows/bump-payload-on-main.yaml index fca88e463f..f647473a1f 100644 --- a/.github/workflows/bump-payload-on-main.yaml +++ b/.github/workflows/bump-payload-on-main.yaml @@ -10,7 +10,7 @@ jobs: bump-payloads: name: "Bump payloads" runs-on: ubuntu-latest - if: github.repository_owner == 'tektoncd' # do not run this elsewhere + if: github.repository_owner == 'tektoncd' # do not run this elsewhere steps: - uses: actions/setup-go@v5 with: diff --git a/.github/workflows/bump-payload-on-releases.yaml b/.github/workflows/bump-payload-on-releases.yaml index 48d4e2f703..3bfc073a6e 100644 --- a/.github/workflows/bump-payload-on-releases.yaml +++ b/.github/workflows/bump-payload-on-releases.yaml @@ -9,7 +9,7 @@ on: # yamllint disable-line rule:truthy jobs: build-release-matrix: runs-on: ubuntu-latest - if: github.repository_owner == 'tektoncd' # do not run this elsewhere + if: github.repository_owner == 'tektoncd' # do not run this elsewhere steps: - id: set-matrix run: | diff --git a/.github/workflows/golangci-lint.yaml b/.github/workflows/golangci-lint.yaml new file mode 100644 index 0000000000..dfb8cd4759 --- /dev/null +++ b/.github/workflows/golangci-lint.yaml @@ -0,0 +1,25 @@ +name: golangci-lint +on: # yamllint disable-line rule:truthy + push: + branches: + - main + pull_request: # yamllint disable-line rule:empty-values + +permissions: + contents: read + checks: write # Used to annotate code in the PR + +jobs: + golangci: + name: lint + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + - uses: actions/setup-go@3041bf56c941b39c61721a86cd11f3bb1338122a # v5.2.0 + with: + go-version: stable + - name: golangci-lint + uses: golangci/golangci-lint-action@971e284b6050e8a5849b72094c50ab08da042db8 # v6.1.1 + with: + version: v1.60 + args: --timeout=10m diff --git a/.github/workflows/helm-release.yaml b/.github/workflows/helm-release.yaml index 2aaee33c0f..eab265be5c 100644 --- a/.github/workflows/helm-release.yaml +++ b/.github/workflows/helm-release.yaml @@ -9,7 +9,7 @@ jobs: permissions: contents: write runs-on: ubuntu-latest - if: github.repository_owner == 'tektoncd' # do not run this elsewhere + if: github.repository_owner == 'tektoncd' # do not run this elsewhere steps: - name: Checkout uses: actions/checkout@v4 diff --git a/pkg/reconciler/common/common.go b/pkg/reconciler/common/common.go index 87e07417a4..50f20a9e83 100644 --- a/pkg/reconciler/common/common.go +++ b/pkg/reconciler/common/common.go @@ -18,7 +18,7 @@ package common import ( "context" - "fmt" + "errors" "strings" "github.com/tektoncd/operator/pkg/apis/operator/v1alpha1" @@ -39,7 +39,7 @@ func PipelineReady(informer informer.TektonPipelineInformer) (*v1alpha1.TektonPi ppln, err := getPipelineRes(informer) if err != nil { if apierrors.IsNotFound(err) { - return nil, fmt.Errorf(PipelineNotFound) + return nil, errors.New(PipelineNotFound) } return nil, err } @@ -47,7 +47,7 @@ func PipelineReady(informer informer.TektonPipelineInformer) (*v1alpha1.TektonPi return nil, v1alpha1.DEPENDENCY_UPGRADE_PENDING_ERR } if !ppln.Status.IsReady() { - return nil, fmt.Errorf(PipelineNotReady) + return nil, errors.New(PipelineNotReady) } return ppln, nil } @@ -72,7 +72,7 @@ func TriggerReady(informer informer.TektonTriggerInformer) (*v1alpha1.TektonTrig trigger, err := getTriggerRes(informer) if err != nil { if apierrors.IsNotFound(err) { - return nil, fmt.Errorf(TriggerNotFound) + return nil, errors.New(TriggerNotFound) } return nil, err } @@ -80,7 +80,7 @@ func TriggerReady(informer informer.TektonTriggerInformer) (*v1alpha1.TektonTrig return nil, v1alpha1.DEPENDENCY_UPGRADE_PENDING_ERR } if !trigger.Status.IsReady() { - return nil, fmt.Errorf(TriggerNotReady) + return nil, errors.New(TriggerNotReady) } return trigger, nil } diff --git a/pkg/reconciler/common/deadlockbreaker.go b/pkg/reconciler/common/deadlockbreaker.go index 073ccd69b9..f0753af0c5 100644 --- a/pkg/reconciler/common/deadlockbreaker.go +++ b/pkg/reconciler/common/deadlockbreaker.go @@ -69,7 +69,7 @@ func isWebhookEndpointsActive(m *manifestival.Manifest, kc kubernetes.Interface, } return false, err } - if endPoint.Subsets == nil || len(endPoint.Subsets) == 0 { + if len(endPoint.Subsets) == 0 { return false, nil } return true, nil diff --git a/pkg/reconciler/kubernetes/tektoninstallerset/client/main_set.go b/pkg/reconciler/kubernetes/tektoninstallerset/client/main_set.go index 86cab7c798..9212c9dfcb 100644 --- a/pkg/reconciler/kubernetes/tektoninstallerset/client/main_set.go +++ b/pkg/reconciler/kubernetes/tektoninstallerset/client/main_set.go @@ -18,6 +18,7 @@ package client import ( "context" + "errors" "fmt" mf "github.com/manifestival/manifestival" @@ -113,7 +114,7 @@ func (i *InstallerSetClient) statusCheck(logger *zap.SugaredLogger, setType stri if !ready.IsTrue() { msg := fmt.Sprintf("%v/%v: installer set not ready, will retry: %v", i.resourceKind, setType, ready.Message) logger.Debugf(msg) - return fmt.Errorf(msg) + return errors.New(msg) } } return nil diff --git a/pkg/reconciler/platform/config.go b/pkg/reconciler/platform/config.go index 55c9b08194..2e0dd364dd 100644 --- a/pkg/reconciler/platform/config.go +++ b/pkg/reconciler/platform/config.go @@ -17,6 +17,7 @@ limitations under the License. package platform import ( + "errors" "flag" "fmt" "log" @@ -132,7 +133,7 @@ func validateConfig(pc *PlatformConfig) error { if len(violations) == 0 { return nil } - return fmt.Errorf(strings.Join(violations, ",")) + return errors.New(strings.Join(violations, ",")) } // stringToControllerNamesSlice returns a []ControllerName diff --git a/test/presubmit-tests.sh b/test/presubmit-tests.sh index f2893d30e3..6adc100eea 100755 --- a/test/presubmit-tests.sh +++ b/test/presubmit-tests.sh @@ -37,7 +37,7 @@ function check_go_lint() { function check_yaml_lint() { header "Testing if yamllint has been done" - local YAML_FILES=$(find . -path ./vendor -prune -o -type f -regex ".*y[a]ml" -print) + local YAML_FILES=$(find . \( -path ./vendor -prune \) -o \( -type d -name testdata -prune \) -o -type f -regex ".*y[a]ml" -print) yamllint -c .yamllint ${YAML_FILES} if [[ $? != 0 ]]; then @@ -49,7 +49,8 @@ function check_yaml_lint() { } function post_build_tests() { - check_go_lint + # golangci-lint is executed now via GitHub actions + # check_go_lint check_yaml_lint }