Skip to content

Comments

Make cilium.l7policy filter a dual filter#581

Merged
jrajahalme merged 4 commits intomainfrom
dual-filter-testing
May 16, 2024
Merged

Make cilium.l7policy filter a dual filter#581
jrajahalme merged 4 commits intomainfrom
dual-filter-testing

Conversation

@jrajahalme
Copy link
Member

@jrajahalme jrajahalme commented Feb 27, 2024

Make cilium.l7policy filter into a dual filter. This allows removal and long term maintenance of the HTTP upstream callback patch.

Note: This should be merged only after the corresponding cilium/cilium PR is ready to merge.

@jrajahalme jrajahalme added the wip work-in-progress, no need to review label Feb 27, 2024
@jrajahalme jrajahalme requested a review from a team as a code owner February 27, 2024 19:14
@jrajahalme jrajahalme requested review from sayboras and removed request for a team February 27, 2024 19:14
@jrajahalme jrajahalme marked this pull request as draft February 27, 2024 19:14
@jrajahalme jrajahalme force-pushed the dual-filter-testing branch from 4702414 to 39b7f42 Compare April 18, 2024 13:05
@jrajahalme jrajahalme force-pushed the dual-filter-testing branch from 39b7f42 to fb824fe Compare April 22, 2024 17:39
@jrajahalme
Copy link
Member Author

rebased

@jrajahalme jrajahalme marked this pull request as ready for review April 22, 2024 17:39
@jrajahalme jrajahalme changed the title Dual filter testing Make cilium.l7policy filter a dual filter Apr 22, 2024
@sayboras sayboras added the dont-merge/preview-only DON'T MERGE label May 1, 2024
@jrajahalme jrajahalme force-pushed the dual-filter-testing branch from fb824fe to 9d85e94 Compare May 6, 2024 15:05
@jrajahalme jrajahalme added kind/enhancement and removed wip work-in-progress, no need to review dont-merge/preview-only DON'T MERGE labels May 8, 2024
Copy link
Member

@sayboras sayboras left a comment

Choose a reason for hiding this comment

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

I have below comments, the rest looks good

@jrajahalme jrajahalme force-pushed the dual-filter-testing branch from 9d85e94 to 22f12cb Compare May 16, 2024 12:36
Move access log entry to filter state so that it can be accessed from
multiple filters.

Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
Envoy HTTP router and TCP proxy append the socket options in filter_state
with key Network::UpstreamSocketOptionsFilterState::key() to the upstream
socket's options. Use this to make Cilium BPF metadata accessible to
Cilium upstream filters when needed.

Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
@jrajahalme jrajahalme force-pushed the dual-filter-testing branch 2 times, most recently from db8400d to f79836c Compare May 16, 2024 12:41
Allow cilium.l7policy filter to be used as an Envoy HTTP upstream
filter. No longer use upstream callback for L7 LB policy enforcement.

Upstream Envoy commit 26a4eb87428dbf3a39ff4f6f61b69538c34d07b6 introduced
'is_upstream' field in 'DualInfo' that allows filter operation to know if
the filter is installed on upstream or downstream filter chain. Due to
the semantic differences between the upstream and downstream filter
execution this is generally needed for dual filter implementations.

Backport this change. This backport will become moot in a future Envoy
version bump with the upstream commit
26a4eb87428dbf3a39ff4f6f61b69538c34d07b6.

Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
@jrajahalme jrajahalme force-pushed the dual-filter-testing branch from f79836c to 8b87626 Compare May 16, 2024 12:48
@jrajahalme
Copy link
Member Author

Changed patch numbering as suggested & rebased.

@jrajahalme jrajahalme added this pull request to the merge queue May 16, 2024
Merged via the queue into main with commit 801c612 May 16, 2024
@jrajahalme jrajahalme deleted the dual-filter-testing branch May 16, 2024 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants