Skip to content

Conversation

joestringer
Copy link
Member

@joestringer joestringer commented Feb 17, 2025

This reverts commit 1037713.

The commit was fine and well-intentioned, but it introduced a hard
dependency from the CLI onto cilium/ebpf library. The CLI is intended as
a tool to run on user systems, not the target system. Introducing a
dependency on libraries that directly interface with ebpf causes trouble
because it means that cilium/ebpf must stub out all declarations on all
platforms, even ones that don't make sense because the platform doesn't
have ebpf support.

So, rather than depending on this library just to do some substring
matches, just retain the copies of the strings in the CLI and make the
comparisons without the library.

Related: #37598

This reverts commit 1037713.

The commit was fine and well-intentioned, but it introduced a hard
dependency from the CLI onto cilium/ebpf library. The CLI is intended as
a tool to run on user systems, not the target system. Introducing a
dependency on libraries that directly interface with ebpf causes trouble
because it means that cilium/ebpf must stub out all declarations on all
platforms, even ones that don't make sense because the platform doesn't
have ebpf support.

So, rather than depending on this library just to do some substring
matches, just retain the copies of the strings in the CLI and make the
comparisons without the library.

Signed-off-by: Joe Stringer <joe@cilium.io>
@joestringer joestringer requested a review from a team as a code owner February 17, 2025 22:15
@joestringer joestringer added the release-note/misc This PR makes changes that have no direct user impact. label Feb 17, 2025
@joestringer joestringer requested a review from a team as a code owner February 17, 2025 22:15
@github-actions github-actions bot added cilium-cli This PR contains changes related with cilium-cli cilium-cli-exclusive This PR only impacts cilium-cli binary labels Feb 17, 2025
@joestringer
Copy link
Member Author

/test

@joestringer
Copy link
Member Author

Filed #37689 for the failure as it is unrelated to the changes in this PR.

@joestringer joestringer added this pull request to the merge queue Feb 18, 2025
@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 Feb 18, 2025
Merged via the queue into main with commit 8f2de93 Feb 18, 2025
220 of 221 checks passed
@joestringer joestringer deleted the pr/joe/remove-cli-ebpf-dependency branch February 18, 2025 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cilium-cli This PR contains changes related with cilium-cli cilium-cli-exclusive This PR only impacts cilium-cli binary ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants