chore(docs): added falco.yaml section about config keys maturity. #3206

FedeDP · 2024-05-20T08:50:25Z

What type of PR is this?

/kind cleanup

Any specific area of the project related to this PR?

What this PR does / why we need it:

Also, rename Experimental -> Incubating and move prometheus_metrics_enabled to Incubating.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

See the proposal: https://github.com/falcosecurity/falco/blob/master/proposals/20231220-features-adoption-and-deprecation.md

Does this PR introduce a user-facing change?:

chore(docs): apply features adoption and deprecation proposal to config file keys

Also, rename `Experimental` -> `Incubating` and move `prometheus_metrics_enabled` to `Incubating`. Signed-off-by: Federico Di Pierro <[email protected]>

FedeDP · 2024-05-20T08:50:31Z

/milestone 0.38.0

FedeDP · 2024-05-20T08:51:10Z

/cc @leogr @incertum @LucaGuerra

FedeDP · 2024-05-20T08:51:56Z

falco.yaml

@@ -749,6 +762,8 @@ webserver:
  # Can be an IPV4 or IPV6 address, defaults to IPV4
  listen_address: 0.0.0.0
  k8s_healthz_endpoint: /healthz
+  # [Incubating] `prometheus_metrics_enabled`


Am not sure about how to integrate the feature level label here, for a sub-option of a Stable config key.

Makes sense to me like this.

This way works for me 👍

FedeDP · 2024-05-20T08:52:07Z

falco.yaml

@@ -525,7 +538,7 @@ json_include_tags_property: true
 # output mechanism. By default, buffering is disabled (false).
 buffered_outputs: false

-# [Experimental] `rule_matching`
+# [Incubating] `rule_matching`


Experimental -> Incubating

urgh ok here we should maybe also introduce a synonym rules_matching @leogr?

I think singular is ok, in this case; it is rule-matching algorithm, feels ok rule_matching: foo to me.

FedeDP · 2024-05-20T08:53:23Z

I was also wondering whether we could improve somehow the way maturity labels are attached, ie: perhaps moving them to the index at the top? Or moving them just the line before the configuration key, like:

# `configs_files`
#
# Falco will load additional configs files specified here.
# Their loading is assumed to be made *after* main config file has been processed,
# exactly in the order they are specified.
# Therefore, loaded config files *can* override values from main config file.
# Also, nested include is not allowed, ie: included config files won't be able to include other config files.
#
# Like for 'rules_files', specifying a folder will load all the configs files present in it in a lexicographical order.
#
# [Stable] 
configs_files:
  - /etc/falco/config.d

incertum · 2024-05-20T08:56:16Z

falco.yaml

@@ -25,7 +25,7 @@
 #
 #     (Falco command-line arguments)
 #     (Falco environment variables)
-# Falco config files
+# Falco configs files


Oh ok we may need to change the sha256 metrics name then as well. @FedeDP would you mind adding a commit for that real quick here? Thanks!

Am not sure though, since we have watch_config_files too, and indeed we have multiple configuration (singular) files (plural), so config_files is not that bad after all, even if it somehow clashes with rules_files (but in this case, we have multiple files with multiple rules inside).
WDYT? Ie i am proposing to rename configs_files to config_files (not a breaking change since we introduced the new key during this release cycle, so we never went out with that).
cc @leogr

I guess config_files would be more appropriate since it uses the singular form config as an adjective to describe the type of files (each file represents a configuration composed of a set of options). N.B. rules_files is different because a rulesfiles describes a single file containing many rules (each file represents a set of rules).

That said, I can't recall why we called it configs vs. config. Did we have a compelling reason to call it so? 🤔

So we keep config (singular form)? Works for me. Could we re-audit if we are consistent everywhere?

Ok Leo is on board with changing configs_files to config_files. I will add a new commit for that.

incertum · 2024-05-20T08:57:28Z

falco.yaml

@@ -749,6 +762,8 @@ webserver:
  # Can be an IPV4 or IPV6 address, defaults to IPV4
  listen_address: 0.0.0.0
  k8s_healthz_endpoint: /healthz
+  # [Incubating] `prometheus_metrics_enabled`


Makes sense to me like this.

FedeDP · 2024-05-20T10:16:37Z

/hold
for discussion

leogr · 2024-05-20T11:27:35Z

falco.yaml

@@ -749,6 +762,8 @@ webserver:
  # Can be an IPV4 or IPV6 address, defaults to IPV4
  listen_address: 0.0.0.0
  k8s_healthz_endpoint: /healthz
+  # [Incubating] `prometheus_metrics_enabled`


This way works for me 👍

leogr · 2024-05-20T11:39:11Z

falco.yaml

@@ -1170,7 +1185,7 @@ base_syscalls:
 # Falco libs #
 ##############

-# [Experimental] `falco_libs` - Potentially subject to more frequent changes
+# [Incubating] `falco_libs` - Potentially subject to more frequent changes


Suggested change

# [Incubating] `falco_libs` - Potentially subject to more frequent changes

# [Incubating] `falco_libs`

IMO, it would be appropriate to remove (or at least clarify what we mean by):

Potentially subject to more frequent changes

For incubating features, long-term support is not guaranteed, and before 1.0, the deprecation period length is 0. This basically means, the feature can be changed or removed at any time. This is the same for all incubating features, so I don't see any compelling reason to make any particular treatment for this specific one.

Agreed, remove it.

leogr · 2024-05-20T11:49:43Z

falco.yaml

@@ -25,7 +25,7 @@
 #
 #     (Falco command-line arguments)
 #     (Falco environment variables)
-# Falco config files
+# Falco configs files


I guess config_files would be more appropriate since it uses the singular form config as an adjective to describe the type of files (each file represents a configuration composed of a set of options). N.B. rules_files is different because a rulesfiles describes a single file containing many rules (each file represents a set of rules).

That said, I can't recall why we called it configs vs. config. Did we have a compelling reason to call it so? 🤔

incertum · 2024-05-20T12:08:15Z

I was also wondering whether we could improve somehow the way maturity labels are attached, ie: perhaps moving them to the index at the top? Or moving them just the line before the configuration key, like:

# `configs_files`
#
# Falco will load additional configs files specified here.
# Their loading is assumed to be made *after* main config file has been processed,
# exactly in the order they are specified.
# Therefore, loaded config files *can* override values from main config file.
# Also, nested include is not allowed, ie: included config files won't be able to include other config files.
#
# Like for 'rules_files', specifying a folder will load all the configs files present in it in a lexicographical order.
#
# [Stable] 
configs_files:
  - /etc/falco/config.d

@leogr we haven't responded to this yet.

Adding also to the index at the top, big +1
Keeping # [Stable] configs_files was good would say, we can also add to the specific sub-config if needed. Don't have a strong opinion here. You can decide.

Signed-off-by: Federico Di Pierro <[email protected]>

…les`. Signed-off-by: Federico Di Pierro <[email protected]>

Signed-off-by: Federico Di Pierro <[email protected]>

FedeDP · 2024-05-20T12:33:59Z

Should've done everything:

configs_files -> config_files
experimental -> incubating
removed - Potentially subject to more frequent changes as @leogr suggested
Added maturity level to config index
merged under same section config_files and watch_config_files settings

Signed-off-by: Federico Di Pierro <[email protected]>

FedeDP · 2024-05-20T12:37:41Z

falco.yaml

-# Falco config files
-#     watch_config_files
+#     load_plugins [Stable]
+#     plugins [Stable]
 # Falco outputs settings


Note for eg: 1.0.0: we might add an outputs. namespace for these settings.
Similarly, for all the others, eg:

services.{grpc,webserver}

log.{stderr,syslog,level,libs}`
...

incertum

/approve

leogr

cc @LucaGuerra pls keep track of this, it may be useful in release comms.

poiana · 2024-05-20T13:50:48Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: FedeDP, incertum, 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:

~~OWNERS~~ [FedeDP,incertum,leogr]

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

FedeDP · 2024-05-20T14:32:31Z

/unhold

chore(docs): added falco.yaml section about config keys maturity.

7321c03

Also, rename `Experimental` -> `Incubating` and move `prometheus_metrics_enabled` to `Incubating`. Signed-off-by: Federico Di Pierro <[email protected]>

poiana added release-note dco-signoff: yes kind/cleanup labels May 20, 2024

poiana added this to the 0.38.0 milestone May 20, 2024

poiana requested review from incertum and Kaizhe May 20, 2024 08:50

poiana added approved size/S labels May 20, 2024

poiana requested review from leogr and LucaGuerra May 20, 2024 08:51

FedeDP commented May 20, 2024

View reviewed changes

incertum reviewed May 20, 2024

View reviewed changes

poiana added the do-not-merge/hold label May 20, 2024

leogr reviewed May 20, 2024

View reviewed changes

FedeDP added 2 commits May 20, 2024 14:28

chore(falco.yaml): added config maturity to index.

0e75e25

Signed-off-by: Federico Di Pierro <[email protected]>

chore(userspace,falco.yaml,unit_tests): configs_files -> `config_fi…

1206b42

…les`. Signed-off-by: Federico Di Pierro <[email protected]>

poiana added size/L and removed size/S labels May 20, 2024

cleanup(falco.yaml): removed useless sentence.

95b6f00

Signed-off-by: Federico Di Pierro <[email protected]>

chore(falco.yaml): rule -> rules.

6760ece

Signed-off-by: Federico Di Pierro <[email protected]>

FedeDP commented May 20, 2024

View reviewed changes

incertum approved these changes May 20, 2024

View reviewed changes

poiana assigned incertum May 20, 2024

poiana added the lgtm label May 20, 2024

leogr approved these changes May 20, 2024

View reviewed changes

poiana assigned leogr May 20, 2024

poiana removed the do-not-merge/hold label May 20, 2024

poiana merged commit 0bf7458 into master May 20, 2024
33 checks passed

poiana deleted the chore/maturity_config branch May 20, 2024 14:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(docs): added falco.yaml section about config keys maturity. #3206

chore(docs): added falco.yaml section about config keys maturity. #3206

FedeDP commented May 20, 2024

FedeDP commented May 20, 2024

FedeDP commented May 20, 2024

FedeDP May 20, 2024

incertum May 20, 2024

leogr May 20, 2024

FedeDP May 20, 2024

incertum May 20, 2024

FedeDP May 20, 2024

FedeDP commented May 20, 2024 •

edited

Loading

incertum May 20, 2024

FedeDP May 20, 2024 •

edited

Loading

leogr May 20, 2024 •

edited

Loading

incertum May 20, 2024

FedeDP May 20, 2024

incertum May 20, 2024

FedeDP commented May 20, 2024

leogr May 20, 2024

leogr May 20, 2024

incertum May 20, 2024

leogr May 20, 2024 •

edited

Loading

incertum commented May 20, 2024

FedeDP commented May 20, 2024

FedeDP May 20, 2024

incertum left a comment

leogr left a comment

poiana commented May 20, 2024

FedeDP commented May 20, 2024

	# [Incubating] `falco_libs` - Potentially subject to more frequent changes
	# [Incubating] `falco_libs`

chore(docs): added falco.yaml section about config keys maturity. #3206

chore(docs): added falco.yaml section about config keys maturity. #3206

Conversation

FedeDP commented May 20, 2024

FedeDP commented May 20, 2024

FedeDP commented May 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FedeDP commented May 20, 2024 • edited Loading

Choose a reason for hiding this comment

FedeDP May 20, 2024 • edited Loading

Choose a reason for hiding this comment

leogr May 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FedeDP commented May 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leogr May 20, 2024 • edited Loading

Choose a reason for hiding this comment

incertum commented May 20, 2024

FedeDP commented May 20, 2024

Choose a reason for hiding this comment

incertum left a comment

Choose a reason for hiding this comment

leogr left a comment

Choose a reason for hiding this comment

poiana commented May 20, 2024

FedeDP commented May 20, 2024

FedeDP commented May 20, 2024 •

edited

Loading

FedeDP May 20, 2024 •

edited

Loading

leogr May 20, 2024 •

edited

Loading

leogr May 20, 2024 •

edited

Loading