new(userspace,unit_tests): validate configs against schema #3302

FedeDP · 2024-08-21T13:41:22Z

What type of PR is this?

/kind feature

Any specific area of the project related to this PR?

/area engine
/area tests

What this PR does / why we need it:

Moreover, configuration tests have been split in multiple source files:

unit_tests/falco/test_configuration_config_files.cpp
unit_tests/falco/test_configuration_env_vars.cpp
unit_tests/falco/test_configuration_schema.cpp

Which issue(s) this PR fixes:

Fixes #2924

Special notes for your reviewer:

Most of the lines changes come from the split of test_configuration.cpp and the schema (alone ~600locs).

Does this PR introduce a user-facing change?:

new(userspace,unit_tests): validate configs against schema

Supersedes #3004

FedeDP · 2024-08-21T13:41:47Z

falco.yaml

@@ -468,7 +468,7 @@ load_plugins: []
 plugins:
  - name: k8saudit
    library_path: libk8saudit.so
-    init_config:
+    init_config: ""


Otherwise falco configuration fails to be validated :D

FedeDP · 2024-08-21T13:42:15Z

unit_tests/falco/test_configuration_env_vars.cpp

Can be skipped, same content as before.

FedeDP · 2024-08-21T13:42:18Z

unit_tests/falco/test_configuration_config_files.cpp

Can be skipped, same content as before.

FedeDP · 2024-08-21T13:42:21Z

unit_tests/falco/test_configuration.cpp

Can be skipped, same content as before.

FedeDP · 2024-08-21T13:42:57Z

unit_tests/falco_test_var.h.in

@@ -2,3 +2,4 @@

 #define TEST_ENGINE_KMOD_CONFIG "${CMAKE_SOURCE_DIR}/unit_tests/falco/test_configs/engine_kmod_config.yaml"
 #define TEST_ENGINE_MODERN_CONFIG "${CMAKE_SOURCE_DIR}/unit_tests/falco/test_configs/engine_modern_config.yaml"
+#define TEST_FALCO_CONFIG "${CMAKE_SOURCE_DIR}/falco.yaml"


The new schema related tests have a test that tries to validate current falco config against the schema so that if a new config key is added but schema is not updated, the test will fail.

FedeDP · 2024-08-21T13:43:49Z

userspace/falco/app/actions/load_config.cpp

+			auto config_path = pair.first;
+			auto validation = pair.second;
+			auto priority = validation == yaml_helper::validation_ok ? falco_logger::level::INFO : falco_logger::level::WARNING;
+			falco_logger::log(priority, std::string("   ") + config_path + " | validation: " + validation + "\n");


Example output:

Wed Aug 21 15:43:33 2024: Falco initialized with configuration files: Wed Aug 21 15:43:33 2024: ../falco.yaml | validation: ok

Note that just a warning is emitted, no fatal error is thrown.

Example warning message (typo on base_syscalls key):

Wed Aug 21 15:50:16 2024: Falco initialized with configuration files: Wed Aug 21 15:50:16 2024: ../falco.yaml | validation: failed for <root>: Object contains a property that could not be validated using 'properties' or 'additionalProperties' constraints: 'base_sysalls'.

Why only a warning and no error on validation issue ?
If it's optional, should we also have a strict mode that will trigger an exception ?

Mmh i think we can revisit this choice later, and make it throw an error. For now, i didn't want to be too strict.
Mind also that if there is any real config error it will be caught by the Falco load config routine, ie: if eg: plugins key is not an array, that will trigger an error while loading the Falco config.

That's also why I chose to only emit a warning for the validation step: it is mostly useful only when a typo is present in the config.
Basically:

foo: bar

triggers just a validation warning since no problem will arise if that line is kept in Falco yaml, but this:

plugins: test

will trigger both a validation warning (of course) AND a Falco load config exception.

IMHO no need to trigger an error for a validation issue alone.

Fair enough

FedeDP · 2024-08-21T13:45:15Z

/milestone 0.39.0

FedeDP · 2024-08-21T13:49:23Z

Closing and reopening to trigger github actions(?)
/close

poiana · 2024-08-21T13:49:27Z

@FedeDP: Closed this PR.

In response to this:

Closing and reopening to trigger github actions(?)
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

FedeDP · 2024-08-21T13:49:29Z

/reopen

poiana · 2024-08-21T13:49:33Z

@FedeDP: Reopened this PR.

In response to this:

/reopen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

FedeDP · 2024-08-21T15:33:44Z

unit_tests/falco/test_configuration_schema.cpp

+	falco_configuration falco_config;
+	config_loaded_res res;
+
+	if (!std::filesystem::exists(TEST_FALCO_CONFIG))


This was failing on wasm CI because it could not find the Falco config; don't really know why:

Cannot read config file (/home/runner/work/falco/falco/falco.yaml): bad file: /home/runner/work/falco/falco/falco.yaml /home/runner/work/falco/falco/unit_tests/falco/test_configuration_schema.cpp:37: Failure Expected: res = falco_config.init_from_file("/home/runner/work/falco/falco/falco.yaml", {}) doesn't throw an exception. Actual: it throws std::exception with description "std::exception".

Anyway, given that the unit test executable can run in a different env from the one where it is built (ie: the repo clone might not be always available), just skip the test when the config file is not present.

FedeDP · 2024-08-21T15:34:12Z

~~Still needs to fix win32 CI.~~

FedeDP · 2024-08-22T08:43:49Z

cmake/modules/CompilerFlags.cmake

 	add_compile_definitions(
 		_HAS_STD_BYTE=0
 		_CRT_SECURE_NO_WARNINGS
 		WIN32
 		MINIMAL_BUILD
 		WIN32_LEAN_AND_MEAN
+		NOMINMAX


This fixes valijson usage on windows.

FedeDP · 2024-08-22T08:44:35Z

userspace/falco/configuration.cpp

+// https://learn.microsoft.com/en-us/cpp/cpp/string-and-character-literals-cpp?view=msvc-170#size-of-string-literals
+// Just use any available online tool, eg: https://jsonformatter.org/json-minify
+// to format the json, add the new fields, and then minify it again.
+static const std::string schema_json_string = R"({"$schema":"http://json-schema.org/draft-06/schema#","$ref":"#/definitions/FalcoConfig","definitions":{"FalcoConfig":{"type":"object","additionalProperties":false,"properties":{"config_files":{"type":"array","items":{"type":"string"}},"watch_config_files":{"type":"boolean"},"rules_files":{"type":"array","items":{"type":"string"}},"rule_files":{"type":"array","items":{"type":"string"}},"rules":{"type":"array","items":{"$ref":"#/definitions/Rule"}},"engine":{"$ref":"#/definitions/Engine"},"load_plugins":{"type":"array","items":{"type":"string"}},"plugins":{"type":"array","items":{"$ref":"#/definitions/Plugin"}},"time_format_iso_8601":{"type":"boolean"},"priority":{"type":"string"},"json_output":{"type":"boolean"},"json_include_output_property":{"type":"boolean"},"json_include_tags_property":{"type":"boolean"},"buffered_outputs":{"type":"boolean"},"rule_matching":{"type":"string"},"outputs_queue":{"$ref":"#/definitions/OutputsQueue"},"stdout_output":{"$ref":"#/definitions/Output"},"syslog_output":{"$ref":"#/definitions/Output"},"file_output":{"$ref":"#/definitions/FileOutput"},"http_output":{"$ref":"#/definitions/HTTPOutput"},"program_output":{"$ref":"#/definitions/ProgramOutput"},"grpc_output":{"$ref":"#/definitions/Output"},"grpc":{"$ref":"#/definitions/Grpc"},"webserver":{"$ref":"#/definitions/Webserver"},"log_stderr":{"type":"boolean"},"log_syslog":{"type":"boolean"},"log_level":{"type":"string"},"libs_logger":{"$ref":"#/definitions/LibsLogger"},"output_timeout":{"type":"integer"},"syscall_event_timeouts":{"$ref":"#/definitions/SyscallEventTimeouts"},"syscall_event_drops":{"$ref":"#/definitions/SyscallEventDrops"},"metrics":{"$ref":"#/definitions/Metrics"},"base_syscalls":{"$ref":"#/definitions/BaseSyscalls"},"falco_libs":{"$ref":"#/definitions/FalcoLibs"}},"title":"FalcoConfig"},"BaseSyscalls":{"type":"object","additionalProperties":false,"properties":{"custom_set":{"type":"array","items":{"type":"string"}},"repair":{"type":"boolean"}},"minProperties":1,"title":"BaseSyscalls"},"Engine":{"type":"object","additionalProperties":false,"properties":{"kind":{"type":"string"},"kmod":{"$ref":"#/definitions/Kmod"},"ebpf":{"$ref":"#/definitions/Ebpf"},"modern_ebpf":{"$ref":"#/definitions/ModernEbpf"},"replay":{"$ref":"#/definitions/Replay"},"gvisor":{"$ref":"#/definitions/Gvisor"}},"required":["kind"],"title":"Engine"},"Ebpf":{"type":"object","additionalProperties":false,"properties":{"probe":{"type":"string"},"buf_size_preset":{"type":"integer"},"drop_failed_exit":{"type":"boolean"}},"required":["probe"],"title":"Ebpf"},"Gvisor":{"type":"object","additionalProperties":false,"properties":{"config":{"type":"string"},"root":{"type":"string"}},"required":["config","root"],"title":"Gvisor"},"Kmod":{"type":"object","additionalProperties":false,"properties":{"buf_size_preset":{"type":"integer"},"drop_failed_exit":{"type":"boolean"}},"minProperties":1,"title":"Kmod"},"ModernEbpf":{"type":"object","additionalProperties":false,"properties":{"cpus_for_each_buffer":{"type":"integer"},"buf_size_preset":{"type":"integer"},"drop_failed_exit":{"type":"boolean"}},"title":"ModernEbpf"},"Replay":{"type":"object","additionalProperties":false,"properties":{"capture_file":{"type":"string"}},"required":["capture_file"],"title":"Replay"},"FalcoLibs":{"type":"object","additionalProperties":false,"properties":{"thread_table_size":{"type":"integer"}},"minProperties":1,"title":"FalcoLibs"},"FileOutput":{"type":"object","additionalProperties":false,"properties":{"enabled":{"type":"boolean"},"keep_alive":{"type":"boolean"},"filename":{"type":"string"}},"minProperties":1,"title":"FileOutput"},"Grpc":{"type":"object","additionalProperties":false,"properties":{"enabled":{"type":"boolean"},"bind_address":{"type":"string"},"threadiness":{"type":"integer"}},"minProperties":1,"title":"Grpc"},"Output":{"type":"object","additionalProperties":false,"properties":{"enabled":{"type":"boolean"}},"minProperties":1,"title":"Output"},"HTTPOutput":{"type":"object","additionalProperties":false,"properties":{"enabled":{"type":"boolean"},"url":{"type":"string","format":"uri","qt-uri-protocols":["http"]},"user_agent":{"type":"string"},"insecure":{"type":"boolean"},"ca_cert":{"type":"string"},"ca_bundle":{"type":"string"},"ca_path":{"type":"string"},"mtls":{"type":"boolean"},"client_cert":{"type":"string"},"client_key":{"type":"string"},"echo":{"type":"boolean"},"compress_uploads":{"type":"boolean"},"keep_alive":{"type":"boolean"}},"minProperties":1,"title":"HTTPOutput"},"LibsLogger":{"type":"object","additionalProperties":false,"properties":{"enabled":{"type":"boolean"},"severity":{"type":"string"}},"minProperties":1,"title":"LibsLogger"},"Metrics":{"type":"object","additionalProperties":false,"properties":{"enabled":{"type":"boolean"},"interval":{"type":"string"},"output_rule":{"type":"boolean"},"output_file":{"type":"string"},"rules_counters_enabled":{"type":"boolean"},"resource_utilization_enabled":{"type":"boolean"},"state_counters_enabled":{"type":"boolean"},"kernel_event_counters_enabled":{"type":"boolean"},"libbpf_stats_enabled":{"type":"boolean"},"plugins_metrics_enabled":{"type":"boolean"},"convert_memory_to_mb":{"type":"boolean"},"include_empty_values":{"type":"boolean"}},"minProperties":1,"title":"Metrics"},"OutputsQueue":{"type":"object","additionalProperties":false,"properties":{"capacity":{"type":"integer"}},"minProperties":1,"title":"OutputsQueue"},"Plugin":{"type":"object","additionalProperties":false,"properties":{"name":{"type":"string"},"library_path":{"type":"string"},"init_config":{"type":"string"},"open_params":{"type":"string"}},"required":["library_path","name"],"title":"Plugin"},"ProgramOutput":{"type":"object","additionalProperties":false,"properties":{"enabled":{"type":"boolean"},"keep_alive":{"type":"boolean"},"program":{"type":"string"}},"required":["program"],"title":"ProgramOutput"},"Rule":{"type":"object","additionalProperties":false,"properties":{"disable":{"$ref":"#/definitions/Able"},"enable":{"$ref":"#/definitions/Able"}},"minProperties":1,"title":"Rule"},"Able":{"type":"object","additionalProperties":false,"properties":{"rule":{"type":"string"},"tag":{"type":"string"}},"minProperties":1,"title":"Able"},"SyscallEventDrops":{"type":"object","additionalProperties":false,"properties":{"threshold":{"type":"number"},"actions":{"type":"array","items":{"type":"string"}},"rate":{"type":"number"},"max_burst":{"type":"integer"},"simulate_drops":{"type":"boolean"}},"minProperties":1,"title":"SyscallEventDrops"},"SyscallEventTimeouts":{"type":"object","additionalProperties":false,"properties":{"max_consecutives":{"type":"integer"}},"minProperties":1,"title":"SyscallEventTimeouts"},"Webserver":{"type":"object","additionalProperties":false,"properties":{"enabled":{"type":"boolean"},"threadiness":{"type":"integer"},"listen_port":{"type":"integer"},"listen_address":{"type":"string"},"k8s_healthz_endpoint":{"type":"string"},"prometheus_metrics_enabled":{"type":"boolean"},"ssl_enabled":{"type":"boolean"},"ssl_certificate":{"type":"string"}},"minProperties":1,"title":"Webserver"}}})";


Minified to avoid MSVC compiler to throw
"error C2026: string too big, trailing characters truncate"

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

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

…eir own source file. Signed-off-by: Federico Di Pierro <[email protected]>

…c to be more testable. Signed-off-by: Federico Di Pierro <[email protected]>

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

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

…lco config is not present. Signed-off-by: Federico Di Pierro <[email protected]>

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

…le definition. Also, minified config schema, since the big schema string leads to an MSVC compiler error. Signed-off-by: Federico Di Pierro <[email protected]>

FedeDP · 2024-08-26T13:53:44Z

Rebased on top of latest master.

incertum

/approve

poiana · 2024-08-26T16:05:54Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: FedeDP, incertum

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]

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

poiana added release-note kind/feature dco-signoff: yes area/engine area/tests approved labels Aug 21, 2024

poiana requested review from incertum and Kaizhe August 21, 2024 13:41

poiana added the size/XXL label Aug 21, 2024