protosanitizer.StripSecrets overhead too big

[tsmetana]

The protosanitizer.StripSecrets is un/marshalling every request to identify sensitive information and replace it. This operation seems to be too costly. Many CSI drivers (or other components) can print the secrets in logs when they're configured e.g. in the StorageClass. The impact of the issue is zero to little but still it might be good to have all the logging sanitized.

I have attempted to fix this in the GCP: kubernetes-sigs/gcp-compute-persistent-disk-csi-driver#747 however the fix was reverted precisely because of the performance impact of the StripSecrets function.

Would it be possible to try to for example identify or replace the secret without the expensive JSON operations? Or any other idea if you could come with some.

[msau42]

There's 2 approaches I can think of:

• Hardcode exactly which fields to strip. Downside is we need to remember to update this list every time there's a new CSI spec.
• Use a generic protobuf method instead of having to marshal/unmarshal to json. I would imagine a library to do this should already exist but I haven't had a chance to dig around. Maybe sig-security would know?

[jsafrane]

CC @pohly - do you have other smart idea?

[pohly]

I had considered to fork the print function that protobuf normally uses and then add skipping of secret fields to that forked code, but that seemed like a rather heavy-handed approach to the problem.

Perhaps the upstream code could be extended to do filtering with a custom filter function. That would be conceptually similar, but without the cost of having to maintain a fork.

[pohly]

Hardcode exactly which fields to strip. Downside is we need to remember to update this list every time there's a new CSI spec.

And then do what? Deep-copy the object, strip those known fields and then print the clone? That should work, but probably would still have higher overhead than a custom print function that directly iterates over the original object.

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Reopen this issue or PR with /reopen
• Mark this issue or PR as fresh with /remove-lifecycle rotten
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

[k8s-ci-robot]

@k8s-triage-robot: Closing this issue.

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Reopen this issue or PR with /reopen
• Mark this issue or PR as fresh with /remove-lifecycle rotten
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

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/test-infra repository.