diff --git a/integrations/operator/apis/resources/v1/loginrule_types.go b/integrations/operator/apis/resources/v1/loginrule_types.go index affc8af6e30e9..5fc62a7b2d3be 100644 --- a/integrations/operator/apis/resources/v1/loginrule_types.go +++ b/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) +} diff --git a/integrations/operator/controllers/resources/role_controller.go b/integrations/operator/controllers/resources/role_controller.go index 56a26035a0d37..e81b4135c7be2 100644 --- a/integrations/operator/controllers/resources/role_controller.go +++ b/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. diff --git a/integrations/operator/controllers/resources/teleport_reconciler.go b/integrations/operator/controllers/resources/teleport_reconciler.go index de63ef2d4df24..5b1ea28fff821 100644 --- a/integrations/operator/controllers/resources/teleport_reconciler.go +++ b/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. diff --git a/integrations/operator/controllers/resources/user_controller.go b/integrations/operator/controllers/resources/user_controller.go index c50f3dd184213..da6d3f1a27a12 100644 --- a/integrations/operator/controllers/resources/user_controller.go +++ b/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()) } diff --git a/integrations/operator/controllers/resources/user_controller_test.go b/integrations/operator/controllers/resources/user_controller_test.go index 68a1204a7bc58..c050e06f28cf3 100644 --- a/integrations/operator/controllers/resources/user_controller_test.go +++ b/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 }), + ) == "" +}