diff --git a/internal/webhooks/notification/v1alpha1/contact_webhook.go b/internal/webhooks/notification/v1alpha1/contact_webhook.go index 7a00a16d..c7c77069 100644 --- a/internal/webhooks/notification/v1alpha1/contact_webhook.go +++ b/internal/webhooks/notification/v1alpha1/contact_webhook.go @@ -27,6 +27,7 @@ var acceptedResourceManagerKinds = []string{"Organization", "Project"} var acceptedAPIGroups = []string{"iam.miloapis.com", "resourcemanager.miloapis.com"} const contactSpecKey = "contactSpecKey" +const contactEmailKey = "contactEmailKey" // buildContactSpecKey returns the composite key used for indexing contact spec (subjectRef + email) func buildContactSpecKey(contact notificationv1alpha1.Contact) string { @@ -52,6 +53,14 @@ func SetupContactWebhooksWithManager(mgr ctrl.Manager) error { return fmt.Errorf("failed to index contactSpecKey: %w", err) } + // Index for contact email + if err := mgr.GetFieldIndexer().IndexField(context.Background(), ¬ificationv1alpha1.Contact{}, contactEmailKey, func(rawObj client.Object) []string { + contact := rawObj.(*notificationv1alpha1.Contact) + return []string{contact.Spec.Email} + }); err != nil { + return fmt.Errorf("failed to index contactEmailKey: %w", err) + } + return ctrl.NewWebhookManagedBy(mgr). For(¬ificationv1alpha1.Contact{}). WithDefaulter(&ContactMutator{ @@ -115,6 +124,18 @@ func (v *ContactValidator) ValidateCreate(ctx context.Context, obj runtime.Objec errs = append(errs, field.Invalid(field.NewPath("spec", "email"), contact.Spec.Email, fmt.Sprintf("invalid email address: %v", err))) } + // Validate that a contact with the same email does not already exist in any namespace + var existingByEmail notificationv1alpha1.ContactList + if err := v.Client.List(ctx, &existingByEmail, + client.MatchingFields{contactEmailKey: contact.Spec.Email}); err != nil { + return nil, errors.NewInternalError(fmt.Errorf("failed to list contacts by email: %w", err)) + } + if len(existingByEmail.Items) > 0 { + dup := field.Duplicate(field.NewPath("spec", "email"), contact.Spec.Email) + dup.Detail = fmt.Sprintf("a Contact named %s already has this email in the cluster", existingByEmail.Items[0].Name) + errs = append(errs, dup) + } + // Validate that a contact with the same subject and email does not already exist var existing notificationv1alpha1.ContactList if err := v.Client.List(ctx, &existing, @@ -123,7 +144,7 @@ func (v *ContactValidator) ValidateCreate(ctx context.Context, obj runtime.Objec } if len(existing.Items) > 0 { dup := field.Duplicate(field.NewPath("spec"), contact.Spec) - dup.Detail = fmt.Sprintf("a Contact named %s already has this subject and email in the same Contact namespace", existing.Items[0].Name) + dup.Detail = fmt.Sprintf("a Contact named %s already has this subject and email in the same Contact cluster", existing.Items[0].Name) errs = append(errs, dup) } @@ -245,6 +266,22 @@ func (v *ContactValidator) ValidateUpdate(ctx context.Context, oldObj, newObj ru if _, err := mail.ParseAddress(contactNew.Spec.Email); err != nil { errs = append(errs, field.Invalid(field.NewPath("spec", "email"), contactNew.Spec.Email, fmt.Sprintf("invalid email address: %v", err))) } + + // Validate that another contact (different object) with the same email does not already exist in any namespace + var existingByEmail notificationv1alpha1.ContactList + if err := v.Client.List(ctx, &existingByEmail, + client.MatchingFields{contactEmailKey: contactNew.Spec.Email}); err != nil { + return nil, errors.NewInternalError(fmt.Errorf("failed to list contacts by email: %w", err)) + } + for _, c := range existingByEmail.Items { + if c.Name == contactNew.Name && c.Namespace == contactNew.Namespace { + continue // skip the object being updated + } + dup := field.Duplicate(field.NewPath("spec", "email"), contactNew.Spec.Email) + dup.Detail = fmt.Sprintf("a Contact named %s already has this email in the same Contact cluster", c.Name) + errs = append(errs, dup) + break + } } // Validate that another contact (different object) with the same subject and email does not already exist diff --git a/internal/webhooks/notification/v1alpha1/contact_webhook_test.go b/internal/webhooks/notification/v1alpha1/contact_webhook_test.go index d6c74ca4..cfa5fcfb 100644 --- a/internal/webhooks/notification/v1alpha1/contact_webhook_test.go +++ b/internal/webhooks/notification/v1alpha1/contact_webhook_test.go @@ -314,6 +314,51 @@ func TestContactValidator_ValidateCreate(t *testing.T) { expectError: true, errorContains: "already has this subject and email", }, + "duplicate email across namespaces": { + // An existing newsletter contact with the same email in a different namespace + // should make the new user-referenced contact invalid + contact: ¬ificationv1alpha1.Contact{ + ObjectMeta: metav1.ObjectMeta{ + Name: "user-contact-ns2", + Namespace: "ns-two", + }, + Spec: notificationv1alpha1.ContactSpec{ + GivenName: "Dup", + FamilyName: "User", + Email: "global@example.com", // same email as the seeded one + SubjectRef: ¬ificationv1alpha1.SubjectReference{ + APIGroup: "iam.miloapis.com", + Kind: "User", + Name: "test-user", + }, + }, + }, + seedObjects: []client.Object{ + // Existing newsletter contact in a different namespace + ¬ificationv1alpha1.Contact{ + ObjectMeta: metav1.ObjectMeta{ + Name: "newsletter-ns1", + Namespace: "ns-one", + }, + Spec: notificationv1alpha1.ContactSpec{ + GivenName: "News", + FamilyName: "Letter", + Email: "global@example.com", + }, + }, + // Backing User for the user-referenced contact + &iamv1alpha1.User{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-user", + }, + Spec: iamv1alpha1.UserSpec{ + Email: "user@example.com", + }, + }, + }, + expectError: true, + errorContains: "already has this email in the cluster", + }, "duplicate user contact": { contact: ¬ificationv1alpha1.Contact{ ObjectMeta: metav1.ObjectMeta{Name: "user-dup"}, @@ -357,6 +402,11 @@ func TestContactValidator_ValidateCreate(t *testing.T) { contact := obj.(*notificationv1alpha1.Contact) return []string{buildContactSpecKey(*contact)} }) + // Register the email index so email-based MatchingFields queries work like the real manager + builder = builder.WithIndex(¬ificationv1alpha1.Contact{}, contactEmailKey, func(obj client.Object) []string { + contact := obj.(*notificationv1alpha1.Contact) + return []string{contact.Spec.Email} + }) if len(tt.seedObjects) > 0 { builder = builder.WithObjects(tt.seedObjects...) } @@ -380,7 +430,10 @@ func TestContactValidator_ValidateCreate(t *testing.T) { func TestContactValidator_ValidateUpdate_Duplicate(t *testing.T) { // Seed two contacts under same subject contactA := ¬ificationv1alpha1.Contact{ - ObjectMeta: metav1.ObjectMeta{Name: "contact-a"}, + ObjectMeta: metav1.ObjectMeta{ + Name: "contact-a", + Namespace: "ns-one", + }, Spec: notificationv1alpha1.ContactSpec{ GivenName: "A", FamilyName: "User", @@ -394,7 +447,10 @@ func TestContactValidator_ValidateUpdate_Duplicate(t *testing.T) { } contactBOriginal := ¬ificationv1alpha1.Contact{ - ObjectMeta: metav1.ObjectMeta{Name: "contact-b"}, + ObjectMeta: metav1.ObjectMeta{ + Name: "contact-b", + Namespace: "ns-two", + }, Spec: notificationv1alpha1.ContactSpec{ GivenName: "B", FamilyName: "User", @@ -414,12 +470,16 @@ func TestContactValidator_ValidateUpdate_Duplicate(t *testing.T) { // User resource so validation passes user existence user := &iamv1alpha1.User{ObjectMeta: metav1.ObjectMeta{Name: "test-user"}, Spec: iamv1alpha1.UserSpec{Email: "user@example.com"}} - // Build fake client with index + // Build fake client with indexes builder := fake.NewClientBuilder().WithScheme(runtimeScheme).WithObjects(user, contactA, contactBOriginal) builder = builder.WithIndex(¬ificationv1alpha1.Contact{}, contactSpecKey, func(obj client.Object) []string { c := obj.(*notificationv1alpha1.Contact) return []string{buildContactSpecKey(*c)} }) + builder = builder.WithIndex(¬ificationv1alpha1.Contact{}, contactEmailKey, func(obj client.Object) []string { + c := obj.(*notificationv1alpha1.Contact) + return []string{c.Spec.Email} + }) fakeClient := builder.Build() validator := &ContactValidator{Client: fakeClient} @@ -427,5 +487,5 @@ func TestContactValidator_ValidateUpdate_Duplicate(t *testing.T) { _, err := validator.ValidateUpdate(context.Background(), contactBOriginal, contactBUpdated) assert.Error(t, err, "expected duplicate validation error on update") - assert.Contains(t, strings.ToLower(err.Error()), "already has this subject and email") + assert.Contains(t, strings.ToLower(err.Error()), "already has this email in the same contact cluster") } diff --git a/internal/webhooks/notification/v1alpha1/contactgroupmembership_webhook.go b/internal/webhooks/notification/v1alpha1/contactgroupmembership_webhook.go index 38823d44..0fb240a4 100644 --- a/internal/webhooks/notification/v1alpha1/contactgroupmembership_webhook.go +++ b/internal/webhooks/notification/v1alpha1/contactgroupmembership_webhook.go @@ -90,7 +90,6 @@ func (v *ContactGroupMembershipValidator) ValidateCreate(ctx context.Context, ob // Check for duplicate membership (same contact already in the target group) var existing notificationv1alpha1.ContactGroupMembershipList if err := v.Client.List(ctx, &existing, - client.InNamespace(cgm.Namespace), client.MatchingFields{contactMembershipCompositeKey: buildContactGroupTupleKey(cgm.Spec.ContactRef, cgm.Spec.ContactGroupRef)}); err != nil { return nil, errors.NewInternalError(fmt.Errorf("failed to list memberships: %w", err)) } @@ -101,7 +100,6 @@ func (v *ContactGroupMembershipValidator) ValidateCreate(ctx context.Context, ob // Check for existing membership removal for the same contact and group var existingRemovals notificationv1alpha1.ContactGroupMembershipRemovalList if err := v.Client.List(ctx, &existingRemovals, - client.InNamespace(cgm.Namespace), client.MatchingFields{contactMembershipRemovalCompositeKey: buildContactGroupTupleKey(cgm.Spec.ContactRef, cgm.Spec.ContactGroupRef)}); err != nil { return nil, errors.NewInternalError(fmt.Errorf("failed to list membership removals: %w", err)) } diff --git a/internal/webhooks/notification/v1alpha1/contactgroupmembershipremoval_webhook.go b/internal/webhooks/notification/v1alpha1/contactgroupmembershipremoval_webhook.go index 7f86650f..768af4c4 100644 --- a/internal/webhooks/notification/v1alpha1/contactgroupmembershipremoval_webhook.go +++ b/internal/webhooks/notification/v1alpha1/contactgroupmembershipremoval_webhook.go @@ -72,7 +72,7 @@ func (v *ContactGroupMembershipRemovalValidator) ValidateCreate(ctx context.Cont // Prevent duplicate removals var existing notificationv1alpha1.ContactGroupMembershipRemovalList - if err := v.Client.List(ctx, &existing, client.InNamespace(removal.Namespace), client.MatchingFields{cgmrByContactGroupTupleKey: buildContactGroupTupleKey(removal.Spec.ContactRef, removal.Spec.ContactGroupRef)}); err != nil { + if err := v.Client.List(ctx, &existing, client.MatchingFields{cgmrByContactGroupTupleKey: buildContactGroupTupleKey(removal.Spec.ContactRef, removal.Spec.ContactGroupRef)}); err != nil { return nil, errors.NewInternalError(fmt.Errorf("failed to list removals: %w", err)) } if len(existing.Items) > 0 {