fixing alert-controller bugs and add tests (#129)

* fixing alert-controller bugs and add tests

apis/coralogix/v1alpha1/alert_types.go

@@ -28,11 +28,11 @@ import (
 	"github.com/coralogix/coralogix-operator/controllers/clientset"
 	alerts "github.com/coralogix/coralogix-operator/controllers/clientset/grpc/alerts/v2"
 	webhooks "github.com/coralogix/coralogix-operator/controllers/clientset/grpc/outbound-webhooks"
+	"github.com/go-logr/logr"
 	"google.golang.org/protobuf/types/known/wrapperspb"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/utils/ptr"
-	"sigs.k8s.io/controller-runtime/pkg/log"
 )
 
 // EDIT THIS FILE!  THIS IS SCAFFOLDING FOR YOU TO OWN!
@@ -155,8 +155,8 @@ type AlertSpec struct {
 	AlertType AlertType `json:"alertType"`
 }
 
-func (a *Alert) ExtractCreateAlertRequest(ctx context.Context) (*alerts.CreateAlertRequest, error) {
-	notificationGroups, err := expandNotificationGroups(ctx, a.Spec.NotificationGroups)
+func (a *Alert) ExtractCreateAlertRequest(ctx context.Context, log logr.Logger) (*alerts.CreateAlertRequest, error) {
+	notificationGroups, err := expandNotificationGroups(ctx, log, a.Spec.NotificationGroups)
 	if err != nil {
 		return nil, err
 	}
@@ -992,10 +992,14 @@ func expandShowInInsight(showInInsight *ShowInInsight) *alerts.ShowInInsight {
 	}
 }
 
-func expandNotificationGroups(ctx context.Context, notificationGroups []NotificationGroup) ([]*alerts.AlertNotificationGroups, error) {
+func expandNotificationGroups(ctx context.Context, log logr.Logger, notificationGroups []NotificationGroup) ([]*alerts.AlertNotificationGroups, error) {
+	webhooksNamesToIds, err := getWebhooksNamesToIds(ctx, log)
+	if err != nil {
+		return nil, err
+	}
 	result := make([]*alerts.AlertNotificationGroups, 0, len(notificationGroups))
 	for i, ng := range notificationGroups {
-		notificationGroup, err := expandNotificationGroup(ctx, ng)
+		notificationGroup, err := expandNotificationGroup(ng, webhooksNamesToIds)
 		if err != nil {
 			return nil, fmt.Errorf("error on notificationGroups[%d] - %s", i, err.Error())
 		}
@@ -1004,9 +1008,22 @@ func expandNotificationGroups(ctx context.Context, notificationGroups []Notifica
 	return result, nil
 }
 
-func expandNotificationGroup(ctx context.Context, notificationGroup NotificationGroup) (*alerts.AlertNotificationGroups, error) {
+func getWebhooksNamesToIds(ctx context.Context, log logr.Logger) (map[string]uint32, error) {
+	webhooksNamesToIds := make(map[string]uint32)
+	log.V(1).Info("Listing all outgoing webhooks")
+	listWebhooksResp, err := WebhooksClient.ListAllOutgoingWebhooks(ctx, &webhooks.ListAllOutgoingWebhooksRequest{})
+	if err != nil {
+		return nil, fmt.Errorf("failed to list all outgoing webhooks %w", err)
+	}
+	for _, webhook := range listWebhooksResp.GetDeployed() {
+		webhooksNamesToIds[webhook.GetName().GetValue()] = webhook.GetExternalId().GetValue()
+	}
+	return webhooksNamesToIds, nil
+}
+
+func expandNotificationGroup(notificationGroup NotificationGroup, webhooksNameToIds map[string]uint32) (*alerts.AlertNotificationGroups, error) {
 	groupFields := utils.StringSliceToWrappedStringSlice(notificationGroup.GroupByFields)
-	notifications, err := expandNotifications(ctx, notificationGroup.Notifications)
+	notifications, err := expandNotifications(notificationGroup.Notifications, webhooksNameToIds)
 	if err != nil {
 		return nil, err
 	}
@@ -1017,19 +1034,19 @@ func expandNotificationGroup(ctx context.Context, notificationGroup Notification
 	}, nil
 }
 
-func expandNotifications(ctx context.Context, notifications []Notification) ([]*alerts.AlertNotification, error) {
+func expandNotifications(notifications []Notification, webhooksNameToIds map[string]uint32) ([]*alerts.AlertNotification, error) {
 	result := make([]*alerts.AlertNotification, 0, len(notifications))
-	for i, n := range notifications {
-		notification, err := expandNotification(ctx, n)
+	for i, notification := range notifications {
+		expandedNotification, err := expandNotification(notification, webhooksNameToIds)
 		if err != nil {
 			return nil, fmt.Errorf("error on notifications[%d] - %s", i, err.Error())
 		}
-		result = append(result, notification)
+		result = append(result, expandedNotification)
 	}
 	return result, nil
 }
 
-func expandNotification(ctx context.Context, notification Notification) (*alerts.AlertNotification, error) {
+func expandNotification(notification Notification, webhooksNameToIds map[string]uint32) (*alerts.AlertNotification, error) {
 	retriggeringPeriodSeconds := wrapperspb.UInt32(uint32(60 * notification.RetriggeringPeriodMinutes))
 	notifyOn := AlertSchemaNotifyOnToProtoNotifyOn[notification.NotifyOn]
 
@@ -1039,10 +1056,7 @@ func expandNotification(ctx context.Context, notification Notification) (*alerts
 	}
 
 	if integrationName := notification.IntegrationName; integrationName != nil {
-		integrationID, err := searchIntegrationID(ctx, *integrationName)
-		if err != nil {
-			return nil, err
-		}
+		integrationID, _ := webhooksNameToIds[*integrationName]
 		result.IntegrationType = &alerts.AlertNotification_IntegrationId{
 			IntegrationId: wrapperspb.UInt32(integrationID),
 		}
@@ -1066,23 +1080,6 @@ func expandNotification(ctx context.Context, notification Notification) (*alerts
 	return result, nil
 }
 
-func searchIntegrationID(ctx context.Context, name string) (uint32, error) {
-	log := log.FromContext(ctx)
-	log.V(1).Info("Listing all outgoing webhooks")
-	listWebhooksResp, err := WebhooksClient.ListAllOutgoingWebhooks(ctx, &webhooks.ListAllOutgoingWebhooksRequest{})
-	if err != nil {
-		log.Error(err, "Failed to list all outgoing webhooks")
-		return 0, err
-	}
-
-	for _, webhook := range listWebhooksResp.GetDeployed() {
-		if webhook.GetName().GetValue() == name {
-			return webhook.GetExternalId().GetValue(), nil
-		}
-	}
-	return 0, fmt.Errorf("integration with name %s not found", name)
-}
-
 func (in *AlertSpec) DeepEqual(actualAlert *AlertStatus) (bool, utils.Diff) {
 	if actualName := actualAlert.Name; actualName != in.Name {
 		return false, utils.Diff{
@@ -1257,7 +1254,7 @@ func getNotificationsByIntegrationNameMap(notifications []Notification) map[stri
 	return notificationsByIntegrationName
 }
 
-func (in *AlertSpec) ExtractUpdateAlertRequest(ctx context.Context, id string) (*alerts.UpdateAlertByUniqueIdRequest, error) {
+func (in *AlertSpec) ExtractUpdateAlertRequest(ctx context.Context, log logr.Logger, id string) (*alerts.UpdateAlertByUniqueIdRequest, error) {
 	uniqueIdentifier := wrapperspb.String(id)
 	enabled := wrapperspb.Bool(in.Active)
 	name := wrapperspb.String(in.Name)
@@ -1266,7 +1263,7 @@ func (in *AlertSpec) ExtractUpdateAlertRequest(ctx context.Context, id string) (
 	metaLabels := expandMetaLabels(in.Labels)
 	expirationDate := expandExpirationDate(in.ExpirationDate)
 	showInInsight := expandShowInInsight(in.ShowInInsight)
-	notificationGroups, err := expandNotificationGroups(ctx, in.NotificationGroups)
+	notificationGroups, err := expandNotificationGroups(ctx, log, in.NotificationGroups)
 	if err != nil {
 		return nil, err
 	}

config/rbac/service_account.yaml

@@ -3,7 +3,7 @@ kind: ServiceAccount
 metadata:
   labels:
     app.kubernetes.io/name: serviceaccount
-    app.kuberentes.io/instance: controller-manager
+    app.kubernetes.io/instance: controller-manager
     app.kubernetes.io/component: rbac
     app.kubernetes.io/created-by: coralogix-operator
     app.kubernetes.io/part-of: coralogix-operator

config/samples/alerts/flow_alert_example.yaml

@@ -5,7 +5,7 @@ metadata:
     app.kubernetes.io/name: alert
     app.kubernetes.io/instance: alert-sample
     app.kubernetes.io/part-of: coralogix-operator
-    app.kuberentes.io/managed-by: kustomize
+    app.kubernetes.io/managed-by: kustomize
     app.kubernetes.io/created-by: coralogix-operator
   name: flow-alert-example
 spec:

config/samples/alerts/lucene_alert.yaml

@@ -5,7 +5,7 @@ metadata:
     app.kubernetes.io/name: alert
     app.kubernetes.io/instance: alert-sample
     app.kubernetes.io/part-of: coralogix-operator
-    app.kuberentes.io/managed-by: kustomize
+    app.kubernetes.io/managed-by: kustomize
     app.kubernetes.io/created-by: coralogix-operator
   name: lucene-alert-example
 spec:

config/samples/alerts/new_value_alert.yaml

@@ -5,7 +5,7 @@ metadata:
     app.kubernetes.io/name: alert
     app.kubernetes.io/instance: alert-sample
     app.kubernetes.io/part-of: coralogix-operator
-    app.kuberentes.io/managed-by: kustomize
+    app.kubernetes.io/managed-by: kustomize
     app.kubernetes.io/created-by: coralogix-operator
   name: new-value-alert-example
 spec:

config/samples/alerts/promql_alert.yaml

@@ -5,7 +5,7 @@ metadata:
     app.kubernetes.io/name: alert
     app.kubernetes.io/instance: alert-sample
     app.kubernetes.io/part-of: coralogix-operator
-    app.kuberentes.io/managed-by: kustomize
+    app.kubernetes.io/managed-by: kustomize
     app.kubernetes.io/created-by: coralogix-operator
   name: promql-alert-example
 spec:

config/samples/alerts/ratio_alert.yaml

@@ -5,7 +5,7 @@ metadata:
     app.kubernetes.io/name: alert
     app.kubernetes.io/instance: alert-sample
     app.kubernetes.io/part-of: coralogix-operator
-    app.kuberentes.io/managed-by: kustomize
+    app.kubernetes.io/managed-by: kustomize
     app.kubernetes.io/created-by: coralogix-operator
   name: ratio-alert-example
 spec:

config/samples/alerts/standard_alert.yaml

@@ -5,7 +5,7 @@ metadata:
     app.kubernetes.io/name: alert
     app.kubernetes.io/instance: alert-sample
     app.kubernetes.io/part-of: coralogix-operator
-    app.kuberentes.io/managed-by: kustomize
+    app.kubernetes.io/managed-by: kustomize
     app.kubernetes.io/created-by: coralogix-operator
   name: standard-alert-example
 spec:

config/samples/alerts/time_relative_alert.yaml

@@ -5,7 +5,7 @@ metadata:
     app.kubernetes.io/name: alert
     app.kubernetes.io/instance: alert-sample
     app.kubernetes.io/part-of: coralogix-operator
-    app.kuberentes.io/managed-by: kustomize
+    app.kubernetes.io/managed-by: kustomize
     app.kubernetes.io/created-by: coralogix-operator
   name: time-relative-example
 spec:

config/samples/alerts/tracing_alert.yaml

@@ -5,7 +5,7 @@ metadata:
     app.kubernetes.io/name: alert
     app.kubernetes.io/instance: alert-sample
     app.kubernetes.io/part-of: coralogix-operator
-    app.kuberentes.io/managed-by: kustomize
+    app.kubernetes.io/managed-by: kustomize
     app.kubernetes.io/created-by: coralogix-operator
   name: tracing-alert-example
 spec:

config/samples/alerts/unique_count_alert.yaml

@@ -5,7 +5,7 @@ metadata:
     app.kubernetes.io/name: alert
     app.kubernetes.io/instance: alert-sample
     app.kubernetes.io/part-of: coralogix-operator
-    app.kuberentes.io/managed-by: kustomize
+    app.kubernetes.io/managed-by: kustomize
     app.kubernetes.io/created-by: coralogix-operator
   name: unique-count-alert-example
 spec:

config/samples/rulegroups/block.yaml

@@ -5,7 +5,7 @@ metadata:
     app.kubernetes.io/name: rulegroup
     app.kubernetes.io/instance: rulegroup-sample
     app.kubernetes.io/part-of: coralogix-operator
-    app.kuberentes.io/managed-by: kustomize
+    app.kubernetes.io/managed-by: kustomize
     app.kubernetes.io/created-by: coralogix-operator
   name: block-rule
 spec:

config/samples/rulegroups/extract.yaml

@@ -5,7 +5,7 @@ metadata:
     app.kubernetes.io/name: rulegroup
     app.kubernetes.io/instance: rulegroup-sample
     app.kubernetes.io/part-of: coralogix-operator
-    app.kuberentes.io/managed-by: kustomize
+    app.kubernetes.io/managed-by: kustomize
     app.kubernetes.io/created-by: coralogix-operator
   name: extract-rule
 spec:

config/samples/rulegroups/extract_timestamp.yaml

@@ -5,7 +5,7 @@ metadata:
     app.kubernetes.io/name: rulegroup
     app.kubernetes.io/instance: rulegroup-sample
     app.kubernetes.io/part-of: coralogix-operator
-    app.kuberentes.io/managed-by: kustomize
+    app.kubernetes.io/managed-by: kustomize
     app.kubernetes.io/created-by: coralogix-operator
   name: extract-timestamp-rule
 spec:

config/samples/rulegroups/json_extract.yaml

@@ -5,7 +5,7 @@ metadata:
     app.kubernetes.io/name: rulegroup
     app.kubernetes.io/instance: rulegroup-sample
     app.kubernetes.io/part-of: coralogix-operator
-    app.kuberentes.io/managed-by: kustomize
+    app.kubernetes.io/managed-by: kustomize
     app.kubernetes.io/created-by: coralogix-operator
   name: json-extract-rule
 spec:

config/samples/rulegroups/mixed_rulegroup.yaml

@@ -5,7 +5,7 @@ metadata:
     app.kubernetes.io/name: rulegroup
     app.kubernetes.io/instance: rulegroup-sample
     app.kubernetes.io/part-of: coralogix-operator
-    app.kuberentes.io/managed-by: kustomize
+    app.kubernetes.io/managed-by: kustomize
     app.kubernetes.io/created-by: coralogix-operator
   name: mixed-rulegroupmixed-rulegroup
 spec:

config/samples/rulegroups/parse.yaml

@@ -5,7 +5,7 @@ metadata:
     app.kubernetes.io/name: rulegroup
     app.kubernetes.io/instance: rulegroup-sample
     app.kubernetes.io/part-of: coralogix-operator
-    app.kuberentes.io/managed-by: kustomize
+    app.kubernetes.io/managed-by: kustomize
     app.kubernetes.io/created-by: coralogix-operator
   name: parsing-rule
 spec:

config/samples/rulegroups/parse_json_field.yaml

@@ -5,7 +5,7 @@ metadata:
     app.kubernetes.io/name: rulegroup
     app.kubernetes.io/instance: rulegroup-sample
     app.kubernetes.io/part-of: coralogix-operator
-    app.kuberentes.io/managed-by: kustomize
+    app.kubernetes.io/managed-by: kustomize
     app.kubernetes.io/created-by: coralogix-operator
   name: parsing-json-field-rule
 spec:

config/samples/rulegroups/remove_fields.yaml

@@ -5,7 +5,7 @@ metadata:
     app.kubernetes.io/name: rulegroup
     app.kubernetes.io/instance: rulegroup-sample
     app.kubernetes.io/part-of: coralogix-operator
-    app.kuberentes.io/managed-by: kustomize
+    app.kubernetes.io/managed-by: kustomize
     app.kubernetes.io/created-by: coralogix-operator
   name: remove-fields
 spec:

config/samples/rulegroups/replace.yaml

@@ -5,7 +5,7 @@ metadata:
     app.kubernetes.io/name: rulegroup
     app.kubernetes.io/instance: rulegroup-sample
     app.kubernetes.io/part-of: coralogix-operator
-    app.kuberentes.io/managed-by: kustomize
+    app.kubernetes.io/managed-by: kustomize
     app.kubernetes.io/created-by: coralogix-operator
   name: replace-rule
 spec:

config/samples/rulegroups/stringify_json_field.yaml

@@ -5,7 +5,7 @@ metadata:
     app.kubernetes.io/name: rulegroup
     app.kubernetes.io/instance: rulegroup-sample
     app.kubernetes.io/part-of: coralogix-operator
-    app.kuberentes.io/managed-by: kustomize
+    app.kubernetes.io/managed-by: kustomize
     app.kubernetes.io/created-by: coralogix-operator
   name: json-stringify-rule
 spec: