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

Check if required capabilities are available #1067

Merged
merged 11 commits into from
Aug 2, 2024
Merged

Conversation

rafaelroquetto
Copy link
Contributor

@rafaelroquetto rafaelroquetto commented Jul 27, 2024

This patch checks if the required Linux capabilities for beyla are present, based on the present configuration (e.g. NetO11y does not require all of the listed capabilities below).

These are:

  • CAP_BPF
  • CAP_PERFMON
  • CAP_DAC_READ_SEARCH
  • CAP_SYS_RESOURCE
  • CAP_CHECKPOINT_RESTORE
  • CAP_SYS_PTRACE
  • CAP_NET_RAW

(see here for more info)

It also introduces a new config option called enforce_sys_caps, defaulted to true. When that option is set to true, Beyla will not continue starting up if the required capabilities are not present and print the list of missing capabilities. Otherwise, startup will continue and the missing capabilities will be listed as a warning (allowing for easy backwards compatibility).

In addition to the capabilities listed above, Beyla startup will now check for the presence of CAP_SYS_ADMIN. This capability is required when mounting the bpf filesystem and for the context propagation functionality(*). When this capability isn't present, Beyla will error gracefully in the former case, whereas it will disable the context propagation probes and resume starting up on the latter.

(*) kernel_probe_write_user(), which is used to inject context data, requires this capability.

Resolves: #246

@rafaelroquetto rafaelroquetto added do-not-merge WIP or something that musn't be merged wip labels Jul 27, 2024
@codecov-commenter
Copy link

codecov-commenter commented Jul 27, 2024

Codecov Report

Attention: Patch coverage is 82.50000% with 14 lines in your changes missing coverage. Please review.

Project coverage is 81.85%. Comparing base (f88413a) to head (f173656).
Report is 2 commits behind head on main.

Files Patch % Lines
cmd/beyla/main.go 0.00% 4 Missing and 1 partial ⚠️
pkg/internal/ebpf/common/common.go 50.00% 2 Missing and 1 partial ⚠️
pkg/beyla/os.go 95.23% 1 Missing and 1 partial ⚠️
pkg/internal/discover/attacher_linux.go 33.33% 1 Missing and 1 partial ⚠️
pkg/internal/helpers/capabilities.go 90.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1067      +/-   ##
==========================================
- Coverage   81.90%   81.85%   -0.06%     
==========================================
  Files         139      140       +1     
  Lines       11329    11407      +78     
==========================================
+ Hits         9279     9337      +58     
- Misses       1534     1547      +13     
- Partials      516      523       +7     
Flag Coverage Δ
integration-test 56.79% <43.75%> (-0.13%) ⬇️
k8s-integration-test 59.10% <38.75%> (-0.23%) ⬇️
oats-test 36.86% <36.25%> (-0.06%) ⬇️
unittests 52.05% <70.00%> (+0.11%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rafaelroquetto rafaelroquetto force-pushed the friendly_msg branch 2 times, most recently from da9e717 to ed1cf1a Compare July 29, 2024 19:38
@rafaelroquetto rafaelroquetto changed the title WIP: User-friendly message if you are running without root permissions WIP: Check if required capabilities are available Jul 29, 2024
@rafaelroquetto rafaelroquetto force-pushed the friendly_msg branch 2 times, most recently from 1c55041 to 85011b5 Compare July 29, 2024 19:53
@rafaelroquetto rafaelroquetto marked this pull request as ready for review July 29, 2024 20:29
@rafaelroquetto rafaelroquetto changed the title WIP: Check if required capabilities are available Check if required capabilities are available Jul 29, 2024
pkg/beyla/os.go Outdated Show resolved Hide resolved
cmd/beyla/main.go Outdated Show resolved Hide resolved
@rafaelroquetto rafaelroquetto removed do-not-merge WIP or something that musn't be merged wip labels Jul 29, 2024
Copy link
Contributor

@mariomac mariomac left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! The code looks good (I have few minor suggestions) but I have some questions:

  • I'd say that the required capabilities for Network metrics are lower than the capabilities for Application metrics (at least when using the Traffic Control Network probes). We should allow users to specify the minimum capabilities to their specific use case.

  • I observed that some environments (e.g. virtual machine with Kind) require also the CAP_SYS_ADMIN capabilities. Don't know why and how to check it programmatically. I guess it's fine if we don't demand CAP_SYS_ADMIN but Beyla fails later.

pkg/beyla/os.go Outdated Show resolved Hide resolved
pkg/beyla/os.go Outdated Show resolved Hide resolved
@rafaelroquetto rafaelroquetto force-pushed the friendly_msg branch 3 times, most recently from cecfc2e to d5b24aa Compare July 31, 2024 19:56
@rafaelroquetto rafaelroquetto added do-not-merge WIP or something that musn't be merged wip labels Jul 31, 2024
@rafaelroquetto rafaelroquetto force-pushed the friendly_msg branch 3 times, most recently from 0e5843a to 973b307 Compare July 31, 2024 21:10
@rafaelroquetto rafaelroquetto marked this pull request as draft July 31, 2024 21:44
@rafaelroquetto rafaelroquetto changed the title Check if required capabilities are available WIP: Check if required capabilities are available Jul 31, 2024
@mariomac mariomac requested a review from grafsean August 1, 2024 15:32
@rafaelroquetto rafaelroquetto changed the title WIP: Check if required capabilities are available Check if required capabilities are available Aug 1, 2024
@rafaelroquetto rafaelroquetto removed the wip label Aug 1, 2024
@rafaelroquetto rafaelroquetto marked this pull request as ready for review August 1, 2024 15:43
Copy link
Contributor

@mariomac mariomac left a comment

Choose a reason for hiding this comment

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

LGTM! Good job.

I left some minor comments but you can just ignore them, or address them but you don't need to wait for a reapproval.

pkg/beyla/os.go Outdated Show resolved Hide resolved
pkg/beyla/os_test.go Show resolved Hide resolved
pkg/internal/discover/attacher_linux.go Show resolved Hide resolved
test/integration/docker-compose-client.yml Show resolved Hide resolved
@rafaelroquetto rafaelroquetto removed the do-not-merge WIP or something that musn't be merged label Aug 2, 2024
Copy link
Contributor

@grafsean grafsean left a comment

Choose a reason for hiding this comment

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

Left a comment with suggested copy change.

docs/sources/configure/options.md Outdated Show resolved Hide resolved
@rafaelroquetto rafaelroquetto merged commit 93aac32 into main Aug 2, 2024
8 checks passed
@rafaelroquetto rafaelroquetto deleted the friendly_msg branch August 2, 2024 14:44
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.

User-friendly message if you are running without root permissions.
4 participants