Robot Upgrade: falco chart upgrade from 2.0.17 to 7.0.2 #3762
Conversation
Summary of ChangesHello @weizhoublue, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request upgrades the falco chart to version 7.0.2, incorporating significant enhancements such as updated dependencies, the introduction of the falco-talon subchart, improved documentation, and integration with k8s-metacollector. The changes aim to provide a more robust and feature-rich Falco deployment experience. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This PR is a major upgrade of the Falco chart from version 2.0.17 to 7.0.2, introducing significant changes, new sub-charts like falco-talon and k8s-metacollector, and updating dependencies. The changes align with modernizing Falco's deployment, such as using falcoctl for artifact management and removing deprecated features. My review focuses on identifying potential issues arising from this large-scale automated upgrade, including configuration bugs, documentation clarity, and template logic. I've found a few critical configuration issues, typos in documentation, and some minor template improvements.
| create_index_template: {{ .Values.config.notifiers.loki.createIndexTemplate }} | ||
| number_of_shards: {{ .Values.config.notifiers.loki.numberOfShards }} | ||
| number_of_replicas: {{ .Values.config.notifiers.loki.numberOfReplicas }} |
There was a problem hiding this comment.
There seems to be a copy-paste error. The configuration for elasticsearch is using values from loki (.Values.config.notifiers.loki.*). It should be using values from .Values.config.notifiers.elasticsearch.*.
create_index_template: {{ .Values.config.notifiers.elasticsearch.createIndexTemplate }}
number_of_shards: {{ .Values.config.notifiers.elasticsearch.numberOfShards }}
number_of_replicas: {{ .Values.config.notifiers.elasticsearch.numberOfReplicas }}| # -- region (if not specified, default region from provider credential chain will be used) | ||
| region: "" | ||
| # -- access key (if not specified, default access_key from provider credential chain will be used) | ||
| accesKey: "" |
There was a problem hiding this comment.
There is a typo in the configuration key accesKey. It should be accessKey. This will cause the value to be ignored by the template, which expects accessKey.
accessKey: ""| The new chart deploys new *k8s* resources and new configuration variables have been added to the `values.yaml` file. People upgrading the chart from `v2.x.y` have to port their configuration variables to the new `values.yaml` file used by the `v3.0.0` chart. | ||
|
|
||
| If you still want to use the old values, because you do not want to take advantage of the new and shiny **falcoctl** tool then just run: | ||
| ```bash= |
There was a problem hiding this comment.
The code block specifier bash= is not standard Markdown and may not render correctly on all platforms (like GitHub). It's better to use just bash. This applies to other code blocks in this file as well (lines 104, 114, 126, 136, 216, 224).
| ```bash= | |
| ```bash |
|
|
||
| ## v4.21.1 | ||
|
|
||
| * removed falco-expoter (now deprecated) references from the readme |
There was a problem hiding this comment.
There is a typo in falco-expoter. It should be falco-exporter.
| * removed falco-expoter (now deprecated) references from the readme | |
| * removed falco-exporter (now deprecated) references from the readme |
|
|
||
| ## v4.5.2 | ||
|
|
||
| * reording scc configuration, making it more robust to plain yaml comparison |
There was a problem hiding this comment.
There is a typo in reording. It should be reordering.
| * reording scc configuration, making it more robust to plain yaml comparison | |
| * reordering scc configuration, making it more robust to plain yaml comparison |
|
|
||
| **Legacy eBPF probe** | ||
|
|
||
| To run Falco with the [eBPF probe](http://falco.org/docs/concepts/event-sources/kernel/#legacy-ebpf-probe) you just need to set `driver.kind=ebpf` as shown in the following snippet: |
There was a problem hiding this comment.
The description for the "Legacy eBPF probe" section is confusing. The text refers to "eBPF probe" but it should be "legacy eBPF probe". Also, the link uses http instead of https.
To run Falco with the [legacy eBPF probe](https://falco.org/docs/concepts/event-sources/kernel/#legacy-ebpf-probe) you just need to set `driver.kind=ebpf` as shown in the following snippet:
| * k8s.pod.namespace.name; | ||
|
|
||
| The [k8smeta](https://github.com/falcosecurity/plugins/tree/master/plugins/k8smeta) plugin exports a whole new | ||
| [field class]https://github.com/falcosecurity/plugins/tree/master/plugins/k8smeta#supported-fields. Note that the new |
There was a problem hiding this comment.
The markdown link for "field class" is broken. It's missing parentheses to properly format it as a link.
[field class](https://github.com/falcosecurity/plugins/tree/master/plugins/k8smeta#supported-fields). Note that the new
| * configure the plugins to be loaded, in this case, the `k8saudit` and `json`; | ||
| * and finally we add our plugins in the `load_plugins` to be loaded by Falco. | ||
|
|
||
| The configuration can be found in the [values-k8saudit.yaml(./values-k8saudit.yaml] file ready to be used: |
There was a problem hiding this comment.
The markdown link for values-k8saudit.yaml is broken. It's missing the correct parenthesis.
The configuration can be found in the [values-k8saudit.yaml](./values-k8saudit.yaml) file ready to be used:
|
|
||
| ``` | ||
| helm delete falco-talon -n falco | ||
| ```` |
There was a problem hiding this comment.
The code block for uninstalling Falco Talon uses four backticks instead of three, which will cause rendering issues.
| ```` |
| name: certs-volume | ||
| readOnly: true | ||
| {{- end }} | ||
| {{- if or .Values.extraVolumeMounts }} |
There was a problem hiding this comment.
The or in this if condition is redundant. if .Values.extraVolumeMounts would be sufficient and cleaner.
{{- if .Values.extraVolumeMounts }}
github-actions
bot
force-pushed
the
upgrade/falco/7.0.2
branch
3 times, most recently
from
150d5a8 to
92ef169
Compare
Signed-off-by: robot <[email protected]>
github-actions
bot
force-pushed
the
upgrade/falco/7.0.2
branch
from
92ef169 to
e3f7f50
Compare
I am robot, upgrade: project falco chart upgrade from 2.0.17 to 7.0.2