Moves lint and format to make, managed by bingo (#132)

This removes a few shell scripts and in the process updates linters to
more current versions. For example, we were using golangci from 2019. To
adjust, some invalid statements were removed.

The main part of this change is using https://github.com/bwplotka/bingo
as the linters we used were go programs anyway. Bingo generates make
entries you can use to on-demang install tools without affecting your
go.mod and without shell scripts.

Co-authored-by: Takeshi Yoneda <takeshi@tetrate.io>
Signed-off-by: Adrian Cole <adrian@tetrate.io>

.bingo/.gitignore

@@ -0,0 +1,12 @@
+
+# Ignore everything
+*
+
+# But not these files:
+!.gitignore
+!*.mod
+!README.md
+!Variables.mk
+!variables.env
+
+*tmp.mod

.bingo/README.md

@@ -0,0 +1,14 @@
+# Project Development Dependencies.
+
+This is directory which stores Go modules with pinned buildable package that is used within this repository, managed by https://github.com/bwplotka/bingo.
+
+* Run `bingo get` to install all tools having each own module file in this directory.
+* Run `bingo get <tool>` to install <tool> that have own module file in this directory.
+* For Makefile: Make sure to put `include .bingo/Variables.mk` in your Makefile, then use $(<upper case tool name>) variable where <tool> is the .bingo/<tool>.mod.
+* For shell: Run `source .bingo/variables.env` to source all environment variable for each tool.
+* For go: Import `.bingo/variables.go` to for variable names.
+* See https://github.com/bwplotka/bingo or -h on how to add, remove or change binaries dependencies.
+
+## Requirements
+
+* Go 1.14+

.bingo/Variables.mk

@@ -0,0 +1,43 @@
+# Auto generated binary variables helper managed by https://github.com/bwplotka/bingo v0.4.0. DO NOT EDIT.
+# All tools are designed to be build inside $GOBIN.
+BINGO_DIR := $(dir $(lastword $(MAKEFILE_LIST)))
+GOPATH ?= $(shell go env GOPATH)
+GOBIN  ?= $(firstword $(subst :, ,${GOPATH}))/bin
+GO     ?= $(shell which go)
+
+# Below generated variables ensure that every time a tool under each variable is invoked, the correct version
+# will be used; reinstalling only if needed.
+# For example for goimports variable:
+#
+# In your main Makefile (for non array binaries):
+#
+#include .bingo/Variables.mk # Assuming -dir was set to .bingo .
+#
+#command: $(GOIMPORTS)
+#	@echo "Running goimports"
+#	@$(GOIMPORTS) <flags/args..>
+#
+GOIMPORTS := $(GOBIN)/goimports-v0.1.0
+$(GOIMPORTS): $(BINGO_DIR)/goimports.mod
+	@# Install binary/ries using Go 1.14+ build command. This is using bwplotka/bingo-controlled, separate go module with pinned dependencies.
+	@echo "(re)installing $(GOBIN)/goimports-v0.1.0"
+	@cd $(BINGO_DIR) && $(GO) build -mod=mod -modfile=goimports.mod -o=$(GOBIN)/goimports-v0.1.0 "golang.org/x/tools/cmd/goimports"
+
+GOLANGCI_LINT := $(GOBIN)/golangci-lint-v1.38.0
+$(GOLANGCI_LINT): $(BINGO_DIR)/golangci-lint.mod
+	@# Install binary/ries using Go 1.14+ build command. This is using bwplotka/bingo-controlled, separate go module with pinned dependencies.
+	@echo "(re)installing $(GOBIN)/golangci-lint-v1.38.0"
+	@cd $(BINGO_DIR) && $(GO) build -mod=mod -modfile=golangci-lint.mod -o=$(GOBIN)/golangci-lint-v1.38.0 "github.com/golangci/golangci-lint/cmd/golangci-lint"
+
+LICENSER := $(GOBIN)/licenser-v0.6.0
+$(LICENSER): $(BINGO_DIR)/licenser.mod
+	@# Install binary/ries using Go 1.14+ build command. This is using bwplotka/bingo-controlled, separate go module with pinned dependencies.
+	@echo "(re)installing $(GOBIN)/licenser-v0.6.0"
+	@cd $(BINGO_DIR) && $(GO) build -mod=mod -modfile=licenser.mod -o=$(GOBIN)/licenser-v0.6.0 "github.com/liamawhite/licenser"
+
+SHFMT := $(GOBIN)/shfmt-v3.2.4
+$(SHFMT): $(BINGO_DIR)/shfmt.mod
+	@# Install binary/ries using Go 1.14+ build command. This is using bwplotka/bingo-controlled, separate go module with pinned dependencies.
+	@echo "(re)installing $(GOBIN)/shfmt-v3.2.4"
+	@cd $(BINGO_DIR) && $(GO) build -mod=mod -modfile=shfmt.mod -o=$(GOBIN)/shfmt-v3.2.4 "mvdan.cc/sh/v3/cmd/shfmt"
+

.bingo/go.mod

@@ -0,0 +1 @@
+module _ // Fake go.mod auto-created by 'bingo' for go -moddir compatibility with non-Go projects. Commit this file, together with other .mod files.

.bingo/goimports.mod

@@ -0,0 +1,5 @@
+module _ // Auto generated by https://github.com/bwplotka/bingo. DO NOT EDIT
+
+go 1.16
+
+require golang.org/x/tools v0.1.0 // cmd/goimports

.bingo/golangci-lint.mod

@@ -0,0 +1,5 @@
+module _ // Auto generated by https://github.com/bwplotka/bingo. DO NOT EDIT
+
+go 1.16
+
+require github.com/golangci/golangci-lint v1.38.0 // cmd/golangci-lint

.bingo/licenser.mod

@@ -0,0 +1,5 @@
+module _ // Auto generated by https://github.com/bwplotka/bingo. DO NOT EDIT
+
+go 1.16
+
+require github.com/liamawhite/licenser v0.6.0

.bingo/shfmt.mod

@@ -0,0 +1,5 @@
+module _ // Auto generated by https://github.com/bwplotka/bingo. DO NOT EDIT
+
+go 1.16
+
+require mvdan.cc/sh/v3 v3.2.4 // cmd/shfmt

.bingo/variables.env

@@ -0,0 +1,18 @@
+# Auto generated binary variables helper managed by https://github.com/bwplotka/bingo v0.4.0. DO NOT EDIT.
+# All tools are designed to be build inside $GOBIN.
+# Those variables will work only until 'bingo get' was invoked, or if tools were installed via Makefile's Variables.mk.
+GOBIN=${GOBIN:=$(go env GOBIN)}
+
+if [ -z "$GOBIN" ]; then
+	GOBIN="$(go env GOPATH)/bin"
+fi
+
+
+GOIMPORTS="${GOBIN}/goimports-v0.1.0"
+
+GOLANGCI_LINT="${GOBIN}/golangci-lint-v1.38.0"
+
+LICENSER="${GOBIN}/licenser-v0.6.0"
+
+SHFMT="${GOBIN}/shfmt-v3.2.4"
+

.circleci/config.yml

@@ -21,7 +21,7 @@ executors:
       - image: registry:2
 
 jobs:
-  lint:
+  check:
     executor: builder
     resource_class: medium+
     environment:
@@ -32,18 +32,18 @@ jobs:
       GOMAXPROCS: "3"
     steps:
       - checkout
-      - run: make init
-      - run: ./ci/install-lint
-      - run: ./ci/lint
-
+      - run: make check
+
   test:
     executor: builder
     resource_class: medium+
     steps:
       - checkout
       - run: make init
-      - run: ./ci/install-envoy
-      - run: ./ci/test
+      # prefetch implicit version needed by pkg/binary/envoy/controlplane/istio_test.go until #136. This avoids:
+      # Unable to start Envoy process: fork/exec /home/circleci/.getenvoy/builds/standard/1.11.0/linux_glibc/bin/envoy: text file busy
+      - run: go run cmd/getenvoy/main.go fetch standard:1.11.0
+      - run: make test.ci GO_TEST_EXTRA_OPTS="-timeout 60s"
       - run:
           name: "Measure test coverage (for now, on a subset of tests)"
           command: make coverage GO_COVERAGE_EXTRA_OPTS="-p 1"
@@ -55,5 +55,5 @@ workflows:
   version: 2
   commit:
     jobs:
-      - lint
+      - check
       - test

.licenserignore

@@ -6,3 +6,7 @@ go.mod
 go.sum
 
 *.pem
+
+.bingo
+.dockerignore
+.idea

CONTRIBUTING.md

@@ -4,8 +4,8 @@ We welcome contributions from the community. Please read the following guideline
 
 ## Coding Style
 
-- The code is linted using a relatively stringent [golang-ci config](./.golangci.yml). To run this linter (and a few others) use the `ci/format` script. To verify, you can run the `ci/lint` script.
-- We follow standard Go table-driven tests and use the [`testify/assert`](https://github.com/stretchr/testify#assert-package) library to assert correctness. To verify all tests pass, you can run the `ci/test` script.
+- The code is linted using a relatively stringent [golang-ci config](./.golangci.yml). To run this linter (and a few others) use run `make check`. To format your files, you can run `make format`.
+- We follow standard Go table-driven tests and use the [`testify/assert`](https://github.com/stretchr/testify#assert-package) library to assert correctness. To verify all tests pass, you can run `make test`.
 
 ## DCO
 

Makefile

@@ -12,6 +12,12 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+# Make sure we pick up any local overrides.
+-include .makerc
+
+# bingo manages go binaries needed for building the project
+include .bingo/Variables.mk
+
 ENVOY = standard:1.11.1
 HUB ?= docker.io/getenvoy
 GETENVOY_TAG ?= dev
@@ -175,3 +181,55 @@ $(foreach lang,$(BUILDERS_LANGS),$(eval $(call GEN_PULL_EXTENSION_BUILDER_IMAGE_
 
 .PHONY: builders.pull
 builders.pull: $(foreach lang,$(BUILDERS_LANGS), pull/builder/$(lang))
+
+##@ Code quality and integrity
+
+LINT_OPTS ?= --timeout 5m
+.PHONY: lint
+# generate must be called while generated source is still used
+lint: generate $(GOLANGCI_LINT) $(SHFMT) $(LICENSER) .golangci.yml  ## Run the linters
+	@echo "--- lint ---"
+	@$(SHFMT) -d .
+	@$(LICENSER) verify -r .
+	@$(GOLANGCI_LINT) run $(LINT_OPTS) --config .golangci.yml
+
+# The goimports tool does not arrange imports in 3 blocks if there are already more than three blocks.
+# To avoid that, before running it, we collapse all imports in one block, then run the formatter.
+.PHONY: format
+format: $(GOIMPORTS) ## Format all Go code
+	@echo "--- format ---"
+	@$(LICENSER) apply -r "Tetrate"
+	@find . -type f -name '*.go' | xargs gofmt -s -w
+	@for f in `find . -name '*.go'`; do \
+	    awk '/^import \($$/,/^\)$$/{if($$0=="")next}{print}' $$f > /tmp/fmt; \
+	    mv /tmp/fmt $$f; \
+	    $(GOIMPORTS) -w -local github.com/tetratelabs/getenvoy $$f; \
+	done
+
+
+# Enforce go version matches what's in go.mod when running `make check` assuming the following:
+# * 'go version' returns output like "go version go1.16 darwin/amd64"
+# * go.mod contains a line like "go 1.16"
+EXPECTED_GO_VERSION_PREFIX := "go version go$(shell sed -ne '/^go /s/.* //gp' go.mod )"
+GO_VERSION := $(shell go version)
+
+.PHONY: check
+check:  ## CI blocks merge until this passes. If this fails, run "make check" locally and commit the difference.
+# case statement because /bin/sh cannot do prefix comparison, awk is awkward and assuming /bin/bash is brittle
+	@case "$(GO_VERSION)" in $(EXPECTED_GO_VERSION_PREFIX)* ) ;; * ) \
+		echo "Expected 'go version' to start with $(EXPECTED_GO_VERSION_PREFIX), but it didn't: $(GO_VERSION)"; \
+		exit 1; \
+	esac
+	@$(MAKE) lint
+	@$(MAKE) format
+	@go mod tidy
+	@if [ ! -z "`git status -s`" ]; then \
+		echo "The following differences will fail CI until committed:"; \
+		git diff; \
+		exit 1; \
+	fi
+
+.PHONY: clean
+clean:   ## Clean all binaries
+	@echo "--- $@ ---"
+	go clean -testcache

data/extension/init/templates/tinygo/envoy.access_loggers/default/main_test.go

@@ -4,7 +4,6 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
-
 	"github.com/tetratelabs/proxy-wasm-go-sdk/proxytest"
 	"github.com/tetratelabs/proxy-wasm-go-sdk/proxywasm/types"
 )

data/extension/init/templates/tinygo/envoy.filters.http/default/main_test.go

@@ -4,7 +4,6 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
-
 	"github.com/tetratelabs/proxy-wasm-go-sdk/proxytest"
 	"github.com/tetratelabs/proxy-wasm-go-sdk/proxywasm/types"
 )

pkg/binary/envoy/controlplane/istio.go

@@ -24,15 +24,14 @@ import (
 
 	"github.com/golang/protobuf/ptypes"
 	durationpb "github.com/golang/protobuf/ptypes/duration"
-
-	"github.com/tetratelabs/getenvoy/pkg/binary"
-	"github.com/tetratelabs/getenvoy/pkg/binary/envoy"
 	"github.com/tetratelabs/log"
 	meshconfig "istio.io/api/mesh/v1alpha1"
-
 	"istio.io/istio/pilot/pkg/model"
 	"istio.io/istio/pkg/bootstrap"
 	"istio.io/istio/pkg/config/mesh"
+
+	"github.com/tetratelabs/getenvoy/pkg/binary"
+	"github.com/tetratelabs/getenvoy/pkg/binary/envoy"
 )
 
 const (

pkg/binary/envoy/controlplane/istio_test.go

@@ -24,11 +24,12 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/assert"
+	"istio.io/istio/pilot/pkg/bootstrap"
+	"istio.io/istio/tests/util"
+
 	"github.com/tetratelabs/getenvoy/pkg/binary/envoy"
 	"github.com/tetratelabs/getenvoy/pkg/binary/envoy/debug"
 	"github.com/tetratelabs/getenvoy/pkg/binary/envoytest"
-	"istio.io/istio/pilot/pkg/bootstrap"
-	"istio.io/istio/tests/util"
 )
 
 func TestMain(m *testing.M) {

pkg/binary/envoy/debug/admin.go

@@ -23,7 +23,6 @@ import (
 	"path/filepath"
 
 	"github.com/tetratelabs/log"
-
 	"github.com/tetratelabs/multierror"
 
 	"github.com/tetratelabs/getenvoy/pkg/binary"

pkg/binary/envoy/debug/log.go

@@ -17,8 +17,9 @@
 package debug
 
 import (
-	"github.com/tetratelabs/getenvoy/pkg/binary/envoy"
 	"github.com/tetratelabs/log"
+
+	"github.com/tetratelabs/getenvoy/pkg/binary/envoy"
 )
 
 // EnableEnvoyLogCollection is a preset option that registers collection of Envoy Access Logs

pkg/binary/envoy/debug/log_linux.go

@@ -20,9 +20,10 @@ import (
 	"os"
 	"path/filepath"
 
+	"github.com/tetratelabs/log"
+
 	"github.com/tetratelabs/getenvoy/pkg/binary"
 	"github.com/tetratelabs/getenvoy/pkg/binary/envoy"
-	"github.com/tetratelabs/log"
 )
 
 // EnableEnvoyLogCollection is a preset option that registers collection of Envoy access logs and stderr