Quay notification support for ImageRepository #112

Allda · 2024-05-17T11:14:08Z

The ImageRepository CR is extended to allow defining a Quay notifications. The notification schema is based on Quay API schema and supports webhook and email configuration.

Quay notification is created using the rest API client and added to a CR status field.

JIRA: ISV-4756

Example:

apiVersion: appstudio.redhat.com/v1alpha1
kind: ImageRepository
metadata:
  name: imagerepository-sample
  namespace: image-controller-system
spec:
  notifications:
    - title: MyNotif
      event: repo_push
      method: webhook 
      config: 
        url: https://foo.com

Update CR after a notification was set:

apiVersion: appstudio.redhat.com/v1alpha1
kind: ImageRepository
metadata:
  annotations:
    kubectl.kubernetes.io/last-applied-configuration: |
      {"apiVersion":"appstudio.redhat.com/v1alpha1","kind":"ImageRepository","metadata":{"annotations":{},"name":"imagerepository-sample-10","namespace":"image-controller-system"},"spec":{"notifications":[{"config":{"url":"https://foo.com"},"event":"repo_push","method":"webhook","title":"MyNofif"}]}}
  resourceVersion: '1822153325'
  name: imagerepository-sample-10
  uid: cc5e5c92-de44-4661-aceb-f4c5389037d3
  creationTimestamp: '2024-05-16T12:21:55Z'
  generation: 3
  namespace: image-controller-system
  finalizers:
    - appstudio.openshift.io/image-repository
spec:
  image:
    name: image-controller-system/imagerepository-sample-10
    visibility: public
  notifications:
    - config:
        url: 'https://foo.com'
      event: repo_push
      method: webhook
      title: MyNofif
    - config:
        url: 'https://examples.com'
      event: repo_push
      method: webhook
      title: MyNofif-2
status:
  credentials:
    generationTimestamp: '2024-05-16T12:21:56Z'
    push-remote-secret: imagerepository-sample-10-image-push
    push-robot-account: image_controller_system_imagerepository_sample_10_8d83bb7277
    push-secret: imagerepository-sample-10-image-push
  image:
    url: quay.io/isv_org_123/image-controller-system/imagerepository-sample-10
    visibility: public
  notifications:
    - title: MyNofif
      uuid: 68eaeac8-c0f8-46c1-8a86-8641c19a0ccd
  state: ready

openshift-ci · 2024-05-17T11:14:19Z

Hi @Allda. Thanks for your PR.

I'm waiting for a konflux-ci member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

mmorhun

The implementation is missing removal of the notifications. As a user I can edit the CR and add/remove the notifications.
Will quay automatically cleanup notifications on the image repository deletion?

mmorhun · 2024-05-20T13:57:27Z

go.mod

@@ -1,6 +1,8 @@
 module github.com/redhat-appstudio/image-controller

-go 1.20
+go 1.21


Is there any reason to do the update?

I wasn't able to run in locally with the previous version and since the new version was already available I bumped it up.

api/v1alpha1/imagerepository_types.go

controllers/imagerepository_controller.go

mmorhun · 2024-05-20T14:07:27Z

controllers/imagerepository_controller.go

+
+	for _, notification := range imageRepository.Spec.Notifications {
+		log.Info("Creating notification in Quay", "Title", notification.Title, "Event", notification.Event, "Method", notification.Method)
+		quayNotification, err := r.QuayClient.CreateNotification(


It will return an error if this function is called second time.
See how, for example, CreateRobotAccount is implemented.

I updated code to not create a duplicated entries if a notification already exists.

Allda · 2024-05-22T14:16:57Z

The implementation is missing removal of the notifications. As a user I can edit the CR and add/remove the notifications. Will quay automatically cleanup notifications on the image repository deletion?

We currently don't have a planned workflow for the removal event. But yeah once the repo is removed all notifications are removed as well.

pkg/quay/quay_debug_test.go

mmorhun · 2024-05-27T13:53:08Z

controllers/imagerepository_controller.go

+func (r *ImageRepositoryReconciler) AddNotifications(ctx context.Context, imageRepository *imagerepositoryv1alpha1.ImageRepository) ([]imagerepositoryv1alpha1.NotificationStatus, error) {
+	log := ctrllog.FromContext(ctx).WithName("ConfigureNotifications")
+
+	if imageRepository.Spec.Notifications == nil {


I think we should check it before calling this method.

Check what?

imageRepository.Spec.Notifications?

aha, I didn't want to add extra complexity to the parent function and instead return early from this one in case notifications are not set.

controllers/imagerepository_controller.go

mmorhun · 2024-05-27T14:16:34Z

controllers/imagerepository_controller.go

@@ -319,6 +364,11 @@ func (r *ImageRepositoryReconciler) ProvisionImageRepository(ctx context.Context
 		}
 	}

+	var notificationStatus []imagerepositoryv1alpha1.NotificationStatus
+	if notificationStatus, err = r.AddNotifications(ctx, imageRepository); err != nil {


Do we want to add the notifications on image repository creation only or at any point?

For the current usecase that we have it is fine to create a notification on CR creation.

It has to be implemented in follow up PR.

mmorhun · 2024-05-27T14:44:12Z

/retest

mmorhun

To have a complete feature we have to implement editing and removing of the notifications.
But to not to block the functionality die date, we can merge this and improve later.

You can ignore EC failure, it's not related to the changes in this PR.

controllers/suite_util_test.go

The ImageRepository CR is extended to allow defining a Quay notifications. The notification schema is based on Quay API schema and supports webhook and email configuration. Quay notification is created using the rest API client and added to a CR status field. JIRA: ISV-4756 Example: apiVersion: appstudio.redhat.com/v1alpha1 kind: ImageRepository metadata: name: imagerepository-sample namespace: image-controller-system spec: notifications: - title: MyNotif event: repo_push method: webhook config: url: https://foo.com Signed-off-by: Ales Raszka <[email protected]>

mmorhun · 2024-05-29T08:51:24Z

/ok-to-test

openshift-ci bot requested review from mmorhun and psturc May 17, 2024 11:14

Allda marked this pull request as draft May 17, 2024 11:14

openshift-ci bot added the do-not-merge/work-in-progress label May 17, 2024

openshift-ci bot added the needs-ok-to-test label May 17, 2024

Allda marked this pull request as ready for review May 20, 2024 11:04

openshift-ci bot removed the do-not-merge/work-in-progress label May 20, 2024

mmorhun reviewed May 20, 2024

View reviewed changes

Allda force-pushed the ISV-4756 branch from a21483f to b9c6147 Compare May 22, 2024 14:14

Allda force-pushed the ISV-4756 branch from b9c6147 to 3275b7d Compare May 23, 2024 05:49

Allda requested a review from mmorhun May 27, 2024 08:24

mmorhun reviewed May 27, 2024

View reviewed changes

mmorhun approved these changes May 28, 2024

View reviewed changes

controllers/suite_util_test.go Outdated Show resolved Hide resolved

openshift-ci bot assigned mmorhun May 28, 2024

openshift-ci bot added the lgtm label May 28, 2024

Allda force-pushed the ISV-4756 branch from 3275b7d to 877f980 Compare May 29, 2024 08:09

openshift-ci bot removed the lgtm label May 29, 2024

mmorhun approved these changes May 29, 2024

View reviewed changes

openshift-ci bot added the lgtm label May 29, 2024

openshift-ci bot added ok-to-test and removed needs-ok-to-test labels May 29, 2024

mmorhun merged commit 4577b7f into konflux-ci:main May 29, 2024
9 of 11 checks passed

mavaras mentioned this pull request Jun 7, 2024

Quay notification add, update and delete support for ImageRepository #122

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Quay notification support for ImageRepository #112

Quay notification support for ImageRepository #112

Allda commented May 17, 2024 •

edited

Loading

openshift-ci bot commented May 17, 2024

mmorhun left a comment

mmorhun May 20, 2024

Allda May 22, 2024

mmorhun May 20, 2024

Allda May 22, 2024

Allda commented May 22, 2024

mmorhun May 27, 2024

Allda May 28, 2024

mmorhun May 28, 2024

Allda May 29, 2024

mmorhun May 27, 2024

Allda May 28, 2024

mmorhun May 29, 2024

mmorhun commented May 27, 2024

mmorhun left a comment •

edited

Loading

mmorhun commented May 29, 2024

Quay notification support for ImageRepository #112

Quay notification support for ImageRepository #112

Conversation

Allda commented May 17, 2024 • edited Loading

openshift-ci bot commented May 17, 2024

mmorhun left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Allda commented May 22, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmorhun commented May 27, 2024

mmorhun left a comment • edited Loading

Choose a reason for hiding this comment

mmorhun commented May 29, 2024

Allda commented May 17, 2024 •

edited

Loading

mmorhun left a comment •

edited

Loading