Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add HTTP URL filter #1236

Merged
merged 2 commits into from
Oct 20, 2023
Merged

Add HTTP URL filter #1236

merged 2 commits into from
Oct 20, 2023

Conversation

glrf
Copy link
Contributor

@glrf glrf commented Oct 2, 2023

We added support for filtering on HTTP URLs in cilium/cilium#28275. This PR adds this feature in the CLI by adding a new flag --http-url

Fixes: #925

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label PR is blocked until the release note is set label Oct 2, 2023
@glrf glrf force-pushed the feat/http-url branch 10 times, most recently from a1dcc14 to 2eda984 Compare October 2, 2023 13:34
@maintainer-s-little-helper
Copy link

Commit 04f7aac does not match "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/contributing/#developer-s-certificate-of-origin

@kaworu kaworu self-requested a review October 4, 2023 15:12
@kaworu kaworu added 🌟 kind/feature This introduces new functionality. release-note/minor This PR introduces functionality that users may find relevant to operating Hubble. labels Oct 4, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label PR is blocked until the release note is set label Oct 4, 2023
@kaworu kaworu added ⌨️ area/cli Impacts the command line interface of any command in the repository. dont-merge/needs-release-note-label PR is blocked until the release note is set labels Oct 4, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed dont-merge/needs-release-note-label PR is blocked until the release note is set labels Oct 4, 2023
@kaworu
Copy link
Member

kaworu commented Oct 4, 2023

Static check failed with

level=error msg="Timeout exceeded: try increasing it by passing --timeout option"

Consider increasing golangci-lint timeout in a first commit to something like 5m here.

@kaworu kaworu added the :basecamp: needs-rebase This PR needs to be rebased because it has merge conflicts. label Oct 9, 2023
@glrf glrf marked this pull request as ready for review October 9, 2023 09:35
@glrf glrf requested review from a team as code owners October 9, 2023 09:35
@glrf glrf requested review from a team as code owners October 9, 2023 09:35
@glrf glrf requested review from rolinh and viktor-kurchenko and removed request for a team October 9, 2023 09:35
glrf added 2 commits October 16, 2023 11:42
Signed-off-by: Fabian Fischer <[email protected]>
We added support for filtering on HTTP URLs in
cilium/cilium#28275. This PR adds this feature
in the CLI by adding a new flag `--http-url`

Fixes: cilium#925

Signed-off-by: Fabian Fischer <[email protected]>
Copy link
Member

@kaworu kaworu left a comment

Choose a reason for hiding this comment

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

Thanks @glrf, patch LGTM.

Note for reviewers: the huge diff in the first commit updating cilium is caused by cilium/cilium#27854 which is pulling in the whole Cilium policy modules into the Hubble API (see this main branch commit). I was curious and did this to investigate:

% go get github.com/cilium/cilium@86a6ab94fdf286971fb26cca97f4c2d0cc15da74
% go mod tidy && go mod vendor && go mod verify
% git add .
% git diff --cached --shortstat
 82 files changed, 4199 insertions(+), 1588 deletions(-)
% go get github.com/cilium/cilium@6fcad1351103594602767024c1e2d17816f69f34
% go mod tidy && go mod vendor && go mod verify
% git add .
% git diff --cached --shortstat
 3756 files changed, 1072929 insertions(+), 1704 deletions(-)

@michi-covalent michi-covalent removed the :basecamp: needs-rebase This PR needs to be rebased because it has merge conflicts. label Oct 19, 2023
Copy link

@viktor-kurchenko viktor-kurchenko left a comment

Choose a reason for hiding this comment

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

LGTM.

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Oct 20, 2023
@kaworu kaworu merged commit 6abc2a8 into cilium:main Oct 20, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⌨️ area/cli Impacts the command line interface of any command in the repository. 🌟 kind/feature This introduces new functionality. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/minor This PR introduces functionality that users may find relevant to operating Hubble.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for filtering on HTTP URL
5 participants