diff --git a/controllers/imagerepository_controller_test.go b/controllers/imagerepository_controller_test.go index aaafe34..9c95d0c 100644 --- a/controllers/imagerepository_controller_test.go +++ b/controllers/imagerepository_controller_test.go @@ -590,28 +590,11 @@ var _ = Describe("Image repository controller", func() { return &quay.Repository{Name: expectedImageName}, nil } - isCreateNotificationInvoked := false - quay.CreateNotificationFunc = func(organization, repository string, notification quay.Notification) (*quay.Notification, error) { - isCreateNotificationInvoked = true - Expect(organization).To(Equal(quay.TestQuayOrg)) - return &quay.Notification{UUID: "uuid"}, nil - } - createImageRepository(imageRepositoryConfig{ - Notifications: []imagerepositoryv1alpha1.Notifications{ - { - Title: "test-notification", - Event: imagerepositoryv1alpha1.NotificationEventRepoPush, - Method: imagerepositoryv1alpha1.NotificationMethodWebhook, - Config: imagerepositoryv1alpha1.NotificationConfig{ - Url: "http://test-url", - }, - }, - }, + Notifications: []imagerepositoryv1alpha1.Notifications{}, }) Eventually(func() bool { return isCreateRepositoryInvoked }, timeout, interval).Should(BeTrue()) - Eventually(func() bool { return isCreateNotificationInvoked }, timeout, interval).Should(BeTrue()) waitImageRepositoryFinalizerOnImageRepository(resourceKey) @@ -626,27 +609,17 @@ var _ = Describe("Image repository controller", func() { Expect(imageRepository.Status.Credentials.PushRobotAccountName).To(HavePrefix(expectedRobotAccountPrefix)) Expect(imageRepository.Status.Credentials.PushSecretName).To(Equal(imageRepository.Name + "-image-push")) Expect(imageRepository.Status.Credentials.GenerationTimestamp).ToNot(BeNil()) - Expect(imageRepository.Status.Notifications).To(HaveLen(1)) + Expect(imageRepository.Status.Notifications).To(HaveLen(0)) }) It("should add notification", func() { - notifications := []quay.Notification{ - { - UUID: "uuid", - Title: "test-notification", - Event: string(imagerepositoryv1alpha1.NotificationEventRepoPush), - Method: string(imagerepositoryv1alpha1.NotificationMethodWebhook), - Config: quay.NotificationConfig{ - Url: "http://test-url", - }, - }, - } + notifications := []quay.Notification{} isCreateNotificationInvoked := false quay.CreateNotificationFunc = func(organization, repository string, notification quay.Notification) (*quay.Notification, error) { notifications = append( notifications, quay.Notification{ - UUID: "uuid2", + UUID: "uuid", Title: notification.Title, Event: notification.Event, Method: notification.Method, @@ -655,7 +628,7 @@ var _ = Describe("Image repository controller", func() { ) isCreateNotificationInvoked = true Expect(organization).To(Equal(quay.TestQuayOrg)) - return &quay.Notification{UUID: "uuid2", Title: notification.Title}, nil + return &quay.Notification{UUID: "uuid", Title: notification.Title}, nil } isGetNotificationsInvoked := false quay.GetNotificationsFunc = func(organization, repository string) ([]quay.Notification, error) { @@ -665,11 +638,11 @@ var _ = Describe("Image repository controller", func() { } newNotification := imagerepositoryv1alpha1.Notifications{ - Title: "test-notification-2", + Title: "test-notification", Event: imagerepositoryv1alpha1.NotificationEventRepoPush, Method: imagerepositoryv1alpha1.NotificationMethodWebhook, Config: imagerepositoryv1alpha1.NotificationConfig{ - Url: "http://test-url-2", + Url: "http://test-url", }, } imageRepository := getImageRepository(resourceKey) @@ -680,28 +653,64 @@ var _ = Describe("Image repository controller", func() { Eventually(func() bool { return isGetNotificationsInvoked }, timeout, interval).Should(BeTrue()) Eventually(func() bool { imageRepository := getImageRepository(resourceKey) - return len(imageRepository.Status.Notifications) == 2 + return len(imageRepository.Status.Notifications) == 1 }, timeout, interval).Should(BeTrue()) }) - It("should delete notification", func() { - notifications := []quay.Notification{ - { - UUID: "uuid", + It("should update notification", func() { + updatedUrl := "http://test-url_new" + isUpdateNotificationInvoked := false + quay.UpdateNotificationFunc = func(organization, repository string, notificationUuid string, notification quay.Notification) (*quay.Notification, error) { + isUpdateNotificationInvoked = true + Expect(organization).To(Equal(quay.TestQuayOrg)) + return &quay.Notification{ + UUID: "uuid_new", Title: "test-notification", Event: string(imagerepositoryv1alpha1.NotificationEventRepoPush), Method: string(imagerepositoryv1alpha1.NotificationMethodWebhook), Config: quay.NotificationConfig{ - Url: "http://test-url", + Url: updatedUrl, }, - }, + }, nil + } + isGetNotificationsInvoked := false + quay.GetNotificationsFunc = func(organization, repository string) ([]quay.Notification, error) { + isGetNotificationsInvoked = true + Expect(organization).To(Equal(quay.TestQuayOrg)) + notifications := []quay.Notification{ + { + UUID: "uuid", + Title: "test-notification", + Event: string(imagerepositoryv1alpha1.NotificationEventRepoPush), + Method: string(imagerepositoryv1alpha1.NotificationMethodWebhook), + Config: quay.NotificationConfig{ + Url: "http://test-url", + }, + }, + } + return notifications, nil + } + imageRepository := getImageRepository(resourceKey) + imageRepository.Spec.Notifications[0].Config.Url = updatedUrl + Expect(k8sClient.Update(ctx, imageRepository)).To(Succeed()) + + Eventually(func() bool { return isUpdateNotificationInvoked }, timeout, interval).Should(BeTrue()) + Eventually(func() bool { return isGetNotificationsInvoked }, timeout, interval).Should(BeTrue()) + Eventually(func() bool { + imageRepository := getImageRepository(resourceKey) + return len(imageRepository.Status.Notifications) == 1 && imageRepository.Spec.Notifications[0].Config.Url == updatedUrl && imageRepository.Status.Notifications[0].UUID == "uuid_new" + }, timeout, interval).Should(BeTrue()) + }) + + It("should delete notification", func() { + notifications := []quay.Notification{ { - UUID: "uuid2", - Title: "test-notification-2", + UUID: "uuid_new", + Title: "test-notification", Event: string(imagerepositoryv1alpha1.NotificationEventRepoPush), Method: string(imagerepositoryv1alpha1.NotificationMethodWebhook), Config: quay.NotificationConfig{ - Url: "http://test-url-2", + Url: "http://test-url_new", }, }, } @@ -720,7 +729,7 @@ var _ = Describe("Image repository controller", func() { } imageRepository := getImageRepository(resourceKey) - Expect(imageRepository.Status.Notifications).To(HaveLen(2)) + Expect(imageRepository.Status.Notifications).To(HaveLen(1)) imageRepository.Spec.Notifications = imageRepository.Spec.Notifications[:len(imageRepository.Spec.Notifications)-1] Expect(k8sClient.Update(ctx, imageRepository)).To(Succeed()) @@ -728,53 +737,46 @@ var _ = Describe("Image repository controller", func() { Eventually(func() bool { return isGetNotificationsInvoked }, timeout, interval).Should(BeTrue()) Eventually(func() bool { imageRepository := getImageRepository(resourceKey) - return len(imageRepository.Status.Notifications) == 1 + return len(imageRepository.Status.Notifications) == 0 }, timeout, interval).Should(BeTrue()) }) - It("should update notification", func() { - updatedUrl := "http://test-url_new" - isUpdateNotificationInvoked := false - quay.UpdateNotificationFunc = func(organization, repository string, notificationUuid string, notification quay.Notification) (*quay.Notification, error) { - isUpdateNotificationInvoked = true + It("should provision image repository with some notifications to create", func() { + deleteImageRepository(resourceKey) + isCreateNotificationInvoked := false + quay.CreateNotificationFunc = func(organization, repository string, notification quay.Notification) (*quay.Notification, error) { + isCreateNotificationInvoked = true Expect(organization).To(Equal(quay.TestQuayOrg)) - return &quay.Notification{ - UUID: "uuid_new", - Title: "test-notification", - Event: string(imagerepositoryv1alpha1.NotificationEventRepoPush), - Method: string(imagerepositoryv1alpha1.NotificationMethodWebhook), - Config: quay.NotificationConfig{ - Url: updatedUrl, - }, - }, nil + return &quay.Notification{UUID: "uuid"}, nil } - isGetNotificationsInvoked := false - quay.GetNotificationsFunc = func(organization, repository string) ([]quay.Notification, error) { - isGetNotificationsInvoked = true - Expect(organization).To(Equal(quay.TestQuayOrg)) - notifications := []quay.Notification{ + createImageRepository(imageRepositoryConfig{ + Notifications: []imagerepositoryv1alpha1.Notifications{ { - UUID: "uuid", Title: "test-notification", - Event: string(imagerepositoryv1alpha1.NotificationEventRepoPush), - Method: string(imagerepositoryv1alpha1.NotificationMethodWebhook), - Config: quay.NotificationConfig{ + Event: imagerepositoryv1alpha1.NotificationEventRepoPush, + Method: imagerepositoryv1alpha1.NotificationMethodWebhook, + Config: imagerepositoryv1alpha1.NotificationConfig{ Url: "http://test-url", }, }, - } - return notifications, nil - } - imageRepository := getImageRepository(resourceKey) - imageRepository.Spec.Notifications[0].Config.Url = updatedUrl - Expect(k8sClient.Update(ctx, imageRepository)).To(Succeed()) + { + Title: "test-notification-2", + Event: imagerepositoryv1alpha1.NotificationEventRepoPush, + Method: imagerepositoryv1alpha1.NotificationMethodWebhook, + Config: imagerepositoryv1alpha1.NotificationConfig{ + Url: "http://test-url-2", + }, + }, + }, + }) + Eventually(func() bool { return isCreateNotificationInvoked }, timeout, interval).Should(BeTrue()) - Eventually(func() bool { return isUpdateNotificationInvoked }, timeout, interval).Should(BeTrue()) - Eventually(func() bool { return isGetNotificationsInvoked }, timeout, interval).Should(BeTrue()) - Eventually(func() bool { - imageRepository := getImageRepository(resourceKey) - return len(imageRepository.Status.Notifications) == 1 && imageRepository.Spec.Notifications[0].Config.Url == updatedUrl && imageRepository.Status.Notifications[0].UUID == "uuid_new" - }, timeout, interval).Should(BeTrue()) + waitImageRepositoryFinalizerOnImageRepository(resourceKey) + + imageRepository := getImageRepository(resourceKey) + Expect(imageRepository.Status.Notifications).To(HaveLen(2)) + Expect(imageRepository.Status.Notifications[0].Title).To(Equal("test-notification")) + Expect(imageRepository.Status.Notifications[1].Title).To(Equal("test-notification-2")) }) }) diff --git a/controllers/imagerepository_notifications.go b/controllers/imagerepository_notifications.go index 44a7491..a048b13 100644 --- a/controllers/imagerepository_notifications.go +++ b/controllers/imagerepository_notifications.go @@ -67,12 +67,14 @@ func (r *ImageRepositoryReconciler) HandleNotifications(ctx context.Context, ima if !r.checkNotificationChangesExists(imageRepository, allNotifications) { return nil } - for index := 0; index < len(imageRepository.Status.Notifications); index++ { - statusNotification := &imageRepository.Status.Notifications[index] // This way we can update the UUID if notification gets updated - existsInSpec := false - for _, notification := range imageRepository.Spec.Notifications { + + log.Info("Starting to handle notifications") + for _, notification := range imageRepository.Spec.Notifications { + existsInStatus := false + for index := 0; index < len(imageRepository.Status.Notifications); index++ { + statusNotification := &imageRepository.Status.Notifications[index] // This way we can update the UUID if notification gets updated if notification.Title == statusNotification.Title { - existsInSpec = true + existsInStatus = true quayNotification, err := r.notificationExistsInQuayByUUID(statusNotification.UUID, imageRepository) if err != nil { return r.handleError(ctx, imageRepository, err, "Couldn't retrieve all Quay notifications") @@ -101,46 +103,50 @@ func (r *ImageRepositoryReconciler) HandleNotifications(ctx context.Context, ima statusNotification.Title = updatedNotification.Title break } - } else { - exists, err := r.notificationExistsInQuayByTitle(notification.Title, imageRepository) - if err != nil { - return r.handleError(ctx, imageRepository, err, "Couldn't retrieve all Quay notifications") - } - if !exists { - // New item in Spec.Notifications: add notification to Quay and Status.Notifications - log.Info("Adding new notification to Quay", "Title", notification.Title, "Event", notification.Event, "Method", notification.Method) - resStatusNotification, err := r.AddNotification(notification, imageRepository) - if err != nil { - log.Error(err, "failed to add notification", "Title", notification.Title, "Event", notification.Event, "Method", notification.Method) - return r.handleError(ctx, imageRepository, err, "Error while adding a notification ("+notification.Title+") to Quay") - } - existsInStatus := false - for _, statusNotificationAux := range imageRepository.Status.Notifications { - if resStatusNotification.UUID == statusNotificationAux.UUID { - existsInStatus = true - break - } - } - if !existsInStatus { - imageRepository.Status.Notifications = append(imageRepository.Status.Notifications, resStatusNotification) - } - } } } - if !existsInSpec { - // Deleted item from Spec.Notifications: delete notification from Quay and Status.Notifications - log.Info("Deleting notification in Quay", "Title", statusNotification.Title, "UUID", statusNotification.UUID) - _, err := r.QuayClient.DeleteNotification( - r.QuayOrganization, - imageRepository.Spec.Image.Name, - statusNotification.UUID) + if !existsInStatus { + log.Info("Adding new notification to Quay", "Title", notification.Title, "Event", notification.Event, "Method", notification.Method) + resStatusNotification, err := r.AddNotification(notification, imageRepository) if err != nil { - log.Error(err, "failed to delete notification", "Title", statusNotification.Title, "UUID", statusNotification.UUID) - return r.handleError(ctx, imageRepository, err, "Error while deleting a notification ("+statusNotification.Title+") to Quay") + log.Error(err, "failed to add notification", "Title", notification.Title, "Event", notification.Event, "Method", notification.Method) + return r.handleError(ctx, imageRepository, err, "Error while adding a notification ("+notification.Title+") to Quay") + } + alreadyInStatus := false + for _, statusNotificationAux := range imageRepository.Status.Notifications { + if resStatusNotification.UUID == statusNotificationAux.UUID { + alreadyInStatus = true + break + } + } + if !alreadyInStatus { + imageRepository.Status.Notifications = append(imageRepository.Status.Notifications, resStatusNotification) + } + } + } + if len(imageRepository.Status.Notifications) > len(imageRepository.Spec.Notifications) { + // There are notifications to be deleted + for index, statusNotification := range imageRepository.Status.Notifications { + existsInSpec := false + for _, notification := range imageRepository.Spec.Notifications { + if notification.Title == statusNotification.Title { + existsInSpec = true + break + } + } + if !existsInSpec { + log.Info("Deleting notification in Quay", "Title", statusNotification.Title, "UUID", statusNotification.UUID) + _, err := r.QuayClient.DeleteNotification( + r.QuayOrganization, + imageRepository.Spec.Image.Name, + statusNotification.UUID) + if err != nil { + log.Error(err, "failed to delete notification", "Title", statusNotification.Title, "UUID", statusNotification.UUID) + return r.handleError(ctx, imageRepository, err, "Error while deleting a notification ("+statusNotification.Title+") to Quay") + } + // Remove notification from CR status + imageRepository.Status.Notifications = append(imageRepository.Status.Notifications[:index], imageRepository.Status.Notifications[index+1:]...) } - // Remove notification from CR status - imageRepository.Status.Notifications = append(imageRepository.Status.Notifications[:index], imageRepository.Status.Notifications[index+1:]...) - index-- } } @@ -193,22 +199,6 @@ func (r *ImageRepositoryReconciler) notificationExistsInQuayByUUID(UUID string, return notification, nil } -func (r *ImageRepositoryReconciler) notificationExistsInQuayByTitle(title string, imageRepository *imagerepositoryv1alpha1.ImageRepository) (bool, error) { - exists := false - allNotifications, err := r.QuayClient.GetNotifications(r.QuayOrganization, imageRepository.Spec.Image.Name) - if err != nil { - return exists, err - } - for _, quayNotification := range allNotifications { - if quayNotification.Title == title { - exists = true - break - } - } - - return exists, nil -} - func isNotificationChanged(notification imagerepositoryv1alpha1.Notifications, quayNotification quay.Notification) bool { return quayNotification.UUID != "" && (quayNotification.Title != notification.Title || quayNotification.Event != string(notification.Event) || quayNotification.Method != string(notification.Method) || quayNotification.Config.Url != notification.Config.Url) }