change TRUNCATED to DATA+OMITTED in kubectl config view #66023
Conversation
k8s-ci-robot
added
release-note
k8s-ci-robot
added
the
needs-ok-to-test
| @@ -27,8 +27,8 @@ import ( | |||
| ) | |||
|
|
|||
| func init() { | |||
| sDec, _ := base64.StdEncoding.DecodeString("REDACTED+") | |||
| redactedBytes = []byte(string(sDec)) | |||
| sDec, _ := base64.StdEncoding.DecodeString("DATA+OMITTED") | |||
There was a problem hiding this comment.
I think we actually want to keep both of these. The ClientKeyData should definitely remain as REDACTED+, since it's actually a secret.
There was a problem hiding this comment.
I'd argue that other secrets like AuthInfos[*].Token and AuthInfos[*].Password should be redacted too. What do you think?
There was a problem hiding this comment.
I would agree, but that's not my call to make 😄
There was a problem hiding this comment.
Okay, I've thought about it, and I think we should scope this PR to just change the public CA to be DATA+OMITTED. We can open another PR about the other fields.
It's not my call (I'm not on a sig/* group), but I think that's the best way to avoid contention.
ibrasho
force-pushed
the
change-TRUNCATED-to-DATA+OMITTED
branch
from
4342b8c to
07d204d
Compare
ibrasho
force-pushed
the
change-TRUNCATED-to-DATA+OMITTED
branch
from
07d204d to
d3aac46
Compare
k8s-ci-robot
added
size/XS
|
LGTM |
|
/ok-to-test |
k8s-ci-robot
removed
the
needs-ok-to-test
ibrasho
force-pushed
the
change-TRUNCATED-to-DATA+OMITTED
branch
from
d3aac46 to
dac8cf6
Compare
k8s-ci-robot
added
size/S
ibrasho
force-pushed
the
change-TRUNCATED-to-DATA+OMITTED
branch
2 times, most recently
from
3ecb1b5 to
abe5539
Compare
|
/test pull-kubernetes-bazel-test |
|
/assign @caesarxuchao |
Signed-off-by: Ibrahim AshShohail <me@ibrasho.com>
|
/retest |
ibrasho
force-pushed
the
change-TRUNCATED-to-DATA+OMITTED
branch
from
abe5539 to
23996b2
Compare
|
Unrelated failures? |
|
All the test failures seem unrelated to me |
|
@ibrasho thanks for doing this work. What do we need to do to get this into a release? |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
k8s-ci-robot
added
the
approved
|
/retest |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: caesarxuchao, ibrasho, yliaog 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:
Approvers can indicate their approval by writing |
|
/retest Review the full test history for this PR. Silence the bot with an |
|
/test all Tests are more than 96 hours old. Re-running tests. |
|
/test all [submit-queue is verifying that this PR is safe to merge] |
|
Automatic merge from submit-queue (batch tested with PRs 60790, 66023, 67549). If you want to cherry-pick this change to another branch, please follow the instructions here. |
|
/sig api-machinery |
k8s-ci-robot
added
the
sig/api-machinery
What this PR does / why we need it:
Based on the discussion in #61573, this PR switches the replacement text for CA certificate data and client certificates and secrets printed using
kubectl config view. Currently,REDACTEDis used, which might give a false impression that the data is a secret (which is not true for the public certificates).This PR changes
REDACTEDtoDATA+OMITTED. The printed string is the base64 encoded string on the byte stream. Some trickery is involved to print a readable string (refer to this comment for more info).Which issue(s) this PR fixes:
Fixes #61573
Special notes for your reviewer:
Release note: