Add config to disable HTTP proxy logging #12665
Conversation
Signed-off-by: Alex Leong <[email protected]>
Signed-off-by: Alex Leong <[email protected]>
Signed-off-by: Alex Leong <[email protected]>
Signed-off-by: Alex Leong <[email protected]>
Signed-off-by: Alex Leong <[email protected]>
| @@ -29,7 +29,7 @@ env: | |||
| value: {{.Values.proxy.requireTLSOnInboundPorts | quote}} | |||
| {{ end -}} | |||
| - name: LINKERD2_PROXY_LOG | |||
| value: {{.Values.proxy.logLevel | quote}} | |||
| value: "{{.Values.proxy.logLevel}}{{ if not (eq .Values.proxy.logHTTPHeaders "insecure") }},linkerd_proxy_http::client[{headers}]=off{{ end }}" | |||
There was a problem hiding this comment.
I think it still is a good practice to leave the quote function in.
There was a problem hiding this comment.
Can you explain? it was my impression that the quote function just adds the quotation marks. Using quote here is difficult because its a pipeline function and we're constructing the string in the template.
| # -- (`off` or `insecure`) If set to `off`, will prevent the proxy from | ||
| # logging HTTP headers. If set to `insecure`, HTTP headers may be logged | ||
| # verbatim. Note that setting this to `insecure` is not alone sufficient to | ||
| # log HTTP headers; the proxy logLevel must also be set to debug. | ||
| logHTTPHeaders: "off" |
There was a problem hiding this comment.
I prefer having a regular boolean here like everywhere else instead of introducing special values. E.g. if people use "true" that would get misinterpreted in the template as "off" (according to the change in _proxy.tpl). To avoid that you'd have to validate the possible values, which we don't do for regular booleans as those are "standard" (true, >0, non-empty vs false, 0, empty).
There was a problem hiding this comment.
The motivation for not using a boolean here is that using a scary sounding value like "insecure" helps users realize that enabling this has security implications. It also leaves the door open to adding other modes in the future like "hash", "redact", or "obfuscate".
| if override, ok := annotations[k8s.ProxyLogHTTPHeaders]; ok { | ||
| values.Proxy.LogHTTPHeaders = override | ||
| } | ||
|
|
There was a problem hiding this comment.
Just a thought, should we do any logging here in case the value is unknown? For example, a label value of insecur will still disable http headers in logs, but perhaps the intention was the opposite. Should we report invalid values to the user somehow?
There was a problem hiding this comment.
good call. updated.
bin/linkerd upgrade --set proxy.logHTTPHeaders=insecur
× Could not render upgrade configuration: failed to render the template: execution error at (linkerd-control-plane/templates/proxy-injector.yaml:77:12): logHTTPHeaders must be one of: insecure | off
For troubleshooting help, visit: https://linkerd.io/upgrade/#troubleshooting
Signed-off-by: Alex Leong <[email protected]>
Signed-off-by: Alex Leong <[email protected]>
Fixes #12620
When the Linkerd proxy log level is set to
debugor higher, the proxy logs HTTP headers which may contain sensitive information.While we want to avoid logging sensitive data by default, logging of HTTP headers can be a helpful debugging tool. Therefore, we add a
proxy.logHTTPHeadersHelm value which prevents the logging of HTTP headers when set to false. The default value of this value is false so that headers cannot be logged unless users opt-in.