Feature: Add mTLS certificate loading for Falco via Helm #565
Conversation
poiana
added
dco-signoff: yes
kind/feature
jgmartinez
force-pushed
the
falcomtlscertload2
branch
from
6f12e0b to
5dac514
Compare
|
@jgmartinez can you please rebase your PR? thanks |
Signed-off-by: Juan Gonzalez <[email protected]>
Signed-off-by: Juan Gonzalez <[email protected]>
jgmartinez
force-pushed
the
falcomtlscertload2
branch
from
922d277 to
5bbab8b
Compare
|
Done @cpanato |
|
@cpanato can you consider looking at this PR? |
|
@CasperGN i will try to take a look today, but it is kubecon |
|
/approve /assign @leogr to check the ssl change path part |
| ca_path: "/etc/falco/certs/" | ||
| # -- Tell Falco to use mTLS | ||
| mtls: false | ||
| # -- Path to the client cert. | ||
| client_cert: "/etc/ssl/certs/client.crt" | ||
| client_cert: "/etc/falco/certs/client/client.crt" | ||
| # -- Path to the client key. | ||
| client_key: "/etc/ssl/certs/client.key" | ||
| client_key: "/etc/falco/certs/client/client.key" | ||
| # -- Whether to echo server answers to stdout |
There was a problem hiding this comment.
Although this is legit, we will risk getting these values overwritten when maintainers update the config (N.B.: these defaults are usually copied/pasted from falco.yaml).
So I'd prefer to keep the /etc/ssl/certs folder unless there's any compelling reason not to do so 🤔
There was a problem hiding this comment.
Reason I changed the path is because if we use /etc/ssl/certs we are overwriting the base image CAs and other certificates, risking getting certificate errors if the container need to trust other servers or other certificate related issues.
This being said, we could mount a folder inside like /etc/ssl/certs/ falco so there's some consistency :)
There was a problem hiding this comment.
Good point. I believe /etc/falco/certs works. Just waiting for @alacuku for a second opinion.
Thank you!
There was a problem hiding this comment.
Hi @jgmartinez, thank you for the contribution.
Left a suggestion.
Signed-off-by: Juan Gonzalez <[email protected]>
jgmartinez
force-pushed
the
falcomtlscertload2
branch
from
4348581 to
62d565d
Compare
|
LGTM label has been added. Git tree hash: 459ade81717e74dbff21402dde24cda52bc38135
|
|
@leogr could you please have a look? |
any updates on this? :) |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alacuku, cpanato, jgmartinez, 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 |
What type of PR is this?
/kind feature
/kind chart-release
Any specific area of the project related to this PR?
/area falco-chart
What this PR does / why we need it:
This PR adds the capability of loading certificates for mTLS communication dynamically via Helm values. It's structured in a way to make it easier to deploy mTLS cryptographic material for Falco http_output when mTLS is enabled.
Which issue(s) this PR fixes:
N/A
Special notes for your reviewer:
Substituting #549
Checklist