refactor webhooks controller (#130)

* refactor webhooks controller
coralogix · Aug 8, 2024 · c054465 · c054465
1 parent 2a42173
commit c054465
Show file tree

Hide file tree

Showing 26 changed files with 413 additions and 110 deletions.
diff --git a/controllers/alphacontrollers/outboundwebhook_controller.go b/controllers/alphacontrollers/outboundwebhook_controller.go
@@ -114,56 +114,48 @@ func (r *OutboundWebhookReconciler) SetupWithManager(mgr ctrl.Manager) error {
 func (r *OutboundWebhookReconciler) create(ctx context.Context, log logr.Logger, webhook *coralogixv1alpha1.OutboundWebhook) error {
 	createRequest, err := webhook.ExtractCreateOutboundWebhookRequest()
 	if err != nil {
-		log.Error(err, fmt.Sprintf("Error to extract create-request out of the outbound-webhook -\n%v", webhook))
-		return err
+		return fmt.Errorf("error to extract create-request out of the outbound-webhook -\n%v", webhook)
 	}
 
 	log.V(int(zapcore.DebugLevel)).Info(fmt.Sprintf("Creating outbound-webhook-\n%s", protojson.Format(createRequest)))
 	createResponse, err := r.OutboundWebhooksClient.CreateOutboundWebhook(ctx, createRequest)
 	if err != nil {
-		log.Error(err, fmt.Sprintf("Received an error while creating outbound-webhook -\n%s", protojson.Format(createRequest)))
-		return err
+		return fmt.Errorf("error to create remote outbound-webhook - %s\n%w", protojson.Format(createRequest), err)
 	}
-	log.V(int(zapcore.DebugLevel)).Info(fmt.Sprintf("outbound-webhook was created-\n%s", protojson.Format(createResponse)))
+	log.V(int(zapcore.DebugLevel)).Info(fmt.Sprintf("outbound-webhook was created- %s", protojson.Format(createResponse)))
 
 	webhook.Status = coralogixv1alpha1.OutboundWebhookStatus{
 		ID:                  ptr.To(createResponse.Id.GetValue()),
 		Name:                webhook.Name,
 		OutboundWebhookType: &coralogixv1alpha1.OutboundWebhookTypeStatus{},
 	}
 	if err = r.Status().Update(ctx, webhook); err != nil {
-		log.Error(err, fmt.Sprintf("Error on updating outbound-webhook status -\n%v", webhook))
-		return err
+		return fmt.Errorf("error to update outbound-webhook status -\n%v", webhook)
 	}
 
 	readRequest := &outboundwebhooks.GetOutgoingWebhookRequest{Id: createResponse.Id}
 	log.V(int(zapcore.DebugLevel)).Info(fmt.Sprintf("Getting outbound-webhook -\n%s", protojson.Format(readRequest)))
 	readResponse, err := r.OutboundWebhooksClient.GetOutboundWebhook(ctx, readRequest)
 	if err != nil {
-		log.Error(err, fmt.Sprintf("Received an error while getting outbound-webhook -\n%s", protojson.Format(readRequest)))
-		return err
+		return fmt.Errorf("error to get outbound-webhook -\n%v", webhook)
 	}
 	log.V(int(zapcore.DebugLevel)).Info(fmt.Sprintf("outbound-webhook was read -\n%s", protojson.Format(readResponse)))
 
 	status, err := getOutboundWebhookStatus(readResponse.GetWebhook())
 	if err != nil {
-		log.Error(err, "Received an error while getting outbound-webhook status")
-		return err
+		return fmt.Errorf("error to flatten outbound-webhook -\n%v", webhook)
 	}
 
 	webhook.Status = *status
 	if err = r.Status().Update(ctx, webhook); err != nil {
-		log.Error(err, "Error on updating outbound-webhook status")
-		return err
+		return fmt.Errorf("error to update outbound-webhook status -\n%v", webhook)
 	}
 
 	if !controllerutil.ContainsFinalizer(webhook, outboundWebhookFinalizerName) {
 		controllerutil.AddFinalizer(webhook, outboundWebhookFinalizerName)
 	}
-
 	if err = r.Client.Update(ctx, webhook); err != nil {
-		log.Error(err, "Error on updating outbound-webhook")
-		return err
+		return fmt.Errorf("error to update outbound-webhook -\n%v", webhook)
 	}
 
 	return nil
@@ -363,86 +355,58 @@ func getOutgoingWebhookEmailGroupStatus(group *outboundwebhooks.EmailGroupConfig
 }
 
 func (r *OutboundWebhookReconciler) update(ctx context.Context, log logr.Logger, webhook *coralogixv1alpha1.OutboundWebhook) error {
-	log.V(int(zapcore.DebugLevel)).Info("Getting outbound-webhook from remote", "id", webhook.Status.ID)
-	remoteOutboundWebhook, err := r.OutboundWebhooksClient.GetOutboundWebhook(ctx, &outboundwebhooks.GetOutgoingWebhookRequest{Id: utils.StringPointerToWrapperspbString(webhook.Status.ID)})
-	if err != nil {
-		if status.Code(err) == codes.NotFound {
-			log.Info("outbound-webhook not found on remote, recreating it", "id", webhook.Status.ID)
-			webhook.Status = coralogixv1alpha1.OutboundWebhookStatus{}
-			if err = r.Status().Update(ctx, webhook); err != nil {
-				log.Error(err, "Error on updating outbound-webhook status")
-				return err
-			}
-			return err
-		}
-		log.Error(err, "Error on getting outbound-webhook", "id", webhook.Status.ID)
-		return err
-	}
-	log.V(int(zapcore.DebugLevel)).Info(fmt.Sprintf("outbound-webhook was read\n%s", protojson.Format(remoteOutboundWebhook)))
-
-	status, err := getOutboundWebhookStatus(remoteOutboundWebhook.GetWebhook())
-	if err != nil {
-		log.Error(err, "Error on flattening outbound-webhook")
-		return err
-	}
-
-	if equal, diff := webhook.Spec.DeepEqual(status); equal {
-		return nil
-	} else {
-		log.Info("outbound-webhook is not equal to remote, updating it", "path", diff.Name, "desired", diff.Desired, "actual", diff.Actual)
-	}
-
 	updateReq, err := webhook.ExtractUpdateOutboundWebhookRequest()
 	if err != nil {
-		log.Error(err, "Error to parse update outbound-webhook request")
-		return err
+		return fmt.Errorf("error to parse update outbound-webhook request -\n%v", webhook)
 	}
 
 	log.V(int(zapcore.DebugLevel)).Info(fmt.Sprintf("updating outbound-webhook\n%s", protojson.Format(updateReq)))
 	_, err = r.OutboundWebhooksClient.UpdateOutboundWebhook(ctx, updateReq)
 	if err != nil {
-		log.Error(err, fmt.Sprintf("Error on remote updating outbound-webhook\n%s", protojson.Format(updateReq)))
-		return err
+		if status.Code(err) == codes.NotFound {
+			webhook.Status = coralogixv1alpha1.OutboundWebhookStatus{}
+			if err = r.Status().Update(ctx, webhook); err != nil {
+				return fmt.Errorf("error to update outbound-webhook status -\n%v", webhook)
+			}
+			return fmt.Errorf("outbound-webhook %s not found on remote, recreating it", *webhook.Status.ID)
+		}
+		return fmt.Errorf("error to update outbound-webhook -\n%v", webhook)
 	}
 
 	log.V(int(zapcore.DebugLevel)).Info("Getting outbound-webhook from remote", "id", webhook.Status.ID)
-	remoteOutboundWebhook, err = r.OutboundWebhooksClient.GetOutboundWebhook(ctx,
+	remoteOutboundWebhook, err := r.OutboundWebhooksClient.GetOutboundWebhook(ctx,
 		&outboundwebhooks.GetOutgoingWebhookRequest{
 			Id: utils.StringPointerToWrapperspbString(webhook.Status.ID),
 		},
 	)
 	if err != nil {
-		log.Error(err, "Error on getting outbound-webhook")
-		return err
+		return fmt.Errorf("error to get outbound-webhook -\n%v", webhook)
 	}
 	log.V(int(zapcore.DebugLevel)).Info(fmt.Sprintf("outbound-webhook was read\n%s", protojson.Format(remoteOutboundWebhook)))
 
-	status, err = getOutboundWebhookStatus(remoteOutboundWebhook.GetWebhook())
+	status, err := getOutboundWebhookStatus(remoteOutboundWebhook.GetWebhook())
 	if err != nil {
-		log.Error(err, "Error on flattening outbound-webhook")
-		return err
+		return fmt.Errorf("error to flatten outbound-webhook -\n%v", webhook)
 	}
 	webhook.Status = *status
-	r.Status().Update(ctx, webhook)
+	if err = r.Status().Update(ctx, webhook); err != nil {
+		return fmt.Errorf("error to update outbound-webhook status -\n%v", webhook)
+	}
 
 	return nil
 }
 
 func (r *OutboundWebhookReconciler) delete(ctx context.Context, log logr.Logger, webhook *coralogixv1alpha1.OutboundWebhook) error {
 	log.V(int(zapcore.DebugLevel)).Info("Deleting outbound-webhook from remote", "id", webhook.Status.ID)
-	_, err := r.OutboundWebhooksClient.DeleteOutboundWebhook(ctx, &outboundwebhooks.DeleteOutgoingWebhookRequest{
-		Id: wrapperspb.String(*webhook.Status.ID),
-	})
-	if err != nil && status.Code(err) != codes.NotFound {
-		log.Error(err, "Error on deleting outbound-webhook from remote")
-		return err
+	if _, err := r.OutboundWebhooksClient.DeleteOutboundWebhook(ctx,
+		&outboundwebhooks.DeleteOutgoingWebhookRequest{Id: wrapperspb.String(*webhook.Status.ID)}); err != nil && status.Code(err) != codes.NotFound {
+		return fmt.Errorf("error to delete outbound-webhook -\n%v", webhook)
 	}
+	log.V(int(zapcore.DebugLevel)).Info("outbound-webhook was deleted from remote", "id", webhook.Status.ID)
 
 	controllerutil.RemoveFinalizer(webhook, outboundWebhookFinalizerName)
-	err = r.Update(ctx, webhook)
-	if err != nil {
-		log.Error(err, "Error on updating outbound-webhook after deletion")
-		return err
+	if err := r.Update(ctx, webhook); err != nil {
+		return fmt.Errorf("error to update outbound-webhook -\n%v", webhook)
 	}
 
 	return nil

diff --git a/controllers/alphacontrollers/outboundwebhook_controller_test.go b/controllers/alphacontrollers/outboundwebhook_controller_test.go
@@ -175,22 +175,6 @@ func TestOutboundWebhookUpdate(t *testing.T) {
 						},
 					},
 				}, nil)
-				params.outboundWebhooksClient.EXPECT().GetOutboundWebhook(params.ctx, gomock.Any()).Return(&ow.GetOutgoingWebhookResponse{
-					Webhook: &ow.OutgoingWebhook{
-						Id:   wrapperspb.String("id"),
-						Name: wrapperspb.String("name"),
-						Type: ow.WebhookType_GENERIC,
-						Url:  wrapperspb.String("url"),
-						Config: &ow.OutgoingWebhook_GenericWebhook{
-							GenericWebhook: &ow.GenericWebhookConfig{
-								Uuid:    wrapperspb.String("uuid"),
-								Method:  ow.GenericWebhookConfig_GET,
-								Headers: map[string]string{"key": "value"},
-								Payload: wrapperspb.String("payload"),
-							},
-						},
-					},
-				}, nil)
 				params.outboundWebhooksClient.EXPECT().UpdateOutboundWebhook(params.ctx, gomock.Any()).Return(&ow.UpdateOutgoingWebhookResponse{}, nil)
 				params.outboundWebhooksClient.EXPECT().GetOutboundWebhook(params.ctx, gomock.Any()).Return(&ow.GetOutgoingWebhookResponse{
 					Webhook: &ow.OutgoingWebhook{
@@ -316,3 +300,103 @@ func TestOutboundWebhookUpdate(t *testing.T) {
 		})
 	}
 }
+
+func TestOutboundWebhookDeletion(t *testing.T) {
+	tests := []struct {
+		name            string
+		params          func(params PrepareOutboundWebhooksParams)
+		outboundWebhook coralogixv1alpha1.OutboundWebhook
+		shouldFail      bool
+	}{
+		{
+			name:       "outbound-webhook deletion success",
+			shouldFail: false,
+			params: func(params PrepareOutboundWebhooksParams) {
+				params.outboundWebhooksClient.EXPECT().CreateOutboundWebhook(params.ctx, gomock.Any()).Return(&ow.CreateOutgoingWebhookResponse{Id: wrapperspb.String("id")}, nil)
+				params.outboundWebhooksClient.EXPECT().GetOutboundWebhook(params.ctx, gomock.Any()).Return(&ow.GetOutgoingWebhookResponse{
+					Webhook: &ow.OutgoingWebhook{
+						Id:   wrapperspb.String("id"),
+						Name: wrapperspb.String("name"),
+						Type: ow.WebhookType_GENERIC,
+						Url:  wrapperspb.String("url"),
+						Config: &ow.OutgoingWebhook_GenericWebhook{
+							GenericWebhook: &ow.GenericWebhookConfig{
+								Uuid:    wrapperspb.String("uuid"),
+								Method:  ow.GenericWebhookConfig_GET,
+								Headers: map[string]string{"key": "value"},
+								Payload: wrapperspb.String("payload"),
+							},
+						},
+					},
+				}, nil)
+				params.outboundWebhooksClient.EXPECT().DeleteOutboundWebhook(params.ctx, gomock.Any()).Return(&ow.DeleteOutgoingWebhookResponse{}, nil)
+			},
+			outboundWebhook: coralogixv1alpha1.OutboundWebhook{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "outbound-webhook-deletion-success",
+					Namespace: "default",
+				},
+				Spec: coralogixv1alpha1.OutboundWebhookSpec{
+					Name: "name",
+					OutboundWebhookType: coralogixv1alpha1.OutboundWebhookType{
+						GenericWebhook: &coralogixv1alpha1.GenericWebhook{
+							Url:     "url",
+							Method:  "Get",
+							Headers: map[string]string{"key": "value"},
+							Payload: pointer.String("payload"),
+						},
+					},
+				},
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			controller := gomock.NewController(t)
+			defer controller.Finish()
+
+			outboundWebhooksClient := mock_clientset.NewMockOutboundWebhooksClientInterface(controller)
+
+			ctx, cancel := context.WithCancel(context.Background())
+			defer cancel()
+
+			if tt.params != nil {
+				tt.params(PrepareOutboundWebhooksParams{
+					ctx:                    ctx,
+					outboundWebhooksClient: outboundWebhooksClient,
+				})
+			}
+
+			reconciler, watcher := setupOutboundWebhooksReconciler(t, ctx, outboundWebhooksClient)
+
+			err := reconciler.Client.Create(ctx, &tt.outboundWebhook)
+
+			assert.NoError(t, err)
+
+			<-watcher.ResultChan()
+
+			_, err = reconciler.Reconcile(ctx, ctrl.Request{
+				NamespacedName: types.NamespacedName{
+					Namespace: tt.outboundWebhook.Namespace,
+					Name:      tt.outboundWebhook.Name,
+				},
+			})
+
+			err = reconciler.Client.Delete(ctx, &tt.outboundWebhook)
+
+			assert.NoError(t, err)
+
+			<-watcher.ResultChan()
+
+			_, err = reconciler.Reconcile(ctx, ctrl.Request{
+				NamespacedName: types.NamespacedName{
+					Namespace: tt.outboundWebhook.Namespace,
+					Name:      tt.outboundWebhook.Name,
+				},
+			})
+
+			assert.NoError(t, err)
+		})
+	}
+}
diff --git a/tests/e2e/outboundwebhooks/aws-event-bridge/01-assert.yaml b/tests/e2e/outboundwebhooks/aws-event-bridge/01-assert.yaml
@@ -0,0 +1,28 @@
+apiVersion: coralogix.com/v1alpha1
+kind: OutboundWebhook
+metadata:
+  labels:
+    app.kubernetes.io/name: outboundwebhook
+    app.kubernetes.io/instance: outboundwebhook-sample
+    app.kubernetes.io/part-of: coralogix-operator
+    app.kubernetes.io/managed-by: kustomize
+    app.kubernetes.io/created-by: coralogix-operator
+  name: aws-event-bridge-webhook
+spec:
+  name: aws-event-bridge-webhook
+  outboundWebhookType:
+    awsEventBridge:
+      eventBusArn: "my-updated-event-bus"
+      detail: "{\"updated-key1\": \"updated-value1\", \"updated-key2\": \"updated-value2\"}"
+      detailType: "myUpdatedDetailType"
+      source: "myUpdatedSource"
+      roleName: "arn:aws:iam::123456789012:role/my-role"
+status:
+  name: aws-event-bridge-webhook
+  outboundWebhookType:
+    awsEventBridge:
+      eventBusArn: "my-updated-event-bus"
+      detail: "{\"updated-key1\": \"updated-value1\", \"updated-key2\": \"updated-value2\"}"
+      detailType: "myUpdatedDetailType"
+      source: "myUpdatedSource"
+      roleName: "arn:aws:iam::123456789012:role/my-role"
diff --git a/tests/e2e/outboundwebhooks/aws-event-bridge/01-aws-event-bridge-install.yaml b/tests/e2e/outboundwebhooks/aws-event-bridge/01-aws-event-bridge-install.yaml
@@ -0,0 +1,19 @@
+apiVersion: coralogix.com/v1alpha1
+kind: OutboundWebhook
+metadata:
+  labels:
+    app.kubernetes.io/name: outboundwebhook
+    app.kubernetes.io/instance: outboundwebhook-sample
+    app.kubernetes.io/part-of: coralogix-operator
+    app.kubernetes.io/managed-by: kustomize
+    app.kubernetes.io/created-by: coralogix-operator
+  name: aws-event-bridge-webhook
+spec:
+  name: aws-event-bridge-webhook
+  outboundWebhookType:
+    awsEventBridge:
+      eventBusArn: "my-updated-event-bus"
+      detail: "{\"updated-key1\": \"updated-value1\", \"updated-key2\": \"updated-value2\"}"
+      detailType: "myUpdatedDetailType"
+      source: "myUpdatedSource"
+      roleName: "arn:aws:iam::123456789012:role/my-role"
diff --git a/...dwebhooks/aws-event-bridge/00-delete.yaml → ...dwebhooks/aws-event-bridge/01-delete.yaml b/...dwebhooks/aws-event-bridge/00-delete.yaml → ...dwebhooks/aws-event-bridge/01-delete.yaml
diff --git a/tests/e2e/outboundwebhooks/demisto/01-assert.yaml b/tests/e2e/outboundwebhooks/demisto/01-assert.yaml
@@ -0,0 +1,24 @@
+apiVersion: coralogix.com/v1alpha1
+kind: OutboundWebhook
+metadata:
+  labels:
+    app.kubernetes.io/name: outboundwebhook
+    app.kubernetes.io/instance: outboundwebhook-sample
+    app.kubernetes.io/part-of: coralogix-operator
+    app.kubernetes.io/managed-by: kustomize
+    app.kubernetes.io/created-by: coralogix-operator
+  name: demisto-webhook
+spec:
+  name: demisto-webhook
+  outboundWebhookType:
+    demisto:
+      uuid: "12345678-1234-1234-1234-123456789013"
+      payload: "{\"updated-key1\": \"updated-value1\", \"updated-key2\": \"updated-value2\"}"
+      url: "https://updated-example.com"
+status:
+    name: demisto-webhook
+    outboundWebhookType:
+        demisto:
+          uuid: "12345678-1234-1234-1234-123456789013"
+          payload: "{\"updated-key1\": \"updated-value1\", \"updated-key2\": \"updated-value2\"}"
+          url: "https://updated-example.com"
diff --git a/...e/outboundwebhooks/demisto/00-delete.yaml → ...e/outboundwebhooks/demisto/01-delete.yaml b/...e/outboundwebhooks/demisto/00-delete.yaml → ...e/outboundwebhooks/demisto/01-delete.yaml
diff --git a/tests/e2e/outboundwebhooks/demisto/01-demisto-install.yaml b/tests/e2e/outboundwebhooks/demisto/01-demisto-install.yaml
@@ -0,0 +1,17 @@
+apiVersion: coralogix.com/v1alpha1
+kind: OutboundWebhook
+metadata:
+  labels:
+    app.kubernetes.io/name: outboundwebhook
+    app.kubernetes.io/instance: outboundwebhook-sample
+    app.kubernetes.io/part-of: coralogix-operator
+    app.kubernetes.io/managed-by: kustomize
+    app.kubernetes.io/created-by: coralogix-operator
+  name: demisto-webhook
+spec:
+  name: demisto-webhook
+  outboundWebhookType:
+    demisto:
+      uuid: "12345678-1234-1234-1234-123456789013"
+      payload: "{\"updated-key1\": \"updated-value1\", \"updated-key2\": \"updated-value2\"}"
+      url: "https://updated-example.com"
diff --git a/...hooks/email-group/00-demisto-install.yaml → ...s/email-group/00-email-group-install.yaml b/...hooks/email-group/00-demisto-install.yaml → ...s/email-group/00-email-group-install.yaml