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
UPSTREAM: <carry>: Enable timeout validator to run in kube-apiserver #263
UPSTREAM: <carry>: Enable timeout validator to run in kube-apiserver #263
Conversation
24bbd73
to
372323f
Compare
/test unit |
enablement.AddPostStartHookOrDie("openshift.io-TokenTimeoutUpdater", func(context genericapiserver.PostStartHookContext) error { | ||
go timeoutValidator.Run(context.StopCh) | ||
return 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.
should we move this inside the if enablement.OpenshiftConfig().OAuthConfig != 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.
what would be the point, we mean to run it all the time, no?
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.
In the oauth-off mode we will get (keycloak), don't we want to disable this?
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.
these authenticators are only configured if webhook authenticators aren't, so that's ok
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.
see
kubernetes/pkg/kubeapiserver/authenticator/config.go
Lines 185 to 199 in f325571
if len(config.WebhookTokenAuthnConfigFile) > 0 { | |
webhookTokenAuth, err := newWebhookTokenAuthenticator(config) | |
if err != nil { | |
return nil, nil, err | |
} | |
tokenAuthenticators = append(tokenAuthenticators, webhookTokenAuth) | |
} else { | |
// openshift gets injected here as a separate token authenticator because we also add a special group to our oauth authorized | |
// tokens that allows us to recognize human users differently than machine users. The openAPI is always correct because we always | |
// configuration service account tokens, so we just have to create and add another authenticator. | |
// TODO make this a webhook authenticator and remove this patch. | |
// TODO - remove in 4.7, kept here not to disrupt authentication during 4.5->4.6 upgrade | |
tokenAuthenticators = AddOAuthServerAuthenticatorIfNeeded(tokenAuthenticators, config.APIAudiences) | |
} |
/test unit |
unit test failure looks consistent? /test unit |
/test kubernetes-e2e |
/test e2e-cmd |
/lgtm |
/retest |
1 similar comment
/retest |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: stlaz, 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 |
/retest Please review the full test history for this PR and help us cut down flakes. |
8 similar comments
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
10 similar comments
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/override ci/prow/e2e-aws-fips |
@sttts: Overrode contexts on behalf of sttts: ci/prow/e2e-aws-fips In response to this:
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. |
/retest Please review the full test history for this PR and help us cut down flakes. |
12 similar comments
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
@vareti: The following test failed, say
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. |
/retest Please review the full test history for this PR and help us cut down flakes. |
What type of PR is this?
/kind feature
What this PR does / why we need it:
This PR has changes to enable timeout validator in kube-apiserver. The timeout validator will not have any effect unless a timeout value is configured in
KubeAPIServerConfig
.Depends openshift/cluster-kube-apiserver-operator#874 and openshift/oauth-server#49 to work
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?:
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: