Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GitHub Actions for lint #3211

Merged
merged 1 commit into from
Aug 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 0 additions & 36 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,41 +2,6 @@ version: 2

jobs:

lint:
working_directory: /work
docker: [{image: 'docker:20.10-git'}]
environment:
DOCKER_BUILDKIT: 1
steps:
- checkout
- setup_remote_docker:
version: 20.10.6
reusable: true
exclusive: false
- run:
name: "Docker version"
command: docker version
- run:
name: "Docker info"
command: docker info
- run:
name: "Shellcheck - build image"
command: |
docker build --progress=plain -f dockerfiles/Dockerfile.shellcheck --tag cli-validator:$CIRCLE_BUILD_NUM .
- run:
name: "Shellcheck"
command: |
docker run --rm cli-validator:$CIRCLE_BUILD_NUM \
make shellcheck
- run:
name: "Lint - build image"
command: |
docker build --progress=plain -f dockerfiles/Dockerfile.lint --tag cli-linter:$CIRCLE_BUILD_NUM .
- run:
name: "Lint"
command: |
docker run --rm cli-linter:$CIRCLE_BUILD_NUM

cross:
working_directory: /work
docker: [{image: 'docker:20.10-git'}]
Expand Down Expand Up @@ -147,7 +112,6 @@ workflows:
version: 2
ci:
jobs:
- lint
- cross
- test
- validate
30 changes: 30 additions & 0 deletions .github/workflows/validate.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
name: validate

on:
workflow_dispatch:
push:
branches:
- 'master'
- '[0-9]+.[0-9]{2}'
tags:
- 'v*'
pull_request:

jobs:
lint:
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
target:
- lint
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The official golangci-lint GitHub Action can also run on Windows, Darwin. Wondering if we can trick it by setting GOOS/GOARCH to also lint non-matching platforms (especially for the CLI, which will be extensively used on macOS and Windows)

Copy link
Member Author

@crazy-max crazy-max Aug 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I think we can have a special matrix to run Windows and MacOS runners in this case. We can do that in follow-up.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it's tricky, because of course we can't run a macOS container, and Windows doesn't support BuildKit for Windows containers (if it would, it's still possible that building takes a long time (Moby Ci can take 10 minutes to build the Dockerfile, a large portion of that is taken by just pulling the base image 😞).

So if it's possible to somehow trick golangci-lint (through GOOS / GOARCH) that would be a lot easier.

If not, then perhaps (for CI) we would have to consider using the official golangci-lint GitHub action, which doesn't (I think?) run containerised, but at least runs fast (and on Windows and macOS). We can still use the BuildKit approach for local use (and for Linux perhaps).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thaJeztah

using the official golangci-lint GitHub action

Yes I think we can do that with https://github.com/golangci/golangci-lint-action for Win/MacOS.

- shellcheck
steps:
-
name: Checkout
uses: actions/checkout@v2
-
name: Run
uses: docker/bake-action@v1
with:
targets: ${{ matrix.target }}
8 changes: 0 additions & 8 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,6 @@ test-coverage: ## run test coverage
fmt:
go list -f {{.Dir}} ./... | xargs gofmt -w -s -d

.PHONY: lint
lint: ## run all the lint tools
gometalinter --config gometalinter.json ./...

.PHONY: binary
binary:
docker buildx bake binary
Expand Down Expand Up @@ -71,10 +67,6 @@ manpages: ## generate man pages from go source and markdown
yamldocs: ## generate documentation YAML files consumed by docs repo
scripts/docs/generate-yaml.sh

.PHONY: shellcheck
shellcheck: ## run shellcheck validation
scripts/validate/shellcheck

.PHONY: help
help: ## print this help
@awk 'BEGIN {FS = ":.*?## "} /^[a-zA-Z0-9_-]+:.*?## / {gsub("\\\\n",sprintf("\n%22c",""), $$2);printf "\033[36m%-20s\033[0m %s\n", $$1, $$2}' $(MAKEFILE_LIST)
Expand Down
12 changes: 12 additions & 0 deletions docker-bake.hcl
Original file line number Diff line number Diff line change
Expand Up @@ -61,3 +61,15 @@ target "cross" {
target "dynbinary-cross" {
inherits = ["dynbinary", "_all_platforms"]
}

target "lint" {
dockerfile = "./dockerfiles/Dockerfile.lint"
target = "lint"
output = ["type=cacheonly"]
}

target "shellcheck" {
dockerfile = "./dockerfiles/Dockerfile.shellcheck"
target = "shellcheck"
output = ["type=cacheonly"]
}
21 changes: 4 additions & 17 deletions docker.Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,7 @@ DOCKER_CLI_GO_BUILD_CACHE ?= y

DEV_DOCKER_IMAGE_NAME = docker-cli-dev$(IMAGE_TAG)
BINARY_NATIVE_IMAGE_NAME = docker-cli-native$(IMAGE_TAG)
LINTER_IMAGE_NAME = docker-cli-lint$(IMAGE_TAG)
CROSS_IMAGE_NAME = docker-cli-cross$(IMAGE_TAG)
VALIDATE_IMAGE_NAME = docker-cli-shell-validate$(IMAGE_TAG)
E2E_IMAGE_NAME = docker-cli-e2e$(IMAGE_TAG)
E2E_ENGINE_VERSION ?=
CACHE_VOLUME_NAME := docker-cli-dev-cache
Expand All @@ -32,17 +30,6 @@ build_docker_image:
# build dockerfile from stdin so that we don't send the build-context; source is bind-mounted in the development environment
cat ./dockerfiles/Dockerfile.dev | docker build ${DOCKER_BUILD_ARGS} --build-arg=GO_VERSION -t $(DEV_DOCKER_IMAGE_NAME) -

# build docker image having the linting tools (dockerfiles/Dockerfile.lint)
.PHONY: build_linter_image
build_linter_image:
# build dockerfile from stdin so that we don't send the build-context; source is bind-mounted in the development environment
cat ./dockerfiles/Dockerfile.lint | docker build ${DOCKER_BUILD_ARGS} --build-arg=GO_VERSION -t $(LINTER_IMAGE_NAME) -

.PHONY: build_shell_validate_image
build_shell_validate_image:
# build dockerfile from stdin so that we don't send the build-context; source is bind-mounted in the development environment
cat ./dockerfiles/Dockerfile.shellcheck | docker build --build-arg=GO_VERSION -t $(VALIDATE_IMAGE_NAME) -

.PHONY: build_binary_native_image
build_binary_native_image:
# build dockerfile from stdin so that we don't send the build-context; source is bind-mounted in the development environment
Expand Down Expand Up @@ -92,8 +79,8 @@ dev: build_docker_image ## start a build container in interactive mode for in-co
shell: dev ## alias for dev

.PHONY: lint
lint: build_linter_image ## run linters
$(DOCKER_RUN) -it $(LINTER_IMAGE_NAME)
lint: ## run linters
docker buildx bake lint

.PHONY: fmt
fmt: ## run gofmt
Expand All @@ -120,8 +107,8 @@ yamldocs: build_docker_image ## generate documentation YAML files consumed by do
$(DOCKER_RUN) -it $(DEV_DOCKER_IMAGE_NAME) make yamldocs

.PHONY: shellcheck
shellcheck: build_shell_validate_image ## run shellcheck validation
$(DOCKER_RUN) -it $(VALIDATE_IMAGE_NAME) make shellcheck
shellcheck: ## run shellcheck validation
docker buildx bake shellcheck

.PHONY: test-e2e
test-e2e: test-e2e-non-experimental test-e2e-experimental test-e2e-connhelper-ssh ## run all e2e tests
Expand Down
28 changes: 10 additions & 18 deletions dockerfiles/Dockerfile.lint
Original file line number Diff line number Diff line change
@@ -1,24 +1,16 @@
# syntax=docker/dockerfile:1.3

ARG GO_VERSION=1.16.6
ARG GOLANGCI_LINTER_SHA="v1.21.0"
ARG GOLANGCI_LINT_VERSION=v1.23.8

FROM golang:${GO_VERSION}-alpine AS build
ENV CGO_ENABLED=0
RUN apk add --no-cache git
ARG GOLANGCI_LINTER_SHA
ARG GO111MODULE=on
RUN --mount=type=cache,target=/root/.cache/go-build \
--mount=type=cache,target=/go/pkg/mod \
go get github.com/golangci/golangci-lint/cmd/golangci-lint@${GOLANGCI_LINTER_SHA}
FROM golangci/golangci-lint:${GOLANGCI_LINT_VERSION}-alpine AS golangci-lint

FROM golang:${GO_VERSION}-alpine AS lint
ENV GO111MODULE=off
ENV CGO_ENABLED=0
ENV DISABLE_WARN_OUTSIDE_CONTAINER=1
COPY --from=build /go/bin/golangci-lint /usr/local/bin
FROM golang:${GO_VERSION}-alpine AS lint
ENV GO111MODULE=off
ENV CGO_ENABLED=0
ENV GOGC=75
WORKDIR /go/src/github.com/docker/cli
ENV GOGC=75
ENTRYPOINT ["/usr/local/bin/golangci-lint"]
CMD ["run", "--config=.golangci.yml"]
COPY . .
COPY --from=golangci-lint /usr/bin/golangci-lint /usr/bin/golangci-lint
RUN --mount=type=bind,target=. \
--mount=type=cache,target=/root/.cache \
golangci-lint run
10 changes: 6 additions & 4 deletions dockerfiles/Dockerfile.shellcheck
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
FROM koalaman/shellcheck-alpine:v0.7.1
RUN apk add --no-cache bash make
# syntax=docker/dockerfile:1.3

FROM koalaman/shellcheck-alpine:v0.7.1 AS shellcheck
WORKDIR /go/src/github.com/docker/cli
ENV DISABLE_WARN_OUTSIDE_CONTAINER=1
COPY . .
RUN --mount=type=bind,target=. \
crazy-max marked this conversation as resolved.
Show resolved Hide resolved
set -eo pipefail; \
find scripts/ contrib/completion/bash -type f | grep -v scripts/winresources | grep -v '.*.ps1' | xargs shellcheck
5 changes: 0 additions & 5 deletions scripts/validate/shellcheck

This file was deleted.