From 33835289f10fbf698e5f45f808432d59ca3992e6 Mon Sep 17 00:00:00 2001 From: joyceliu Date: Fri, 28 Jun 2024 11:12:48 +0800 Subject: [PATCH] doc: add github action Signed-off-by: joyceliu --- .github/workflows/golangci-lint.yml | 65 +++++++ .golangci.yaml | 259 +++++++++++++++++++++++++ Makefile | 11 +- hack/verify-dockerfiles.sh | 0 pkg/controllers/pipeline_controller.go | 12 +- pkg/modules/assert_test.go | 6 +- pkg/modules/command_test.go | 6 +- pkg/modules/copy_test.go | 6 +- pkg/modules/debug_test.go | 6 +- pkg/modules/fetch_test.go | 6 +- pkg/modules/gen_cert_test.go | 6 +- pkg/modules/template_test.go | 6 +- 12 files changed, 357 insertions(+), 32 deletions(-) create mode 100644 .github/workflows/golangci-lint.yml create mode 100644 .golangci.yaml create mode 100644 hack/verify-dockerfiles.sh diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml new file mode 100644 index 00000000..b6530fd8 --- /dev/null +++ b/.github/workflows/golangci-lint.yml @@ -0,0 +1,65 @@ +name: golangci-lint + +on: + pull_request: + types: [opened, edited, synchronize, reopened] + +# Remove all permissions from GITHUB_TOKEN except metadata. +permissions: {} + +jobs: + golangci: + name: lint + runs-on: ubuntu-latest + if: github.repository == 'kubesphere/kubekey' + steps: + - name: Checkout + uses: actions/checkout@v3 + with: + fetch-depth: 0 + + - name: Setup golang + uses: actions/setup-go@v5 + with: + go-version: 1.22 + + - name: golangci-lint + uses: golangci/golangci-lint-action@v6 + with: + version: v1.59 + + verify: + name: verify + runs-on: ubuntu-latest + if: github.repository == 'kubesphere/kubekey' + steps: + - name: Checkout + uses: actions/checkout@v3 + with: + fetch-depth: 0 + + - name: Setup golang + uses: actions/setup-go@v5 + with: + go-version: 1.22 + + - name: Verify + run: ALL_VERIFY_CHECKS=goimports make verify + + test: + name: test + runs-on: ubuntu-latest + if: github.repository == 'kubesphere/kubekey' + steps: + - name: Checkout + uses: actions/checkout@v3 + with: + fetch-depth: 0 + + - name: Setup golang + uses: actions/setup-go@v5 + with: + go-version: 1.22 + + - name: Test + run: make test diff --git a/.golangci.yaml b/.golangci.yaml new file mode 100644 index 00000000..657f5897 --- /dev/null +++ b/.golangci.yaml @@ -0,0 +1,259 @@ +linters: + disable-all: true + enable: + - asciicheck + - bodyclose + - containedctx + - deadcode + - depguard + - dogsled + - errcheck + - exportloopref + - gci + - goconst + - gocritic + - gofmt + - goimports + - goprintffuncname + - gosec + - gosimple + - govet + - importas + - ineffassign + - misspell + - nakedret + - nilerr + - noctx + - nolintlint + - prealloc + - predeclared + - revive + - rowserrcheck + - staticcheck + - structcheck + - stylecheck + - thelper + - typecheck + - unconvert + - unparam + - unused + - varcheck + - whitespace + +linters-settings: + godot: + # declarations - for top level declaration comments (default); + # toplevel - for top level comments; + # all - for all comments. + scope: toplevel + exclude: + - '^ \+.*' + - '^ ANCHOR.*' + ifshort: + # Maximum length of variable declaration measured in number of characters, after which linter won't suggest using short syntax. + max-decl-chars: 50 + gci: + local-prefixes: "github.com/kubesphere/kubekey" + importas: + no-unaliased: true + alias: + # oci + - pkg: github.com/opencontainers/image-spec/specs-go/v1 + alias: imagev1 + # Kubernetes + - pkg: "k8s.io/api/core/v1" + alias: corev1 + - pkg: "k8s.io/api/batch/v1" + alias: batchv1 + - pkg: "k8s.io/api/rbac/v1" + alias: rbacv1 + - pkg: "k8s.io/apimachinery/pkg/apis/meta/v1" + alias: metav1 + - pkg: "k8s.io/apimachinery/pkg/api/errors" + alias: apierrors + - pkg: "k8s.io/client-go/tools/cache" + alias: cgcache + - pkg: "k8s.io/client-go/util/cert" + alias: certutil + - pkg: "k8s.io/component-base/cli/flag" + alias: cliflag + - pkg: "k8s.io/utils/exec/testing" + alias: testingexec + - pkg: "k8s.io/utils/net" + alias: netutils + # Controller Runtime + - pkg: "sigs.k8s.io/controller-runtime" + alias: ctrl + - pkg: "sigs.k8s.io/controller-runtime/pkg/client" + alias: ctrlclient + - pkg: "sigs.k8s.io/controller-runtime/pkg/controller" + alias: ctrlcontroller + - pkg: "sigs.k8s.io/controller-runtime/pkg/finalizer" + alias: ctrlfinalizer + # kubekey + - pkg: "github.com/kubesphere/kubekey/v4/pkg/const" + alias: _const + - pkg: "github.com/kubesphere/kubekey/v4/pkg/apis/kubekey/v1" + alias: kubekeyv1 + - pkg: "github.com/kubesphere/kubekey/v4/pkg/apis/kubekey/v1alpha1" + alias: kubekeyv1alpha1 + - pkg: "github.com/kubesphere/kubekey/v4/pkg/apis/core/v1" + alias: kkcorev1 + nolintlint: + allow-unused: false + allow-leading-space: false + require-specific: true + revive: + rules: + # The following rules are recommended https://github.com/mgechev/revive#recommended-configuration + - name: blank-imports + - name: context-as-argument + - name: context-keys-type + - name: dot-imports + - name: error-return + - name: error-strings + - name: error-naming + - name: exported + #- name: if-return # TODO This is a recommended rule with many findings which may require it's own pr. + - name: increment-decrement + - name: var-naming + - name: var-declaration + - name: package-comments + - name: range + - name: receiver-naming + - name: time-naming + - name: unexported-return + - name: indent-error-flow + - name: errorf + - name: empty-block + - name: superfluous-else + #- name: unused-parameter # TODO This is a recommended rule with many findings which may require it's own pr. + - name: unreachable-code + - name: redefines-builtin-id + # + # Rules in addition to the recommended configuration above. + # + - name: bool-literal-in-expr + - name: constant-logical-expr + gosec: + excludes: + - G307 # Deferring unsafe method "Close" on type "\*os.File" + - G108 # Profiling endpoint is automatically exposed on /debug/pprof + gocritic: + enabled-tags: + - experimental + disabled-checks: + - appendAssign + - dupImport # https://github.com/go-critic/go-critic/issues/845 + - evalOrder + - ifElseChain + - octalLiteral + - regexpSimplify + - sloppyReassign + - truncateCmp + - typeDefFirst + - unnamedResult + - unnecessaryDefer + - whyNoLint + - wrapperFunc + - commentFormatting + - filepathJoin + - rangeValCopy + - hugeParam +issues: + max-same-issues: 0 + max-issues-per-linter: 0 + # We are disabling default golangci exclusions because we want to help reviewers to focus on reviewing the most relevant + # changes in PRs and avoid nitpicking. + exclude-use-default: false + exclude-rules: + - linters: + - revive + text: "exported: exported method .*\\.(Reconcile|SetupWithManager|SetupWebhookWithManager) should have comment or be unexported" + - linters: + - errcheck + text: Error return value of .((os\.)?std(out|err)\..*|.*Close|.*Flush|os\.Remove(All)?|.*print(f|ln)?|os\.(Un)?Setenv). is not checked + # Exclude revive's exported for certain packages and code, e.g. tests and fake. + - linters: + - revive + text: exported (method|function|type|const) (.+) should have comment or be unexported + source: (func|type).*Fake.* + - linters: + - revive + text: exported (method|function|type|const) (.+) should have comment or be unexported + path: fake_\.go + - linters: + - revive + text: exported (method|function|type|const) (.+) should have comment or be unexported + path: .*test/(providers|framework|e2e).*.go + - linters: + - errcheck + text: Error return value is not checked + path: _test\.go + - linters: + - errcheck + text: Error return value of (.+) is not checked + path: _test\.go + - linters: + - gosec + text: "G108: Profiling endpoint is automatically exposed on /debug/pprof" + - linters: + - godot + text: "Comment should end in a period" + path: "(.*)/(v1beta1|v1beta2)/(.*)types.go" + - linters: + - errcheck + text: Error return value of .((os\.)?std(out|err)\..*|.*Close|.*Flush|os\.Remove(All)?|.*print(f|ln)?|os\.(Un)?Setenv). is not checked + # With Go 1.16, the new embed directive can be used with an un-named import, + # revive (previously, golint) only allows these to be imported in a main.go, which wouldn't work for us. + # This directive allows the embed package to be imported with an underscore everywhere. + - linters: + - revive + source: _ "embed" + # This directive allows the variable in defaults.go files to have underscore + - linters: + - revive + text: "var-naming: don't use underscores in Go names; func (.+) should be (.+)" + path: .*/defaults.go + # Disable unparam "always receives" which might not be really + # useful when building libraries. + - linters: + - unparam + text: always receives + # Dot imports for gomega or ginkgo are allowed + # within test files. + - path: _test\.go + text: should not use dot imports + - path: (framework|e2e)/.*.go + text: should not use dot imports + - path: _test\.go + text: cyclomatic complexity + - linters: + - unparam + text: (.+) - (`t`|`g`) is unused + - path: _test\.go + text: cyclomatic complexity + # Append should be able to assign to a different var/slice. + - linters: + - gocritic + text: "appendAssign: append result not assigned to the same slice" + # We don't care about defer in for loops in test files. + - linters: + - gocritic + text: "deferInLoop: Possible resource leak, 'defer' is called in the 'for' loop" + path: _test\.go + +run: + timeout: 10m + go: "1.19" + build-tags: + - tools + - e2e + - containers_image_openpgp + - exclude_graphdriver_devicemapper + - exclude_graphdriver_btrfs + skip-files: + - "zz_generated.*\\.go$" + - "vendored_openapi\\.go$" + - "cmd" + allow-parallel-runners: true diff --git a/Makefile b/Makefile index ce0c72f4..0717f8bf 100644 --- a/Makefile +++ b/Makefile @@ -187,14 +187,15 @@ lint: $(GOLANGCI_LINT) ## Lint the codebase $(GOLANGCI_LINT) run -v $(GOLANGCI_LINT_EXTRA_ARGS) cd $(TEST_DIR); $(GOLANGCI_LINT) run -v $(GOLANGCI_LINT_EXTRA_ARGS) cd $(TOOLS_DIR); $(GOLANGCI_LINT) run -v $(GOLANGCI_LINT_EXTRA_ARGS) - ./scripts/ci-lint-dockerfiles.sh $(HADOLINT_VER) $(HADOLINT_FAILURE_THRESHOLD) -.PHONY: lint-dockerfiles -lint-dockerfiles: - ./scripts/ci-lint-dockerfiles.sh $(HADOLINT_VER) $(HADOLINT_FAILURE_THRESHOLD) +.PHONY: verify-dockerfiles +verify-dockerfiles: + ./hack/ci-lint-dockerfiles.sh $(HADOLINT_VER) $(HADOLINT_FAILURE_THRESHOLD) + +ALL_VERIFY_CHECKS ?= modules gen goimports .PHONY: verify -verify: $(addprefix verify-,$(ALL_VERIFY_CHECKS)) lint-dockerfiles ## Run all verify-* targets +verify: $(addprefix verify-,$(ALL_VERIFY_CHECKS)) ## Run all verify-* targets .PHONY: verify-modules verify-modules: ## Verify go modules are up to date diff --git a/hack/verify-dockerfiles.sh b/hack/verify-dockerfiles.sh new file mode 100644 index 00000000..e69de29b diff --git a/pkg/controllers/pipeline_controller.go b/pkg/controllers/pipeline_controller.go index a1741040..ae87af5c 100644 --- a/pkg/controllers/pipeline_controller.go +++ b/pkg/controllers/pipeline_controller.go @@ -23,7 +23,7 @@ import ( batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" - "k8s.io/apimachinery/pkg/api/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/tools/record" @@ -60,7 +60,7 @@ func (r PipelineReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct pipeline := &kubekeyv1.Pipeline{} err := r.Client.Get(ctx, req.NamespacedName, pipeline) if err != nil { - if errors.IsNotFound(err) { + if apierrors.IsNotFound(err) { klog.V(5).InfoS("pipeline not found", "pipeline", ctrlclient.ObjectKeyFromObject(pipeline)) return ctrl.Result{}, nil } @@ -108,7 +108,7 @@ func (r *PipelineReconciler) dealRunningPipeline(ctx context.Context, pipeline * jobs := &batchv1.JobList{} if err := r.Client.List(ctx, jobs, ctrlclient.InNamespace(pipeline.Namespace), ctrlclient.MatchingLabels{ jobLabel: pipeline.Name, - }); err != nil && !errors.IsNotFound(err) { + }); err != nil && !apierrors.IsNotFound(err) { return ctrl.Result{}, err } else if len(jobs.Items) != 0 { // could find exist job @@ -137,7 +137,7 @@ func (r *PipelineReconciler) dealRunningPipeline(ctx context.Context, pipeline * jobs := &batchv1.CronJobList{} if err := r.Client.List(ctx, jobs, ctrlclient.InNamespace(pipeline.Namespace), ctrlclient.MatchingLabels{ jobLabel: pipeline.Name, - }); err != nil && !errors.IsNotFound(err) { + }); err != nil && !apierrors.IsNotFound(err) { return ctrl.Result{}, err } else if len(jobs.Items) != 0 { @@ -198,7 +198,7 @@ func (r *PipelineReconciler) checkServiceAccount(ctx context.Context, pipeline k var sa = &corev1.ServiceAccount{} if err := r.Client.Get(ctx, ctrlclient.ObjectKey{Namespace: pipeline.Namespace, Name: saName}, sa); err != nil { - if !errors.IsNotFound(err) { + if !apierrors.IsNotFound(err) { klog.ErrorS(err, "get service account", "pipeline", ctrlclient.ObjectKeyFromObject(&pipeline)) return err } @@ -213,7 +213,7 @@ func (r *PipelineReconciler) checkServiceAccount(ctx context.Context, pipeline k var rb = &rbacv1.ClusterRoleBinding{} if err := r.Client.Get(ctx, ctrlclient.ObjectKey{Namespace: pipeline.Namespace, Name: saName}, rb); err != nil { - if !errors.IsNotFound(err) { + if !apierrors.IsNotFound(err) { klog.ErrorS(err, "create role binding error", "pipeline", ctrlclient.ObjectKeyFromObject(&pipeline)) return err } diff --git a/pkg/modules/assert_test.go b/pkg/modules/assert_test.go index 8b1be2d1..fe946113 100644 --- a/pkg/modules/assert_test.go +++ b/pkg/modules/assert_test.go @@ -21,7 +21,7 @@ import ( "testing" "time" - testassert "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/assert" "k8s.io/apimachinery/pkg/runtime" ) @@ -113,8 +113,8 @@ func TestAssert(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), time.Second*5) defer cancel() acStdout, acStderr := ModuleAssert(ctx, tc.opt) - testassert.Equal(t, tc.exceptStdout, acStdout) - testassert.Equal(t, tc.exceptStderr, acStderr) + assert.Equal(t, tc.exceptStdout, acStdout) + assert.Equal(t, tc.exceptStderr, acStderr) }) } } diff --git a/pkg/modules/command_test.go b/pkg/modules/command_test.go index a6827d94..36cb0fee 100644 --- a/pkg/modules/command_test.go +++ b/pkg/modules/command_test.go @@ -22,7 +22,7 @@ import ( "testing" "time" - testassert "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/assert" "k8s.io/apimachinery/pkg/runtime" ) @@ -73,8 +73,8 @@ func TestCommand(t *testing.T) { ctx, cancel := context.WithTimeout(tc.ctx, time.Second*5) defer cancel() acStdout, acStderr := ModuleCommand(ctx, tc.opt) - testassert.Equal(t, tc.exceptStdout, acStdout) - testassert.Equal(t, tc.exceptStderr, acStderr) + assert.Equal(t, tc.exceptStdout, acStdout) + assert.Equal(t, tc.exceptStderr, acStderr) }) } } diff --git a/pkg/modules/copy_test.go b/pkg/modules/copy_test.go index ba866226..074f462c 100644 --- a/pkg/modules/copy_test.go +++ b/pkg/modules/copy_test.go @@ -22,7 +22,7 @@ import ( "testing" "time" - testassert "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/assert" "k8s.io/apimachinery/pkg/runtime" ) @@ -109,8 +109,8 @@ func TestCopy(t *testing.T) { ctx, cancel := context.WithTimeout(tc.ctx, time.Second*5) defer cancel() acStdout, acStderr := ModuleCopy(ctx, tc.opt) - testassert.Equal(t, tc.exceptStdout, acStdout) - testassert.Equal(t, tc.exceptStderr, acStderr) + assert.Equal(t, tc.exceptStdout, acStdout) + assert.Equal(t, tc.exceptStderr, acStderr) }) } } diff --git a/pkg/modules/debug_test.go b/pkg/modules/debug_test.go index 0644be27..a9e0d25a 100644 --- a/pkg/modules/debug_test.go +++ b/pkg/modules/debug_test.go @@ -21,7 +21,7 @@ import ( "testing" "time" - testassert "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/assert" "k8s.io/apimachinery/pkg/runtime" ) @@ -78,8 +78,8 @@ func TestDebug(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), time.Second*5) defer cancel() acStdout, acStderr := ModuleDebug(ctx, tc.opt) - testassert.Equal(t, tc.exceptStdout, acStdout) - testassert.Equal(t, tc.exceptStderr, acStderr) + assert.Equal(t, tc.exceptStdout, acStdout) + assert.Equal(t, tc.exceptStderr, acStderr) }) } } diff --git a/pkg/modules/fetch_test.go b/pkg/modules/fetch_test.go index fdf99132..ced8ebf0 100644 --- a/pkg/modules/fetch_test.go +++ b/pkg/modules/fetch_test.go @@ -21,7 +21,7 @@ import ( "testing" "time" - testassert "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/assert" "k8s.io/apimachinery/pkg/runtime" ) @@ -65,8 +65,8 @@ func TestFetch(t *testing.T) { ctx, cancel := context.WithTimeout(tc.ctx, time.Second*5) defer cancel() acStdout, acStderr := ModuleFetch(ctx, tc.opt) - testassert.Equal(t, tc.exceptStdout, acStdout) - testassert.Equal(t, tc.exceptStderr, acStderr) + assert.Equal(t, tc.exceptStdout, acStdout) + assert.Equal(t, tc.exceptStderr, acStderr) }) } } diff --git a/pkg/modules/gen_cert_test.go b/pkg/modules/gen_cert_test.go index 44b8669d..415071f2 100644 --- a/pkg/modules/gen_cert_test.go +++ b/pkg/modules/gen_cert_test.go @@ -21,7 +21,7 @@ import ( "os" "testing" - testassert "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/assert" "k8s.io/apimachinery/pkg/runtime" ) @@ -61,8 +61,8 @@ func TestModuleGenCert(t *testing.T) { for _, testcase := range testcases { t.Run(testcase.name, func(t *testing.T) { stdout, stderr := ModuleGenCert(context.Background(), testcase.opt) - testassert.Equal(t, testcase.exceptStdout, stdout) - testassert.Equal(t, testcase.exceptStderr, stderr) + assert.Equal(t, testcase.exceptStdout, stdout) + assert.Equal(t, testcase.exceptStderr, stderr) }) } } diff --git a/pkg/modules/template_test.go b/pkg/modules/template_test.go index e1e8914b..005c6a2b 100644 --- a/pkg/modules/template_test.go +++ b/pkg/modules/template_test.go @@ -24,7 +24,7 @@ import ( "testing" "time" - testassert "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/assert" "k8s.io/apimachinery/pkg/runtime" ) @@ -71,8 +71,8 @@ func TestTemplate(t *testing.T) { ctx, cancel := context.WithTimeout(tc.ctx, time.Second*5) defer cancel() acStdout, acStderr := ModuleTemplate(ctx, tc.opt) - testassert.Equal(t, tc.exceptStdout, acStdout) - testassert.Equal(t, tc.exceptStderr, acStderr) + assert.Equal(t, tc.exceptStdout, acStdout) + assert.Equal(t, tc.exceptStderr, acStderr) }) } }