From d834b7bbf50e8f29d871421b164c3a104748918d Mon Sep 17 00:00:00 2001 From: Jarno Rajahalme Date: Sun, 2 Feb 2025 17:19:53 +0100 Subject: [PATCH 01/12] build: Add clang-tidy support with include cleaner Add the following clang packages to install_clang function: - clangd-17 - lldb-17 - clang-tools-17 - cland-tidy-17 Add make targets: - compile_commands.json: create the compile commands database needed for both clang-tidy and clangd. Include absl which otherwise was unreachable for clangd. - tidy: Use run-clang-tidy-17 to run clang-tidy in parallel. It is still very slow. - tidy-fix: Run clang tidy and fix errors. Use with caution, as the fixes do not always compile. Add query option --incompatible_merge_fixed_and_default_shell_env to .bazelrc so that query does not trigger re-download of envoy dependency. This same option is set on 'build' in 'envoy.bazelrc'. Add some include rules that seem necessary to .clangd and implement the same rules in .clang-tidy. Signed-off-by: Jarno Rajahalme --- .bazelrc | 3 ++ .clang-tidy | 120 +++++++++++++++++++++++++++++++++++++++++++++++++++ .clangd | 13 +++--- .gitignore | 1 + Makefile | 2 +- Makefile.dev | 26 ++++++++++- 6 files changed, 157 insertions(+), 8 deletions(-) create mode 100644 .clang-tidy diff --git a/.bazelrc b/.bazelrc index bad509951..86b45cc4e 100644 --- a/.bazelrc +++ b/.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 diff --git a/.clang-tidy b/.clang-tidy new file mode 100644 index 000000000..954513add --- /dev/null +++ b/.clang-tidy @@ -0,0 +1,120 @@ +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' + +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 diff --git a/.clangd b/.clangd index fa6120eff..b94390a5a 100644 --- a/.clangd +++ b/.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 , 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 diff --git a/.gitignore b/.gitignore index ef20ca8ea..1ed21c252 100644 --- a/.gitignore +++ b/.gitignore @@ -4,6 +4,7 @@ !\.bazelversion !\.clangd !\.clang-format +!\.clang-tidy !\.github !\.gitignore diff --git a/Makefile b/Makefile index 9409d76ae..6ca8e74f0 100644 --- a/Makefile +++ b/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 diff --git a/Makefile.dev b/Makefile.dev index 201c55a03..c5b8e9e65 100644 --- a/Makefile.dev +++ b/Makefile.dev @@ -44,11 +44,33 @@ precheck: force-non-root FORMAT_EXCLUDED_PREFIXES = "./linux/" "./proxylib/" "./starter/" "./vendor/" "./go/" "./envoy_build_config/" +# 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") + +# 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) + 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." + $(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." 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 + $(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 From 7b08f139cedf0fdd9120f80005855d83230164d7 Mon Sep 17 00:00:00 2001 From: Jarno Rajahalme Date: Sun, 2 Feb 2025 19:28:47 +0100 Subject: [PATCH 02/12] GH: Add clang-tidy workflow Add ci-clang-tidy workflow, linting files changed from 'main'. Signed-off-by: Jarno Rajahalme --- .github/workflows/ci-clang-tidy.yaml | 69 ++++++++++++++++++++++++++++ Dockerfile | 18 ++++++++ Dockerfile.builder | 2 +- 3 files changed, 88 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/ci-clang-tidy.yaml diff --git a/.github/workflows/ci-clang-tidy.yaml b/.github/workflows/ci-clang-tidy.yaml new file mode 100644 index 000000000..dd8b5433a --- /dev/null +++ b/.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 diff --git a/Dockerfile b/Dockerfile index 204d6f9f2..ce98addff 100644 --- a/Dockerfile +++ b/Dockerfile @@ -105,6 +105,24 @@ RUN BAZEL_BUILD_OPTS="${BAZEL_BUILD_OPTS}" PKG_BUILD=1 V=$V DEBUG=$DEBUG make V= 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="maintainer@cilium.io" +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 # diff --git a/Dockerfile.builder b/Dockerfile.builder index 9178407e8..e2a15c153 100644 --- a/Dockerfile.builder +++ b/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/* From b54ae372086c3c5eacf9ead9ac8835b06db15a20 Mon Sep 17 00:00:00 2001 From: Jarno Rajahalme Date: Thu, 16 Jan 2025 13:32:02 +0100 Subject: [PATCH 03/12] build: Rename 'check' and 'fix' targets Now that we have new targets 'tidy' and 'tidy-fix', rename the old clang-format targets 'check' and 'fix' as 'format' and 'format-fix', respectively. Signed-off-by: Jarno Rajahalme --- Dockerfile | 2 +- Makefile.dev | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Dockerfile b/Dockerfile index ce98addff..0ca7c2b7e 100644 --- a/Dockerfile +++ b/Dockerfile @@ -100,7 +100,7 @@ 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 / diff --git a/Makefile.dev b/Makefile.dev index c5b8e9e65..f1b166639 100644 --- a/Makefile.dev +++ b/Makefile.dev @@ -66,10 +66,10 @@ 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) -check: force-non-root +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." -fix: force-non-root +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. From 95c1371f93fa34d04f7bc9b6390526da58bd983c Mon Sep 17 00:00:00 2001 From: Jarno Rajahalme Date: Fri, 31 Jan 2025 14:55:38 +0100 Subject: [PATCH 04/12] tls_wrapper: Rename policy_socket_option as policy_ref We do not have a policy socket options any more, but a CiliumPolicyFilterState that contains a weak reference to the policy map. Rename 'policy_socket_option' as 'policy_ref' to make this a bit clearer. Signed-off-by: Jarno Rajahalme --- cilium/tls_wrapper.cc | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/cilium/tls_wrapper.cc b/cilium/tls_wrapper.cc index 6a0f364bb..1c53a4e8e 100644 --- a/cilium/tls_wrapper.cc +++ b/cilium/tls_wrapper.cc @@ -87,24 +87,24 @@ class SslSocketWrapper : public Network::TransportSocket, Logger::Loggableconnection(); ENVOY_CONN_LOG(trace, "retrieving policy filter state", conn); - auto policy_socket_option = + auto policy_ref = conn.streamInfo().filterState()->getDataReadOnly( Cilium::CiliumPolicyFilterState::key()); - if (policy_socket_option) { - const auto& policy = policy_socket_option->getPolicy(); + if (policy_ref) { + const auto& policy = policy_ref->getPolicy(); // Resolve the destination security ID and port uint32_t destination_identity = 0; - uint32_t destination_port = policy_socket_option->port_; + uint32_t destination_port = policy_ref->port_; const Network::Address::Ip* dip = nullptr; bool is_client = state_ == Extensions::TransportSockets::Tls::InitialState::Client; - if (!policy_socket_option->ingress_) { + if (!policy_ref->ingress_) { Network::Address::InstanceConstSharedPtr dst_address = is_client ? callbacks_->connection().connectionInfoProvider().remoteAddress() : callbacks_->connection().connectionInfoProvider().localAddress(); @@ -112,7 +112,7 @@ class SslSocketWrapper : public Network::TransportSocket, Logger::Loggableip(); if (dip) { destination_port = dip->port(); - destination_identity = policy_socket_option->resolvePolicyId(dip); + destination_identity = policy_ref->resolvePolicyId(dip); } else { ENVOY_CONN_LOG(warn, "cilium.tls_wrapper: Non-IP destination address: {}", conn, dst_address->asString()); @@ -123,11 +123,10 @@ class SslSocketWrapper : public Network::TransportSocket, Logger::Loggablesni_; + const auto& sni = policy_ref->sni_; - auto remote_id = policy_socket_option->ingress_ ? policy_socket_option->source_identity_ - : destination_identity; - auto port_policy = policy.findPortPolicy(policy_socket_option->ingress_, destination_port); + auto remote_id = policy_ref->ingress_ ? policy_ref->source_identity_ : destination_identity; + auto port_policy = policy.findPortPolicy(policy_ref->ingress_, destination_port); const Envoy::Ssl::ContextConfig* config = nullptr; bool raw_socket_allowed = false; Envoy::Ssl::ContextSharedPtr ctx = @@ -157,7 +156,7 @@ class SslSocketWrapper : public Network::TransportSocket, Logger::Loggable"); - if (policy_socket_option->ingress_) { + if (policy_ref->ingress_) { Network::Address::InstanceConstSharedPtr src_address = is_client ? callbacks_->connection().connectionInfoProvider().localAddress() : callbacks_->connection().connectionInfoProvider().remoteAddress(); @@ -176,9 +175,9 @@ class SslSocketWrapper : public Network::TransportSocket, Logger::Loggablepod_ip_, - policy_socket_option->ingress_ ? "source" : "destination", ipStr, remote_id, - destination_port, sni); + conn, is_client ? "client" : "server", policy_ref->pod_ip_, + policy_ref->ingress_ ? "source" : "destination", ipStr, remote_id, destination_port, + sni); } } else { ENVOY_CONN_LOG(warn, From bf013cd75123355eeda5d53e981378f485760556 Mon Sep 17 00:00:00 2001 From: Jarno Rajahalme Date: Sat, 1 Feb 2025 17:17:43 +0100 Subject: [PATCH 05/12] conntrack: Use Thread::LockGuard Signed-off-by: Jarno Rajahalme --- cilium/conntrack.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cilium/conntrack.cc b/cilium/conntrack.cc index efbd01353..7fa3a450f 100644 --- a/cilium/conntrack.cc +++ b/cilium/conntrack.cc @@ -12,8 +12,8 @@ #include "envoy/common/platform.h" #include "envoy/network/address.h" +#include "source/common/common/lock_guard.h" #include "source/common/common/logger.h" -#include "source/common/common/thread.h" #include "source/common/common/utility.h" #include "absl/container/flat_hash_map.h" @@ -160,7 +160,7 @@ CtMap::openMap6(const std::string& map_name) { } void CtMap::closeMaps(const absl::flat_hash_set& to_be_closed) { - std::lock_guard guard(maps_mutex_); + Thread::LockGuard guard(maps_mutex_); for (const auto& name : to_be_closed) { auto ct4 = ct_maps4_.find(name); @@ -224,7 +224,7 @@ uint32_t CtMap::lookupSrcIdentity(const std::string& map_name, const Network::Ad if (dip->version() == Network::Address::IpVersion::v4) { // Lock for the duration of the map lookup and conntrack lookup - std::lock_guard guard(maps_mutex_); + Thread::LockGuard guard(maps_mutex_); auto it = ct_maps4_.find(map_name); if (it == ct_maps4_.end()) { it = openMap4(map_name); @@ -242,7 +242,7 @@ uint32_t CtMap::lookupSrcIdentity(const std::string& map_name, const Network::Ad } } else { // Lock for the duration of the map lookup and conntrack lookup - std::lock_guard guard(maps_mutex_); + Thread::LockGuard guard(maps_mutex_); auto it = ct_maps6_.find(map_name); if (it == ct_maps6_.end()) { it = openMap6(map_name); From bfe3d11d59d5379ddcc78b123421ce85c5d73d08 Mon Sep 17 00:00:00 2001 From: Jarno Rajahalme Date: Sun, 2 Feb 2025 12:20:19 +0100 Subject: [PATCH 06/12] proxylib: Separate data_len from input_len Use a separate local variable 'data_len' to make it clear that it does not need to be updated before 'input_len' is set. Signed-off-by: Jarno Rajahalme --- cilium/proxylib.cc | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/cilium/proxylib.cc b/cilium/proxylib.cc index b4b724dcd..7504e2eab 100644 --- a/cilium/proxylib.cc +++ b/cilium/proxylib.cc @@ -115,7 +115,7 @@ GoFilter::InstancePtr GoFilter::NewInstance(Network::Connection& conn, const std FilterResult GoFilter::Instance::OnIO(bool reply, Buffer::Instance& data, bool end_stream) { auto& dir = reply ? reply_ : orig_; - int64_t input_len = data.length(); + int64_t data_len = data.length(); // Pass bytes based on an earlier verdict? if (dir.pass_bytes_ > 0) { @@ -124,11 +124,11 @@ FilterResult GoFilter::Instance::OnIO(bool reply, Buffer::Instance& data, bool e ASSERT(dir.need_bytes_ == 0); // Passed bytes can't be needed // Can return immediately if passing more that we have input. // May need to process injected data even when there is no input left. - if (dir.pass_bytes_ > input_len) { - if (input_len > 0) { + if (dir.pass_bytes_ > data_len) { + if (data_len > 0) { ENVOY_CONN_LOG(debug, "Cilium Network::OnIO: Passing all input: {} bytes: {} ", conn_, - input_len, data.toString()); - dir.pass_bytes_ -= input_len; + data_len, data.toString()); + dir.pass_bytes_ -= data_len; } return FILTER_OK; // all of 'data' is passed to the next filter } @@ -142,19 +142,18 @@ FilterResult GoFilter::Instance::OnIO(bool reply, Buffer::Instance& data, bool e ASSERT(dir.need_bytes_ == 0); // Dropped bytes can't be needed // Can return immediately if passing more that we have input. // May need to process injected data even when there is no input left. - if (dir.drop_bytes_ > input_len) { - if (input_len > 0) { + if (dir.drop_bytes_ > data_len) { + if (data_len > 0) { ENVOY_CONN_LOG(debug, "Cilium Network::OnIO: Dropping all input: {} bytes: {} ", conn_, - input_len, data.toString()); - dir.drop_bytes_ -= input_len; - data.drain(input_len); + data_len, data.toString()); + dir.drop_bytes_ -= data_len; + data.drain(data_len); } return FILTER_OK; // everything was dropped, nothing more to be done } ENVOY_CONN_LOG(debug, "Cilium Network::OnIO: Dropping first {} bytes of input: {}", conn_, dir.drop_bytes_, data.toString()); data.drain(dir.drop_bytes_); - input_len -= dir.drop_bytes_; dir.drop_bytes_ = 0; // At frame boundary, more data may remain } @@ -164,7 +163,7 @@ FilterResult GoFilter::Instance::OnIO(bool reply, Buffer::Instance& data, bool e dir.buffer_.move(data); ASSERT(data.length() == 0); auto& input = dir.buffer_; - input_len = input.length(); + int64_t input_len = input.length(); auto& output = data; // Move pre-passed input to output. From 287f8b24fcc46c46d2201a2bf2f06079ebf7226e Mon Sep 17 00:00:00 2001 From: Jarno Rajahalme Date: Sun, 2 Feb 2025 17:22:07 +0100 Subject: [PATCH 07/12] accesslog: Rename parameter with trailing underlines clang-tidy fix does not correctly handle renaming when removing trailing newlines from parameter names, so do this manually. Signed-off-by: Jarno Rajahalme --- cilium/accesslog.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cilium/accesslog.cc b/cilium/accesslog.cc index 2e6f9c222..44cfc3ee2 100644 --- a/cilium/accesslog.cc +++ b/cilium/accesslog.cc @@ -57,15 +57,15 @@ AccessLog::~AccessLog() { logs.erase(path_); } -void AccessLog::Log(AccessLog::Entry& entry__, ::cilium::EntryType entry_type) { - ::cilium::LogEntry& entry = entry__.entry_; +void AccessLog::Log(AccessLog::Entry& log_entry, ::cilium::EntryType entry_type) { + ::cilium::LogEntry& entry = log_entry.entry_; entry.set_entry_type(entry_type); if (entry_type != ::cilium::EntryType::Response) { - if (entry__.request_logged_) { + if (log_entry.request_logged_) { ENVOY_LOG_MISC(warn, "cilium.AccessLog: Request is logged twice"); } - entry__.request_logged_ = true; + log_entry.request_logged_ = true; } // encode protobuf From f203c080533f857c2eca3fbc82b715b1262a2cac Mon Sep 17 00:00:00 2001 From: Jarno Rajahalme Date: Mon, 3 Feb 2025 20:11:47 +0100 Subject: [PATCH 08/12] treewide: Fix includes Remove unnecessary includes, as well as unnecessary IWYU pragmas, while still keeping both clangd and clang-tidy happy. Signed-off-by: Jarno Rajahalme --- cilium/accesslog.cc | 6 ------ cilium/bpf.cc | 2 +- cilium/bpf.h | 2 -- cilium/bpf_metadata.cc | 4 +--- cilium/conntrack.cc | 4 ++-- cilium/filter_state_cilium_policy.cc | 4 ---- cilium/filter_state_cilium_policy.h | 4 ---- cilium/health_check_sink.h | 2 +- cilium/host_map.h | 2 +- cilium/ipcache.cc | 2 +- cilium/network_filter.cc | 4 +--- cilium/network_policy.cc | 3 ++- cilium/network_policy.h | 4 ++-- cilium/policy_id.h | 2 +- cilium/privileged_service_client.h | 7 ++++++- cilium/socket_option_cilium_mark.cc | 3 --- cilium/tls_wrapper.cc | 2 +- cilium/tls_wrapper.h | 2 +- cilium/uds_client.cc | 3 +-- cilium/websocket.cc | 4 +--- cilium/websocket_config.cc | 3 --- tests/accesslog_server.cc | 2 -- tests/cilium_http_integration.cc | 1 + tests/cilium_tls_http_integration_test.cc | 1 + tests/cilium_tls_tcp_integration_test.cc | 1 + tests/health_check_sink_server.cc | 5 ----- tests/uds_server.cc | 2 +- 27 files changed, 27 insertions(+), 54 deletions(-) diff --git a/cilium/accesslog.cc b/cilium/accesslog.cc index 44cfc3ee2..6c6609fcb 100644 --- a/cilium/accesslog.cc +++ b/cilium/accesslog.cc @@ -1,10 +1,5 @@ #include "accesslog.h" -#include -#include -#include -#include - #include #include #include @@ -20,7 +15,6 @@ #include "source/common/common/lock_guard.h" #include "source/common/common/logger.h" #include "source/common/common/thread.h" -#include "source/common/protobuf/protobuf.h" // IWYU pragma: keep #include "source/common/protobuf/utility.h" #include "absl/strings/numbers.h" diff --git a/cilium/bpf.cc b/cilium/bpf.cc index 3ab5cfa3a..9e2b1c67b 100644 --- a/cilium/bpf.cc +++ b/cilium/bpf.cc @@ -1,8 +1,8 @@ #include "cilium/bpf.h" -#include #include +#include #include #include #include diff --git a/cilium/bpf.h b/cilium/bpf.h index f09b30a37..9cb04a98a 100644 --- a/cilium/bpf.h +++ b/cilium/bpf.h @@ -1,7 +1,5 @@ #pragma once -#include - #include #include diff --git a/cilium/bpf_metadata.cc b/cilium/bpf_metadata.cc index 4fd5062e1..2d5359630 100644 --- a/cilium/bpf_metadata.cc +++ b/cilium/bpf_metadata.cc @@ -1,10 +1,8 @@ #include "cilium/bpf_metadata.h" -#include #include #include #include -#include #include #include @@ -33,7 +31,7 @@ #include "source/common/network/socket_option_factory.h" #include "source/common/network/socket_option_impl.h" #include "source/common/network/utility.h" -#include "source/common/protobuf/protobuf.h" // IWYU pragma: keep +#include "source/common/protobuf/protobuf.h" #include "source/common/protobuf/utility.h" #include "absl/strings/string_view.h" diff --git a/cilium/conntrack.cc b/cilium/conntrack.cc index 7fa3a450f..61648b0ac 100644 --- a/cilium/conntrack.cc +++ b/cilium/conntrack.cc @@ -1,10 +1,10 @@ #include "conntrack.h" -#include #include -#include +#include // IWYU pragma: keep #include +#include #include #include #include diff --git a/cilium/filter_state_cilium_policy.cc b/cilium/filter_state_cilium_policy.cc index a6dc6ac4d..ed57b95be 100644 --- a/cilium/filter_state_cilium_policy.cc +++ b/cilium/filter_state_cilium_policy.cc @@ -1,9 +1,5 @@ #include "cilium/filter_state_cilium_policy.h" -#include -#include - -#include #include #include "source/common/common/macros.h" diff --git a/cilium/filter_state_cilium_policy.h b/cilium/filter_state_cilium_policy.h index 92be513e8..83cc476ee 100644 --- a/cilium/filter_state_cilium_policy.h +++ b/cilium/filter_state_cilium_policy.h @@ -1,9 +1,5 @@ #pragma once -#include -#include - -#include #include #include #include diff --git a/cilium/health_check_sink.h b/cilium/health_check_sink.h index 378784a4e..12d1f1bce 100644 --- a/cilium/health_check_sink.h +++ b/cilium/health_check_sink.h @@ -10,7 +10,7 @@ #include "envoy/upstream/health_check_event_sink.h" #include "source/common/common/thread.h" -#include "source/common/protobuf/protobuf.h" // IWYU pragma: keep +#include "source/common/protobuf/protobuf.h" #include "absl/base/thread_annotations.h" #include "cilium/api/health_check_sink.pb.h" diff --git a/cilium/host_map.h b/cilium/host_map.h index f0b8e56ca..78b2f6163 100644 --- a/cilium/host_map.h +++ b/cilium/host_map.h @@ -27,7 +27,7 @@ #include "source/common/common/macros.h" #include "source/common/network/utility.h" #include "source/common/protobuf/message_validator_impl.h" -#include "source/common/protobuf/protobuf.h" // IWYU pragma: keep +#include "source/common/protobuf/protobuf.h" #include "source/common/protobuf/utility.h" #include "absl/container/flat_hash_map.h" diff --git a/cilium/ipcache.cc b/cilium/ipcache.cc index 45a7cbb30..18c9ed2a4 100644 --- a/cilium/ipcache.cc +++ b/cilium/ipcache.cc @@ -1,8 +1,8 @@ #include "ipcache.h" -#include #include +#include // IWYU pragma: keep #include #include #include diff --git a/cilium/network_filter.cc b/cilium/network_filter.cc index d16461042..e895ee212 100644 --- a/cilium/network_filter.cc +++ b/cilium/network_filter.cc @@ -1,7 +1,5 @@ #include "cilium/network_filter.h" -#include - #include #include #include @@ -22,7 +20,7 @@ #include "source/common/common/logger.h" #include "source/common/network/upstream_server_name.h" #include "source/common/network/upstream_subject_alt_names.h" -#include "source/common/protobuf/protobuf.h" // IWYU pragma: keep +#include "source/common/protobuf/protobuf.h" #include "source/common/protobuf/utility.h" #include "absl/status/statusor.h" diff --git a/cilium/network_policy.cc b/cilium/network_policy.cc index b4684cbfa..95ae285b6 100644 --- a/cilium/network_policy.cc +++ b/cilium/network_policy.cc @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -36,7 +37,7 @@ #include "source/common/init/target_impl.h" #include "source/common/init/watcher_impl.h" #include "source/common/network/utility.h" -#include "source/common/protobuf/protobuf.h" // IWYU pragma: keep +#include "source/common/protobuf/protobuf.h" #include "source/common/protobuf/utility.h" #include "source/extensions/config_subscription/grpc/grpc_subscription_impl.h" #include "source/server/transport_socket_config_impl.h" diff --git a/cilium/network_policy.h b/cilium/network_policy.h index 8e38de2cd..ac17e74e9 100644 --- a/cilium/network_policy.h +++ b/cilium/network_policy.h @@ -26,7 +26,7 @@ #include "envoy/ssl/context.h" #include "envoy/ssl/context_config.h" #include "envoy/stats/scope.h" -#include "envoy/stats/stats_macros.h" +#include "envoy/stats/stats_macros.h" // IWYU pragma: keep #include "source/common/common/assert.h" #include "source/common/common/logger.h" @@ -34,7 +34,7 @@ #include "source/common/common/thread.h" #include "source/common/init/target_impl.h" #include "source/common/protobuf/message_validator_impl.h" -#include "source/common/protobuf/protobuf.h" // IWYU pragma: keep +#include "source/common/protobuf/protobuf.h" #include "source/common/protobuf/utility.h" #include "source/server/transport_socket_config_impl.h" diff --git a/cilium/policy_id.h b/cilium/policy_id.h index 98ab9f6b9..42134635e 100644 --- a/cilium/policy_id.h +++ b/cilium/policy_id.h @@ -1,6 +1,6 @@ #pragma once -#include +#include namespace Envoy { namespace Cilium { diff --git a/cilium/privileged_service_client.h b/cilium/privileged_service_client.h index 6a961d74e..8e5b0f822 100644 --- a/cilium/privileged_service_client.h +++ b/cilium/privileged_service_client.h @@ -4,9 +4,14 @@ #error "Linux platform file is part of non-Linux build." #endif +#include +#include + +#include +#include + #include "envoy/api/os_sys_calls_common.h" -#include "source/common/common/assert.h" #include "source/common/singleton/threadsafe_singleton.h" #include "starter/privileged_service_protocol.h" diff --git a/cilium/socket_option_cilium_mark.cc b/cilium/socket_option_cilium_mark.cc index d1b6a3f52..34823a93e 100644 --- a/cilium/socket_option_cilium_mark.cc +++ b/cilium/socket_option_cilium_mark.cc @@ -1,8 +1,5 @@ #include "cilium/socket_option_cilium_mark.h" -#include -#include - #include #include diff --git a/cilium/tls_wrapper.cc b/cilium/tls_wrapper.cc index 1c53a4e8e..9fa95c198 100644 --- a/cilium/tls_wrapper.cc +++ b/cilium/tls_wrapper.cc @@ -21,7 +21,7 @@ #include "source/common/common/logger.h" #include "source/common/network/raw_buffer_socket.h" #include "source/common/network/transport_socket_options_impl.h" -#include "source/common/protobuf/protobuf.h" // IWYU pragma: keep +#include "source/common/protobuf/protobuf.h" #include "source/common/tls/ssl_socket.h" #include "absl/status/statusor.h" diff --git a/cilium/tls_wrapper.h b/cilium/tls_wrapper.h index 78d67881f..1042db80c 100644 --- a/cilium/tls_wrapper.h +++ b/cilium/tls_wrapper.h @@ -8,7 +8,7 @@ #include "envoy/server/transport_socket_config.h" #include "envoy/stats/stats_macros.h" // IWYU pragma: keep -#include "source/common/protobuf/protobuf.h" // IWYU pragma: keep +#include "source/common/protobuf/protobuf.h" #include "absl/status/statusor.h" diff --git a/cilium/uds_client.cc b/cilium/uds_client.cc index 0bd3ebef0..4d62ee3f7 100644 --- a/cilium/uds_client.cc +++ b/cilium/uds_client.cc @@ -1,12 +1,11 @@ #include "cilium/uds_client.h" -#include #include -#include #include #include #include +#include #include #include diff --git a/cilium/websocket.cc b/cilium/websocket.cc index 8b657a687..20e51cb10 100644 --- a/cilium/websocket.cc +++ b/cilium/websocket.cc @@ -1,7 +1,5 @@ #include "cilium/websocket.h" -#include - #include #include #include @@ -18,7 +16,7 @@ #include "source/common/common/logger.h" #include "source/common/http/headers.h" #include "source/common/network/utility.h" -#include "source/common/protobuf/protobuf.h" // IWYU pragma: keep +#include "source/common/protobuf/protobuf.h" #include "source/common/protobuf/utility.h" #include "source/common/stream_info/bool_accessor_impl.h" #include "source/common/tcp_proxy/tcp_proxy.h" diff --git a/cilium/websocket_config.cc b/cilium/websocket_config.cc index 0e51c8828..c23318df9 100644 --- a/cilium/websocket_config.cc +++ b/cilium/websocket_config.cc @@ -1,11 +1,9 @@ #include "cilium/websocket_config.h" -#include #include #include #include -#include #include #include #include @@ -21,7 +19,6 @@ #include "source/common/common/assert.h" #include "source/common/common/base64.h" #include "source/common/http/request_id_extension_impl.h" -#include "source/common/protobuf/protobuf.h" // IWYU pragma: keep #include "source/common/protobuf/utility.h" #include "absl/strings/ascii.h" diff --git a/tests/accesslog_server.cc b/tests/accesslog_server.cc index 15e371bc0..37d20bb4a 100644 --- a/tests/accesslog_server.cc +++ b/tests/accesslog_server.cc @@ -1,7 +1,5 @@ #include "tests/accesslog_server.h" -#include - #include #include #include diff --git a/tests/cilium_http_integration.cc b/tests/cilium_http_integration.cc index ecf962542..5d4263502 100644 --- a/tests/cilium_http_integration.cc +++ b/tests/cilium_http_integration.cc @@ -7,6 +7,7 @@ #include #include +#include "envoy/http/codec.h" #include "envoy/network/address.h" #include "source/common/common/base_logger.h" diff --git a/tests/cilium_tls_http_integration_test.cc b/tests/cilium_tls_http_integration_test.cc index 39c530499..f1f62cb89 100644 --- a/tests/cilium_tls_http_integration_test.cc +++ b/tests/cilium_tls_http_integration_test.cc @@ -9,6 +9,7 @@ #include "envoy/common/exception.h" #include "envoy/extensions/transport_sockets/tls/v3/tls.pb.h" +#include "envoy/http/codec.h" #include "envoy/network/address.h" #include "envoy/network/connection.h" #include "envoy/network/transport_socket.h" diff --git a/tests/cilium_tls_tcp_integration_test.cc b/tests/cilium_tls_tcp_integration_test.cc index 0a0c79fb1..4424a1acc 100644 --- a/tests/cilium_tls_tcp_integration_test.cc +++ b/tests/cilium_tls_tcp_integration_test.cc @@ -18,6 +18,7 @@ #include "envoy/common/exception.h" #include "envoy/event/dispatcher.h" #include "envoy/extensions/transport_sockets/tls/v3/tls.pb.h" +#include "envoy/http/codec.h" #include "envoy/network/address.h" #include "envoy/network/connection.h" #include "envoy/network/transport_socket.h" diff --git a/tests/health_check_sink_server.cc b/tests/health_check_sink_server.cc index d9b2070c0..075ec8685 100644 --- a/tests/health_check_sink_server.cc +++ b/tests/health_check_sink_server.cc @@ -1,10 +1,5 @@ #include "tests/health_check_sink_server.h" -#include -#include -#include -#include - #include #include #include diff --git a/tests/uds_server.cc b/tests/uds_server.cc index aef8a4253..cbff6cd3d 100644 --- a/tests/uds_server.cc +++ b/tests/uds_server.cc @@ -1,10 +1,10 @@ #include "tests/uds_server.h" -#include #include #include #include +#include #include #include #include From ece6295ebcbe074bd54881fdaa40cac5947037a3 Mon Sep 17 00:00:00 2001 From: Jarno Rajahalme Date: Sat, 1 Feb 2025 15:56:45 +0100 Subject: [PATCH 09/12] treewide: Replace typedefs with "using" Replace typedefs with "using". Doing this as a separate step as clang-tidy fix will make errors doing this. Signed-off-by: Jarno Rajahalme --- cilium/accesslog.h | 2 +- cilium/bpf_metadata.h | 2 +- cilium/conntrack.cc | 12 ++++++------ cilium/conntrack.h | 2 +- cilium/host_map.h | 2 +- cilium/ipcache.cc | 10 +++++----- cilium/ipcache.h | 2 +- cilium/l7policy.h | 2 +- cilium/network_filter.h | 2 +- cilium/network_policy.h | 6 +++--- cilium/proxylib.h | 26 +++++++++++++------------- cilium/secret_watcher.h | 2 +- cilium/websocket_codec.h | 2 +- cilium/websocket_config.h | 2 +- 14 files changed, 37 insertions(+), 37 deletions(-) diff --git a/cilium/accesslog.h b/cilium/accesslog.h index 063f0151e..1cde10886 100644 --- a/cilium/accesslog.h +++ b/cilium/accesslog.h @@ -68,7 +68,7 @@ class AccessLog : public UDSClient { const std::string path_; }; -typedef std::shared_ptr AccessLogSharedPtr; +using AccessLogSharedPtr = std::shared_ptr; } // namespace Cilium } // namespace Envoy diff --git a/cilium/bpf_metadata.h b/cilium/bpf_metadata.h index f956912b6..e2a02553b 100644 --- a/cilium/bpf_metadata.h +++ b/cilium/bpf_metadata.h @@ -164,7 +164,7 @@ class Config : public Cilium::PolicyResolver, const IPAddressPair& sourceAddresses); }; -typedef std::shared_ptr ConfigSharedPtr; +using ConfigSharedPtr = std::shared_ptr; /** * Implementation of a bpf metadata listener filter. diff --git a/cilium/conntrack.cc b/cilium/conntrack.cc index 61648b0ac..d7a99e3a5 100644 --- a/cilium/conntrack.cc +++ b/cilium/conntrack.cc @@ -30,12 +30,12 @@ namespace Cilium { // them to a separate include file we can include here instead of // copying them! -typedef uint64_t __u64; -typedef uint32_t __be32; // Beware of the byte order! -typedef uint32_t __u32; -typedef uint16_t __be16; // Beware of the byte order! -typedef uint16_t __u16; -typedef uint8_t __u8; +using __u64 = uint64_t; +using __be32 = uint32_t; // Beware of the byte order! +using __u32 = uint32_t; +using __be16 = uint16_t; // Beware of the byte order! +using __u16 = uint16_t; +using __u8 = uint8_t; #define TUPLE_F_OUT 0 #define TUPLE_F_IN 1 diff --git a/cilium/conntrack.h b/cilium/conntrack.h index b258b7862..5d13afc8c 100644 --- a/cilium/conntrack.h +++ b/cilium/conntrack.h @@ -79,7 +79,7 @@ class CtMap : public Singleton::Instance, Logger::Loggable { std::string bpf_root_; }; -typedef std::shared_ptr CtMapSharedPtr; +using CtMapSharedPtr = std::shared_ptr; } // namespace Cilium } // namespace Envoy diff --git a/cilium/host_map.h b/cilium/host_map.h index 78b2f6163..50ad07d74 100644 --- a/cilium/host_map.h +++ b/cilium/host_map.h @@ -187,7 +187,7 @@ class PolicyHostMap : public Singleton::Instance, std::vector>> ipv6_to_policy_; }; - typedef std::shared_ptr ThreadLocalHostMapSharedPtr; + using ThreadLocalHostMapSharedPtr = std::shared_ptr; const ThreadLocalHostMap* getHostMap() const { return tls_->get().get() ? &tls_->getTyped() : nullptr; diff --git a/cilium/ipcache.cc b/cilium/ipcache.cc index 18c9ed2a4..1ded93147 100644 --- a/cilium/ipcache.cc +++ b/cilium/ipcache.cc @@ -28,11 +28,11 @@ namespace Cilium { // them to a separate include file we can include here instead of // copying them! -typedef uint32_t __be32; // Beware of the byte order! -typedef uint64_t __u64; -typedef uint32_t __u32; -typedef uint16_t __u16; -typedef uint8_t __u8; +using __be32 = uint32_t; // Beware of the byte order! +using __u64 = uint64_t; +using __u32 = uint32_t; +using __u16 = uint16_t; +using __u8 = uint8_t; PACKED_STRUCT(struct ipcache_key { struct bpf_lpm_trie_key lpm_key; diff --git a/cilium/ipcache.h b/cilium/ipcache.h index 76c5bf95a..5b76ca67e 100644 --- a/cilium/ipcache.h +++ b/cilium/ipcache.h @@ -30,7 +30,7 @@ class IPCache : public Singleton::Instance, public Bpf { std::string bpf_root_; }; -typedef std::shared_ptr IPCacheSharedPtr; +using IPCacheSharedPtr = std::shared_ptr; } // namespace Cilium } // namespace Envoy diff --git a/cilium/l7policy.h b/cilium/l7policy.h index f00c89bb8..fb552f658 100644 --- a/cilium/l7policy.h +++ b/cilium/l7policy.h @@ -60,7 +60,7 @@ class Config : public Logger::Loggable { Cilium::AccessLogSharedPtr access_log_; }; -typedef std::shared_ptr ConfigSharedPtr; +using ConfigSharedPtr = std::shared_ptr; // Each request gets their own instance of this filter, and // they can run parallel from multiple worker threads, all accessing diff --git a/cilium/network_filter.h b/cilium/network_filter.h index 8f7653e1f..6ec85e593 100644 --- a/cilium/network_filter.h +++ b/cilium/network_filter.h @@ -42,7 +42,7 @@ class Config : Logger::Loggable { Cilium::AccessLogSharedPtr access_log_; }; -typedef std::shared_ptr ConfigSharedPtr; +using ConfigSharedPtr = std::shared_ptr; /** * Implementation of a Cilium network filter. diff --git a/cilium/network_policy.h b/cilium/network_policy.h index ac17e74e9..43c0b260f 100644 --- a/cilium/network_policy.h +++ b/cilium/network_policy.h @@ -59,7 +59,7 @@ namespace Cilium { // other given this comparison predicate). // On lookups we'll set both ends of the port range to the same port number, which will find the one // range that it overlaps with, if one exists. -typedef std::pair PortRange; +using PortRange = std::pair; struct PortRangeCompare { bool operator()(const PortRange& a, const PortRange& b) const { // return true if range 'a.first - a.second' is below range 'b.first - b.second'. @@ -68,12 +68,12 @@ struct PortRangeCompare { }; class PortNetworkPolicyRules; -typedef std::list RulesList; +using RulesList = std::list; // PolicyMap is keyed by port ranges, and contains a list of PortNetworkPolicyRules's applicable // to this range. A list is needed as rules may come from multiple sources (e.g., resulting from // use of named ports and numbered ports in Cilium Network Policy at the same time). -typedef absl::btree_map PolicyMap; +using PolicyMap = absl::btree_map; // PortPolicy holds a reference to a set of rules in a policy map that apply to the given port. // Methods then iterate through the set to determine if policy allows or denies. This is needed to diff --git a/cilium/proxylib.h b/cilium/proxylib.h index f560c20c5..73a6c752b 100644 --- a/cilium/proxylib.h +++ b/cilium/proxylib.h @@ -108,19 +108,19 @@ struct GoStringPair { GoString value; }; -typedef GoSlice GoKeyValueSlice; -typedef uint64_t (*GoOpenModuleCB)(GoKeyValueSlice, bool); -typedef void (*GoCloseModuleCB)(uint64_t); +using GoKeyValueSlice = GoSlice; +using GoOpenModuleCB = uint64_t (*)(GoKeyValueSlice, bool); +using GoCloseModuleCB = void (*)(uint64_t); -typedef ResetableSlice GoBufferSlice; -typedef FilterResult (*GoOnNewConnectionCB)(uint64_t, GoString, uint64_t, bool, uint32_t, uint32_t, - GoString, GoString, GoString, GoBufferSlice*, - GoBufferSlice*); +using GoBufferSlice = ResetableSlice; +using GoOnNewConnectionCB = FilterResult (*)(uint64_t, GoString, uint64_t, bool, uint32_t, uint32_t, + GoString, GoString, GoString, GoBufferSlice*, + GoBufferSlice*); -typedef GoSlice> GoDataSlices; // Scatter-gather buffer list as '[][]byte' -typedef ResetableSlice GoFilterOpSlice; -typedef FilterResult (*GoOnDataCB)(uint64_t, bool, bool, GoDataSlices*, GoFilterOpSlice*); -typedef void (*GoCloseCB)(uint64_t); +using GoDataSlices = GoSlice>; // Scatter-gather buffer list as '[][]byte' +using GoFilterOpSlice = ResetableSlice; +using GoOnDataCB = FilterResult (*)(uint64_t, bool, bool, GoDataSlices*, GoFilterOpSlice*); +using GoCloseCB = void (*)(uint64_t); class GoFilter : public Logger::Loggable { public: @@ -166,7 +166,7 @@ class GoFilter : public Logger::Loggable { Direction reply_; uint64_t connection_id_ = 0; }; - typedef std::unique_ptr InstancePtr; + using InstancePtr = std::unique_ptr; InstancePtr NewInstance(Network::Connection& conn, const std::string& go_proto, bool ingress, uint32_t src_id, uint32_t dst_id, const std::string& src_addr, @@ -181,7 +181,7 @@ class GoFilter : public Logger::Loggable { uint64_t go_module_id_{0}; }; -typedef std::shared_ptr GoFilterSharedPtr; +using GoFilterSharedPtr = std::shared_ptr; } // namespace Cilium } // namespace Envoy diff --git a/cilium/secret_watcher.h b/cilium/secret_watcher.h index 02a07b9f1..944932e27 100644 --- a/cilium/secret_watcher.h +++ b/cilium/secret_watcher.h @@ -26,7 +26,7 @@ namespace Envoy { namespace Cilium { // Facility for SDS config override for testing -typedef envoy::config::core::v3::ConfigSource (*getSDSConfigFunc)(const std::string& name); +using getSDSConfigFunc = envoy::config::core::v3::ConfigSource (*)(const std::string&); extern getSDSConfigFunc getSDSConfig; void setSDSConfigFunc(getSDSConfigFunc); void resetSDSConfigFunc(); diff --git a/cilium/websocket_codec.h b/cilium/websocket_codec.h index 619977e66..67d3f3bae 100644 --- a/cilium/websocket_codec.h +++ b/cilium/websocket_codec.h @@ -127,7 +127,7 @@ class Codec : Logger::Loggable { Buffer::OwnedImpl handshake_buffer_{}; bool accepted_{false}; }; -typedef std::unique_ptr CodecPtr; +using CodecPtr = std::unique_ptr; } // namespace WebSocket } // namespace Cilium diff --git a/cilium/websocket_config.h b/cilium/websocket_config.h index 79f50e5ec..fd27a755f 100644 --- a/cilium/websocket_config.h +++ b/cilium/websocket_config.h @@ -92,7 +92,7 @@ class Config : public Logger::Loggable { Cilium::AccessLogSharedPtr access_log_; }; -typedef std::shared_ptr ConfigSharedPtr; +using ConfigSharedPtr = std::shared_ptr; } // namespace WebSocket } // namespace Cilium From 675fa43806916638356dc1328c8190d270fb01df Mon Sep 17 00:00:00 2001 From: Jarno Rajahalme Date: Sun, 2 Feb 2025 17:20:15 +0100 Subject: [PATCH 10/12] treewide: Apply tidy fixes Signed-off-by: Jarno Rajahalme --- cilium/accesslog.cc | 39 +- cilium/accesslog.h | 18 +- cilium/bpf.cc | 15 +- cilium/bpf_metadata.cc | 35 +- cilium/bpf_metadata.h | 9 +- cilium/conntrack.cc | 8 +- cilium/filter_state_cilium_policy.h | 8 +- cilium/grpc_subscription.cc | 2 +- cilium/grpc_subscription.h | 2 +- cilium/health_check_sink.cc | 5 +- cilium/host_map.h | 2 +- cilium/ipcache.cc | 16 +- cilium/ipcache.h | 6 +- cilium/l7policy.cc | 22 +- cilium/l7policy.h | 2 +- cilium/network_filter.cc | 34 +- cilium/network_filter.h | 2 +- cilium/network_policy.cc | 147 ++--- cilium/network_policy.h | 20 +- cilium/privileged_service_client.cc | 16 +- cilium/privileged_service_client.h | 10 +- cilium/proxylib.cc | 8 +- cilium/proxylib.h | 18 +- cilium/secret_watcher.cc | 36 +- cilium/secret_watcher.h | 6 +- cilium/tls_wrapper.cc | 4 +- cilium/uds_client.cc | 7 +- cilium/uds_client.h | 4 +- cilium/websocket.cc | 4 +- cilium/websocket.h | 16 +- cilium/websocket_codec.cc | 6 +- cilium/websocket_config.cc | 10 +- cilium/websocket_config.h | 2 +- tests/accesslog_server.cc | 2 +- tests/accesslog_server.h | 9 +- tests/accesslog_test.cc | 4 +- tests/bpf_metadata.cc | 8 +- tests/bpf_metadata.h | 2 +- tests/cilium_http_integration.cc | 2 +- tests/cilium_http_integration.h | 8 +- tests/cilium_http_integration_test.cc | 128 ++-- .../cilium_http_upstream_integration_test.cc | 78 +-- tests/cilium_network_policy_test.cc | 567 +++++++++--------- tests/cilium_tls_http_integration_test.cc | 26 +- tests/cilium_tls_integration.cc | 2 + tests/cilium_tls_tcp_integration_test.cc | 2 + ...cilium_websocket_decap_integration_test.cc | 6 +- ...cilium_websocket_encap_integration_test.cc | 20 +- tests/health_check_sink_server.cc | 2 +- tests/health_check_sink_server.h | 3 +- tests/uds_server.cc | 11 +- tests/uds_server.h | 2 +- 52 files changed, 740 insertions(+), 681 deletions(-) diff --git a/cilium/accesslog.cc b/cilium/accesslog.cc index 6c6609fcb..47a722af0 100644 --- a/cilium/accesslog.cc +++ b/cilium/accesslog.cc @@ -28,13 +28,14 @@ namespace Cilium { Thread::MutexBasicLockable AccessLog::logs_mutex; std::map> AccessLog::logs; -AccessLogSharedPtr AccessLog::Open(const std::string& path, TimeSource& time_source) { +AccessLogSharedPtr AccessLog::open(const std::string& path, TimeSource& time_source) { Thread::LockGuard guard(logs_mutex); auto it = logs.find(path); if (it != logs.end()) { auto log = it->second.lock(); - if (log) + if (log) { return log; + } // expired, remove logs.erase(path); } @@ -51,7 +52,7 @@ AccessLog::~AccessLog() { logs.erase(path_); } -void AccessLog::Log(AccessLog::Entry& log_entry, ::cilium::EntryType entry_type) { +void AccessLog::log(AccessLog::Entry& log_entry, ::cilium::EntryType entry_type) { ::cilium::LogEntry& entry = log_entry.entry_; entry.set_entry_type(entry_type); @@ -66,7 +67,7 @@ void AccessLog::Log(AccessLog::Entry& log_entry, ::cilium::EntryType entry_type) std::string msg; entry.SerializeToString(&msg); - UDSClient::Log(msg); + UDSClient::log(msg); } #define CONST_STRING_VIEW(NAME, STR) const absl::string_view NAME = {STR, sizeof(STR) - 1} @@ -78,7 +79,7 @@ CONST_STRING_VIEW(xForwardedProtoSV, "x-forwarded-proto"); CONST_STRING_VIEW(xRequestIdSV, "x-request-id"); CONST_STRING_VIEW(statusSV, ":status"); -void AccessLog::Entry::InitFromConnection( +void AccessLog::Entry::initFromConnection( const std::string& policy_name, uint32_t proxy_id, bool ingress, uint32_t source_identity, const Network::Address::InstanceConstSharedPtr& source_address, uint32_t destination_identity, const Network::Address::InstanceConstSharedPtr& destination_address, TimeSource* time_source) { @@ -103,7 +104,7 @@ void AccessLog::Entry::InitFromConnection( } } -bool AccessLog::Entry::UpdateFromMetadata(const std::string& l7proto, +bool AccessLog::Entry::updateFromMetadata(const std::string& l7proto, const ProtobufWkt::Struct& metadata) { bool changed = false; @@ -140,14 +141,14 @@ bool AccessLog::Entry::UpdateFromMetadata(const std::string& l7proto, return changed; } -void AccessLog::Entry::InitFromRequest(const std::string& policy_name, uint32_t proxy_id, +void AccessLog::Entry::initFromRequest(const std::string& policy_name, uint32_t proxy_id, bool ingress, uint32_t source_identity, const Network::Address::InstanceConstSharedPtr& src_address, uint32_t destination_identity, const Network::Address::InstanceConstSharedPtr& dst_address, const StreamInfo::StreamInfo& info, const Http::RequestHeaderMap& headers) { - InitFromConnection(policy_name, proxy_id, ingress, source_identity, src_address, + initFromConnection(policy_name, proxy_id, ingress, source_identity, src_address, destination_identity, dst_address, nullptr); auto time = info.startTime(); @@ -170,10 +171,10 @@ void AccessLog::Entry::InitFromRequest(const std::string& policy_name, uint32_t ::cilium::HttpLogEntry* http_entry = entry_.mutable_http(); http_entry->set_http_protocol(proto); - UpdateFromRequest(destination_identity, dst_address, headers); + updateFromRequest(destination_identity, dst_address, headers); } -void AccessLog::Entry::UpdateFromRequest( +void AccessLog::Entry::updateFromRequest( uint32_t destination_identity, const Network::Address::InstanceConstSharedPtr& dst_address, const Http::RequestHeaderMap& headers) { // Destination may have changed @@ -214,7 +215,7 @@ void AccessLog::Entry::UpdateFromRequest( }); } -void AccessLog::Entry::UpdateFromResponse(const Http::ResponseHeaderMap& headers, +void AccessLog::Entry::updateFromResponse(const Http::ResponseHeaderMap& headers, TimeSource& time_source) { auto time = time_source.systemTime(); entry_.set_timestamp( @@ -263,19 +264,23 @@ void AccessLog::Entry::UpdateFromResponse(const Http::ResponseHeaderMap& headers }); } -void AccessLog::Entry::AddRejected(absl::string_view key, absl::string_view value) { - for (auto entry : entry_.http().rejected_headers()) - if (entry.key() == key && entry.value() == value) +void AccessLog::Entry::addRejected(absl::string_view key, absl::string_view value) { + for (const auto& entry : entry_.http().rejected_headers()) { + if (entry.key() == key && entry.value() == value) { return; + } + } ::cilium::KeyValue* kv = entry_.mutable_http()->add_rejected_headers(); kv->set_key(key.data(), key.size()); kv->set_value(value.data(), value.size()); } -void AccessLog::Entry::AddMissing(absl::string_view key, absl::string_view value) { - for (auto entry : entry_.http().missing_headers()) - if (entry.key() == key && entry.value() == value) +void AccessLog::Entry::addMissing(absl::string_view key, absl::string_view value) { + for (const auto& entry : entry_.http().missing_headers()) { + if (entry.key() == key && entry.value() == value) { return; + } + } ::cilium::KeyValue* kv = entry_.mutable_http()->add_missing_headers(); kv->set_key(key.data(), key.size()); kv->set_value(value.data(), value.size()); diff --git a/cilium/accesslog.h b/cilium/accesslog.h index 1cde10886..2dd4af8fd 100644 --- a/cilium/accesslog.h +++ b/cilium/accesslog.h @@ -26,38 +26,38 @@ constexpr absl::string_view AccessLogKey = "cilium.accesslog.entry"; class AccessLog : public UDSClient { public: - static std::shared_ptr Open(const std::string& path, TimeSource& time_source); + static std::shared_ptr open(const std::string& path, TimeSource& time_source); ~AccessLog(); // wrapper for protobuf class Entry : public StreamInfo::FilterState::Object { public: - void InitFromRequest(const std::string& policy_name, uint32_t proxy_id, bool ingress, + void initFromRequest(const std::string& policy_name, uint32_t proxy_id, bool ingress, uint32_t source_identity, const Network::Address::InstanceConstSharedPtr& source_address, uint32_t destination_identity, const Network::Address::InstanceConstSharedPtr& destination_address, const StreamInfo::StreamInfo&, const Http::RequestHeaderMap&); - void UpdateFromRequest(uint32_t destination_identity, + void updateFromRequest(uint32_t destination_identity, const Network::Address::InstanceConstSharedPtr& destination_address, const Http::RequestHeaderMap&); - void UpdateFromResponse(const Http::ResponseHeaderMap&, TimeSource&); + void updateFromResponse(const Http::ResponseHeaderMap&, TimeSource&); - void InitFromConnection(const std::string& policy_name, uint32_t proxy_id, bool ingress, + void initFromConnection(const std::string& policy_name, uint32_t proxy_id, bool ingress, uint32_t source_identity, const Network::Address::InstanceConstSharedPtr& source_address, uint32_t destination_identity, const Network::Address::InstanceConstSharedPtr& destination_address, TimeSource* time_source); - bool UpdateFromMetadata(const std::string& l7proto, const ProtobufWkt::Struct& metadata); - void AddRejected(absl::string_view key, absl::string_view value); - void AddMissing(absl::string_view key, absl::string_view value); + bool updateFromMetadata(const std::string& l7proto, const ProtobufWkt::Struct& metadata); + void addRejected(absl::string_view key, absl::string_view value); + void addMissing(absl::string_view key, absl::string_view value); ::cilium::LogEntry entry_{}; bool request_logged_ = false; }; - void Log(Entry& entry, ::cilium::EntryType); + void log(Entry& entry, ::cilium::EntryType); private: explicit AccessLog(const std::string& path, TimeSource& time_source) diff --git a/cilium/bpf.cc b/cilium/bpf.cc index 9e2b1c67b..2ecaea927 100644 --- a/cilium/bpf.cc +++ b/cilium/bpf.cc @@ -18,7 +18,7 @@ namespace Envoy { namespace Cilium { enum { - BPF_KEY_MAX_LEN = 64, + BpfKeyMaxLen = 64, }; Bpf::Bpf(uint32_t map_type, uint32_t key_size, uint32_t value_size) @@ -27,8 +27,9 @@ Bpf::Bpf(uint32_t map_type, uint32_t key_size, uint32_t value_size) Bpf::~Bpf() { close(); } void Bpf::close() { - if (fd_ >= 0) + if (fd_ >= 0) { ::close(fd_); + } fd_ = -1; } @@ -39,11 +40,12 @@ bool Bpf::open(const std::string& path) { close(); // store the path for later - if (path != path_) + if (path != path_) { path_ = path; + } auto& cilium_calls = PrivilegedService::Singleton::get(); - auto ret = cilium_calls.bpf_open(path.c_str()); + auto ret = cilium_calls.bpfOpen(path.c_str()); fd_ = ret.return_value_; if (fd_ >= 0) { // Open fdinfo to check the map type and key and value size. @@ -116,12 +118,13 @@ bool Bpf::open(const std::string& path) { bool Bpf::lookup(const void* key, void* value) { // Try reopen if open failed previously if (fd_ < 0) { - if (!open(path_)) + if (!open(path_)) { return false; + } } auto& cilium_calls = PrivilegedService::Singleton::get(); - auto result = cilium_calls.bpf_lookup(fd_, key, key_size_, value, value_size_); + auto result = cilium_calls.bpfLookup(fd_, key, key_size_, value, value_size_); if (result.return_value_ == 0) { return true; diff --git a/cilium/bpf_metadata.cc b/cilium/bpf_metadata.cc index 2d5359630..93e2294d8 100644 --- a/cilium/bpf_metadata.cc +++ b/cilium/bpf_metadata.cc @@ -236,7 +236,7 @@ Config::Config(const ::cilium::BpfMetadata& config, // later. return std::make_shared(bpf_root); }); - ipcache_ = IPCache::NewIPCache(context.serverFactoryContext(), bpf_root); + ipcache_ = IPCache::newIpCache(context.serverFactoryContext(), bpf_root); if (bpf_root != ct_maps_->bpfRoot()) { // bpf root may not change during runtime throw EnvoyException(fmt::format("cilium.bpf_metadata: Invalid bpf_root: {}", bpf_root)); @@ -273,7 +273,8 @@ uint32_t Config::resolvePolicyId(const Network::Address::Ip* ip) const { uint32_t Config::resolveSourceIdentity(const PolicyInstance& policy, const Network::Address::Ip* sip, - const Network::Address::Ip* dip, bool ingress, bool isL7LB) { + const Network::Address::Ip* dip, bool ingress, + bool is_l7_lb) { uint32_t source_identity = 0; // Resolve the source security ID from conntrack map, or from ip cache @@ -281,7 +282,7 @@ uint32_t Config::resolveSourceIdentity(const PolicyInstance& policy, const std::string& ct_name = policy.conntrackName(); if (ct_name.length() > 0) { source_identity = ct_maps_->lookupSrcIdentity(ct_name, sip, dip, ingress); - } else if (isL7LB) { + } else if (is_l7_lb) { // non-local source should be in the global conntrack source_identity = ct_maps_->lookupSrcIdentity("global", sip, dip, ingress); } @@ -297,24 +298,24 @@ uint32_t Config::resolveSourceIdentity(const PolicyInstance& policy, // Returns a new IPAddressPair that keeps the source address and fills in the other address version // from the given IPAddressPair. IPAddressPair -Config::getIPAddressPairFrom(const Network::Address::InstanceConstSharedPtr sourceAddress, +Config::getIPAddressPairFrom(const Network::Address::InstanceConstSharedPtr source_address, const IPAddressPair& addresses) { auto addressPair = IPAddressPair(); - switch (sourceAddress->ip()->version()) { + switch (source_address->ip()->version()) { case Network::Address::IpVersion::v4: - addressPair.ipv4_ = sourceAddress; + addressPair.ipv4_ = source_address; if (addresses.ipv6_) { sockaddr_in6 sa6 = *reinterpret_cast(addresses.ipv6_->sockAddr()); - sa6.sin6_port = htons(sourceAddress->ip()->port()); + sa6.sin6_port = htons(source_address->ip()->port()); addressPair.ipv6_ = std::make_shared(sa6); } break; case Network::Address::IpVersion::v6: - addressPair.ipv6_ = sourceAddress; + addressPair.ipv6_ = source_address; if (addresses.ipv4_) { sockaddr_in sa4 = *reinterpret_cast(addresses.ipv4_->sockAddr()); - sa4.sin_port = htons(sourceAddress->ip()->port()); + sa4.sin_port = htons(source_address->ip()->port()); addressPair.ipv4_ = std::make_shared(&sa4); } break; @@ -347,11 +348,12 @@ const PolicyInstance& Config::getPolicy(const std::string& pod_ip) const { // This is the case for L7 LB listeners only. This is needed to allow traffic forwarded by Cilium // Ingress (which is implemented as an egress listener!). bool allow_egress = !enforce_policy_on_l7lb_ && !is_ingress_ && is_l7lb_; - if (npmap_ == nullptr) - return allow_egress ? NetworkPolicyMap::GetAllowAllEgressPolicy() - : NetworkPolicyMap::GetDenyAllPolicy(); + if (npmap_ == nullptr) { + return allow_egress ? NetworkPolicyMap::getAllowAllEgressPolicy() + : NetworkPolicyMap::getDenyAllPolicy(); + } - return npmap_->GetPolicyInstance(pod_ip, allow_egress); + return npmap_->getPolicyInstance(pod_ip, allow_egress); } absl::optional @@ -457,8 +459,9 @@ Config::extractSocketMetadata(Network::ConnectionSocket& socket) { } // Enforce ingress policy on the incoming Ingress traffic? - if (enforce_policy_on_l7lb_) + if (enforce_policy_on_l7lb_) { ingress_source_identity = source_identity; + } source_identity = new_source_identity; @@ -507,11 +510,11 @@ Config::extractSocketMetadata(Network::ConnectionSocket& socket) { uint32_t identity_id = (source_identity & 0xFFFF) << 16; mark = ((is_ingress_) ? 0x0A00 : 0x0B00) | cluster_id | identity_id; } - return absl::optional(Cilium::BpfMetadata::SocketMetadata( + return {Cilium::BpfMetadata::SocketMetadata( mark, ingress_source_identity, source_identity, is_ingress_, is_l7lb_, dip->port(), std::move(pod_ip), std::move(src_address), std::move(source_addresses.ipv4_), std::move(source_addresses.ipv6_), std::move(dst_address), weak_from_this(), proxy_id_, - std::move(proxylib_l7proto), sni)); + std::move(proxylib_l7proto), sni)}; } Network::FilterStatus Instance::onAccept(Network::ListenerFilterCallbacks& cb) { diff --git a/cilium/bpf_metadata.h b/cilium/bpf_metadata.h index e2a02553b..277359af6 100644 --- a/cilium/bpf_metadata.h +++ b/cilium/bpf_metadata.h @@ -68,6 +68,7 @@ struct SocketMetadata : public Logger::Loggable { if (!proxylib_l7_proto_.empty()) { const auto& old_protocols = socket.requestedApplicationProtocols(); std::vector protocols; + protocols.reserve(old_protocols.size()); for (const auto& old_protocol : old_protocols) { protocols.emplace_back(old_protocol); } @@ -128,7 +129,7 @@ class Config : public Cilium::PolicyResolver, public: Config(const ::cilium::BpfMetadata& config, Server::Configuration::ListenerFactoryContext& context); - virtual ~Config() {} + ~Config() override = default; // PolicyResolver uint32_t resolvePolicyId(const Network::Address::Ip*) const override; @@ -155,13 +156,13 @@ class Config : public Cilium::PolicyResolver, private: uint32_t resolveSourceIdentity(const PolicyInstance& policy, const Network::Address::Ip* sip, - const Network::Address::Ip* dip, bool ingress, bool isL7LB); + const Network::Address::Ip* dip, bool ingress, bool is_l7_lb); - IPAddressPair getIPAddressPairFrom(const Network::Address::InstanceConstSharedPtr sourceAddress, + IPAddressPair getIPAddressPairFrom(const Network::Address::InstanceConstSharedPtr source_address, const IPAddressPair& addresses); const Network::Address::Ip* selectIPVersion(const Network::Address::IpVersion version, - const IPAddressPair& sourceAddresses); + const IPAddressPair& source_addresses); }; using ConfigSharedPtr = std::shared_ptr; diff --git a/cilium/conntrack.cc b/cilium/conntrack.cc index d7a99e3a5..14c8e1b55 100644 --- a/cilium/conntrack.cc +++ b/cilium/conntrack.cc @@ -58,7 +58,7 @@ PACKED_STRUCT(struct ipv4_ct_tuple { __u8 flags; }); -struct ct_entry { +struct CtEntry { __u64 rx_packets; __u64 rx_bytes; __u64 tx_packets; @@ -82,10 +82,10 @@ struct ct_entry { }; CtMap::CtMap4::CtMap4() - : Bpf(BPF_MAP_TYPE_HASH, sizeof(struct ipv4_ct_tuple), sizeof(struct ct_entry)) {} + : Bpf(BPF_MAP_TYPE_HASH, sizeof(struct ipv4_ct_tuple), sizeof(struct CtEntry)) {} CtMap::CtMap6::CtMap6() - : Bpf(BPF_MAP_TYPE_HASH, sizeof(struct ipv6_ct_tuple), sizeof(struct ct_entry)) {} + : Bpf(BPF_MAP_TYPE_HASH, sizeof(struct ipv6_ct_tuple), sizeof(struct CtEntry)) {} CtMap::CtMaps4::CtMaps4(const std::string& bpf_root, const std::string& map_name) : ok_(false) { // Open the IPv4 bpf maps from Cilium specific paths @@ -190,7 +190,7 @@ uint32_t CtMap::lookupSrcIdentity(const std::string& map_name, const Network::Ad struct ipv4_ct_tuple key4 {}; struct ipv6_ct_tuple key6 {}; - struct ct_entry value {}; + struct CtEntry value {}; if (sip->version() == Network::Address::IpVersion::v4 && dip->version() == Network::Address::IpVersion::v4) { diff --git a/cilium/filter_state_cilium_policy.h b/cilium/filter_state_cilium_policy.h index 83cc476ee..f876afc0f 100644 --- a/cilium/filter_state_cilium_policy.h +++ b/cilium/filter_state_cilium_policy.h @@ -46,16 +46,18 @@ class CiliumPolicyFilterState : public StreamInfo::FilterState::Object, uint32_t resolvePolicyId(const Network::Address::Ip* ip) const { const auto resolver = policy_resolver_.lock(); - if (resolver) + if (resolver) { return resolver->resolvePolicyId(ip); + } return Cilium::ID::WORLD; // default to WORLD policy ID if resolver is no longer available } const PolicyInstance& getPolicy() const { const auto resolver = policy_resolver_.lock(); - if (resolver) + if (resolver) { return resolver->getPolicy(pod_ip_); - return NetworkPolicyMap::GetDenyAllPolicy(); + } + return NetworkPolicyMap::getDenyAllPolicy(); } // policyUseUpstreamDestinationAddress returns 'true' if policy enforcement should be done on the diff --git a/cilium/grpc_subscription.cc b/cilium/grpc_subscription.cc index 51c5e0fe0..7f384a793 100644 --- a/cilium/grpc_subscription.cc +++ b/cilium/grpc_subscription.cc @@ -95,7 +95,7 @@ TypeUrlToServiceMap& typeUrlToServiceMap() { class NopConfigValidatorsImpl : public Envoy::Config::CustomConfigValidators { public: - NopConfigValidatorsImpl() {} + NopConfigValidatorsImpl() = default; void executeValidators(absl::string_view, const std::vector&) override {} diff --git a/cilium/grpc_subscription.h b/cilium/grpc_subscription.h index 125ec39e9..496ee0568 100644 --- a/cilium/grpc_subscription.h +++ b/cilium/grpc_subscription.h @@ -28,7 +28,7 @@ class GrpcMuxImpl : public Config::GrpcMuxImpl { GrpcMuxImpl(Config::GrpcMuxContext& grpc_mux_context, bool skip_subsequent_node) : Config::GrpcMuxImpl(grpc_mux_context, skip_subsequent_node) {} - ~GrpcMuxImpl() override {} + ~GrpcMuxImpl() override = default; void onStreamEstablished() override { new_stream_ = true; diff --git a/cilium/health_check_sink.cc b/cilium/health_check_sink.cc index f7a0df53f..f9268c9d3 100644 --- a/cilium/health_check_sink.cc +++ b/cilium/health_check_sink.cc @@ -33,9 +33,10 @@ HealthCheckEventPipeSink::HealthCheckEventPipeSink(const cilium::HealthCheckEven auto it = udss.find(path); if (it != udss.end()) { uds_client_ = it->second.lock(); - if (!uds_client_) + if (!uds_client_) { // expired, remove udss.erase(path); + } } if (!uds_client_) { // Not found, allocate and store as a weak_ptr @@ -52,7 +53,7 @@ void HealthCheckEventPipeSink::log(envoy::data::core::v3::HealthCheckEvent event } std::string msg; event.SerializeToString(&msg); - uds_client_->Log(msg); + uds_client_->log(msg); }; Upstream::HealthCheckEventSinkPtr HealthCheckEventPipeSinkFactory::createHealthCheckEventSink( diff --git a/cilium/host_map.h b/cilium/host_map.h index 50ad07d74..dda9943e0 100644 --- a/cilium/host_map.h +++ b/cilium/host_map.h @@ -97,7 +97,7 @@ class PolicyHostMap : public Singleton::Instance, public: PolicyHostMap(Server::Configuration::CommonFactoryContext& context); PolicyHostMap(ThreadLocal::SlotAllocator& tls); - ~PolicyHostMap() { + ~PolicyHostMap() override { ENVOY_LOG(debug, "Cilium PolicyHostMap({}): PolicyHostMap is deleted NOW!", name_); } diff --git a/cilium/ipcache.cc b/cilium/ipcache.cc index 1ded93147..12355ff1a 100644 --- a/cilium/ipcache.cc +++ b/cilium/ipcache.cc @@ -50,7 +50,7 @@ PACKED_STRUCT(struct ipcache_key { }; }); -struct remote_endpoint_info { +struct RemoteEndpointInfo { __u32 sec_label; __u32 tunnel_endpoint; __u16 pad; @@ -62,31 +62,31 @@ struct remote_endpoint_info { SINGLETON_MANAGER_REGISTRATION(cilium_ipcache); -IPCacheSharedPtr IPCache::NewIPCache(Server::Configuration::ServerFactoryContext& context, +IPCacheSharedPtr IPCache::newIpCache(Server::Configuration::ServerFactoryContext& context, const std::string& bpf_root) { return context.singletonManager().getTyped( SINGLETON_MANAGER_REGISTERED_NAME(cilium_ipcache), [&bpf_root] { auto ipcache = std::make_shared(bpf_root); - if (!ipcache->Open()) { + if (!ipcache->open()) { ipcache.reset(); } return ipcache; }); } -IPCacheSharedPtr IPCache::GetIPCache(Server::Configuration::ServerFactoryContext& context) { +IPCacheSharedPtr IPCache::getIpCache(Server::Configuration::ServerFactoryContext& context) { return context.singletonManager().getTyped( SINGLETON_MANAGER_REGISTERED_NAME(cilium_ipcache)); } IPCache::IPCache(const std::string& bpf_root) - : Bpf(BPF_MAP_TYPE_LPM_TRIE, sizeof(struct ipcache_key), sizeof(struct remote_endpoint_info)), + : Bpf(BPF_MAP_TYPE_LPM_TRIE, sizeof(struct ipcache_key), sizeof(struct RemoteEndpointInfo)), bpf_root_(bpf_root) {} -bool IPCache::Open() { +bool IPCache::open() { // Open the bpf maps from Cilium specific paths std::string path(bpf_root_ + "/tc/globals/cilium_ipcache"); - if (!open(path)) { + if (!Bpf::open(path)) { ENVOY_LOG(info, "cilium.ipcache: Cannot open ipcache map at {}", path); return false; } @@ -96,7 +96,7 @@ bool IPCache::Open() { uint32_t IPCache::resolve(const Network::Address::Ip* ip) { struct ipcache_key key {}; - struct remote_endpoint_info value {}; + struct RemoteEndpointInfo value {}; if (ip->version() == Network::Address::IpVersion::v4) { key.lpm_key = {32 + 32, {}}; diff --git a/cilium/ipcache.h b/cilium/ipcache.h index 5b76ca67e..32d772025 100644 --- a/cilium/ipcache.h +++ b/cilium/ipcache.h @@ -15,12 +15,12 @@ namespace Cilium { class IPCache : public Singleton::Instance, public Bpf { public: - static std::shared_ptr NewIPCache(Server::Configuration::ServerFactoryContext& context, + static std::shared_ptr newIpCache(Server::Configuration::ServerFactoryContext& context, const std::string& bpf_root); - static std::shared_ptr GetIPCache(Server::Configuration::ServerFactoryContext& context); + static std::shared_ptr getIpCache(Server::Configuration::ServerFactoryContext& context); IPCache(const std::string& bpf_root); - bool Open(); + bool open(); const std::string& bpfRoot() { return bpf_root_; } diff --git a/cilium/l7policy.cc b/cilium/l7policy.cc index 354b9fe4a..c6c66ad54 100644 --- a/cilium/l7policy.cc +++ b/cilium/l7policy.cc @@ -72,7 +72,7 @@ Config::Config(const std::string& access_log_path, const std::string& denied_403 : time_source_(time_source), stats_{ALL_CILIUM_STATS(POOL_COUNTER_PREFIX(scope, "cilium"))}, denied_403_body_(denied_403_body), is_upstream_(is_upstream), access_log_(nullptr) { if (access_log_path.length()) { - access_log_ = AccessLog::Open(access_log_path, time_source); + access_log_ = AccessLog::open(access_log_path, time_source); } if (denied_403_body_.length() == 0) { denied_403_body_ = "Access denied"; @@ -87,9 +87,9 @@ Config::Config(const ::cilium::L7Policy& config, TimeSource& time_source, Stats: bool is_upstream) : Config(config.access_log_path(), config.denied_403_body(), time_source, scope, is_upstream) {} -void Config::Log(AccessLog::Entry& entry, ::cilium::EntryType type) { +void Config::log(AccessLog::Entry& entry, ::cilium::EntryType type) { if (access_log_) { - access_log_->Log(entry, type); + access_log_->log(entry, type); } } @@ -206,7 +206,7 @@ Http::FilterHeadersStatus AccessFilter::decodeHeaders(Http::RequestHeaderMap& he bool denied = false; // Enforce Ingress policy only in the downstream filter if (!config_->is_upstream_) { - log_entry_->InitFromRequest( + log_entry_->initFromRequest( policy_fs->pod_ip_, policy_fs->proxy_id_, policy_fs->ingress_, policy_fs->source_identity_, callbacks_->streamInfo().downstreamAddressProvider().remoteAddress(), 0, callbacks_->streamInfo().downstreamAddressProvider().localAddress(), @@ -243,24 +243,24 @@ Http::FilterHeadersStatus AccessFilter::decodeHeaders(Http::RequestHeaderMap& he // Update the log entry with the chosen destination address and current headers, as remaining // filters, upstream, and/or policy may have altered headers. - log_entry_->UpdateFromRequest(destination_identity, dst_address, headers); + log_entry_->updateFromRequest(destination_identity, dst_address, headers); if (!allowed_) { callbacks_->sendLocalReply(Http::Code::Forbidden, config_->denied_403_body_, nullptr, absl::nullopt, absl::string_view()); - config_->Log(*log_entry_, ::cilium::EntryType::Denied); + config_->log(*log_entry_, ::cilium::EntryType::Denied); return Http::FilterHeadersStatus::StopIteration; } // Log as a forwarded request - config_->Log(*log_entry_, ::cilium::EntryType::Request); + config_->log(*log_entry_, ::cilium::EntryType::Request); return Http::FilterHeadersStatus::Continue; } void AccessFilter::onStreamComplete() { // Request may have been left unlogged due to an error and/or missing local reply if (log_entry_ && !log_entry_->request_logged_) { - config_->Log(*log_entry_, ::cilium::EntryType::Request); + config_->log(*log_entry_, ::cilium::EntryType::Request); } } @@ -289,12 +289,12 @@ Http::FilterHeadersStatus AccessFilter::encodeHeaders(Http::ResponseHeaderMap& h logType = ::cilium::EntryType::Denied; config_->stats_.access_denied_.inc(); } - config_->Log(*log_entry_, logType); + config_->log(*log_entry_, logType); } // Log the response - log_entry_->UpdateFromResponse(headers, config_->time_source_); - config_->Log(*log_entry_, ::cilium::EntryType::Response); + log_entry_->updateFromResponse(headers, config_->time_source_); + config_->log(*log_entry_, ::cilium::EntryType::Response); return Http::FilterHeadersStatus::Continue; } diff --git a/cilium/l7policy.h b/cilium/l7policy.h index fb552f658..de4d4758b 100644 --- a/cilium/l7policy.h +++ b/cilium/l7policy.h @@ -49,7 +49,7 @@ class Config : public Logger::Loggable { Config(const ::cilium::L7Policy& config, TimeSource& time_source, Stats::Scope& scope, bool is_upstream); - void Log(AccessLog::Entry&, ::cilium::EntryType); + void log(AccessLog::Entry&, ::cilium::EntryType); TimeSource& time_source_; FilterStats stats_; diff --git a/cilium/network_filter.cc b/cilium/network_filter.cc index e895ee212..399872a82 100644 --- a/cilium/network_filter.cc +++ b/cilium/network_filter.cc @@ -78,16 +78,16 @@ Config::Config(const ::cilium::NetworkFilter& config, : time_source_(context.serverFactoryContext().timeSource()), access_log_(nullptr) { const auto& access_log_path = config.access_log_path(); if (access_log_path.length()) { - access_log_ = Cilium::AccessLog::Open(access_log_path, time_source_); + access_log_ = Cilium::AccessLog::open(access_log_path, time_source_); } if (config.proxylib().length() > 0) { proxylib_ = std::make_shared(config.proxylib(), config.proxylib_params()); } } -void Config::Log(Cilium::AccessLog::Entry& entry, ::cilium::EntryType type) { +void Config::log(Cilium::AccessLog::Entry& entry, ::cilium::EntryType type) { if (access_log_) { - access_log_->Log(entry, type); + access_log_->log(entry, type); } } @@ -115,13 +115,13 @@ Network::FilterStatus Instance::onNewConnection() { // Pass SNI before the upstream callback so that it is available when upstream connection is // initialized. const auto sni = conn.requestedServerName(); - if (sni != "") { + if (!sni.empty()) { ENVOY_CONN_LOG(trace, "cilium.network: SNI: {}", conn, sni); } // Pass metadata from tls_inspector to the filterstate, if any & not already // set via upstream cluster config. - if (sni != "") { + if (!sni.empty()) { auto filterState = conn.streamInfo().filterState(); auto have_sni = filterState->hasData(Network::UpstreamServerName::key()); @@ -197,7 +197,7 @@ Network::FilterStatus Instance::onNewConnection() { if (port_policy.useProxylib(remote_id_, l7proto_)) { // Initialize Go parser if requested if (config_->proxylib_.get() != nullptr) { - go_parser_ = config_->proxylib_->NewInstance( + go_parser_ = config_->proxylib_->newInstance( conn, l7proto_, policy_fs->ingress_, policy_fs->source_identity_, destination_identity, stream_info.downstreamAddressProvider().remoteAddress()->asString(), dst_address->asString(), policy_name); @@ -206,7 +206,7 @@ Network::FilterStatus Instance::onNewConnection() { return false; } } else { // no Go parser, initialize logging for metadata based access control - log_entry_.InitFromConnection(policy_name, policy_fs->proxy_id_, policy_fs->ingress_, + log_entry_.initFromConnection(policy_name, policy_fs->proxy_id_, policy_fs->ingress_, policy_fs->source_identity_, stream_info.downstreamAddressProvider().remoteAddress(), destination_identity, dst_address, &config_->time_source_); @@ -236,18 +236,18 @@ Network::FilterStatus Instance::onData(Buffer::Instance& data, bool end_stream) } if (go_parser_) { FilterResult res = - go_parser_->OnIO(false, data, end_stream); // 'false' marks original direction data + go_parser_->onIo(false, data, end_stream); // 'false' marks original direction data ENVOY_CONN_LOG(trace, "cilium.network::onData: \'GoFilter::OnIO\' returned {}", conn, Envoy::Cilium::toString(res)); if (res != FILTER_OK) { // Drop the connection due to an error - go_parser_->Close(); + go_parser_->close(); reason = "proxylib error"; goto drop_close; } - if (go_parser_->WantReplyInject()) { + if (go_parser_->wantReplyInject()) { ENVOY_CONN_LOG(trace, "cilium.network::onData: calling write() on an empty buffer", conn); // We have no idea when, if ever new data will be received on the @@ -258,10 +258,10 @@ Network::FilterStatus Instance::onData(Buffer::Instance& data, bool end_stream) conn.write(empty, false); } - go_parser_->SetOrigEndStream(end_stream); + go_parser_->setOrigEndStream(end_stream); } else if (!l7proto_.empty()) { const auto& metadata = conn.streamInfo().dynamicMetadata(); - bool changed = log_entry_.UpdateFromMetadata(l7proto_, metadata.filter_metadata().at(l7proto_)); + bool changed = log_entry_.updateFromMetadata(l7proto_, metadata.filter_metadata().at(l7proto_)); // Policy may have changed since the connection was established, get fresh policy const auto policy_fs = @@ -279,13 +279,13 @@ Network::FilterStatus Instance::onData(Buffer::Instance& data, bool end_stream) const auto& policy = policy_fs->getPolicy(); auto port_policy = policy.findPortPolicy(policy_fs->ingress_, destination_port_); if (!port_policy.allowed(remote_id_, metadata)) { - config_->Log(log_entry_, ::cilium::EntryType::Denied); + config_->log(log_entry_, ::cilium::EntryType::Denied); reason = "metadata policy drop"; goto drop_close; } else { // accesslog only if metadata has changed if (changed) { - config_->Log(log_entry_, ::cilium::EntryType::Request); + config_->log(log_entry_, ::cilium::EntryType::Request); } } } @@ -300,20 +300,20 @@ Network::FilterStatus Instance::onData(Buffer::Instance& data, bool end_stream) Network::FilterStatus Instance::onWrite(Buffer::Instance& data, bool end_stream) { if (go_parser_) { FilterResult res = - go_parser_->OnIO(true, data, end_stream); // 'true' marks reverse direction data + go_parser_->onIo(true, data, end_stream); // 'true' marks reverse direction data ENVOY_CONN_LOG(trace, "cilium.network::OnWrite: \'GoFilter::OnIO\' returned {}", callbacks_->connection(), Envoy::Cilium::toString(res)); if (res != FILTER_OK) { // Drop the connection due to an error - go_parser_->Close(); + go_parser_->close(); return Network::FilterStatus::StopIteration; } // XXX: Unfortunately continueReading() continues from the next filter, and // there seems to be no way to trigger the whole filter chain to be called. - go_parser_->SetReplyEndStream(end_stream); + go_parser_->setReplyEndStream(end_stream); } return Network::FilterStatus::Continue; diff --git a/cilium/network_filter.h b/cilium/network_filter.h index 6ec85e593..0add9f96d 100644 --- a/cilium/network_filter.h +++ b/cilium/network_filter.h @@ -33,7 +33,7 @@ class Config : Logger::Loggable { Config(const ::cilium::NetworkFilter& config, Server::Configuration::FactoryContext& context); Config(const Json::Object& config, Server::Configuration::FactoryContext& context); - void Log(Cilium::AccessLog::Entry&, ::cilium::EntryType); + void log(Cilium::AccessLog::Entry&, ::cilium::EntryType); Cilium::GoFilterSharedPtr proxylib_; TimeSource& time_source_; diff --git a/cilium/network_policy.cc b/cilium/network_policy.cc index 95ae285b6..bfaac8f49 100644 --- a/cilium/network_policy.cc +++ b/cilium/network_policy.cc @@ -79,16 +79,17 @@ class HeaderMatch : public Logger::Loggable { HeaderMatch(const NetworkPolicyMap& parent, const cilium::HeaderMatch& config) : name_(config.name()), value_(config.value()), match_action_(config.match_action()), mismatch_action_(config.mismatch_action()) { - if (config.value_sds_secret().length() > 0) + if (config.value_sds_secret().length() > 0) { secret_ = std::make_unique(parent, config.value_sds_secret()); + } } void logRejected(Cilium::AccessLog::Entry& log_entry, absl::string_view value) const { - log_entry.AddRejected(name_.get(), !secret_ ? value : "[redacted]"); + log_entry.addRejected(name_.get(), !secret_ ? value : "[redacted]"); } void logMissing(Cilium::AccessLog::Entry& log_entry, absl::string_view value) const { - log_entry.AddMissing(name_.get(), !secret_ ? value : "[redacted]"); + log_entry.addMissing(name_.get(), !secret_ ? value : "[redacted]"); } // Returns 'true' if matching can continue @@ -100,9 +101,9 @@ class HeaderMatch : public Logger::Loggable { // Get secret value? if (secret_) { auto* secret_value = secret_->value(); - if (secret_value) + if (secret_value) { match_value = secret_value; - else if (value_.length() == 0) { + } else if (value_.length() == 0) { // fail if secret has no value and the inline value to match is also empty ENVOY_LOG(info, "Cilium HeaderMatch missing SDS secret value for header {}", name_); return false; @@ -111,9 +112,9 @@ class HeaderMatch : public Logger::Loggable { // Perform presence match if the value to match is empty bool isPresentMatch = match_value->length() == 0; - if (isPresentMatch) + if (isPresentMatch) { matches = header_value.result().has_value(); - else if (header_value.result().has_value()) { + } else if (header_value.result().has_value()) { const absl::string_view val = header_value.result().value(); if (val.length() == match_value->length()) { // Use constant time comparison for security reason @@ -154,8 +155,9 @@ class HeaderMatch : public Logger::Loggable { // presence match failed, nothing to do return true; } - if (!header_value.result().has_value()) + if (!header_value.result().has_value()) { return true; // nothing to remove + } // Remove the header with an incorrect value headers.remove(name_); @@ -163,8 +165,9 @@ class HeaderMatch : public Logger::Loggable { return true; case cilium::HeaderMatch::REPLACE_ON_MISMATCH: // Log the wrong value as rejected, if the header existed with a wrong value - if (header_value.result().has_value()) + if (header_value.result().has_value()) { logRejected(log_entry, header_value.result().value()); + } // Set the expected value headers.setCopy(name_, *match_value); // Log the expected value as missing @@ -247,7 +250,7 @@ class HttpNetworkPolicyRule : public Logger::Loggable { // Should only be called after 'allowed' returns 'true'. // Returns 'true' if matching can continue - bool HeaderMatches(Envoy::Http::RequestHeaderMap& headers, + bool headerMatches(Envoy::Http::RequestHeaderMap& headers, Cilium::AccessLog::Entry& log_entry) const { bool accepted = true; for (const auto& header_match : header_matches_) { @@ -260,7 +263,7 @@ class HttpNetworkPolicyRule : public Logger::Loggable { void toString(int indent, std::string& res) const { bool first = true; - if (headers_.size() > 0) { + if (!headers_.empty()) { if (first) { first = false; res.append(indent - 2, ' ').append("- "); @@ -308,9 +311,9 @@ class HttpNetworkPolicyRule : public Logger::Loggable { } } } - if (header_matches_.size() > 0) { + if (!header_matches_.empty()) { if (first) { - first = false; + // first = false; // not used after, so no need to update res.append(indent - 2, ' ').append("- "); } else { res.append(indent, ' '); @@ -409,7 +412,7 @@ class PortNetworkPolicyRule : public Logger::Loggable { bool allowed(uint32_t remote_id, bool& denied) const { // Remote ID must match if we have any. - if (remotes_.size() > 0) { + if (!remotes_.empty()) { auto match = remotes_.find(remote_id); if (match != remotes_.end()) { // remote ID matched @@ -434,7 +437,7 @@ class PortNetworkPolicyRule : public Logger::Loggable { bool allowed(uint32_t remote_id, absl::string_view sni, bool& denied) const { // sni must match if we have any - if (allowed_snis_.size() > 0) { + if (!allowed_snis_.empty()) { if (sni.length() == 0) { return false; } @@ -451,7 +454,7 @@ class PortNetworkPolicyRule : public Logger::Loggable { if (!allowed(remote_id, denied)) { return false; } - if (http_rules_.size() > 0) { + if (!http_rules_.empty()) { bool allowed = false; for (const auto& rule : http_rules_) { if (rule.allowed(headers)) { @@ -462,7 +465,7 @@ class PortNetworkPolicyRule : public Logger::Loggable { } // orherwise evaluate all rules to run all the header actions, // and remember if any of them matched - if (rule.HeaderMatches(headers, log_entry)) { + if (rule.headerMatches(headers, log_entry)) { allowed = true; } } @@ -501,7 +504,7 @@ class PortNetworkPolicyRule : public Logger::Loggable { return false; // request is denied if any deny rule matches } } - if (l7_allow_rules_.size() > 0) { + if (!l7_allow_rules_.empty()) { for (const auto& rule : l7_allow_rules_) { if (rule.matches(metadata)) { ENVOY_LOG(trace, @@ -571,7 +574,7 @@ class PortNetworkPolicyRule : public Logger::Loggable { res.append(indent, ' ').append("deny: true\n"); } - if (allowed_snis_.size() > 0) { + if (!allowed_snis_.empty()) { res.append(indent, ' ').append("allowed_snis: ["); int count = 0; for (auto& sni : allowed_snis_) { @@ -583,7 +586,7 @@ class PortNetworkPolicyRule : public Logger::Loggable { res.append("]\n"); } - if (http_rules_.size() > 0) { + if (!http_rules_.empty()) { res.append(indent, ' ').append("http_rules:\n"); for (auto& rule : http_rules_) { rule.toString(indent + 2, res); @@ -593,13 +596,13 @@ class PortNetworkPolicyRule : public Logger::Loggable { if (l7_proto_.length() > 0) { res.append(indent, ' ').append("l7_proto: \"").append(l7_proto_).append("\"\n"); } - if (l7_allow_rules_.size() > 0) { + if (!l7_allow_rules_.empty()) { res.append(indent, ' ').append("l7_allow_rules:\n"); for (auto& rule : l7_allow_rules_) { rule.toString(indent + 2, res); } } - if (l7_deny_rules_.size() > 0) { + if (!l7_deny_rules_.empty()) { res.append(indent, ' ').append("l7_deny_rules:\n"); for (auto& rule : l7_deny_rules_) { rule.toString(indent + 2, res); @@ -624,10 +627,10 @@ using PortNetworkPolicyRuleConstSharedPtr = std::shared_ptr { public: - PortNetworkPolicyRules() {} + PortNetworkPolicyRules() = default; PortNetworkPolicyRules(const NetworkPolicyMap& parent, const Protobuf::RepeatedPtrField& rules) { - if (rules.size() == 0) { + if (rules.empty()) { ENVOY_LOG(trace, "Cilium L7 PortNetworkPolicyRules(): No rules, will allow " "everything."); } @@ -649,7 +652,7 @@ class PortNetworkPolicyRules : public Logger::Loggable { bool allowed(uint32_t remote_id, Envoy::Http::RequestHeaderMap& headers, Cilium::AccessLog::Entry& log_entry, bool& denied) const { // Empty set matches any payload from anyone - if (rules_.size() == 0) { + if (rules_.empty()) { return true; } @@ -671,7 +674,7 @@ class PortNetworkPolicyRules : public Logger::Loggable { bool allowed(uint32_t remote_id, absl::string_view sni, bool& denied) const { // Empty set matches any payload from anyone - if (rules_.size() == 0) { + if (rules_.empty()) { return true; } @@ -701,7 +704,7 @@ class PortNetworkPolicyRules : public Logger::Loggable { bool allowed(uint32_t remote_id, const envoy::config::core::v3::Metadata& metadata, bool& denied) const { // Empty set matches any payload from anyone - if (rules_.size() == 0) { + if (rules_.empty()) { return true; } @@ -725,8 +728,9 @@ class PortNetworkPolicyRules : public Logger::Loggable { for (const auto& rule : rules_) { Ssl::ContextSharedPtr server_context = rule->getServerTlsContext(remote_id, sni, config, raw_socket_allowed); - if (server_context) + if (server_context) { return server_context; + } } return nullptr; } @@ -737,8 +741,9 @@ class PortNetworkPolicyRules : public Logger::Loggable { for (const auto& rule : rules_) { Ssl::ContextSharedPtr client_context = rule->getClientTlsContext(remote_id, sni, config, raw_socket_allowed); - if (client_context) + if (client_context) { return client_context; + } } return nullptr; } @@ -761,34 +766,38 @@ class PortNetworkPolicyRules : public Logger::Loggable { PortPolicy::PortPolicy(const PolicyMap& map, const RulesList& wildcard_rules, uint16_t port) : map_(map), wildcard_rules_(wildcard_rules), port_rules_(map_.find({port, port})) {} -// for_range is used for policy lookups, so it will need to check both port-specific and +// forRange is used for policy lookups, so it will need to check both port-specific and // wildcard-port rules, as either of them could contain rules that must be evaluated (i.e., deny // or header match rules with side effects). -bool PortPolicy::for_range( +bool PortPolicy::forRange( std::function allowed) const { bool allow = false; bool denied = false; if (port_rules_ != map_.cend()) { for (auto& rules : port_rules_->second) { // Skip if allowed - if (allow && rules.can_short_circuit_) + if (allow && rules.can_short_circuit_) { continue; - if (allowed(rules, denied)) + } + if (allowed(rules, denied)) { allow = true; + } } } // Wildcard port can deny a specific remote, so need to check for it too. for (auto& rules : wildcard_rules_) { // Skip if allowed - if (allow && rules.can_short_circuit_) + if (allow && rules.can_short_circuit_) { continue; - if (allowed(rules, denied)) + } + if (allowed(rules, denied)) { allow = true; + } } return allow && !denied; } -// for_first_range is used for proxylib parser and TLS context selection. +// forFirstRange is used for proxylib parser and TLS context selection. // // rules for the specific ports are checked first, and within there singe-port ranges are placed in // the front, while actual ranges are placed in the back. This results in the following precedence @@ -798,43 +807,45 @@ bool PortPolicy::for_range( // 2. port ranges (e.g., ports 80-90) // 3. Wildcard port rules // -bool PortPolicy::for_first_range(std::function f) const { +bool PortPolicy::forFirstRange(std::function f) const { if (port_rules_ != map_.cend()) { for (auto& rules : port_rules_->second) { - if (f(rules)) + if (f(rules)) { return true; + } } } // Check the wildcard port entry for (auto& rules : wildcard_rules_) { - if (f(rules)) + if (f(rules)) { return true; + } } return false; } bool PortPolicy::useProxylib(uint32_t remote_id, std::string& l7_proto) const { - return for_first_range([&](const PortNetworkPolicyRules& rules) -> bool { + return forFirstRange([&](const PortNetworkPolicyRules& rules) -> bool { return rules.useProxylib(remote_id, l7_proto); }); } bool PortPolicy::allowed(uint32_t remote_id, Envoy::Http::RequestHeaderMap& headers, Cilium::AccessLog::Entry& log_entry) const { - return for_range([&](const PortNetworkPolicyRules& rules, bool& denied) -> bool { + return forRange([&](const PortNetworkPolicyRules& rules, bool& denied) -> bool { return rules.allowed(remote_id, headers, log_entry, denied); }); } bool PortPolicy::allowed(uint32_t remote_id, absl::string_view sni) const { - return for_range([&](const PortNetworkPolicyRules& rules, bool& denied) -> bool { + return forRange([&](const PortNetworkPolicyRules& rules, bool& denied) -> bool { return rules.allowed(remote_id, sni, denied); }); } bool PortPolicy::allowed(uint32_t remote_id, const envoy::config::core::v3::Metadata& metadata) const { - return for_range([&](const PortNetworkPolicyRules& rules, bool& denied) -> bool { + return forRange([&](const PortNetworkPolicyRules& rules, bool& denied) -> bool { return rules.allowed(remote_id, metadata, denied); }); } @@ -843,7 +854,7 @@ Ssl::ContextSharedPtr PortPolicy::getServerTlsContext(uint32_t remote_id, absl:: const Ssl::ContextConfig** config, bool& raw_socket_allowed) const { Ssl::ContextSharedPtr ret; - for_first_range([&](const PortNetworkPolicyRules& rules) -> bool { + forFirstRange([&](const PortNetworkPolicyRules& rules) -> bool { ret = rules.getServerTlsContext(remote_id, sni, config, raw_socket_allowed); return ret != nullptr; }); @@ -854,7 +865,7 @@ Ssl::ContextSharedPtr PortPolicy::getClientTlsContext(uint32_t remote_id, absl:: const Ssl::ContextConfig** config, bool& raw_socket_allowed) const { Ssl::ContextSharedPtr ret; - for_first_range([&](const PortNetworkPolicyRules& rules) -> bool { + forFirstRange([&](const PortNetworkPolicyRules& rules) -> bool { ret = rules.getClientTlsContext(remote_id, sni, config, raw_socket_allowed); return ret != nullptr; }); @@ -862,7 +873,7 @@ Ssl::ContextSharedPtr PortPolicy::getClientTlsContext(uint32_t remote_id, absl:: } // Ranges overlap when one is not completely below or above the other -bool inline ranges_overlap(const PortRange& a, const PortRange& b) { +bool inline rangesOverlap(const PortRange& a, const PortRange& b) { // !(a.second < b.first || a.first > b.second) return a.second >= b.first && a.first <= b.second; } @@ -922,8 +933,9 @@ class PortNetworkPolicy : public Logger::Loggable { while (it != rules_.begin()) { last_overlap = it; it--; - if (!ranges_overlap(it->first, rule_range)) + if (!rangesOverlap(it->first, rule_range)) { break; + } } it = last_overlap; // Move back up to the frontmost overlapping entry @@ -967,8 +979,9 @@ class PortNetworkPolicy : public Logger::Loggable { // update the start range if a new start entry was added, which can happen only at the // beginning of this loop when port is still at the beginning of the rule range being // added. - if (port == rule_range.first) + if (port == rule_range.first) { start_range = new_range; + } // absl::btree_map insertion invalidates iterators, have to update. it = ++new_pair.first; // one past the new entry if (end_port < range.first) { @@ -1015,7 +1028,7 @@ class PortNetworkPolicy : public Logger::Loggable { // Add rules to all the overlapping entries bool singular = rule_range.first == rule_range.second; auto rules = PortNetworkPolicyRules(parent, rule.rules()); - for (; it != rules_.end() && ranges_overlap(it->first, rule_range); it++) { + for (; it != rules_.end() && rangesOverlap(it->first, rule_range); it++) { auto range = it->first; auto& list = it->second; ENVOY_LOG(trace, "Cilium L7 PortNetworkPolicy(): Adding rules for [{}-{}] to [{}-{}]", @@ -1041,23 +1054,23 @@ class PortNetworkPolicy : public Logger::Loggable { } void toString(int indent, std::string& res) const { - if (rules_.size() == 0) { + if (rules_.empty()) { res.append(indent, ' ').append("rules: []\n"); } else { res.append(indent, ' ').append("rules:\n"); - for (auto entry : rules_) { + for (const auto& entry : rules_) { res.append(indent + 2, ' ') .append(fmt::format("[{}-{}]:\n", entry.first.first, entry.first.second)); - for (auto rule : entry.second) { + for (const auto& rule : entry.second) { rule.toString(indent + 4, res); } } } - if (wildcard_rules_.size() == 0) { + if (wildcard_rules_.empty()) { res.append(indent, ' ').append("wildcard_rules: []\n"); } else { res.append(indent, ' ').append("wildcard_rules:\n"); - for (auto rule : wildcard_rules_) { + for (const auto& rule : wildcard_rules_) { rule.toString(indent + 2, res); } } @@ -1107,7 +1120,7 @@ class PolicyInstanceImpl : public PolicyInstance { const IPAddressPair& getEndpointIPs() const override { return endpoint_ips_; } - std::string String() const override { + std::string string() const override { std::string res; res.append("ingress:\n"); ingress_.toString(2, res); @@ -1237,10 +1250,10 @@ NetworkPolicyMap::onConfigUpdate(const std::vectorOpen(); + ipcache->open(); } } @@ -1262,7 +1275,7 @@ NetworkPolicyMap::onConfigUpdate(const std::vectorfind(endpoint_ip); if (it != map->end()) { @@ -1480,7 +1493,7 @@ NetworkPolicyMap::GetPolicyInstanceImpl(const std::string& endpoint_ip) const { return nullptr; } -// GetPolicyInstance return a const reference to a policy in the policy map for the given +// getPolicyInstance return a const reference to a policy in the policy map for the given // 'endpoint_ip'. If there is no policy for the given IP, a default policy is returned, // controlled by the 'default_allow_egress' argument as follows: // @@ -1491,9 +1504,9 @@ NetworkPolicyMap::GetPolicyInstanceImpl(const std::string& endpoint_ip) const { // server error" if no policy is found. This mirrors what bpf datapath does if no policy entry is // found in the bpf policy map. The default deny for ingress with default allow for egress is needed // for Cilium Ingress when there is no egress policy enforcement for the Ingress traffic. -const PolicyInstance& NetworkPolicyMap::GetPolicyInstance(const std::string& endpoint_ip, +const PolicyInstance& NetworkPolicyMap::getPolicyInstance(const std::string& endpoint_ip, bool default_allow_egress) const { - const auto* policy = GetPolicyInstanceImpl(endpoint_ip); + const auto* policy = getPolicyInstanceImpl(endpoint_ip); return policy != nullptr ? *policy : default_allow_egress ? *static_cast(&AllowAllEgressPolicy) : *static_cast(&DenyAllPolicy); diff --git a/cilium/network_policy.h b/cilium/network_policy.h index 43c0b260f..a19405bc5 100644 --- a/cilium/network_policy.h +++ b/cilium/network_policy.h @@ -113,8 +113,8 @@ class PortPolicy : public Logger::Loggable { bool& raw_socket_allowed) const; private: - bool for_range(std::function allowed) const; - bool for_first_range(std::function f) const; + bool forRange(std::function allowed) const; + bool forFirstRange(std::function f) const; const PolicyMap& map_; const RulesList& wildcard_rules_; @@ -123,7 +123,7 @@ class PortPolicy : public Logger::Loggable { class IPAddressPair { public: - IPAddressPair(){}; + IPAddressPair() = default; IPAddressPair(Network::Address::InstanceConstSharedPtr& ipv4, Network::Address::InstanceConstSharedPtr& ipv6) : ipv4_(ipv4), ipv6_(ipv6){}; @@ -163,7 +163,7 @@ class PolicyInstance { virtual const IPAddressPair& getEndpointIPs() const PURE; - virtual std::string String() const PURE; + virtual std::string string() const PURE; virtual void tlsWrapperMissingPolicyInc() const PURE; }; @@ -224,7 +224,7 @@ class NetworkPolicyMap : public Singleton::Instance, public: NetworkPolicyMap(Server::Configuration::FactoryContext& context); NetworkPolicyMap(Server::Configuration::FactoryContext& context, Cilium::CtMapSharedPtr& ct); - ~NetworkPolicyMap(); + ~NetworkPolicyMap() override; // subscription_->start() calls onConfigUpdate(), which uses // shared_from_this(), which cannot be called before a shared @@ -237,16 +237,16 @@ class NetworkPolicyMap : public Singleton::Instance, subscription_ = std::move(subscription); } - const PolicyInstance& GetPolicyInstance(const std::string& endpoint_policy_name, + const PolicyInstance& getPolicyInstance(const std::string& endpoint_policy_name, bool allow_egress) const; static DenyAllPolicyInstanceImpl DenyAllPolicy; - static PolicyInstance& GetDenyAllPolicy(); + static PolicyInstance& getDenyAllPolicy(); static AllowAllEgressPolicyInstanceImpl AllowAllEgressPolicy; - static PolicyInstance& GetAllowAllEgressPolicy(); + static PolicyInstance& getAllowAllEgressPolicy(); bool exists(const std::string& endpoint_policy_name) const { - return GetPolicyInstanceImpl(endpoint_policy_name) != nullptr; + return getPolicyInstanceImpl(endpoint_policy_name) != nullptr; } // run the given function after all the threads have scheduled @@ -312,7 +312,7 @@ class NetworkPolicyMap : public Singleton::Instance, return map_ptr_.exchange(map, std::memory_order_release); } - const PolicyInstance* GetPolicyInstanceImpl(const std::string& endpoint_policy_name) const; + const PolicyInstance* getPolicyInstanceImpl(const std::string& endpoint_policy_name) const; void removeInitManager(); diff --git a/cilium/privileged_service_client.cc b/cilium/privileged_service_client.cc index 9352e1acf..d72f35d08 100644 --- a/cilium/privileged_service_client.cc +++ b/cilium/privileged_service_client.cc @@ -38,7 +38,7 @@ ProtocolClient::ProtocolClient() (get_capabilities(CAP_PERMITTED) & ~(1UL << CAP_NET_BIND_SERVICE)) == 0, "cilium-envoy running with privileges, exiting"); - if (!check_privileged_service()) { + if (!checkPrivilegedService()) { // No Cilium privileged service detected close(); } @@ -90,7 +90,7 @@ ssize_t ProtocolClient::transact(MessageHeader& req, size_t req_len, const void* return size; } -bool ProtocolClient::check_privileged_service() { +bool ProtocolClient::checkPrivilegedService() { // Dump the effective capabilities of the privileged service process DumpRequest req; Response resp; @@ -105,8 +105,8 @@ bool ProtocolClient::check_privileged_service() { return true; } -Envoy::Api::SysCallIntResult ProtocolClient::bpf_open(const char* path) { - if (!have_cilium_privileged_service()) { +Envoy::Api::SysCallIntResult ProtocolClient::bpfOpen(const char* path) { + if (!haveCiliumPrivilegedService()) { return {-1, EPERM}; } @@ -123,9 +123,9 @@ Envoy::Api::SysCallIntResult ProtocolClient::bpf_open(const char* path) { return Envoy::Api::SysCallIntResult{resp.return_value_, resp.errno_}; } -Envoy::Api::SysCallIntResult ProtocolClient::bpf_lookup(int fd, const void* key, uint32_t key_size, - void* value, uint32_t value_size) { - if (!have_cilium_privileged_service()) { +Envoy::Api::SysCallIntResult ProtocolClient::bpfLookup(int fd, const void* key, uint32_t key_size, + void* value, uint32_t value_size) { + if (!haveCiliumPrivilegedService()) { return {-1, EPERM}; } @@ -140,7 +140,7 @@ Envoy::Api::SysCallIntResult ProtocolClient::bpf_lookup(int fd, const void* key, Envoy::Api::SysCallIntResult ProtocolClient::setsockopt(int sockfd, int level, int optname, const void* optval, socklen_t optlen) { - if (!have_cilium_privileged_service()) { + if (!haveCiliumPrivilegedService()) { return {-1, EPERM}; } diff --git a/cilium/privileged_service_client.h b/cilium/privileged_service_client.h index 8e5b0f822..bcae265ba 100644 --- a/cilium/privileged_service_client.h +++ b/cilium/privileged_service_client.h @@ -39,13 +39,13 @@ class ProtocolClient : public Protocol { protected: // Read-only bpf syscalls - Envoy::Api::SysCallIntResult bpf_open(const char* path); - Envoy::Api::SysCallIntResult bpf_lookup(int fd, const void* key, uint32_t key_size, void* value, - uint32_t value_size); + Envoy::Api::SysCallIntResult bpfOpen(const char* path); + Envoy::Api::SysCallIntResult bpfLookup(int fd, const void* key, uint32_t key_size, void* value, + uint32_t value_size); private: - bool check_privileged_service(); - bool have_cilium_privileged_service() const { return is_open(); } + bool checkPrivilegedService(); + bool haveCiliumPrivilegedService() const { return is_open(); } ssize_t transact(MessageHeader& req, size_t req_len, const void* data, size_t datalen, int* fd, Response& resp, void* buf = nullptr, size_t bufsize = 0, bool assert = true); diff --git a/cilium/proxylib.cc b/cilium/proxylib.cc index 7504e2eab..7875a0d96 100644 --- a/cilium/proxylib.cc +++ b/cilium/proxylib.cc @@ -90,7 +90,7 @@ GoFilter::~GoFilter() { } } -GoFilter::InstancePtr GoFilter::NewInstance(Network::Connection& conn, const std::string& go_proto, +GoFilter::InstancePtr GoFilter::newInstance(Network::Connection& conn, const std::string& go_proto, bool ingress, uint32_t src_id, uint32_t dst_id, const std::string& src_addr, const std::string& dst_addr, @@ -113,7 +113,7 @@ GoFilter::InstancePtr GoFilter::NewInstance(Network::Connection& conn, const std return parser; } -FilterResult GoFilter::Instance::OnIO(bool reply, Buffer::Instance& data, bool end_stream) { +FilterResult GoFilter::Instance::onIo(bool reply, Buffer::Instance& data, bool end_stream) { auto& dir = reply ? reply_ : orig_; int64_t data_len = data.length(); @@ -311,7 +311,7 @@ FilterResult GoFilter::Instance::OnIO(bool reply, Buffer::Instance& data, bool e return FILTER_PARSER_ERROR; } - inject_buf_exhausted = dir.inject_slice_.at_capacity(); + inject_buf_exhausted = dir.inject_slice_.atCapacity(); // Make space for more injected data dir.inject_slice_.reset(); @@ -327,7 +327,7 @@ FilterResult GoFilter::Instance::OnIO(bool reply, Buffer::Instance& data, bool e return res; } -void GoFilter::Instance::Close() { +void GoFilter::Instance::close() { (*parent_.go_close_)(connection_id_); connection_id_ = 0; conn_.close(Network::ConnectionCloseType::FlushWrite); diff --git a/cilium/proxylib.h b/cilium/proxylib.h index 73a6c752b..f826d159e 100644 --- a/cilium/proxylib.h +++ b/cilium/proxylib.h @@ -89,7 +89,7 @@ template struct ResetableSlice : GoSlice { return len; } - bool at_capacity() { + bool atCapacity() { // Return true if all of the available space was used, not affected by // draining return (data_ + len_) >= (base_ + cap_); @@ -137,19 +137,19 @@ class GoFilter : public Logger::Loggable { } } - void Close(); + void close(); - FilterResult OnIO(bool reply, Buffer::Instance& data, bool end_stream); + FilterResult onIo(bool reply, Buffer::Instance& data, bool end_stream); - bool WantReplyInject() const { return reply_.WantToInject(); } - void SetOrigEndStream(bool end_stream) { orig_.closed_ = end_stream; } - void SetReplyEndStream(bool end_stream) { reply_.closed_ = end_stream; } + bool wantReplyInject() const { return reply_.wantToInject(); } + void setOrigEndStream(bool end_stream) { orig_.closed_ = end_stream; } + void setReplyEndStream(bool end_stream) { reply_.closed_ = end_stream; } struct Direction { Direction() : inject_slice_(inject_buf_, sizeof(inject_buf_)) {} - bool WantToInject() const { return !closed_ && inject_slice_.len() > 0; } - void Close() { closed_ = true; } + bool wantToInject() const { return !closed_ && inject_slice_.len() > 0; } + void close() { closed_ = true; } Buffer::OwnedImpl buffer_; // Buffered data in this direction int64_t need_bytes_{0}; // Number of additional data bytes needed before can parse again @@ -168,7 +168,7 @@ class GoFilter : public Logger::Loggable { }; using InstancePtr = std::unique_ptr; - InstancePtr NewInstance(Network::Connection& conn, const std::string& go_proto, bool ingress, + InstancePtr newInstance(Network::Connection& conn, const std::string& go_proto, bool ingress, uint32_t src_id, uint32_t dst_id, const std::string& src_addr, const std::string& dst_addr, const std::string& policy_name) const; diff --git a/cilium/secret_watcher.cc b/cilium/secret_watcher.cc index fdfd32d12..0774c656f 100644 --- a/cilium/secret_watcher.cc +++ b/cilium/secret_watcher.cc @@ -47,8 +47,8 @@ secretProvider(Server::Configuration::TransportSocketFactoryContext& context, } // namespace -getSDSConfigFunc getSDSConfig = &getCiliumSDSConfig; -void setSDSConfigFunc(getSDSConfigFunc func) { getSDSConfig = func; } +GetSdsConfigFunc getSDSConfig = &getCiliumSDSConfig; +void setSDSConfigFunc(GetSdsConfigFunc func) { getSDSConfig = func; } void resetSDSConfigFunc() { getSDSConfig = &getCiliumSDSConfig; } SecretWatcher::SecretWatcher(const NetworkPolicyMap& parent, const std::string& sds_name) @@ -98,26 +98,26 @@ namespace { void setCommonConfig(const cilium::TLSContext config, envoy::extensions::transport_sockets::tls::v3::CommonTlsContext* tls_context) { - if (config.validation_context_sds_secret() != "") { + if (!config.validation_context_sds_secret().empty()) { auto sds_secret = tls_context->mutable_validation_context_sds_secret_config(); sds_secret->set_name(config.validation_context_sds_secret()); auto* config_source = sds_secret->mutable_sds_config(); *config_source = getSDSConfig(config.validation_context_sds_secret()); - } else if (config.trusted_ca() != "") { + } else if (!config.trusted_ca().empty()) { auto validation_context = tls_context->mutable_validation_context(); auto trusted_ca = validation_context->mutable_trusted_ca(); trusted_ca->set_inline_string(config.trusted_ca()); } - if (config.tls_sds_secret() != "") { + if (!config.tls_sds_secret().empty()) { auto sds_secret = tls_context->add_tls_certificate_sds_secret_configs(); sds_secret->set_name(config.tls_sds_secret()); auto* config_source = sds_secret->mutable_sds_config(); *config_source = getSDSConfig(config.tls_sds_secret()); - } else if (config.certificate_chain() != "") { + } else if (!config.certificate_chain().empty()) { auto tls_certificate = tls_context->add_tls_certificates(); auto certificate_chain = tls_certificate->mutable_certificate_chain(); certificate_chain->set_inline_string(config.certificate_chain()); - if (config.private_key() != "") { + if (!config.private_key().empty()) { auto private_key = tls_certificate->mutable_private_key(); private_key->set_inline_string(config.private_key()); } else { @@ -138,14 +138,15 @@ DownstreamTLSContext::DownstreamTLSContext(const NetworkPolicyMap& parent, const cilium::TLSContext config) : TLSContext(parent, "server") { // Server config always needs the TLS certificate to present to the client - if (config.tls_sds_secret() == "" && config.certificate_chain() == "") + if (config.tls_sds_secret().empty() && config.certificate_chain().empty()) { throw EnvoyException("Downstream TLS Context: missing certificate chain"); + } envoy::extensions::transport_sockets::tls::v3::DownstreamTlsContext context_config; auto tls_context = context_config.mutable_common_tls_context(); // Check if client certificate is required - if (config.validation_context_sds_secret() != "" || config.trusted_ca() != "") { + if (!config.validation_context_sds_secret().empty() || !config.trusted_ca().empty()) { auto require_tls_certificate = context_config.mutable_require_client_certificate(); require_tls_certificate->set_value(true); } @@ -156,6 +157,7 @@ DownstreamTLSContext::DownstreamTLSContext(const NetworkPolicyMap& parent, } auto server_config_or_error = Extensions::TransportSockets::Tls::ServerContextConfigImpl::create( context_config, parent.transportFactoryContext(), false); + // NOLINTNEXTLINE(performance-unnecessary-copy-initialization) THROW_IF_NOT_OK(server_config_or_error.status()); server_config_ = std::move(server_config_or_error.value()); @@ -163,6 +165,7 @@ DownstreamTLSContext::DownstreamTLSContext(const NetworkPolicyMap& parent, ENVOY_LOG(debug, "Server secret is updated."); auto ctx_or_error = manager_.createSslServerContext(scope_, *server_config_, server_names_, nullptr); + // NOLINTNEXTLINE(performance-unnecessary-copy-initialization) THROW_IF_NOT_OK(ctx_or_error.status()); auto ctx = std::move(ctx_or_error.value()); { @@ -174,18 +177,20 @@ DownstreamTLSContext::DownstreamTLSContext(const NetworkPolicyMap& parent, return absl::OkStatus(); }; server_config_->setSecretUpdateCallback(create_server_context); - if (server_config_->isReady()) + if (server_config_->isReady()) { static_cast(create_server_context()); - else + } else { parent.transportFactoryContext().initManager().add(init_target_); + } } UpstreamTLSContext::UpstreamTLSContext(const NetworkPolicyMap& parent, cilium::TLSContext config) : TLSContext(parent, "client") { // Client context always needs the trusted CA for server certificate validation // TODO: Default to system default trusted CAs? - if (config.validation_context_sds_secret() == "" && config.trusted_ca() == "") + if (config.validation_context_sds_secret().empty() && config.trusted_ca().empty()) { throw EnvoyException("Upstream TLS Context: missing trusted CA"); + } envoy::extensions::transport_sockets::tls::v3::UpstreamTlsContext context_config; auto tls_context = context_config.mutable_common_tls_context(); @@ -199,12 +204,14 @@ UpstreamTLSContext::UpstreamTLSContext(const NetworkPolicyMap& parent, cilium::T } auto client_config_or_error = Extensions::TransportSockets::Tls::ClientContextConfigImpl::create( context_config, parent.transportFactoryContext()); + // NOLINTNEXTLINE(performance-unnecessary-copy-initialization) THROW_IF_NOT_OK(client_config_or_error.status()); client_config_ = std::move(client_config_or_error.value()); auto create_client_context = [this]() { ENVOY_LOG(debug, "Client secret is updated."); auto ctx_or_error = manager_.createSslClientContext(scope_, *client_config_); + // NOLINTNEXTLINE(performance-unnecessary-copy-initialization) THROW_IF_NOT_OK(ctx_or_error.status()); auto ctx = std::move(ctx_or_error.value()); { @@ -216,10 +223,11 @@ UpstreamTLSContext::UpstreamTLSContext(const NetworkPolicyMap& parent, cilium::T return absl::OkStatus(); }; client_config_->setSecretUpdateCallback(create_client_context); - if (client_config_->isReady()) + if (client_config_->isReady()) { static_cast(create_client_context()); - else + } else { parent.transportFactoryContext().initManager().add(init_target_); + } } } // namespace Cilium diff --git a/cilium/secret_watcher.h b/cilium/secret_watcher.h index 944932e27..a0811ee81 100644 --- a/cilium/secret_watcher.h +++ b/cilium/secret_watcher.h @@ -26,9 +26,9 @@ namespace Envoy { namespace Cilium { // Facility for SDS config override for testing -using getSDSConfigFunc = envoy::config::core::v3::ConfigSource (*)(const std::string&); -extern getSDSConfigFunc getSDSConfig; -void setSDSConfigFunc(getSDSConfigFunc); +using GetSdsConfigFunc = envoy::config::core::v3::ConfigSource (*)(const std::string&); +extern GetSdsConfigFunc getSDSConfig; +void setSDSConfigFunc(GetSdsConfigFunc); void resetSDSConfigFunc(); class SecretWatcher : public Logger::Loggable { diff --git a/cilium/tls_wrapper.cc b/cilium/tls_wrapper.cc index 9fa95c198..f6756b5df 100644 --- a/cilium/tls_wrapper.cc +++ b/cilium/tls_wrapper.cc @@ -44,7 +44,7 @@ class SslSocketWrapper : public Network::TransportSocket, Logger::Loggable 0) { - if (!try_connect()) + if (!tryConnect()) { continue; // retry + } ssize_t sent = ::send(fd_, msg.data(), length, MSG_DONTWAIT | MSG_EOR | MSG_NOSIGNAL); if (sent == -1) { @@ -74,7 +75,7 @@ void UDSClient::Log(const std::string& msg) { fd_mutex_.unlock(); } -bool UDSClient::try_connect() { +bool UDSClient::tryConnect() { if (fd_ != -1) { if (errno_ == 0) { return true; diff --git a/cilium/uds_client.h b/cilium/uds_client.h index 0232d5d57..664899069 100644 --- a/cilium/uds_client.h +++ b/cilium/uds_client.h @@ -21,13 +21,13 @@ class UDSClient : Logger::Loggable { UDSClient(const std::string& path, TimeSource& time_source); ~UDSClient(); - void Log(const std::string& msg); + void log(const std::string& msg); const std::string& asString() const { return addr_->asString(); } absl::string_view asStringView() const { return addr_->asStringView(); } private: - bool try_connect() ABSL_EXCLUSIVE_LOCKS_REQUIRED(fd_mutex_); + bool tryConnect() ABSL_EXCLUSIVE_LOCKS_REQUIRED(fd_mutex_); Thread::MutexBasicLockable fd_mutex_; std::shared_ptr addr_; diff --git a/cilium/websocket.cc b/cilium/websocket.cc index 20e51cb10..65c3718c1 100644 --- a/cilium/websocket.cc +++ b/cilium/websocket.cc @@ -168,7 +168,7 @@ Network::FilterStatus Instance::onNewConnection() { destination_identity = 0; } // Initialize the log entry - log_entry_.InitFromConnection(pod_ip, proxy_id, is_ingress, identity, + log_entry_.initFromConnection(pod_ip, proxy_id, is_ingress, identity, callbacks_->connection().connectionInfoProvider().remoteAddress(), destination_identity, dst_address, &config_->time_source_); @@ -258,7 +258,7 @@ void Instance::onHandshakeRequest(const Http::RequestHeaderMap& headers) { } // Initialize the log entry - log_entry_.UpdateFromRequest(destination_identity, orig_dst_address, headers); + log_entry_.updateFromRequest(destination_identity, orig_dst_address, headers); } } // namespace WebSocket diff --git a/cilium/websocket.h b/cilium/websocket.h index 4d978f66a..ce02ee31a 100644 --- a/cilium/websocket.h +++ b/cilium/websocket.h @@ -38,24 +38,24 @@ class Instance : public Network::Filter, // WebSocket::CodecCallbacks const ConfigSharedPtr& config() override { return config_; } void onHandshakeCreated(const Http::RequestHeaderMap& headers) override { - log_entry_.UpdateFromRequest(0, nullptr, headers); + log_entry_.updateFromRequest(0, nullptr, headers); } - void onHandshakeSent() override { config_->Log(log_entry_, ::cilium::EntryType::Request); } + void onHandshakeSent() override { config_->log(log_entry_, ::cilium::EntryType::Request); } void onHandshakeRequest(const Http::RequestHeaderMap& headers) override; void onHandshakeResponse(const Http::ResponseHeaderMap& headers) override { - log_entry_.UpdateFromResponse(headers, config_->time_source_); - config_->Log(log_entry_, ::cilium::EntryType::Response); + log_entry_.updateFromResponse(headers, config_->time_source_); + config_->log(log_entry_, ::cilium::EntryType::Response); } void onHandshakeResponseSent(const Http::ResponseHeaderMap& headers) override { bool accepted = headers.Status() && headers.getStatusValue() == "101"; if (accepted) { - config_->Log(log_entry_, ::cilium::EntryType::Request); + config_->log(log_entry_, ::cilium::EntryType::Request); } else { - config_->Log(log_entry_, ::cilium::EntryType::Denied); + config_->log(log_entry_, ::cilium::EntryType::Denied); config_->stats_.access_denied_.inc(); } - log_entry_.UpdateFromResponse(headers, config_->time_source_); - config_->Log(log_entry_, ::cilium::EntryType::Response); + log_entry_.updateFromResponse(headers, config_->time_source_); + config_->log(log_entry_, ::cilium::EntryType::Response); } void injectEncoded(Buffer::Instance& data, bool end_stream) override; diff --git a/cilium/websocket_codec.cc b/cilium/websocket_codec.cc index 527f63f7c..9a56f7f13 100644 --- a/cilium/websocket_codec.cc +++ b/cilium/websocket_codec.cc @@ -102,7 +102,7 @@ class HttpParser : public Logger::Loggable { return message_complete_; } - bool versionIsHttp1_1() { + bool versionIsHttp11() { ENVOY_LOG(trace, "websocket: http_parser got version major: {} minor: {}", parser_.http_major, parser_.http_minor); return parser_.http_major == 1 && parser_.http_minor == 1; @@ -557,7 +557,7 @@ void Codec::decode(Buffer::Instance& data, bool end_stream) { const Http::ResponseHeaderMap& headers = parser.headers(); parent_->onHandshakeResponse(headers); - if (!parser.versionIsHttp1_1()) { + if (!parser.versionIsHttp11()) { config->stats_.handshake_invalid_http_version_.inc(); return closeOnError(handshake_buffer_, "unsupported HTTP protocol"); } @@ -603,7 +603,7 @@ void Codec::decode(Buffer::Instance& data, bool end_stream) { const Http::RequestHeaderMap& headers = parser.headers(); parent_->onHandshakeRequest(headers); - if (!parser.versionIsHttp1_1()) { + if (!parser.versionIsHttp11()) { config->stats_.handshake_invalid_http_version_.inc(); return closeOnError(handshake_buffer_, "unsupported HTTP protocol"); } diff --git a/cilium/websocket_config.cc b/cilium/websocket_config.cc index c23318df9..44936089a 100644 --- a/cilium/websocket_config.cc +++ b/cilium/websocket_config.cc @@ -72,7 +72,7 @@ Config::Config(Server::Configuration::FactoryContext& context, bool client, } if (!access_log_path.empty()) { - access_log_ = AccessLog::Open(access_log_path, time_source_); + access_log_ = AccessLog::open(access_log_path, time_source_); } const uint64_t timeout = DurationUtil::durationToMilliseconds(handshake_timeout); @@ -107,8 +107,8 @@ Config::Config(const ::cilium::WebSocketClient& config, } if (key_.empty()) { uint64_t random[2]; // 16 bytes - for (size_t i = 0; i < sizeof(random) / sizeof(random[0]); i++) { - random[i] = random_.random(); + for (unsigned long& i : random) { + i = random_.random(); } key_ = Base64::encode(reinterpret_cast(random), sizeof(random)); } @@ -128,9 +128,9 @@ std::string Config::keyResponse(absl::string_view key) { return Base64::encode(reinterpret_cast(sha1.data()), sha1.size()); } -void Config::Log(AccessLog::Entry& entry, ::cilium::EntryType type) { +void Config::log(AccessLog::Entry& entry, ::cilium::EntryType type) { if (access_log_) { - access_log_->Log(entry, type); + access_log_->log(entry, type); } } diff --git a/cilium/websocket_config.h b/cilium/websocket_config.h index fd27a755f..eddf5d258 100644 --- a/cilium/websocket_config.h +++ b/cilium/websocket_config.h @@ -68,7 +68,7 @@ class Config : public Logger::Loggable { static std::string keyResponse(absl::string_view key); - void Log(Cilium::AccessLog::Entry&, ::cilium::EntryType); + void log(Cilium::AccessLog::Entry&, ::cilium::EntryType); TimeSource& time_source_; Event::Dispatcher& dispatcher_; diff --git a/tests/accesslog_server.cc b/tests/accesslog_server.cc index 37d20bb4a..d82db94f1 100644 --- a/tests/accesslog_server.cc +++ b/tests/accesslog_server.cc @@ -18,7 +18,7 @@ namespace Envoy { AccessLogServer::AccessLogServer(const std::string path) : UDSServer(path, std::bind(&AccessLogServer::msgCallback, this, std::placeholders::_1)) {} -AccessLogServer::~AccessLogServer() {} +AccessLogServer::~AccessLogServer() = default; void AccessLogServer::clear() { absl::MutexLock lock(&mutex_); diff --git a/tests/accesslog_server.h b/tests/accesslog_server.h index 1aa33da39..67a51ab8c 100644 --- a/tests/accesslog_server.h +++ b/tests/accesslog_server.h @@ -27,24 +27,27 @@ class AccessLogServer : public UDSServer { template bool expectRequestTo(P&& pred, std::chrono::milliseconds timeout = TestUtility::DefaultTimeout) { auto maybe_entry = waitForMessage(::cilium::EntryType::Request, timeout); - if (maybe_entry.has_value()) + if (maybe_entry.has_value()) { return pred(maybe_entry.value()); + } return false; } template bool expectResponseTo(P&& pred, std::chrono::milliseconds timeout = TestUtility::DefaultTimeout) { auto maybe_entry = waitForMessage(::cilium::EntryType::Response, timeout); - if (maybe_entry.has_value()) + if (maybe_entry.has_value()) { return pred(maybe_entry.value()); + } return false; } template bool expectDeniedTo(P&& pred, std::chrono::milliseconds timeout = TestUtility::DefaultTimeout) { auto maybe_entry = waitForMessage(::cilium::EntryType::Denied, timeout); - if (maybe_entry.has_value()) + if (maybe_entry.has_value()) { return pred(maybe_entry.value()); + } return false; } diff --git a/tests/accesslog_test.cc b/tests/accesslog_test.cc index 63548cc14..bc7c5ffa5 100644 --- a/tests/accesslog_test.cc +++ b/tests/accesslog_test.cc @@ -38,7 +38,7 @@ TEST_F(CiliumTest, AccessLog) { AccessLog::Entry log; - log.InitFromRequest("1.2.3.4", 42, true, 1, source_address, 173, destination_address, + log.initFromRequest("1.2.3.4", 42, true, 1, source_address, 173, destination_address, connection.stream_info_, headers); EXPECT_EQ(log.entry_.is_ingress(), true); @@ -67,7 +67,7 @@ TEST_F(CiliumTest, AccessLog) { Http::TestResponseHeaderMapImpl response_headers{{"my-response-header", "response"}}; NiceMock time_source; - log.UpdateFromResponse(response_headers, time_source); + log.updateFromResponse(response_headers, time_source); // Unmodified EXPECT_EQ(log.entry_.has_http(), true); diff --git a/tests/bpf_metadata.cc b/tests/bpf_metadata.cc index f0143569b..93542a25e 100644 --- a/tests/bpf_metadata.cc +++ b/tests/bpf_metadata.cc @@ -80,8 +80,8 @@ createPolicyMap(const std::string& config, Server::Configuration::FactoryContext& context) { return context.serverFactoryContext().singletonManager().getTyped( "cilium_network_policy_singleton", [&config, &secret_configs, &context] { - if (secret_configs.size() > 0) { - for (auto sds_pair : secret_configs) { + if (!secret_configs.empty()) { + for (const auto& sds_pair : secret_configs) { auto& name = sds_pair.first; auto& sds_config = sds_pair.second; std::string sds_path = TestEnvironment::writeStringToFileForTest( @@ -190,9 +190,9 @@ TestConfig::extractSocketMetadata(Network::ConnectionSocket& socket) { policy.useProxylib(is_ingress_, port, is_ingress_ ? source_identity : destination_identity, l7proto); - return absl::optional(Cilium::BpfMetadata::SocketMetadata( + return {Cilium::BpfMetadata::SocketMetadata( 0, 0, source_identity, is_ingress_, is_l7lb_, port, std::move(pod_ip), nullptr, nullptr, - nullptr, original_dst_address, shared_from_this(), 0, std::move(l7proto), "")); + nullptr, original_dst_address, shared_from_this(), 0, std::move(l7proto), "")}; } } // namespace BpfMetadata diff --git a/tests/bpf_metadata.h b/tests/bpf_metadata.h index 710b8714b..e57b29f09 100644 --- a/tests/bpf_metadata.h +++ b/tests/bpf_metadata.h @@ -36,7 +36,7 @@ class TestConfig : public Config { public: TestConfig(const ::cilium::TestBpfMetadata& config, Server::Configuration::ListenerFactoryContext& context); - ~TestConfig(); + ~TestConfig() override; absl::optional extractSocketMetadata(Network::ConnectionSocket& socket) override; diff --git a/tests/cilium_http_integration.cc b/tests/cilium_http_integration.cc index 5d4263502..4f9de953e 100644 --- a/tests/cilium_http_integration.cc +++ b/tests/cilium_http_integration.cc @@ -33,7 +33,7 @@ CiliumHttpIntegrationTest::CiliumHttpIntegrationTest(const std::string& config) #endif } -CiliumHttpIntegrationTest::~CiliumHttpIntegrationTest() {} +CiliumHttpIntegrationTest::~CiliumHttpIntegrationTest() = default; void CiliumHttpIntegrationTest::createEnvoy() { // fake upstreams have been created by now, use the port from the 1st upstream diff --git a/tests/cilium_http_integration.h b/tests/cilium_http_integration.h index 00cb4182f..3ce1f1db7 100644 --- a/tests/cilium_http_integration.h +++ b/tests/cilium_http_integration.h @@ -26,7 +26,7 @@ class CiliumHttpIntegrationTest : public HttpIntegrationTest, public testing::TestWithParam { public: CiliumHttpIntegrationTest(const std::string& config); - ~CiliumHttpIntegrationTest(); + ~CiliumHttpIntegrationTest() override; void createEnvoy() override; @@ -63,8 +63,9 @@ class CiliumHttpIntegrationTest : public HttpIntegrationTest, getHeader(const Protobuf::RepeatedPtrField<::cilium::KeyValue>& headers, const std::string& name) { for (const auto& entry : headers) { - if (Http::LowerCaseString(entry.key()) == Http::LowerCaseString(name)) + if (Http::LowerCaseString(entry.key()) == Http::LowerCaseString(name)) { return entry.value(); + } } return absl::nullopt; } @@ -73,8 +74,9 @@ class CiliumHttpIntegrationTest : public HttpIntegrationTest, const std::string& name, const std::string& value = "") { for (const auto& entry : headers) { if (Http::LowerCaseString(entry.key()) == Http::LowerCaseString(name) && - (value == "" || entry.value() == value)) + (value.empty() || entry.value() == value)) { return true; + } } return false; } diff --git a/tests/cilium_http_integration_test.cc b/tests/cilium_http_integration_test.cc index b01f36ca8..1cd87c131 100644 --- a/tests/cilium_http_integration_test.cc +++ b/tests/cilium_http_integration_test.cc @@ -316,7 +316,7 @@ class CiliumIntegrationTest : public CiliumHttpIntegrationTest { } } - void Denied(Http::TestRequestHeaderMapImpl&& headers) { + void denied(Http::TestRequestHeaderMapImpl&& headers) { initialize(); codec_client_ = makeHttpConnection(lookupPort("http")); auto response = codec_client_->makeHeaderOnlyRequest(headers); @@ -345,7 +345,7 @@ class CiliumIntegrationTest : public CiliumHttpIntegrationTest { cleanupUpstreamAndDownstream(); } - void Accepted(Http::TestRequestHeaderMapImpl&& headers) { + void accepted(Http::TestRequestHeaderMapImpl&& headers) { initialize(); codec_client_ = makeHttpConnection(lookupPort("http")); auto response = sendRequestAndWaitForResponse(headers, 0, default_response_headers_, 0); @@ -390,7 +390,7 @@ class HostMapTest : public CiliumHttpIntegrationTest { std::string testPolicyFmt() override { return "version_info: \"0\""; } - void InvalidHostMap(const std::string& config, const char* exmsg) { + void invalidHostMap(const std::string& config, const char* exmsg) { std::string path = TestEnvironment::writeStringToFileForTest("host_map_fail.yaml", config); envoy::service::discovery::v3::DiscoveryResponse message; ThreadLocal::InstanceImpl tls; @@ -464,7 +464,7 @@ TEST_P(HostMapTest, HostMapValid) { TEST_P(HostMapTest, HostMapInvalidNonCIDRBits) { if (GetParam() == Network::Address::IpVersion::v4) { - InvalidHostMap(R"EOF(version_info: "0" + invalidHostMap(R"EOF(version_info: "0" resources: - "@type": type.googleapis.com/cilium.NetworkPolicyHosts policy: 11 @@ -472,7 +472,7 @@ TEST_P(HostMapTest, HostMapInvalidNonCIDRBits) { )EOF", "NetworkPolicyHosts: Non-prefix bits set in '127.0.0.1/31'"); } else { - InvalidHostMap(R"EOF(version_info: "0" + invalidHostMap(R"EOF(version_info: "0" resources: - "@type": type.googleapis.com/cilium.NetworkPolicyHosts policy: 11 @@ -484,7 +484,7 @@ TEST_P(HostMapTest, HostMapInvalidNonCIDRBits) { TEST_P(HostMapTest, HostMapInvalidPrefixLengths) { if (GetParam() == Network::Address::IpVersion::v4) { - InvalidHostMap( + invalidHostMap( R"EOF(version_info: "0" resources: - "@type": type.googleapis.com/cilium.NetworkPolicyHosts @@ -493,7 +493,7 @@ TEST_P(HostMapTest, HostMapInvalidPrefixLengths) { )EOF", "NetworkPolicyHosts: Invalid prefix length in '127.0.0.1/33'"); } else { - InvalidHostMap(R"EOF(version_info: "0" + invalidHostMap(R"EOF(version_info: "0" resources: - "@type": type.googleapis.com/cilium.NetworkPolicyHosts policy: 11 @@ -505,7 +505,7 @@ TEST_P(HostMapTest, HostMapInvalidPrefixLengths) { TEST_P(HostMapTest, HostMapInvalidPrefixLengths2) { if (GetParam() == Network::Address::IpVersion::v4) { - InvalidHostMap( + invalidHostMap( R"EOF(version_info: "0" resources: - "@type": type.googleapis.com/cilium.NetworkPolicyHosts @@ -514,7 +514,7 @@ TEST_P(HostMapTest, HostMapInvalidPrefixLengths2) { )EOF", "NetworkPolicyHosts: Invalid prefix length in '127.0.0.1/32a'"); } else { - InvalidHostMap(R"EOF(version_info: "0" + invalidHostMap(R"EOF(version_info: "0" resources: - "@type": type.googleapis.com/cilium.NetworkPolicyHosts policy: 11 @@ -526,7 +526,7 @@ TEST_P(HostMapTest, HostMapInvalidPrefixLengths2) { TEST_P(HostMapTest, HostMapInvalidPrefixLengths3) { if (GetParam() == Network::Address::IpVersion::v4) { - InvalidHostMap( + invalidHostMap( R"EOF(version_info: "0" resources: - "@type": type.googleapis.com/cilium.NetworkPolicyHosts @@ -535,7 +535,7 @@ TEST_P(HostMapTest, HostMapInvalidPrefixLengths3) { )EOF", "NetworkPolicyHosts: Invalid prefix length in '127.0.0.1/ 32'"); } else { - InvalidHostMap(R"EOF(version_info: "0" + invalidHostMap(R"EOF(version_info: "0" resources: - "@type": type.googleapis.com/cilium.NetworkPolicyHosts policy: 11 @@ -547,7 +547,7 @@ TEST_P(HostMapTest, HostMapInvalidPrefixLengths3) { TEST_P(HostMapTest, HostMapDuplicateEntry) { if (GetParam() == Network::Address::IpVersion::v4) { - InvalidHostMap(R"EOF(version_info: "0" + invalidHostMap(R"EOF(version_info: "0" resources: - "@type": type.googleapis.com/cilium.NetworkPolicyHosts policy: 11 @@ -556,7 +556,7 @@ TEST_P(HostMapTest, HostMapDuplicateEntry) { "NetworkPolicyHosts: Duplicate host entry '127.0.0.1' for " "policy 11, already mapped to 11"); } else { - InvalidHostMap(R"EOF(version_info: "0" + invalidHostMap(R"EOF(version_info: "0" resources: - "@type": type.googleapis.com/cilium.NetworkPolicyHosts policy: 11 @@ -569,7 +569,7 @@ TEST_P(HostMapTest, HostMapDuplicateEntry) { TEST_P(HostMapTest, HostMapDuplicateEntry2) { if (GetParam() == Network::Address::IpVersion::v4) { - InvalidHostMap(R"EOF(version_info: "0" + invalidHostMap(R"EOF(version_info: "0" resources: - "@type": type.googleapis.com/cilium.NetworkPolicyHosts policy: 11 @@ -581,7 +581,7 @@ TEST_P(HostMapTest, HostMapDuplicateEntry2) { "NetworkPolicyHosts: Duplicate host entry '127.0.0.1' for " "policy 12, already mapped to 11"); } else { - InvalidHostMap(R"EOF(version_info: "0" + invalidHostMap(R"EOF(version_info: "0" resources: - "@type": type.googleapis.com/cilium.NetworkPolicyHosts policy: 11 @@ -597,7 +597,7 @@ TEST_P(HostMapTest, HostMapDuplicateEntry2) { TEST_P(HostMapTest, HostMapInvalidAddress) { if (GetParam() == Network::Address::IpVersion::v4) { - InvalidHostMap( + invalidHostMap( R"EOF(version_info: "0" resources: - "@type": type.googleapis.com/cilium.NetworkPolicyHosts @@ -606,7 +606,7 @@ TEST_P(HostMapTest, HostMapInvalidAddress) { )EOF", "NetworkPolicyHosts: Invalid host entry '255.256.0.0' for policy 11"); } else { - InvalidHostMap( + invalidHostMap( R"EOF(version_info: "0" resources: - "@type": type.googleapis.com/cilium.NetworkPolicyHosts @@ -619,7 +619,7 @@ TEST_P(HostMapTest, HostMapInvalidAddress) { TEST_P(HostMapTest, HostMapInvalidAddress2) { if (GetParam() == Network::Address::IpVersion::v4) { - InvalidHostMap( + invalidHostMap( R"EOF(version_info: "0" resources: - "@type": type.googleapis.com/cilium.NetworkPolicyHosts @@ -628,7 +628,7 @@ TEST_P(HostMapTest, HostMapInvalidAddress2) { )EOF", "NetworkPolicyHosts: Invalid host entry '255.255.0.0 ' for policy 11"); } else { - InvalidHostMap( + invalidHostMap( R"EOF(version_info: "0" resources: - "@type": type.googleapis.com/cilium.NetworkPolicyHosts @@ -641,7 +641,7 @@ TEST_P(HostMapTest, HostMapInvalidAddress2) { TEST_P(HostMapTest, HostMapInvalidDefaults) { if (GetParam() == Network::Address::IpVersion::v4) { - InvalidHostMap(R"EOF(version_info: "0" + invalidHostMap(R"EOF(version_info: "0" resources: - "@type": type.googleapis.com/cilium.NetworkPolicyHosts policy: 11 @@ -649,7 +649,7 @@ TEST_P(HostMapTest, HostMapInvalidDefaults) { )EOF", "NetworkPolicyHosts: Non-prefix bits set in '128.0.0.0/0'"); } else { - InvalidHostMap(R"EOF(version_info: "0" + invalidHostMap(R"EOF(version_info: "0" resources: - "@type": type.googleapis.com/cilium.NetworkPolicyHosts policy: 11 @@ -660,7 +660,7 @@ TEST_P(HostMapTest, HostMapInvalidDefaults) { } TEST_P(CiliumIntegrationTest, DeniedPathPrefix) { - Denied({{":method", "GET"}, {":path", "/prefix"}, {":authority", "host"}}); + denied({{":method", "GET"}, {":path", "/prefix"}, {":authority", "host"}}); // Validate that missing headers are access logged correctly EXPECT_TRUE(expectAccessLogDeniedTo([](const ::cilium::LogEntry& entry) { @@ -670,7 +670,7 @@ TEST_P(CiliumIntegrationTest, DeniedPathPrefix) { } TEST_P(CiliumIntegrationTest, AllowedPathPrefix) { - Accepted({{":method", "GET"}, + accepted({{":method", "GET"}, {":path", "/allowed"}, {":authority", "host"}, {"bearer-token", "d4ef0f5011f163ac"}}); @@ -685,7 +685,7 @@ TEST_P(CiliumIntegrationTest, AllowedPathPrefix) { } TEST_P(CiliumIntegrationTest, AllowedPathPrefixWrongHeader) { - Accepted({{":method", "GET"}, + accepted({{":method", "GET"}, {":path", "/allowed"}, {":authority", "host"}, {"bearer-token", "wrong-value"}, @@ -707,7 +707,7 @@ TEST_P(CiliumIntegrationTest, AllowedPathPrefixWrongHeader) { TEST_P(CiliumIntegrationTest, MultipleRequests) { // 1st request - Accepted({{":method", "GET"}, + accepted({{":method", "GET"}, {":path", "/allowed"}, {":authority", "host"}, {"bearer-token", "d4ef0f5011f163ac"}}); @@ -721,7 +721,7 @@ TEST_P(CiliumIntegrationTest, MultipleRequests) { })); // 2nd request - Accepted({{":method", "GET"}, + accepted({{":method", "GET"}, {":path", "/allowed"}, {":authority", "host"}, {"bearer-token", "wrong-value"}, @@ -742,7 +742,7 @@ TEST_P(CiliumIntegrationTest, MultipleRequests) { } TEST_P(CiliumIntegrationTest, AllowedPathRegex) { - Accepted({{":method", "GET"}, {":path", "/maybe/public"}, {":authority", "host"}}); + accepted({{":method", "GET"}, {":path", "/maybe/public"}, {":authority", "host"}}); // Validate that missing headers are access logged correctly EXPECT_TRUE(expectAccessLogRequestTo([](const ::cilium::LogEntry& entry) { @@ -752,7 +752,7 @@ TEST_P(CiliumIntegrationTest, AllowedPathRegex) { } TEST_P(CiliumIntegrationTest, AllowedPathRegexDeleteHeader) { - Accepted({{":method", "GET"}, + accepted({{":method", "GET"}, {":path", "/maybe/public"}, {":authority", "host"}, {"User-Agent", "test"}}); @@ -767,7 +767,7 @@ TEST_P(CiliumIntegrationTest, AllowedPathRegexDeleteHeader) { } TEST_P(CiliumIntegrationTest, AllowedHostRegexDeleteHeader) { - Accepted({{":method", "GET"}, + accepted({{":method", "GET"}, {":path", "/maybe/private"}, {":authority", "hostREGEXname"}, {"header42", "test"}}); @@ -783,7 +783,7 @@ TEST_P(CiliumIntegrationTest, AllowedHostRegexDeleteHeader) { } TEST_P(CiliumIntegrationTest, DeniedPath) { - Denied({{":method", "GET"}, {":path", "/maybe/private"}, {":authority", "host"}}); + denied({{":method", "GET"}, {":path", "/maybe/private"}, {":authority", "host"}}); // Validate that missing headers are access logged correctly EXPECT_TRUE(expectAccessLogDeniedTo([](const ::cilium::LogEntry& entry) { @@ -793,7 +793,7 @@ TEST_P(CiliumIntegrationTest, DeniedPath) { } TEST_P(CiliumIntegrationTest, AllowedHostString) { - Accepted({{":method", "GET"}, {":path", "/maybe/private"}, {":authority", "allowedHOST"}}); + accepted({{":method", "GET"}, {":path", "/maybe/private"}, {":authority", "allowedHOST"}}); // Validate that missing headers are access logged correctly EXPECT_TRUE(expectAccessLogRequestTo([](const ::cilium::LogEntry& entry) { @@ -806,7 +806,7 @@ TEST_P(CiliumIntegrationTest, AllowedHostString) { } TEST_P(CiliumIntegrationTest, AllowedReplaced) { - Accepted({{":method", "GET"}, {":path", "/allowed"}, {":authority", "allowedHOST"}}); + accepted({{":method", "GET"}, {":path", "/allowed"}, {":authority", "allowedHOST"}}); // Validate that missing headers are access logged correctly EXPECT_TRUE(expectAccessLogRequestTo([](const ::cilium::LogEntry& entry) { @@ -821,7 +821,7 @@ TEST_P(CiliumIntegrationTest, AllowedReplaced) { } TEST_P(CiliumIntegrationTest, Denied42) { - Denied({{":method", "GET"}, + denied({{":method", "GET"}, {":path", "/allowed"}, {":authority", "host"}, {"header42", "anything"}}); @@ -839,7 +839,7 @@ TEST_P(CiliumIntegrationTest, Denied42) { } TEST_P(CiliumIntegrationTest, AllowedReplacedAndDeleted) { - Accepted({{":method", "GET"}, + accepted({{":method", "GET"}, {":path", "/allowed"}, {":authority", "allowedHOST"}, {"header42", "anything"}}); @@ -858,7 +858,7 @@ TEST_P(CiliumIntegrationTest, AllowedReplacedAndDeleted) { } TEST_P(CiliumIntegrationTest, AllowedHostRegex) { - Accepted({{":method", "GET"}, {":path", "/maybe/private"}, {":authority", "hostREGEXname"}}); + accepted({{":method", "GET"}, {":path", "/maybe/private"}, {":authority", "hostREGEXname"}}); // Validate that missing headers are access logged correctly EXPECT_TRUE(expectAccessLogRequestTo([](const ::cilium::LogEntry& entry) { @@ -868,7 +868,7 @@ TEST_P(CiliumIntegrationTest, AllowedHostRegex) { } TEST_P(CiliumIntegrationTest, DeniedMethod) { - Denied({{":method", "POST"}, {":path", "/maybe/private"}, {":authority", "host"}}); + denied({{":method", "POST"}, {":path", "/maybe/private"}, {":authority", "host"}}); // Validate that missing headers are access logged correctly EXPECT_TRUE(expectAccessLogDeniedTo([](const ::cilium::LogEntry& entry) { @@ -878,7 +878,7 @@ TEST_P(CiliumIntegrationTest, DeniedMethod) { } TEST_P(CiliumIntegrationTest, AcceptedMethod) { - Accepted({{":method", "PUT"}, {":path", "/public/opinions"}, {":authority", "host"}}); + accepted({{":method", "PUT"}, {":path", "/public/opinions"}, {":authority", "host"}}); // Validate that missing headers are access logged correctly EXPECT_TRUE(expectAccessLogRequestTo([](const ::cilium::LogEntry& entry) { @@ -888,7 +888,7 @@ TEST_P(CiliumIntegrationTest, AcceptedMethod) { } TEST_P(CiliumIntegrationTest, L3DeniedPath) { - Denied({{":method", "GET"}, {":path", "/only-2-allowed"}, {":authority", "host"}}); + denied({{":method", "GET"}, {":path", "/only-2-allowed"}, {":authority", "host"}}); // Validate that missing headers are access logged correctly EXPECT_TRUE(expectAccessLogDeniedTo([](const ::cilium::LogEntry& entry) { @@ -899,9 +899,9 @@ TEST_P(CiliumIntegrationTest, L3DeniedPath) { class CiliumIntegrationPortTest : public CiliumIntegrationTest { public: - CiliumIntegrationPortTest() : CiliumIntegrationTest() {} + CiliumIntegrationPortTest() = default; - std::string testPolicyFmt() { + std::string testPolicyFmt() override { return TestEnvironment::substitute(R"EOF(version_info: "0" resources: - "@type": type.googleapis.com/cilium.NetworkPolicy @@ -945,18 +945,18 @@ INSTANTIATE_TEST_SUITE_P(IpVersions, CiliumIntegrationPortTest, testing::ValuesIn(TestEnvironment::getIpVersionsForTest())); TEST_P(CiliumIntegrationPortTest, DuplicatePortAllowedPath) { - Accepted({{":method", "GET"}, {":path", "/only-2-allowed"}, {":authority", "host"}}); + accepted({{":method", "GET"}, {":path", "/only-2-allowed"}, {":authority", "host"}}); } TEST_P(CiliumIntegrationPortTest, DuplicatePortAllowedPath2) { - Accepted({{":method", "GET"}, {":path", "/also-2-allowed"}, {":authority", "host"}}); + accepted({{":method", "GET"}, {":path", "/also-2-allowed"}, {":authority", "host"}}); } class CiliumIntegrationPortRangeTest : public CiliumIntegrationTest { public: - CiliumIntegrationPortRangeTest() : CiliumIntegrationTest() {} + CiliumIntegrationPortRangeTest() = default; - std::string testPolicyFmt() { + std::string testPolicyFmt() override { return TestEnvironment::substitute(BASIC_POLICY_fmt + R"EOF( - end_port: {0} rules: - remote_policies: [ 2 ] @@ -1013,39 +1013,39 @@ INSTANTIATE_TEST_SUITE_P(IpVersions, CiliumIntegrationEgressTest, testing::ValuesIn(TestEnvironment::getIpVersionsForTest())); TEST_P(CiliumIntegrationEgressTest, DeniedPathPrefix) { - Denied({{":method", "GET"}, {":path", "/prefix"}, {":authority", "host"}}); + denied({{":method", "GET"}, {":path", "/prefix"}, {":authority", "host"}}); } TEST_P(CiliumIntegrationEgressTest, AllowedPathPrefix) { - Accepted({{":method", "GET"}, {":path", "/allowed"}, {":authority", "host"}}); + accepted({{":method", "GET"}, {":path", "/allowed"}, {":authority", "host"}}); } TEST_P(CiliumIntegrationEgressTest, AllowedPathRegex) { - Accepted({{":method", "GET"}, {":path", "/maybe/public"}, {":authority", "host"}}); + accepted({{":method", "GET"}, {":path", "/maybe/public"}, {":authority", "host"}}); } TEST_P(CiliumIntegrationEgressTest, DeniedPath) { - Denied({{":method", "GET"}, {":path", "/maybe/private"}, {":authority", "host"}}); + denied({{":method", "GET"}, {":path", "/maybe/private"}, {":authority", "host"}}); } TEST_P(CiliumIntegrationEgressTest, AllowedHostString) { - Accepted({{":method", "GET"}, {":path", "/maybe/private"}, {":authority", "allowedHOST"}}); + accepted({{":method", "GET"}, {":path", "/maybe/private"}, {":authority", "allowedHOST"}}); } TEST_P(CiliumIntegrationEgressTest, AllowedHostRegex) { - Accepted({{":method", "GET"}, {":path", "/maybe/private"}, {":authority", "hostREGEXname"}}); + accepted({{":method", "GET"}, {":path", "/maybe/private"}, {":authority", "hostREGEXname"}}); } TEST_P(CiliumIntegrationEgressTest, DeniedMethod) { - Denied({{":method", "POST"}, {":path", "/maybe/private"}, {":authority", "host"}}); + denied({{":method", "POST"}, {":path", "/maybe/private"}, {":authority", "host"}}); } TEST_P(CiliumIntegrationEgressTest, AcceptedMethod) { - Accepted({{":method", "PUT"}, {":path", "/public/opinions"}, {":authority", "host"}}); + accepted({{":method", "PUT"}, {":path", "/public/opinions"}, {":authority", "host"}}); } TEST_P(CiliumIntegrationEgressTest, L3DeniedPath) { - Denied({{":method", "GET"}, {":path", "/only-2-allowed"}, {":authority", "host"}}); + denied({{":method", "GET"}, {":path", "/only-2-allowed"}, {":authority", "host"}}); } const std::string L34_POLICY_fmt = R"EOF(version_info: "0" @@ -1063,20 +1063,22 @@ const std::string L34_POLICY_fmt = R"EOF(version_info: "0" class CiliumIntegrationEgressL34Test : public CiliumIntegrationEgressTest { public: - CiliumIntegrationEgressL34Test() {} + CiliumIntegrationEgressL34Test() = default; - std::string testPolicyFmt() { return TestEnvironment::substitute(L34_POLICY_fmt, GetParam()); } + std::string testPolicyFmt() override { + return TestEnvironment::substitute(L34_POLICY_fmt, GetParam()); + } }; INSTANTIATE_TEST_SUITE_P(IpVersions, CiliumIntegrationEgressL34Test, testing::ValuesIn(TestEnvironment::getIpVersionsForTest())); TEST_P(CiliumIntegrationEgressL34Test, DeniedPathPrefix) { - Denied({{":method", "GET"}, {":path", "/prefix"}, {":authority", "host"}}); + denied({{":method", "GET"}, {":path", "/prefix"}, {":authority", "host"}}); } TEST_P(CiliumIntegrationEgressL34Test, DeniedPathPrefix2) { - Denied({{":method", "GET"}, {":path", "/allowed"}, {":authority", "host"}}); + denied({{":method", "GET"}, {":path", "/allowed"}, {":authority", "host"}}); } const std::string HEADER_ACTION_MISSING_SDS_POLICY_fmt = R"EOF(version_info: "1" @@ -1128,7 +1130,7 @@ const std::string HEADER_ACTION_MISSING_SDS_POLICY2_fmt = R"EOF(version_info: "2 class SDSIntegrationTest : public CiliumIntegrationTest { public: - SDSIntegrationTest() : CiliumIntegrationTest() { + SDSIntegrationTest() { // switch back to SDS secrets so that we can test with a missing secret. // File based secret fails if the file does not exist, while SDS should allow for secret to be // created in future. @@ -1165,7 +1167,7 @@ INSTANTIATE_TEST_SUITE_P(IpVersions, SDSIntegrationTest, testing::ValuesIn(TestEnvironment::getIpVersionsForTest())); TEST_P(SDSIntegrationTest, TestDeniedL3) { - Denied({{":method", "GET"}, {":path", "/only42"}, {":authority", "host"}}); + denied({{":method", "GET"}, {":path", "/only42"}, {":authority", "host"}}); // Validate that missing headers are access logged correctly EXPECT_TRUE(expectAccessLogDeniedTo([](const ::cilium::LogEntry& entry) { @@ -1182,7 +1184,7 @@ TEST_P(SDSIntegrationTest, TestDeniedL3) { } TEST_P(SDSIntegrationTest, TestDeniedL3SpoofedXFF) { - Denied({{":method", "GET"}, + denied({{":method", "GET"}, {":path", "/only42"}, {":authority", "host"}, {"x-forwarded-for", "192.168.1.1"}}); @@ -1203,7 +1205,7 @@ TEST_P(SDSIntegrationTest, TestDeniedL3SpoofedXFF) { } TEST_P(SDSIntegrationTest, TestMissingSDSSecretOnUpdate) { - Accepted({{":method", "GET"}, {":path", "/allowed2"}, {":authority", "host"}}); + accepted({{":method", "GET"}, {":path", "/allowed2"}, {":authority", "host"}}); // Validate that missing headers are access logged correctly EXPECT_TRUE(expectAccessLogRequestTo([](const ::cilium::LogEntry& entry) { @@ -1231,7 +1233,7 @@ TEST_P(SDSIntegrationTest, TestMissingSDSSecretOnUpdate) { absl::SleepFor(absl::Milliseconds(100)); // 2nd round, on updated policy - Denied({{":method", "GET"}, {":path", "/allowed"}, {":authority", "host"}}); + denied({{":method", "GET"}, {":path", "/allowed"}, {":authority", "host"}}); // Validate that missing headers are access logged correctly EXPECT_TRUE(expectAccessLogDeniedTo([](const ::cilium::LogEntry& entry) { @@ -1254,7 +1256,7 @@ TEST_P(SDSIntegrationTest, TestMissingSDSSecretOnUpdate) { // Reduce flakiness by allowing some time for the policy to be updated before the following test absl::SleepFor(absl::Milliseconds(100)); - Denied({{":method", "GET"}, {":path", "/allowed"}, {":authority", "host"}}); + denied({{":method", "GET"}, {":path", "/allowed"}, {":authority", "host"}}); // Validate that missing headers are access logged correctly EXPECT_TRUE(expectAccessLogDeniedTo([](const ::cilium::LogEntry& entry) { diff --git a/tests/cilium_http_upstream_integration_test.cc b/tests/cilium_http_upstream_integration_test.cc index 8bcebbd15..c93931c28 100644 --- a/tests/cilium_http_upstream_integration_test.cc +++ b/tests/cilium_http_upstream_integration_test.cc @@ -353,7 +353,7 @@ class CiliumIntegrationTest : public CiliumHttpIntegrationTest { } } - void Denied(Http::TestRequestHeaderMapImpl&& headers) { + void denied(Http::TestRequestHeaderMapImpl&& headers) { initialize(); codec_client_ = makeHttpConnection(lookupPort("http")); auto response = codec_client_->makeHeaderOnlyRequest(headers); @@ -382,7 +382,7 @@ class CiliumIntegrationTest : public CiliumHttpIntegrationTest { cleanupUpstreamAndDownstream(); } - void Accepted(Http::TestRequestHeaderMapImpl&& headers) { + void accepted(Http::TestRequestHeaderMapImpl&& headers) { initialize(); codec_client_ = makeHttpConnection(lookupPort("http")); auto response = sendRequestAndWaitForResponse(headers, 0, default_response_headers_, 0); @@ -419,7 +419,7 @@ INSTANTIATE_TEST_SUITE_P(IpVersions, CiliumIntegrationTest, testing::ValuesIn(TestEnvironment::getIpVersionsForTest())); TEST_P(CiliumIntegrationTest, DeniedPathPrefix) { - Denied({{":method", "GET"}, {":path", "/prefix"}, {":authority", "host"}}); + denied({{":method", "GET"}, {":path", "/prefix"}, {":authority", "host"}}); // Validate that missing headers are access logged correctly EXPECT_TRUE(expectAccessLogDeniedTo([](const ::cilium::LogEntry& entry) { @@ -429,7 +429,7 @@ TEST_P(CiliumIntegrationTest, DeniedPathPrefix) { } TEST_P(CiliumIntegrationTest, AllowedPathPrefix) { - Accepted({{":method", "GET"}, + accepted({{":method", "GET"}, {":path", "/allowed"}, {":authority", "host"}, {"bearer-token", "d4ef0f5011f163ac"}}); @@ -444,7 +444,7 @@ TEST_P(CiliumIntegrationTest, AllowedPathPrefix) { } TEST_P(CiliumIntegrationTest, AllowedPathPrefixWrongHeader) { - Accepted({{":method", "GET"}, + accepted({{":method", "GET"}, {":path", "/allowed"}, {":authority", "host"}, {"bearer-token", "wrong-value"}, @@ -466,7 +466,7 @@ TEST_P(CiliumIntegrationTest, AllowedPathPrefixWrongHeader) { TEST_P(CiliumIntegrationTest, MultipleRequests) { // 1st request - Accepted({{":method", "GET"}, + accepted({{":method", "GET"}, {":path", "/allowed"}, {":authority", "host"}, {"bearer-token", "d4ef0f5011f163ac"}}); @@ -480,7 +480,7 @@ TEST_P(CiliumIntegrationTest, MultipleRequests) { })); // 2nd request - Accepted({{":method", "GET"}, + accepted({{":method", "GET"}, {":path", "/allowed"}, {":authority", "host"}, {"bearer-token", "wrong-value"}, @@ -501,7 +501,7 @@ TEST_P(CiliumIntegrationTest, MultipleRequests) { } TEST_P(CiliumIntegrationTest, AllowedPathRegex) { - Accepted({{":method", "GET"}, {":path", "/maybe/public"}, {":authority", "host"}}); + accepted({{":method", "GET"}, {":path", "/maybe/public"}, {":authority", "host"}}); // Validate that missing headers are access logged correctly EXPECT_TRUE(expectAccessLogRequestTo([](const ::cilium::LogEntry& entry) { @@ -511,7 +511,7 @@ TEST_P(CiliumIntegrationTest, AllowedPathRegex) { } TEST_P(CiliumIntegrationTest, AllowedPathRegexDeleteHeader) { - Accepted({{":method", "GET"}, + accepted({{":method", "GET"}, {":path", "/maybe/public"}, {":authority", "host"}, {"User-Agent", "test"}}); @@ -526,7 +526,7 @@ TEST_P(CiliumIntegrationTest, AllowedPathRegexDeleteHeader) { } TEST_P(CiliumIntegrationTest, AllowedHostRegexDeleteHeader) { - Accepted({{":method", "GET"}, + accepted({{":method", "GET"}, {":path", "/maybe/private"}, {":authority", "hostREGEXname"}, {"header42", "test"}}); @@ -542,7 +542,7 @@ TEST_P(CiliumIntegrationTest, AllowedHostRegexDeleteHeader) { } TEST_P(CiliumIntegrationTest, DeniedPath) { - Denied({{":method", "GET"}, {":path", "/maybe/private"}, {":authority", "host"}}); + denied({{":method", "GET"}, {":path", "/maybe/private"}, {":authority", "host"}}); // Validate that missing headers are access logged correctly EXPECT_TRUE(expectAccessLogDeniedTo([](const ::cilium::LogEntry& entry) { @@ -552,7 +552,7 @@ TEST_P(CiliumIntegrationTest, DeniedPath) { } TEST_P(CiliumIntegrationTest, AllowedHostString) { - Accepted({{":method", "GET"}, {":path", "/maybe/private"}, {":authority", "allowedHOST"}}); + accepted({{":method", "GET"}, {":path", "/maybe/private"}, {":authority", "allowedHOST"}}); // Validate that missing headers are access logged correctly EXPECT_TRUE(expectAccessLogRequestTo([](const ::cilium::LogEntry& entry) { @@ -565,7 +565,7 @@ TEST_P(CiliumIntegrationTest, AllowedHostString) { } TEST_P(CiliumIntegrationTest, AllowedReplaced) { - Accepted({{":method", "GET"}, {":path", "/allowed"}, {":authority", "allowedHOST"}}); + accepted({{":method", "GET"}, {":path", "/allowed"}, {":authority", "allowedHOST"}}); // Validate that missing headers are access logged correctly EXPECT_TRUE(expectAccessLogRequestTo([](const ::cilium::LogEntry& entry) { @@ -580,7 +580,7 @@ TEST_P(CiliumIntegrationTest, AllowedReplaced) { } TEST_P(CiliumIntegrationTest, Denied42) { - Denied({{":method", "GET"}, + denied({{":method", "GET"}, {":path", "/allowed"}, {":authority", "host"}, {"header42", "anything"}}); @@ -598,7 +598,7 @@ TEST_P(CiliumIntegrationTest, Denied42) { } TEST_P(CiliumIntegrationTest, AllowedReplacedAndDeleted) { - Accepted({{":method", "GET"}, + accepted({{":method", "GET"}, {":path", "/allowed"}, {":authority", "allowedHOST"}, {"header42", "anything"}}); @@ -617,7 +617,7 @@ TEST_P(CiliumIntegrationTest, AllowedReplacedAndDeleted) { } TEST_P(CiliumIntegrationTest, AllowedHostRegex) { - Accepted({{":method", "GET"}, {":path", "/maybe/private"}, {":authority", "hostREGEXname"}}); + accepted({{":method", "GET"}, {":path", "/maybe/private"}, {":authority", "hostREGEXname"}}); // Validate that missing headers are access logged correctly EXPECT_TRUE(expectAccessLogRequestTo([](const ::cilium::LogEntry& entry) { @@ -627,7 +627,7 @@ TEST_P(CiliumIntegrationTest, AllowedHostRegex) { } TEST_P(CiliumIntegrationTest, DeniedMethod) { - Denied({{":method", "POST"}, {":path", "/maybe/private"}, {":authority", "host"}}); + denied({{":method", "POST"}, {":path", "/maybe/private"}, {":authority", "host"}}); // Validate that missing headers are access logged correctly EXPECT_TRUE(expectAccessLogDeniedTo([](const ::cilium::LogEntry& entry) { @@ -637,7 +637,7 @@ TEST_P(CiliumIntegrationTest, DeniedMethod) { } TEST_P(CiliumIntegrationTest, AcceptedMethod) { - Accepted({{":method", "PUT"}, {":path", "/public/opinions"}, {":authority", "host"}}); + accepted({{":method", "PUT"}, {":path", "/public/opinions"}, {":authority", "host"}}); // Validate that missing headers are access logged correctly EXPECT_TRUE(expectAccessLogRequestTo([](const ::cilium::LogEntry& entry) { @@ -647,7 +647,7 @@ TEST_P(CiliumIntegrationTest, AcceptedMethod) { } TEST_P(CiliumIntegrationTest, L3DeniedPath) { - Denied({{":method", "GET"}, {":path", "/only-2-allowed"}, {":authority", "host"}}); + denied({{":method", "GET"}, {":path", "/only-2-allowed"}, {":authority", "host"}}); // Validate that missing headers are access logged correctly EXPECT_TRUE(expectAccessLogDeniedTo([](const ::cilium::LogEntry& entry) { @@ -678,39 +678,39 @@ INSTANTIATE_TEST_SUITE_P(IpVersions, CiliumIntegrationEgressTest, testing::ValuesIn(TestEnvironment::getIpVersionsForTest())); TEST_P(CiliumIntegrationEgressTest, DeniedPathPrefix) { - Denied({{":method", "GET"}, {":path", "/prefix"}, {":authority", "host"}}); + denied({{":method", "GET"}, {":path", "/prefix"}, {":authority", "host"}}); } TEST_P(CiliumIntegrationEgressTest, AllowedPathPrefix) { - Accepted({{":method", "GET"}, {":path", "/allowed"}, {":authority", "host"}}); + accepted({{":method", "GET"}, {":path", "/allowed"}, {":authority", "host"}}); } TEST_P(CiliumIntegrationEgressTest, AllowedPathRegex) { - Accepted({{":method", "GET"}, {":path", "/maybe/public"}, {":authority", "host"}}); + accepted({{":method", "GET"}, {":path", "/maybe/public"}, {":authority", "host"}}); } TEST_P(CiliumIntegrationEgressTest, DeniedPath) { - Denied({{":method", "GET"}, {":path", "/maybe/private"}, {":authority", "host"}}); + denied({{":method", "GET"}, {":path", "/maybe/private"}, {":authority", "host"}}); } TEST_P(CiliumIntegrationEgressTest, AllowedHostString) { - Accepted({{":method", "GET"}, {":path", "/maybe/private"}, {":authority", "allowedHOST"}}); + accepted({{":method", "GET"}, {":path", "/maybe/private"}, {":authority", "allowedHOST"}}); } TEST_P(CiliumIntegrationEgressTest, AllowedHostRegex) { - Accepted({{":method", "GET"}, {":path", "/maybe/private"}, {":authority", "hostREGEXname"}}); + accepted({{":method", "GET"}, {":path", "/maybe/private"}, {":authority", "hostREGEXname"}}); } TEST_P(CiliumIntegrationEgressTest, DeniedMethod) { - Denied({{":method", "POST"}, {":path", "/maybe/private"}, {":authority", "host"}}); + denied({{":method", "POST"}, {":path", "/maybe/private"}, {":authority", "host"}}); } TEST_P(CiliumIntegrationEgressTest, AcceptedMethod) { - Accepted({{":method", "PUT"}, {":path", "/public/opinions"}, {":authority", "host"}}); + accepted({{":method", "PUT"}, {":path", "/public/opinions"}, {":authority", "host"}}); } TEST_P(CiliumIntegrationEgressTest, L3DeniedPath) { - Denied({{":method", "GET"}, {":path", "/only-2-allowed"}, {":authority", "host"}}); + denied({{":method", "GET"}, {":path", "/only-2-allowed"}, {":authority", "host"}}); } const std::string L34_POLICY_fmt = R"EOF(version_info: "0" @@ -728,20 +728,22 @@ const std::string L34_POLICY_fmt = R"EOF(version_info: "0" class CiliumIntegrationEgressL34Test : public CiliumIntegrationEgressTest { public: - CiliumIntegrationEgressL34Test() {} + CiliumIntegrationEgressL34Test() = default; - std::string testPolicyFmt() { return TestEnvironment::substitute(L34_POLICY_fmt, GetParam()); } + std::string testPolicyFmt() override { + return TestEnvironment::substitute(L34_POLICY_fmt, GetParam()); + } }; INSTANTIATE_TEST_SUITE_P(IpVersions, CiliumIntegrationEgressL34Test, testing::ValuesIn(TestEnvironment::getIpVersionsForTest())); TEST_P(CiliumIntegrationEgressL34Test, DeniedPathPrefix) { - Denied({{":method", "GET"}, {":path", "/prefix"}, {":authority", "host"}}); + denied({{":method", "GET"}, {":path", "/prefix"}, {":authority", "host"}}); } TEST_P(CiliumIntegrationEgressL34Test, DeniedPathPrefix2) { - Denied({{":method", "GET"}, {":path", "/allowed"}, {":authority", "host"}}); + denied({{":method", "GET"}, {":path", "/allowed"}, {":authority", "host"}}); } const std::string HEADER_ACTION_MISSING_SDS_POLICY_fmt = R"EOF(version_info: "1" @@ -793,7 +795,7 @@ const std::string HEADER_ACTION_MISSING_SDS_POLICY2_fmt = R"EOF(version_info: "2 class SDSIntegrationTest : public CiliumIntegrationTest { public: - SDSIntegrationTest() : CiliumIntegrationTest() { + SDSIntegrationTest() { // switch back to SDS secrets so that we can test with a missing secret. // File based secret fails if the file does not exist, while SDS should allow for secret to be // created in future. @@ -830,7 +832,7 @@ INSTANTIATE_TEST_SUITE_P(IpVersions, SDSIntegrationTest, testing::ValuesIn(TestEnvironment::getIpVersionsForTest())); TEST_P(SDSIntegrationTest, TestDeniedL3) { - Denied({{":method", "GET"}, {":path", "/only42"}, {":authority", "host"}}); + denied({{":method", "GET"}, {":path", "/only42"}, {":authority", "host"}}); // Validate that missing headers are access logged correctly EXPECT_TRUE(expectAccessLogDeniedTo([](const ::cilium::LogEntry& entry) { @@ -845,7 +847,7 @@ TEST_P(SDSIntegrationTest, TestDeniedL3) { } TEST_P(SDSIntegrationTest, TestDeniedL3SpoofedXFF) { - Denied({{":method", "GET"}, + denied({{":method", "GET"}, {":path", "/only42"}, {":authority", "host"}, {"x-forwarded-for", "192.168.1.1"}}); @@ -863,7 +865,7 @@ TEST_P(SDSIntegrationTest, TestDeniedL3SpoofedXFF) { } TEST_P(SDSIntegrationTest, TestMissingSDSSecretOnUpdate) { - Accepted({{":method", "GET"}, {":path", "/allowed2"}, {":authority", "host"}}); + accepted({{":method", "GET"}, {":path", "/allowed2"}, {":authority", "host"}}); // Validate that missing headers are access logged correctly EXPECT_TRUE(expectAccessLogRequestTo([](const ::cilium::LogEntry& entry) { @@ -891,7 +893,7 @@ TEST_P(SDSIntegrationTest, TestMissingSDSSecretOnUpdate) { absl::SleepFor(absl::Milliseconds(100)); // 2nd round, on updated policy - Denied({{":method", "GET"}, {":path", "/allowed"}, {":authority", "host"}}); + denied({{":method", "GET"}, {":path", "/allowed"}, {":authority", "host"}}); // Validate that missing headers are access logged correctly EXPECT_TRUE(expectAccessLogDeniedTo([](const ::cilium::LogEntry& entry) { @@ -911,7 +913,7 @@ TEST_P(SDSIntegrationTest, TestMissingSDSSecretOnUpdate) { // Reduce flakiness by allowing some time for the policy to be updated before the following test absl::SleepFor(absl::Milliseconds(100)); - Denied({{":method", "GET"}, {":path", "/allowed"}, {":authority", "host"}}); + denied({{":method", "GET"}, {":path", "/allowed"}, {":authority", "host"}}); // Validate that missing headers are access logged correctly EXPECT_TRUE(expectAccessLogDeniedTo([](const ::cilium::LogEntry& entry) { diff --git a/tests/cilium_network_policy_test.cc b/tests/cilium_network_policy_test.cc index a16e5a1fd..5101489e3 100644 --- a/tests/cilium_network_policy_test.cc +++ b/tests/cilium_network_policy_test.cc @@ -54,7 +54,7 @@ class CiliumNetworkPolicyTest : public ::testing::Test { logger.setLevel(spdlog::level::trace); } } - ~CiliumNetworkPolicyTest() override {} + ~CiliumNetworkPolicyTest() override = default; void SetUp() override { // Mock SDS secrets with a real implementation, which will not return anything if there is no @@ -86,9 +86,9 @@ class CiliumNetworkPolicyTest : public ::testing::Test { return message.version_info(); } - testing::AssertionResult Validate(const std::string& pod_ip, const std::string& expected) { - const auto& policy = policy_map_->GetPolicyInstance(pod_ip, false); - auto str = policy.String(); + testing::AssertionResult validate(const std::string& pod_ip, const std::string& expected) { + const auto& policy = policy_map_->getPolicyInstance(pod_ip, false); + auto str = policy.string(); if (str != expected) { return testing::AssertionFailure() << "Policy:\n" << str << "Does not match expected:\n" @@ -97,29 +97,29 @@ class CiliumNetworkPolicyTest : public ::testing::Test { return testing::AssertionSuccess(); } - testing::AssertionResult Allowed(bool ingress, const std::string& pod_ip, uint64_t remote_id, + testing::AssertionResult allowed(bool ingress, const std::string& pod_ip, uint64_t remote_id, uint16_t port, Http::TestRequestHeaderMapImpl&& headers) { - const auto& policy = policy_map_->GetPolicyInstance(pod_ip, false); + const auto& policy = policy_map_->getPolicyInstance(pod_ip, false); Cilium::AccessLog::Entry log_entry; return policy.allowed(ingress, remote_id, port, headers, log_entry) ? testing::AssertionSuccess() : testing::AssertionFailure(); } - testing::AssertionResult IngressAllowed(const std::string& pod_ip, uint64_t remote_id, + testing::AssertionResult ingressAllowed(const std::string& pod_ip, uint64_t remote_id, uint16_t port, Http::TestRequestHeaderMapImpl&& headers = {}) { - return Allowed(true, pod_ip, remote_id, port, std::move(headers)); + return allowed(true, pod_ip, remote_id, port, std::move(headers)); } - testing::AssertionResult EgressAllowed(const std::string& pod_ip, uint64_t remote_id, + testing::AssertionResult egressAllowed(const std::string& pod_ip, uint64_t remote_id, uint16_t port, Http::TestRequestHeaderMapImpl&& headers = {}) { - return Allowed(false, pod_ip, remote_id, port, std::move(headers)); + return allowed(false, pod_ip, remote_id, port, std::move(headers)); } - testing::AssertionResult TlsAllowed(bool ingress, const std::string& pod_ip, uint64_t remote_id, + testing::AssertionResult tlsAllowed(bool ingress, const std::string& pod_ip, uint64_t remote_id, uint16_t port, absl::string_view sni, bool& tls_socket_required, bool& raw_socket_allowed) { - const auto& policy = policy_map_->GetPolicyInstance(pod_ip, false); + const auto& policy = policy_map_->getPolicyInstance(pod_ip, false); auto port_policy = policy.findPortPolicy(ingress, port); const Envoy::Ssl::ContextConfig* config = nullptr; @@ -147,47 +147,52 @@ class CiliumNetworkPolicyTest : public ::testing::Test { } // config must exist if ctx is returned - if (ctx != nullptr) + if (ctx != nullptr) { EXPECT_TRUE(config != nullptr); + } EXPECT_TRUE(allowed == (tls_socket_required || raw_socket_allowed)); - if (!allowed) + if (!allowed) { return testing::AssertionFailure() << pod_ip << " policy not allowing id " << remote_id << " on port " << port << " with SNI \"" << sni << "\""; + } // sanity check EXPECT_TRUE(!(tls_socket_required && raw_socket_allowed) && tls_socket_required || raw_socket_allowed); - if (raw_socket_allowed) + if (raw_socket_allowed) { return testing::AssertionSuccess() << pod_ip << " policy allows id " << remote_id << " on port " << port << " with SNI \"" << sni << "\" without TLS socket"; + } - if (tls_socket_required && ctx != nullptr) + if (tls_socket_required && ctx != nullptr) { return testing::AssertionSuccess() << pod_ip << " policy allows id " << remote_id << " on port " << port << " with SNI \"" << sni << "\" with TLS socket"; + } - if (tls_socket_required && ctx == nullptr) + if (tls_socket_required && ctx == nullptr) { return testing::AssertionSuccess() << pod_ip << " policy allows id " << remote_id << " on port " << port << " with SNI \"" << sni << "\" but missing TLS context"; + } return testing::AssertionFailure(); } - testing::AssertionResult TlsIngressAllowed(const std::string& pod_ip, uint64_t remote_id, + testing::AssertionResult tlsIngressAllowed(const std::string& pod_ip, uint64_t remote_id, uint16_t port, absl::string_view sni, bool& tls_socket_required, bool& raw_socket_allowed) { - return TlsAllowed(true, pod_ip, remote_id, port, sni, tls_socket_required, raw_socket_allowed); + return tlsAllowed(true, pod_ip, remote_id, port, sni, tls_socket_required, raw_socket_allowed); } - testing::AssertionResult TlsEgressAllowed(const std::string& pod_ip, uint64_t remote_id, + testing::AssertionResult tlsEgressAllowed(const std::string& pod_ip, uint64_t remote_id, uint16_t port, absl::string_view sni, bool& tls_socket_required, bool& raw_socket_allowed) { - return TlsAllowed(false, pod_ip, remote_id, port, sni, tls_socket_required, raw_socket_allowed); + return tlsAllowed(false, pod_ip, remote_id, port, sni, tls_socket_required, raw_socket_allowed); } std::string updatesRejectedStatName() { return policy_map_->stats_.updates_rejected_.name(); } @@ -203,7 +208,7 @@ TEST_F(CiliumNetworkPolicyTest, UpdatesRejectedStatName) { TEST_F(CiliumNetworkPolicyTest, EmptyPolicyUpdate) { EXPECT_TRUE(policy_map_->onConfigUpdate({}, "1").ok()); - EXPECT_FALSE(Validate("10.1.2.3", "")); // Policy not found + EXPECT_FALSE(validate("10.1.2.3", "")); // Policy not found } TEST_F(CiliumNetworkPolicyTest, SimplePolicyUpdate) { @@ -211,7 +216,7 @@ TEST_F(CiliumNetworkPolicyTest, SimplePolicyUpdate) { EXPECT_NO_THROW(version = updateFromYaml(R"EOF(version_info: "0" )EOF")); EXPECT_EQ(version, "0"); - EXPECT_FALSE(Validate("10.1.2.3", "")); // Policy not found + EXPECT_FALSE(validate("10.1.2.3", "")); // Policy not found } TEST_F(CiliumNetworkPolicyTest, OverlappingPortRange) { @@ -272,39 +277,39 @@ TEST_F(CiliumNetworkPolicyTest, OverlappingPortRange) { wildcard_rules: [] )EOF"; - EXPECT_TRUE(Validate("10.1.2.3", expected)); + EXPECT_TRUE(validate("10.1.2.3", expected)); // Ingress from 42 is allowed on port 23 - EXPECT_TRUE(IngressAllowed("10.1.2.3", 42, 23)); - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 23)); - EXPECT_FALSE(IngressAllowed("10.1.2.3", 44, 23)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 42, 23)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 23)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 44, 23)); // port 92 is denied from everyone - EXPECT_FALSE(IngressAllowed("10.1.2.3", 42, 92)); - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 92)); - EXPECT_FALSE(IngressAllowed("10.1.2.3", 44, 92)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 42, 92)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 92)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 44, 92)); // Ingress from 43 is allowed on all ports of the range: - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 39)); - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 40)); - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 79)); - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 80)); - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 81)); - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 99)); - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 100)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 39)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 40)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 79)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 80)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 81)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 99)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 100)); // 44 is only allowed to port 80 - EXPECT_FALSE(IngressAllowed("10.1.2.3", 44, 39)); - EXPECT_FALSE(IngressAllowed("10.1.2.3", 44, 40)); - EXPECT_FALSE(IngressAllowed("10.1.2.3", 44, 79)); - EXPECT_TRUE(IngressAllowed("10.1.2.3", 44, 80)); - EXPECT_FALSE(IngressAllowed("10.1.2.3", 44, 81)); - EXPECT_FALSE(IngressAllowed("10.1.2.3", 44, 99)); - EXPECT_FALSE(IngressAllowed("10.1.2.3", 44, 100)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 44, 39)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 44, 40)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 44, 79)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 44, 80)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 44, 81)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 44, 99)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 44, 100)); // No egress is allowed: - EXPECT_FALSE(EgressAllowed("10.1.2.3", 43, 8080)); - EXPECT_FALSE(EgressAllowed("10.1.2.3", 44, 8080)); + EXPECT_FALSE(egressAllowed("10.1.2.3", 43, 8080)); + EXPECT_FALSE(egressAllowed("10.1.2.3", 44, 8080)); // Same with policies added in reverse order EXPECT_NO_THROW(updateFromYaml(R"EOF(version_info: "1" @@ -330,39 +335,39 @@ TEST_F(CiliumNetworkPolicyTest, OverlappingPortRange) { - remote_policies: [ 45 ] )EOF")); - EXPECT_TRUE(Validate("10.1.2.3", expected)); + EXPECT_TRUE(validate("10.1.2.3", expected)); // Ingress from 42 is allowed on port 23 - EXPECT_TRUE(IngressAllowed("10.1.2.3", 42, 23)); - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 23)); - EXPECT_FALSE(IngressAllowed("10.1.2.3", 44, 23)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 42, 23)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 23)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 44, 23)); // port 92 is denied from everyone - EXPECT_FALSE(IngressAllowed("10.1.2.3", 42, 92)); - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 92)); - EXPECT_FALSE(IngressAllowed("10.1.2.3", 44, 92)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 42, 92)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 92)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 44, 92)); // Ingress from 43 is allowed on all ports of the range: - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 39)); - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 40)); - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 79)); - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 80)); - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 81)); - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 99)); - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 100)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 39)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 40)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 79)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 80)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 81)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 99)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 100)); // 44 is only allowed to port 80 - EXPECT_FALSE(IngressAllowed("10.1.2.3", 44, 39)); - EXPECT_FALSE(IngressAllowed("10.1.2.3", 44, 40)); - EXPECT_FALSE(IngressAllowed("10.1.2.3", 44, 79)); - EXPECT_TRUE(IngressAllowed("10.1.2.3", 44, 80)); - EXPECT_FALSE(IngressAllowed("10.1.2.3", 44, 81)); - EXPECT_FALSE(IngressAllowed("10.1.2.3", 44, 99)); - EXPECT_FALSE(IngressAllowed("10.1.2.3", 44, 100)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 44, 39)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 44, 40)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 44, 79)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 44, 80)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 44, 81)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 44, 99)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 44, 100)); // No egress is allowed: - EXPECT_FALSE(EgressAllowed("10.1.2.3", 43, 8080)); - EXPECT_FALSE(EgressAllowed("10.1.2.3", 44, 8080)); + EXPECT_FALSE(egressAllowed("10.1.2.3", 43, 8080)); + EXPECT_FALSE(egressAllowed("10.1.2.3", 44, 8080)); } TEST_F(CiliumNetworkPolicyTest, OverlappingPortRanges) { @@ -402,25 +407,25 @@ TEST_F(CiliumNetworkPolicyTest, OverlappingPortRanges) { wildcard_rules: [] )EOF"; - EXPECT_TRUE(Validate("10.1.2.3", expected)); + EXPECT_TRUE(validate("10.1.2.3", expected)); // Ingress from 43 is allowed to ports 80-8080 only: - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 79)); - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 80)); - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 81)); - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 4039)); - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 4040)); - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 4041)); - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 8079)); - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 8080)); - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 8081)); - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 9998)); - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 9999)); - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 10000)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 79)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 80)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 81)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 4039)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 4040)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 4041)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 8079)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 8080)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 8081)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 9998)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 9999)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 10000)); // No egress is allowed: - EXPECT_FALSE(EgressAllowed("10.1.2.3", 43, 8080)); - EXPECT_FALSE(EgressAllowed("10.1.2.3", 44, 8080)); + EXPECT_FALSE(egressAllowed("10.1.2.3", 43, 8080)); + EXPECT_FALSE(egressAllowed("10.1.2.3", 44, 8080)); // Same with policies added in reverse order EXPECT_NO_THROW(updateFromYaml(R"EOF(version_info: "1" @@ -460,25 +465,25 @@ TEST_F(CiliumNetworkPolicyTest, OverlappingPortRanges) { wildcard_rules: [] )EOF"; - EXPECT_TRUE(Validate("10.1.2.3", expected)); + EXPECT_TRUE(validate("10.1.2.3", expected)); // Ingress from 43 is allowed to ports 80-8080 only: - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 79)); - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 80)); - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 81)); - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 4039)); - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 4040)); - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 4041)); - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 8079)); - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 8080)); - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 8081)); - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 9998)); - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 9999)); - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 10000)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 79)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 80)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 81)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 4039)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 4040)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 4041)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 8079)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 8080)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 8081)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 9998)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 9999)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 10000)); // No egress is allowed: - EXPECT_FALSE(EgressAllowed("10.1.2.3", 43, 8080)); - EXPECT_FALSE(EgressAllowed("10.1.2.3", 44, 8080)); + EXPECT_FALSE(egressAllowed("10.1.2.3", 43, 8080)); + EXPECT_FALSE(egressAllowed("10.1.2.3", 44, 8080)); } TEST_F(CiliumNetworkPolicyTest, DuplicatePorts) { @@ -510,15 +515,15 @@ TEST_F(CiliumNetworkPolicyTest, DuplicatePorts) { wildcard_rules: [] )EOF"; - EXPECT_TRUE(Validate("10.1.2.3", expected)); + EXPECT_TRUE(validate("10.1.2.3", expected)); // Ingress from 43 is allowed on port 80 only: - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 80)); - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 8080)); - EXPECT_FALSE(IngressAllowed("10.1.2.3", 44, 8080)); - EXPECT_FALSE(IngressAllowed("10.1.2.3", 44, 80)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 80)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 8080)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 44, 8080)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 44, 80)); // No egress is allowed: - EXPECT_FALSE(EgressAllowed("10.1.2.3", 43, 8080)); + EXPECT_FALSE(egressAllowed("10.1.2.3", 43, 8080)); } TEST_F(CiliumNetworkPolicyTest, DuplicatePortRange) { @@ -554,18 +559,18 @@ TEST_F(CiliumNetworkPolicyTest, DuplicatePortRange) { wildcard_rules: [] )EOF"; - EXPECT_TRUE(Validate("10.1.2.3", expected)); + EXPECT_TRUE(validate("10.1.2.3", expected)); // Ingress is allowed: - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 79)); - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 80)); - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 81)); - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 8079)); - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 8080)); - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 8081)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 79)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 80)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 81)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 8079)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 8080)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 8081)); // No egress is allowed: - EXPECT_FALSE(EgressAllowed("10.1.2.3", 43, 8080)); + EXPECT_FALSE(egressAllowed("10.1.2.3", 43, 8080)); } TEST_F(CiliumNetworkPolicyTest, InvalidPortRange) { @@ -590,9 +595,9 @@ TEST_F(CiliumNetworkPolicyTest, InvalidPortRange) { "PortNetworkPolicy: Invalid port range, end port is less than start port 80-60"); // No ingress is allowed: - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 80)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 80)); // No egress is allowed: - EXPECT_FALSE(EgressAllowed("10.1.2.3", 43, 8080)); + EXPECT_FALSE(egressAllowed("10.1.2.3", 43, 8080)); } TEST_F(CiliumNetworkPolicyTest, InvalidWildcardPortRange) { @@ -617,9 +622,9 @@ TEST_F(CiliumNetworkPolicyTest, InvalidWildcardPortRange) { "PortNetworkPolicy: Invalid port range including the wildcard zero port 0-80"); // No ingress is allowed: - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 80)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 80)); // No egress is allowed: - EXPECT_FALSE(EgressAllowed("10.1.2.3", 43, 8080)); + EXPECT_FALSE(egressAllowed("10.1.2.3", 43, 8080)); } // Zero end port is treated as no range @@ -658,19 +663,19 @@ TEST_F(CiliumNetworkPolicyTest, ZeroPortRange) { wildcard_rules: [] )EOF"; - EXPECT_TRUE(Validate("10.1.2.3", expected)); + EXPECT_TRUE(validate("10.1.2.3", expected)); // Allowed remote ID, port, & path: - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 80)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 80)); // Wrong remote ID: - EXPECT_FALSE(IngressAllowed("10.1.2.3", 40, 80)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 40, 80)); // Allowed port: - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 8080, {{":path", "/allowed"}})); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 8080, {{":path", "/allowed"}})); // Path is ignored: - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 80, {{":path", "/notallowed"}})); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 80, {{":path", "/notallowed"}})); // No egress is allowed: - EXPECT_FALSE(EgressAllowed("10.1.2.3", 43, 80)); + EXPECT_FALSE(egressAllowed("10.1.2.3", 43, 80)); } TEST_F(CiliumNetworkPolicyTest, HttpPolicyUpdate) { @@ -680,7 +685,7 @@ TEST_F(CiliumNetworkPolicyTest, HttpPolicyUpdate) { EXPECT_EQ(version, "0"); EXPECT_FALSE(policy_map_->exists("10.1.2.3")); // No policy for the pod - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 80, {{":path", "/allowed"}})); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 80, {{":path", "/allowed"}})); // 1st update EXPECT_NO_THROW(version = updateFromYaml(R"EOF(version_info: "1" @@ -717,19 +722,19 @@ TEST_F(CiliumNetworkPolicyTest, HttpPolicyUpdate) { wildcard_rules: [] )EOF"; - EXPECT_TRUE(Validate("10.1.2.3", expected)); + EXPECT_TRUE(validate("10.1.2.3", expected)); // Allowed remote ID, port, & path: - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 80, {{":path", "/allowed"}})); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 80, {{":path", "/allowed"}})); // Wrong remote ID: - EXPECT_FALSE(IngressAllowed("10.1.2.3", 40, 80, {{":path", "/allowed"}})); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 40, 80, {{":path", "/allowed"}})); // Wrong port: - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 8080, {{":path", "/allowed"}})); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 8080, {{":path", "/allowed"}})); // Wrong path: - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 80, {{":path", "/notallowed"}})); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 80, {{":path", "/notallowed"}})); // No egress is allowed: - EXPECT_FALSE(EgressAllowed("10.1.2.3", 43, 80, {{":path", "/public"}})); + EXPECT_FALSE(egressAllowed("10.1.2.3", 43, 80, {{":path", "/public"}})); // 2nd update EXPECT_NO_THROW(version = updateFromYaml(R"EOF(version_info: "2" @@ -784,27 +789,27 @@ TEST_F(CiliumNetworkPolicyTest, HttpPolicyUpdate) { wildcard_rules: [] )EOF"; - EXPECT_TRUE(Validate("10.1.2.3", expected)); + EXPECT_TRUE(validate("10.1.2.3", expected)); // Allowed remote ID, port, & path: - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 80, {{":path", "/allowed"}})); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 80, {{":path", "/allowed"}})); // Wrong remote ID: - EXPECT_FALSE(IngressAllowed("10.1.2.3", 40, 80, {{":path", "/allowed"}})); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 40, 80, {{":path", "/allowed"}})); // Wrong port: - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 8080, {{":path", "/allowed"}})); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 8080, {{":path", "/allowed"}})); // Wrong path: - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 80, {{":path", "/notallowed"}})); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 80, {{":path", "/notallowed"}})); // Allowed remote ID, port, & path: - EXPECT_TRUE(EgressAllowed("10.1.2.3", 43, 80, {{":path", "/public"}})); + EXPECT_TRUE(egressAllowed("10.1.2.3", 43, 80, {{":path", "/public"}})); // Allowed remote ID, port, & path: - EXPECT_TRUE(EgressAllowed("10.1.2.3", 44, 80, {{":path", "/public"}})); + EXPECT_TRUE(egressAllowed("10.1.2.3", 44, 80, {{":path", "/public"}})); // Wrong remote ID: - EXPECT_FALSE(EgressAllowed("10.1.2.3", 40, 80, {{":path", "/public"}})); + EXPECT_FALSE(egressAllowed("10.1.2.3", 40, 80, {{":path", "/public"}})); // Wrong port: - EXPECT_FALSE(EgressAllowed("10.1.2.3", 43, 8080, {{":path", "/public"}})); + EXPECT_FALSE(egressAllowed("10.1.2.3", 43, 8080, {{":path", "/public"}})); // Wrong path: - EXPECT_FALSE(EgressAllowed("10.1.2.3", 43, 80, {{":path", "/publicz"}})); + EXPECT_FALSE(egressAllowed("10.1.2.3", 43, 80, {{":path", "/publicz"}})); // 3rd update with Ingress deny rules EXPECT_NO_THROW(version = updateFromYaml(R"EOF(version_info: "2" @@ -874,27 +879,27 @@ TEST_F(CiliumNetworkPolicyTest, HttpPolicyUpdate) { wildcard_rules: [] )EOF"; - EXPECT_TRUE(Validate("10.1.2.3", expected)); + EXPECT_TRUE(validate("10.1.2.3", expected)); // Denied remote ID, port, & path: - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 80, {{":path", "/allowed"}})); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 80, {{":path", "/allowed"}})); // Wrong remote ID: - EXPECT_FALSE(IngressAllowed("10.1.2.3", 40, 80, {{":path", "/allowed"}})); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 40, 80, {{":path", "/allowed"}})); // Wrong port: - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 8080, {{":path", "/allowed"}})); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 8080, {{":path", "/allowed"}})); // Denied remote ID & wrong path: - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 80, {{":path", "/notallowed"}})); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 80, {{":path", "/notallowed"}})); // Allowed remote ID, port, & path: - EXPECT_TRUE(EgressAllowed("10.1.2.3", 43, 80, {{":path", "/public"}})); + EXPECT_TRUE(egressAllowed("10.1.2.3", 43, 80, {{":path", "/public"}})); // Allowed remote ID, port, & path: - EXPECT_TRUE(EgressAllowed("10.1.2.3", 44, 80, {{":path", "/public"}})); + EXPECT_TRUE(egressAllowed("10.1.2.3", 44, 80, {{":path", "/public"}})); // Wrong remote ID: - EXPECT_FALSE(EgressAllowed("10.1.2.3", 40, 80, {{":path", "/public"}})); + EXPECT_FALSE(egressAllowed("10.1.2.3", 40, 80, {{":path", "/public"}})); // Wrong port: - EXPECT_FALSE(EgressAllowed("10.1.2.3", 43, 8080, {{":path", "/public"}})); + EXPECT_FALSE(egressAllowed("10.1.2.3", 43, 8080, {{":path", "/public"}})); // Wrong path: - EXPECT_FALSE(EgressAllowed("10.1.2.3", 43, 80, {{":path", "/publicz"}})); + EXPECT_FALSE(egressAllowed("10.1.2.3", 43, 80, {{":path", "/publicz"}})); } TEST_F(CiliumNetworkPolicyTest, HttpOverlappingPortRanges) { @@ -904,7 +909,7 @@ TEST_F(CiliumNetworkPolicyTest, HttpOverlappingPortRanges) { EXPECT_EQ(version, "0"); EXPECT_FALSE(policy_map_->exists("10.1.2.3")); // No policy for the pod - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 80, {{":path", "/allowed"}})); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 80, {{":path", "/allowed"}})); // 1st update EXPECT_NO_THROW(version = updateFromYaml(R"EOF(version_info: "1" @@ -955,20 +960,20 @@ TEST_F(CiliumNetworkPolicyTest, HttpOverlappingPortRanges) { wildcard_rules: [] )EOF"; - EXPECT_TRUE(Validate("10.1.2.3", expected)); + EXPECT_TRUE(validate("10.1.2.3", expected)); // Allowed remote ID, port, & method OR path: - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 80, {{":method", "PUSH"}, {":path", "/allowed"}})); - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 80, {{":method", "GET"}, {":path", "/also_allowed"}})); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 80, {{":method", "PUSH"}, {":path", "/allowed"}})); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 80, {{":method", "GET"}, {":path", "/also_allowed"}})); // Wrong remote ID: - EXPECT_FALSE(IngressAllowed("10.1.2.3", 40, 80, {{":path", "/allowed"}})); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 40, 80, {{":path", "/allowed"}})); // Wrong port: - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 8080, {{":path", "/allowed"}})); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 8080, {{":path", "/allowed"}})); // Wrong path: - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 80, {{":path", "/notallowed"}})); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 80, {{":path", "/notallowed"}})); // No egress is allowed: - EXPECT_FALSE(EgressAllowed("10.1.2.3", 43, 80, {{":path", "/public"}})); + EXPECT_FALSE(egressAllowed("10.1.2.3", 43, 80, {{":path", "/public"}})); // 2nd update with overlapping port range and a single port EXPECT_NO_THROW(version = updateFromYaml(R"EOF(version_info: "2" @@ -1034,27 +1039,27 @@ TEST_F(CiliumNetworkPolicyTest, HttpOverlappingPortRanges) { wildcard_rules: [] )EOF"; - EXPECT_TRUE(Validate("10.1.2.3", expected)); + EXPECT_TRUE(validate("10.1.2.3", expected)); // Allowed remote ID, port, & method OR path: - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 70, {{":method", "PUSH"}, {":path", "/allowed"}})); - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 80, {{":method", "PUSH"}, {":path", "/allowed"}})); - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 90, {{":method", "PUSH"}, {":path", "/allowed"}})); - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 80, {{":method", "GET"}, {":path", "/also_allowed"}})); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 70, {{":method", "PUSH"}, {":path", "/allowed"}})); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 80, {{":method", "PUSH"}, {":path", "/allowed"}})); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 90, {{":method", "PUSH"}, {":path", "/allowed"}})); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 80, {{":method", "GET"}, {":path", "/also_allowed"}})); // wrong port for GET EXPECT_FALSE( - IngressAllowed("10.1.2.3", 43, 70, {{":method", "GET"}, {":path", "/also_allowed"}})); + ingressAllowed("10.1.2.3", 43, 70, {{":method", "GET"}, {":path", "/also_allowed"}})); EXPECT_FALSE( - IngressAllowed("10.1.2.3", 43, 90, {{":method", "GET"}, {":path", "/also_allowed"}})); + ingressAllowed("10.1.2.3", 43, 90, {{":method", "GET"}, {":path", "/also_allowed"}})); // Wrong remote ID: - EXPECT_FALSE(IngressAllowed("10.1.2.3", 40, 80, {{":path", "/allowed"}})); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 40, 80, {{":path", "/allowed"}})); // Wrong port: - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 8080, {{":path", "/allowed"}})); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 8080, {{":path", "/allowed"}})); // Wrong path: - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 80, {{":path", "/notallowed"}})); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 80, {{":path", "/notallowed"}})); // No egress is allowed: - EXPECT_FALSE(EgressAllowed("10.1.2.3", 43, 80, {{":path", "/public"}})); + EXPECT_FALSE(egressAllowed("10.1.2.3", 43, 80, {{":path", "/public"}})); // 3rd update with overlapping port ranges EXPECT_NO_THROW(version = updateFromYaml(R"EOF(version_info: "2" @@ -1121,28 +1126,28 @@ TEST_F(CiliumNetworkPolicyTest, HttpOverlappingPortRanges) { wildcard_rules: [] )EOF"; - EXPECT_TRUE(Validate("10.1.2.3", expected)); + EXPECT_TRUE(validate("10.1.2.3", expected)); // Allowed remote ID, port, & method OR path: - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 70, {{":method", "PUSH"}, {":path", "/allowed"}})); - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 80, {{":method", "PUSH"}, {":path", "/allowed"}})); - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 90, {{":method", "PUSH"}, {":path", "/allowed"}})); - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 80, {{":method", "GET"}, {":path", "/also_allowed"}})); - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 90, {{":method", "GET"}, {":path", "/also_allowed"}})); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 70, {{":method", "PUSH"}, {":path", "/allowed"}})); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 80, {{":method", "PUSH"}, {":path", "/allowed"}})); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 90, {{":method", "PUSH"}, {":path", "/allowed"}})); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 80, {{":method", "GET"}, {":path", "/also_allowed"}})); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 90, {{":method", "GET"}, {":path", "/also_allowed"}})); EXPECT_TRUE( - IngressAllowed("10.1.2.3", 43, 8080, {{":method", "GET"}, {":path", "/also_allowed"}})); + ingressAllowed("10.1.2.3", 43, 8080, {{":method", "GET"}, {":path", "/also_allowed"}})); // wrong port for GET EXPECT_FALSE( - IngressAllowed("10.1.2.3", 43, 70, {{":method", "GET"}, {":path", "/also_allowed"}})); + ingressAllowed("10.1.2.3", 43, 70, {{":method", "GET"}, {":path", "/also_allowed"}})); // Wrong remote ID: - EXPECT_FALSE(IngressAllowed("10.1.2.3", 40, 80, {{":path", "/allowed"}})); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 40, 80, {{":path", "/allowed"}})); // Wrong port: - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 8080, {{":path", "/allowed"}})); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 8080, {{":path", "/allowed"}})); // Wrong path: - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 80, {{":path", "/notallowed"}})); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 80, {{":path", "/notallowed"}})); // No egress is allowed: - EXPECT_FALSE(EgressAllowed("10.1.2.3", 43, 80, {{":path", "/public"}})); + EXPECT_FALSE(egressAllowed("10.1.2.3", 43, 80, {{":path", "/public"}})); } TEST_F(CiliumNetworkPolicyTest, TcpPolicyUpdate) { @@ -1152,7 +1157,7 @@ TEST_F(CiliumNetworkPolicyTest, TcpPolicyUpdate) { EXPECT_EQ(version, "0"); EXPECT_FALSE(policy_map_->exists("10.1.2.3")); // No policy for the pod - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 80, {{":path", "/allowed"}})); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 80, {{":path", "/allowed"}})); // 1st update EXPECT_NO_THROW(version = updateFromYaml(R"EOF(version_info: "1" @@ -1169,16 +1174,16 @@ TEST_F(CiliumNetworkPolicyTest, TcpPolicyUpdate) { EXPECT_EQ(version, "1"); EXPECT_TRUE(policy_map_->exists("10.1.2.3")); // Allowed remote ID & port: - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 80, {{":path", "/allowed"}})); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 80, {{":path", "/allowed"}})); // Wrong remote ID: - EXPECT_FALSE(IngressAllowed("10.1.2.3", 40, 80, {{":path", "/allowed"}})); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 40, 80, {{":path", "/allowed"}})); // Wrong port: - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 8080, {{":path", "/allowed"}})); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 8080, {{":path", "/allowed"}})); // Path does not matter: - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 80, {{":path", "/notallowed"}})); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 80, {{":path", "/notallowed"}})); // No egress is allowed: - EXPECT_FALSE(EgressAllowed("10.1.2.3", 43, 80, {{":path", "/public"}})); + EXPECT_FALSE(egressAllowed("10.1.2.3", 43, 80, {{":path", "/public"}})); // 2nd update EXPECT_NO_THROW(version = updateFromYaml(R"EOF(version_info: "2" @@ -1199,24 +1204,24 @@ TEST_F(CiliumNetworkPolicyTest, TcpPolicyUpdate) { EXPECT_EQ(version, "2"); EXPECT_TRUE(policy_map_->exists("10.1.2.3")); // Allowed remote ID & port: - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 80, {{":path", "/allowed"}})); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 80, {{":path", "/allowed"}})); // Wrong remote ID: - EXPECT_FALSE(IngressAllowed("10.1.2.3", 40, 80, {{":path", "/allowed"}})); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 40, 80, {{":path", "/allowed"}})); // Wrong port: - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 8080, {{":path", "/allowed"}})); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 8080, {{":path", "/allowed"}})); // Path does not matter - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 80, {{":path", "/notallowed"}})); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 80, {{":path", "/notallowed"}})); // Allowed remote ID & port: - EXPECT_TRUE(EgressAllowed("10.1.2.3", 43, 80, {{":path", "/public"}})); + EXPECT_TRUE(egressAllowed("10.1.2.3", 43, 80, {{":path", "/public"}})); // Allowed remote ID & port: - EXPECT_TRUE(EgressAllowed("10.1.2.3", 44, 80, {{":path", "/public"}})); + EXPECT_TRUE(egressAllowed("10.1.2.3", 44, 80, {{":path", "/public"}})); // Wrong remote ID: - EXPECT_FALSE(EgressAllowed("10.1.2.3", 40, 80, {{":path", "/public"}})); + EXPECT_FALSE(egressAllowed("10.1.2.3", 40, 80, {{":path", "/public"}})); // Wrong port: - EXPECT_FALSE(EgressAllowed("10.1.2.3", 43, 8080, {{":path", "/public"}})); + EXPECT_FALSE(egressAllowed("10.1.2.3", 43, 8080, {{":path", "/public"}})); // Path does not matter: - EXPECT_TRUE(EgressAllowed("10.1.2.3", 43, 80, {{":path", "/publicz"}})); + EXPECT_TRUE(egressAllowed("10.1.2.3", 43, 80, {{":path", "/publicz"}})); } TEST_F(CiliumNetworkPolicyTest, PortRanges) { @@ -1226,7 +1231,7 @@ TEST_F(CiliumNetworkPolicyTest, PortRanges) { EXPECT_EQ(version, "0"); EXPECT_FALSE(policy_map_->exists("10.1.2.3")); // No policy for the pod - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 80)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 80)); // 1st update EXPECT_NO_THROW(version = updateFromYaml(R"EOF(version_info: "1" @@ -1244,20 +1249,20 @@ TEST_F(CiliumNetworkPolicyTest, PortRanges) { EXPECT_EQ(version, "1"); EXPECT_TRUE(policy_map_->exists("10.1.2.3")); // Allowed remote ID & port: - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 80)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 80)); // Path does not matter - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 80, {{":path", "/notallowed"}})); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 80, {{":path", "/notallowed"}})); // Port within the range: - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 4040)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 4040)); // Port at the end of the range: - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 8080)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 8080)); // Port out of range: - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 79)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 79)); // Port out of range: - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 8081)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 8081)); // No egress is allowed: - EXPECT_FALSE(EgressAllowed("10.1.2.3", 43, 80)); + EXPECT_FALSE(egressAllowed("10.1.2.3", 43, 80)); // 2nd update EXPECT_NO_THROW(version = updateFromYaml(R"EOF(version_info: "2" @@ -1285,53 +1290,53 @@ TEST_F(CiliumNetworkPolicyTest, PortRanges) { EXPECT_TRUE(policy_map_->exists("10.1.2.3")); // Allowed remote ID & port: - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 80)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 80)); // Wrong remote ID: - EXPECT_FALSE(EgressAllowed("10.1.2.3", 40, 80)); + EXPECT_FALSE(egressAllowed("10.1.2.3", 40, 80)); // Path does not matter - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 80, {{":path", "/notallowed"}})); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 80, {{":path", "/notallowed"}})); // Port within the range: - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 4040)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 4040)); // Port at the end of the range: - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 8080)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 8080)); // Port out of range: - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 79)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 79)); // Port out of range: - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 8081)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 8081)); // Allowed remote ID & port: - EXPECT_TRUE(IngressAllowed("10.1.2.3", 44, 9000)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 44, 9000)); // Port within the range: - EXPECT_TRUE(IngressAllowed("10.1.2.3", 44, 9500)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 44, 9500)); // Port at the end of the range: - EXPECT_TRUE(IngressAllowed("10.1.2.3", 44, 9999)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 44, 9999)); // Port out of range: - EXPECT_FALSE(IngressAllowed("10.1.2.3", 44, 8999)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 44, 8999)); // Port out of range: - EXPECT_FALSE(IngressAllowed("10.1.2.3", 44, 10000)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 44, 10000)); // Wrong remote IDs: - EXPECT_FALSE(IngressAllowed("10.1.2.3", 44, 80)); - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 9000)); - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 9500)); - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 9999)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 44, 80)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 9000)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 9500)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 9999)); // Allowed remote ID & port: - EXPECT_TRUE(EgressAllowed("10.1.2.3", 43, 80)); + EXPECT_TRUE(egressAllowed("10.1.2.3", 43, 80)); // Path does not matter: - EXPECT_TRUE(EgressAllowed("10.1.2.3", 43, 80, {{":path", "/publicz"}})); + EXPECT_TRUE(egressAllowed("10.1.2.3", 43, 80, {{":path", "/publicz"}})); // Allowed remote ID & port: - EXPECT_TRUE(EgressAllowed("10.1.2.3", 44, 80)); + EXPECT_TRUE(egressAllowed("10.1.2.3", 44, 80)); // Wrong remote ID: - EXPECT_FALSE(EgressAllowed("10.1.2.3", 40, 80)); + EXPECT_FALSE(egressAllowed("10.1.2.3", 40, 80)); // Port within the range: - EXPECT_TRUE(EgressAllowed("10.1.2.3", 43, 85)); + EXPECT_TRUE(egressAllowed("10.1.2.3", 43, 85)); // Port at the end of the range: - EXPECT_TRUE(EgressAllowed("10.1.2.3", 43, 90)); + EXPECT_TRUE(egressAllowed("10.1.2.3", 43, 90)); // Port out of range: - EXPECT_FALSE(EgressAllowed("10.1.2.3", 43, 79)); + EXPECT_FALSE(egressAllowed("10.1.2.3", 43, 79)); // Port out of range: - EXPECT_FALSE(EgressAllowed("10.1.2.3", 43, 91)); + EXPECT_FALSE(egressAllowed("10.1.2.3", 43, 91)); // 3rd update, ranges with HTTP EXPECT_NO_THROW(version = updateFromYaml(R"EOF(version_info: "2" @@ -1370,21 +1375,21 @@ TEST_F(CiliumNetworkPolicyTest, PortRanges) { EXPECT_TRUE(policy_map_->exists("10.1.2.3")); // Allowed remote ID, port, & path: - EXPECT_TRUE(EgressAllowed("10.1.2.3", 43, 80, {{":path", "/allowed"}})); + EXPECT_TRUE(egressAllowed("10.1.2.3", 43, 80, {{":path", "/allowed"}})); // Wrong path: - EXPECT_FALSE(EgressAllowed("10.1.2.3", 43, 80, {{":path", "/publicz"}})); + EXPECT_FALSE(egressAllowed("10.1.2.3", 43, 80, {{":path", "/publicz"}})); // Allowed remote ID & port: - EXPECT_TRUE(EgressAllowed("10.1.2.3", 44, 80, {{":path", "/allows"}})); + EXPECT_TRUE(egressAllowed("10.1.2.3", 44, 80, {{":path", "/allows"}})); // Wrong remote ID: - EXPECT_FALSE(EgressAllowed("10.1.2.3", 40, 80, {{":path", "/public"}})); + EXPECT_FALSE(egressAllowed("10.1.2.3", 40, 80, {{":path", "/public"}})); // Port within the range: - EXPECT_TRUE(EgressAllowed("10.1.2.3", 43, 85, {{":path", "/allows"}})); + EXPECT_TRUE(egressAllowed("10.1.2.3", 43, 85, {{":path", "/allows"}})); // Port at the end of the range: - EXPECT_TRUE(EgressAllowed("10.1.2.3", 43, 90, {{":path", "/public"}})); + EXPECT_TRUE(egressAllowed("10.1.2.3", 43, 90, {{":path", "/public"}})); // Port out of range: - EXPECT_FALSE(EgressAllowed("10.1.2.3", 43, 79, {{":path", "/allows"}})); + EXPECT_FALSE(egressAllowed("10.1.2.3", 43, 79, {{":path", "/allows"}})); // Port out of range: - EXPECT_FALSE(EgressAllowed("10.1.2.3", 43, 91, {{":path", "/public"}})); + EXPECT_FALSE(egressAllowed("10.1.2.3", 43, 91, {{":path", "/public"}})); } TEST_F(CiliumNetworkPolicyTest, HttpPolicyUpdateToMissingSDS) { @@ -1394,7 +1399,7 @@ TEST_F(CiliumNetworkPolicyTest, HttpPolicyUpdateToMissingSDS) { EXPECT_EQ(version, "0"); EXPECT_FALSE(policy_map_->exists("10.1.2.3")); // No policy for the pod - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 80, {{":path", "/allowed"}})); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 80, {{":path", "/allowed"}})); // 1st update EXPECT_NO_THROW(version = updateFromYaml(R"EOF(version_info: "1" @@ -1416,16 +1421,16 @@ TEST_F(CiliumNetworkPolicyTest, HttpPolicyUpdateToMissingSDS) { EXPECT_EQ(version, "1"); EXPECT_TRUE(policy_map_->exists("10.1.2.3")); // Allowed remote ID, port, & path: - EXPECT_TRUE(IngressAllowed("10.1.2.3", 43, 80, {{":path", "/allowed"}})); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 80, {{":path", "/allowed"}})); // Wrong remote ID: - EXPECT_FALSE(IngressAllowed("10.1.2.3", 40, 80, {{":path", "/allowed"}})); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 40, 80, {{":path", "/allowed"}})); // Wrong port: - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 8080, {{":path", "/allowed"}})); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 8080, {{":path", "/allowed"}})); // Wrong path: - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 80, {{":path", "/notallowed"}})); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 80, {{":path", "/notallowed"}})); // No egress is allowed: - EXPECT_FALSE(EgressAllowed("10.1.2.3", 43, 80, {{":path", "/public"}})); + EXPECT_FALSE(egressAllowed("10.1.2.3", 43, 80, {{":path", "/public"}})); // 2nd update EXPECT_NO_THROW(version = updateFromYaml(R"EOF(version_info: "2" @@ -1451,13 +1456,13 @@ TEST_F(CiliumNetworkPolicyTest, HttpPolicyUpdateToMissingSDS) { EXPECT_EQ(version, "2"); EXPECT_TRUE(policy_map_->exists("10.1.2.3")); // Drop due to the missing SDS secret - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 80, {{":path", "/allowed"}})); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 80, {{":path", "/allowed"}})); // Wrong remote ID: - EXPECT_FALSE(IngressAllowed("10.1.2.3", 40, 80, {{":path", "/allowed"}})); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 40, 80, {{":path", "/allowed"}})); // Wrong port: - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 8080, {{":path", "/allowed"}})); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 8080, {{":path", "/allowed"}})); // Wrong path: - EXPECT_FALSE(IngressAllowed("10.1.2.3", 43, 80, {{":path", "/notallowed"}})); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 80, {{":path", "/notallowed"}})); } TEST_F(CiliumNetworkPolicyTest, TlsPolicyUpdate) { @@ -1470,9 +1475,9 @@ TEST_F(CiliumNetworkPolicyTest, TlsPolicyUpdate) { EXPECT_EQ(version, "0"); EXPECT_FALSE(policy_map_->exists("10.1.2.3")); // No policy for the pod - EXPECT_FALSE(TlsIngressAllowed("10.1.2.3", 43, 80, "", tls_socket_required, raw_socket_allowed)); + EXPECT_FALSE(tlsIngressAllowed("10.1.2.3", 43, 80, "", tls_socket_required, raw_socket_allowed)); // SNI does not make a difference - EXPECT_FALSE(TlsIngressAllowed("10.1.2.3", 43, 80, "example.com", tls_socket_required, + EXPECT_FALSE(tlsIngressAllowed("10.1.2.3", 43, 80, "example.com", tls_socket_required, raw_socket_allowed)); // 1st update without TLS requirements @@ -1490,27 +1495,27 @@ TEST_F(CiliumNetworkPolicyTest, TlsPolicyUpdate) { EXPECT_EQ(version, "1"); EXPECT_TRUE(policy_map_->exists("10.1.2.3")); // Allowed remote ID & port: - EXPECT_TRUE(TlsIngressAllowed("10.1.2.3", 43, 80, "example.com", tls_socket_required, + EXPECT_TRUE(tlsIngressAllowed("10.1.2.3", 43, 80, "example.com", tls_socket_required, raw_socket_allowed)); EXPECT_FALSE(tls_socket_required); EXPECT_TRUE(raw_socket_allowed); // SNI does not matter: - EXPECT_TRUE(TlsIngressAllowed("10.1.2.3", 43, 80, "", tls_socket_required, raw_socket_allowed)); + EXPECT_TRUE(tlsIngressAllowed("10.1.2.3", 43, 80, "", tls_socket_required, raw_socket_allowed)); EXPECT_FALSE(tls_socket_required); EXPECT_TRUE(raw_socket_allowed); // Wrong remote ID: - EXPECT_FALSE(TlsIngressAllowed("10.1.2.3", 40, 80, "example.com", tls_socket_required, + EXPECT_FALSE(tlsIngressAllowed("10.1.2.3", 40, 80, "example.com", tls_socket_required, raw_socket_allowed)); EXPECT_FALSE(tls_socket_required); EXPECT_FALSE(raw_socket_allowed); // Wrong port: - EXPECT_FALSE(TlsIngressAllowed("10.1.2.3", 43, 8080, "example.com", tls_socket_required, + EXPECT_FALSE(tlsIngressAllowed("10.1.2.3", 43, 8080, "example.com", tls_socket_required, raw_socket_allowed)); EXPECT_FALSE(tls_socket_required); EXPECT_FALSE(raw_socket_allowed); // No egress is allowed: - EXPECT_FALSE(TlsEgressAllowed("10.1.2.3", 43, 80, "", tls_socket_required, raw_socket_allowed)); + EXPECT_FALSE(tlsEgressAllowed("10.1.2.3", 43, 80, "", tls_socket_required, raw_socket_allowed)); EXPECT_FALSE(tls_socket_required); EXPECT_FALSE(raw_socket_allowed); @@ -1530,37 +1535,37 @@ TEST_F(CiliumNetworkPolicyTest, TlsPolicyUpdate) { EXPECT_EQ(version, "2"); EXPECT_TRUE(policy_map_->exists("10.1.2.3")); // Allowed remote ID, port, SNI: - EXPECT_TRUE(TlsIngressAllowed("10.1.2.3", 43, 80, "example.com", tls_socket_required, + EXPECT_TRUE(tlsIngressAllowed("10.1.2.3", 43, 80, "example.com", tls_socket_required, raw_socket_allowed)); EXPECT_FALSE(tls_socket_required); EXPECT_TRUE(raw_socket_allowed); // Allowed remote ID, port, incorrect SNI: - EXPECT_FALSE(TlsIngressAllowed("10.1.2.3", 43, 80, "www.example.com", tls_socket_required, + EXPECT_FALSE(tlsIngressAllowed("10.1.2.3", 43, 80, "www.example.com", tls_socket_required, raw_socket_allowed)); EXPECT_FALSE(tls_socket_required); EXPECT_FALSE(raw_socket_allowed); // Allowed remote ID, port, SNI: EXPECT_TRUE( - TlsIngressAllowed("10.1.2.3", 43, 80, "cilium.io", tls_socket_required, raw_socket_allowed)); + tlsIngressAllowed("10.1.2.3", 43, 80, "cilium.io", tls_socket_required, raw_socket_allowed)); EXPECT_FALSE(tls_socket_required); EXPECT_TRUE(raw_socket_allowed); // Missing SNI: - EXPECT_FALSE(TlsIngressAllowed("10.1.2.3", 43, 80, "", tls_socket_required, raw_socket_allowed)); + EXPECT_FALSE(tlsIngressAllowed("10.1.2.3", 43, 80, "", tls_socket_required, raw_socket_allowed)); EXPECT_FALSE(tls_socket_required); EXPECT_FALSE(raw_socket_allowed); // Wrong remote ID: - EXPECT_FALSE(TlsIngressAllowed("10.1.2.3", 40, 80, "example.com", tls_socket_required, + EXPECT_FALSE(tlsIngressAllowed("10.1.2.3", 40, 80, "example.com", tls_socket_required, raw_socket_allowed)); EXPECT_FALSE(tls_socket_required); EXPECT_FALSE(raw_socket_allowed); // Wrong port: - EXPECT_FALSE(TlsIngressAllowed("10.1.2.3", 43, 8080, "example.com", tls_socket_required, + EXPECT_FALSE(tlsIngressAllowed("10.1.2.3", 43, 8080, "example.com", tls_socket_required, raw_socket_allowed)); EXPECT_FALSE(tls_socket_required); EXPECT_FALSE(raw_socket_allowed); // No egress is allowed: - EXPECT_FALSE(TlsEgressAllowed("10.1.2.3", 43, 80, "", tls_socket_required, raw_socket_allowed)); + EXPECT_FALSE(tlsEgressAllowed("10.1.2.3", 43, 80, "", tls_socket_required, raw_socket_allowed)); EXPECT_FALSE(tls_socket_required); EXPECT_FALSE(raw_socket_allowed); @@ -1585,36 +1590,36 @@ TEST_F(CiliumNetworkPolicyTest, TlsPolicyUpdate) { EXPECT_TRUE(policy_map_->exists("10.1.2.3")); // Allowed remote ID, port, SNI: EXPECT_TRUE( - TlsEgressAllowed("10.1.2.3", 43, 80, "example.com", tls_socket_required, raw_socket_allowed)); + tlsEgressAllowed("10.1.2.3", 43, 80, "example.com", tls_socket_required, raw_socket_allowed)); EXPECT_TRUE(tls_socket_required); EXPECT_FALSE(raw_socket_allowed); // Allowed remote ID, port, incorrect SNI: - EXPECT_FALSE(TlsEgressAllowed("10.1.2.3", 43, 80, "www.example.com", tls_socket_required, + EXPECT_FALSE(tlsEgressAllowed("10.1.2.3", 43, 80, "www.example.com", tls_socket_required, raw_socket_allowed)); EXPECT_FALSE(tls_socket_required); EXPECT_FALSE(raw_socket_allowed); // Allowed remote ID, port, SNI: EXPECT_TRUE( - TlsEgressAllowed("10.1.2.3", 43, 80, "cilium.io", tls_socket_required, raw_socket_allowed)); + tlsEgressAllowed("10.1.2.3", 43, 80, "cilium.io", tls_socket_required, raw_socket_allowed)); EXPECT_TRUE(tls_socket_required); EXPECT_FALSE(raw_socket_allowed); // Missing SNI: - EXPECT_FALSE(TlsEgressAllowed("10.1.2.3", 43, 80, "", tls_socket_required, raw_socket_allowed)); + EXPECT_FALSE(tlsEgressAllowed("10.1.2.3", 43, 80, "", tls_socket_required, raw_socket_allowed)); EXPECT_FALSE(tls_socket_required); EXPECT_FALSE(raw_socket_allowed); // Wrong remote ID: EXPECT_FALSE( - TlsEgressAllowed("10.1.2.3", 40, 80, "example.com", tls_socket_required, raw_socket_allowed)); + tlsEgressAllowed("10.1.2.3", 40, 80, "example.com", tls_socket_required, raw_socket_allowed)); EXPECT_FALSE(tls_socket_required); EXPECT_FALSE(raw_socket_allowed); // Wrong port: - EXPECT_FALSE(TlsEgressAllowed("10.1.2.3", 43, 8080, "example.com", tls_socket_required, + EXPECT_FALSE(tlsEgressAllowed("10.1.2.3", 43, 8080, "example.com", tls_socket_required, raw_socket_allowed)); EXPECT_FALSE(tls_socket_required); EXPECT_FALSE(raw_socket_allowed); // No igress is allowed: - EXPECT_FALSE(TlsIngressAllowed("10.1.2.3", 43, 80, "", tls_socket_required, raw_socket_allowed)); + EXPECT_FALSE(tlsIngressAllowed("10.1.2.3", 43, 80, "", tls_socket_required, raw_socket_allowed)); EXPECT_FALSE(tls_socket_required); EXPECT_FALSE(raw_socket_allowed); @@ -1636,37 +1641,37 @@ TEST_F(CiliumNetworkPolicyTest, TlsPolicyUpdate) { EXPECT_EQ(version, "2"); EXPECT_TRUE(policy_map_->exists("10.1.2.3")); // Allowed remote ID, port, SNI: - EXPECT_TRUE(TlsIngressAllowed("10.1.2.3", 43, 80, "example.com", tls_socket_required, + EXPECT_TRUE(tlsIngressAllowed("10.1.2.3", 43, 80, "example.com", tls_socket_required, raw_socket_allowed)); EXPECT_TRUE(tls_socket_required); EXPECT_FALSE(raw_socket_allowed); // Allowed remote ID, port, incorrect SNI: - EXPECT_FALSE(TlsIngressAllowed("10.1.2.3", 43, 80, "www.example.com", tls_socket_required, + EXPECT_FALSE(tlsIngressAllowed("10.1.2.3", 43, 80, "www.example.com", tls_socket_required, raw_socket_allowed)); EXPECT_FALSE(tls_socket_required); EXPECT_FALSE(raw_socket_allowed); // Allowed remote ID, port, SNI: EXPECT_TRUE( - TlsIngressAllowed("10.1.2.3", 43, 80, "cilium.io", tls_socket_required, raw_socket_allowed)); + tlsIngressAllowed("10.1.2.3", 43, 80, "cilium.io", tls_socket_required, raw_socket_allowed)); EXPECT_TRUE(tls_socket_required); EXPECT_FALSE(raw_socket_allowed); // Missing SNI: - EXPECT_FALSE(TlsIngressAllowed("10.1.2.3", 43, 80, "", tls_socket_required, raw_socket_allowed)); + EXPECT_FALSE(tlsIngressAllowed("10.1.2.3", 43, 80, "", tls_socket_required, raw_socket_allowed)); EXPECT_FALSE(tls_socket_required); EXPECT_FALSE(raw_socket_allowed); // Wrong remote ID: - EXPECT_FALSE(TlsIngressAllowed("10.1.2.3", 40, 80, "example.com", tls_socket_required, + EXPECT_FALSE(tlsIngressAllowed("10.1.2.3", 40, 80, "example.com", tls_socket_required, raw_socket_allowed)); EXPECT_FALSE(tls_socket_required); EXPECT_FALSE(raw_socket_allowed); // Wrong port: - EXPECT_FALSE(TlsIngressAllowed("10.1.2.3", 43, 8080, "example.com", tls_socket_required, + EXPECT_FALSE(tlsIngressAllowed("10.1.2.3", 43, 8080, "example.com", tls_socket_required, raw_socket_allowed)); EXPECT_FALSE(tls_socket_required); EXPECT_FALSE(raw_socket_allowed); // No egress is allowed: - EXPECT_FALSE(TlsEgressAllowed("10.1.2.3", 43, 80, "", tls_socket_required, raw_socket_allowed)); + EXPECT_FALSE(tlsEgressAllowed("10.1.2.3", 43, 80, "", tls_socket_required, raw_socket_allowed)); EXPECT_FALSE(tls_socket_required); EXPECT_FALSE(raw_socket_allowed); @@ -1688,36 +1693,36 @@ TEST_F(CiliumNetworkPolicyTest, TlsPolicyUpdate) { EXPECT_TRUE(policy_map_->exists("10.1.2.3")); // Allowed remote ID, port, SNI: EXPECT_TRUE( - TlsEgressAllowed("10.1.2.3", 43, 80, "example.com", tls_socket_required, raw_socket_allowed)); + tlsEgressAllowed("10.1.2.3", 43, 80, "example.com", tls_socket_required, raw_socket_allowed)); EXPECT_TRUE(tls_socket_required); EXPECT_FALSE(raw_socket_allowed); // Allowed remote ID, port, SNI: - EXPECT_TRUE(TlsEgressAllowed("10.1.2.3", 43, 80, "www.example.com", tls_socket_required, + EXPECT_TRUE(tlsEgressAllowed("10.1.2.3", 43, 80, "www.example.com", tls_socket_required, raw_socket_allowed)); EXPECT_TRUE(tls_socket_required); EXPECT_FALSE(raw_socket_allowed); // Allowed remote ID, port, SNI: EXPECT_TRUE( - TlsEgressAllowed("10.1.2.3", 43, 80, "cilium.io", tls_socket_required, raw_socket_allowed)); + tlsEgressAllowed("10.1.2.3", 43, 80, "cilium.io", tls_socket_required, raw_socket_allowed)); EXPECT_TRUE(tls_socket_required); EXPECT_FALSE(raw_socket_allowed); // Empty SNI: - EXPECT_TRUE(TlsEgressAllowed("10.1.2.3", 43, 80, "", tls_socket_required, raw_socket_allowed)); + EXPECT_TRUE(tlsEgressAllowed("10.1.2.3", 43, 80, "", tls_socket_required, raw_socket_allowed)); EXPECT_TRUE(tls_socket_required); EXPECT_FALSE(raw_socket_allowed); // Wrong remote ID: EXPECT_FALSE( - TlsEgressAllowed("10.1.2.3", 40, 80, "example.com", tls_socket_required, raw_socket_allowed)); + tlsEgressAllowed("10.1.2.3", 40, 80, "example.com", tls_socket_required, raw_socket_allowed)); EXPECT_FALSE(tls_socket_required); EXPECT_FALSE(raw_socket_allowed); // Wrong port: - EXPECT_FALSE(TlsEgressAllowed("10.1.2.3", 43, 8080, "example.com", tls_socket_required, + EXPECT_FALSE(tlsEgressAllowed("10.1.2.3", 43, 8080, "example.com", tls_socket_required, raw_socket_allowed)); EXPECT_FALSE(tls_socket_required); EXPECT_FALSE(raw_socket_allowed); // No igress is allowed: - EXPECT_FALSE(TlsIngressAllowed("10.1.2.3", 43, 80, "", tls_socket_required, raw_socket_allowed)); + EXPECT_FALSE(tlsIngressAllowed("10.1.2.3", 43, 80, "", tls_socket_required, raw_socket_allowed)); EXPECT_FALSE(tls_socket_required); EXPECT_FALSE(raw_socket_allowed); } diff --git a/tests/cilium_tls_http_integration_test.cc b/tests/cilium_tls_http_integration_test.cc index f1f62cb89..27f6ed34c 100644 --- a/tests/cilium_tls_http_integration_test.cc +++ b/tests/cilium_tls_http_integration_test.cc @@ -230,7 +230,7 @@ const std::string BASIC_TLS_POLICY_fmt = R"EOF(version_info: "0" class CiliumHttpTLSIntegrationTest : public CiliumHttpIntegrationTest { public: CiliumHttpTLSIntegrationTest(const std::string& config) : CiliumHttpIntegrationTest(config) {} - ~CiliumHttpTLSIntegrationTest() {} + ~CiliumHttpTLSIntegrationTest() override = default; void initialize() override { CiliumHttpIntegrationTest::initialize(); @@ -275,6 +275,7 @@ class CiliumHttpTLSIntegrationTest : public CiliumHttpIntegrationTest { auto server_config_or_error = Extensions::TransportSockets::Tls::ServerContextConfigImpl::create(tls_context, factory_context_, false); + // NOLINTNEXTLINE(performance-unnecessary-copy-initialization) THROW_IF_NOT_OK(server_config_or_error.status()); auto cfg = std::move(server_config_or_error.value()); @@ -282,6 +283,7 @@ class CiliumHttpTLSIntegrationTest : public CiliumHttpIntegrationTest { auto factory_or_error = Extensions::TransportSockets::Tls::ServerSslSocketFactory::create( std::move(cfg), context_manager_, *upstream_stats_store->rootScope(), std::vector{}); + // NOLINTNEXTLINE(performance-unnecessary-copy-initialization) THROW_IF_NOT_OK(factory_or_error.status()); return std::move(factory_or_error.value()); } @@ -297,7 +299,7 @@ class CiliumHttpTLSIntegrationTest : public CiliumHttpIntegrationTest { }; } - void Denied(Http::TestRequestHeaderMapImpl&& headers) { + void denied(Http::TestRequestHeaderMapImpl&& headers) { initialize(); auto response = codec_client_->makeHeaderOnlyRequest(headers); ASSERT_TRUE(response->waitForEndStream()); @@ -307,7 +309,7 @@ class CiliumHttpTLSIntegrationTest : public CiliumHttpIntegrationTest { cleanupUpstreamAndDownstream(); } - void Failed(Http::TestRequestHeaderMapImpl&& headers) { + void failed(Http::TestRequestHeaderMapImpl&& headers) { initialize(); auto response = codec_client_->makeHeaderOnlyRequest(headers); ASSERT_TRUE(response->waitForEndStream()); @@ -317,7 +319,7 @@ class CiliumHttpTLSIntegrationTest : public CiliumHttpIntegrationTest { cleanupUpstreamAndDownstream(); } - void Accepted(Http::TestRequestHeaderMapImpl&& headers) { + void accepted(Http::TestRequestHeaderMapImpl&& headers) { initialize(); auto response = sendRequestAndWaitForResponse(headers, 0, default_response_headers_, 0); @@ -348,38 +350,38 @@ INSTANTIATE_TEST_SUITE_P(IpVersions, CiliumTLSHttpIntegrationTest, testing::ValuesIn(TestEnvironment::getIpVersionsForTest())); TEST_P(CiliumTLSHttpIntegrationTest, DeniedPathPrefix) { - Denied({{":method", "GET"}, {":path", "/prefix"}, {":authority", "localhost"}}); + denied({{":method", "GET"}, {":path", "/prefix"}, {":authority", "localhost"}}); } TEST_P(CiliumTLSHttpIntegrationTest, AllowedPathPrefix) { - Accepted({{":method", "GET"}, {":path", "/allowed"}, {":authority", "localhost"}}); + accepted({{":method", "GET"}, {":path", "/allowed"}, {":authority", "localhost"}}); } TEST_P(CiliumTLSHttpIntegrationTest, AllowedPathPrefixStrippedHeader) { - Accepted({{":method", "GET"}, + accepted({{":method", "GET"}, {":path", "/allowed"}, {":authority", "localhost"}, {"x-envoy-original-dst-host", "1.1.1.1:9999"}}); } TEST_P(CiliumTLSHttpIntegrationTest, AllowedPathRegex) { - Accepted({{":method", "GET"}, {":path", "/maybe/public"}, {":authority", "localhost"}}); + accepted({{":method", "GET"}, {":path", "/maybe/public"}, {":authority", "localhost"}}); } TEST_P(CiliumTLSHttpIntegrationTest, DeniedPath) { - Denied({{":method", "GET"}, {":path", "/maybe/private"}, {":authority", "localhost"}}); + denied({{":method", "GET"}, {":path", "/maybe/private"}, {":authority", "localhost"}}); } TEST_P(CiliumTLSHttpIntegrationTest, DeniedMethod) { - Denied({{":method", "POST"}, {":path", "/maybe/private"}, {":authority", "localhost"}}); + denied({{":method", "POST"}, {":path", "/maybe/private"}, {":authority", "localhost"}}); } TEST_P(CiliumTLSHttpIntegrationTest, AcceptedMethod) { - Accepted({{":method", "PUT"}, {":path", "/public/opinions"}, {":authority", "localhost"}}); + accepted({{":method", "PUT"}, {":path", "/public/opinions"}, {":authority", "localhost"}}); } TEST_P(CiliumTLSHttpIntegrationTest, L3DeniedPath) { - Denied({{":method", "GET"}, {":path", "/only-2-allowed"}, {":authority", "localhost"}}); + denied({{":method", "GET"}, {":path", "/only-2-allowed"}, {":authority", "localhost"}}); } } // namespace Cilium diff --git a/tests/cilium_tls_integration.cc b/tests/cilium_tls_integration.cc index f931087e0..57d15db27 100644 --- a/tests/cilium_tls_integration.cc +++ b/tests/cilium_tls_integration.cc @@ -40,11 +40,13 @@ createClientSslTransportSocketFactory(Ssl::ContextManager& context_manager, Api: ON_CALL(mock_factory_ctx.server_context_, api()).WillByDefault(testing::ReturnRef(api)); auto cfg_or_error = Extensions::TransportSockets::Tls::ClientContextConfigImpl::create( tls_context, mock_factory_ctx); + // NOLINTNEXTLINE(performance-unnecessary-copy-initialization) THROW_IF_NOT_OK(cfg_or_error.status()); auto cfg = std::move(cfg_or_error.value()); static auto* client_stats_store = new Stats::TestIsolatedStoreImpl(); auto factory_or_error = Extensions::TransportSockets::Tls::ClientSslSocketFactory::create( std::move(cfg), context_manager, *client_stats_store->rootScope()); + // NOLINTNEXTLINE(performance-unnecessary-copy-initialization) THROW_IF_NOT_OK(factory_or_error.status()); return std::move(factory_or_error.value()); } diff --git a/tests/cilium_tls_tcp_integration_test.cc b/tests/cilium_tls_tcp_integration_test.cc index 4424a1acc..84052f13d 100644 --- a/tests/cilium_tls_tcp_integration_test.cc +++ b/tests/cilium_tls_tcp_integration_test.cc @@ -147,6 +147,7 @@ class CiliumTLSIntegrationTest : public CiliumTcpIntegrationTest { auto cfg_or_error = Extensions::TransportSockets::Tls::ServerContextConfigImpl::create( tls_context, factory_context_, false); + // NOLINTNEXTLINE(performance-unnecessary-copy-initialization) THROW_IF_NOT_OK(cfg_or_error.status()); auto cfg = std::move(cfg_or_error.value()); @@ -154,6 +155,7 @@ class CiliumTLSIntegrationTest : public CiliumTcpIntegrationTest { auto server_or_error = Extensions::TransportSockets::Tls::ServerSslSocketFactory::create( std::move(cfg), context_manager_, *upstream_stats_store->rootScope(), std::vector{}); + // NOLINTNEXTLINE(performance-unnecessary-copy-initialization) THROW_IF_NOT_OK(server_or_error.status()); return std::move(server_or_error.value()); } diff --git a/tests/cilium_websocket_decap_integration_test.cc b/tests/cilium_websocket_decap_integration_test.cc index 253006843..498f2d415 100644 --- a/tests/cilium_websocket_decap_integration_test.cc +++ b/tests/cilium_websocket_decap_integration_test.cc @@ -115,7 +115,7 @@ class CiliumWebSocketIntegrationTest : public CiliumHttpIntegrationTest { GetParam()); } - void Denied(Http::TestRequestHeaderMapImpl&& headers) { + void denied(Http::TestRequestHeaderMapImpl&& headers) { codec_client_ = makeHttpConnection(lookupPort("http")); auto response = codec_client_->makeHeaderOnlyRequest(headers); ASSERT_TRUE(response->waitForEndStream()); @@ -131,7 +131,7 @@ INSTANTIATE_TEST_SUITE_P(IpVersions, CiliumWebSocketIntegrationTest, TEST_P(CiliumWebSocketIntegrationTest, DeniedNonWebSocket) { initialize(); - Denied({{":method", "GET"}, {":path", "/"}, {":authority", "host"}}); + denied({{":method", "GET"}, {":path", "/"}, {":authority", "host"}}); } TEST_P(CiliumWebSocketIntegrationTest, AcceptedWebSocket) { @@ -348,7 +348,7 @@ TEST_P(CiliumWebSocketIntegrationTest, AcceptedWebSocket) { ASSERT_TRUE(fake_upstream_connection->waitForData(seen_data_len + 13 - 5, &data)); ASSERT_EQ(data.substr(seen_data_len), msg2.data() + 5); - seen_data_len = data.length(); + // seen_data_len = data.length(); // not used after, no need to update ASSERT_TRUE(fake_upstream_connection->write(msg2)); diff --git a/tests/cilium_websocket_encap_integration_test.cc b/tests/cilium_websocket_encap_integration_test.cc index 9722d7d25..126a857ab 100644 --- a/tests/cilium_websocket_encap_integration_test.cc +++ b/tests/cilium_websocket_encap_integration_test.cc @@ -158,7 +158,7 @@ static const char EXPECTED_HANDSHAKE_FMT[] = namespace { -size_t normalize_x_request_id(std::string& headers) { +size_t normalizeXRequestId(std::string& headers) { auto idx = headers.find(X_REQUEST_ID_HEADER HEADER_SEPARATOR); if (idx != std::string::npos) { idx += sizeof(X_REQUEST_ID_HEADER HEADER_SEPARATOR) - 1; // w/o the \0 in the end @@ -189,7 +189,7 @@ TEST_P(CiliumWebSocketIntegrationTest, CiliumWebSocketHandshakeNonHTTPResponse) ASSERT_TRUE(fake_upstream_connection->waitForData(expected_handshake.length(), &received_handshake, std::chrono::seconds(10))); - ASSERT_EQ(normalize_x_request_id(received_handshake), sizeof(X_REQUEST_ID_VALUE) - 1); + ASSERT_EQ(normalizeXRequestId(received_handshake), sizeof(X_REQUEST_ID_VALUE) - 1); ASSERT_EQ(received_handshake, expected_handshake); ASSERT_TRUE(fake_upstream_connection->write("\x82\x5" "world")); @@ -217,7 +217,7 @@ TEST_P(CiliumWebSocketIntegrationTest, CiliumWebSocketHandshakeInvalidResponse) fmt::format(fmt::runtime(EXPECTED_HANDSHAKE_FMT), original_dst_address->asString()); std::string received_data; ASSERT_TRUE(fake_upstream_connection->waitForData(expected_handshake.length(), &received_data)); - ASSERT_EQ(normalize_x_request_id(received_data), sizeof(X_REQUEST_ID_VALUE) - 1); + ASSERT_EQ(normalizeXRequestId(received_data), sizeof(X_REQUEST_ID_VALUE) - 1); ASSERT_EQ(received_data, expected_handshake); // Handshake response with invalid hash value @@ -244,7 +244,7 @@ TEST_P(CiliumWebSocketIntegrationTest, CiliumWebSocketHandshakeSuccess) { fmt::format(fmt::runtime(EXPECTED_HANDSHAKE_FMT), original_dst_address->asString()); std::string received_data; ASSERT_TRUE(fake_upstream_connection->waitForData(expected_handshake.length(), &received_data)); - ASSERT_EQ(normalize_x_request_id(received_data), sizeof(X_REQUEST_ID_VALUE) - 1); + ASSERT_EQ(normalizeXRequestId(received_data), sizeof(X_REQUEST_ID_VALUE) - 1); ASSERT_EQ(received_data, expected_handshake); // Handshake response with the correct hash value @@ -283,7 +283,7 @@ TEST_P(CiliumWebSocketIntegrationTest, CiliumWebSocketHandshakeNoData) { fmt::format(fmt::runtime(EXPECTED_HANDSHAKE_FMT), original_dst_address->asString()); std::string received_data; ASSERT_TRUE(fake_upstream_connection->waitForData(expected_handshake.length(), &received_data)); - ASSERT_EQ(normalize_x_request_id(received_data), sizeof(X_REQUEST_ID_VALUE) - 1); + ASSERT_EQ(normalizeXRequestId(received_data), sizeof(X_REQUEST_ID_VALUE) - 1); ASSERT_EQ(received_data, expected_handshake); // Handshake response with the correct hash value @@ -314,7 +314,7 @@ TEST_P(CiliumWebSocketIntegrationTest, CiliumWebSocketDownstreamDisconnect) { fmt::format(fmt::runtime(EXPECTED_HANDSHAKE_FMT), original_dst_address->asString()); std::string received_data; ASSERT_TRUE(fake_upstream_connection->waitForData(expected_handshake.length(), &received_data)); - ASSERT_EQ(normalize_x_request_id(received_data), sizeof(X_REQUEST_ID_VALUE) - 1); + ASSERT_EQ(normalizeXRequestId(received_data), sizeof(X_REQUEST_ID_VALUE) - 1); ASSERT_EQ(received_data, expected_handshake); // Handshake response with the correct hash value @@ -368,7 +368,7 @@ TEST_P(CiliumWebSocketIntegrationTest, CiliumWebSocketLargeWrite) { fmt::format(fmt::runtime(EXPECTED_HANDSHAKE_FMT), original_dst_address->asString()); std::string received_data; ASSERT_TRUE(fake_upstream_connection->waitForData(expected_handshake.length(), &received_data)); - ASSERT_EQ(normalize_x_request_id(received_data), sizeof(X_REQUEST_ID_VALUE) - 1); + ASSERT_EQ(normalizeXRequestId(received_data), sizeof(X_REQUEST_ID_VALUE) - 1); ASSERT_EQ(received_data, expected_handshake); // Handshake response with the correct hash value @@ -430,7 +430,7 @@ TEST_P(CiliumWebSocketIntegrationTest, CiliumWebSocketDownstreamFlush) { fmt::format(fmt::runtime(EXPECTED_HANDSHAKE_FMT), original_dst_address->asString()); std::string received_data; ASSERT_TRUE(fake_upstream_connection->waitForData(expected_handshake.length(), &received_data)); - ASSERT_EQ(normalize_x_request_id(received_data), sizeof(X_REQUEST_ID_VALUE) - 1); + ASSERT_EQ(normalizeXRequestId(received_data), sizeof(X_REQUEST_ID_VALUE) - 1); ASSERT_EQ(received_data, expected_handshake); // Handshake response with the correct hash value @@ -485,7 +485,7 @@ TEST_P(CiliumWebSocketIntegrationTest, CiliumWebSocketUpstreamFlush) { fmt::format(fmt::runtime(EXPECTED_HANDSHAKE_FMT), original_dst_address->asString()); std::string received_data; ASSERT_TRUE(fake_upstream_connection->waitForData(expected_handshake.length(), &received_data)); - ASSERT_EQ(normalize_x_request_id(received_data), sizeof(X_REQUEST_ID_VALUE) - 1); + ASSERT_EQ(normalizeXRequestId(received_data), sizeof(X_REQUEST_ID_VALUE) - 1); ASSERT_EQ(received_data, expected_handshake); // Handshake response with the correct hash value @@ -532,7 +532,7 @@ TEST_P(CiliumWebSocketIntegrationTest, CiliumWebSocketUpstreamFlushEnvoyExit) { fmt::format(fmt::runtime(EXPECTED_HANDSHAKE_FMT), original_dst_address->asString()); std::string received_data; ASSERT_TRUE(fake_upstream_connection->waitForData(expected_handshake.length(), &received_data)); - ASSERT_EQ(normalize_x_request_id(received_data), sizeof(X_REQUEST_ID_VALUE) - 1); + ASSERT_EQ(normalizeXRequestId(received_data), sizeof(X_REQUEST_ID_VALUE) - 1); ASSERT_EQ(received_data, expected_handshake); // Handshake response with the correct hash value diff --git a/tests/health_check_sink_server.cc b/tests/health_check_sink_server.cc index 075ec8685..a5ed3ac94 100644 --- a/tests/health_check_sink_server.cc +++ b/tests/health_check_sink_server.cc @@ -20,7 +20,7 @@ HealthCheckSinkServer::HealthCheckSinkServer(const std::string path) : UDSServer(path, std::bind(&HealthCheckSinkServer::msgCallback, this, std::placeholders::_1)) { } -HealthCheckSinkServer::~HealthCheckSinkServer() {} +HealthCheckSinkServer::~HealthCheckSinkServer() = default; void HealthCheckSinkServer::clear() { absl::MutexLock lock(&mutex_); diff --git a/tests/health_check_sink_server.h b/tests/health_check_sink_server.h index 03226775e..72851ea18 100644 --- a/tests/health_check_sink_server.h +++ b/tests/health_check_sink_server.h @@ -28,8 +28,9 @@ class HealthCheckSinkServer : public UDSServer { template bool expectEventTo(P&& pred, std::chrono::milliseconds timeout = TestUtility::DefaultTimeout) { auto maybe_event = waitForEvent(timeout); - if (maybe_event.has_value()) + if (maybe_event.has_value()) { return pred(maybe_event.value()); + } return false; } diff --git a/tests/uds_server.cc b/tests/uds_server.cc index cbff6cd3d..b8bb3322b 100644 --- a/tests/uds_server.cc +++ b/tests/uds_server.cc @@ -36,14 +36,14 @@ UDSServer::UDSServer(const std::string& path, std::functionasStringView()); if (::bind(fd_, addr_->sockAddr(), addr_->sockAddrLen()) == -1) { ENVOY_LOG(warn, "Bind to {} failed: {}", addr_->asStringView(), Envoy::errorDetails(errno)); - Close(); + close(); return; } ENVOY_LOG(trace, "Listening on {}", addr_->asStringView()); if (::listen(fd_, 5) == -1) { ENVOY_LOG(warn, "Listen on {} failed: {}", addr_->asStringView(), Envoy::errorDetails(errno)); - Close(); + close(); return; } @@ -54,7 +54,7 @@ UDSServer::UDSServer(const std::string& path, std::function= 0) { - Close(); + close(); ENVOY_LOG(trace, "Waiting on unix domain socket server to close: {}", Envoy::errorDetails(errno)); thread_->join(); @@ -62,7 +62,7 @@ UDSServer::~UDSServer() { } } -void UDSServer::Close() { +void UDSServer::close() { ::shutdown(fd_, SHUT_RD); ::shutdown(fd2_, SHUT_RD); errno = 0; @@ -94,8 +94,9 @@ void UDSServer::threadRoutine() { ENVOY_LOG(trace, "Unix domain socket server blocking recv on fd: {}", fd2_.load()); ssize_t received = ::recv(fd2_, buf, sizeof(buf), 0); if (received < 0) { - if (errno == EINTR) + if (errno == EINTR) { continue; + } ENVOY_LOG(warn, "Unix domain socket server recv on fd {} failed: {}", fd2_.load(), Envoy::errorDetails(errno)); break; diff --git a/tests/uds_server.h b/tests/uds_server.h index 8fb4bf043..68028d97b 100644 --- a/tests/uds_server.h +++ b/tests/uds_server.h @@ -18,7 +18,7 @@ class UDSServer : public Logger::Loggable { ~UDSServer(); private: - void Close(); + void close(); void threadRoutine(); std::function msg_cb_; From 3271cda6bc47cc6b964aa7e99d18de4a4911b5df Mon Sep 17 00:00:00 2001 From: Jarno Rajahalme Date: Sun, 2 Feb 2025 22:10:41 +0100 Subject: [PATCH 11/12] proxylib: rename local variables Rename 'ops' as 'op_slice' and 'ops_' as 'ops'. This helps reduce confusion with member variables that have the '_' suffix. Signed-off-by: Jarno Rajahalme --- cilium/proxylib.cc | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/cilium/proxylib.cc b/cilium/proxylib.cc index 7875a0d96..126bffd8f 100644 --- a/cilium/proxylib.cc +++ b/cilium/proxylib.cc @@ -196,15 +196,15 @@ FilterResult GoFilter::Instance::onIo(bool reply, Buffer::Instance& data, bool e dir.need_bytes_ = 0; const int max_ops = 16; // Make shorter for testing purposes - FilterOp ops_[max_ops]; - GoFilterOpSlice ops(ops_, max_ops); + FilterOp ops[max_ops]; + GoFilterOpSlice op_slice(ops, max_ops); FilterResult res; bool terminal_op_seen = false; bool inject_buf_exhausted = false; do { - ops.reset(); + op_slice.reset(); Buffer::RawSliceVector raw_slices = input.getRawSlices(); int64_t total_length = 0; @@ -221,14 +221,14 @@ FilterResult GoFilter::Instance::onIo(bool reply, Buffer::Instance& data, bool e ENVOY_CONN_LOG(trace, "Cilium Network::OnIO: Calling go module with {} bytes of data", conn_, total_length); - res = (*parent_.go_on_data_)(connection_id_, reply, end_stream, &input_slices, &ops); + res = (*parent_.go_on_data_)(connection_id_, reply, end_stream, &input_slices, &op_slice); ENVOY_CONN_LOG(trace, "Cilium Network::OnIO: \'go_on_data\' returned {}, ops({})", conn_, - toString(res), ops.len()); + toString(res), op_slice.len()); if (res == FILTER_OK) { // Process all returned filter operations. - for (int i = 0; i < ops.len(); i++) { - auto op = ops_[i].op; - auto n_bytes = ops_[i].n_bytes; + for (int i = 0; i < op_slice.len(); i++) { + auto op = ops[i].op; + auto n_bytes = ops[i].n_bytes; if (n_bytes == 0) { ENVOY_CONN_LOG(warn, "Cilium Network::OnIO: INVALID op ({}) length: {} bytes", conn_, op, @@ -317,7 +317,7 @@ FilterResult GoFilter::Instance::onIo(bool reply, Buffer::Instance& data, bool e dir.inject_slice_.reset(); // Loop back if ops or inject buffer was exhausted - } while (!terminal_op_seen && (ops.len() == max_ops || inject_buf_exhausted)); + } while (!terminal_op_seen && (op_slice.len() == max_ops || inject_buf_exhausted)); if (output.length() < 100) { ENVOY_CONN_LOG(debug, "Cilium Network::OnIO: Output on return: {}", conn_, output.toString()); From 8279d8b480bb417d122887a5c9fe260cbb822d31 Mon Sep 17 00:00:00 2001 From: Jarno Rajahalme Date: Sun, 2 Feb 2025 20:12:28 +0100 Subject: [PATCH 12/12] treewide: tidy local variables to lower_case Signed-off-by: Jarno Rajahalme --- .clang-tidy | 2 + cilium/bpf_metadata.cc | 12 +++--- cilium/host_map.cc | 8 ++-- cilium/host_map.h | 4 +- cilium/l7policy.cc | 6 +-- cilium/network_filter.cc | 20 ++++----- cilium/network_policy.cc | 18 ++++---- cilium/socket_option_ip_transparent.cc | 6 +-- cilium/socket_option_source_address.cc | 6 +-- cilium/tls_wrapper.cc | 8 ++-- tests/cilium_tls_http_integration_test.cc | 6 +-- ...cilium_websocket_decap_integration_test.cc | 42 +++++++++---------- tests/health_check_sink_test.cc | 10 ++--- tests/metadata_config_test.cc | 4 +- 14 files changed, 77 insertions(+), 75 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 954513add..9cd4a3d2f 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -106,6 +106,8 @@ CheckOptions: value: 'CamelCase' - key: readability-identifier-naming.FunctionCase value: 'camelBack' +- key: readability-identifier-naming.LocalVariableCase + value: 'lower_case' HeaderFilterRegex: '^./source/.*|^./contrib/.*|^./test/.*|^./envoy/.*' diff --git a/cilium/bpf_metadata.cc b/cilium/bpf_metadata.cc index 93e2294d8..dc560cbf2 100644 --- a/cilium/bpf_metadata.cc +++ b/cilium/bpf_metadata.cc @@ -300,28 +300,28 @@ uint32_t Config::resolveSourceIdentity(const PolicyInstance& policy, IPAddressPair Config::getIPAddressPairFrom(const Network::Address::InstanceConstSharedPtr source_address, const IPAddressPair& addresses) { - auto addressPair = IPAddressPair(); + auto address_pair = IPAddressPair(); switch (source_address->ip()->version()) { case Network::Address::IpVersion::v4: - addressPair.ipv4_ = source_address; + address_pair.ipv4_ = source_address; if (addresses.ipv6_) { sockaddr_in6 sa6 = *reinterpret_cast(addresses.ipv6_->sockAddr()); sa6.sin6_port = htons(source_address->ip()->port()); - addressPair.ipv6_ = std::make_shared(sa6); + address_pair.ipv6_ = std::make_shared(sa6); } break; case Network::Address::IpVersion::v6: - addressPair.ipv6_ = source_address; + address_pair.ipv6_ = source_address; if (addresses.ipv4_) { sockaddr_in sa4 = *reinterpret_cast(addresses.ipv4_->sockAddr()); sa4.sin_port = htons(source_address->ip()->port()); - addressPair.ipv4_ = std::make_shared(&sa4); + address_pair.ipv4_ = std::make_shared(&sa4); } break; } - return addressPair; + return address_pair; } const Network::Address::Ip* Config::selectIPVersion(const Network::Address::IpVersion version, diff --git a/cilium/host_map.cc b/cilium/host_map.cc index e7ab6c854..75a35b9d2 100644 --- a/cilium/host_map.cc +++ b/cilium/host_map.cc @@ -32,15 +32,15 @@ namespace Cilium { template unsigned int checkPrefix(T addr, bool have_prefix, unsigned int plen, absl::string_view host) { - const unsigned int PLEN_MAX = sizeof(T) * 8; + const unsigned int plen_max = sizeof(T) * 8; if (!have_prefix) { - return PLEN_MAX; + return plen_max; } - if (plen > PLEN_MAX) { + if (plen > plen_max) { throw EnvoyException(fmt::format("NetworkPolicyHosts: Invalid prefix length in \'{}\'", host)); } // Check for 1-bits after the prefix - if ((plen == 0 && addr) || (plen > 0 && addr & ntoh((T(1) << (PLEN_MAX - plen)) - 1))) { + if ((plen == 0 && addr) || (plen > 0 && addr & ntoh((T(1) << (plen_max - plen)) - 1))) { throw EnvoyException(fmt::format("NetworkPolicyHosts: Non-prefix bits set in \'{}\'", host)); } return plen; diff --git a/cilium/host_map.h b/cilium/host_map.h index dda9943e0..c50e88c7f 100644 --- a/cilium/host_map.h +++ b/cilium/host_map.h @@ -62,8 +62,8 @@ template <> inline absl::uint128 hton(absl::uint128 addr) { } template I masked(I addr, unsigned int plen) { - const unsigned int PLEN_MAX = sizeof(I) * 8; - return plen == 0 ? I(0) : addr & ~hton((I(1) << (PLEN_MAX - plen)) - 1); + const unsigned int plen_max = sizeof(I) * 8; + return plen == 0 ? I(0) : addr & ~hton((I(1) << (plen_max - plen)) - 1); }; class PolicyHostDecoder : public Envoy::Config::OpaqueResourceDecoder { diff --git a/cilium/l7policy.cc b/cilium/l7policy.cc index c6c66ad54..47aebc6df 100644 --- a/cilium/l7policy.cc +++ b/cilium/l7policy.cc @@ -282,14 +282,14 @@ Http::FilterHeadersStatus AccessFilter::encodeHeaders(Http::ResponseHeaderMap& h if (!log_entry_->request_logged_) { // Default logging local errors as "forwarded". // The response log will contain the locally generated HTTP error code. - auto logType = ::cilium::EntryType::Request; + auto log_type = ::cilium::EntryType::Request; if (headers.Status()->value() == "403") { // Log as a denied request. - logType = ::cilium::EntryType::Denied; + log_type = ::cilium::EntryType::Denied; config_->stats_.access_denied_.inc(); } - config_->log(*log_entry_, logType); + config_->log(*log_entry_, log_type); } // Log the response diff --git a/cilium/network_filter.cc b/cilium/network_filter.cc index 399872a82..e6660021a 100644 --- a/cilium/network_filter.cc +++ b/cilium/network_filter.cc @@ -122,19 +122,19 @@ Network::FilterStatus Instance::onNewConnection() { // Pass metadata from tls_inspector to the filterstate, if any & not already // set via upstream cluster config. if (!sni.empty()) { - auto filterState = conn.streamInfo().filterState(); + auto filter_state = conn.streamInfo().filterState(); auto have_sni = - filterState->hasData(Network::UpstreamServerName::key()); - auto have_san = filterState->hasData( + filter_state->hasData(Network::UpstreamServerName::key()); + auto have_san = filter_state->hasData( Network::UpstreamSubjectAltNames::key()); if (!have_sni && !have_san) { - filterState->setData(Network::UpstreamServerName::key(), - std::make_unique(sni), - StreamInfo::FilterState::StateType::Mutable); - filterState->setData(Network::UpstreamSubjectAltNames::key(), - std::make_unique( - std::vector{std::string(sni)}), - StreamInfo::FilterState::StateType::Mutable); + filter_state->setData(Network::UpstreamServerName::key(), + std::make_unique(sni), + StreamInfo::FilterState::StateType::Mutable); + filter_state->setData(Network::UpstreamSubjectAltNames::key(), + std::make_unique( + std::vector{std::string(sni)}), + StreamInfo::FilterState::StateType::Mutable); } } diff --git a/cilium/network_policy.cc b/cilium/network_policy.cc index bfaac8f49..a4c7e095e 100644 --- a/cilium/network_policy.cc +++ b/cilium/network_policy.cc @@ -59,8 +59,8 @@ namespace Cilium { uint64_t NetworkPolicyMap::instance_id_ = 0; IPAddressPair::IPAddressPair(const cilium::NetworkPolicy& proto) { - for (const auto& ipAddr : proto.endpoint_ips()) { - auto ip = Network::Utility::parseInternetAddressNoThrow(ipAddr); + for (const auto& ip_addr : proto.endpoint_ips()) { + auto ip = Network::Utility::parseInternetAddressNoThrow(ip_addr); if (ip) { switch (ip->ip()->version()) { case Network::Address::IpVersion::v4: @@ -111,8 +111,8 @@ class HeaderMatch : public Logger::Loggable { } // Perform presence match if the value to match is empty - bool isPresentMatch = match_value->length() == 0; - if (isPresentMatch) { + bool is_present_match = match_value->length() == 0; + if (is_present_match) { matches = header_value.result().has_value(); } else if (header_value.result().has_value()) { const absl::string_view val = header_value.result().value(); @@ -151,7 +151,7 @@ class HeaderMatch : public Logger::Loggable { logMissing(log_entry, *match_value); return true; case cilium::HeaderMatch::DELETE_ON_MISMATCH: - if (isPresentMatch) { + if (is_present_match) { // presence match failed, nothing to do return true; } @@ -1372,12 +1372,12 @@ ProtobufTypes::MessagePtr NetworkPolicyMap::dumpNetworkPolicyConfigs(const Matchers::StringMatcher& name_matcher) { ENVOY_LOG(debug, "Writing NetworkPolicies to NetworkPoliciesConfigDump"); - std::vector policyEndpointIds; + std::vector policy_endpoint_ids; auto config_dump = std::make_unique(); for (const auto& item : *load()) { // filter duplicates (policies are stored per endpoint ip) - if (std::find(policyEndpointIds.begin(), policyEndpointIds.end(), - item.second->policy_proto_.endpoint_id()) != policyEndpointIds.end()) { + if (std::find(policy_endpoint_ids.begin(), policy_endpoint_ids.end(), + item.second->policy_proto_.endpoint_id()) != policy_endpoint_ids.end()) { continue; } @@ -1386,7 +1386,7 @@ NetworkPolicyMap::dumpNetworkPolicyConfigs(const Matchers::StringMatcher& name_m } config_dump->mutable_networkpolicies()->Add()->CopyFrom(item.second->policy_proto_); - policyEndpointIds.emplace_back(item.second->policy_proto_.endpoint_id()); + policy_endpoint_ids.emplace_back(item.second->policy_proto_.endpoint_id()); } return config_dump; diff --git a/cilium/socket_option_ip_transparent.cc b/cilium/socket_option_ip_transparent.cc index 8aa45f273..1c2175ec6 100644 --- a/cilium/socket_option_ip_transparent.cc +++ b/cilium/socket_option_ip_transparent.cc @@ -32,8 +32,8 @@ bool IpTransparentSocketOption::setOption( auto& cilium_calls = PrivilegedService::Singleton::get(); - auto ipVersion = socket.ipVersion(); - if (!ipVersion.has_value()) { + auto ip_version = socket.ipVersion(); + if (!ip_version.has_value()) { ENVOY_LOG(critical, "Socket address family is not available, can not choose source address"); return false; } @@ -44,7 +44,7 @@ bool IpTransparentSocketOption::setOption( auto ip_socket_level = SOL_IP; auto ip_transparent_socket_option = IP_TRANSPARENT; auto ip_transparent_socket_option_name = "IP_TRANSPARENT"; - if (*ipVersion == Network::Address::IpVersion::v6) { + if (*ip_version == Network::Address::IpVersion::v6) { ip_socket_level = SOL_IPV6; ip_transparent_socket_option = IPV6_TRANSPARENT; ip_transparent_socket_option_name = "IPV6_TRANSPARENT"; diff --git a/cilium/socket_option_source_address.cc b/cilium/socket_option_source_address.cc index 0e3390f05..81d30da2e 100644 --- a/cilium/socket_option_source_address.cc +++ b/cilium/socket_option_source_address.cc @@ -40,8 +40,8 @@ bool SourceAddressSocketOption::setOption( return true; } - auto ipVersion = socket.ipVersion(); - if (!ipVersion.has_value()) { + auto ip_version = socket.ipVersion(); + if (!ip_version.has_value()) { ENVOY_LOG(critical, "Socket address family is not available, can not choose source address"); return false; } @@ -50,7 +50,7 @@ bool SourceAddressSocketOption::setOption( if (!source_address && (ipv4_source_address_ || ipv6_source_address_)) { // Select source address based on the socket address family source_address = ipv6_source_address_; - if (*ipVersion == Network::Address::IpVersion::v4) { + if (*ip_version == Network::Address::IpVersion::v4) { source_address = ipv4_source_address_; } } diff --git a/cilium/tls_wrapper.cc b/cilium/tls_wrapper.cc index f6756b5df..104764493 100644 --- a/cilium/tls_wrapper.cc +++ b/cilium/tls_wrapper.cc @@ -155,7 +155,7 @@ class SslSocketWrapper : public Network::TransportSocket, Logger::Loggable"); + std::string ip_str(""); if (policy_ref->ingress_) { Network::Address::InstanceConstSharedPtr src_address = is_client ? callbacks_->connection().connectionInfoProvider().localAddress() @@ -163,12 +163,12 @@ class SslSocketWrapper : public Network::TransportSocket, Logger::Loggableip(); if (sip) { - ipStr = sip->addressAsString(); + ip_str = sip->addressAsString(); } } } else { if (dip) { - ipStr = dip->addressAsString(); + ip_str = dip->addressAsString(); } } ENVOY_CONN_LOG( @@ -176,7 +176,7 @@ class SslSocketWrapper : public Network::TransportSocket, Logger::Loggablepod_ip_, - policy_ref->ingress_ ? "source" : "destination", ipStr, remote_id, destination_port, + policy_ref->ingress_ ? "source" : "destination", ip_str, remote_id, destination_port, sni); } } else { diff --git a/tests/cilium_tls_http_integration_test.cc b/tests/cilium_tls_http_integration_test.cc index 27f6ed34c..4e6322c2c 100644 --- a/tests/cilium_tls_http_integration_test.cc +++ b/tests/cilium_tls_http_integration_test.cc @@ -240,12 +240,12 @@ class CiliumHttpTLSIntegrationTest : public CiliumHttpIntegrationTest { Network::Address::InstanceConstSharedPtr address = Ssl::getSslAddress(version_, lookupPort("http")); context_ = createClientSslTransportSocketFactory(context_manager_, *api_); - Network::ClientConnectionPtr ssl_client_ = dispatcher_->createClientConnection( + Network::ClientConnectionPtr ssl_client = dispatcher_->createClientConnection( address, Network::Address::InstanceConstSharedPtr(), context_->createTransportSocket(nullptr, nullptr), nullptr, nullptr); - ssl_client_->enableHalfClose(true); - codec_client_ = makeHttpConnection(std::move(ssl_client_)); + ssl_client->enableHalfClose(true); + codec_client_ = makeHttpConnection(std::move(ssl_client)); } void createUpstreams() override { diff --git a/tests/cilium_websocket_decap_integration_test.cc b/tests/cilium_websocket_decap_integration_test.cc index 498f2d415..191b20a84 100644 --- a/tests/cilium_websocket_decap_integration_test.cc +++ b/tests/cilium_websocket_decap_integration_test.cc @@ -156,14 +156,14 @@ TEST_P(CiliumWebSocketIntegrationTest, AcceptedWebSocket) { response->waitForHeaders(); EXPECT_EQ("101", response->headers().getStatusValue()); - auto clientConn = codec_client_->connection(); + auto client_conn = codec_client_->connection(); // Create websocket framed data & write it on the client connection Buffer::OwnedImpl buf{"\x82\x5" "hello"}; - clientConn->write(buf, false); + client_conn->write(buf, false); // Run the dispatcher so that the write event is handled - clientConn->dispatcher().run(Event::Dispatcher::RunType::NonBlock); + client_conn->dispatcher().run(Event::Dispatcher::RunType::NonBlock); std::string data; ASSERT_TRUE(fake_upstream_connection->waitForData(5, &data)); @@ -187,9 +187,9 @@ TEST_P(CiliumWebSocketIntegrationTest, AcceptedWebSocket) { "hello21" "\x82\x3" "foo"); - clientConn->write(buf, false); + client_conn->write(buf, false); // Run the dispatcher so that the write event is handled - clientConn->dispatcher().run(Event::Dispatcher::RunType::NonBlock); + client_conn->dispatcher().run(Event::Dispatcher::RunType::NonBlock); ASSERT_TRUE(fake_upstream_connection->waitForData(seen_data_len + 16, &data)); ASSERT_EQ(data.substr(seen_data_len), "hello2hello21foo"); @@ -211,9 +211,9 @@ TEST_P(CiliumWebSocketIntegrationTest, AcceptedWebSocket) { "len16", 9}; buf.add(frame16); - clientConn->write(buf, false); + client_conn->write(buf, false); // Run the dispatcher so that the write event is handled - clientConn->dispatcher().run(Event::Dispatcher::RunType::NonBlock); + client_conn->dispatcher().run(Event::Dispatcher::RunType::NonBlock); ASSERT_TRUE(fake_upstream_connection->waitForData(seen_data_len + 5, &data)); ASSERT_EQ(data.substr(seen_data_len), "len16"); @@ -238,9 +238,9 @@ TEST_P(CiliumWebSocketIntegrationTest, AcceptedWebSocket) { "len64", 15}; buf.add(frame64); - clientConn->write(buf, false); + client_conn->write(buf, false); // Run the dispatcher so that the write event is handled - clientConn->dispatcher().run(Event::Dispatcher::RunType::NonBlock); + client_conn->dispatcher().run(Event::Dispatcher::RunType::NonBlock); ASSERT_TRUE(fake_upstream_connection->waitForData(seen_data_len + 5, &data)); ASSERT_EQ(data.substr(seen_data_len), "len64"); @@ -259,9 +259,9 @@ TEST_P(CiliumWebSocketIntegrationTest, AcceptedWebSocket) { "hello" "\x82\xe" "gap "); - clientConn->write(buf, false); + client_conn->write(buf, false); // Run the dispatcher so that the write event is handled - clientConn->dispatcher().run(Event::Dispatcher::RunType::NonBlock); + client_conn->dispatcher().run(Event::Dispatcher::RunType::NonBlock); ASSERT_TRUE(fake_upstream_connection->waitForData(seen_data_len + 9, &data)); ASSERT_EQ(data.substr(seen_data_len), "hellogap "); @@ -273,9 +273,9 @@ TEST_P(CiliumWebSocketIntegrationTest, AcceptedWebSocket) { buf.add("in between" "\x82\x3" "foo"); - clientConn->write(buf, false); + client_conn->write(buf, false); // Run the dispatcher so that the write event is handled - clientConn->dispatcher().run(Event::Dispatcher::RunType::NonBlock); + client_conn->dispatcher().run(Event::Dispatcher::RunType::NonBlock); ASSERT_TRUE(fake_upstream_connection->waitForData(seen_data_len + 13, &data)); ASSERT_EQ(data.substr(seen_data_len), "in betweenfoo"); @@ -298,9 +298,9 @@ TEST_P(CiliumWebSocketIntegrationTest, AcceptedWebSocket) { buf.add("\x82\x8e"); buf.add(mask, 4); buf.add(masked.data(), masked.length()); - clientConn->write(buf, false); + client_conn->write(buf, false); // Run the dispatcher so that the write event is handled - clientConn->dispatcher().run(Event::Dispatcher::RunType::NonBlock); + client_conn->dispatcher().run(Event::Dispatcher::RunType::NonBlock); ASSERT_TRUE(fake_upstream_connection->waitForData(seen_data_len + 14, &data)); ASSERT_EQ(data.substr(seen_data_len), msg); @@ -326,15 +326,15 @@ TEST_P(CiliumWebSocketIntegrationTest, AcceptedWebSocket) { // Write frame header buf.add("\x82\x8d"); buf.add(mask2, 4); - clientConn->write(buf, false); + client_conn->write(buf, false); // Run the dispatcher so that the write event is handled - clientConn->dispatcher().run(Event::Dispatcher::RunType::NonBlock); + client_conn->dispatcher().run(Event::Dispatcher::RunType::NonBlock); // Write 5 first bytes buf.add(masked2.data(), 5); - clientConn->write(buf, false); + client_conn->write(buf, false); // Run the dispatcher so that the write event is handled - clientConn->dispatcher().run(Event::Dispatcher::RunType::NonBlock); + client_conn->dispatcher().run(Event::Dispatcher::RunType::NonBlock); ASSERT_TRUE(fake_upstream_connection->waitForData(seen_data_len + 5, &data)); ASSERT_EQ(data.substr(seen_data_len), absl::string_view(msg2.data(), 5)); @@ -342,9 +342,9 @@ TEST_P(CiliumWebSocketIntegrationTest, AcceptedWebSocket) { // Write remaining bytes buf.add(masked2.data() + 5, masked2.length() - 5); - clientConn->write(buf, false); + client_conn->write(buf, false); // Run the dispatcher so that the write event is handled - clientConn->dispatcher().run(Event::Dispatcher::RunType::NonBlock); + client_conn->dispatcher().run(Event::Dispatcher::RunType::NonBlock); ASSERT_TRUE(fake_upstream_connection->waitForData(seen_data_len + 13 - 5, &data)); ASSERT_EQ(data.substr(seen_data_len), msg2.data() + 5); diff --git a/tests/health_check_sink_test.cc b/tests/health_check_sink_test.cc index 42e14c554..0be1d49a4 100644 --- a/tests/health_check_sink_test.cc +++ b/tests/health_check_sink_test.cc @@ -54,7 +54,7 @@ TEST(HealthCheckEventPipeSink, logTest) { // Set up server std::string normal_path("test_path"); - HealthCheckSinkServer eventSink(normal_path); + HealthCheckSinkServer event_sink(normal_path); // Set up client factory auto factory = @@ -92,7 +92,7 @@ TEST(HealthCheckEventPipeSink, logTest) { pipe_sink->log(eject_event); EXPECT_TRUE( - eventSink.expectEventTo([&](const envoy::data::core::v3::HealthCheckEvent& observed_event) { + event_sink.expectEventTo([&](const envoy::data::core::v3::HealthCheckEvent& observed_event) { return Protobuf::util::MessageDifferencer::Equals(observed_event, eject_event); })); @@ -119,7 +119,7 @@ TEST(HealthCheckEventPipeSink, logTest) { pipe_sink2->log(add_event); EXPECT_TRUE( - eventSink.expectEventTo([&](const envoy::data::core::v3::HealthCheckEvent& observed_event) { + event_sink.expectEventTo([&](const envoy::data::core::v3::HealthCheckEvent& observed_event) { return Protobuf::util::MessageDifferencer::Equals(observed_event, add_event); })); @@ -127,7 +127,7 @@ TEST(HealthCheckEventPipeSink, logTest) { // Set up server #define ABSTRACT_PATH "@another\0test_path" std::string abstract_name(ABSTRACT_PATH, sizeof(ABSTRACT_PATH) - 1); - HealthCheckSinkServer eventSink3(abstract_name); + HealthCheckSinkServer event_sink3(abstract_name); // Set up 3rd client on a different socket cilium::HealthCheckEventPipeSink config3; @@ -138,7 +138,7 @@ TEST(HealthCheckEventPipeSink, logTest) { pipe_sink3->log(eject_event); EXPECT_TRUE( - eventSink3.expectEventTo([&](const envoy::data::core::v3::HealthCheckEvent& observed_event) { + event_sink3.expectEventTo([&](const envoy::data::core::v3::HealthCheckEvent& observed_event) { return Protobuf::util::MessageDifferencer::Equals(observed_event, eject_event); })); } diff --git a/tests/metadata_config_test.cc b/tests/metadata_config_test.cc index 0e13825d7..786d00ae7 100644 --- a/tests/metadata_config_test.cc +++ b/tests/metadata_config_test.cc @@ -79,8 +79,8 @@ class MetadataConfigTest : public testing::Test { })); ON_CALL(context_.init_manager_, initialize(_)) .WillByDefault(Invoke([this](const Init::Watcher& watcher) { - for (auto& handle_ : target_handles_) { - handle_->initialize(watcher); + for (auto& handle : target_handles_) { + handle->initialize(watcher); } }));