Merge pull request #12 from datum-cloud/fix/contactgroup-idempotent-creation

refactor: update GetDeterministicContactGroupDisplayName to use UID

Author: JoseSzycho

internal/controller/contactgroup_controller.go

@@ -195,7 +195,7 @@ func (r *ContactGroupController) Reconcile(ctx context.Context, req ctrl.Request
 	// First creation – condition not present yet
 	case existingCond == nil:
 		// Create ContactGroup on email provider
-		emailProviderContactGroup, err := r.EmailProvider.CreateContactGroupIdempotent(ctx, *contactGroup)
+		emailProviderContactGroup, err := r.EmailProvider.CreateContactGroup(ctx, *contactGroup)
 		if err != nil {
 			log.Error(err, "Failed to create ContactGroup on email provider")
 			return ctrl.Result{}, fmt.Errorf("failed to create ContactGroup on email provider: %w", err)

internal/controller/contactgroup_controller_test.go

@@ -149,29 +149,6 @@ var _ = ginkgo.Describe("ContactGroupController", func() {
 			gomega.Expect(cond.ObservedGeneration).To(gomega.Equal(int64(2)))
 		})
 	})
-
-	ginkgo.Context("when the contact group already exists on the provider", func() {
-		ginkgo.It("does not create a new group and uses the existing provider ID", func() {
-			// Pre-populate provider with an existing group with the same display name
-			provider.listOut = emailprovider.ListContactGroupsOutput{
-				ContactGroups: []emailprovider.GetContactGroupOutput{{
-					ContactGroupID: "cg-existing",
-					DisplayName:    emailprovider.GetDeterministicContactGroupDisplayName(group),
-				}},
-			}
-
-			_, err := controller.Reconcile(ctx, ctrl.Request{NamespacedName: types.NamespacedName{Name: group.Name, Namespace: group.Namespace}})
-			gomega.Expect(err).NotTo(gomega.HaveOccurred())
-
-			fetched := &notificationv1.ContactGroup{}
-			gomega.Expect(k8sClient.Get(ctx, types.NamespacedName{Name: group.Name, Namespace: group.Namespace}, fetched)).To(gomega.Succeed())
-
-			cond := meta.FindStatusCondition(fetched.Status.Conditions, notificationv1.ContactGroupReadyCondition)
-			gomega.Expect(cond).NotTo(gomega.BeNil())
-			gomega.Expect(fetched.Status.ProviderID).To(gomega.Equal("cg-existing"))
-			gomega.Expect(provider.createdGroups).To(gomega.BeEmpty()) // no create called
-		})
-	})
 })
 
 var _ = ginkgo.Describe("contactGroupFinalizer", func() {

internal/emailprovider/service.go

@@ -69,22 +69,13 @@ func (s *Service) Send(ctx context.Context,
 	})
 }
 
-// CreateContactGroupIdempotent creates a contact group on the email provider.
-// This is an indempotent operation. If a contact group with the same display name already exists, it will return the existing contact group.
-func (s *Service) CreateContactGroupIdempotent(ctx context.Context, cg notificationmiloapiscomv1alpha1.ContactGroup) (CreateContactGroupOutput, error) {
+// CreateContactGroup creates a contact group on the email provider.
+func (s *Service) CreateContactGroup(ctx context.Context, cg notificationmiloapiscomv1alpha1.ContactGroup) (CreateContactGroupOutput, error) {
 	displayName := GetDeterministicContactGroupDisplayName(&cg)
-	contactGroup, err := s.GetContactGroupByDisplayName(ctx, displayName)
-	if err != nil {
-		if errors.IsNotFound(err) {
-			return s.provider.CreateContactGroup(ctx, CreateContactGroupInput{
-				DisplayName: displayName,
-			})
-		}
-		return CreateContactGroupOutput{}, err
-	}
-	return CreateContactGroupOutput{
-		ContactGroupID: contactGroup.ContactGroupID,
-	}, nil
+	return s.provider.CreateContactGroup(ctx, CreateContactGroupInput{
+		DisplayName: displayName,
+	})
+
 }
 
 // GetContactGroup returns the email provider contact group id of the contact group.

internal/emailprovider/utils.go

@@ -1,14 +1,12 @@
 package emailprovider
 
 import (
-	"fmt"
-
 	notificationmiloapiscomv1alpha1 "go.miloapis.com/milo/pkg/apis/notification/v1alpha1"
 )
 
 // getDeterministicContactGroupName returns a deterministic contact group displayname for the contact group.
 // As the email provider does not support namespaces or custom identifiers, we need to use a deterministic name for the contact group
 // stored in the email provider.
 func GetDeterministicContactGroupDisplayName(contactGroup *notificationmiloapiscomv1alpha1.ContactGroup) string {
-	return fmt.Sprintf("%s-%s-%s", contactGroup.Spec.DisplayName, contactGroup.Spec.Visibility, contactGroup.Namespace)
+	return string(contactGroup.UID)
 }