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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 2 commits into from
May 3, 2024
Merged

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

merged 2 commits into from
May 3, 2024

Conversation

sboschman
Copy link
Contributor

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:

@poiana poiana added kind/bug Something isn't working dco-signoff: yes kind/feature New feature or request labels May 1, 2024
@poiana poiana requested a review from leogr May 1, 2024 06:14
@poiana poiana requested a review from mstemm May 1, 2024 06:14
@poiana poiana added the size/M label May 1, 2024
@sboschman
Copy link
Contributor Author

@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
Copy link
Member

alacuku commented May 2, 2024

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

@poiana poiana added size/L and removed size/M labels May 2, 2024
@sboschman
Copy link
Contributor Author

/retest

@sboschman
Copy link
Contributor Author

@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
Copy link
Member

alacuku commented May 2, 2024

@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 GitHub Actions
Copy link

github-actions bot commented May 2, 2024

Rules files suggestions

rules

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

No changes detected

@sboschman
Copy link
Contributor Author

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
Copy link
Member

alacuku commented May 2, 2024

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
Copy link
Member

alacuku commented May 2, 2024

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

@sboschman
fix(plugins/k8saudit/rules): split rbac rules by individual rbac object
029593e 
Signed-off-by: Sverre Boschman <1142569+sboschman@users.noreply.github.com>
@poiana poiana added size/M and removed size/L labels May 2, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented May 2, 2024

Rules files suggestions

rules

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

Major changes:

  • Rule K8s Role/Clusterrolebinding Deleted has been removed
  • Rule K8s Role/Clusterrole Deleted has been removed
  • Rule K8s Role/Clusterrolebinding Created has been removed
  • Rule K8s Role/Clusterrole Created has been removed

Minor changes:

  • Rule K8s RoleBinding Created has been added
  • Rule K8s ClusterRole Deleted has been added
  • Rule K8s Role Created has been added
  • Rule K8s RoleBinding Deleted has been added
  • Rule K8s Role Deleted has been added
  • Rule K8s ClusterRole Created has been added
  • Rule K8s ClusterRoleBinding Deleted has been added
  • Rule K8s ClusterRoleBinding Created has been added
  • Macro rolebinding has been added

@sboschman
Copy link
Contributor Author

☝️ @alacuku nice looking rules comparison 💪

@sboschman sboschman mentioned this pull request May 2, 2024
Merged
@leogr
Copy link
Member

leogr commented May 2, 2024

@Issif could you take a look at this please? 🙏

@leogr
Copy link
Member

leogr commented May 2, 2024

/assign

Issif
Issif reviewed May 2, 2024
View reviewed changes
Copy link
Member

@Issif Issif left a comment

Choose a reason for hiding this comment

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

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).

@sboschman
chore(plugins/k8saudit): bump plugin version to 0.9.0
e36500b 
Signed-off-by: Sverre Boschman <1142569+sboschman@users.noreply.github.com>
@github-actions GitHub Actions
Copy link

github-actions bot commented May 3, 2024

Rules files suggestions

rules

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

Major changes:

  • Rule K8s Role/Clusterrolebinding Created has been removed
  • Rule K8s Role/Clusterrolebinding Deleted has been removed
  • Rule K8s Role/Clusterrole Created has been removed
  • Rule K8s Role/Clusterrole Deleted has been removed

Minor changes:

  • Rule K8s ClusterRole Created has been added
  • Rule K8s RoleBinding Deleted has been added
  • Rule K8s Role Created has been added
  • Rule K8s ClusterRole Deleted has been added
  • Rule K8s ClusterRoleBinding Created has been added
  • Rule K8s Role Deleted has been added
  • Rule K8s RoleBinding Created has been added
  • Rule K8s ClusterRoleBinding Deleted has been added
  • Macro rolebinding has been added

@sboschman
Copy link
Contributor Author

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
Copy link
Member

Issif commented May 3, 2024

Good to me, cc @leogr

/lgtm

@poiana poiana added the lgtm label May 3, 2024
@poiana
Copy link
Contributor

poiana commented May 3, 2024

LGTM label has been added.

Git tree hash: c9800ba45cb8a40255fc1eca0b5aef61eb6fb0bd

@poiana
Copy link
Contributor

poiana commented May 3, 2024

[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:

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

@poiana poiana added the approved label May 3, 2024
@poiana poiana merged commit 42fcdae into falcosecurity:main May 3, 2024
12 checks passed
@leogr leogr mentioned this pull request May 3, 2024
Open
@sboschman sboschman deleted the k8saudit-rbac-rules branch May 3, 2024 09:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Issif Issif Issif left review comments

@leogr leogr leogr approved these changes

@mstemm mstemm Awaiting requested review from mstemm

Assignees

@leogr leogr

@Issif Issif

Labels
approved area/plugins dco-signoff: yes kind/bug Something isn't working kind/feature New feature or request lgtm size/M
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

k8s_audit_rules missing rolebinding
5 participants
@sboschman @alacuku @leogr @Issif @poiana