[Automation] Add CI linters/checks

.github/workflows/pr.yml → .github/workflows/ggshield.yml

@@ -1,6 +1,8 @@
 name: GitGuardian scan
 
-on: [push, pull_request]
+on:
+  workflow_dispatch:
+  push:
 
 jobs:
   scanning:
@@ -16,6 +18,6 @@ jobs:
         env:
           GITHUB_PUSH_BEFORE_SHA: ${{ github.event.before }}
           GITHUB_PUSH_BASE_SHA: ${{ github.event.base }}
-          GITHUB_PULL_BASE_SHA:  ${{ github.event.pull_request.base.sha }}
+          GITHUB_PULL_BASE_SHA: ${{ github.event.pull_request.base.sha }}
           GITHUB_DEFAULT_BRANCH: ${{ github.event.repository.default_branch }}
           GITGUARDIAN_API_KEY: ${{ secrets.GITGUARDIAN_API_KEY }}

.github/workflows/main.yml

@@ -16,15 +16,15 @@ on:
 
 env:
   # Even though we can test against multiple versions, this one is considered a target version.
-  TARGET_GOLANG_VERSION: "1.18"
+  TARGET_GOLANG_VERSION: "1.19"
   PROTOC_VERSION: "3.19.4"
 
 jobs:
   test-multiple-go-versions:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        go: ["1.18"] # As we are relying on generics, we can't go lower than 1.18.
+        go: ["1.19"] # As we are relying on generics, we can't go lower than 1.18.
       fail-fast: false
     name: Go ${{ matrix.go }} test
     steps:
@@ -58,13 +58,15 @@ jobs:
         uses: guyarb/[email protected]
         with:
           test-results: test_results.json
-
-      # TODO(@okdas): move coverage to a separate job
+      - name: Run golangci-lint
+        # Only run if the test failed on target version to avoid duplicated annotations on GitHub.
+        if: ${{ always() && env.TARGET_GOLANG_VERSION == matrix.go }}
+        uses: golangci/golangci-lint-action@v3
       - name: create coverage report
-        if: ${{ env.TARGET_GOLANG_VERSION == matrix.go }}
+        if: ${{ always() && env.TARGET_GOLANG_VERSION == matrix.go }}
         run: make test_all_with_coverage
       - name: Upload coverage to Codecov
-        if: ${{ env.TARGET_GOLANG_VERSION == matrix.go }}
+        if: ${{ always() && env.TARGET_GOLANG_VERSION == matrix.go }}
         uses: codecov/codecov-action@v3
 
   # TODO(@okdas): reuse artifacts built by the previous job instead
@@ -87,8 +89,11 @@ jobs:
         id: meta
         uses: docker/metadata-action@v4
         with:
+          # poktnetwork/pocket-v1
+          # ^ the name of the repo on docker hub. For some reason, sometimes docker hub does not authenticate the user
+          # resulting in an error when trying to push to the repo.
+          # TODO(@okdas): figure out why that happens, fix or stick to ghcr.io/AWS ECR.
           images: |
-            poktnetwork/pocket-v1
             ghcr.io/pokt-network/pocket-v1
           tags: |
             type=schedule${{ matrix.imageType == 'dev' && ',suffix=-dev' || '' }}
@@ -99,12 +104,12 @@ jobs:
             type=ref,event=pr${{ matrix.imageType == 'dev' && ',suffix=-dev' || '' }}
             type=sha${{ matrix.imageType == 'dev' && ',suffix=-dev' || '' }}
             type=raw,value=latest,enable={{is_default_branch}}${{ matrix.imageType == 'dev' && ',suffix=-dev' || '' }}
-      # Turned off until we set up docker hub
-      - name: Login to  DockerHub
-        uses: docker/login-action@v2
-        with:
-          username: ${{ secrets.DOCKERHUB_USERNAME }}
-          password: ${{ secrets.DOCKERHUB_TOKEN }}
+      # Turned off until user auth is fixed, see note above.
+      # - name: Login to  DockerHub
+      #   uses: docker/login-action@v2
+      #   with:
+      #     username: ${{ secrets.DOCKERHUB_USERNAME }}
+      #     password: ${{ secrets.DOCKERHUB_TOKEN }}
       - name: Login to GitHub Container Registry
         uses: docker/login-action@v2
         with:

.golangci.yml

@@ -0,0 +1,69 @@
+linters:
+  enable:
+    # TODO: investigate what linters we want to enable (many disabled by default!)
+    # https://golangci-lint.run/usage/linters/#disabled-by-default
+
+    # Default linters
+    - errcheck
+    - gosimple
+    - govet
+    - ineffassign
+    - staticcheck
+    - typecheck
+    - unused
+
+    # Additional linters
+    - gosec
+    - misspell
+    - promlinter
+    - unparam
+
+    # Gocritic allows to use ruleguard - custom linting rules
+    - gocritic
+linters-settings:
+  # Gocritic settings
+  # https://go-critic.com/overview
+  gocritic:
+    enabled-checks:
+      - ruleguard
+    enabled-tags:
+      - diagnostic
+      - experimental
+      - opinionated # This might be too much, but we can try it out
+      - performance
+      - style
+    disabled-checks:
+      - dupImport # https://github.com/go-critic/go-critic/issues/845
+      - ifElseChain
+      # - octalLiteral
+      # - whyNoLint
+    settings:
+      ruleguard:
+        # Enable debug to identify which 'Where' condition was rejected.
+        # The value of the parameter is the name of a function in a ruleguard file.
+        #
+        # When a rule is evaluated:
+        # If:
+        #   The Match() clause is accepted; and
+        #   One of the conditions in the Where() clause is rejected,
+        # Then:
+        #   ruleguard prints the specific Where() condition that was rejected.
+        #
+        # The flag is passed to the ruleguard 'debug-group' argument.
+        # Default: ""
+        # debug: "testEq"
+        # Determines the behavior when an error occurs while parsing ruleguard files.
+        # If flag is not set, log error and skip rule files that contain an error.
+        # If flag is set, the value must be a comma-separated list of error conditions.
+        # - 'all':    fail on all errors.
+        # - 'import': ruleguard rule imports a package that cannot be found.
+        # - 'dsl':    gorule file does not comply with the ruleguard DSL.
+        # Default: ""
+        failOn: dsl
+        rules: "misc/linting-rules/*.go"
+run:
+  go: "1.19"
+  skip-dirs:
+    - misc/linting-rules
+  build-tags:
+    - codeanalysis

Makefile

@@ -317,6 +317,11 @@ test_p2p:
 test_p2p_addrbook:
 	go test -run AddrBook -v -count=1 ./p2p/...
 
+.PHONY: lint
+## Run all linters that are triggered by the CI pipeline
+lint:
+	golangci-lint run ./...
+
 .PHONY: benchmark_sortition
 ## Benchmark the Sortition library
 benchmark_sortition:

build/Dockerfile.debian.dev

@@ -1,7 +1,7 @@
 # Purpose of this container image is to ship pocket binary with additional
 # tools such as dlv, curl, etc.
 
-ARG TARGET_GOLANG_VERSION=1.18
+ARG TARGET_GOLANG_VERSION=1.19
 
 FROM golang:${TARGET_GOLANG_VERSION}-bullseye AS builder
 
@@ -33,7 +33,7 @@ RUN set -eux; \
     unzip -q protoc.zip -d /usr/local/; \
     protoc --version
 
-# dlv 
+# dlv
 RUN go install github.com/go-delve/delve/cmd/dlv@latest; \
     dlv version
 
@@ -52,4 +52,4 @@ RUN go get -d -v ./app/pocket
 RUN go build -o /usr/local/bin/pocket ./app/pocket
 RUN go build -o /usr/local/bin/client ./app/client
 
-CMD ["/usr/local/bin/pocket"]
+CMD ["/usr/local/bin/pocket"]

build/Dockerfile.debian.prod

@@ -1,6 +1,6 @@
 # Purpose of this container image is to ship pocket binary with minimal dependencies.
 
-ARG TARGET_GOLANG_VERSION=1.18
+ARG TARGET_GOLANG_VERSION=1.19
 
 FROM golang:${TARGET_GOLANG_VERSION}-bullseye AS builder
 
@@ -53,4 +53,4 @@ RUN apt-get update -qq && \
 
 COPY --from=builder /go/bin/pocket /usr/local/bin/pocket
 
-CMD ["/usr/local/bin/pocket"]
+CMD ["/usr/local/bin/pocket"]

docs/development/README.md

@@ -10,6 +10,12 @@ Please note that this repository is under very active development and breaking c
     - [Running Unit Tests](#running-unit-tests)
     - [Running LocalNet](#running-localnet)
   - [Code Organization](#code-organization)
+    - [Linters](#linters)
+      - [Installation of golangci-lint](#installation-of-golangci-lint)
+      - [Running linters locally](#running-linters-locally)
+      - [VSCode Integration](#vscode-integration)
+      - [Configuration](#configuration)
+      - [Custom linters](#custom-linters)
 
 ## LFG - Development
 
@@ -156,3 +162,44 @@ Pocket
 ├── utility          # Implementation of the Utility module
 ├── Makefile         # [to-be-deleted] The source of targets used to develop, build and test
 ```
+
+### Linters
+
+We are utilizing `golangci-lint` to run the linters. It is a wrapper around a number of linters and is configured to run many of them at once. The linters are configured to run on every commit and pull request via CI, and all code issues are populating as GitHub annotations to let developers and reviewers easily locate an issue.
+
+#### Installation of golangci-lint
+
+Please follow the instructions on the [golangci-lint](https://golangci-lint.run/usage/install/#local-installation) website.
+
+#### Running linters locally
+
+You can run `golangci-lint` locally against all packages with:
+
+```bash
+make lint
+```
+
+If you need to specify any additional flags, you can run `golangci-lint` directly as it picks up the configuration from the `.golangci.yml` file.
+
+#### VSCode Integration
+
+`golangci-lint` has an integration with VSCode. Per [documentation](https://golangci-lint.run/usage/integrations/), recommended settings are:
+
+```json
+"go.lintTool":"golangci-lint",
+"go.lintFlags": [
+  "--fast"
+]
+```
+
+#### Configuration
+
+`golangci-lint` is a sophisticated tool as it includes a lot of linters, including our own custom linting rules. As a result, a configuration file might become quite large. The configuration file is located at [`.golangci.yml`](../../.golangci.yml).
+
+The official documentation includes a list of different linters and their configuration options. Please refer to [this page](https://golangci-lint.run/usage/linters/) for more information.
+
+#### Custom linters
+
+We can write custom linters using [`go-ruleguard`](https://go-ruleguard.github.io/). The rules are located in the [`misc/linting-rules`](../../misc/linting-rules) directory. The rules are written in the [Ruleguard DSL](https://github.com/quasilyte/go-ruleguard/blob/master/_docs/dsl.md), if you've never worked with ruleguard in the past, it makes sense to go through [introduction article](https://quasilyte.dev/blog/post/ruleguard/) and [Ruleguard by example tour](https://go-ruleguard.github.io/by-example/).
+
+Ruleguard is run via `gocritic` linter which is a part of `golangci-lint`, so if you wish to change configuration or debug a particular rule, you can modify the `.golangci.yml` file.

docs/releases/README.md

@@ -0,0 +1,47 @@
+# Building & Versioning
+
+- [Building & Versioning](#building--versioning)
+  - [Release Tag Versioning](#release-tag-versioning)
+  - [[WIP] Build Versioning](#wip-build-versioning)
+  - [Container Images](#container-images)
+    - [Tags](#tags)
+    - [Extended images with additional tooling](#extended-images-with-additional-tooling)
+  - [[TODO] Magefile build system](#todo-magefile-build-system)
+
+## Release Tag Versioning
+
+We follow Go's [Module Version Numbering](https://go.dev/doc/modules/version-numbers) for software releases along with typical [Software release life cycles](https://en.wikipedia.org/wiki/Software_release_life_cycle).
+
+For example, `v0.0.1-alpha.pre.1` is the tag used for the first milestone merge and `v0.0.1-alpha.1` can be used for the first official alpha release.
+
+## [WIP] Build Versioning
+
+Automatic development / test / production builds are still a work in progress, but we plan to incorporate the following when we reach that point:
+
+- `+dirty` for uncommited changes
+- `-version` flag that can be injected or defaults to `UNKNOWN`
+- `branch_name` and a shortened `commit_hash` should be included
+
+For example, `X.Y.Z[-<pre_release_tag][+branch_name][+short_hash][+dirty]` is the name of a potential build we will release in the future.
+
+## Container Images
+
+Our images are hosted on Github's Container Registry (GHCR) and are available at `ghcr.io/poktnetwork/pocket-v1`. You can find the list of latest images [here](https://github.com/pokt-network/pocket/pkgs/container/pocket-v1).
+
+### Tags
+
+Code built from the default branch (i.e. `main`) is tagged as `latest`.
+
+Code built from commits in Pull Requests, is tagged as `pr-<number>`, as well as `sha-<7 digit sha>`.
+
+Once releases are managed, they will be tagged with the version number (e.g. `v0.0.1-alpha.pre.1`).
+
+
+### Extended images with additional tooling
+
+Extended images with additional tooling are built to aid in troubleshoot and debugging. The extended image is formatted as `<tag>-dev`. For example, `latest-dev`, or `pr-123-dev`.
+
+
+## [TODO] Magefile build system
+
+Once the V1 implementation reaches the stage of testable binaries, we are looking to use [Mage](https://magefile.org/) which is being tracked in [pocket/issues/43](https://github.com/pokt-network/pocket/issues/43) that'll inject a version with the `-version` flag.

go.mod

@@ -23,6 +23,7 @@ require (
 	github.com/dgraph-io/badger/v3 v3.2103.2
 	github.com/jackc/pgconn v1.11.0
 	github.com/jordanorelli/lexnum v0.0.0-20141216151731-460eeb125754
+	github.com/quasilyte/go-ruleguard/dsl v0.3.21
 )
 
 require (

go.sum

@@ -352,6 +352,8 @@ github.com/prometheus/procfs v0.1.3/go.mod h1:lV6e/gmhEcM9IjHGsFOCxxuZ+z1YqCvr4O
 github.com/prometheus/procfs v0.6.0/go.mod h1:cz+aTbrPOrUb4q7XlbU9ygM+/jj0fzG6c1xBZuNvfVA=
 github.com/prometheus/procfs v0.7.3 h1:4jVXhlkAyzOScmCkXBTOLRLTz8EeU+eyjrwB/EPq0VU=
 github.com/prometheus/procfs v0.7.3/go.mod h1:cz+aTbrPOrUb4q7XlbU9ygM+/jj0fzG6c1xBZuNvfVA=
+github.com/quasilyte/go-ruleguard/dsl v0.3.21 h1:vNkC6fC6qMLzCOGbnIHOd5ixUGgTbp3Z4fGnUgULlDA=
+github.com/quasilyte/go-ruleguard/dsl v0.3.21/go.mod h1:KeCP03KrjuSO0H1kTuZQCWlQPulDV6YMIXmpQss17rU=
 github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4=
 github.com/rogpeppe/go-internal v1.6.1 h1:/FiVV8dS/e+YqF2JvO3yXRFbBLTIuSDkuC7aBOAvL+k=
 github.com/rogpeppe/go-internal v1.6.1/go.mod h1:xXDCJY+GAPziupqXw64V24skbSoqbTEfhy4qGm1nDQc=

misc/linting-rules/tests.go

@@ -0,0 +1,19 @@
+//nolint // it's not a Go code file
+//go:build !codeanalysis
+// +build !codeanalysis
+
+// This file includes our custom linters.
+// If you want to add/modify an existing linter, please check out ruleguard documentation: https://github.com/quasilyte/go-ruleguard#documentation
+
+package gorules
+
+import (
+	"github.com/quasilyte/go-ruleguard/dsl"
+)
+
+// This is a custom linter that checks ensures a use of require.Equal
+func EqualInsteadOfTrue(m dsl.Matcher) {
+	m.Match(`require.True($t, $x == $y, $*args)`).
+		Suggest(`require.Equal($t, $x, $y, $args)`).
+		Report(`use require.Equal instead of require.True`)
+}