Encryption config: correctly handle overlapping providers #82434
Conversation
k8s-ci-robot
added
release-note-none
|
/priority important-soon |
k8s-ci-robot
added
priority/important-soon
|
/retest |
| // but a failure in the first one should not prevent a later match from being attempted. | ||
| // Thus we never short-circuit on a prefix match that results in an error. | ||
| if err != nil { | ||
| lastErr = err |
There was a problem hiding this comment.
Should we use the last err or the first err?
There was a problem hiding this comment.
I do not know if there a good way to say which error is the most useful. They are all equal in my mind because they match the required prefix.
There was a problem hiding this comment.
Also they are likely to all be the same, some variation on:
no matching key was found for the provided AES transformer
There was a problem hiding this comment.
Also they are likely to all be the same, some variation on:
no matching key was found for the provided AES transformer
Hmm that may be too broad in the case of you accidentally changing the key but not its name. I can aggregate them all?
There was a problem hiding this comment.
Or summarize? If we have to live debug this it’s has gotten really real and we’re likely to be panicking
This change updates NewPrefixTransformers to not short-circuit on the first transformer that has a matching prefix. If the same type of encryption ProviderConfiguration is used more than once, they will share the same prefix. A failure in the first one should not prevent a later match from being attempted. Added TestCBCKeyRotationWithOverlappingProviders unit test to prevent regressions. Note that this test explicitly exercises this flow using an EncryptionConfiguration object as the structure of the resulting transformer is an important part of the check. Signed-off-by: Monis Khan <mkhan@redhat.com>
enj
force-pushed
the
enj/i/enc_overlapping_prefix
branch
from
3bf06e7
to
4dc16f2
Compare
|
/lgtm For 1.17 and backport to 1.16/1.15 |
k8s-ci-robot
added
the
lgtm
|
/cherrypick release-1.16 |
|
/cherrypick release-1.15 |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: enj, smarterclayton 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 |
k8s-ci-robot
added
the
approved
|
/retest |
|
/cherrypick release-1.16 |
…stream-release-1.16 Automated cherry pick of #82434: Encryption config: correctly handle overlapping providers
This change updates NewPrefixTransformers to not short-circuit on
the first transformer that has a matching prefix. If the same type
of encryption ProviderConfiguration is used more than once, they
will share the same prefix. A failure in the first one should not
prevent a later match from being attempted.
Added TestCBCKeyRotationWithOverlappingProviders unit test to
prevent regressions. Note that this test explicitly exercises this
flow using an EncryptionConfiguration object as the structure of the
resulting transformer is an important part of the check.
Signed-off-by: Monis Khan mkhan@redhat.com
/kind bug