Helm chart: allow unprivileged deployment of Beyla #1128
Conversation
marevers
requested review from
grcevski,
mariomac,
marctc and
rafaelroquetto
as code owners
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1128 +/- ##
==========================================
+ Coverage 73.06% 82.01% +8.94%
==========================================
Files 139 140 +1
Lines 11553 11562 +9
==========================================
+ Hits 8441 9482 +1041
+ Misses 2454 1555 -899
+ Partials 658 525 -133
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
charts/beyla/values.yaml
Outdated
| @@ -59,13 +59,22 @@ podSecurityContext: {} | |||
| # fsGroup: 2000 | |||
|
|
|||
| securityContext: | |||
| # -- For an unprivileged / less privileged setup, use privileged: false and uncomment the required capabilities in the values file. | |||
There was a problem hiding this comment.
Could we instead of make users uncomment this add them conditionally?
There was a problem hiding this comment.
That is absolutely possible. In that case I would propose the following:
We don't use the condition based on securityContext.privileged anymore, instead we add a top-level attribute called privileged and set it by default true which results in the setup like before. If this is set to false, we set the securityContext how it needs to be.
In order to make adding the capabilities SYS_RESOURCE and SYS_ADMIN optional, because they are not always required, we can add a property extraCapabilities that allows for adding those two if needed and other ones.
The only 'downside' I see from this approach is that the control over securityContext is removed from the user once the new privileged attribute is set to false. But I would almost say that is okay, seeing as an unprivileged setup requires a very specific set of configurations to work.
There was a problem hiding this comment.
Sounds reasonoble to me @marevers ! thanks
There was a problem hiding this comment.
Ok, I will make the required changes and push and then re-request review.
* Make privileged optional, add required conditions for the unprivileged / less privileged setup * Update README, bump Beyla version to 1.8.0 and chart version to 1.4.0 * Fix conditions * Make securityContext setup conditional based on new privileged attribute * Fix comments failing validation
Closes #1075
This PR adds a new property
privilegedwhich can be set tofalseto add the required configurations to the DaemonSet (based on the unprivileged example).Chart version is increased to 1.4.0 and app version to 1.8.0.