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

custom rules first steps #1077

Merged
merged 5 commits into from
Jul 26, 2023
Merged

custom rules first steps #1077

merged 5 commits into from
Jul 26, 2023

Conversation

vjjmiras
Copy link
Contributor

What type of PR is this?

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

/kind bug

/kind cleanup

/kind design

/kind user-interface

/kind content

/kind translation

Any specific area of the project related to this PR?

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

/area blog

/area documentation

/area videos

What this PR does / why we need it:

Which issue(s) this PR fixes:

This document completes the PR #1075

Special notes for your reviewer:

/cc @incertum, tagging you here since for a review, please :-)

@poiana poiana requested review from Issif and mstemm July 24, 2023 12:28
incertum
incertum reviewed Jul 24, 2023
View reviewed changes
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.

This is great. Just wondering if we can better cross-link this page to the existing basic pages and now also the new style guide. I think redundancy is good here aka let's not remove things from other places and instead just always link back and forth between pages?

content/en/docs/rules/custom-rules.md Outdated Show resolved Hide resolved
content/en/docs/rules/custom-rules.md Outdated Show resolved Hide resolved
vjjmiras added 2 commits July 25, 2023 16:14
@vjjmiras
new(docs): custom rules best practices
30624d5 
Signed-off-by: Vicente J. Jiménez Miras <[email protected]>
@vjjmiras
update(docs): add some suggestions and rename the title
05ec93d 
Signed-off-by: Vicente J. Jiménez Miras <[email protected]>
@vjjmiras vjjmiras changed the title WIP: custom rules first steps custom rules first steps Jul 25, 2023
leogr
leogr reviewed Jul 25, 2023
View reviewed changes
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.

This is impressive 🤩

Overall, SGTM. I've just left some minor suggestions.

Also, I'd like to clarify that this guide applies to the syscall data source (the default Falco use case). However, plugin rules may be different. For example, the evt.type recommendation does not apply to plugin rules.

content/en/docs/rules/custom-rules.md Outdated Show resolved Hide resolved
content/en/docs/rules/custom-rules.md Outdated Show resolved Hide resolved
content/en/docs/rules/custom-rules.md Outdated Show resolved Hide resolved
content/en/docs/rules/custom-rules.md Outdated Show resolved Hide resolved
@vjjmiras
update(docs): add tags to the rule structure
e2f1a9c 
Signed-off-by: Vicente J. Jiménez Miras <[email protected]>
@incertum
Copy link
Contributor

LGTM

nate-double-u
nate-double-u reviewed Jul 26, 2023
View reviewed changes
Copy link
Contributor

@nate-double-u nate-double-u left a comment

Choose a reason for hiding this comment

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

I've gone through and done a copy-edit pass in line.

Feel free to take or leave any bit of advice -- some of my word choice suggestions come down to my trying to be brief.

content/en/docs/rules/custom-ruleset.md Outdated Show resolved Hide resolved
content/en/docs/rules/custom-ruleset.md Outdated Show resolved Hide resolved
content/en/docs/rules/custom-ruleset.md Outdated Show resolved Hide resolved
content/en/docs/rules/custom-ruleset.md Outdated Show resolved Hide resolved
content/en/docs/rules/custom-ruleset.md Outdated Show resolved Hide resolved
content/en/docs/rules/custom-ruleset.md Outdated Show resolved Hide resolved
content/en/docs/rules/custom-ruleset.md Outdated Show resolved Hide resolved
content/en/docs/rules/custom-ruleset.md Outdated Show resolved Hide resolved
content/en/docs/rules/custom-ruleset.md Outdated Show resolved Hide resolved
content/en/docs/rules/custom-ruleset.md Outdated Show resolved Hide resolved
@vjjmiras @nate-double-u
update(docs): apply suggestions from code review
0341e0f 
Co-authored-by: Nate W <[email protected]>
Signed-off-by: Vicente JJ. Miras <[email protected]>
@vjjmiras
Copy link
Contributor Author

eel free to take or leave any bit of advice -- some of my word choice suggestions come down to my trying to be brief.

Thanks a lot, Nate. Great suggestions.

@vjjmiras vjjmiras requested a review from leogr July 26, 2023 10:35
leogr
leogr approved these changes Jul 26, 2023
View reviewed changes
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.

🥳

@poiana poiana added the lgtm label Jul 26, 2023
@poiana
Copy link

poiana commented Jul 26, 2023

LGTM label has been added.

Git tree hash: 87e7587b0eaed9dcfee37b65955c5d6a3665c336

@poiana
Copy link

poiana commented Jul 26, 2023

[APPROVALNOTIFIER] This PR is APPROVED

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

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

@leogr leogr mentioned this pull request Jul 26, 2023
Closed
@poiana poiana merged commit edaa737 into falcosecurity:master Jul 26, 2023
@leogr leogr mentioned this pull request Jul 26, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@incertum incertum incertum left review comments

@nate-double-u nate-double-u nate-double-u left review comments

@leogr leogr leogr approved these changes

@Issif Issif Awaiting requested review from Issif

@mstemm mstemm Awaiting requested review from mstemm

Assignees

@leogr leogr

Labels
approved area/documentation dco-signoff: yes kind/content lgtm size/XL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@vjjmiras @incertum @poiana @leogr @nate-double-u