-
Notifications
You must be signed in to change notification settings - Fork 183
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
Introduce changes from master to sig-auth-acceptance branch #300
base: sig-auth-acceptance
Are you sure you want to change the base?
Introduce changes from master to sig-auth-acceptance branch #300
Conversation
f1f64b6
to
ee29cc5
Compare
7c1476a
to
5c0c8bc
Compare
5c0c8bc
to
96e75e7
Compare
96e75e7
to
c7e1aff
Compare
/lgtm |
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.
much smaller change than I thought it would be 👍
@@ -232,7 +232,7 @@ func Run(cfg *server.KubeRBACProxyConfig) error { | |||
|
|||
handler := identityheaders.WithAuthHeaders(proxy, cfg.KubeRBACProxyInfo.UpstreamHeaders) | |||
handler = kubefilters.WithAuthorization(handler, authz, scheme.Codecs) | |||
handler = kubefilters.WithAuthentication(handler, authenticator, http.HandlerFunc(filters.UnauthorizedHandler), cfg.DelegatingAuthentication.APIAudiences) | |||
handler = kubefilters.WithAuthentication(handler, authenticator, http.HandlerFunc(filters.UnauthorizedHandler), cfg.DelegatingAuthentication.APIAudiences, 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 happens when you supply the request header nil here? It won't get autodetected from the cluster, right?
IssuerURL: config.IssuerURL, | ||
ClientID: config.ClientID, | ||
tokenAuthenticator, err := oidc.New(ctx, oidc.Options{ | ||
JWTAuthenticator: apiserver.JWTAuthenticator{ |
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.
can we remove the old options and just wire the new config, since we have the chance?
@@ -116,29 +120,30 @@ func (o *ProxyOptions) Validate() []error { | |||
return errs | |||
} | |||
|
|||
func (o *ProxyOptions) ApplyTo(c *server.KubeRBACProxyInfo, a *serverconfig.AuthenticationInfo) error { | |||
func (o *ProxyOptions) ApplyTo(config *server.KubeRBACProxyConfig) error { |
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 previous was better in terms of isolation.
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.
Yes, I know, but I need it to set up HTTP/2 disablement. We can revert it in k8s 1.31.
What
Why