Handle CheckCert Correctly for OTLP Outputs#1287
Handle CheckCert Correctly for OTLP Outputs#1287rrhubenov wants to merge 1 commit intofalcosecurity:masterfrom
CheckCert Correctly for OTLP Outputs#1287Conversation
Signed-off-by: Radoslav Hubenov <rrhubenov@gmail.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: rrhubenov The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Welcome @rrhubenov! It looks like this is your first PR to falcosecurity/falcosidekick 🎉 |
|
Issues go stale after 90d of inactivity. Mark the issue as fresh with Stale issues rot after an additional 30d of inactivity and eventually close. If this issue is safe to close now please do so with Provide feedback via https://github.com/falcosecurity/community. /lifecycle stale |
|
/remove-lifecycle stale |
|
Closing and reopening to trigger the CI |
|
@leogr: Closed this PR. DetailsIn response to this:
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. |
|
/reopen |
|
@leogr: Reopened this PR. DetailsIn response to this:
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. |
leogr
left a comment
There was a problem hiding this comment.
Hey @rrhubenov
Thank you for this PR. It SGTM, but I think that users who had CheckCert: false to talk to a non-TLS collector will break, since the client now attempts a TLS handshake.
We need either a new Insecure config field or just documenting the potential breaking change. Wdyt?
What type of PR is this?
/kind bug
/area outputs
What this PR does / why we need it:
The
OTLPoutputs, when configured withCheckCert: falsewould use anInsecureoption, which results in TLS plaintext communication rather than the expected, which isTLSbut without checking the validity of the server certificate.This PR fixes this issue by passing the correct option to achieve the desired correct effect.
This PR also fixes a bug where the
Traces.CheckCertfield was checked for theLogsoutput rather thanLogs.CheckCert.Which issue(s) this PR fixes:
NONE
Special notes for your reviewer: