libs

libs copied to clipboard

What type of PR is this?

Uncomment one (or more) /kind <> lines:

/kind bug

/kind cleanup

/kind design

/kind documentation

/kind failing-test

/kind feature

Any specific area of the project related to this PR?

Uncomment one (or more) /area <> lines:

/area API-version

/area build

/area CI

/area driver-kmod

/area driver-bpf

/area driver-modern-bpf

/area libscap-engine-bpf

/area libscap-engine-gvisor

/area libscap-engine-kmod

/area libscap-engine-modern-bpf

/area libscap-engine-nodriver

/area libscap-engine-noop

/area libscap-engine-source-plugin

/area libscap-engine-savefile

/area libscap-engine-udig

/area libscap

/area libpman

/area libsinsp

/area tests

/area proposals

Does this PR require a change in the driver versions?

/version driver-API-version-major

/version driver-API-version-minor

/version driver-API-version-patch

/version driver-SCHEMA-version-major

/version driver-SCHEMA-version-minor

/version driver-SCHEMA-version-patch

What this PR does / why we need it:

This adds pkg-config (.pc) files for libsinsp and libscap. I was able to successfully build Logray locally using them, and installing into /tmp/falco-libs produces the following output (jsoncpp and tbb were installed separately using Homebrew):

$ PKG_CONFIG_PATH=/tmp/falco-libs/lib/pkgconfig pkg-config --cflags libsinsp 
-I/tmp/falco-libs/include/falcosecurity/userspace/libsinsp -I/tmp/falco-libs/include/falcosecurity/userspace/libscap -I/usr/local/Cellar/jsoncpp/1.9.5/include -I/usr/local/Cellar/tbb/2021.5.0_2/include

$ PKG_CONFIG_PATH=/tmp/falco-libs/lib/pkgconfig pkg-config --libs libsinsp
-L/tmp/falco-libs/lib/falcosecurity -L/usr/local/Cellar/jsoncpp/1.9.5/lib -L/usr/local/Cellar/tbb/2021.5.0_2/lib -lsinsp -lscap -lscap_event_schema -lscap_engine_util -lscap_engine_nodriver -lscap_engine_savefile -lscap_engine_source_plugin -lscap_engine_udig -ljsoncpp -ltbb

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: geraldcombs Once this PR has been reviewed and has the lgtm label, please assign lucaguerra for approval by writing /assign @lucaguerra in a comment. For more information see:The Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

Hi Gerald! Thanks for this great PR! :) Do you think we could leverage something like cmake's get_target_property() instead of manually updating all the cmake modules? I've done something similar here: https://github.com/FedeDP/libmodule/blob/bec233bddcbbb16b45cc6edc04a6264bed404fbd/Lib/CMakeLists.txt#L10 Do you think it is feasible? I think it is way more future proof :)

Thank you btw, this is really a great help for people using libs for external projects!

Do you think we could leverage something like cmake's get_target_property() instead of manually updating all the cmake modules?

I used get_target_property to build the scap library list, but the pkg-config module list is a bit more tricky. If I add

get_target_property(sinsp_link_libraries sinsp LINK_LIBRARIES)
message(WARNING "sinsp link libraries: ${sinsp_link_libraries}")

cmake prints

CMake Warning at userspace/libsinsp/CMakeLists.txt:332 (message):
  sinsp link libraries:
  scap;/usr/local/lib/libjsoncpp.dylib;/usr/local/lib/libtbb.dylib;dl;pthread

We could build a module list based on entries that start with "/", but we run into the next problem: There doesn't seem to be a clean way to map library names to pkg-config module names. Some pkg-config modules use the plain library name, and some start with "lib". For example, c-ares, curl, jsoncpp, and tbb have modules named libcares.pc, libcurl.pc, jsoncpp.pc, and tbb.pc respectively. I suppose we could map them manually, e.g.

set(libcares_pc_module libcares.pc)
[ ... ]
set(libtbb_pc_module tbb.pc)
foreach (_link_lib sinsp_link_libraries)
  set(libname [ basename of the library ])
  if (${${libname}_pc_module)
    list(APPEND SINSP_PKG_CONFIG_REQUIRES ${${libname}_pc_module)
  endif()
endforeach()

This wouldn't remove the maintenance burden entirely, but it would move it to one place (the _pc_module variable list).

Also, any preferences on naming the files libsinsp.pc and libscap.pc vs sinsp.pc and scap.pc?

/ok-to-test

OK, it now builds the list of libsinsp requirements based on the LINK_LIBRARIES property. We still have to manage a library-to-.pc-name map, but it's in one place instead of being scattered around cmake/modules..

/milestone 0.9.0

Hey @geraldcombs could you rebase pls?

/milestone 0.10.0

Hey @geraldcombs could you rebase pls?

Done!

I don't think we will make it for the 0.10.0. Moving to 0.11.0. /milestone 0.11.0

LGTM label has been added.

Git tree hash: da1b68fd3b6bc4d5eaa01d15fd422710403cfc8c

Hey @geraldcombs

Unfortunately, this breaks some CI jobs :(

cc @Andreagit97

Unfortunately, this breaks some CI jobs :(

Looks like another REMOVE_DUPLICATES issue. I've pushed an attempted fix.

Hi @geraldcombs! I think that, if you rebase this one and add a CI job to:

• build libs as shared
• install them in the system
• use the pkg config files to build eg: the sinsp-example as an out of tree build (ie: by manually invoking gcc pkg-config ...)

We will be sure that we won't break shared libs nor pkg-config anymore and we can then merge this one ;)

Thank you!

Started test on Falco:

• https://github.com/falcosecurity/falco/actions/runs/4487519254
• https://app.circleci.com/pipelines/github/falcosecurity/falco/3862/workflows/eeee2f49-a5ec-4bea-8f37-740b1d1aed01

Falco CI is failing but that is not important since libs ci is failing too :smile:

Hi @geraldcombs! I think that, if you rebase this one and add a CI job to:

* build libs as shared

* install them in the system

* use the pkg config files to build eg: the sinsp-example as an out of tree build (ie: by manually invoking gcc `pkg-config ...`)

I added a build-shared-libs-linux-amd64 job which should take care of this (although I in no way claim to be a GitHub CI expert).

It seems like we need to enable PIC for shared build (i thought cmake enabled it by default though)!

It seems like we need to enable PIC for shared build (i thought cmake enabled it by default though)!

It looks like that's due to .github/install-deps.sh installing static versions of re2 and valijson.

Hi! Can you rebase? We still need to understand why new CI job is not finding headers, but they were installed actually:

make[4]: *** /lib/modules/5.15.0-1034-azure/build: No such file or directory. Stop.

Aaaand we need to use sudo on host :/ I always forget about that!

I took the freedom to push onto the PR to fix the CI :) It is working!!! :rocket: Now i'll open a branch on Falco and if everything is fine we can finally merge this! Thank you very much for this huge effort :)

EDIT:

• [x] https://app.circleci.com/pipelines/github/falcosecurity/falco/3899/workflows/770b2453-aa59-4108-9a87-a64f9d52db5f
• [x] https://github.com/falcosecurity/falco/actions/runs/4605257330

LGTM label has been added.

Git tree hash: 730b9c28e3dbdf144df8980597969ecf141dc8d6

/hold to answer my little question above.

LGTM label has been added.

Git tree hash: ca6af9ceff3476e9ed1cd3e90503e37a84914229

/unhold

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: FedeDP, geraldcombs, jasondellaluce

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• ~~OWNERS~~ [FedeDP,jasondellaluce]

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

@araujof @terylt i am pretty sure this is interesting news for you :)