From b73cc57b204fd1924b4b24b07622458a6dbef67b Mon Sep 17 00:00:00 2001 From: Michael Bolot Date: Fri, 14 Oct 2022 15:21:17 -0500 Subject: [PATCH] Adding validate phase to the CI Adds a validate phase to the CI which runs a linter. Also fixes linter issues discovered during the initial run --- .drone.yml | 16 ++++++ .golangci.json | 67 ++++++++++++++++++++++++ Makefile | 5 +- pkg/auth/cli/webhookcli.go | 2 +- pkg/auth/filter.go | 2 +- pkg/podimpersonation/podimpersonation.go | 8 +-- pkg/resources/counts/counts.go | 4 +- pkg/schema/converter/crd.go | 2 +- pkg/schema/converter/k8stonorman.go | 2 +- pkg/schema/factory.go | 6 +-- pkg/server/handler/handlers.go | 5 -- pkg/stores/metrics/metrics_store.go | 2 +- pkg/stores/partition/parallel.go | 19 ++++--- pkg/stores/proxy/rbac_store.go | 13 +++-- scripts/validate.sh | 12 +++++ 15 files changed, 128 insertions(+), 37 deletions(-) create mode 100644 .golangci.json create mode 100644 scripts/validate.sh diff --git a/.drone.yml b/.drone.yml index ae9db171..0902f3ae 100644 --- a/.drone.yml +++ b/.drone.yml @@ -29,6 +29,22 @@ steps: - pull_request --- kind: pipeline +name: validate + +steps: + - name: validate + image: registry.suse.com/bci/bci-base:15.4 + commands: + - zypper in -y go=1.19 git tar gzip make + - curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s v1.49.0 + - mv ./bin/golangci-lint /usr/local/bin/golangci-lint + - make validate + when: + event: + - push + - pull_request +--- +kind: pipeline name: test steps: diff --git a/.golangci.json b/.golangci.json new file mode 100644 index 00000000..b998d60c --- /dev/null +++ b/.golangci.json @@ -0,0 +1,67 @@ +{ + "linters": { + "disable-all": true, + "enable": [ + "govet", + "revive", + "goimports", + "misspell", + "ineffassign", + "gofmt" + ] + }, + "linters-settings": { + "govet": { + "check-shadowing": false + }, + "gofmt": { + "simplify": false + } + }, + "run": { + "skip-dirs": [ + "vendor", + "tests", + "pkg/client", + "pkg/generated" + ], + "tests": false, + "timeout": "10m" + }, + "issues": { + "exclude-rules": [ + { + "linters": "govet", + "text": "^(nilness|structtag)" + }, + { + "path":"pkg/apis/management.cattle.io/v3/globaldns_types.go", + "text":".*lobalDns.*" + }, + { + "path": "pkg/apis/management.cattle.io/v3/zz_generated_register.go", + "text":".*lobalDns.*" + }, + { + "path":"pkg/apis/management.cattle.io/v3/zz_generated_list_types.go", + "text":".*lobalDns.*" + }, + { + "linters": "revive", + "text": "should have comment" + }, + { + "linters": "revive", + "text": "should be of the form" + }, + { + "linters": "revive", + "text": "by other packages, and that stutters" + }, + { + "linters": "typecheck", + "text": "imported but not used as apierrors" + } + ] + } +} \ No newline at end of file diff --git a/Makefile b/Makefile index 5dc381cc..5f2ebc16 100644 --- a/Makefile +++ b/Makefile @@ -11,4 +11,7 @@ run-host: build docker run $(DOCKER_ARGS) --net=host --uts=host --rm -it -v ${HOME}/.kube:/root/.kube steve --kubeconfig /root/.kube/config --http-listen-port 8989 --https-listen-port 0 test: - bash scripts/test.sh \ No newline at end of file + bash scripts/test.sh + +validate: + bash scripts/validate.sh \ No newline at end of file diff --git a/pkg/auth/cli/webhookcli.go b/pkg/auth/cli/webhookcli.go index e3d9c4d1..8ead8ae5 100644 --- a/pkg/auth/cli/webhookcli.go +++ b/pkg/auth/cli/webhookcli.go @@ -1,12 +1,12 @@ package cli import ( - "k8s.io/client-go/tools/clientcmd" "os" "time" "github.com/rancher/steve/pkg/auth" "github.com/urfave/cli" + "k8s.io/client-go/tools/clientcmd" ) type WebhookConfig struct { diff --git a/pkg/auth/filter.go b/pkg/auth/filter.go index 472366a4..26cfb745 100644 --- a/pkg/auth/filter.go +++ b/pkg/auth/filter.go @@ -2,7 +2,6 @@ package auth import ( "io/ioutil" - "k8s.io/client-go/rest" "net/http" "strings" "time" @@ -13,6 +12,7 @@ import ( "k8s.io/apiserver/pkg/authentication/user" "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/plugin/pkg/authenticator/token/webhook" + "k8s.io/client-go/rest" "k8s.io/client-go/tools/clientcmd" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" "k8s.io/client-go/transport" diff --git a/pkg/podimpersonation/podimpersonation.go b/pkg/podimpersonation/podimpersonation.go index 30e3ceda..01b82169 100644 --- a/pkg/podimpersonation/podimpersonation.go +++ b/pkg/podimpersonation/podimpersonation.go @@ -128,10 +128,10 @@ type PodOptions struct { // CreatePod will create a pod with a service account that impersonates as user. Corresponding // ClusterRoles, ClusterRoleBindings, and ServiceAccounts will be create. // IMPORTANT NOTES: -// 1. To ensure this is used securely the namespace assigned to the pod must be a dedicated -// namespace used only for the purpose of running impersonated pods. This is to ensure -// proper protection for the service accounts created. -// 2. The pod must KUBECONFIG env var set to where you expect the kubeconfig to reside +// 1. To ensure this is used securely the namespace assigned to the pod must be a dedicated +// namespace used only for the purpose of running impersonated pods. This is to ensure +// proper protection for the service accounts created. +// 2. The pod must KUBECONFIG env var set to where you expect the kubeconfig to reside func (s *PodImpersonation) CreatePod(ctx context.Context, user user.Info, pod *v1.Pod, podOptions *PodOptions) (*v1.Pod, error) { if podOptions == nil { podOptions = &PodOptions{} diff --git a/pkg/resources/counts/counts.go b/pkg/resources/counts/counts.go index a44d44b1..aa51b136 100644 --- a/pkg/resources/counts/counts.go +++ b/pkg/resources/counts/counts.go @@ -280,7 +280,7 @@ func removeSummary(counts Summary, summary summary.Summary) Summary { if counts.States == nil { counts.States = map[string]int{} } - counts.States[simpleState(summary)] -= 1 + counts.States[simpleState(summary)]-- } return counts } @@ -297,7 +297,7 @@ func addSummary(counts Summary, summary summary.Summary) Summary { if counts.States == nil { counts.States = map[string]int{} } - counts.States[simpleState(summary)] += 1 + counts.States[simpleState(summary)]++ } return counts } diff --git a/pkg/schema/converter/crd.go b/pkg/schema/converter/crd.go index 1e021452..874f8a62 100644 --- a/pkg/schema/converter/crd.go +++ b/pkg/schema/converter/crd.go @@ -6,7 +6,7 @@ import ( "github.com/rancher/steve/pkg/schema/table" apiextv1 "github.com/rancher/wrangler/pkg/generated/controllers/apiextensions.k8s.io/v1" "github.com/rancher/wrangler/pkg/schemas" - "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" ) diff --git a/pkg/schema/converter/k8stonorman.go b/pkg/schema/converter/k8stonorman.go index cba8e106..1c3c9415 100644 --- a/pkg/schema/converter/k8stonorman.go +++ b/pkg/schema/converter/k8stonorman.go @@ -5,7 +5,7 @@ import ( "strings" "github.com/rancher/apiserver/pkg/types" - "github.com/rancher/wrangler/pkg/generated/controllers/apiextensions.k8s.io/v1" + v1 "github.com/rancher/wrangler/pkg/generated/controllers/apiextensions.k8s.io/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/discovery" ) diff --git a/pkg/schema/factory.go b/pkg/schema/factory.go index 5f7ebd9f..b6f0e4b3 100644 --- a/pkg/schema/factory.go +++ b/pkg/schema/factory.go @@ -41,11 +41,11 @@ func (c *Collection) Schemas(user user.Info) (*types.APISchemas, error) { func (c *Collection) removeOldRecords(access *accesscontrol.AccessSet, user user.Info) { current, ok := c.userCache.Get(user.GetName()) if ok { - currentId, cOk := current.(string) - if cOk && currentId != access.ID { + currentID, cOk := current.(string) + if cOk && currentID != access.ID { // we only want to keep around one record per user. If our current access record is invalid, purge the //record of it from the cache, so we don't keep duplicates - c.purgeUserRecords(currentId) + c.purgeUserRecords(currentID) c.userCache.Remove(user.GetName()) } } diff --git a/pkg/server/handler/handlers.go b/pkg/server/handler/handlers.go index 0b4fa3e7..79cfe5ba 100644 --- a/pkg/server/handler/handlers.go +++ b/pkg/server/handler/handlers.go @@ -9,11 +9,6 @@ import ( func k8sAPI(sf schema.Factory, apiOp *types.APIRequest) { vars := mux.Vars(apiOp.Request) - group := vars["group"] - if group == "core" { - group = "" - } - apiOp.Name = vars["name"] apiOp.Type = vars["type"] diff --git a/pkg/stores/metrics/metrics_store.go b/pkg/stores/metrics/metrics_store.go index 1e316a1d..6a434c4b 100644 --- a/pkg/stores/metrics/metrics_store.go +++ b/pkg/stores/metrics/metrics_store.go @@ -63,4 +63,4 @@ func (s *Store) Watch(apiOp *types.APIRequest, schema *types.APISchema, w types. apiEvent, err := s.Store.Watch(apiOp, schema, w) m.RecordProxyStoreResponseTime(err, float64(time.Since(storeStart).Milliseconds())) return apiEvent, err -} \ No newline at end of file +} diff --git a/pkg/stores/partition/parallel.go b/pkg/stores/partition/parallel.go index e1901d31..1b77486a 100644 --- a/pkg/stores/partition/parallel.go +++ b/pkg/stores/partition/parallel.go @@ -170,17 +170,16 @@ func (p *ParallelPartitionLister) feeder(ctx context.Context, state listState, l } capacity = 0 return nil - } else { - result <- list.Objects - capacity -= len(list.Objects) - if list.Continue == "" { - return nil - } - // loop again and get more data - state.Continue = list.Continue - state.PartitionName = partition.Name() - state.Offset = 0 } + result <- list.Objects + capacity -= len(list.Objects) + if list.Continue == "" { + return nil + } + // loop again and get more data + state.Continue = list.Continue + state.PartitionName = partition.Name() + state.Offset = 0 } }) } diff --git a/pkg/stores/proxy/rbac_store.go b/pkg/stores/proxy/rbac_store.go index a5fc4167..31c73bca 100644 --- a/pkg/stores/proxy/rbac_store.go +++ b/pkg/stores/proxy/rbac_store.go @@ -166,14 +166,13 @@ func isPassthroughUnconstrained(apiOp *types.APIRequest, schema *types.APISchema if apiOp.Namespace != "" { if resources[apiOp.Namespace].All { return nil, true - } else { - return []partition.Partition{ - Partition{ - Namespace: apiOp.Namespace, - Names: resources[apiOp.Namespace].Names, - }, - }, false } + return []partition.Partition{ + Partition{ + Namespace: apiOp.Namespace, + Names: resources[apiOp.Namespace].Names, + }, + }, false } var result []partition.Partition diff --git a/scripts/validate.sh b/scripts/validate.sh new file mode 100644 index 00000000..257ac7e3 --- /dev/null +++ b/scripts/validate.sh @@ -0,0 +1,12 @@ +#!/bin/bash + +set -e +golangci-lint run +go mod tidy +go mod verify +unclean=$(git status --porcelain --untracked-files=no) +if [ -n "$unclean" ]; then + echo "Encountered dirty repo!"; + echo "$unclean"; + exit 1; +fi