diff --git a/x-pack/solutions/observability/plugins/slo/server/routes/slo/create_slo.ts b/x-pack/solutions/observability/plugins/slo/server/routes/slo/create_slo.ts index 857a8c8987373..304f03221645a 100644 --- a/x-pack/solutions/observability/plugins/slo/server/routes/slo/create_slo.ts +++ b/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, diff --git a/x-pack/solutions/observability/plugins/slo/server/routes/slo/inspect_slo.ts b/x-pack/solutions/observability/plugins/slo/server/routes/slo/inspect_slo.ts index e4af6958d310e..fb9738cfbb293 100644 --- a/x-pack/solutions/observability/plugins/slo/server/routes/slo/inspect_slo.ts +++ b/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, diff --git a/x-pack/solutions/observability/plugins/slo/server/services/create_slo.test.ts b/x-pack/solutions/observability/plugins/slo/server/services/create_slo.test.ts index e112bfbaf2d3d..e5c9b3917da3c 100644 --- a/x-pack/solutions/observability/plugins/slo/server/services/create_slo.test.ts +++ b/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; let mockLogger: jest.Mocked; let mockRepository: jest.Mocked; let mockTransformManager: jest.Mocked; @@ -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 () => { diff --git a/x-pack/solutions/observability/plugins/slo/server/services/create_slo.ts b/x-pack/solutions/observability/plugins/slo/server/services/create_slo.ts index 800d6da001cc4..ae5748e3a3ac4 100644 --- a/x-pack/solutions/observability/plugins/slo/server/services/create_slo.ts +++ b/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({ + 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`); } diff --git a/x-pack/solutions/observability/plugins/slo/server/services/mocks/index.ts b/x-pack/solutions/observability/plugins/slo/server/services/mocks/index.ts index 0b3c1d4d07453..eee093686fe25 100644 --- a/x-pack/solutions/observability/plugins/slo/server/services/mocks/index.ts +++ b/x-pack/solutions/observability/plugins/slo/server/services/mocks/index.ts @@ -50,7 +50,6 @@ const createSLORepositoryMock = (): jest.Mocked => { findAllByIds: jest.fn(), deleteById: jest.fn(), search: jest.fn(), - exists: jest.fn(), }; }; diff --git a/x-pack/solutions/observability/plugins/slo/server/services/slo_repository.ts b/x-pack/solutions/observability/plugins/slo/server/services/slo_repository.ts index d3bdc04f830e6..5c531c1e6d616 100644 --- a/x-pack/solutions/observability/plugins/slo/server/services/slo_repository.ts +++ b/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; create(slo: SLODefinition): Promise; update(slo: SLODefinition): Promise; findAllByIds(ids: string[]): Promise; @@ -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({ - type: SO_SLO_TYPE, - perPage: 0, - filter: `slo.attributes.id:(${id})`, - }); - - return findResponse.total > 0; - } - async create(slo: SLODefinition): Promise { await this.soClient.create(SO_SLO_TYPE, toStoredSLO(slo)); return slo; diff --git a/x-pack/test/api_integration/deployment_agnostic/apis/observability/slo/create_slo.ts b/x-pack/test/api_integration/deployment_agnostic/apis/observability/slo/create_slo.ts index 40d6969e406b1..fc9ba56984545 100644 --- a/x-pack/test/api_integration/deployment_agnostic/apis/observability/slo/create_slo.ts +++ b/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( diff --git a/x-pack/test/api_integration/deployment_agnostic/services/slo_api.ts b/x-pack/test/api_integration/deployment_agnostic/services/slo_api.ts index 8ee202b2cf23e..87658972929e9 100644 --- a/x-pack/test/api_integration/deployment_agnostic/services/slo_api.ts +++ b/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`)