build: support clang tidy

.bazelrc

@@ -30,3 +30,6 @@ build:release --define manual_stamp=manual_stamp
 # Always have LD_LIBRARY_PATH=/usr/cross-compat/lib defined in the test environment.
 # The path does not need to exist, but can be created when needed for running tests.
 build --test_env=LD_LIBRARY_PATH=/usr/cilium-cross-compat/lib
+
+# use same env option for query as upstream is using for build
+query --incompatible_merge_fixed_and_default_shell_env

.clang-tidy

@@ -0,0 +1,122 @@
+Checks: >
+  -clang-analyzer-core.NonNullParamChecker,
+  -clang-analyzer-optin.cplusplus.UninitializedObject,
+  abseil-duration-*,
+  abseil-faster-strsplit-delimiter,
+  abseil-no-namespace,
+  abseil-redundant-strcat-calls,
+  abseil-str-cat-append,
+  abseil-string-find-startswith,
+  abseil-upgrade-duration-conversions,
+  bugprone-assert-side-effect,
+  bugprone-unused-raii,
+  bugprone-use-after-move,
+  clang-analyzer-core.DivideZero,
+  misc-unused-using-decls,
+  modernize-deprecated-headers,
+  modernize-loop-convert,
+  modernize-make-shared,
+  modernize-make-unique,
+  modernize-return-braced-init-list,
+  modernize-use-default-member-init,
+  modernize-use-equals-default,
+  modernize-use-nullptr,
+  modernize-use-override,
+  modernize-use-using,
+  performance-faster-string-find,
+  performance-for-range-copy,
+  performance-inefficient-algorithm,
+  performance-inefficient-vector-operation,
+  performance-noexcept-move-constructor,
+  performance-move-constructor-init,
+  performance-type-promotion-in-math-fn,
+  performance-unnecessary-copy-initialization,
+  readability-braces-around-statements,
+  readability-container-size-empty,
+  readability-identifier-naming,
+  readability-redundant-control-flow,
+  readability-redundant-member-init,
+  readability-redundant-smartptr-get,
+  readability-redundant-string-cstr
+
+CheckOptions:
+- key: cppcoreguidelines-unused-variable.IgnorePattern
+  value: "^_$"
+- key: bugprone-assert-side-effect.AssertMacros
+  value: 'ASSERT'
+- key: bugprone-dangling-handle.HandleClasses
+  value: 'std::basic_string_view;std::experimental::basic_string_view;absl::string_view'
+- key: misc-include-cleaner.IgnoreHeaders
+  value: 'fmt/format\.h;fmt/compile\.h;asm-generic/socket\.h;asm/unistd_32\.h;asm/unistd_64\.h;bits/.*;google/protobuf/.*;linux/in\.h;linux/in6\.h;mutex'
+- key: modernize-use-auto.MinTypeNameLength
+  value: '10'
+- key: readability-identifier-naming.ClassCase
+  value: 'CamelCase'
+- key: readability-identifier-naming.EnumCase
+  value: 'CamelCase'
+- key: readability-identifier-naming.EnumConstantCase
+  value: 'CamelCase'
+# Ignore GoogleTest function macros.
+- key: readability-identifier-naming.FunctionIgnoredRegexp
+  # To have the regex chomped correctly fence all items with `|` (other than first/last)
+  value: >-
+    (^AbslHashValue$|
+    |^called_count$|
+    |^case_sensitive$|
+    |^Create$|
+    |^envoy_resolve_dns$|
+    |^evconnlistener_free$|
+    |^event_base_free$|
+    |^(get|set)EVP_PKEY$|
+    |^has_value$|
+    |^Ip6(ntohl|htonl)$|
+    |^get_$|
+    |^HeaderHasValue(Ref)?$|
+    |^HeaderValueOf$|
+    |^Is(Superset|Subset)OfHeaders$|
+    |^LLVMFuzzerInitialize$|
+    |^LLVMFuzzerTestOneInput$|
+    |^Locality$|
+    |^MOCK_METHOD$|
+    |^PrepareCall$|
+    |^PrintTo$|
+    |^resolve_dns$|
+    |^result_type$|
+    |Returns(Default)?WorkerId$|
+    |^sched_getaffinity$|
+    |^shutdownThread_$|
+    |^envoy_dynamic_module(.*)$|
+    |TEST|
+    |^use_count$)
+- key: readability-identifier-naming.ParameterCase
+  value: 'lower_case'
+- key: readability-identifier-naming.ParameterIgnoredRegexp
+  value: (^cname_ttl_$)
+- key: readability-identifier-naming.PrivateMemberCase
+  value: 'lower_case'
+- key: readability-identifier-naming.PrivateMemberSuffix
+  value: '_'
+- key: readability-identifier-naming.StructCase
+  value: 'CamelCase'
+- key: readability-identifier-naming.TypeAliasCase
+  value: 'CamelCase'
+- key: readability-identifier-naming.TypeAliasIgnoredRegexp
+  value: '(result_type)'
+- key: readability-identifier-naming.UnionCase
+  value: 'CamelCase'
+- key: readability-identifier-naming.FunctionCase
+  value: 'camelBack'
+- key: readability-identifier-naming.LocalVariableCase
+  value: 'lower_case'
+
+HeaderFilterRegex: '^./source/.*|^./contrib/.*|^./test/.*|^./envoy/.*'
+
+UseColor: true
+
+WarningsAsErrors: '*'
+
+## The version here is arbitrary since any change to this file will
+## trigger a full run of clang-tidy against all files.
+## It can be useful as it seems some header changes may not trigger the
+## expected rerun.
+# v0

.clangd

@@ -6,12 +6,15 @@ Diagnostics:
   MissingIncludes: Strict
   Includes:
     IgnoreHeader:
-      - "asm/unistd_32.h" # private -> use unistd.h
-      - "asm/unistd_64.h" # private -> use unistd.h
-      - "bits/basic_string\\.h" # private -> use string
+      - "fmt/format\.h" # Do not remove or add this
+      - "fmt/compile\.h" # private -> use fmt/format.h
+      - "asm-generic/socket\.h" # private -> use sys/socket.h
+      - "asm/unistd_32\.h" # private -> use unistd.h
+      - "asm/unistd_64\.h" # private -> use unistd.h
+      - "bits/.*" # private -> use standard headers like <string>, etc.
       - "google/protobuf/.*" # checked by envoy linting -> use source/common/protobuf/protobuf.h
-      - "linux/in\\.h" # private -> use netinet/in.h
-      - "linux/in6\\.h" # private -> use netinet/in.h
+      - "linux/in\.h" # private -> use netinet/in.h
+      - "linux/in6\.h" # private -> use netinet/in.h
       - "mutex" # checked by envoy linting -> use source/common/common/thread.h
 # CompileFlags:
 #   CompilationDatabase: ./compile_commands.json

.github/workflows/ci-clang-tidy.yaml

@@ -0,0 +1,69 @@
+name: CI clang-tidy
+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
+
+concurrency:
+  group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.event.after }}
+  cancel-in-progress: true
+
+jobs:
+  tidy:
+    timeout-minutes: 60
+    name: Lint source style
+    runs-on: ubuntu-latest-64-cores-256gb
+    steps:
+      - name: Checkout PR Source Code
+        uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
+        with:
+          ref: ${{ github.event.pull_request.head.sha }}
+          persist-credentials: false
+          fetch-depth: 2
+
+      - name: Prep for build
+        run: |
+          echo "${{ github.event.pull_request.head.sha }}" >SOURCE_VERSION
+          echo "ENVOY_VERSION=$(cat ENVOY_VERSION)" >> $GITHUB_ENV
+          echo "BUILDER_DOCKER_HASH=$(git ls-tree --full-tree HEAD -- ./Dockerfile.builder | awk '{ print $3 }')" >> $GITHUB_ENV
+          # git diff filter has everything else than deleted files (those need not be tidied)
+          echo "TIDY_SOURCES=$(git diff --name-only --diff-filter=ACMRTUXB main | grep -E '(.h$|.cc$)' | tr '\n' ' ')" >> $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.BUILDER_DOCKER_HASH }} &> /dev/null; do sleep 15s; done
+
+      - name: Run clang-tidy
+        uses: docker/build-push-action@ca877d9245402d1537745e0e356eab47c3520991 # v6.13.0
+        id: docker_clang_tidy
+        with:
+          target: clang-tidy
+          provenance: false
+          context: .
+          file: ./Dockerfile
+          platforms: linux/amd64
+          outputs: type=local,dest=clang-tidy-results
+          build-args: |
+            BUILDER_BASE=quay.io/${{ github.repository_owner }}/cilium-envoy-builder-dev:${{ env.BUILDER_DOCKER_HASH }}
+          cache-from: type=local,src=/tmp/buildx-cache
+          push: false
+
+      - name: Check for failure
+        run: |
+          if grep -q "^make:.*Error" clang-tidy-results/clang-tidy-output.txt; then
+            git diff > clang-tidy-results/clang-tidy.diff
+            exit 1
+          fi
+
+      - name: Upload clang-tidy results
+        if: failure()
+        uses: actions/upload-artifact@65c4c4a1ddee5b72f698fdd19549f0f0fb45cf08 # v4.6.0
+        with:
+          name: clang-tidy-results
+          path: clang-tidy-results/
+          retention-days: 5

.gitignore

@@ -4,6 +4,7 @@
 !\.bazelversion
 !\.clangd
 !\.clang-format
+!\.clang-tidy
 !\.github
 !\.gitignore
 

Dockerfile

@@ -100,11 +100,29 @@ ENV TARGETARCH=$TARGETARCH
 #
 # Check format
 #
-RUN BAZEL_BUILD_OPTS="${BAZEL_BUILD_OPTS}" PKG_BUILD=1 V=$V DEBUG=$DEBUG make V=1 check > format-output.txt
+RUN BAZEL_BUILD_OPTS="${BAZEL_BUILD_OPTS}" PKG_BUILD=1 V=$V DEBUG=$DEBUG make V=1 format > format-output.txt
 
 FROM scratch AS format
 COPY --from=check-format /cilium/proxy/format-output.txt /
 
+# clang-tidy
+FROM --platform=$BUILDPLATFORM $BUILDER_BASE AS run-clang-tidy-fix
+LABEL maintainer="[email protected]"
+WORKDIR /cilium/proxy
+COPY --chown=1337:1337 . ./
+ARG V
+ARG BAZEL_BUILD_OPTS
+ARG DEBUG
+ARG TIDY_SOURCES="cilium/*.h cilium/*.cc tests/*.h tests/*.cc starter/*.h starter/*.cc"
+ENV TARGETARCH=$TARGETARCH
+#
+# Run clang tidy
+#
+RUN TIDY_SOURCES="${TIDY_SOURCES}" BAZEL_BUILD_OPTS="${BAZEL_BUILD_OPTS}" PKG_BUILD=1 V=$V DEBUG=$DEBUG make V=1 tidy-fix > clang-tidy-output.txt
+
+FROM scratch AS clang-tidy
+COPY --from=run-clang-tidy-fix /cilium/proxy/clang-tidy-output.txt /
+
 #
 # Extract installed cilium-envoy binaries to an otherwise empty image
 #

Dockerfile.builder

@@ -30,7 +30,7 @@ RUN apt-get update && \
     apt-add-repository -y "deb http://apt.llvm.org/jammy/ llvm-toolchain-jammy-17 main" && \
     apt-get update && \
     apt-get install -y --no-install-recommends \
-      clang-17 clang-tools-17 llvm-17-dev lldb-17 lld-17 clang-format-17 libc++-17-dev libc++abi-17-dev && \
+      clang-17 clang-tidy-17 clang-tools-17 llvm-17-dev lldb-17 lld-17 clang-format-17 libc++-17-dev libc++abi-17-dev && \
     apt-get purge --auto-remove && \
     apt-get clean && \
     rm -rf /var/lib/apt/lists/* /tmp/* /var/tmp/*

Makefile

@@ -104,7 +104,7 @@ else
   # Install clang if needed
   define install_clang
 	$(SUDO) apt info clang-17 || $(call add_clang_apt_source,$(shell lsb_release -cs))
-	$(SUDO) apt install -y clang-17 llvm-17-dev lld-17 clang-format-17
+	$(SUDO) apt install -y clang-17 clangd-17 llvm-17-dev lld-17 lldb-17 clang-format-17 clang-tools-17 clang-tidy-17
   endef
 endif
 

Makefile.dev

@@ -44,11 +44,33 @@ precheck: force-non-root
 
 FORMAT_EXCLUDED_PREFIXES = "./linux/" "./proxylib/" "./starter/"  "./vendor/" "./go/" "./envoy_build_config/"
 
-check: force-non-root
-	$(BAZEL) $(BAZEL_OPTS) run @envoy//tools/code_format:check_format -- --path "$(PWD)" --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."
+# The default set of sources assumes all relevant sources are dependecies of some tests!
+TIDY_SOURCES ?= $(shell bazel query 'kind("source file", deps(//tests/...))' 2>/dev/null | sed -n "s/\/\/cilium:/cilium\//p; s/\/\/tests:/tests\//p")
 
-fix: force-non-root
-	$(BAZEL) $(BAZEL_OPTS) run @envoy//tools/code_format:check_format -- --path "$(PWD)" --skip_envoy_build_rule_check --add-excluded-prefixes $(FORMAT_EXCLUDED_PREFIXES) --bazel_tools_check_excluded_paths="." --build_fixer_check_excluded_paths="./" fix
+# Must pass our bazel options to avoid discarding the analysis cache due to different options
+# between this, check and build!
+# Depend on the WORKSPACE and TIDY_SOURCES so that the database will be re-built if
+# Envoy dependency or any of the source files has changed.
+compile_commands.json: WORKSPACE $(TIDY_SOURCES) force-non-root
+	BAZEL_STARTUP_OPTION_LIST="$(BAZEL_OPTS)" BAZEL_BUILD_OPTION_LIST="$(BAZEL_BUILD_OPTS)" tools/gen_compilation_database.py --include_all //cilium/... //starter/... //tests/... @com_google_absl//absl/...
+
+# Default number of jobs, derived from available memory
+TIDY_JOBS ?= $$(( $(shell sed -n "s/^MemAvailable: *\([0-9]*\).*\$$/\1/p" /proc/meminfo) / 4500000 ))
+
+# tidy uses clang-tidy-17, .clang-tidy must be present in the project directory and configured to
+# ignore the same headers as .clangd. Unfortunately the configuration format is different.
+tidy: compile_commands.json force-non-root
+	run-clang-tidy-17 -quiet -extra-arg="-Wno-unknown-pragmas" -checks=misc-include-cleaner -j $(TIDY_JOBS) $(TIDY_SOURCES)
+
+tidy-fix: compile_commands.json force-non-root
+	echo "clang-tidy fix results can contain duplicate includes, check before committing!"
+	run-clang-tidy-17 -fix -format -style=file -quiet -extra-arg="-Wno-unknown-pragmas" -checks=misc-include-cleaner -j $(TIDY_JOBS) $(TIDY_SOURCES)
+
+format: force-non-root
+	$(BAZEL) $(BAZEL_OPTS) run $(BAZEL_BUILD_OPTS) @envoy//tools/code_format:check_format -- --path "$(PWD)" --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."
+
+format-fix: force-non-root
+	$(BAZEL) $(BAZEL_OPTS) run $(BAZEL_BUILD_OPTS) @envoy//tools/code_format:check_format -- --path "$(PWD)" --skip_envoy_build_rule_check --add-excluded-prefixes $(FORMAT_EXCLUDED_PREFIXES) --bazel_tools_check_excluded_paths="." --build_fixer_check_excluded_paths="./" fix
 
 # Run tests without debug by default.
 tests:  $(COMPILER_DEP) force-non-root SOURCE_VERSION proxylib/libcilium.so install-bazelisk