Bug: ACM-5052, Policy gets shortly into non-compliant state #144
Conversation
JeffeyL
force-pushed
the
ACM-5052
branch
2 times, most recently
from
68084ca
to
40fe477
Compare
There was a problem hiding this comment.
Looks great! Just small comments that aren't really necessary.
I think #145 will fix the timeout in the most recent CI run, so we could wait for that to merge, then re-base this. Or maybe a re-run will randomly work.
test/resources/case32_secret_stringdata/case32_create_secret.yaml
Outdated
Show resolved
Hide resolved
JeffeyL
force-pushed
the
ACM-5052
branch
2 times, most recently
from
59158ee
to
5175a2a
Compare
test/resources/case32_secret_stringdata/case32_create_secret.yaml
Outdated
Show resolved
Hide resolved
| @@ -2556,6 +2558,30 @@ func handleSingleKey( | |||
| mergedValue.(map[string]interface{}), existingValue.(map[string]interface{})) | |||
| } | |||
|
|
|||
| if key == "stringData" && existingObj.GetKind() == "Secret" { | |||
There was a problem hiding this comment.
somewhere throw an error?
JeffeyL
force-pushed
the
ACM-5052
branch
2 times, most recently
from
ceec8c6
to
d6f7646
Compare
yiraeChristineKim
requested review from
mprahl,
dhaiducek,
yiraeChristineKim and
JustinKuli
JeffeyL
force-pushed
the
ACM-5052
branch
2 times, most recently
from
7334545
to
81065d0
Compare
| @@ -2411,6 +2412,7 @@ func handleSingleKey( | |||
|
|
|||
| desiredValue := formatTemplate(desiredObj, key) | |||
| existingValue := existingObj.UnstructuredContent()[key] | |||
|
|
|||
There was a problem hiding this comment.
Can you remove this blank
|
/unhold |
test/resources/case32_secret_stringdata/case32_create_secret.yaml
Outdated
Show resolved
Hide resolved
Solution: decode automatically converted stringData prior to evaluation Signed-off-by: Jeffrey Luo <jeluo@redhat.com>
|
@JustinKuli requested changes here, so he'll need to re-review it for it to merge. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dhaiducek, JeffeyL, JustinKuli 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 |
openshift-merge-robot
merged commit 2bd6142
into
open-cluster-management-io:main
|
@JeffeyL: only open-cluster-management-io org members may request cherry picks. You can still do the cherry-pick manually. 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. |
|
/cherry-pick release-2.7 |
|
@yiraeChristineKim: only open-cluster-management-io org members may request cherry picks. You can still do the cherry-pick manually. 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. |
Description of problem: When
stringDatais set on a Secret within a policy, Kubernetes automatically converts it to a base64 encodeddatafield. The next time the policy is evaluated, the controller expects to see astringDatafield - however,stringDatais no longer set (due to the conversion) so the controller sees a mismatch.Solution: When the controller handles a single key, a check has been added for the exact case where
stringDatais set within a Secret. In this case, thedatafield will be base64 decoded and then used for comparison.Ref: https://issues.redhat.com/browse/ACM-5052