Changed required_engine_version from 0.26.0 to 26. #195
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: petterreinholdtsen The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
poiana
added
area/maturity-incubating
Rules files suggestionsfalco_rules.yamlComparing Minor changes:
falco-sandbox_rules.yamlComparing No changes detected falco-incubating_rules.yamlComparing Minor changes:
|
The 0.36.2 parser reject the 0.26.0 value and falco --version report engine_version as 26, so using this number as the value. Signed-off-by: Petter Reinholdtsen <[email protected]>
petterreinholdtsen
force-pushed
the
required_engine_version-single-number
branch
from
657bc59 to
d3fdcd5
Compare
Rules files suggestionsfalco_rules.yamlComparing Minor changes:
falco-sandbox_rules.yamlComparing No changes detected falco-incubating_rules.yamlComparing Minor changes:
|
|
Hello @petterreinholdtsen, this was intended, as we are currently transitioning from expressing |
|
[Lorenzo Susini]
Hello @petterreinholdtsen, this was intended, as we are currently
transitioning from expressing `required_engine_version` as pure number
to a semver string.
Aha. I would strongly recommend to implement in steps, where first a
new software version with support for sematic versioning is made widely
available, and then change the provide rule set to use this new file
format. This will allow existing users of falco to use the latest rule
set despite not having the very latest version of the user space
daemon.
--
Happy hacking
Petter Reinholdtsen
|
Hi @petterreinholdtsen I understand your frustration. We officially state that "... This means that rules in the main branch can become incompatible with the latest stable Falco release if, for example, new output fields are introduced" in the README, see https://github.com/falcosecurity/rules#falco-rules-1. The reason for this is that we cannot leave all changes to the last minute when releasing the new Falco binary. We hope this is understandable and we tried our best to emphasize that the main branch reflects the current dev state in all docs. I believe for the most part the current dev rules can still be adopted after quickly testing them and spotting maybe 1-2 rules that cannot yet be adopted until the next Falco release is out. Would it be ok to close this PR and mark as resolved? |
|
[Melissa Kilby]
Would it be ok to close this PR and mark as resolved?
Well, it is your loss, not mine, to close it without any changes done to
the code base. I already did the change in my local edition.
If you do not want to keep the rules backwards compatible, and not
provide users with a transition period, it is of course up to you. I
believe it is a bad choice reflecting badly on maturity of a project,
but it is good to learn that it is at least a concious choice and not
something that was done by accident.
In this particular case, as far as I can tell, there are no new output
fields introduced. The new rules are parsed by 0.36.2, and the only
incompatibility introduced is that some (but not all) files switched
structure of the required_engine_version value for no apparent reason. The
engine version of 0.36.2 is 26, and I assume 0.26.0 is equivalent of 26,
as the documentation state. If this is incorrect, my patch is wrong.
…--
Happy hacking
Petter Reinholdtsen
|
I understand that you're frustrated that the main/dev branch isn't compatible with your needs, but I want to assure you that we follow standard software engineering practices. The stable release branches are compatible with each other. This may not be the news you were hoping for, but I hope you can understand our position. The decision to transition to semver as our versioning system was made collaboratively by all maintainers. Please note that the example provided in the README is merely one illustration of potential compatibility issues that may arise. This change to semver is aligned with the project's best interests, ensuring long-term compatibility and stability. |
I would like to specify that supporting the new version scheme inherently will require a new engine version in order to work (from 26 to 0.27.0, in this case). I think the source of confusion here is that the version referenced here is still 26, whereas the the engine at that version was not meant to support the sem-ver version notation. So the right fix, IMO, should be to set the new required engine version to 0.27.0, instead of reverting it to 26. Note, your would have encountered the same exact failure if we decided to update This being said, I definitely see your point, however we never advertised that pulling files straight from our mainline branches is something we support as stable. The things we are able of committing to maintain as stable, given the current availability of maintainers and contributors, are the release branches and tags. Note, the intentionality is also reflected by the CI jobs running on mainline -- see here for example: https://github.com/falcosecurity/rules/actions/runs/6849300053. We validate our rules against different Falco versions, and we're quite aware that Falco 0.36.0 does not validate the current latest ones. |
|
cc @LucaGuerra |
|
Closing in favor of #218 |
|
@leogr: Closed this PR. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
The parser in version 0.36.2 reject the 0.26.0 value and
falco --versionreport engine_version as 26, so using this number as the value./kind bug
/area rules
/area maturity-incubating
/area maturity-sandbox
/area maturity-deprecated