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

update(userspace/falco): introduce new engine_version_semver key in v… #2899

Merged
merged 1 commit into from
Nov 8, 2023

Conversation

loresuso
Copy link
Member

@loresuso loresuso commented Nov 2, 2023

…ersions endpoint

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

/kind release

Any specific area of the project related to this PR?

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

/area build

/area engine

/area tests

/area proposals

/area CI

What this PR does / why we need it:
With this PR we introduce a new key in the Falco /versions endpoint. This key will be used by falcoctl to match new artifact configs containing a full semver string for representing the required engine version.

The rationale for this change is the following:

  • Falco is already able to read engine_version both as numeric valuer or semver string
  • Rules repositories must be updated to use a full semver string for required_engine_version (already did it for Falco rules, will need this also for plugin rules). This will require a major bump for each artifact
  • New artifact config layers will contains the new engine_version_semver key
  • Let's also point out that falcoctl is already "keys agnostic". It just starts processing and matching keys starting from the config layer, and as long as it finds the same key in the /versions endpoint, we are fine.

With this being said, we end up in this situation (example for falco rules, will be the same for plugin rules):

Falco version Rules and CI versions Outcome
< 0.37 <= 2.0.0 Nothing changed, will work as before
< 0.37 > 2.0.0 If you upgrade only the rules, falcoctl will find two different keys and will not be able to match them, causing an error in falcoctl, not upgrading the rules. Also, if you manually start Falco with the new rules (on host, maybe) Falco will not start. I think this is perfectly fine, cause the rule will have a new major, so you must update Falco as well (would be the same if the engine version was only bumped, even if it was numeric)
>= 0.37 <= 2.0.0 Falco is updated. You can continue to follow 2.x.y rules, because it is able to read both a numeric engine version and a semver one. If you want, you can also follow the new rules (3.0.0)(see below)
>= 0.37 >2.0.0 Everything is new, so it will work as expected by matching the new key

PRs for updating the config layers are coming soon!

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

update(userspace/falco): add `engine_version_semver` key in `/versions` endpoint

@loresuso
Copy link
Member Author

loresuso commented Nov 2, 2023

cc @alacuku, @LucaGuerra for visibility

@alacuku
Copy link
Member

alacuku commented Nov 2, 2023

Hey @loresuso, thanks for the PR!

I think we should write some integrations tests in falcoctl to cover all the cases described above. To make sure that we are not going to break running instances of falco when upgrading them. I wuold be happy to help writing those tests.

@loresuso
Copy link
Member Author

loresuso commented Nov 2, 2023

@alacuku Agreed, we should somehow mock the version endpoint for the falcoctl test and try out the function that checks the requirments. Happy to review if you are up for doing it!

@FedeDP
Copy link
Contributor

FedeDP commented Nov 7, 2023

/milestone 0.37.0

@poiana poiana added this to the 0.37.0 milestone Nov 7, 2023
Copy link
Contributor

@FedeDP FedeDP left a comment

Choose a reason for hiding this comment

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

/approve

@poiana
Copy link
Contributor

poiana commented Nov 7, 2023

LGTM label has been added.

Git tree hash: 17f09781c489476f8b20c1c9b60fdfe4cc109bb8

@poiana poiana added the approved label Nov 7, 2023
Copy link
Contributor

@incertum incertum left a comment

Choose a reason for hiding this comment

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

/approve

@poiana
Copy link
Contributor

poiana commented Nov 8, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: FedeDP, incertum, leogr, loresuso

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,incertum,leogr]

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

6 participants