[v14] integrations/operator: propagate revision + fix tests

integrations/operator/apis/resources/v1/loginrule_types.go

@@ -112,3 +112,11 @@ func (l *LoginRuleResource) SetOrigin(origin string) {
 func (l *LoginRuleResource) GetMetadata() types.Metadata {
 	return *l.LoginRule.Metadata
 }
+
+func (l *LoginRuleResource) GetRevision() string {
+	return l.LoginRule.Metadata.GetRevision()
+}
+
+func (l *LoginRuleResource) SetRevision(rev string) {
+	l.LoginRule.Metadata.SetRevision(rev)
+}

integrations/operator/controllers/resources/role_controller.go

@@ -171,6 +171,9 @@ func (r *RoleReconciler) Upsert(ctx context.Context, obj kclient.Object) error {
 		}
 	}
 
+	if existingResource != nil {
+		teleportResource.SetRevision(existingResource.GetRevision())
+	}
 	r.AddTeleportResourceOrigin(teleportResource)
 
 	// If an error happens we want to put it in status.conditions before returning.

integrations/operator/controllers/resources/teleport_reconciler.go

@@ -35,6 +35,8 @@ type TeleportResource interface {
 	GetName() string
 	SetOrigin(string)
 	GetMetadata() types.Metadata
+	GetRevision() string
+	SetRevision(string)
 }
 
 // TeleportKubernetesResource is a Kubernetes resource representing a Teleport resource
@@ -63,10 +65,15 @@ type TeleportResourceClient[T TeleportResource] interface {
 }
 
 // TeleportResourceMutator can be implemented by TeleportResourceClients
-// to edit a resource before its creation/update. In case of update mutations
-// the existing resource is passed as well.
+// to edit a resource before its creation/update.
 type TeleportResourceMutator[T TeleportResource] interface {
-	Mutate(new, existing T)
+	Mutate(new T)
+}
+
+// TeleportExistingResourceMutator can be implemented by TeleportResourceClients
+// to edit a resource before its update based on the existing one.
+type TeleportExistingResourceMutator[T TeleportResource] interface {
+	MutateExisting(new, existing T)
 }
 
 // NewTeleportResourceReconciler instanciates a TeleportResourceReconciler from a TeleportResourceClient.
@@ -133,14 +140,20 @@ func (r TeleportResourceReconciler[T, K]) Upsert(ctx context.Context, obj kclien
 
 	teleportResource.SetOrigin(types.OriginKubernetes)
 
-	// We apply resource-specific mutations.
-	if mutator, ok := r.resourceClient.(TeleportResourceMutator[T]); ok {
-		mutator.Mutate(teleportResource, existingResource)
-	}
-
 	if !exists {
+		// This is a new resource
+		if mutator, ok := r.resourceClient.(TeleportResourceMutator[T]); ok {
+			mutator.Mutate(teleportResource)
+		}
+
 		err = r.resourceClient.Create(ctx, teleportResource)
 	} else {
+		// This is a resource update, we must propagate the revision
+		teleportResource.SetRevision(existingResource.GetRevision())
+		if mutator, ok := r.resourceClient.(TeleportExistingResourceMutator[T]); ok {
+			mutator.MutateExisting(teleportResource, existingResource)
+		}
+
 		err = r.resourceClient.Update(ctx, teleportResource)
 	}
 	// If an error happens we want to put it in status.conditions before returning.

integrations/operator/controllers/resources/user_controller.go

@@ -73,8 +73,8 @@ func (r userClient) Delete(ctx context.Context, name string) error {
 	return trace.Wrap(teleportClient.DeleteUser(ctx, name))
 }
 
-// Mutate ensures the spec.createdBy property is persisted
-func (r userClient) Mutate(newUser, existingUser types.User) {
+// MutateExisting ensures the spec.createdBy property is persisted
+func (r userClient) MutateExisting(newUser, existingUser types.User) {
 	if existingUser != nil {
 		newUser.SetCreatedBy(existingUser.GetCreatedBy())
 	}

integrations/operator/controllers/resources/user_controller_test.go

@@ -21,9 +21,10 @@ import (
 	"testing"
 	"time"
 
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/gravitational/trace"
 	"github.com/mitchellh/mapstructure"
-	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -347,7 +348,7 @@ func TestUserUpdate(t *testing.T) {
 		require.NoError(t, err)
 
 		// TeleportUser was updated with new roles
-		return assert.ElementsMatch(t, tUser.GetRoles(), []string{"x", "z"})
+		return compareRoles([]string{"x", "z"}, tUser.GetRoles())
 	})
 
 	// Updating the user in K8S
@@ -373,7 +374,7 @@ func TestUserUpdate(t *testing.T) {
 		require.NoError(t, err)
 
 		// TeleportUser updated with new roles
-		return assert.ElementsMatch(t, tUser.GetRoles(), []string{"x", "z", "y"})
+		return compareRoles([]string{"x", "y", "z"}, tUser.GetRoles())
 	})
 	require.Equal(t, setup.OperatorName, tUser.GetCreatedBy().User.Name, "createdBy has not been erased")
 }
@@ -419,3 +420,11 @@ func getUserStatusConditionError(object map[string]interface{}) []metav1.Conditi
 	}
 	return conditionsWithError
 }
+
+func compareRoles(expected, actual []string) bool {
+	return cmp.Diff(
+		expected,
+		actual,
+		cmpopts.SortSlices(func(a, b string) bool { return a < b }),
+	) == ""
+}