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

# Backport

This will backport the following commits from `main` to `9.0`:
- [[SLO] Check for unique SLO ids across spaces
(#214496)](https://github.com/elastic/kibana/pull/214496)

<!--- Backport version: 9.6.6 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sorenlouv/backport)

<!--BACKPORT [{"author":{"name":"Bailey
Cash","email":"bailey.cash@elastic.co"},"sourceCommit":{"committedDate":"2025-03-18T14:44:58Z","message":"[SLO]
Check for unique SLO ids across spaces (#214496)\n\n## Summary
\nResolves #212784 \nEnsure that when an SLO is created, the id is
verified across all\nspaces.\n\n## Release Notes\nEnsure that when an
SLO is created, the id is verified across all\nspaces.\n\n## Testing\n1.
Create an SLO and save the id returned in the response in a space
\"A\"\n2. Create a second SLO with the id saved from the first SLO in
the\nrequest in a different space \"B\"\n3. User should receive a 409
error from the SLO API.\n\n---------\n\nCo-authored-by: kibanamachine
<42973632+kibanamachine@users.noreply.github.com>\nCo-authored-by:
Elastic Machine
<elasticmachine@users.noreply.github.com>","sha":"56f1ebfca6300b1da68cf6fa721f450077aa1878","branchLabelMapping":{"^v9.1.0$":"main","^v8.19.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:Security","backport
missing","backport:prev-minor","Team:obs-ux-management","v9.1.0"],"title":"[SLO]
Check for unique SLO ids across
spaces","number":214496,"url":"https://github.com/elastic/kibana/pull/214496","mergeCommit":{"message":"[SLO]
Check for unique SLO ids across spaces (#214496)\n\n## Summary
\nResolves #212784 \nEnsure that when an SLO is created, the id is
verified across all\nspaces.\n\n## Release Notes\nEnsure that when an
SLO is created, the id is verified across all\nspaces.\n\n## Testing\n1.
Create an SLO and save the id returned in the response in a space
\"A\"\n2. Create a second SLO with the id saved from the first SLO in
the\nrequest in a different space \"B\"\n3. User should receive a 409
error from the SLO API.\n\n---------\n\nCo-authored-by: kibanamachine
<42973632+kibanamachine@users.noreply.github.com>\nCo-authored-by:
Elastic Machine
<elasticmachine@users.noreply.github.com>","sha":"56f1ebfca6300b1da68cf6fa721f450077aa1878"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/214496","number":214496,"mergeCommit":{"message":"[SLO]
Check for unique SLO ids across spaces (#214496)\n\n## Summary
\nResolves #212784 \nEnsure that when an SLO is created, the id is
verified across all\nspaces.\n\n## Release Notes\nEnsure that when an
SLO is created, the id is verified across all\nspaces.\n\n## Testing\n1.
Create an SLO and save the id returned in the response in a space
\"A\"\n2. Create a second SLO with the id saved from the first SLO in
the\nrequest in a different space \"B\"\n3. User should receive a 409
error from the SLO API.\n\n---------\n\nCo-authored-by: kibanamachine
<42973632+kibanamachine@users.noreply.github.com>\nCo-authored-by:
Elastic Machine
<elasticmachine@users.noreply.github.com>","sha":"56f1ebfca6300b1da68cf6fa721f450077aa1878"}}]}]
BACKPORT-->

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
This commit is contained in:
Bailey Cash 2025-03-21 12:07:37 -04:00 committed by GitHub
parent 5ac71db387
commit 6fe8399078
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 102 additions and 26 deletions

View file

@ -6,6 +6,7 @@
*/
import { createSLOParamsSchema } from '@kbn/slo-schema';
import { SavedObjectsClient } from '@kbn/core/server';
import { createSloServerRoute } from '../create_slo_server_route';
import { assertPlatinumLicense } from './utils/assert_platinum_license';
import { getSpaceId } from './utils/get_space_id';
@ -38,6 +39,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);
@ -66,6 +71,7 @@ export const createSLORoute = createSloServerRoute({
esClient,
scopedClusterClient,
repository,
internalSoClient,
transformManager,
summaryTransformManager,
logger,

View file

@ -6,6 +6,7 @@
*/
import { createSLOParamsSchema } from '@kbn/slo-schema';
import { SavedObjectsClient } from '@kbn/core/server';
import { executeWithErrorHandler } from '../../errors';
import {
CreateSLO,
@ -40,6 +41,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);
@ -63,6 +68,7 @@ export const inspectSLORoute = createSloServerRoute({
esClient,
scopedClusterClient,
repository,
internalSoClient,
transformManager,
summaryTransformManager,
logger,

View file

@ -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 () => {

View file

@ -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`);
}

View file

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

View file

@ -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;

View file

@ -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');
@ -55,6 +56,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 () => {
@ -125,6 +128,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(

View file

@ -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`)