[RAC] Fix index names used by RBAC, delete hardcoded map of Kibana fe…

…atures to index names (#109567) (#110068) **Ticket:** #102089 🚨 **This PR is critical for Observability 7.15** 🚨 ## Summary This PR introduces changes that fix the usage of alerts-as-data index naming in RBAC. It builds on top of #109346 and replaces #108872. TODO: - [x] Address #109346 (review) - [x] Make changes to `AlertsClient.getAuthorizedAlertsIndices()` so it starts using `RuleDataService` to get index names by feature ids. - [x] Delete the hardcoded `mapConsumerToIndexName` where we had incorrect index names. - [x] Close #108872 ### Checklist Delete any items that are not applicable to this PR. - [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios Co-authored-by: Georgii Gorbachev <[email protected]>
elastic · Aug 25, 2021 · 8db2bc1 · 8db2bc1
1 parent aa7680e
commit 8db2bc1
Show file tree

Hide file tree

Showing 33 changed files with 162 additions and 126 deletions.
diff --git a/packages/kbn-rule-data-utils/src/alerts_as_data_rbac.ts b/packages/kbn-rule-data-utils/src/alerts_as_data_rbac.ts
@@ -26,17 +26,9 @@ export const AlertConsumers = {
 export type AlertConsumers = typeof AlertConsumers[keyof typeof AlertConsumers];
 export type STATUS_VALUES = 'open' | 'acknowledged' | 'closed' | 'in-progress'; // TODO: remove 'in-progress' after migration to 'acknowledged'
 
-export const mapConsumerToIndexName: Record<AlertConsumers, string | string[]> = {
-  apm: '.alerts-observability-apm',
-  logs: '.alerts-observability.logs',
-  infrastructure: '.alerts-observability.metrics',
-  observability: '.alerts-observability',
-  siem: '.alerts-security.alerts',
-  uptime: '.alerts-observability.uptime',
-};
-export type ValidFeatureId = keyof typeof mapConsumerToIndexName;
+export type ValidFeatureId = AlertConsumers;
 
-export const validFeatureIds = Object.keys(mapConsumerToIndexName);
+export const validFeatureIds = Object.values(AlertConsumers).map((v) => v as string);
 export const isValidFeatureId = (a: unknown): a is ValidFeatureId =>
   typeof a === 'string' && validFeatureIds.includes(a);
 

diff --git a/x-pack/plugins/monitoring/common/es_glob_patterns.test.ts b/x-pack/plugins/monitoring/common/es_glob_patterns.test.ts
@@ -19,7 +19,7 @@ const testIndices = [
   '.ds-metrics-system.process.summary-default-2021.05.25-00000',
   '.kibana_shahzad_9',
   '.kibana-felix-log-stream_8.0.0_001',
-  '.kibana_smith_alerts-observability-apm-000001',
+  '.kibana_smith_alerts-observability.apm.alerts-000001',
   '.ds-logs-endpoint.events.process-default-2021.05.26-000001',
   '.kibana_dominiqueclarke54_8.0.0_001',
   '.kibana-cmarcondes-19_8.0.0_001',
@@ -63,7 +63,7 @@ const onlySystemIndices = [
   '.ds-metrics-system.process.summary-default-2021.05.25-00000',
   '.kibana_shahzad_9',
   '.kibana-felix-log-stream_8.0.0_001',
-  '.kibana_smith_alerts-observability-apm-000001',
+  '.kibana_smith_alerts-observability.apm.alerts-000001',
   '.ds-logs-endpoint.events.process-default-2021.05.26-000001',
   '.kibana_dominiqueclarke54_8.0.0_001',
   '.kibana-cmarcondes-19_8.0.0_001',
@@ -85,7 +85,7 @@ const kibanaNoTaskIndices = [
   '.kibana_shahzad_1',
   '.kibana_shahzad_9',
   '.kibana-felix-log-stream_8.0.0_001',
-  '.kibana_smith_alerts-observability-apm-000001',
+  '.kibana_smith_alerts-observability.apm.alerts-000001',
   '.kibana_dominiqueclarke54_8.0.0_001',
   '.kibana-cmarcondes-19_8.0.0_001',
   '.kibana_dominiqueclarke55-alerts-8.0.0-000001',

diff --git a/x-pack/plugins/observability/public/pages/alerts/index.tsx b/x-pack/plugins/observability/public/pages/alerts/index.tsx
@@ -72,7 +72,6 @@ export function AlertsPage({ routeParams }: AlertsPageProps) {
           registrationContexts: [
             'observability.apm',
             'observability.logs',
-            'observability.infrastructure',
             'observability.metrics',
             'observability.uptime',
           ],

diff --git a/x-pack/plugins/observability/server/routes/rules.ts b/x-pack/plugins/observability/server/routes/rules.ts
@@ -6,6 +6,7 @@
  */
 
 import * as t from 'io-ts';
+import { Dataset } from '../../../rule_registry/server';
 import { createObservabilityServerRoute } from './create_observability_server_route';
 import { createObservabilityServerRouteRepository } from './create_observability_server_route_repository';
 
@@ -24,7 +25,7 @@ const alertsDynamicIndexPatternRoute = createObservabilityServerRoute({
     const { namespace, registrationContexts } = params.query;
     const indexNames = registrationContexts.flatMap((registrationContext) => {
       const indexName = ruleDataService
-        .getRegisteredIndexInfo(registrationContext)
+        .findIndexByName(registrationContext, Dataset.alerts)
         ?.getPrimaryAlias(namespace);
 
       if (indexName != null) {

diff --git a/x-pack/plugins/rule_registry/README.md b/x-pack/plugins/rule_registry/README.md
@@ -73,7 +73,7 @@ await plugins.ruleRegistry.createOrUpdateComponentTemplate({
 await plugins.ruleRegistry.createOrUpdateIndexTemplate({
   name: plugins.ruleRegistry.getFullAssetName('apm-index-template'),
   body: {
-    index_patterns: [plugins.ruleRegistry.getFullAssetName('observability-apm*')],
+    index_patterns: [plugins.ruleRegistry.getFullAssetName('observability.apm*')],
     composed_of: [
       // Technical component template, required
       plugins.ruleRegistry.getFullAssetName(TECHNICAL_COMPONENT_TEMPLATE_NAME),
@@ -85,7 +85,7 @@ await plugins.ruleRegistry.createOrUpdateIndexTemplate({
 // Finally, create the rule data client that can be injected into rule type
 // executors and API endpoints
 const ruleDataClient = new RuleDataClient({
-  alias: plugins.ruleRegistry.getFullAssetName('observability-apm'),
+  alias: plugins.ruleRegistry.getFullAssetName('observability.apm'),
   getClusterClient: async () => {
     const coreStart = await getCoreStart();
     return coreStart.elasticsearch.client.asInternalUser;

diff --git a/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts
@@ -13,14 +13,13 @@ import type {
   getEsQueryConfig as getEsQueryConfigTyped,
   getSafeSortIds as getSafeSortIdsTyped,
   isValidFeatureId as isValidFeatureIdTyped,
-  mapConsumerToIndexName as mapConsumerToIndexNameTyped,
   STATUS_VALUES,
+  ValidFeatureId,
 } from '@kbn/rule-data-utils';
 import {
   getEsQueryConfig as getEsQueryConfigNonTyped,
   getSafeSortIds as getSafeSortIdsNonTyped,
   isValidFeatureId as isValidFeatureIdNonTyped,
-  mapConsumerToIndexName as mapConsumerToIndexNameNonTyped,
   // @ts-expect-error
 } from '@kbn/rule-data-utils/target_node/alerts_as_data_rbac';
 
@@ -42,11 +41,11 @@ import {
   SPACE_IDS,
 } from '../../common/technical_rule_data_field_names';
 import { ParsedTechnicalFields } from '../../common/parse_technical_fields';
+import { Dataset, RuleDataPluginService } from '../rule_data_plugin_service';
 
 const getEsQueryConfig: typeof getEsQueryConfigTyped = getEsQueryConfigNonTyped;
 const getSafeSortIds: typeof getSafeSortIdsTyped = getSafeSortIdsNonTyped;
 const isValidFeatureId: typeof isValidFeatureIdTyped = isValidFeatureIdNonTyped;
-const mapConsumerToIndexName: typeof mapConsumerToIndexNameTyped = mapConsumerToIndexNameNonTyped;
 
 // TODO: Fix typings https://github.com/elastic/kibana/issues/101776
 type NonNullableProps<Obj extends {}, Props extends keyof Obj> = Omit<Obj, Props> &
@@ -71,6 +70,7 @@ export interface ConstructorOptions {
   authorization: PublicMethodsOf<AlertingAuthorization>;
   auditLogger?: AuditLogger;
   esClient: ElasticsearchClient;
+  ruleDataService: RuleDataPluginService;
 }
 
 export interface UpdateOptions<Params extends AlertTypeParams> {
@@ -115,15 +115,17 @@ export class AlertsClient {
   private readonly authorization: PublicMethodsOf<AlertingAuthorization>;
   private readonly esClient: ElasticsearchClient;
   private readonly spaceId: string | undefined;
+  private readonly ruleDataService: RuleDataPluginService;
 
-  constructor({ auditLogger, authorization, logger, esClient }: ConstructorOptions) {
-    this.logger = logger;
-    this.authorization = authorization;
-    this.esClient = esClient;
-    this.auditLogger = auditLogger;
+  constructor(options: ConstructorOptions) {
+    this.logger = options.logger;
+    this.authorization = options.authorization;
+    this.esClient = options.esClient;
+    this.auditLogger = options.auditLogger;
     // If spaceId is undefined, it means that spaces is disabled
     // Otherwise, if space is enabled and not specified, it is "default"
     this.spaceId = this.authorization.getSpaceId();
+    this.ruleDataService = options.ruleDataService;
   }
 
   private getOutcome(
@@ -666,15 +668,18 @@ export class AlertsClient {
         authorizedFeatures.add(ruleType.producer);
       }
 
-      const toReturn = Array.from(authorizedFeatures).flatMap((feature) => {
-        if (featureIds.includes(feature) && isValidFeatureId(feature)) {
-          if (feature === 'siem') {
-            return `${mapConsumerToIndexName[feature]}-${this.spaceId}`;
-          } else {
-            return `${mapConsumerToIndexName[feature]}`;
-          }
+      const validAuthorizedFeatures = Array.from(authorizedFeatures).filter(
+        (feature): feature is ValidFeatureId =>
+          featureIds.includes(feature) && isValidFeatureId(feature)
+      );
+
+      const toReturn = validAuthorizedFeatures.flatMap((feature) => {
+        const indices = this.ruleDataService.findIndicesByFeature(feature, Dataset.alerts);
+        if (feature === 'siem') {
+          return indices.map((i) => `${i.baseName}-${this.spaceId}`);
+        } else {
+          return indices.map((i) => i.baseName);
         }
-        return [];
       });
 
       return toReturn;

diff --git a/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client_factory.test.ts b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client_factory.test.ts
@@ -13,6 +13,8 @@ import { loggingSystemMock } from 'src/core/server/mocks';
 import { securityMock } from '../../../security/server/mocks';
 import { AuditLogger } from '../../../security/server';
 import { alertingAuthorizationMock } from '../../../alerting/server/authorization/alerting_authorization.mock';
+import { ruleDataPluginServiceMock } from '../rule_data_plugin_service/rule_data_plugin_service.mock';
+import { RuleDataPluginService } from '../rule_data_plugin_service';
 
 jest.mock('./alerts_client');
 
@@ -24,6 +26,7 @@ const alertsClientFactoryParams: AlertsClientFactoryProps = {
   getAlertingAuthorization: (_: KibanaRequest) => alertingAuthMock,
   securityPluginSetup,
   esClient: {} as ElasticsearchClient,
+  ruleDataService: (ruleDataPluginServiceMock.create() as unknown) as RuleDataPluginService,
 };
 
 const fakeRequest = ({
@@ -64,6 +67,7 @@ describe('AlertsClientFactory', () => {
       logger: alertsClientFactoryParams.logger,
       auditLogger,
       esClient: {},
+      ruleDataService: alertsClientFactoryParams.ruleDataService,
     });
   });
 

diff --git a/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client_factory.ts b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client_factory.ts
@@ -5,17 +5,19 @@
  * 2.0.
  */
 
-import { ElasticsearchClient, KibanaRequest, Logger } from 'src/core/server';
 import { PublicMethodsOf } from '@kbn/utility-types';
-import { SecurityPluginSetup } from '../../../security/server';
+import { ElasticsearchClient, KibanaRequest, Logger } from 'src/core/server';
 import { AlertingAuthorization } from '../../../alerting/server';
+import { SecurityPluginSetup } from '../../../security/server';
+import { RuleDataPluginService } from '../rule_data_plugin_service';
 import { AlertsClient } from './alerts_client';
 
 export interface AlertsClientFactoryProps {
   logger: Logger;
   esClient: ElasticsearchClient;
   getAlertingAuthorization: (request: KibanaRequest) => PublicMethodsOf<AlertingAuthorization>;
   securityPluginSetup: SecurityPluginSetup | undefined;
+  ruleDataService: RuleDataPluginService | null;
 }
 
 export class AlertsClientFactory {
@@ -26,6 +28,7 @@ export class AlertsClientFactory {
     request: KibanaRequest
   ) => PublicMethodsOf<AlertingAuthorization>;
   private securityPluginSetup!: SecurityPluginSetup | undefined;
+  private ruleDataService!: RuleDataPluginService | null;
 
   public initialize(options: AlertsClientFactoryProps) {
     /**
@@ -40,6 +43,7 @@ export class AlertsClientFactory {
     this.logger = options.logger;
     this.esClient = options.esClient;
     this.securityPluginSetup = options.securityPluginSetup;
+    this.ruleDataService = options.ruleDataService;
   }
 
   public async create(request: KibanaRequest): Promise<AlertsClient> {
@@ -50,6 +54,7 @@ export class AlertsClientFactory {
       authorization: getAlertingAuthorization(request),
       auditLogger: securityPluginSetup?.audit.asScoped(request),
       esClient: this.esClient,
+      ruleDataService: this.ruleDataService!,
     });
   }
 }
diff --git a/x-pack/plugins/rule_registry/server/alert_data_client/tests/bulk_update.test.ts b/x-pack/plugins/rule_registry/server/alert_data_client/tests/bulk_update.test.ts
@@ -18,6 +18,8 @@ import { elasticsearchClientMock } from 'src/core/server/elasticsearch/client/mo
 import { alertingAuthorizationMock } from '../../../../alerting/server/authorization/alerting_authorization.mock';
 import { AuditLogger } from '../../../../security/server';
 import { AlertingAuthorizationEntity } from '../../../../alerting/server';
+import { ruleDataPluginServiceMock } from '../../rule_data_plugin_service/rule_data_plugin_service.mock';
+import { RuleDataPluginService } from '../../rule_data_plugin_service';
 
 const alertingAuthMock = alertingAuthorizationMock.create();
 const esClientMock = elasticsearchClientMock.createElasticsearchClient();
@@ -30,6 +32,7 @@ const alertsClientParams: jest.Mocked<ConstructorOptions> = {
   authorization: alertingAuthMock,
   esClient: esClientMock,
   auditLogger,
+  ruleDataService: (ruleDataPluginServiceMock.create() as unknown) as RuleDataPluginService,
 };
 
 const DEFAULT_SPACE = 'test_default_space_id';
@@ -78,7 +81,7 @@ describe('bulkUpdate()', () => {
   describe('ids', () => {
     describe('audit log', () => {
       test('logs successful event in audit logger', async () => {
-        const indexName = '.alerts-observability-apm.alerts';
+        const indexName = '.alerts-observability.apm.alerts';
         const alertsClient = new AlertsClient(alertsClientParams);
         esClientMock.mget.mockResolvedValueOnce(
           elasticsearchClientMock.createApiResponse({
@@ -107,7 +110,7 @@ describe('bulkUpdate()', () => {
                 {
                   update: {
                     _id: fakeAlertId,
-                    _index: '.alerts-observability-apm.alerts',
+                    _index: '.alerts-observability.apm.alerts',
                     result: 'updated',
                     status: 200,
                   },
@@ -135,7 +138,7 @@ describe('bulkUpdate()', () => {
       });
 
       test('audit error access if user is unauthorized for given alert', async () => {
-        const indexName = '.alerts-observability-apm.alerts';
+        const indexName = '.alerts-observability.apm.alerts';
         const alertsClient = new AlertsClient(alertsClientParams);
         esClientMock.mget.mockResolvedValueOnce(
           elasticsearchClientMock.createApiResponse({
@@ -181,7 +184,7 @@ describe('bulkUpdate()', () => {
       });
 
       test('logs multiple error events in audit logger', async () => {
-        const indexName = '.alerts-observability-apm.alerts';
+        const indexName = '.alerts-observability.apm.alerts';
         const alertsClient = new AlertsClient(alertsClientParams);
         esClientMock.mget.mockResolvedValueOnce(
           elasticsearchClientMock.createApiResponse({
@@ -257,7 +260,7 @@ describe('bulkUpdate()', () => {
   describe('query', () => {
     describe('audit log', () => {
       test('logs successful event in audit logger', async () => {
-        const indexName = '.alerts-observability-apm.alerts';
+        const indexName = '.alerts-observability.apm.alerts';
         const alertsClient = new AlertsClient(alertsClientParams);
         esClientMock.search.mockResolvedValueOnce(
           elasticsearchClientMock.createApiResponse({
@@ -276,7 +279,7 @@ describe('bulkUpdate()', () => {
                 hits: [
                   {
                     _id: fakeAlertId,
-                    _index: '.alerts-observability-apm.alerts',
+                    _index: '.alerts-observability.apm.alerts',
                     _source: {
                       [ALERT_RULE_TYPE_ID]: 'apm.error_rate',
                       [ALERT_RULE_CONSUMER]: 'apm',
@@ -317,7 +320,7 @@ describe('bulkUpdate()', () => {
       });
 
       test('audit error access if user is unauthorized for given alert', async () => {
-        const indexName = '.alerts-observability-apm.alerts';
+        const indexName = '.alerts-observability.apm.alerts';
         const alertsClient = new AlertsClient(alertsClientParams);
         esClientMock.search.mockResolvedValueOnce(
           elasticsearchClientMock.createApiResponse({
@@ -336,7 +339,7 @@ describe('bulkUpdate()', () => {
                 hits: [
                   {
                     _id: fakeAlertId,
-                    _index: '.alerts-observability-apm.alerts',
+                    _index: '.alerts-observability.apm.alerts',
                     _source: {
                       [ALERT_RULE_TYPE_ID]: fakeRuleTypeId,
                       [ALERT_RULE_CONSUMER]: 'apm',
@@ -378,7 +381,7 @@ describe('bulkUpdate()', () => {
       });
 
       test('logs multiple error events in audit logger', async () => {
-        const indexName = '.alerts-observability-apm.alerts';
+        const indexName = '.alerts-observability.apm.alerts';
         const alertsClient = new AlertsClient(alertsClientParams);
         esClientMock.search.mockResolvedValueOnce(
           elasticsearchClientMock.createApiResponse({
@@ -397,7 +400,7 @@ describe('bulkUpdate()', () => {
                 hits: [
                   {
                     _id: successfulAuthzHit,
-                    _index: '.alerts-observability-apm.alerts',
+                    _index: '.alerts-observability.apm.alerts',
                     _source: {
                       [ALERT_RULE_TYPE_ID]: 'apm.error_rate',
                       [ALERT_RULE_CONSUMER]: 'apm',
@@ -407,7 +410,7 @@ describe('bulkUpdate()', () => {
                   },
                   {
                     _id: unsuccessfulAuthzHit,
-                    _index: '.alerts-observability-apm.alerts',
+                    _index: '.alerts-observability.apm.alerts',
                     _source: {
                       [ALERT_RULE_TYPE_ID]: fakeRuleTypeId,
                       [ALERT_RULE_CONSUMER]: 'apm',