Update redacting functionality to redact all sensitive info in config when printing with view #109189

mpuckett159 · 2022-03-31T07:56:56Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

Updates redacting functionality of kubectl config view to properly redact all config keys with a datapolicy tag. This is accomplished by introducing a new function RedactSecrets in client-go and then consuming it in kubectl.

Example usages

No flags:

❯ ./_output/local/bin/darwin/amd64/kubectl config view
apiVersion: v1
clusters: null
contexts: null
current-context: ""
kind: Config
preferences: {}
users:
- name: foo
  user:
    auth-provider:
      config:
        refresh-token: test1233
      name: ""
    client-certificate-data: DATA+OMITTED
    client-key-data: DATA+OMITTED
    exec:
      args:
      - test
      command: foo1
      env:
      - name: test
        value: vla2
      provideClusterInfo: false
    token: REDACTED
    password: REDACTED
    username: mpuckett

With --raw:

❯ ./_output/local/bin/darwin/amd64/kubectl config view --raw         
apiVersion: v1
clusters: null
contexts: null
current-context: ""
kind: Config
preferences: {}
users:
- name: foo
  user:
    auth-provider:
      config:
        refresh-token: test1233
      name: ""
    client-certificate-data: YWZmYWRzZmFzZGZz
    client-key-data: YWZmYWRzZmFzZGZz
    exec:
      args:
      - test
      command: foo1
      env:
      - name: test
        value: vla2
      provideClusterInfo: false
    token: YWZmYWRzZmFzZGZz
    password: fake-password
    username: mpuckett

Which issue(s) this PR fixes:

Fixes kubernetes/kubectl#1193

Does this PR introduce a user-facing change?

kubectl config view now automatically redacts any secret fields marked with a datapolicy tag

k8s-ci-robot · 2022-03-31T07:57:05Z

Hi @mpuckett159. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

mpuckett159 · 2022-03-31T15:49:34Z

You'll notice in the above examples that users.foo.auth-provider.config.refresh-token is not redacted. I'm not exactly sure how we should god about doing that if it is desired to redact that, because this value is set by a simple map[string]string which can't have the datapolicy that I'm using to determine whether to redact or not. We could search for map keys that contain token or password maybe, even secret but because this is kind of a free form field it might just be better to manually remove the output of the entire object.

fedebongio · 2022-03-31T16:40:13Z

/remove-sig api-machinery
/sig auth

ardaguclu

Thanks for the PR @mpuckett159 !

staging/src/k8s.io/kubectl/pkg/cmd/config/view.go

staging/src/k8s.io/client-go/tools/clientcmd/api/helpers.go

ardaguclu · 2022-06-07T12:41:09Z

/ok-to-test

leilajal · 2022-06-09T16:33:26Z

/remove-sig api-machinery
/triage accepted

mpuckett159 · 2022-07-21T03:40:48Z

/retest-required

k8s-triage-robot · 2022-10-19T05:35:59Z

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

mpuckett159 · 2022-10-19T05:42:45Z

/remove-lifecycle stale

mpuckett159 · 2022-10-24T18:58:24Z

Sorry for the churn there, not exactly sure what I was thinking there 🙄

soltysh · 2022-10-24T19:36:45Z

/test unit

k8s-ci-robot · 2022-10-24T19:36:47Z

@soltysh: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test pull-kubernetes-conformance-kind-ga-only-parallel
/test pull-kubernetes-coverage-unit
/test pull-kubernetes-dependencies
/test pull-kubernetes-dependencies-go-canary
/test pull-kubernetes-e2e-gce
/test pull-kubernetes-e2e-gce-100-performance
/test pull-kubernetes-e2e-gce-big-performance
/test pull-kubernetes-e2e-gce-canary
/test pull-kubernetes-e2e-gce-network-proxy-http-connect
/test pull-kubernetes-e2e-gce-no-stage
/test pull-kubernetes-e2e-gce-scale-performance-manual
/test pull-kubernetes-e2e-gce-ubuntu-containerd
/test pull-kubernetes-e2e-gce-ubuntu-containerd-canary
/test pull-kubernetes-e2e-kind
/test pull-kubernetes-e2e-kind-ipv6
/test pull-kubernetes-integration
/test pull-kubernetes-integration-go-canary
/test pull-kubernetes-kubemark-e2e-gce-scale
/test pull-kubernetes-node-e2e-containerd
/test pull-kubernetes-typecheck
/test pull-kubernetes-unit
/test pull-kubernetes-unit-go-canary
/test pull-kubernetes-update
/test pull-kubernetes-verify
/test pull-kubernetes-verify-go-canary
/test pull-kubernetes-verify-govet-levee

The following commands are available to trigger optional jobs:

/test check-dependency-stats
/test pull-ci-kubernetes-unit-windows
/test pull-e2e-gce-cloud-provider-disabled
/test pull-kubernetes-conformance-image-test
/test pull-kubernetes-conformance-kind-ga-only
/test pull-kubernetes-conformance-kind-ipv6-parallel
/test pull-kubernetes-cross
/test pull-kubernetes-e2e-capz-azure-disk
/test pull-kubernetes-e2e-capz-azure-disk-vmss
/test pull-kubernetes-e2e-capz-azure-file
/test pull-kubernetes-e2e-capz-azure-file-vmss
/test pull-kubernetes-e2e-capz-conformance
/test pull-kubernetes-e2e-capz-ha-control-plane
/test pull-kubernetes-e2e-capz-windows-containerd
/test pull-kubernetes-e2e-containerd-gce
/test pull-kubernetes-e2e-gce-alpha-features
/test pull-kubernetes-e2e-gce-correctness
/test pull-kubernetes-e2e-gce-csi-serial
/test pull-kubernetes-e2e-gce-device-plugin-gpu
/test pull-kubernetes-e2e-gce-kubetest2
/test pull-kubernetes-e2e-gce-network-proxy-grpc
/test pull-kubernetes-e2e-gce-storage-disruptive
/test pull-kubernetes-e2e-gce-storage-slow
/test pull-kubernetes-e2e-gce-storage-snapshot
/test pull-kubernetes-e2e-gce-ubuntu-containerd-serial
/test pull-kubernetes-e2e-gci-gce-autoscaling
/test pull-kubernetes-e2e-gci-gce-ingress
/test pull-kubernetes-e2e-gci-gce-ipvs
/test pull-kubernetes-e2e-kind-canary
/test pull-kubernetes-e2e-kind-dual-canary
/test pull-kubernetes-e2e-kind-ipv6-canary
/test pull-kubernetes-e2e-kind-ipvs-dual-canary
/test pull-kubernetes-e2e-kind-multizone
/test pull-kubernetes-e2e-kops-aws
/test pull-kubernetes-e2e-kubelet-credential-provider
/test pull-kubernetes-e2e-ubuntu-gce-network-policies
/test pull-kubernetes-kind-dra
/test pull-kubernetes-kubemark-e2e-gce-big
/test pull-kubernetes-local-e2e
/test pull-kubernetes-node-crio-cgrpv2-e2e
/test pull-kubernetes-node-crio-cgrpv2-e2e-kubetest2
/test pull-kubernetes-node-crio-e2e
/test pull-kubernetes-node-crio-e2e-kubetest2
/test pull-kubernetes-node-e2e-containerd-alpha-features
/test pull-kubernetes-node-e2e-containerd-features
/test pull-kubernetes-node-e2e-containerd-features-kubetest2
/test pull-kubernetes-node-e2e-containerd-kubetest2
/test pull-kubernetes-node-kubelet-credential-provider
/test pull-kubernetes-node-kubelet-serial-containerd
/test pull-kubernetes-node-kubelet-serial-containerd-kubetest2
/test pull-kubernetes-node-kubelet-serial-cpu-manager
/test pull-kubernetes-node-kubelet-serial-cpu-manager-kubetest2
/test pull-kubernetes-node-kubelet-serial-crio-cgroupv1
/test pull-kubernetes-node-kubelet-serial-crio-cgroupv2
/test pull-kubernetes-node-kubelet-serial-hugepages
/test pull-kubernetes-node-kubelet-serial-memory-manager
/test pull-kubernetes-node-kubelet-serial-topology-manager
/test pull-kubernetes-node-kubelet-serial-topology-manager-kubetest2
/test pull-kubernetes-node-memoryqos-cgrpv2
/test pull-kubernetes-node-swap-fedora
/test pull-kubernetes-node-swap-fedora-serial
/test pull-kubernetes-node-swap-ubuntu-serial
/test pull-kubernetes-unit-experimental
/test pull-publishing-bot-validate

Use /test all to run the following jobs that were automatically triggered:

pull-kubernetes-conformance-kind-ga-only-parallel
pull-kubernetes-dependencies
pull-kubernetes-e2e-gce-100-performance
pull-kubernetes-e2e-gce-ubuntu-containerd
pull-kubernetes-e2e-kind
pull-kubernetes-e2e-kind-ipv6
pull-kubernetes-integration
pull-kubernetes-node-e2e-containerd
pull-kubernetes-typecheck
pull-kubernetes-unit
pull-kubernetes-verify
pull-kubernetes-verify-govet-levee

In response to this:

/test unit

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.

soltysh · 2022-10-24T19:37:16Z

/test pull-kubernetes-unit

soltysh

/lgtm
/approve
/label tide/merge-method-squash

sttts · 2022-10-25T17:12:07Z

staging/src/k8s.io/client-go/tools/clientcmd/api/helpers_test.go

-	//     client-certificate-data: REDACTED
-	//     client-key-data: REDACTED
+	//     client-certificate-data: DATA+OMITTED
+	//     client-key-data: DATA+OMITTED


did this really change?

This test is for the ShortenConfig() and MinifyConfig() functions. ShortenConfig used to perform the action of redacting the secrets, but no longer handles that process and instead only omits byte data.

sttts · 2022-11-08T12:21:01Z

/approve

k8s-ci-robot · 2022-11-08T12:21:27Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mpuckett159, soltysh, sttts

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:

~~staging/src/k8s.io/client-go/OWNERS~~ [sttts]
~~staging/src/k8s.io/kubectl/OWNERS~~ [soltysh]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

… when printing with view (kubernetes#109189) * Add RedactSecrets function * Move RedactSecrets method to existing RawBytesData case * Update TestRedactSecrets to use new pattern of os.CreateTemp()

k8s-ci-robot added the needs-priority Indicates a PR lacks a `priority/foo` label and requires one. label Mar 31, 2022

k8s-ci-robot requested review from deads2k and soltysh March 31, 2022 07:57

k8s-ci-robot added area/kubectl sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/cli Categorizes an issue or PR as relevant to SIG CLI. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Mar 31, 2022

mpuckett159 changed the title ~~Update redacting functionality to redact all sensitive info in config~~ Update redacting functionality to redact all sensitive info in config when printing with view Mar 31, 2022

k8s-ci-robot added sig/auth Categorizes an issue or PR as relevant to SIG Auth. and removed sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. labels Mar 31, 2022

enj added this to Needs Triage in SIG Auth Old Apr 9, 2022

ardaguclu reviewed Jun 7, 2022

View reviewed changes

k8s-ci-robot added the triage/accepted Indicates an issue or PR is ready to be actively worked on. label Jun 9, 2022

mpuckett159 force-pushed the feature/redact-config branch 2 times, most recently from ad84892 to 8c54589 Compare July 21, 2022 01:06

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 19, 2022

k8s-ci-robot added do-not-merge/contains-merge-commits Indicates a PR which contains merge commits. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Oct 19, 2022

mpuckett159 force-pushed the feature/redact-config branch from 752612d to 8d84ad6 Compare October 24, 2022 18:56

k8s-ci-robot removed the do-not-merge/contains-merge-commits Indicates a PR which contains merge commits. label Oct 24, 2022

mpuckett159 and others added 3 commits October 24, 2022 12:55

Add RedactSecrets function

0cf8da5

Move RedactSecrets method to existing RawBytesData case

28d7326

Update TestRedactSecrets to use new pattern of os.CreateTemp()

5a578aa

mpuckett159 force-pushed the feature/redact-config branch from 8d84ad6 to 5a578aa Compare October 24, 2022 19:58

soltysh approved these changes Oct 24, 2022

View reviewed changes

k8s-ci-robot added tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Oct 24, 2022

sttts reviewed Oct 25, 2022

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 8, 2022

k8s-ci-robot merged commit e721272 into kubernetes:master Nov 8, 2022

SIG Auth Old automation moved this from In Progress to Closed / Done Nov 8, 2022

k8s-ci-robot added this to the v1.26 milestone Nov 8, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update redacting functionality to redact all sensitive info in config when printing with view #109189

Update redacting functionality to redact all sensitive info in config when printing with view #109189

mpuckett159 commented Mar 31, 2022 •

edited

k8s-ci-robot commented Mar 31, 2022

mpuckett159 commented Mar 31, 2022

fedebongio commented Mar 31, 2022

ardaguclu left a comment

ardaguclu commented Jun 7, 2022

leilajal commented Jun 9, 2022

mpuckett159 commented Jul 21, 2022

k8s-triage-robot commented Oct 19, 2022

mpuckett159 commented Oct 19, 2022

mpuckett159 commented Oct 24, 2022

soltysh commented Oct 24, 2022

k8s-ci-robot commented Oct 24, 2022

soltysh commented Oct 24, 2022

soltysh left a comment

sttts Oct 25, 2022

sttts Oct 25, 2022

mpuckett159 Oct 25, 2022 •

edited

sttts commented Nov 8, 2022

k8s-ci-robot commented Nov 8, 2022

Update redacting functionality to redact all sensitive info in config when printing with view #109189

Update redacting functionality to redact all sensitive info in config when printing with view #109189

Conversation

mpuckett159 commented Mar 31, 2022 • edited

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Does this PR introduce a user-facing change?

k8s-ci-robot commented Mar 31, 2022

mpuckett159 commented Mar 31, 2022

fedebongio commented Mar 31, 2022

ardaguclu left a comment

Choose a reason for hiding this comment

ardaguclu commented Jun 7, 2022

leilajal commented Jun 9, 2022

mpuckett159 commented Jul 21, 2022

k8s-triage-robot commented Oct 19, 2022

mpuckett159 commented Oct 19, 2022

mpuckett159 commented Oct 24, 2022

soltysh commented Oct 24, 2022

k8s-ci-robot commented Oct 24, 2022

soltysh commented Oct 24, 2022

soltysh left a comment

Choose a reason for hiding this comment

sttts Oct 25, 2022

Choose a reason for hiding this comment

sttts Oct 25, 2022

Choose a reason for hiding this comment

mpuckett159 Oct 25, 2022 • edited

Choose a reason for hiding this comment

sttts commented Nov 8, 2022

k8s-ci-robot commented Nov 8, 2022

mpuckett159 commented Mar 31, 2022 •

edited

mpuckett159 Oct 25, 2022 •

edited