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

new(proposals): libraries donation #1530

Merged
merged 1 commit into from
Feb 4, 2021
Merged

Conversation

leodido
Copy link
Member

@leodido leodido commented Jan 19, 2021

Co-authored-by: Lorenzo Fontana lo@linux.com
Signed-off-by: Leonardo Di Donato leodidonato@gmail.com

What type of PR is this?

/kind documentation

Any specific area of the project related to this PR?

/area proposals

What this PR does / why we need it:

Donate:

  • libsinsp
  • libscap
  • the kernel module driver
  • the eBPF driver sources

by moving them to the Falco project.

Which issue(s) this PR fixes:

NONE

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

docs(proposals): libraries and drivers donation

Donate:
- libsinsp
- libscap
- the kernel module driver
- the eBPF driver sources

by moving them to the Falco project.

Co-authored-by: Lorenzo Fontana <lo@linux.com>
Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
@deepskyblue86
Copy link
Contributor

How do you think to proceed with their versioning? I read everything will be in falcosecurity/libs, do you want to release everything with the same version?

@leodido
Copy link
Member Author

leodido commented Jan 19, 2021

@deepskyblue86 at the moment Falco uses a commit hash to reference the libs. That hash is also the "driver version".

See

set(SYSDIG_CHECKSUM "SHA256=9de717b3a4b611ea6df56afee05171860167112f74bb7717b394bcc88ac843cd")

This mechanism proved to work pretty well (at least, better than the previous one).

Anyway, this proposal is about the donation of the libraries (and the drivers' sources) to the falcosecurity organization. That's the goal here. :)

Changes to their versioning scheme, like versioning them separately, need to be eventually proposed and discussed in a follow-up proposal. Because such changes would impact a lot of areas (build system, release system, prebuilt drivers machinery, etc.).

@deepskyblue86
Copy link
Contributor

Sorry @leodido, but I raised it because I read the following:

  1. Distribute the libsinsp.so library and headers as an artifact (rpm, deb, tar.gz) following the falcosecurity current process
  2. Distribute the libsinsp.a library and headers as an artifact (rpm, deb, tar.gz) following the falcosecurity current process
  3. Creation of the CI scripts using the Falco CI and Falco Infra
  4. The CI scripts will need to publish the artifacts in the current falcosecurity artifacts repository
  5. Artifacts will be pushed for every tag (release) and for every master merge (development release)
    ...

Falco

  1. When distributing the deb and rpm, libscap and libsinsp will need to be install dependencies and not anymore compiled into Falco

so I assumed it was to be discussed now.

@leodido
Copy link
Member Author

leodido commented Jan 19, 2021

In fact, we are discussing it, Angelo :)

What I was trying to say is: the falcosecurity current process (at the moment) relies on that hash (version), which is the same for all the 3 (libscap, libsinp, drivers) underlying pieces.

Changing the versioning scheme would require to also change the current falcosecurity processes for building and distributing software. Which rather is not the scope of this proposal.

In this document we just propose to donate those libraries by keeping the versioning, building, releasing etc as much as similar to the existing one.

@deepskyblue86
Copy link
Contributor

Discussed privately with @leodido. I was just raising the issue of having a semver for the libraries and not just the git commit hash, due to deb/rpm package management. Of course, it can just be something like x.y.z. if needed.

We also discussed having just a single package with all these components — like falcosecurity-libs — for release and maintenance convenience, given that there's already the idea to have them with the same version.

@leogr
Copy link
Member

leogr commented Jan 19, 2021

In this document we just propose to donate those libraries by keeping the versioning, building, releasing etc as much as similar to the existing one.

👍 this proposal explains "the full plan", and I really love that.

Initial steps to start the migration are:

  1. Extract libsinsp from draios/sysdig/userspace/libsinsp (keeping the commit history) into falcosecurity/libs

  2. The migration comes first, then we can do additional PRs for the points below so that we do only one thing at a time and keep the history linear

  3. Keep the same code, refactorings will need to be done in subsequent PRs and approved separately

And I totally agree with that. Doing so will allow us to start using the migrated libraries almost immediately. Since I wanted to try, I already did a PoC (will share soon 😸 ).

Then we can proceed with other refinements in a step-by-step fashion.

Btw, if the main question here is "Would you like to accept the donation?" - as I believe - my answer is: a strong YES 😀

Copy link
Member

@leogr leogr left a comment

Choose a reason for hiding this comment

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

I strongly agree with this proposal, so I'm approving. 🥳

I have also created a sort of PoC (see below) to demonstrate that the very first steps of this migration can be done without any disruption for our users.

N.B. The two links above are just examples that I wanted to share. They might not exactly represent the final work.

@poiana
Copy link
Contributor

poiana commented Jan 29, 2021

LGTM label has been added.

Git tree hash: 3fa12f13f4ac6a2e9aa5b639ae39a26670437379

@poiana
Copy link
Contributor

poiana commented Feb 4, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fntlnz, leogr

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:

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

@poiana poiana merged commit 19fe724 into master Feb 4, 2021
@poiana poiana deleted the new/proposal-move-libraries branch February 4, 2021 16:29
@leodido
Copy link
Member Author

leodido commented Apr 9, 2021

/milestone 0.28.0

@poiana poiana added this to the 0.28.0 milestone Apr 9, 2021
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.

5 participants