diff --git a/integrations/operator/controllers/resources/role_controller.go b/integrations/operator/controllers/resources/role_controller.go index 63876fcf7247d..d1809690652f3 100644 --- a/integrations/operator/controllers/resources/role_controller.go +++ b/integrations/operator/controllers/resources/role_controller.go @@ -20,190 +20,53 @@ package resources import ( "context" - "fmt" "github.com/gravitational/trace" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" - ctrl "sigs.k8s.io/controller-runtime" kclient "sigs.k8s.io/controller-runtime/pkg/client" "github.com/gravitational/teleport/api/client" "github.com/gravitational/teleport/api/types" - v5 "github.com/gravitational/teleport/integrations/operator/apis/resources/v5" + resourcesv5 "github.com/gravitational/teleport/integrations/operator/apis/resources/v5" ) -const teleportRoleKind = "TeleportRole" - -// TODO(for v12): Have the Role controller to use the generic Teleport reconciler -// This means we'll have to move back to a statically typed client. -// This will require removing the crdgen hack, fixing TeleportRole JSON serialization - -var TeleportRoleGVKV5 = schema.GroupVersionKind{ - Group: v5.GroupVersion.Group, - Version: v5.GroupVersion.Version, - Kind: teleportRoleKind, +// roleClient implements TeleportResourceClient and offers CRUD methods needed to reconcile roles +type roleClient struct { + teleportClient *client.Client } -// RoleReconciler reconciles a TeleportRole object -type RoleReconciler struct { - kclient.Client - Scheme *runtime.Scheme - TeleportClient *client.Client +// Get gets the Teleport role of a given name +func (r roleClient) Get(ctx context.Context, name string) (types.Role, error) { + role, err := r.teleportClient.GetRole(ctx, name) + return role, trace.Wrap(err) } -//+kubebuilder:rbac:groups=resources.teleport.dev,resources=roles,verbs=get;list;watch;create;update;patch;delete -//+kubebuilder:rbac:groups=resources.teleport.dev,resources=roles/status,verbs=get;update;patch -//+kubebuilder:rbac:groups=resources.teleport.dev,resources=roles/finalizers,verbs=update - -// Reconcile is part of the main kubernetes reconciliation loop which aims to -// move the current state of the cluster closer to the desired state. -// -// For more details, check Reconcile and its Result here: -// - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.11.0/pkg/reconcile -func (r *RoleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { - // The TeleportRole OpenAPI spec does not validate typing of Label fields like `node_labels`. - // This means we can receive invalid data, by default it won't be unmarshalled properly and will crash the operator. - // To handle this more gracefully we unmarshall first in an unstructured object. - // The unstructured object will be converted later to a typed one, in r.UpsertExternal. - // See `/operator/crdgen/schemagen.go` and https://github.com/gravitational/teleport/issues/15204 for context. - // TODO: (Check how to handle multiple versions) - obj, err := GetUnstructuredObjectFromGVK(TeleportRoleGVKV5) - if err != nil { - return ctrl.Result{}, trace.Wrap(err, "creating object in which the CR will be unmarshalled") - } - return ResourceBaseReconciler{ - Client: r.Client, - DeleteExternal: r.Delete, - UpsertExternal: r.Upsert, - }.Do(ctx, req, obj) +// Create creates a Teleport role +func (r roleClient) Create(ctx context.Context, role types.Role) error { + _, err := r.teleportClient.CreateRole(ctx, role) + return trace.Wrap(err) } -// SetupWithManager sets up the controller with the Manager. -func (r *RoleReconciler) SetupWithManager(mgr ctrl.Manager) error { - // The TeleportRole OpenAPI spec does not validate typing of Label fields like `node_labels`. - // This means we can receive invalid data, by default it won't be unmarshalled properly and will crash the operator - // To handle this more gracefully we unmarshall first in an unstructured object. - // The unstructured object will be converted later to a typed one, in r.UpsertExternal. - // See `/operator/crdgen/schemagen.go` and https://github.com/gravitational/teleport/issues/15204 for context - // TODO: (Check how to handle multiple versions) - obj, err := GetUnstructuredObjectFromGVK(TeleportRoleGVKV5) - if err != nil { - return trace.Wrap(err, "creating the model object for the manager watcher/client") - } - - return ctrl.NewControllerManagedBy(mgr). - For(obj). - WithEventFilter(buildPredicate()). - Complete(r) +// Update updates a Teleport role +func (r roleClient) Update(ctx context.Context, role types.Role) error { + _, err := r.teleportClient.UpdateRole(ctx, role) + return trace.Wrap(err) } -func (r *RoleReconciler) Delete(ctx context.Context, obj kclient.Object) error { - return r.TeleportClient.DeleteRole(ctx, obj.GetName()) +// Delete deletes a Teleport role +func (r roleClient) Delete(ctx context.Context, name string) error { + return trace.Wrap(r.teleportClient.DeleteRole(ctx, name)) } -func (r *RoleReconciler) Upsert(ctx context.Context, obj kclient.Object) error { - // We receive an unstructured object. We convert it to a typed TeleportRole object and gracefully handle errors. - u, ok := obj.(*unstructured.Unstructured) - if !ok { - return fmt.Errorf("failed to convert Object into resource object: %T", obj) +// NewRoleReconciler instantiates a new Kubernetes controller reconciling role resources +func NewRoleReconciler(client kclient.Client, tClient *client.Client) (Reconciler, error) { + roleClient := &roleClient{ + teleportClient: tClient, } - k8sResource := &v5.TeleportRole{} - // If an error happens we want to put it in status.conditions before returning. - err := runtime.DefaultUnstructuredConverter.FromUnstructuredWithValidation( - u.Object, - k8sResource, true, /* returnUnknownFields */ + resourceReconciler, err := NewTeleportResourceReconciler[types.Role, *resourcesv5.TeleportRole]( + client, + roleClient, ) - updateErr := updateStatus(updateStatusConfig{ - ctx: ctx, - client: r.Client, - k8sResource: k8sResource, - condition: getStructureConditionFromError(err), - }) - if err != nil || updateErr != nil { - return trace.NewAggregate(err, updateErr) - } - - // Converting the Kubernetes resource into a Teleport one, checking potential ownership issues. - teleportResource := k8sResource.ToTeleport() - existingResource, err := r.TeleportClient.GetRole(ctx, teleportResource.GetName()) - updateErr = updateStatus(updateStatusConfig{ - ctx: ctx, - client: r.Client, - k8sResource: k8sResource, - condition: getReconciliationConditionFromError(err, true /* ignoreNotFound */), - }) - if err != nil && !trace.IsNotFound(err) || updateErr != nil { - return trace.NewAggregate(err, updateErr) - } - - if err == nil { - // The resource already exists - newOwnershipCondition, isOwned := checkOwnership(existingResource) - if updateErr = updateStatus(updateStatusConfig{ - ctx: ctx, - client: r.Client, - k8sResource: k8sResource, - condition: newOwnershipCondition, - }); updateErr != nil { - return trace.Wrap(updateErr) - } - if !isOwned { - return trace.AlreadyExists("unowned resource '%s' already exists", existingResource.GetName()) - } - } else { - // The resource does not yet exist - if updateErr = updateStatus(updateStatusConfig{ - ctx: ctx, - client: r.Client, - k8sResource: k8sResource, - condition: newResourceCondition, - }); updateErr != nil { - return trace.Wrap(updateErr) - } - } - if existingResource != nil { - teleportResource.SetRevision(existingResource.GetRevision()) - } - r.AddTeleportResourceOrigin(teleportResource) - - // If an error happens we want to put it in status.conditions before returning. - _, err = r.TeleportClient.UpsertRole(ctx, teleportResource) - updateErr = updateStatus(updateStatusConfig{ - ctx: ctx, - client: r.Client, - k8sResource: k8sResource, - condition: getReconciliationConditionFromError(err, false /* ignoreNotFound */), - }) - // We update the status conditions on exit - return trace.NewAggregate(err, updateErr) -} - -func (r *RoleReconciler) AddTeleportResourceOrigin(resource types.Role) { - metadata := resource.GetMetadata() - if metadata.Labels == nil { - metadata.Labels = make(map[string]string) - } - metadata.Labels[types.OriginLabel] = types.OriginKubernetes - resource.SetMetadata(metadata) -} - -func GetUnstructuredObjectFromGVK(gvk schema.GroupVersionKind) (*unstructured.Unstructured, error) { - if gvk.Empty() { - return nil, trace.BadParameter("cannot create an object for an empty GVK, aborting") - } - obj := unstructured.Unstructured{} - obj.SetGroupVersionKind(gvk) - return &obj, nil -} - -func NewRoleReconciler(client kclient.Client, tClient *client.Client) (Reconciler, error) { - return &RoleReconciler{ - Client: client, - Scheme: Scheme, - TeleportClient: tClient, - }, nil + return resourceReconciler, trace.Wrap(err, "building teleport resource reconciler") } diff --git a/integrations/operator/controllers/resources/role_controller_test.go b/integrations/operator/controllers/resources/role_controller_test.go index 3c6f7994c1fbc..36fdd55171a89 100644 --- a/integrations/operator/controllers/resources/role_controller_test.go +++ b/integrations/operator/controllers/resources/role_controller_test.go @@ -29,6 +29,7 @@ import ( "github.com/stretchr/testify/require" kerrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/yaml" "k8s.io/client-go/util/retry" kclient "sigs.k8s.io/controller-runtime/pkg/client" @@ -39,6 +40,12 @@ import ( "github.com/gravitational/teleport/integrations/operator/controllers/resources" ) +var TeleportRoleGVKV5 = schema.GroupVersionKind{ + Group: resourcesv5.GroupVersion.Group, + Version: resourcesv5.GroupVersion.Version, + Kind: "TeleportRole", +} + // When I create or delete a TeleportRole CR in Kubernetes, // the corresponding TeleportRole must be created/deleted in Teleport. func TestRoleCreation(t *testing.T) { @@ -190,7 +197,7 @@ allow: roleName := validRandomResourceName("role-") - obj, err := resources.GetUnstructuredObjectFromGVK(resources.TeleportRoleGVKV5) + obj, err := resources.GetUnstructuredObjectFromGVK(TeleportRoleGVKV5) require.NoError(t, err) obj.Object["spec"] = roleManifest obj.SetName(roleName) @@ -400,58 +407,6 @@ func k8sCreateRole(ctx context.Context, t *testing.T, kc kclient.Client, role *r require.NoError(t, err) } -func TestAddTeleportResourceOriginRole(t *testing.T) { - r := resources.RoleReconciler{} - tests := []struct { - name string - resource types.Role - }{ - { - name: "origin already set correctly", - resource: &types.RoleV6{ - Metadata: types.Metadata{ - Name: "user with correct origin", - Labels: map[string]string{types.OriginLabel: types.OriginKubernetes}, - }, - }, - }, - { - name: "origin already set incorrectly", - resource: &types.RoleV6{ - Metadata: types.Metadata{ - Name: "user with correct origin", - Labels: map[string]string{types.OriginLabel: types.OriginConfigFile}, - }, - }, - }, - { - name: "origin not set", - resource: &types.RoleV6{ - Metadata: types.Metadata{ - Name: "user with correct origin", - Labels: map[string]string{"foo": "bar"}, - }, - }, - }, - { - name: "no labels", - resource: &types.RoleV6{ - Metadata: types.Metadata{ - Name: "user with no labels", - }, - }, - }, - } - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - r.AddTeleportResourceOrigin(tc.resource) - metadata := tc.resource.GetMetadata() - require.Contains(t, metadata.Labels, types.OriginLabel) - require.Equal(t, types.OriginKubernetes, metadata.Labels[types.OriginLabel]) - }) - } -} - func getRoleStatusConditionError(object map[string]interface{}) []metav1.Condition { var conditionsWithError []metav1.Condition var status resourcesv5.TeleportRoleStatus diff --git a/integrations/operator/controllers/resources/utils.go b/integrations/operator/controllers/resources/utils.go index e4a798b0bc771..c8327b0c40a87 100644 --- a/integrations/operator/controllers/resources/utils.go +++ b/integrations/operator/controllers/resources/utils.go @@ -25,6 +25,8 @@ import ( "github.com/gravitational/trace" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" kclient "sigs.k8s.io/controller-runtime/pkg/client" ctrllog "sigs.k8s.io/controller-runtime/pkg/log" @@ -156,3 +158,14 @@ func updateStatus(config updateStatusConfig) error { } return trace.Wrap(statusErr) } + +// GetUnstructuredObjectFromGVK creates a new empty unstructured object with the +// given Group Version and Kind. +func GetUnstructuredObjectFromGVK(gvk schema.GroupVersionKind) (*unstructured.Unstructured, error) { + if gvk.Empty() { + return nil, trace.BadParameter("cannot create an object for an empty GVK, aborting") + } + obj := unstructured.Unstructured{} + obj.SetGroupVersionKind(gvk) + return &obj, nil +}