[8.x] [9.0] [SLO] Check for unique SLO ids across spaces (#214496) (#215379)

x-pack/solutions/observability/plugins/slo/server/routes/slo/create_slo.ts

@@ -6,6 +6,7 @@
  */
 
 import { createSLOParamsSchema } from '@kbn/slo-schema';
+import { SavedObjectsClient } from '@kbn/core/server';
 import {
   CreateSLO,
   DefaultSummaryTransformManager,
@@ -37,6 +38,10 @@ export const createSLORoute = createSloServerRoute({
     const scopedClusterClient = core.elasticsearch.client;
     const esClient = core.elasticsearch.client.asCurrentUser;
     const soClient = core.savedObjects.client;
+    const [coreStart] = await corePlugins.getStartServices();
+    const internalSoClient = new SavedObjectsClient(
+      coreStart.savedObjects.createInternalRepository()
+    );
     const basePath = corePlugins.http.basePath;
     const repository = new KibanaSavedObjectsSLORepository(soClient, logger);
 
@@ -65,6 +70,7 @@ export const createSLORoute = createSloServerRoute({
       esClient,
       scopedClusterClient,
       repository,
+      internalSoClient,
       transformManager,
       summaryTransformManager,
       logger,

x-pack/solutions/observability/plugins/slo/server/routes/slo/inspect_slo.ts

@@ -6,6 +6,7 @@
  */
 
 import { createSLOParamsSchema } from '@kbn/slo-schema';
+import { SavedObjectsClient } from '@kbn/core/server';
 import {
   CreateSLO,
   DefaultSummaryTransformManager,
@@ -39,6 +40,10 @@ export const inspectSLORoute = createSloServerRoute({
     const esClient = core.elasticsearch.client.asCurrentUser;
     const username = core.security.authc.getCurrentUser()?.username!;
     const soClient = core.savedObjects.client;
+    const [coreStart] = await corePlugins.getStartServices();
+    const internalSoClient = new SavedObjectsClient(
+      coreStart.savedObjects.createInternalRepository()
+    );
     const repository = new KibanaSavedObjectsSLORepository(soClient, logger);
     const dataViewsService = await dataViews.dataViewsServiceFactory(soClient, esClient);
 
@@ -62,6 +67,7 @@ export const inspectSLORoute = createSloServerRoute({
       esClient,
       scopedClusterClient,
       repository,
+      internalSoClient,
       transformManager,
       summaryTransformManager,
       logger,

x-pack/solutions/observability/plugins/slo/server/services/create_slo.test.ts

@@ -12,6 +12,7 @@ import {
   loggingSystemMock,
   ScopedClusterClientMock,
 } from '@kbn/core/server/mocks';
+import { savedObjectsClientMock } from '@kbn/core-saved-objects-api-server-mocks';
 import { MockedLogger } from '@kbn/logging-mocks';
 import { CreateSLO } from './create_slo';
 import { fiveMinute, oneMinute } from './fixtures/duration';
@@ -24,10 +25,12 @@ import {
 import { SLORepository } from './slo_repository';
 import { TransformManager } from './transform_manager';
 import { SecurityHasPrivilegesResponse } from '@elastic/elasticsearch/lib/api/types';
+import { SavedObjectsClientContract } from '@kbn/core-saved-objects-api-server';
 
 describe('CreateSLO', () => {
   let mockEsClient: ElasticsearchClientMock;
   let mockScopedClusterClient: ScopedClusterClientMock;
+  let mockSavedObjectsClient: jest.Mocked<SavedObjectsClientContract>;
   let mockLogger: jest.Mocked<MockedLogger>;
   let mockRepository: jest.Mocked<SLORepository>;
   let mockTransformManager: jest.Mocked<TransformManager>;
@@ -39,6 +42,7 @@ describe('CreateSLO', () => {
   beforeEach(() => {
     mockEsClient = elasticsearchServiceMock.createElasticsearchClient();
     mockScopedClusterClient = elasticsearchServiceMock.createScopedClusterClient();
+    mockSavedObjectsClient = savedObjectsClientMock.create();
     mockLogger = loggingSystemMock.createLogger();
     mockRepository = createSLORepositoryMock();
     mockTransformManager = createTransformManagerMock();
@@ -47,6 +51,7 @@ describe('CreateSLO', () => {
       mockEsClient,
       mockScopedClusterClient,
       mockRepository,
+      mockSavedObjectsClient,
       mockTransformManager,
       mockSummaryTransformManager,
       mockLogger,
@@ -58,10 +63,15 @@ describe('CreateSLO', () => {
 
   describe('happy path', () => {
     beforeEach(() => {
-      mockRepository.exists.mockResolvedValue(false);
       mockEsClient.security.hasPrivileges.mockResolvedValue({
         has_all_requested: true,
       } as SecurityHasPrivilegesResponse);
+      mockSavedObjectsClient.find.mockResolvedValue({
+        saved_objects: [],
+        per_page: 20,
+        page: 0,
+        total: 0,
+      });
     });
 
     it('calls the expected services', async () => {
@@ -168,18 +178,15 @@ describe('CreateSLO', () => {
 
   describe('unhappy path', () => {
     beforeEach(() => {
-      mockRepository.exists.mockResolvedValue(false);
       mockEsClient.security.hasPrivileges.mockResolvedValue({
         has_all_requested: true,
       } as SecurityHasPrivilegesResponse);
-    });
-
-    it('throws a SLOIdConflict error when the SLO already exists', async () => {
-      mockRepository.exists.mockResolvedValue(true);
-
-      const sloParams = createSLOParams({ indicator: createAPMTransactionErrorRateIndicator() });
-
-      await expect(createSLO.execute(sloParams)).rejects.toThrowError(/SLO \[.*\] already exists/);
+      mockSavedObjectsClient.find.mockResolvedValue({
+        saved_objects: [],
+        per_page: 20,
+        page: 0,
+        total: 0,
+      });
     });
 
     it('throws a SecurityException error when the user does not have the required privileges', async () => {

x-pack/solutions/observability/plugins/slo/server/services/create_slo.ts

@@ -4,13 +4,20 @@
  * 2.0; you may not use this file except in compliance with the Elastic License
  * 2.0.
  */
-import { IngestPutPipelineRequest } from '@elastic/elasticsearch/lib/api/types';
+import { IngestPutPipelineRequest } from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
 import { TransformPutTransformRequest } from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
-import { ElasticsearchClient, IBasePath, IScopedClusterClient, Logger } from '@kbn/core/server';
+import {
+  ElasticsearchClient,
+  IBasePath,
+  IScopedClusterClient,
+  Logger,
+  SavedObjectsClientContract,
+} from '@kbn/core/server';
 import { ALL_VALUE, CreateSLOParams, CreateSLOResponse } from '@kbn/slo-schema';
 import { asyncForEach } from '@kbn/std';
 import { merge } from 'lodash';
 import { v4 as uuidv4 } from 'uuid';
+import { ALL_SPACES_ID } from '@kbn/spaces-plugin/common/constants';
 import {
   SLO_MODEL_VERSION,
   SUMMARY_TEMP_INDEX_NAME,
@@ -21,7 +28,7 @@ import {
 } from '../../common/constants';
 import { getSLIPipelineTemplate } from '../assets/ingest_templates/sli_pipeline_template';
 import { getSummaryPipelineTemplate } from '../assets/ingest_templates/summary_pipeline_template';
-import { Duration, DurationUnit, SLODefinition } from '../domain/models';
+import { Duration, DurationUnit, SLODefinition, StoredSLODefinition } from '../domain/models';
 import { validateSLO } from '../domain/services';
 import { SLOIdConflict, SecurityException } from '../errors';
 import { retryTransientEsErrors } from '../utils/retry';
@@ -30,12 +37,14 @@ import { createTempSummaryDocument } from './summary_transform_generator/helpers
 import { TransformManager } from './transform_manager';
 import { assertExpectedIndicatorSourceIndexPrivileges } from './utils/assert_expected_indicator_source_index_privileges';
 import { getTransformQueryComposite } from './utils/get_transform_compite_query';
+import { SO_SLO_TYPE } from '../saved_objects';
 
 export class CreateSLO {
   constructor(
     private esClient: ElasticsearchClient,
     private scopedClusterClient: IScopedClusterClient,
     private repository: SLORepository,
+    private internalSOClient: SavedObjectsClientContract,
     private transformManager: TransformManager,
     private summaryTransformManager: TransformManager,
     private logger: Logger,
@@ -123,7 +132,15 @@ export class CreateSLO {
   }
 
   private async assertSLOInexistant(slo: SLODefinition) {
-    const exists = await this.repository.exists(slo.id);
+    const findResponse = await this.internalSOClient.find<StoredSLODefinition>({
+      type: SO_SLO_TYPE,
+      perPage: 0,
+      filter: `slo.attributes.id:(${slo.id})`,
+      namespaces: [ALL_SPACES_ID],
+    });
+
+    const exists = findResponse.total > 0;
+
     if (exists) {
       throw new SLOIdConflict(`SLO [${slo.id}] already exists`);
     }

x-pack/solutions/observability/plugins/slo/server/services/mocks/index.ts

@@ -50,7 +50,6 @@ const createSLORepositoryMock = (): jest.Mocked<SLORepository> => {
     findAllByIds: jest.fn(),
     deleteById: jest.fn(),
     search: jest.fn(),
-    exists: jest.fn(),
   };
 };
 

x-pack/solutions/observability/plugins/slo/server/services/slo_repository.ts

@@ -16,7 +16,6 @@ import { SLONotFound } from '../errors';
 import { SO_SLO_TYPE } from '../saved_objects';
 
 export interface SLORepository {
-  exists(id: string): Promise<boolean>;
   create(slo: SLODefinition): Promise<SLODefinition>;
   update(slo: SLODefinition): Promise<SLODefinition>;
   findAllByIds(ids: string[]): Promise<SLODefinition[]>;
@@ -32,16 +31,6 @@ export interface SLORepository {
 export class KibanaSavedObjectsSLORepository implements SLORepository {
   constructor(private soClient: SavedObjectsClientContract, private logger: Logger) {}
 
-  async exists(id: string) {
-    const findResponse = await this.soClient.find<StoredSLODefinition>({
-      type: SO_SLO_TYPE,
-      perPage: 0,
-      filter: `slo.attributes.id:(${id})`,
-    });
-
-    return findResponse.total > 0;
-  }
-
   async create(slo: SLODefinition): Promise<SLODefinition> {
     await this.soClient.create<StoredSLODefinition>(SO_SLO_TYPE, toStoredSLO(slo));
     return slo;

x-pack/test/api_integration/deployment_agnostic/apis/observability/slo/create_slo.ts

@@ -17,6 +17,7 @@ import { TransformHelper, createTransformHelper } from './helpers/transform';
 
 export default function ({ getService }: DeploymentAgnosticFtrProviderContext) {
   const esClient = getService('es');
+  const spaceApi = getService('spaces');
   const sloApi = getService('sloApi');
   const logger = getService('log');
   const retry = getService('retry');
@@ -51,6 +52,8 @@ export default function ({ getService }: DeploymentAgnosticFtrProviderContext) {
       await cleanup({ client: esClient, config: DATA_FORGE_CONFIG, logger });
       await sloApi.deleteAllSLOs(adminRoleAuthc);
       await samlAuth.invalidateM2mApiKeyWithRoleScope(adminRoleAuthc);
+      await spaceApi.delete('space1');
+      await spaceApi.delete('space2');
     });
 
     it('creates a new slo and transforms', async () => {
@@ -121,6 +124,40 @@ export default function ({ getService }: DeploymentAgnosticFtrProviderContext) {
       });
     });
 
+    it('creates two SLOs with matching ids across different spaces', async () => {
+      const spaceApiResponse = await spaceApi.create({
+        name: 'space1',
+        id: 'space1',
+        initials: '1',
+      });
+      expect(spaceApiResponse.space).property('id');
+
+      const {
+        space: { id: spaceId1 },
+      } = spaceApiResponse;
+      const sloApiResponse = await sloApi.createWithSpace(
+        DEFAULT_SLO,
+        spaceId1,
+        adminRoleAuthc,
+        200
+      );
+      expect(sloApiResponse).property('id');
+
+      const { id } = sloApiResponse;
+      const spaceApiResponse2 = await spaceApi.create({
+        name: 'space2',
+        id: 'space2',
+        initials: '2',
+      });
+
+      const {
+        space: { id: spaceId2 },
+      } = spaceApiResponse;
+      expect(spaceApiResponse2.space).property('id');
+
+      await sloApi.createWithSpace({ ...DEFAULT_SLO, id }, spaceId2, adminRoleAuthc, 409);
+    });
+
     describe('groupBy smoke tests', () => {
       it('creates instanceId for SLOs with multi groupBy', async () => {
         const apiResponse = await sloApi.create(

x-pack/test/api_integration/deployment_agnostic/services/slo_api.ts

@@ -24,6 +24,21 @@ export function SloApiProvider({ getService }: DeploymentAgnosticFtrProviderCont
       return body;
     },
 
+    async createWithSpace(
+      slo: CreateSLOInput & { id?: string },
+      spaceId: string,
+      roleAuthc: RoleCredentials,
+      expectedStatus: 200 | 409
+    ) {
+      const { body } = await supertestWithoutAuth
+        .post(`/s/${spaceId}/api/observability/slos`)
+        .set(roleAuthc.apiKeyHeader)
+        .set(samlAuth.getInternalRequestHeader())
+        .send(slo)
+        .expect(expectedStatus);
+      return body;
+    },
+
     async reset(id: string, roleAuthc: RoleCredentials) {
       const { body } = await supertestWithoutAuth
         .post(`/api/observability/slos/${id}/_reset`)