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

docs: improve documentation and description of base_syscalls option #2515

Merged
merged 2 commits into from May 18, 2023

Conversation

happy-dude
Copy link
Contributor

@happy-dude happy-dude commented Apr 27, 2023

What type of PR is this?

/kind documentation

What this PR does / why we need it:

  • Improve documentation the base_syscalls option.
  • Shuffles and re-words a few blurbs to help provide the user with context and actionable items

Which issue(s) this PR fixes:

(no issue ticket filed)

Special notes for your reviewer:

cc @incertum @jasondellaluce for review, thanks for help!

Does this PR introduce a user-facing change?:

No, since this is a net-new feature going into Falco 0.35+

NONE

@happy-dude
Copy link
Contributor Author

Generally, I moved things around in a format where it flows from

  • state engine context
  • default behavior
  • configuring just custom_set
  • configuring repair
  • syntax usage
  • suggestions

@happy-dude
Copy link
Contributor Author

Also, I would like to add a blurb about the -A command line flag, although I'm not fully informed of it's effects in relation to the varied ways base_syscalls can be configured.

falco.yaml Show resolved Hide resolved
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.

@happy-dude 🤩 OMG this is amazing! Could we get your help to improve the config descriptions throughout over time 🙃 ?

Thanks so much truly ❤️ !

Re -A @jasondellaluce would you have ideas? It's not technically related to base_syscalls and we do have good warning messages, but at the same time mentioning it here could not hurt if we find a good way of doing it?

falco.yaml Outdated Show resolved Hide resolved
# When enabled, this option is designed to
# (1) ensure that Falco's state engine is successfully built-up correctly
# (2) be the most system resource-friendly by activating the least number of
# additional syscalls (outside of those enabled for enabled rules)
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️ amazing Stanley!!!! Much better way of selling this feature.

falco.yaml Outdated Show resolved Hide resolved
falco.yaml Show resolved Hide resolved
@happy-dude
Copy link
Contributor Author

Resolved some suggestions, typos, and wording, thanks @incertum !

@happy-dude
Copy link
Contributor Author

Commit + rebased another suggestion.

Also added a new commit that cleans up some extraneous whitespace on some lines. Can drop if it doesn't belong in this PR 👍

falco.yaml Outdated Show resolved Hide resolved
falco.yaml Show resolved Hide resolved
falco.yaml Outdated Show resolved Hide resolved
falco.yaml Outdated Show resolved Hide resolved
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.

This is an incredible work! Thank you very much @happy-dude !
Great research and wording!!
Thank you, left some comments!

falco.yaml Outdated Show resolved Hide resolved
falco.yaml Outdated Show resolved Hide resolved
falco.yaml Outdated Show resolved Hide resolved
falco.yaml Outdated Show resolved Hide resolved
falco.yaml Outdated Show resolved Hide resolved
@FedeDP
Copy link
Contributor

FedeDP commented May 2, 2023

/milestone 0.35.0

@poiana poiana added this to the 0.35.0 milestone May 2, 2023
@happy-dude
Copy link
Contributor Author

Apologies for the delay, thanks for the suggestions, Federico!

Made use of the super-neat GitHub batch + signoff + commit feature and rebased latest suggestions :)

Removing WIP tag from PR and looking for approvals for merge 🙏
Thanks for the suggestions, everyone!

@happy-dude happy-dude changed the title (WIP) docs: improve documentation and description of base_syscalls option docs: improve documentation and description of base_syscalls option May 8, 2023
FedeDP
FedeDP previously approved these changes May 8, 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 poiana added the lgtm label May 8, 2023
@poiana
Copy link

poiana commented May 8, 2023

LGTM label has been added.

Git tree hash: ac830d8e3ccf5414d58ccbe9f822ccbfd8a22710

@poiana poiana added the approved label May 8, 2023
falco.yaml Outdated Show resolved Hide resolved
happy-dude and others added 2 commits May 15, 2023 09:53
Co-authored-by: Federico Di Pierro <nierro92@gmail.com>
Co-authored-by: Melissa Kilby <melissa.kilby.oss@gmail.com>
Signed-off-by: Stanley Chan <pocketgamer5000@gmail.com>
Signed-off-by: Stanley Chan <pocketgamer5000@gmail.com>
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 poiana added the lgtm label May 15, 2023
@poiana
Copy link

poiana commented May 15, 2023

LGTM label has been added.

Git tree hash: d7c47eda10886aa16261e9e3074ecbd83887d612

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

poiana commented May 18, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: FedeDP, Happy-Dude, leogr

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 merged commit 3403225 into falcosecurity:master May 18, 2023
16 checks passed
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.

None yet

5 participants