search

falcosecurity / plugins

Falco plugins registry
Apache License 2.0
78 stars 71 forks source link

fix(plugins/k8saudit/rules): split rbac rules by individual rbac object #484

Closed sboschman closed 2 months ago

sboschman commented 2 months ago

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 plugins

/area registry

/area build

/area documentation

What this PR does / why we need it: k8saudit ruleset is only detecting create/delete events for ClusterRoleBinding objects. As the rules do detect the create/delete events for Role objects, it makes sense to detect create/delete events for RoleBinding objects as well.

As Role and RoleBinding are namespace scoped objects, I did split the rules out for each individual rbac object to include the namespace field into the output. As well as to make it easier to see the difference between cluster wide and namespace scoped objects by rule name, instead of having to parse out the 'resources' field.

Which issue(s) this PR fixes:

Fixes #319

Special notes for your reviewer:

sboschman commented 2 months ago

@alacuku, it looks like the new tag format is missing somewhere, the actual git tag is plugins/k8saudit/v0.8.0

error: pathspec 'tags/k8saudit-0.8.0' did not match any file(s) known to git

Hopefully you have time to have a look 🙏

alacuku commented 2 months ago

The #482 has not been merged yet. Could you rebase on that?

sboschman commented 2 months ago

/retest

sboschman commented 2 months ago

@alacuku don't think #482 fixes this issue, test still fails after the rebase (this pr updates the k8saudit rules, it has nothing to do with k8saudit-gke)

alacuku commented 2 months ago

@sboschman, #482 extends the rules checker so it impacts all the plugins. Found bug, related to how we built the tag for the plugins. I just pushed the fix. Could you please rebase on it?

github-actions[bot] commented 2 months ago

Rules files suggestions

rules

Comparing fcc9b5e4bb07f986a55a1aa4b6a36dcf7aa89d73 with latest tag plugins/k8saudit/v0.8.0

No changes detected

sboschman commented 2 months ago

nice @alacuku , the build succeeds

Not sure what this 'rules files suggestions' is supposed to do, it says:

No changes detected

But this PR changed the rules... so should it show a diff in the rules?

alacuku commented 2 months ago

nice @alacuku , the build succeeds

Not sure what this 'rules files suggestions' is supposed to do, it says:

No changes detected

But this PR changed the rules... so should it show a diff in the rules?

I'm not the original author of the CI, so can't say. I modified that part of the CI maybe I missed something. I'll have a look at it right now.

alacuku commented 2 months ago

@sboschman, I pushed the fix #482. Could you test it?

github-actions[bot] commented 2 months ago

Rules files suggestions

rules

Comparing 029593e335101df1d4f0734a8d447ed8db718203 with latest tag plugins/k8saudit/v0.8.0

Major changes:

Minor changes:

sboschman commented 2 months ago

☝️ @alacuku nice looking rules comparison 💪

leogr commented 2 months ago

@Issif could you take a look at this please? :pray:

leogr commented 2 months ago

/assign

github-actions[bot] commented 2 months ago

Rules files suggestions

rules

Comparing e36500b1ce48e046c05467f587b83d6120562bcb with latest tag plugins/k8saudit/v0.8.0

Major changes:

Minor changes:

sboschman commented 2 months ago

It makes totally sense, and the changes are correct to me. IMHO it requires also a bump of the version of the rules (with the relevant changelog).

@Issif version bump and changelog are included

Issif commented 2 months ago

Good to me, cc @leogr

/lgtm

poiana commented 2 months ago

LGTM label has been added.

Git tree hash: c9800ba45cb8a40255fc1eca0b5aef61eb6fb0bd

poiana commented 2 months ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Issif, leogr, sboschman

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](https://github.com/falcosecurity/plugins/blob/main/OWNERS)~~ [leogr] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment