-
Notifications
You must be signed in to change notification settings - Fork 38.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
storage: transformers: pass a context.Context #108174
storage: transformers: pass a context.Context #108174
Conversation
9a24739
to
3f06d71
Compare
@@ -199,13 +200,13 @@ func TestEncryptionProviderConfigCorrect(t *testing.T) { | |||
} | |||
|
|||
for _, testCase := range transformers { | |||
transformedData, err := testCase.Transformer.TransformToStorage(originalText, context) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The diff is a little larger than it has to be since I renamed these so as to not collide with the package name.
@@ -2250,7 +2250,7 @@ func TestConsistentList(t *testing.T) { | |||
transformer.store = store | |||
|
|||
for i := 0; i < 5; i++ { | |||
if err := transformer.createObject(); err != nil { | |||
if err := transformer.createObject(context.TODO()); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My follow-up will be to change these, at which point only the top-level contexts will need any change.
3f06d71
to
bcd5ab3
Compare
bcd5ab3
to
b1a13f6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a few tweaks on tests, lgtm otherwise
staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/config_test.go
Outdated
Show resolved
Hide resolved
staging/src/k8s.io/apiserver/pkg/storage/value/encrypt/aes/aes_test.go
Outdated
Show resolved
Hide resolved
staging/src/k8s.io/apiserver/pkg/storage/value/encrypt/aes/aes_test.go
Outdated
Show resolved
Hide resolved
staging/src/k8s.io/apiserver/pkg/storage/value/encrypt/envelope/envelope_test.go
Outdated
Show resolved
Hide resolved
staging/src/k8s.io/apiserver/pkg/storage/value/encrypt/envelope/envelope_test.go
Outdated
Show resolved
Hide resolved
staging/src/k8s.io/apiserver/pkg/storage/value/encrypt/secretbox/secretbox_test.go
Outdated
Show resolved
Hide resolved
When an envelope transformer calls out to KMS (for instance), it will be very helpful to pass a `context.Context` to allow for cancellation. This patch does that, while passing the previously-expected additional data via a context value. Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
b1a13f6
to
27312fe
Compare
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: liggitt, stevekuznetsov 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 |
/triage accepted |
When an envelope transformer calls out to KMS (for instance), it will be
very helpful to pass a
context.Context
to allow for cancellation. Thispatch does that, while passing the previously-expected additional data
via a context value.
Signed-off-by: Steve Kuznetsov skuznets@redhat.com
/kind cleanup
/sig api-machinery
/cc @deads2k @liggitt @enj