diff --git a/plugins/notifications-backend-module-slack/config.d.ts b/plugins/notifications-backend-module-slack/config.d.ts index f620f0e206..8cd450940e 100644 --- a/plugins/notifications-backend-module-slack/config.d.ts +++ b/plugins/notifications-backend-module-slack/config.d.ts @@ -22,6 +22,12 @@ export interface Config { * @visibility secret */ token?: string; + /** + * Broadcast notification receivers when receiver is set to config + * These can be Slack User IDs, Slack User Email addresses, Slack Channel + * Names, or Slack Channel IDs. Any valid identifier that chat.postMessage can accept. + */ + broadcastChannels?: string[]; }>; }; }; diff --git a/plugins/notifications-backend-module-slack/package.json b/plugins/notifications-backend-module-slack/package.json index 2bc7a8eb00..7254d3ba6d 100644 --- a/plugins/notifications-backend-module-slack/package.json +++ b/plugins/notifications-backend-module-slack/package.json @@ -46,8 +46,8 @@ "@slack/bolt": "^3.21.4", "@slack/types": "^2.14.0", "@slack/web-api": "^7.5.0", - "dataloader": "2.2.2", - "p-throttle": "4.1.1" + "dataloader": "^2.0.0", + "p-throttle": "^4.1.1" }, "devDependencies": { "@backstage/backend-test-utils": "workspace:^", @@ -55,7 +55,7 @@ "@backstage/plugin-catalog-node": "workspace:^", "@backstage/test-utils": "workspace:^", "@faker-js/faker": "^8.4.1", - "msw": "^2.2.14" + "msw": "^2.0.0" }, "configSchema": "config.d.ts" } diff --git a/plugins/notifications-backend-module-slack/src/index.ts b/plugins/notifications-backend-module-slack/src/index.ts index 14cde37d13..f8a9e248f7 100644 --- a/plugins/notifications-backend-module-slack/src/index.ts +++ b/plugins/notifications-backend-module-slack/src/index.ts @@ -20,9 +20,5 @@ * @packageDocumentation */ -export { - ANNOTATION_SLACK_CHANNEL_ID, - ANNOTATION_SLACK_CHANNEL_NAME, - ANNOTATION_SLACK_USER_ID, -} from './lib'; +export { ANNOTATION_SLACK_BOT_NOTIFY } from './lib'; export { notificationsModuleSlack as default } from './module'; 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 5396a71736..4c891ed3ab 100644 --- a/plugins/notifications-backend-module-slack/src/lib/SlackNotificationProcessor.test.ts +++ b/plugins/notifications-backend-module-slack/src/lib/SlackNotificationProcessor.test.ts @@ -63,7 +63,7 @@ const DEFAULT_ENTITIES_RESPONSE = { name: 'mock', namespace: 'default', annotations: { - 'slack.com/user-id': 'U12345678', + 'slack.com/bot-notify': 'U12345678', }, }, spec: { @@ -77,7 +77,7 @@ const DEFAULT_ENTITIES_RESPONSE = { name: 'mock', namespace: 'default', annotations: { - 'slack.com/channel-id': 'C12345678', + 'slack.com/bot-notify': 'C12345678', }, }, } as unknown as Entity, @@ -136,17 +136,12 @@ describe('SlackNotificationProcessor', () => { blocks: [ { type: 'section', - text: { - type: 'mrkdwn', - text: 'No description provided', - }, accessory: { type: 'button', text: { type: 'plain_text', text: 'View More', }, - url: '', action_id: 'button-action', }, }, @@ -172,114 +167,205 @@ describe('SlackNotificationProcessor', () => { }); }); - it('should send a notification to a user', async () => { - const slack = new WebClient(); + describe('when a user notification is sent directly', () => { + it('should send a notification to a user', async () => { + const slack = new WebClient(); - const processor = SlackNotificationProcessor.fromConfig(config, { - auth, - discovery, - logger, - catalog: catalogServiceMock({ - entities: DEFAULT_ENTITIES_RESPONSE.items, - }), - slack, - })[0]; + 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', - created: new Date(), - payload: { - title: 'notification', - link: '/catalog/user/default/jane.doe', - }, - }, - { - recipients: { type: 'entity', entityRef: 'user:default/mock' }, - payload: { title: 'notification' }, - }, - ); - - expect(slack.chat.postMessage).toHaveBeenCalledWith({ - channel: 'U12345678', - text: 'notification', - attachments: [ + await processor.postProcess( { - color: '#00A699', - blocks: [ - { - type: 'section', - text: { - type: 'mrkdwn', - text: 'No description provided', - }, - accessory: { - type: 'button', - text: { - type: 'plain_text', - text: 'View More', - }, - url: '', - action_id: 'button-action', - }, - }, - { - type: 'context', - elements: [ - { - type: 'plain_text', - text: 'Severity: normal', - emoji: true, - }, - { - type: 'plain_text', - text: 'Topic: N/A', - emoji: true, - }, - ], - }, - ], - fallback: 'notification', + origin: 'plugin', + id: '1234', + user: 'user:default/mock', + created: new Date(), + payload: { + title: 'notification', + link: '/catalog/user/default/jane.doe', + }, }, - ], + { + recipients: { type: 'entity', entityRef: 'user:default/mock' }, + payload: { title: 'notification' }, + }, + ); + + expect(slack.chat.postMessage).toHaveBeenCalledWith({ + channel: 'U12345678', + text: 'notification', + attachments: [ + { + color: '#00A699', + blocks: [ + { + type: 'section', + accessory: { + type: 'button', + text: { + type: 'plain_text', + text: 'View More', + }, + action_id: 'button-action', + }, + }, + { + type: 'context', + elements: [ + { + type: 'plain_text', + text: 'Severity: normal', + emoji: true, + }, + { + type: 'plain_text', + text: 'Topic: N/A', + emoji: true, + }, + ], + }, + ], + fallback: 'notification', + }, + ], + }); }); }); - it('should not send broadcast messages', async () => { - const slack = new WebClient(); - const processor = SlackNotificationProcessor.fromConfig(config, { - auth, - discovery, - logger, - catalog: catalogServiceMock({ - entities: DEFAULT_ENTITIES_RESPONSE.items, - }), - slack, - })[0]; + describe('when a user notification is expanded from a group', () => { + it('should not send a notification', async () => { + const slack = new WebClient(); - processor.processOptions({ - recipients: { type: 'broadcast' }, - payload: { title: 'notification' }, - }); - processor.postProcess( - { - origin: 'plugin', - id: '1234', - user: null, - created: new Date(), - payload: { - title: 'notification', - link: '/catalog/user/default/jane.doe', + 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', + created: new Date(), + payload: { + title: 'notification', + link: '/catalog/user/default/jane.doe', + }, }, - }, - { + { + recipients: { type: 'entity', entityRef: 'group:default/group' }, + payload: { title: 'notification' }, + }, + ); + + expect(slack.chat.postMessage).not.toHaveBeenCalled(); + }); + }); + + describe('when broadcast channels are not configured', () => { + it('should not send broadcast messages', 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: 'broadcast' }, payload: { title: 'notification' }, - }, - ); + }); - expect(slack.chat.postMessage).not.toHaveBeenCalled(); + await processor.postProcess( + { + origin: 'plugin', + id: '1234', + user: null, + created: new Date(), + payload: { + title: 'notification', + link: '/catalog/user/default/jane.doe', + }, + }, + { + recipients: { type: 'broadcast' }, + payload: { title: 'notification' }, + }, + ); + expect(slack.chat.postMessage).not.toHaveBeenCalled(); + }); + }); + + describe('when broadcast channels are configured', () => { + it('should send broadcast messages', async () => { + const slack = new WebClient(); + const broadcastConfig = mockServices.rootConfig({ + data: { + app: { + baseUrl: 'https://example.org', + }, + notifications: { + processors: { + slack: [ + { + token: 'mock-token', + broadcastChannels: ['C12345678', 'D12345678'], + }, + ], + }, + }, + }, + }); + + const processor = SlackNotificationProcessor.fromConfig(broadcastConfig, { + auth, + discovery, + logger, + catalog: catalogServiceMock({ + entities: DEFAULT_ENTITIES_RESPONSE.items, + }), + slack, + })[0]; + + await processor.processOptions({ + recipients: { type: 'broadcast' }, + payload: { title: 'notification' }, + }); + + await processor.postProcess( + { + origin: 'plugin', + id: '1234', + user: null, + created: new Date(), + payload: { + title: 'notification', + link: '/catalog/user/default/jane.doe', + }, + }, + { + recipients: { type: 'broadcast' }, + payload: { title: 'notification' }, + }, + ); + expect(slack.chat.postMessage).toHaveBeenCalledTimes(2); + }); }); }); diff --git a/plugins/notifications-backend-module-slack/src/lib/SlackNotificationProcessor.ts b/plugins/notifications-backend-module-slack/src/lib/SlackNotificationProcessor.ts index 225f2f20ae..3328c55408 100644 --- a/plugins/notifications-backend-module-slack/src/lib/SlackNotificationProcessor.ts +++ b/plugins/notifications-backend-module-slack/src/lib/SlackNotificationProcessor.ts @@ -33,11 +33,7 @@ import { Counter, metrics } from '@opentelemetry/api'; import { ChatPostMessageArguments, WebClient } from '@slack/web-api'; import DataLoader from 'dataloader'; import pThrottle from 'p-throttle'; -import { - ANNOTATION_SLACK_CHANNEL_ID, - ANNOTATION_SLACK_CHANNEL_NAME, - ANNOTATION_SLACK_USER_ID, -} from './constants'; +import { ANNOTATION_SLACK_BOT_NOTIFY } from './constants'; import { toChatPostMessageArgs } from './util'; export class SlackNotificationProcessor implements NotificationProcessor { @@ -48,6 +44,7 @@ export class SlackNotificationProcessor implements NotificationProcessor { private readonly sendNotifications; private readonly messagesSent: Counter; private readonly messagesFailed: Counter; + private readonly broadcastChannels?: string[]; static fromConfig( config: Config, @@ -57,6 +54,7 @@ export class SlackNotificationProcessor implements NotificationProcessor { logger: LoggerService; catalog: CatalogApi; slack?: WebClient; + broadcastChannels?: string[]; }, ): SlackNotificationProcessor[] { const slackConfig = @@ -64,8 +62,10 @@ export class SlackNotificationProcessor implements NotificationProcessor { return slackConfig.map(c => { const token = c.getString('token'); const slack = options.slack ?? new WebClient(token); + const broadcastChannels = c.getOptionalStringArray('broadcastChannels'); return new SlackNotificationProcessor({ slack, + broadcastChannels, ...options, }); }); @@ -77,12 +77,14 @@ export class SlackNotificationProcessor implements NotificationProcessor { discovery: DiscoveryService; logger: LoggerService; catalog: CatalogApi; + broadcastChannels?: string[]; }) { - const { auth, catalog, logger, slack } = options; + const { auth, catalog, logger, slack, broadcastChannels } = options; this.logger = logger; this.catalog = catalog; this.auth = auth; this.slack = slack; + this.broadcastChannels = broadcastChannels; const meter = metrics.getMeter('default'); this.messagesSent = meter.createCounter( @@ -146,15 +148,14 @@ export class SlackNotificationProcessor implements NotificationProcessor { await Promise.all( entityRefs.map(async entityRef => { const compoundEntityRef = parseEntityRef(entityRef); - // skip users as they are sent direct messages, but allow all other entity kinds - // to have a channel id annotation. + // skip users as they are sent direct messages if (compoundEntityRef.kind === 'user') { return; } let channel; try { - channel = await this.getChannelId(entityRef); + channel = await this.getSlackNotificationTarget(entityRef); } catch (error) { this.logger.error( `Failed to get Slack channel for entity: ${ @@ -197,31 +198,50 @@ export class SlackNotificationProcessor implements NotificationProcessor { notification: Notification, options: NotificationSendOptions, ): Promise { - if (options.recipients.type === 'broadcast' || !notification.user) { + const destinations: string[] = []; + + // Handle broadcast case + if (notification.user === null) { + destinations.push(...(this.broadcastChannels ?? [])); + } else if (options.recipients.type === 'entity') { + // Handle user-specific notification + const entityRefs = [options.recipients.entityRef].flat(); + if (entityRefs.some(e => parseEntityRef(e).kind === 'group')) { + // We've already dispatched a slack channel message, so let's not send a DM. + return; + } + + const destination = await this.getSlackNotificationTarget( + notification.user, + ); + + if (!destination) { + this.logger.error( + `No slack.com/bot-notify annotation found for user: ${notification.user}`, + ); + return; + } + + destinations.push(destination); + } + + // If no destinations, nothing to do + if (destinations.length === 0) { return; } - const entityRefs = [options.recipients.entityRef].flat(); - if (entityRefs.some(e => parseEntityRef(e).kind === 'group')) { - // We've already dispatched a slack channel message, so let's not send a DM. - return; - } + // Prepare outbound messages + const outbound = destinations.map(channel => + toChatPostMessageArgs({ channel, payload: options.payload }), + ); - const destination = await this.getSlackUserId(notification.user); - if (!destination) { - this.logger.error(`No email found for user entity: ${notification.user}`); - return; - } - - const payload = toChatPostMessageArgs({ - channel: destination, - payload: options.payload, + // Log debug info + outbound.forEach(payload => { + this.logger.debug(`Sending notification: ${JSON.stringify(payload)}`); }); - this.logger.debug(`Sending DM notification: ${JSON.stringify(payload)}`); - - // batch it up - await this.sendNotifications([payload]); + // Send notifications + await this.sendNotifications(outbound); } async getEntities( @@ -235,11 +255,7 @@ export class SlackNotificationProcessor implements NotificationProcessor { const response = await this.catalog.getEntitiesByRefs( { entityRefs: entityRefs.slice(), - fields: [ - `metadata.annotations.${ANNOTATION_SLACK_CHANNEL_NAME}`, - `metadata.annotations.${ANNOTATION_SLACK_CHANNEL_ID}`, - `metadata.annotations.${ANNOTATION_SLACK_USER_ID}`, - ], + fields: [`metadata.annotations.${ANNOTATION_SLACK_BOT_NOTIFY}`], }, { token, @@ -249,16 +265,9 @@ export class SlackNotificationProcessor implements NotificationProcessor { return response.items; } - async getSlackUserId(entityRef: string): Promise { - const entityLoader = new DataLoader( - entityRefs => this.getEntities(entityRefs), - ); - const entity = await entityLoader.load(entityRef); - - return entity?.metadata?.annotations?.[ANNOTATION_SLACK_USER_ID]; - } - - async getChannelId(entityRef: string): Promise { + async getSlackNotificationTarget( + entityRef: string, + ): Promise { const entityLoader = new DataLoader( entityRefs => this.getEntities(entityRefs), ); @@ -269,10 +278,7 @@ export class SlackNotificationProcessor implements NotificationProcessor { throw new NotFoundError(`Entity not found: ${entityRef}`); } - return ( - entity?.metadata?.annotations?.[ANNOTATION_SLACK_CHANNEL_ID] || - entity?.metadata?.annotations?.[ANNOTATION_SLACK_CHANNEL_NAME] - ); + return entity?.metadata?.annotations?.[ANNOTATION_SLACK_BOT_NOTIFY]; } async sendNotification(args: ChatPostMessageArguments): Promise { diff --git a/plugins/notifications-backend-module-slack/src/lib/constants.ts b/plugins/notifications-backend-module-slack/src/lib/constants.ts index 24c1868d55..cdf052da7e 100644 --- a/plugins/notifications-backend-module-slack/src/lib/constants.ts +++ b/plugins/notifications-backend-module-slack/src/lib/constants.ts @@ -16,18 +16,14 @@ /** * @public - * The annotation key for the entity's Slack user ID + * The annotation key for the entity's Slack ID. This can be + * any valid chat.postMessage destination including: + * - A user ID (U12345678) + * - A channel ID (C12345678) + * - A DM ID (D12345678) + * - A group ID (S12345678) + * + * It can also be a user's email address or a channel name, + * however IDs are preferred. */ -export const ANNOTATION_SLACK_USER_ID = 'slack.com/user-id'; - -/** - * @public - * The annotation key for the entity's Slack channel name. - */ -export const ANNOTATION_SLACK_CHANNEL_NAME = 'slack.com/channel-name'; - -/** - * @public - * The annotation key for the entity's Slack channel ID. - */ -export const ANNOTATION_SLACK_CHANNEL_ID = 'slack.com/channel-id'; +export const ANNOTATION_SLACK_BOT_NOTIFY = 'slack.com/bot-notify'; diff --git a/plugins/notifications-backend-module-slack/src/lib/util.ts b/plugins/notifications-backend-module-slack/src/lib/util.ts index e39701d88a..5e2adbeec5 100644 --- a/plugins/notifications-backend-module-slack/src/lib/util.ts +++ b/plugins/notifications-backend-module-slack/src/lib/util.ts @@ -46,17 +46,19 @@ export function toSlackBlockKit(payload: NotificationPayload): KnownBlock[] { return [ { type: 'section', - text: { - type: 'mrkdwn', - text: description ?? 'No description provided', - }, + ...(description && { + text: { + type: 'mrkdwn', + text: description ?? 'No description provided', + }, + }), accessory: { type: 'button', text: { type: 'plain_text', text: 'View More', }, - url: link ?? '', + ...(link && { url: link }), action_id: 'button-action', }, },