kapp-controller logs the proxy credentials

[ssamrit-vmw]

What steps did you take:
Configured the kapp-controller-config with proxy URLs in the http://username:password@ip:port format.

What happened:
The kapp-controller logs had the URLs with credentials in plain text.

What did you expect:
We expect the proxy credentials to be masked when logging proxy URLs.

cc: @cppforlife

Environment:

• kapp Controller version (execute kubectl get deployment -n kapp-controller kapp-controller -o yaml and the annotation is kbld.k14s.io/images): https://github.com/vmware-tanzu/carvel-kapp-controller/releases/tag/v0.23.0
• Kubernetes version (use kubectl version): v1.21.1

---

Vote on this request

This is an invitation to the community to vote on issues, to help us prioritize our backlog. Use the "smiley face" up to the right of this comment to vote.

👍 "I would like to see this addressed as soon as possible"
👎 "There are other more important things to focus on right now"

We are also happy to receive and review Pull Requests if you want to help working on this issue.

[danielhelfand]

Thanks for opening this @ssamrit-vmw.

It looks like this is taking place here. It was added in #65 probably as a way to debug the feature and also publish the information to end users, but it doesn't look like it considered this kind of format at the time.

I would support not logging the proxies at all or simply mentioning that proxies are configured.

[danielhelfand]

One thing to consider with this issue is that the original decision to log the proxies was done to support testing kapp-controller: https://github.com/vmware-tanzu/carvel-kapp-controller/blob/85e814cda7109169809ede1c8a4f211739ad15d2/test/e2e/kappcontroller/proxy_test.go#L14

One interesting thing to consider with this story is whether there is a better testing approach than examining the logs to determine if proxies are being configured correctly. If this is the case, we could remove the logging all together.

Alternatively, we should try and see if there is a masking approach that makes sense so we can preserve the logs currently displayed.

[neil-hickey]

It feels like there are two distinct concerns here:

1. We should not be logging any sensitive information.
2. I want to debug my kapp-controller and verify that the secret data for proxy matches the controller implementation.

The tests can be satisfied by just logging "HTTP(s) proxy is enabled." instead of the ip etc, which solves this issue. However, an operator may want to debug the proxy ip and the logs seem to be a sensible place to view that. WDTY @danielhelfand ?

I intend to remove the current logging with something like "<PROXY_TYPE> is enabled", and verify with the community if they would like for us to log any proxy info.

[neil-hickey]

@ssamrit-vmw could you maybe help me understand if logging the proxy information is a requirement / expectation for you all?

[danielhelfand]

The tests can be satisfied by just logging "HTTP(s) proxy is enabled." instead of the ip etc, which solves this issue. However, an operator may want to debug the proxy ip and the logs seem to be a sensible place to view that. WDTY @danielhelfand ?

I would be fine with this approach of removing the logging all together like you are suggesting. Seems reasonable as you are saying to check with community users as well to verify.

[ssamrit-vmw]

@ssamrit-vmw could you maybe help me understand if logging the proxy information is a requirement / expectation for you all?

For our usecase in TMC, we are not using proxy details from kapp-controller logs for testing or anything else, so it is fine for us even if it is not logged.