github: Add format check to CI

Add a new format check workflow to CI. To run format check locally: - on docker: docker build --target=format -o out . - see results in: out/format-output.txt - last line should be "PASS" - command line: make check To fix format errors, run "make fix" locally. Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
cilium · Oct 11, 2023 · 79f9a08 · 79f9a08
1 parent f1a1608
commit 79f9a08
Show file tree

Hide file tree

Showing 7 changed files with 86 additions and 9 deletions.
diff --git a/.github/workflows/ci-check-format.yaml b/.github/workflows/ci-check-format.yaml
@@ -0,0 +1,59 @@
+name: CI check format
+on:
+  pull_request_target:
+    types: [opened, synchronize, reopened]
+
+# By specifying the access of one of the scopes, all of those that are not specified are set to 'none'.
+permissions:
+  # To be able to access the repository with actions/checkout
+  contents: read
+
+jobs:
+  format:
+    timeout-minutes: 30
+    name: Check source format
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout PR Source Code
+        uses: actions/checkout@8ade135a41bc03ea155e62e844d188df1ea18608 # v4.1.0
+        with:
+          ref: ${{ github.event.pull_request.head.sha }}
+          persist-credentials: false
+
+      - name: Prep for build
+        run: |
+          echo "${{ github.event.pull_request.head.sha }}" >SOURCE_VERSION
+          echo "ENVOY_VERSION=$(cat ENVOY_VERSION)" >> $GITHUB_ENV
+          echo "BAZEL_VERSION=$(cat .bazelversion)" >> $GITHUB_ENV
+          echo "BUILDER_DOCKER_HASH=$(git ls-tree --full-tree HEAD -- ./Dockerfile.builder | awk '{ print $3 }')" >> $GITHUB_ENV
+
+      - name: Wait for cilium-envoy-builder to be available
+        timeout-minutes: 45
+        shell: bash
+        run: until docker manifest inspect quay.io/${{ github.repository_owner }}/cilium-envoy-builder-dev:${{ env.BAZEL_VERSION }}-${{ env.BUILDER_DOCKER_HASH }} &> /dev/null; do sleep 15s; done
+
+      - name: Check format
+        uses: docker/build-push-action@0565240e2d4ab88bba5387d719585280857ece09 # v5.0.0
+        id: docker_format_ciak
+        with:
+          target: format
+          provenance: false
+          context: .
+          file: ./Dockerfile
+          platforms: linux/amd64
+          outputs: type=local,dest=check-format-results
+          build-args: |
+            BUILDER_BASE=quay.io/${{ github.repository_owner }}/cilium-envoy-builder-dev:${{ env.BAZEL_VERSION }}-${{ env.BUILDER_DOCKER_HASH }}
+          cache-from: type=local,src=/tmp/buildx-cache
+          push: false
+
+      - name: Check for failure
+        run: ! grep "^Format check failed" check-format-results/format-output.txt
+
+      - name: Upload Format results
+        if: failure()
+        uses: actions/upload-artifact@a8a3f3ad30e3422c9c7b888a15615d19a852ae32 # v3.1.3
+        with:
+          name: check-format-results
+          path: check-format-results/format-output.txt
+          retention-days: 5
diff --git a/Dockerfile b/Dockerfile
@@ -90,6 +90,24 @@ FROM empty-builder-archive as builder-archive
 ARG COPY_CACHE_EXT
 COPY --from=builder /tmp/bazel-cache${COPY_CACHE_EXT}/ /tmp/bazel-cache/
 
+# Format check
+FROM --platform=$BUILDPLATFORM $BUILDER_BASE as check-format
+LABEL maintainer="maintainer@cilium.io"
+WORKDIR /cilium/proxy
+COPY --chown=1337:1337 . ./
+ARG V
+ARG BAZEL_BUILD_OPTS
+ARG DEBUG
+ENV TARGETARCH=$TARGETARCH
+#
+# Check format
+#
+RUN export PATH=$PATH:/usr/local/go/bin:/cilium/proxy/go/bin && \
+	BAZEL_BUILD_OPTS="${BAZEL_BUILD_OPTS}" PKG_BUILD=1 V=$V DEBUG=$DEBUG make V=1 check > format-output.txt
+
+FROM scratch as format
+COPY --from=check-format /cilium/proxy/format-output.txt /
+
 #
 # Extract installed cilium-envoy binaries to an otherwise empty image
 #

diff --git a/Dockerfile.dockerignore b/Dockerfile.dockerignore
@@ -26,11 +26,9 @@
 /BUILD_DEP_HASHES
 /.gitignore
 /Dockerfile*
-/Makefile.dev
 /Makefile.docker
 /README*
 /envoy_bootstrap_v2.patch
-/.clang-format
 /tests/
 /Makefile.api
 /envoy_binary_test.sh
diff --git a/Dockerfile.tests.dockerignore b/Dockerfile.tests.dockerignore
@@ -26,7 +26,6 @@
 /BUILD_DEP_HASHES
 /.gitignore
 /Dockerfile*
-/Makefile.dev
 /Makefile.docker
 /README*
 /envoy_bootstrap_v2.patch

diff --git a/Makefile b/Makefile
@@ -62,10 +62,15 @@ else
   BAZEL_BUILD_OPTS += --config=release
 endif
 
+include Makefile.dev
 ifdef PKG_BUILD
   all: cilium-envoy-starter cilium-envoy
+
+  .PHONY: install-bazel
+  install-bazel:
+	echo "Bazel assumed to be installed in the builder image"
+
 else
-  include Makefile.dev
   include Makefile.docker
 
   # Fetch and install Bazel if needed

diff --git a/Makefile.dev b/Makefile.dev
@@ -47,7 +47,7 @@ envoy-debug: $(COMPILER_DEP) SOURCE_VERSION install-bazel
 	@$(ECHO_BAZEL)
 	$(BAZEL) $(BAZEL_OPTS) build $(BAZEL_BUILD_OPTS) -c dbg //:cilium-envoy $(BAZEL_FILTER)
 
-$(CHECK_FORMAT): force-non-root SOURCE_VERSION install-bazel
+$(CHECK_FORMAT): force-non-root SOURCE_VERSION install-bazel clang.bazelrc
 	$(BAZEL) $(BAZEL_OPTS) build $(BAZEL_BUILD_OPTS) //:check_format.py
 
 veryclean: force-non-root clean
@@ -63,10 +63,10 @@ precheck: force-non-root
 ~/go/bin/buildozer:
 	go install github.com/bazelbuild/buildtools/buildozer@latest
 
-FORMAT_EXCLUDED_PREFIXES = "./linux/" "./proxylib/" "./starter/"  "./vendor/"
+FORMAT_EXCLUDED_PREFIXES = "./linux/" "./proxylib/" "./starter/"  "./vendor/" "./go/"
 
 check: $(CHECK_FORMAT) $(BUILDIFIER) $(BUILDOZER) force-non-root
-	CLANG_FORMAT=$(CLANG_FORMAT) BUILDIFIER=$(BUILDIFIER) BUILDOZER=$(BUILDOZER) $(CHECK_FORMAT) --skip_envoy_build_rule_check --add-excluded-prefixes $(FORMAT_EXCLUDED_PREFIXES) --bazel_tools_check_excluded_paths="./" --build_fixer_check_excluded_paths="./" check
+	CLANG_FORMAT=$(CLANG_FORMAT) BUILDIFIER=$(BUILDIFIER) BUILDOZER=$(BUILDOZER) $(CHECK_FORMAT) --skip_envoy_build_rule_check --add-excluded-prefixes $(FORMAT_EXCLUDED_PREFIXES) --bazel_tools_check_excluded_paths="./" --build_fixer_check_excluded_paths="./" check || echo "Format check failed, run 'make fix' locally to fix formatting errors."
 
 fix: $(CHECK_FORMAT) $(BUILDIFIER) $(BUILDOZER) force-non-root
 	CLANG_FORMAT=$(CLANG_FORMAT) BUILDIFIER=$(BUILDIFIER) BUILDOZER=$(BUILDOZER) $(CHECK_FORMAT) --skip_envoy_build_rule_check --add-excluded-prefixes $(FORMAT_EXCLUDED_PREFIXES) --bazel_tools_check_excluded_paths="." --build_fixer_check_excluded_paths="./" fix

diff --git a/Makefile.docker b/Makefile.docker
@@ -138,7 +138,6 @@ Dockerfile.builder.tests.dockerignore:
 # Release does not need Go API or test files
 Dockerfile.dockerignore: .dockerignore Makefile.docker
 	cp $< $@
-	echo "/.clang-format" >>$@
 	echo "/tests/" >>$@
 	echo "/Makefile.api" >>$@
 	echo "/envoy_binary_test.sh" >>$@
@@ -165,7 +164,6 @@ GIT_IGNORE_FILES := $(shell find -H . -not -path "./_build*" -not -path "./vendo
 	    $< >>$@
 	echo "/.gitignore" >>$@
 	echo "/Dockerfile*" >>$@
-	echo "/Makefile.dev" >>$@
 	echo "/Makefile.docker" >>$@
 	echo "/README*" >>$@
 	echo "/envoy_bootstrap_v2.patch" >>$@