Skip to content
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.

Sign up for GitHub

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

Jump to bottom

kmsv2: ignore cache key expiration on reads #118084

Merged
merged 1 commit into from May 22, 2023
Merged

kmsv2: ignore cache key expiration on reads #118084

merged 1 commit into from May 22, 2023

Conversation

enj
Copy link
Member

@enj enj commented May 17, 2023

/kind feature

NONE

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. kind/feature Categorizes issue or PR as related to a new feature. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. area/apiserver sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/auth Categorizes an issue or PR as relevant to SIG Auth. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels May 17, 2023
@enj
Copy link
Member Author

enj commented May 17, 2023

/triage accepted
/milestone v1.28

@k8s-ci-robot k8s-ci-robot added this to the v1.28 milestone May 17, 2023
@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels May 17, 2023
@mikedanese
Copy link
Member

Unit test failures are real.

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 17, 2023
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: faa2c096cbafef6d45790888627ba0ab2fd86a0f

@enj enj force-pushed the enj/f/cache_get_expired branch from f7fa0ab to f62f8c0 Compare May 18, 2023 02:06
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 18, 2023
aramase
aramase reviewed May 18, 2023
View reviewed changes
Copy link
Member

@aramase aramase left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 18, 2023
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 10fff9e206507923ac8a59b3f997b05faead9aa9

deads2k
deads2k reviewed May 22, 2023
View reviewed changes
staging/src/k8s.io/apimachinery/pkg/util/cache/expiring_test.go
if _, ok := c.Get("b"); ok {
t.Fatalf("we should not have found b key")
}
c.AllowExpiredGet = true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add a Set to be sure this properly clears the data even for the AllowExpiredGet case.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@enj
kmsv2: ignore cache key expiration on reads
86a258b 
Signed-off-by: Monis Khan <mok@microsoft.com>
@enj enj force-pushed the enj/f/cache_get_expired branch from f62f8c0 to 86a258b Compare May 22, 2023 15:51
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 22, 2023
@k8s-ci-robot k8s-ci-robot requested a review from aramase May 22, 2023 15:51
@deads2k
Copy link
Contributor

deads2k commented May 22, 2023

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 22, 2023
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 87cb97bbba73ad37b9921b390fdc7d9989dc16b3

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, enj

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 22, 2023
@k8s-ci-robot k8s-ci-robot merged commit 27661b6 into kubernetes:master May 22, 2023
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@deads2k deads2k deads2k left review comments

@mikedanese mikedanese Awaiting requested review from mikedanese

@wojtek-t wojtek-t Awaiting requested review from wojtek-t

@aramase aramase Awaiting requested review from aramase

Assignees

@mikedanese mikedanese

@deads2k deads2k

@aramase aramase

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/apiserver cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. release-note-none Denotes a PR that doesn't merit a release note. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/auth Categorizes an issue or PR as relevant to SIG Auth. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
SIG Auth
Archived in project
SIG Auth: KMS v2
Status: ✅ Done
Milestone
v1.28
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@enj @mikedanese @k8s-ci-robot @deads2k @aramase