Skip to content

[v1.37] policy: Deduplicate SecretWatchers and TLS Contexts - #2035

Draft
fristonio wants to merge 2 commits into
v1.37from
pr/fristonio/v1.37/policy-secret-sharing
Draft

[v1.37] policy: Deduplicate SecretWatchers and TLS Contexts#2035
fristonio wants to merge 2 commits into
v1.37from
pr/fristonio/v1.37/policy-secret-sharing

Conversation

@fristonio

Copy link
Copy Markdown
Member

Backport of #2006

@fristonio
fristonio force-pushed the pr/fristonio/v1.37/policy-secret-sharing branch from cd9b436 to 9cad24e Compare September 2, 2026 23:21
@sayboras

sayboras commented Sep 3, 2026

Copy link
Copy Markdown
Member

understand that it's still in draft, however, it's better to rebase with v1.37 latest branch, so that some of CI changes are included.

jrajahalme and others added 2 commits September 2, 2026 23:04
Network policies may contain hundreds or thousands of HeaderMatch or TLS
rules referring to a small set of Secrets. Each reference previously
constructed a separate SecretWatcher or Envoy TLS ContextConfigImpl,
including its SDS provider callbacks and other context state.

Add separate weak caches for SecretWatchers and upstream and downstream
TLS contexts, keyed by the SDS name or Cilium TLSContext protobuf. Reuse
a live context when an equivalent policy configuration is encountered.

Reset the caches together with the policy maps after an NPDS stream restart,
preserving the existing agent-restart behavior. Prune expired cache entries
asynchronously after policy updates and worker quiescence, keeping pruning
off the policy-update hot path.

Benchmark an NPDS update containing 1,000 policies that reference the same
SDS-derived CA validation context. The CA is delivered through a real dynamic
SDS provider rather than embedded in the policy TLS contexts.

Optimized benchmark results (3 repetitions):

                            CPU time   Retained heap   Heap/policy
  TLS context cache enabled   1.47 ms      1.581 MiB      1.619 KiB
  TLS context cache disabled  3.95 ms      4.990 MiB      5.109 KiB

This reduces CPU time by approximately 63% and retained heap by 68%, saving
about 3.41 MiB per 1,000-policy update and increasing throughput by 2.68x.

The benchmark uses Envoy's small, single-certificate test CA. Production SDS
resources commonly contain larger CA bundles comparable to those shipped
with operating systems, so the benchmark likely understates the memory impact
of retaining duplicate certificate-related context state.

Benchmark results for deduplication of SecretWatchers on HeaderMatches
are more modest (duplicating header sized secrets carries less overhead),
but are still positive around 20-25% for CPU and memory.

[ upstream commit 7dfaf94 ]

Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
Signed-off-by: Deepesh Pathak <deepeshpathak09@gmail.com>
This commit adds `google_benchmark` as an extra dependency which is not
availbale as a transitive dependency in envoy 1.37.

Signed-off-by: Deepesh Pathak <deepeshpathak09@gmail.com>
@fristonio
fristonio force-pushed the pr/fristonio/v1.37/policy-secret-sharing branch from 9cad24e to 8b1dacc Compare September 3, 2026 06:05
@jrajahalme

Copy link
Copy Markdown
Member

The 2nd commit is not necessary, as Envoy 1.37 already has this dependency, albeit with a different name:

        "@com_github_google_benchmark//:benchmark",

The benchmark test failure is due to Envoy 1.37 not supporting running benchmarks from outside of the Envoy repo. IMO the best fix is to add a v1.37 specific patch to apply the upstream 1.38 fix (envoyproxy/envoy@bc6c805) for this as a preparatory commit to the backport commit:

diff --git a/WORKSPACE b/WORKSPACE
index 2a875bdbd..ead065ec6 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -45,6 +45,7 @@ git_repository(
         "@//patches:0006-test-integration-Defer-fake-upstream-read-enable-un.patch",
         "@//patches:0007-config-add-grpc-mux-stream-event-callback.patch",
         "@//patches:0008-repo-Make-yq-dependency-optional-for-CI-config-parsi.patch",
+        "@//patches:0009-test-use-bazel_tools-runfiles-to-locate-benchmark-bin.patch",
     ],
     # // clang-format off: Envoy's format check: Only repository_locations.bzl may contains URL references
     remote = "https://github.com/envoyproxy/envoy.git",
diff --git a/patches/0009-test-use-bazel_tools-runfiles-to-locate-benchmark-bin.patch b/patches/0009-test-use-bazel_tools-runfiles-to-locate-benchmark-bin.patch
new file mode 100644
index 000000000..c9a962df9
--- /dev/null
+++ b/patches/0009-test-use-bazel_tools-runfiles-to-locate-benchmark-bin.patch
@@ -0,0 +1,57 @@
+From bc6c8052f90b3eda7d7bb3c5387b22cd6d98c96d Mon Sep 17 00:00:00 2001
+From: Alex Bakon <abakon@netflix.com>
+Date: Wed, 14 Jan 2026 21:15:28 +0000
+Subject: [PATCH] test: use bazel_tools runfiles to locate benchmark bin
+
+Use the bazel_tools runfiles library for Bash to locate the benchmark
+binary to be run. The previous approach only worked for benchmarks
+defined in the Envoy repository, while this approach allows the macro to
+be used in projects that import Envoy as a dependency.
+
+Signed-off-by: Alex Bakon <abakon@netflix.com>
+---
+ bazel/envoy_test.bzl                 | 3 ++-
+ bazel/test_for_benchmark_wrapper.sh | 15 ++++++++++++++-
+ 2 files changed, 16 insertions(+), 2 deletions(-)
+
+diff --git a/bazel/envoy_test.bzl b/bazel/envoy_test.bzl
+--- a/bazel/envoy_test.bzl
++++ b/bazel/envoy_test.bzl
+@@ -309,9 +309,10 @@ def envoy_benchmark_test(
+     sh_test(
+         name = name,
+         srcs = [repository + "//bazel:test_for_benchmark_wrapper.sh"],
++        deps = ["@bazel_tools//tools/bash/runfiles"],
+         data = [":" + benchmark_binary] + data,
+         exec_properties = exec_properties,
+-        args = ["%s/%s" % (native.package_name(), benchmark_binary)],
++        args = ["$(rlocationpath %s)" % native.package_relative_label(benchmark_binary)],
+         tags = tags + ["nocoverage"],
+         **kargs
+     )
+diff --git a/bazel/test_for_benchmark_wrapper.sh b/bazel/test_for_benchmark_wrapper.sh
+--- a/bazel/test_for_benchmark_wrapper.sh
++++ b/bazel/test_for_benchmark_wrapper.sh
+@@ -1,6 +1,19 @@
+ #!/usr/bin/env bash
++#
++# Disable warnings about non-constant sources in runfiles.bash initialization.
++# shellcheck disable=SC1090
++
++# Copy-pasted from the Bazel Bash runfiles library v3.
++set -uo pipefail; set +e; f=bazel_tools/tools/bash/runfiles/runfiles.bash
++source "${RUNFILES_DIR:-/dev/null}/$f" 2>/dev/null || \
++  source "$(grep -sm1 "^$f " "${RUNFILES_MANIFEST_FILE:-/dev/null}" | cut -f2- -d' ')" 2>/dev/null || \
++  source "$0.runfiles/$f" 2>/dev/null || \
++  source "$(grep -sm1 "^$f " "$0.runfiles_manifest" | cut -f2- -d' ')" 2>/dev/null || \
++  source "$(grep -sm1 "^$f " "$0.exe.runfiles_manifest" | cut -f2- -d' ')" 2>/dev/null || \
++  { echo>&2 "ERROR: cannot find $f"; exit 1; }; f=; set -e
++# --- end runfiles.bash initialization v3 ---
+ 
+ # Set the benchmark time to 0 to just verify that the benchmark runs to
+ # completion.  We're interacting with two different flag parsers, so the order
+ # of flags and the -- matters.
+-"${TEST_SRCDIR}/envoy/${1}" "${@:2}" --skip_expensive_benchmarks -- --benchmark_min_time=0s
++$(rlocation "${1}") "${@:2}" --skip_expensive_benchmarks -- --benchmark_min_time=0s
+-- 
+2.43.0

The other CI flake (in tests/bpf_metadata_integration_test.cc:648) is fixed by #2007, both should be backported.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants