encryption: fix freezing key controller#615
encryption: fix freezing key controller#615openshift-merge-robot merged 4 commits intoopenshift:masterfrom
Conversation
openshift-ci-robot
added
the
size/L
openshift-ci-robot
added
the
approved
sttts
force-pushed
the
sttts-encryption-frozen-key-controller
branch
from
54f1ce1 to
2977f17
Compare
| }, | ||
|
|
||
| { | ||
| name: "create a new write key when the previous key expired and another read key exists", |
There was a problem hiding this comment.
shouldn't we validate if a new key was created? (validateFunc function)
| "list:pods:kms", | ||
| "get:secrets:kms", | ||
| "list:secrets:openshift-config-managed", | ||
| "create:secrets:openshift-config-managed", |
There was a problem hiding this comment.
this only says that something was created ;)
There was a problem hiding this comment.
that's a general issue in this test, isn't it? But what else do we create in the key controller than keys? :)
There was a problem hiding this comment.
yeah, hard to beat that argument :)
| for i, k := range recentFirstSortedKeys { | ||
| if allMigrated, missing, _ := MigratedFor(grs, k); allMigrated { | ||
| return recentFirstSortedKeys[:i+1] | ||
| if i+1 < len(recentFirstSortedKeys) { |
There was a problem hiding this comment.
this looks to be safe, we could have a unit test for it.
There was a problem hiding this comment.
what do you want to test?
There was a problem hiding this comment.
we have tests that "one more read-key" is fine. So this is covered. The rest here is just Golang standard behaviour.
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: p0lyn0mial, sttts The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
…ey-controller encryption: fix freezing key controller
4 participants
The key controller was still just expecting one read-key, while with the recent change for backup/restore we always preserve one more key. This made the key controller freeze after the first two read-keys were in the config.
While fixing the upper, we also run into the case that during pruning this one more read-key is preserved, but this read-key must be backed. Hence, we have to preserve also all other unbacked read-keys in between.