From 76638c12f47b0a18fe5b5f764217441efd452586 Mon Sep 17 00:00:00 2001 From: Jackson Chen Date: Wed, 23 Apr 2025 14:54:25 -0400 Subject: [PATCH 1/2] notifications: use user email if slack annotation is missing Signed-off-by: Jackson Chen --- .../lib/SlackNotificationProcessor.test.ts | 175 ++++++++++++++++++ .../src/lib/SlackNotificationProcessor.ts | 50 ++++- 2 files changed, 222 insertions(+), 3 deletions(-) diff --git a/plugins/notifications-backend-module-slack/src/lib/SlackNotificationProcessor.test.ts b/plugins/notifications-backend-module-slack/src/lib/SlackNotificationProcessor.test.ts index 4c891ed3ab..f5893ff456 100644 --- a/plugins/notifications-backend-module-slack/src/lib/SlackNotificationProcessor.test.ts +++ b/plugins/notifications-backend-module-slack/src/lib/SlackNotificationProcessor.test.ts @@ -50,6 +50,7 @@ jest.mock('@slack/web-api', () => { }, ], })), + lookupByEmail: jest.fn(), }, }; return { WebClient: jest.fn(() => mockSlack) }; @@ -81,6 +82,27 @@ const DEFAULT_ENTITIES_RESPONSE = { }, }, } as unknown as Entity, + { + kind: 'User', + metadata: { + name: 'mock-without-slack-annotation', + namespace: 'default', + annotations: {}, + }, + spec: { + profile: { + email: 'test@example.com', + }, + }, + } as unknown as Entity, + { + kind: 'Group', + metadata: { + name: 'mock-without-slack-annotation', + namespace: 'default', + annotations: {}, + }, + } as unknown as Entity, ], }; @@ -368,4 +390,157 @@ describe('SlackNotificationProcessor', () => { expect(slack.chat.postMessage).toHaveBeenCalledTimes(2); }); }); + + describe('when slack.com/bot-notify annotation is missing', () => { + it('should not send notification to a group without annotation', async () => { + const slack = new WebClient(); + + const processor = SlackNotificationProcessor.fromConfig(config, { + auth, + discovery, + logger, + catalog: catalogServiceMock({ + entities: DEFAULT_ENTITIES_RESPONSE.items, + }), + slack, + })[0]; + + await processor.processOptions({ + recipients: { + type: 'entity', + entityRef: 'group:default/mock-without-slack-annotation', + }, + payload: { title: 'notification' }, + }); + + expect(slack.chat.postMessage).not.toHaveBeenCalled(); + }); + + it('should not send notification to a user without annotation and email', async () => { + const slack = new WebClient(); + + const processor = SlackNotificationProcessor.fromConfig(config, { + auth, + discovery, + logger, + catalog: catalogServiceMock({ + entities: [DEFAULT_ENTITIES_RESPONSE.items[2]], + }), + slack, + })[0]; + + await processor.postProcess( + { + origin: 'plugin', + id: '1234', + user: 'user:default/mock-without-slack-annotation', + created: new Date(), + payload: { + title: 'notification', + link: '/catalog/user/default/jane.doe', + }, + }, + { + recipients: { + type: 'entity', + entityRef: 'user:default/mock-without-slack-annotation', + }, + payload: { title: 'notification' }, + }, + ); + + expect(slack.chat.postMessage).not.toHaveBeenCalled(); + }); + + it('should try to find user by email when annotation is missing', async () => { + const slack = new WebClient(); + (slack.users.lookupByEmail as jest.Mock).mockResolvedValueOnce({ + ok: true, + user: { id: 'U12345678' }, + }); + + const processor = SlackNotificationProcessor.fromConfig(config, { + auth, + discovery, + logger, + catalog: catalogServiceMock({ + entities: DEFAULT_ENTITIES_RESPONSE.items, + }), + slack, + })[0]; + + await processor.postProcess( + { + origin: 'plugin', + id: '1234', + user: 'user:default/mock-without-slack-annotation', + created: new Date(), + payload: { + title: 'notification', + link: '/catalog/user/default/jane.doe', + }, + }, + { + recipients: { + type: 'entity', + entityRef: 'user:default/mock-without-slack-annotation', + }, + payload: { title: 'notification' }, + }, + ); + + expect(slack.users.lookupByEmail).toHaveBeenCalledWith({ + email: 'test@example.com', + }); + expect(slack.chat.postMessage).toHaveBeenCalledWith( + expect.objectContaining({ + channel: 'U12345678', + }), + ); + }); + + it('should log warning when email lookup fails', async () => { + const slack = new WebClient(); + (slack.users.lookupByEmail as jest.Mock).mockRejectedValueOnce( + new Error('User not found'), + ); + + const processor = SlackNotificationProcessor.fromConfig(config, { + auth, + discovery, + logger, + catalog: catalogServiceMock({ + entities: DEFAULT_ENTITIES_RESPONSE.items, + }), + slack, + })[0]; + + await processor.postProcess( + { + origin: 'plugin', + id: '1234', + user: 'user:default/mock-without-slack-annotation', + created: new Date(), + payload: { + title: 'notification', + link: '/catalog/user/default/jane.doe', + }, + }, + { + recipients: { + type: 'entity', + entityRef: 'user:default/mock-without-slack-annotation', + }, + payload: { title: 'notification' }, + }, + ); + + expect(logger.warn).toHaveBeenCalledWith( + expect.stringContaining( + 'Failed to lookup Slack user by email test@example.com', + ), + ); + expect(slack.chat.postMessage).not.toHaveBeenCalled(); + }); + }); }); diff --git a/plugins/notifications-backend-module-slack/src/lib/SlackNotificationProcessor.ts b/plugins/notifications-backend-module-slack/src/lib/SlackNotificationProcessor.ts index 3328c55408..ca51eeb068 100644 --- a/plugins/notifications-backend-module-slack/src/lib/SlackNotificationProcessor.ts +++ b/plugins/notifications-backend-module-slack/src/lib/SlackNotificationProcessor.ts @@ -20,7 +20,12 @@ import { LoggerService, } from '@backstage/backend-plugin-api'; import { CatalogApi } from '@backstage/catalog-client'; -import { Entity, parseEntityRef } from '@backstage/catalog-model'; +import { + Entity, + isUserEntity, + parseEntityRef, + UserEntity, +} from '@backstage/catalog-model'; import { Config } from '@backstage/config'; import { NotFoundError } from '@backstage/errors'; import { Notification } from '@backstage/plugin-notifications-common'; @@ -255,7 +260,11 @@ export class SlackNotificationProcessor implements NotificationProcessor { const response = await this.catalog.getEntitiesByRefs( { entityRefs: entityRefs.slice(), - fields: [`metadata.annotations.${ANNOTATION_SLACK_BOT_NOTIFY}`], + fields: [ + `kind`, + `spec.profile.email`, + `metadata.annotations.${ANNOTATION_SLACK_BOT_NOTIFY}`, + ], }, { token, @@ -278,7 +287,42 @@ export class SlackNotificationProcessor implements NotificationProcessor { throw new NotFoundError(`Entity not found: ${entityRef}`); } - return entity?.metadata?.annotations?.[ANNOTATION_SLACK_BOT_NOTIFY]; + const slackId = await this.resolveSlackId(entity); + return slackId; + } + + private async resolveSlackId(entity: Entity): Promise { + // First try to get Slack ID from annotations + const slackId = entity.metadata?.annotations?.[ANNOTATION_SLACK_BOT_NOTIFY]; + if (slackId) { + return slackId; + } + + // If no Slack ID in annotations and entity is a User, try to find by email + if (isUserEntity(entity)) { + return this.findSlackIdByEmail(entity); + } + + return undefined; + } + + private async findSlackIdByEmail( + entity: UserEntity, + ): Promise { + const email = entity.spec?.profile?.email; + if (!email) { + return undefined; + } + + try { + const user = await this.slack.users.lookupByEmail({ email }); + return user.user?.id; + } catch (error) { + this.logger.warn( + `Failed to lookup Slack user by email ${email}: ${error}`, + ); + return undefined; + } } async sendNotification(args: ChatPostMessageArguments): Promise { From a1c5bbb1e52709a239afe3b7043ca706a3ed9c55 Mon Sep 17 00:00:00 2001 From: Jackson Chen Date: Wed, 23 Apr 2025 15:39:36 -0400 Subject: [PATCH 2/2] add changeset Signed-off-by: Jackson Chen --- .changeset/spotty-doors-design.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/spotty-doors-design.md diff --git a/.changeset/spotty-doors-design.md b/.changeset/spotty-doors-design.md new file mode 100644 index 0000000000..22ee61c9b0 --- /dev/null +++ b/.changeset/spotty-doors-design.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-notifications-backend-module-slack': patch +--- + +Added email-based Slack User ID lookup if `metadata.annotations.slack.com/bot-notify` is missing from user entity