Skip to content

Commit

Permalink
Refactor operator to support several types of resources + propagate l…
Browse files Browse the repository at this point in the history
…abels (#38992)

* Refactor operator resource reconciler to support resource153 + add label propagation

I initially just wanted to add label propagation (which is required for
OpenSSH and OpenSSHEICE servers).

However I ran into an issue: not all Teleport resources implement the
`ResourceWithLabels` interface.  Basically we have 4 cases:
- resources implementing `ResourceWithLabels` (e.g. User, Role, ...)
- resources implementing `ResourceWithOrigin` but not
  `ResourceWithLabels` (e.g. GitHub connector, ProvisionToken, ...)
- resources that have been tweaked to implement only the operator
  `TeleportResource` interface (`LoginRule`)
- resources implementing `Resource153`

To support all those resoucres, we can either:
- edit each resource so they expose the same methods (e.g.
  ProvisionToken exposes `GetLabels` instead of `GetStaticLabels`)
- wrap each ressource in a wrapper that implements the operator
  TeleportResource interface
- add multiple reconcilers, one for each resource kind

I don't think it's sane for the operator to force changes on Teleport
resources. Especially as those are known to be non consistent. I also
don't want the operator to maintain a copy of each resource, this is not
viable on the long term.

For those reasons I chose the 3rd approach and built 3 reconcilers. Then
I deducplicated the code to end up with a generic reconciler that can be
tuned by passing an adapter. The adapter is an empty struct that has all
the methods to extract the information the operator requires from the
resource.

The resulting PR is large and heavily uses generics but the amount of
logical changes is quite low (the only diff is that we now copy labels).
I can walk you through the PR if needed.

With this PR, the operator supports properly any resource implementing
`types.Resource153`, this will be useful to add `TeleportBotV1` support.

* Update integrations/operator/controllers/reconcilers/base.go

Co-authored-by: Noah Stride <[email protected]>

* use structured logging

---------

Co-authored-by: Noah Stride <[email protected]>
  • Loading branch information
hugoShaka and strideynet committed Mar 7, 2024
1 parent 23c4fff commit 9766732
Show file tree
Hide file tree
Showing 42 changed files with 1,138 additions and 803 deletions.
4 changes: 2 additions & 2 deletions integrations/operator/apis/resources/v1/loginrule_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,8 @@ func (l *LoginRuleResource) SetOrigin(origin string) {
l.LoginRule.Metadata.SetOrigin(origin)
}

func (l *LoginRuleResource) GetMetadata() types.Metadata {
return *l.LoginRule.Metadata
func (l *LoginRuleResource) Origin() string {
return l.LoginRule.Metadata.Origin()
}

func (l *LoginRuleResource) GetRevision() string {
Expand Down
32 changes: 32 additions & 0 deletions integrations/operator/controllers/reconciler.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/*
* Teleport
* Copyright (C) 2024 Gravitational, Inc.
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

package controllers

import (
"sigs.k8s.io/controller-runtime/pkg/manager"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
)

// Reconciler extends the reconcile.Reconciler interface by adding a
// SetupWithManager function that creates a controller in the given manager.
// Every reconciler from the reconcilers package must implement this interface.
type Reconciler interface {
reconcile.Reconciler
SetupWithManager(mgr manager.Manager) error
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*
* Teleport
* Copyright (C) 2023 Gravitational, Inc.
* Copyright (C) 2024 Gravitational, Inc.
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as published by
Expand All @@ -16,7 +16,7 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

package resources
package reconcilers

import (
"context"
Expand All @@ -30,15 +30,19 @@ import (
ctrllog "sigs.k8s.io/controller-runtime/pkg/log"
)

// TODO(hugoShaka) : merge the base reconciler with the generic reocnciler.
// This was a separate struct for backward compatibility but we removed the last
// controller relying directly on the base reconciler.

const (
// DeletionFinalizer is a name of finalizer added to resource's 'finalizers' field
// DeletionFinalizer is a name of finalizer added to Resource's 'finalizers' field
// for tracking deletion events.
DeletionFinalizer = "resources.teleport.dev/deletion"
// AnnotationFlagIgnore is the Kubernetes annotation containing the "ignore" flag.
// When set to true, the operator will not reconcile the CR.
AnnotationFlagIgnore = "teleport.dev/ignore"
// AnnotationFlagKeep is the Kubernetes annotation containing the "keep" flag.
// When set to true, the operator will not delete the Teleport resource if the
// When set to true, the operator will not delete the Teleport Resource if the
// CR is deleted.
AnnotationFlagKeep = "teleport.dev/keep"
)
Expand All @@ -53,26 +57,26 @@ type ResourceBaseReconciler struct {
}

/*
Do will receive an update request and reconcile the resource.
Do will receive an update request and reconcile the Resource.
When an event arrives we must propagate that change into the Teleport cluster.
We have two types of events: update/create and delete.
For creating/updating we check if the resource exists in Teleport
For creating/updating we check if the Resource exists in Teleport
- if it does, we update it
- otherwise we create it
Always using the state of the resource in the cluster as the source of truth.
Always using the state of the Resource in the cluster as the source of truth.
For deleting, the recommendation is to use finalizers.
Finalizers allow us to map an external resource to a kubernetes resource.
So, when we create or update a resource, we add our own finalizer to the kubernetes resource list of finalizers.
Finalizers allow us to map an external Resource to a kubernetes Resource.
So, when we create or update a Resource, we add our own finalizer to the kubernetes Resource list of finalizers.
For a delete event which has our finalizer: the resource is deleted in Teleport.
For a delete event which has our finalizer: the Resource is deleted in Teleport.
If it doesn't have the finalizer, we do nothing.
----
Every time we update a resource in Kubernetes (adding finalizers or the OriginLabel), we end the reconciliation process.
Every time we update a Resource in Kubernetes (adding finalizers or the OriginLabel), we end the reconciliation process.
Afterwards, we receive the request again and we progress to the next step.
This allow us to progress with smaller changes and avoid a long-running reconciliation.
*/
Expand All @@ -85,7 +89,7 @@ func (r ResourceBaseReconciler) Do(ctx context.Context, req ctrl.Request, obj kc
log.Info("not found")
return ctrl.Result{}, nil
}
log.Error(err, "failed to get resource")
log.Error(err, "failed to get Resource")
return ctrl.Result{}, trace.Wrap(err)
}

Expand Down Expand Up @@ -140,7 +144,7 @@ func isIgnored(obj kclient.Object) bool {
return checkAnnotationFlag(obj, AnnotationFlagIgnore, false /* defaults to false */)
}

// isKept checks if the Teleport resource should be kept if the CR is deleted
// isKept checks if the Teleport Resource should be kept if the CR is deleted
func isKept(obj kclient.Object) bool {
return checkAnnotationFlag(obj, AnnotationFlagKeep, false /* defaults to false */)
}
Loading

0 comments on commit 9766732

Please sign in to comment.