[release-4.1] Bug 1734622: resourceapply: don't log secret data #472
Conversation
|
/assign @enj |
openshift-ci-robot
added
the
size/M
openshift-ci-robot
added
the
approved
sttts
force-pushed
the
sttts-no-secret-logging
branch
from
0cc90de to
03e1162
Compare
openshift-ci-robot
added
size/S
| @@ -196,6 +196,10 @@ func ApplyConfigMap(client coreclientv1.ConfigMapsGetter, recorder events.Record | |||
|
|
|||
| // ApplySecret merges objectmeta, requires data | |||
| func ApplySecret(client coreclientv1.SecretsGetter, recorder events.Recorder, required *corev1.Secret) (*corev1.Secret, bool, error) { | |||
| if len(required.StringData) > 0 { | |||
| return nil, false, fmt.Errorf("Secret.stringData is not supported") | |||
There was a problem hiding this comment.
Because it's normalized by the server to Data, and we would have to add that normalization here before the DeepEqual. Not impossible, just not done.
| safeExisting := existing.DeepCopy() | ||
|
|
||
| for s := range safeExisting.Data { | ||
| safeExisting.Data[s] = []byte("OLD") |
There was a problem hiding this comment.
maybe we want OLD, NEW and MODIFIED as the states to track?
There was a problem hiding this comment.
what do you mean with 3 states? We only have two.
There was a problem hiding this comment.
Ic what you mean. But note that we create a patch below. We cannot have OLD and NEW for all fields, only those which changed.
sttts
force-pushed
the
sttts-no-secret-logging
branch
from
03e1162 to
2798694
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: enj, 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:
Approvers can indicate their approval by writing |
|
/cherrypick release-4.1 |
|
@enj: once the present PR merges, I will cherry-pick it on top of release-4.1 in a new PR and assign it to you. In response to this:
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. |
|
@enj: new pull request created: #495 In response to this:
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. |
|
/retitle [release-4.1] Bug 1734622: resourceapply: don't log secret data |
|
/bugzilla 1734622 |
stevekuznetsov
changed the title
|
@sttts: All pull requests linked via external trackers have merged. The Bugzilla bug has been moved to the MODIFIED state. In response to this:
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. |
@enj This one was for master branch, not release-4.1. What am I missing? |
resourceapply: don't log secret data
Log OLD and NEW instead if a data value changed.
Reject
stringDatabecause we don't support that right now (could be added if needed, but not in this PR).