Skip to content

Commit

Permalink
Switch CertificateSigningRequest controller to controller-runtime (#…
Browse files Browse the repository at this point in the history
…6615)

* Add integration test for CSR controller

* Switch controller to native controller-runtime controller

* Minor cosmetics

* Run make generate

* Address PR review feedback

* [Cleanup] Remove certificates/v1beta1 occurrences

* Address PR review feedback

* Remove certificates/v1beta1 usage in GAC graph

Co-authored-by: Rafael Franzke <rafael.franzke@sap.com>
  • Loading branch information
ary1992 and rfranzke committed Sep 9, 2022
1 parent 7a6c42a commit 29a679f
Show file tree
Hide file tree
Showing 37 changed files with 977 additions and 911 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ data:
concurrentSyncs: {{ required ".Values.global.controller.config.controllers.bastion.concurrentSyncs is required" .Values.global.controller.config.controllers.bastion.concurrentSyncs }}
maxLifetime: {{ required ".Values.global.controller.config.controllers.bastion.maxLifetime is required" .Values.global.controller.config.controllers.bastion.maxLifetime }}
{{- end }}
{{- if .Values.global.controller.config.controllers.certificateSigningRequest }}
certificateSigningRequest:
concurrentSyncs: {{ required ".Values.global.controller.config.controllers.certificateSigningRequest.concurrentSyncs is required" .Values.global.controller.config.controllers.certificateSigningRequest.concurrentSyncs }}
{{- end }}
{{- if .Values.global.controller.config.controllers.cloudProfile }}
cloudProfile:
concurrentSyncs: {{ required ".Values.global.controller.config.controllers.cloudProfile.concurrentSyncs is required" .Values.global.controller.config.controllers.cloudProfile.concurrentSyncs }}
Expand Down
2 changes: 2 additions & 0 deletions charts/gardener/controlplane/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,8 @@ global:
syncPeriod: 30m
exposureClass:
concurrentSyncs: 5
certificateSigningRequest:
concurrentSyncs: 5
leaderElection:
leaderElect: true
leaseDuration: 15s
Expand Down
2 changes: 1 addition & 1 deletion cmd/gardener-controller-manager/app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ func run(ctx context.Context, log logr.Logger, cfg *config.ControllerManagerConf
}

log.Info("Adding controllers to manager")
if err := controller.AddControllersToManager(mgr, cfg); err != nil {
if err := controller.AddControllersToManager(mgr, cfg, restConfig); err != nil {
return fmt.Errorf("failed adding controllers to manager: %w", err)
}

Expand Down
2 changes: 0 additions & 2 deletions cmd/gardenlet/app/gardenlet.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ import (
"github.com/spf13/cobra"
"github.com/spf13/pflag"
certificatesv1 "k8s.io/api/certificates/v1"
certificatesv1beta1 "k8s.io/api/certificates/v1beta1"
coordinationv1 "k8s.io/api/coordination/v1"
corev1 "k8s.io/api/core/v1"
eventsv1 "k8s.io/api/events/v1"
Expand Down Expand Up @@ -334,7 +333,6 @@ func NewGardenlet(ctx context.Context, cfg *config.GardenletConfiguration) (*Gar
&gardencorev1beta1.Project{},
&gardencorev1beta1.SecretBinding{},
&certificatesv1.CertificateSigningRequest{},
&certificatesv1beta1.CertificateSigningRequest{},
&coordinationv1.Lease{},
&corev1.Namespace{},
&corev1.ConfigMap{},
Expand Down
22 changes: 11 additions & 11 deletions docs/concepts/controller-manager.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,17 @@ Clients like `gardenctl` are advised to not re-use `Bastion`s whose deletion tim
Refer to [GEP-15](../proposals/15-manage-bastions-and-ssh-key-pair-rotation.md) for more information on the lifecycle of
`Bastion` resources.

### [`CertificateSigningRequest` Controller](../../pkg/controllermanager/controller/certificatesigningrequest)

After the [gardenlet](./gardenlet.md) gets deployed on the Seed cluster it needs to establish itself as a trusted party to communicate with the Gardener API server. It runs through a bootstrap flow similar to the [kubelet bootstrap](https://kubernetes.io/docs/reference/access-authn-authz/kubelet-tls-bootstrapping/) process.

On startup the gardenlet uses a `kubeconfig` with a [bootstrap token](https://kubernetes.io/docs/reference/access-authn-authz/bootstrap-tokens/) which authenticates it as being part of the `system:bootstrappers` group. This kubeconfig is used to create a [`CertificateSigningRequest`]( https://kubernetes.io/docs/reference/access-authn-authz/certificate-signing-requests/) (CSR) against the Gardener API server.

The controller in `gardener-controller-manager` checks whether the `CertificateSigningRequest` has the expected organisation, common name and usages which the gardenlet would request.

It only auto-approves the CSR if the client making the request is allowed to "create" the
`certificatesigningrequests/seedclient` subresource. Clients with the `system:bootstrappers` group are bound to the `gardener.cloud:system:seed-bootstrapper` `ClusterRole`, hence, they have such privileges. As the bootstrap kubeconfig for the gardenlet contains a bootstrap token which is authenticated as being part of the [`systems:bootstrappers` group](../../charts/gardener/controlplane/charts/application/templates/clusterrolebinding-seed-bootstrapper.yaml), its created CSR gets auto-approved.

### [`CloudProfile` Controller](../../pkg/controllermanager/controller/cloudprofile)

`CloudProfile`s are essential when it comes to reconciling `Shoot`s since they contain constraints (like valid machine types, Kubernetes versions, or machine images) and sometimes also some global configuration for the respective environment (typically via provider-specific configuration in `.spec.providerConfig`).
Expand Down Expand Up @@ -253,14 +264,3 @@ In case a `Lease` is not renewed for the configured amount in `config.controller
2. Additionally, conditions and constraints of all `Shoot` resources scheduled on the affected seed are set to `Unknown` as well
because a striking Gardenlet won't be able to maintain these conditions any more.
3. If the gardenlet's client certificate has expired (identified based on the `.status.clientCertificateExpirationTimestamp` field in the `Seed` resource) and if it is managed by a `ManagedSeed` then this will be triggered for a reconciliation. This will trigger the bootstrapping process again and allows gardenlets to obtain a fresh client certificate.

### "CertificateSigningRequest" controller

After the [gardenlet](./gardenlet.md) gets deployed on the Seed cluster it needs to establish itself as a trusted party to communicate with the Gardener API server. It runs through a bootstrap flow similar to the [kubelet bootstrap](https://kubernetes.io/docs/reference/access-authn-authz/kubelet-tls-bootstrapping/) process.

On startup the gardenlet uses a `kubeconfig` with a [bootstrap token](https://kubernetes.io/docs/reference/access-authn-authz/bootstrap-tokens/) which authenticates it as being part of the `system:bootstrappers` group. This kubeconfig is used to create a [`CertificateSigningRequest`]( https://kubernetes.io/docs/reference/access-authn-authz/certificate-signing-requests/) (CSR) against the Gardener API server.

The controller in `gardener-controller-manager` checks whether the `CertificateSigningRequest` has the expected organisation, common name and usages which the gardenlet would request.

It only auto-approves the CSR if the client making the request is allowed to "create" the
`certificatesigningrequests/seedclient` subresource. Clients with the `system:bootstrappers` group are bound to the `gardener.cloud:system:seed-bootstrapper` `ClusterRole`, hence, they have such privileges. As the bootstrap kubeconfig for the gardenlet contains a bootstrap token which is authenticated as being part of the [`systems:bootstrappers` group](../../charts/gardener/controlplane/charts/application/templates/clusterrolebinding-seed-bootstrapper.yaml), its created CSR gets auto-approved.
2 changes: 2 additions & 0 deletions example/20-componentconfig-gardener-controller-manager.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ controllers:
bastion:
maxLifetime: 24h
concurrentSyncs: 5
certificateSigningRequest:
concurrentSyncs: 5
secretBinding:
concurrentSyncs: 5
seed:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ import (
"github.com/go-logr/logr"
admissionv1 "k8s.io/api/admission/v1"
certificatesv1 "k8s.io/api/certificates/v1"
certificatesv1beta1 "k8s.io/api/certificates/v1beta1"
coordinationv1 "k8s.io/api/coordination/v1"
corev1 "k8s.io/api/core/v1"
rbacv1 "k8s.io/api/rbac/v1"
Expand Down Expand Up @@ -244,37 +243,17 @@ func (h *handler) admitCertificateSigningRequest(seedName string, request admiss
return admission.Errored(http.StatusBadRequest, fmt.Errorf("unexpected operation: %q", request.Operation))
}

var (
req []byte
usages []certificatesv1.KeyUsage
)

switch request.Resource.Version {
case "v1":
csr := &certificatesv1.CertificateSigningRequest{}
if err := h.decoder.Decode(request, csr); err != nil {
return admission.Errored(http.StatusBadRequest, err)
}

req = csr.Spec.Request
usages = csr.Spec.Usages

case "v1beta1":
csr := &certificatesv1beta1.CertificateSigningRequest{}
if err := h.decoder.Decode(request, csr); err != nil {
return admission.Errored(http.StatusBadRequest, err)
}

req = csr.Spec.Request
usages = kutil.CertificatesV1beta1UsagesToCertificatesV1Usages(csr.Spec.Usages)
csr := &certificatesv1.CertificateSigningRequest{}
if err := h.decoder.Decode(request, csr); err != nil {
return admission.Errored(http.StatusBadRequest, err)
}

x509cr, err := utils.DecodeCertificateRequest(req)
x509cr, err := utils.DecodeCertificateRequest(csr.Spec.Request)
if err != nil {
return admission.Errored(http.StatusBadRequest, err)
}

if ok, reason := gutil.IsSeedClientCert(x509cr, usages); !ok {
if ok, reason := gutil.IsSeedClientCert(x509cr, csr.Spec.Usages); !ok {
return admission.Errored(http.StatusForbidden, fmt.Errorf("can only create CSRs for seed clusters: %s", reason))
}

Expand Down

0 comments on commit 29a679f

Please sign in to comment.