Linter configuration (#2495)

* Update buildbox dockerfile to include lint. Implement grpc/golint targets as host-local builds in top-level Makefile. Move grpc/golint targets to build.assets/Makefile for docker-based builds. * Set the target linter package configuration to everything - this will vary with each linter PR though and will be set back to everything after all linter warnings have been addressed
gravitational · Jun 2, 2021 · 6bd0d2e · 6bd0d2e
1 parent 5d9abe3
commit 6bd0d2e
Show file tree

Hide file tree

Showing 5 changed files with 176 additions and 32 deletions.
diff --git a/.drone.yml b/.drone.yml
@@ -26,6 +26,14 @@ steps:
     volumes:
       - name: dockersock
         path: /var/run
+  - name: lint
+    image: docker:git
+    commands:
+      - apk add --no-cache make bash libc6-compat
+      - make -C build.assets golint
+    volumes:
+      - name: dockersock
+        path: /var/run
   - name: build
     image: docker:git
     environment:
@@ -634,6 +642,6 @@ volumes:
     temp: {}
 ---
 kind: signature
-hmac: 6ee463482d9943a4b9974c97cad601bf274c544f6cd55b5c2b7939c036fa510d
+hmac: a5a4d3e7ce9069e99d8fc7a23b44838aca0f0dd3ca9565ef21a3aef6fa142bea
 
 ...
diff --git a/.golangci.yml b/.golangci.yml
@@ -0,0 +1,65 @@
+issues:
+  exclude-rules:
+    - linters: gosimple
+      text: "S1002: should omit comparison to bool constant"
+    - linters:
+        - gosec
+      text: "G204: Subprocess launched with variable"
+    - linters:
+        - gosec
+      text: "G204: Subprocess launched with function call as argument or cmd arguments"
+  exclude-use-default: true
+  max-same-issues: 0
+  max-issues-per-linter: 0
+
+linters-settings:
+  gosec:
+    excludes:
+      - G204
+  gofmt:
+    simplify: false
+  exhaustive:
+    default-signifies-exhaustive: true
+
+linters:
+  disable-all: true
+  enable:
+    - revive
+    - goimports
+    - bodyclose
+    - deadcode
+    - depguard
+    - dogsled
+    - errcheck
+    - exportloopref
+    - exhaustive
+    - gofmt
+    - goimports
+    - goprintffuncname
+    - gosec
+    - gosimple
+    - govet
+    - ineffassign
+    - misspell
+    - nakedret
+    - noctx
+    - nolintlint
+    - rowserrcheck
+    - staticcheck
+    - structcheck
+    - stylecheck
+    - typecheck
+    - unconvert
+    - unparam
+    - unused
+    - varcheck
+
+output:
+  uniq-by-line: false
+
+run:
+  skip-dirs:
+    - vendor
+    - mage
+  skip-dirs-use-default: false
+  timeout: 5m
diff --git a/Makefile b/Makefile
@@ -9,7 +9,8 @@
 # - make install  : build via `go install`. The output goes into GOPATH/bin/
 # - make clean    : remove the build output and artifacts
 #
-TOP := $(realpath $(dir $(CURDIR)/$(word $(words $(MAKEFILE_LIST)),$(MAKEFILE_LIST))))
+MKFILE_PATH := $(abspath $(lastword $(MAKEFILE_LIST)))
+TOP := $(realpath $(patsubst %/,%,$(dir $(MKFILE_PATH))))
 
 OPS_URL ?=
 
@@ -25,10 +26,16 @@ K8S_VER := 1.21.0
 # 1.13.5 -> 11305, 1.13.12 -> 11312, 2.0.0 -> 20000 and so on
 K8S_VER_SUFFIX := $(shell printf "%d%02d%02d" $(shell echo $(K8S_VER) | sed "s/\./ /g"))
 GOLFLAGS ?= -w -s
+GOLINT ?= golangci-lint
+# TODO(dima): target package configuration (check everything).
+# Package configuration will vary as new PRs are merged.
+GOLINT_PACKAGES ?= ./lib/... \
+	./tool/.. \
+	./e/..
+
+GOPATH ?= $(shell go env GOPATH)
 
 ETCD_VER := v2.3.7
-# Version of the version tool
-VERSION_TAG := 0.0.2
 
 FIO_VER ?= 3.15
 FIO_TAG := fio-$(FIO_VER)
@@ -204,14 +211,15 @@ USER := $(shell echo $${SUDO_USER:-$$USER})
 TEST_ETCD ?= false
 TEST_K8S ?= false
 
-# grpc
-PROTOC_VER ?= 3.10.0
-PROTOC_PLATFORM := linux-x86_64
-GOGO_PROTO_TAG ?= v1.3.0
-GRPC_GATEWAY_TAG ?= v1.11.3
-
 BINARIES ?= tele gravity
 
+GRPC_PROTOS = \
+	$(TOP)/lib/install/proto/installer.proto \
+	$(TOP)/lib/rpc/proto/agent.proto \
+	$(TOP)/lib/rpc/proto/discovery.proto \
+	$(TOP)/lib/network/validation/proto/validation.proto
+GRPC_PROTO_OUTPUTS = $(GRPC_PROTOS:.proto=.pb.go)
+
 export
 
 # the default target is a containerized CI/CD build
@@ -238,14 +246,21 @@ production:
 	GRAVITY="$(GRAVITY_OUT) --state-dir=$(TMP)" $(MAKE) -C build.assets production
 	rm -rf $(TMP)
 
+#
+# generate GRPC files in docker container
+#
+.PHONY: grpc-docker
+grpc-docker:
+	$(MAKE) -C build.assets grpc
+
 #
 # generate GRPC files
 #
 .PHONY: grpc
-grpc:
-	PROTOC_VER=$(PROTOC_VER) PROTOC_PLATFORM=$(PROTOC_PLATFORM) \
-	GOGO_PROTO_TAG=$(GOGO_PROTO_TAG) GRPC_GATEWAY_TAG=$(GRPC_GATEWAY_TAG) VERSION_TAG=$(VERSION_TAG) \
-	$(MAKE) -C build.assets grpc
+grpc: $(GRPC_PROTO_OUTPUTS)
+
+$(GRPC_PROTO_OUTPUTS): $(GRPC_PROTOS)
+	make -C $(@D)
 
 #
 # build tsh binary
@@ -621,7 +636,7 @@ tele-mac: flags
 #
 .PHONY: goinstall
 goinstall: remove-temp-files compile | $(GRAVITY_BUILDDIR)
-	for bin in ${BINARIES} ; do \
+	for bin in $(BINARIES) ; do \
 		cp $(GOPATH)/bin/$${bin} $(GRAVITY_BUILDDIR)/$${bin} ; \
 	done
 	$(GRAVITY) package delete $(GRAVITY_PKG) $(DELETE_OPTS) && \
@@ -632,7 +647,7 @@ $(GRAVITY_BUILDDIR):
 	mkdir -p $@
 
 .PHONY: $(BINARIES)
-$(BINARIES): selinux grpc
+$(BINARIES): selinux grpc-docker
 	GO111MODULE=on go install -mod=vendor -ldflags $(GRAVITY_LINKFLAGS) -tags "$(GRAVITY_BUILDTAGS)" $(GRAVITY_PKG_PATH)/tool/$@
 
 .PHONY: dev
@@ -730,4 +745,30 @@ clean-codegen:
 selinux:
 	$(MAKE) -C build.assets	selinux
 
+.PHONY: golint
+golint: golangci-verify
+	#TODO(dima): enable when the linter warning PRs have been merged
+	#$(GOLINT) run -c .golangci.yml \
+	#	$(GOLINT_PACKAGES)
+
+GOLANGCI_REQUIRED_MAJOR := 1
+GOLANGCI_REQUIRED_MINOR := 39
+GOLANGCI_INSTALLED := $(shell command -v $(GOLINT) 2> /dev/null)
+
+.PHONY: golangci-verify
+golangci-verify: GOLANGCI_INSTALLED_VER=$(eval value := $$(shell $(GOLINT) version --format=short 2>&1))$(value)
+golangci-verify: GOLANGCI_INSTALLED_VER_MAJOR=$(eval value := $$(shell $(GOLINT) version --format=short 2>&1 | cut -f1 -d.))$(value)
+golangci-verify: GOLANGCI_INSTALLED_VER_MINOR=$(eval value := $$(shell $(GOLINT) version --format=short 2>&1 | cut -f2 -d.))$(value)
+golangci-verify:
+ifndef GOLANGCI_INSTALLED
+	@echo This step requires $(GOLINT) to be installed
+	exit 1
+endif
+	@if [ "$(GOLANGCI_INSTALLED_VER_MAJOR:v%=%)" -eq "$(GOLANGCI_REQUIRED_MAJOR)" ] && \
+		[ "$(GOLANGCI_INSTALLED_VER_MINOR)" -lt "$(GOLANGCI_REQUIRED_MINOR)" ]; then \
+		echo "Installed $(GOLINT) version: $(GOLANGCI_INSTALLED_VER)"; \
+		echo "This step requires $(GOLINT) version 1.39.0 or newer"; \
+		exit 1; \
+	fi;
+
 include build.assets/etcd.mk
diff --git a/build.assets/Dockerfile b/build.assets/Dockerfile
@@ -5,6 +5,7 @@ ARG PROTOC_PLATFORM
 ARG GOGO_PROTO_TAG
 ARG GRPC_GATEWAY_TAG
 ARG VERSION_TAG
+ARG GOLANGCI_LINT_VER
 ARG UID
 ARG GID
 
@@ -19,7 +20,7 @@ RUN apt-get -q -y update --fix-missing && apt-get -q -y install libaio-dev zlib1
 RUN getent group  $GID || groupadd builder --gid=$GID -o; \
     getent passwd $UID || useradd builder --uid=$UID --gid=$GID --create-home --shell=/bin/sh;
 
-RUN (mkdir -p /opt/protoc && \
+RUN set -ex && (mkdir -p /opt/protoc && \
      mkdir -p /.cache && \
      chown -R $UID:$GID /gopath && \
      chown -R $UID:$GID /opt/protoc && \
@@ -35,19 +36,24 @@ ENV LANGUAGE="en_US.UTF-8" \
      GOPATH="/gopath" \
      PATH="$PATH:/opt/protoc/bin:/opt/go/bin:/gopath/bin"
 
-RUN (mkdir -p /gopath/src/github.com/gravitational && \
+RUN set -ex && (mkdir -p /gopath/src/github.com/gravitational && \
      cd /gopath/src/github.com/gravitational && \
      git clone https://github.com/gravitational/version.git && \
      cd /gopath/src/github.com/gravitational/version && \
      git checkout ${VERSION_TAG} && \
      go install github.com/gravitational/version/cmd/linkflags)
 
-RUN (wget --quiet -O /tmp/${TARBALL} ${PROTOC_URL} && \
+RUN set -ex && (wget --quiet -O /tmp/${TARBALL} ${PROTOC_URL} && \
      unzip -d /opt/protoc /tmp/${TARBALL} && \
      mkdir -p /gopath/src/github.com/gogo/ /gopath/src/github.com/grpc-ecosystem && \
      git clone https://github.com/gogo/protobuf --branch ${GOGO_PROTO_TAG} /gopath/src/github.com/gogo/protobuf && cd /gopath/src/github.com/gogo/protobuf && make install && \
      git clone https://github.com/grpc-ecosystem/grpc-gateway --branch ${GRPC_GATEWAY_TAG} /gopath/src/github.com/grpc-ecosystem/grpc-gateway && cd /gopath/src/github.com/grpc-ecosystem/grpc-gateway && pwd && go install ./protoc-gen-grpc-gateway)
 
+ENV GOLANGCI_RELEASE_TARBALL https://github.com/golangci/golangci-lint/releases/download/v${GOLANGCI_LINT_VER}/golangci-lint-${GOLANGCI_LINT_VER}-linux-amd64.tar.gz
+
+RUN set -ex && \
+	curl -sSfL ${GOLANGCI_RELEASE_TARBALL} | tar xz --strip-components=1 -C $(go env GOPATH)/bin golangci-lint-${GOLANGCI_LINT_VER}-linux-amd64/golangci-lint
+
 ENV PROTO_INCLUDE "/usr/local/include":"/gopath/src":"${GRPC_GATEWAY_ROOT}/third_party/googleapis":"${GOGOPROTO_ROOT}/gogoproto"
 
 

diff --git a/build.assets/Makefile b/build.assets/Makefile
@@ -7,7 +7,9 @@
 #
 
 OPS_URL ?=
-LOCALDIR := $(dir $(CURDIR)/$(word $(words $(MAKEFILE_LIST)),$(MAKEFILE_LIST)))
+MKFILE_PATH := $(abspath $(lastword $(MAKEFILE_LIST)))
+LOCALDIR := $(realpath $(patsubst %/,%,$(dir $(MKFILE_PATH))))
+TOP := $(abspath $(LOCALDIR)/..)
 LOCAL_BUILDDIR ?= /gopath/src/github.com/gravitational/gravity/build
 # Path to the local gravity build assets directory used inside the container
 LOCAL_GRAVITY_BUILDDIR ?= /gopath/src/github.com/gravitational/gravity/build/$(GRAVITY_VERSION)
@@ -100,12 +102,15 @@ BASH ?= /bin/bash
 # packages to test
 TEST_PACKAGES ?= $(GRAVITY_PKG_PATH)/lib/... $(GRAVITY_PKG_PATH)/tool/...
 
-GRPC_PROTOS = \
-	$(TOP)/lib/install/proto/installer.proto \
-	$(TOP)/lib/rpc/proto/agent.proto \
-	$(TOP)/lib/rpc/proto/discovery.proto \
-	$(TOP)/lib/network/validation/proto/validation.proto
-GRPC_PROTO_OUTPUTS = $(GRPC_PROTOS:.proto=.pb.go)
+# Version of the version tool
+VERSION_TAG := 0.0.2
+GOLANGCI_LINT_VER := 1.40.1
+
+# grpc
+PROTOC_VER ?= 3.10.0
+PROTOC_PLATFORM := linux-x86_64
+GOGO_PROTO_TAG ?= v1.3.0
+GRPC_GATEWAY_TAG ?= v1.11.3
 
 # Export variables for recursive make invocations
 export GRAVITY_PKG_PATH
@@ -194,13 +199,11 @@ endif
 # generate gRPC code
 #
 .PHONY: grpc
-grpc: buildbox $(GRPC_PROTO_OUTPUTS)
-
-$(GRPC_PROTO_OUTPUTS): $(GRPC_PROTOS)
+grpc:
 	docker run --rm=true $(NOROOT) \
-		   -v $(TOP):$(SRCDIR) \
-		   $(BBOX) \
-		   dumb-init make -C $(subst $(TOP),$(SRCDIR),$(@D))
+	   -v $(TOP):$(SRCDIR) \
+	   $(BBOX) \
+	   dumb-init make -C $(SRCDIR) grpc
 
 #
 # this is a temporary target until we upgrade docker packages
@@ -237,6 +240,26 @@ build-in-container: buildbox
 		   $(BBOX) \
 		   dumb-init make -C $(SRCDIR)/build.assets -j $(TARGETS)
 
+#
+# golint gravity binaries in a buildbox container
+#
+.PHONY: golint
+golint: buildbox
+	docker run --rm=true $(NOROOT) \
+		   -v $(TOP):$(SRCDIR) \
+		   -w /gopath/src/github.com/gravitational/gravity \
+		   -e "LOCAL_BUILDDIR=$(LOCAL_BUILDDIR)" \
+		   -e "LOCAL_GRAVITY_BUILDDIR=$(LOCAL_GRAVITY_BUILDDIR)" \
+		   -e "GRAVITY_PKG_PATH=$(GRAVITY_PKG_PATH)" \
+		   -e "GRAVITY_VERSION=$(GRAVITY_VERSION)" \
+		   -e "GRAVITY_TAG=$(GRAVITY_TAG)" \
+		   -e 'GRAVITY_LINKFLAGS=$(GRAVITY_LINKFLAGS)' \
+		   -e "GRAVITY_BUILDFLAGS=$(GRAVITY_BUILDFLAGS)" \
+		   -e "GRAVITY_BUILDTAGS=$(GRAVITY_BUILDTAGS)" \
+		   $(GOCACHE_DOCKER_OPTIONS) \
+		   $(BBOX) \
+		   dumb-init make -C $(SRCDIR) golint
+
 #
 # build gravity binaries on host
 #
@@ -698,6 +721,7 @@ buildbox:
 		--build-arg GOGO_PROTO_TAG=$(GOGO_PROTO_TAG) \
 		--build-arg GRPC_GATEWAY_TAG=$(GRPC_GATEWAY_TAG) \
 		--build-arg VERSION_TAG=$(VERSION_TAG) \
+		--build-arg GOLANGCI_LINT_VER=$(GOLANGCI_LINT_VER) \
 		--build-arg UID=$(shell id -u) \
 		--build-arg GID=$(shell id -g) \
 		$(DOCKER_ARGS) --tag $(BBOX) .