Allow specifying additional accepted issuers for the shoot kapi server #5498
Conversation
gardener-robot
added
kind/api-change
|
/cc @vpnachev |
|
/assign |
| if config.ServiceAccountConfig.AcceptedIssuers != nil { | ||
| out.AcceptedIssuers = append([]string{}, config.ServiceAccountConfig.AcceptedIssuers...) | ||
| } |
There was a problem hiding this comment.
Why not simply assigning out.AcceptedIssuers = config.ServiceAccountConfig.AcceptedIssuers?
There was a problem hiding this comment.
My thought here was that it seems more robust to have different backing arrays for the out.AcceptedIssuers and config.ServiceAccountConfig.AcceptedIssuers slices.
There was a problem hiding this comment.
Documentation of b.Shoot.GetInfo() states that the method should be used only for reading the data of the shoot resource. This is why it seems more logical to me to use a different underlying array since I do not expect modifying the out object to actually change the shoot's spec object.
There was a problem hiding this comment.
OK, but we don't do this anywhere else, so I would still vote for going the straight-forward way for consistency reasons.
There was a problem hiding this comment.
I get your point and I did the requested changes. The behavior we are supporting is still rather confusing for me. I am leaving a sample test for reference.
It("Failing test", func() {
botanist.Shoot.SetInfo(&gardencorev1beta1.Shoot{
Spec: gardencorev1beta1.ShootSpec{
Kubernetes: gardencorev1beta1.Kubernetes{
KubeAPIServer: &gardencorev1beta1.KubeAPIServerConfig{
ServiceAccountConfig: &gardencorev1beta1.ServiceAccountConfig{
AcceptedIssuers: []string{"a", "b"},
},
},
},
},
})
serviceAccountConfig, err := botanist.computeKubeAPIServerServiceAccountConfig(ctx, botanist.Shoot.GetInfo().Spec.Kubernetes.KubeAPIServer, "something.here.com")
Expect(err).NotTo(HaveOccurred())
serviceAccountConfig.AcceptedIssuers[0] = "c"
Expect(botanist.Shoot.GetInfo().Spec.Kubernetes.KubeAPIServer.ServiceAccountConfig.AcceptedIssuers[0]).To(Equal("a"))
})
pkg/operation/botanist/component/kubeapiserver/kube_apiserver_test.go
Outdated
Show resolved
Hide resolved
gardener#5498) * Allow specifying additional accepted issuers for the shoot kapi server * Address comments * Address comments
gardener#5498) * Allow specifying additional accepted issuers for the shoot kapi server * Address comments * Address comments
How to categorize this PR?
/area control-plane
/kind api-change
/kind enhancement
What this PR does / why we need it:
In k8s 1.22+ there is a posibility to specify the flag
--service-account-issuermultiple times for thekube-apiserver. Right now Gardener sets this flag to the value of.kubeAPIServer.serviceAccountConfig.issueror defaults it to the API server's address. If one wants to change this flag, all of the already generated tokens will be invalidated and the change would cause disturbance. Askube-apiserver's documentation here (see--service-account-issuerflag) states if the flag is specified multiple times then the first occurrence will be used to generate new service account tokens and all other occurrences will be used to determine if a service account token is accepted.This PR introduces a new field in the shoot spec called
.kubeAPIServer.serviceAccountConfig.acceptedIssuerswhich will allow the configuration of multipleaccepted issuers. This will allow a non-disruptive change of theissuerof thekube-apiserver. See here and read the note regarding the--service-account-issuerflag.Having the ability to change the
issuerof akube-apiserverwill give the opportunity to expose theopenid-configurationandjwksof a cluster without enabling anonymous authentication. This will also allow serving the OpenID discovery documents on an endpoint with certificate signed by a trusted certificate authority.Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
I am not sure if changing the order in theprotobufstruct tags can have some implications.Release note: