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

Add Config Observer for AccessTokenInactivityTimeout in OAuth Cluster Config #874

Merged
merged 4 commits into from Aug 1, 2020
Merged

Add Config Observer for AccessTokenInactivityTimeout in OAuth Cluster Config #874

merged 4 commits into from Aug 1, 2020

Conversation

vareti
Copy link
Contributor

@vareti vareti commented Jun 3, 2020 •
edited

Currently, access tokens are invalidated only when their token age expires. This PR wires in the bits needed to expire the token based on user inactivity.
The config observer observes if AccessTokenInactivityTimeoutSeconds field is set OAuth cluster config. If timeout is set to non-zero value, it appends the below config to KubeAPIServerConfig.

oauthConfig:
  tokenConfig:
    accessTokenInactivityTimeoutSeconds: <timeout value>

When AccessTokenInactivityTimeoutSeconds is either zero or not present in OAuth cluster config, it removes the above the config from KubeAPIServerConfig.

@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 3, 2020
@vareti vareti force-pushed the inactivity-timeout branch 6 times, most recently from 9432cae to 8900920 Compare June 8, 2020 21:20
@vareti vareti mentioned this pull request Jun 8, 2020
Merged
@vareti vareti changed the title [WIP] add access token inactivity timeout argument to kube-apiserver Add Config Observer for TokenConfig in OAuth Cluster Config Jun 8, 2020
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 8, 2020
@vareti vareti changed the title Add Config Observer for TokenConfig in OAuth Cluster Config Add Config Observer for AccessTokenInactivityTimeoutSeconds in OAuth Cluster Config Jun 8, 2020
@deads2k
Copy link
Contributor

deads2k commented Jun 16, 2020

/hold

We need an e2e test proving the kube-apiserver actually does the right thing after reacting to this setting. aws-operator is the spot for now.

@openshift-ci-robot openshift-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jun 16, 2020
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 17, 2020
@vareti
Copy link
Contributor Author

vareti commented Jun 18, 2020

/retest

1 similar comment
@vareti
Copy link
Contributor Author

vareti commented Jun 18, 2020

/retest

@vareti vareti mentioned this pull request Jun 19, 2020
Merged
@vareti vareti force-pushed the inactivity-timeout branch 3 times, most recently from b757ce8 to 136ef79 Compare June 19, 2020 17:48
@vareti vareti mentioned this pull request Jun 30, 2020
Closed
@vareti vareti force-pushed the inactivity-timeout branch 2 times, most recently from 59c903d to d4320ec Compare July 1, 2020 12:52
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 31, 2020
@sttts
Copy link
Contributor

sttts commented Jul 31, 2020

/lgtm
/approve

@openshift-ci-robot openshift-ci-robot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jul 31, 2020
@sttts
Copy link
Contributor

sttts commented Jul 31, 2020

Terraform error.

/retest

@sttts
Copy link
Contributor

sttts commented Jul 31, 2020

AWS lease error.

/retest

@vareti
Copy link
Contributor Author

vareti commented Jul 31, 2020

/test e2e-aws-serial

1 similar comment
@vareti
Copy link
Contributor Author

vareti commented Jul 31, 2020

/test e2e-aws-serial

@vareti
Copy link
Contributor Author

vareti commented Jul 31, 2020

/test e2e-aws-operator

2 similar comments
@vareti
Copy link
Contributor Author

vareti commented Aug 1, 2020

/test e2e-aws-operator

@vareti
Copy link
Contributor Author

vareti commented Aug 1, 2020

/test e2e-aws-operator

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Aug 1, 2020
@sttts
Copy link
Contributor

sttts commented Aug 1, 2020

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 1, 2020
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Aug 1, 2020
@sttts
Copy link
Contributor

sttts commented Aug 1, 2020

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 1, 2020
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sttts, vareti

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

@vareti
Copy link
Contributor Author

vareti commented Aug 1, 2020

/test e2e-aws

@openshift-ci-robot
Copy link

openshift-ci-robot commented Aug 1, 2020 •
edited

@vareti: The following tests failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/e2e-aws-operator-encryption-perf 1993da4 link /test e2e-aws-operator-encryption-perf
ci/prow/e2e-aws-operator-encryption 1993da4 link /test e2e-aws-operator-encryption

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

@vareti
Copy link
Contributor Author

vareti commented Aug 1, 2020

/test e2e-aws-serial

@openshift-merge-robot openshift-merge-robot merged commit d54e109 into openshift:master Aug 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stlaz stlaz stlaz left review comments

@sttts sttts sttts left review comments

@deads2k deads2k Awaiting requested review from deads2k

@soltysh soltysh Awaiting requested review from soltysh

Assignees

@sttts sttts

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@vareti @deads2k @sttts @openshift-ci-robot @stlaz @openshift-merge-robot