🐛 Fix frontproxy #3023
🐛 Fix frontproxy #3023
Conversation
Signed-off-by: Mangirdas Judeikis <[email protected]>
kcp-ci-bot
added
release-note
| @@ -117,6 +117,7 @@ func (c *Authentication) ApplyTo(authenticationInfo *genericapiserver.Authentica | |||
| versionedInformers.Core().V1().Secrets().Lister(), | |||
| versionedInformers.Core().V1().ServiceAccounts().Lister(), | |||
| ) | |||
| authenticatorConfig.SecretsWriter = tokenGetterClient.CoreV1().Secrets() | |||
There was a problem hiding this comment.
It is used in LegacyServiceAccount validator. It just fails to start if this is not set and APIServer fails .
I think its for SA management of secrets
There was a problem hiding this comment.
"ts":1697038810878.5505,"caller":"dynamiccertificates/dynamic_serving_content.go:113","msg":"Loaded a new cert/key pair","v":2,"name":"serving-cert::/etc/kcp-front-proxy/tls/tls.crt::/etc/kcp-front-proxy/tls/tls.key"}
{"ts":1697038811067.4224,"caller":"dynamiccertificates/dynamic_cafile_content.go:119","msg":"Loaded a new CA Bundle and Verifier","v":2,"name":"client-ca-bundle::/etc/kcp-front-proxy/client-ca/tls.crt"}
{"ts":1697038811067.9343,"caller":"transport/cert_rotation.go:137","msg":"Starting client certificate rotation controller\n","v":3}
{"ts":1697038811068.258,"caller":"cli/run.go:74","msg":"command failed","err":"while creating legacy validator, err: SecretsWriter must be provided"}
API server fails to start with this. We missed this in rebase
There was a problem hiding this comment.
Though cert rotation also worries me. Why in the frontproxy? Or in general: why is this part of the authentication plugin? Seems odd.
There was a problem hiding this comment.
Might be how the front proxy was worried together. At this point it's not even starting. But you say authentication plugin should not even be there?
There was a problem hiding this comment.
If this is the case, this means we have wrong api-server flavor config in frontproxy case.
Maybe worth merging this for now as it blocks usage of main branch all together with front proxy and looking how to get rid of it as followup?
There was a problem hiding this comment.
The auth plugin is expected to be there. We use it in the proxy. I just wonder what else ppl have put into the auth plugins and whether we want to inherit it.
There was a problem hiding this comment.
It's used to set kubernetes.io/legacy-token-last-used on the service account secret. It's properly cluster-scoped, and only active when we enable service account lookup (which we do it seems, and that's probably not a good idea from the front-proxy).
|
/retest |
|
ping @sttts |
|
/lgtm I think this shows that we have to rethink service account handling in the front-proxy. The front-proxy will never know all serviceaccounts in an informer, and it changing last-used annotations is also questionable. In sharded, potentially multi-region mode this needs more thought. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sttts 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 |
|
LGTM label has been added. Git tree hash: c57d07e5284f4666dff89707d6cf043d8f7750d2
|
kcp-ci-bot
added
the
approved
Summary
Fix frontproxy deployment
Related issue(s)
Fixes #
Release Notes