Skip to content

Commit

Permalink
feat(slo): Assert user has correct source index privileges when creat…
Browse files Browse the repository at this point in the history 
…ing, updating or reseting an SLO (#199233)
  • Loading branch information
@kdelemme
kdelemme authored Nov 12, 2024
1 parent a613277 commit da85efe
Show file tree
Hide file tree
Showing 11 changed files with 194 additions and 68 deletions.
22 changes: 11 additions & 11 deletions ...k/plugins/observability_solution/slo/server/services/__snapshots__/reset_slo.test.ts.snap
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

36 changes: 36 additions & 0 deletions x-pack/plugins/observability_solution/slo/server/services/create_slo.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
} from './mocks';
import { SLORepository } from './slo_repository';
import { TransformManager } from './transform_manager';
import { SecurityHasPrivilegesResponse } from '@elastic/elasticsearch/lib/api/types';

describe('CreateSLO', () => {
let mockEsClient: ElasticsearchClientMock;
Expand Down Expand Up @@ -55,11 +56,19 @@ describe('CreateSLO', () => {
});

describe('happy path', () => {
beforeEach(() => {
mockRepository.exists.mockResolvedValue(false);
mockEsClient.security.hasPrivileges.mockResolvedValue({
has_all_requested: true,
} as SecurityHasPrivilegesResponse);
});

it('calls the expected services', async () => {
const sloParams = createSLOParams({
id: 'unique-id',
indicator: createAPMTransactionErrorRateIndicator(),
});

mockTransformManager.install.mockResolvedValue('slo-id-revision');
mockSummaryTransformManager.install.mockResolvedValue('slo-summary-id-revision');

Expand Down Expand Up @@ -157,6 +166,33 @@ 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/);
});

it('throws a SecurityException error when the user does not have the required privileges', async () => {
mockEsClient.security.hasPrivileges.mockResolvedValue({
has_all_requested: false,
} as SecurityHasPrivilegesResponse);

const sloParams = createSLOParams({ indicator: createAPMTransactionErrorRateIndicator() });

await expect(createSLO.execute(sloParams)).rejects.toThrowError(
"Missing ['read', 'view_index_metadata'] privileges on the source index [metrics-apm*]"
);
});

it('rollbacks completed operations when rollup transform install fails', async () => {
mockTransformManager.install.mockRejectedValue(new Error('Rollup transform install error'));
const sloParams = createSLOParams({ indicator: createAPMTransactionErrorRateIndicator() });
Expand Down
29 changes: 15 additions & 14 deletions x-pack/plugins/observability_solution/slo/server/services/create_slo.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,30 +4,30 @@
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
import { IScopedClusterClient } from '@kbn/core/server';
import { IngestPutPipelineRequest } from '@elastic/elasticsearch/lib/api/types';
import { TransformPutTransformRequest } from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
import { ElasticsearchClient, IBasePath, Logger } from '@kbn/core/server';
import { ElasticsearchClient, IBasePath, IScopedClusterClient, Logger } from '@kbn/core/server';
import { ALL_VALUE, CreateSLOParams, CreateSLOResponse } from '@kbn/slo-schema';
import { asyncForEach } from '@kbn/std';
import { v4 as uuidv4 } from 'uuid';
import { IngestPutPipelineRequest } from '@elastic/elasticsearch/lib/api/types';
import {
SLO_MODEL_VERSION,
SLO_SUMMARY_TEMP_INDEX_NAME,
getSLOPipelineId,
getSLOSummaryPipelineId,
getSLOSummaryTransformId,
getSLOTransformId,
SLO_MODEL_VERSION,
SLO_SUMMARY_TEMP_INDEX_NAME,
} from '../../common/constants';
import { getSLOPipelineTemplate } from '../assets/ingest_templates/slo_pipeline_template';
import { getSLOSummaryPipelineTemplate } from '../assets/ingest_templates/slo_summary_pipeline_template';
import { Duration, DurationUnit, SLODefinition } from '../domain/models';
import { validateSLO } from '../domain/services';
import { SecurityException, SLOIdConflict } from '../errors';
import { SLOIdConflict, SecurityException } from '../errors';
import { retryTransientEsErrors } from '../utils/retry';
import { SLORepository } from './slo_repository';
import { createTempSummaryDocument } from './summary_transform_generator/helpers/create_temp_summary';
import { TransformManager } from './transform_manager';
import { assertExpectedIndicatorSourceIndexPrivileges } from './utils/assert_expected_indicator_source_index_privileges';
import { getTransformQueryComposite } from './utils/get_transform_compite_query';

export class CreateSLO {
Expand All @@ -46,16 +46,11 @@ export class CreateSLO {
const slo = this.toSLO(params);
validateSLO(slo);

const rollbackOperations = [];

const sloAlreadyExists = await this.repository.checkIfSLOExists(slo);

if (sloAlreadyExists) {
throw new SLOIdConflict(`SLO [${slo.id}] already exists`);
}
await this.assertSLOInexistant(slo);
await assertExpectedIndicatorSourceIndexPrivileges(slo, this.esClient);

const rollbackOperations = [];
const createPromise = this.repository.create(slo);

rollbackOperations.push(() => this.repository.deleteById(slo.id, true));

const rollupTransformId = getSLOTransformId(slo.id, slo.revision);
Expand Down Expand Up @@ -123,6 +118,12 @@ export class CreateSLO {
return this.toResponse(slo);
}

private async assertSLOInexistant(slo: SLODefinition) {
const exists = await this.repository.exists(slo.id);
if (exists) {
throw new SLOIdConflict(`SLO [${slo.id}] already exists`);
}
}
async createTempSummaryDocument(slo: SLODefinition) {
return await retryTransientEsErrors(
() =>
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/observability_solution/slo/server/services/mocks/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ const createSLORepositoryMock = (): jest.Mocked<SLORepository> => {
findAllByIds: jest.fn(),
deleteById: jest.fn(),
search: jest.fn(),
checkIfSLOExists: jest.fn(),
exists: jest.fn(),
};
};

Expand Down
75 changes: 50 additions & 25 deletions x-pack/plugins/observability_solution/slo/server/services/reset_slo.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,15 @@
* 2.0.
*/

import { ElasticsearchClient } from '@kbn/core/server';
import { SecurityHasPrivilegesResponse } from '@elastic/elasticsearch/lib/api/types';
import {
ElasticsearchClientMock,
elasticsearchServiceMock,
httpServiceMock,
loggingSystemMock,
ScopedClusterClientMock,
} from '@kbn/core/server/mocks';
import { MockedLogger } from '@kbn/logging-mocks';

import { SLO_MODEL_VERSION } from '../../common/constants';
import { createSLO } from './fixtures/slo';
import {
Expand All @@ -31,7 +31,7 @@ describe('ResetSLO', () => {
let mockRepository: jest.Mocked<SLORepository>;
let mockTransformManager: jest.Mocked<TransformManager>;
let mockSummaryTransformManager: jest.Mocked<TransformManager>;
let mockEsClient: jest.Mocked<ElasticsearchClient>;
let mockEsClient: ElasticsearchClientMock;
let mockScopedClusterClient: ScopedClusterClientMock;
let loggerMock: jest.Mocked<MockedLogger>;
let resetSLO: ResetSLO;
Expand Down Expand Up @@ -60,37 +60,62 @@ describe('ResetSLO', () => {
jest.useRealTimers();
});

it('resets all associated resources', async () => {
const slo = createSLO({ id: 'irrelevant', version: 1 });
mockRepository.findById.mockResolvedValueOnce(slo);
mockRepository.update.mockImplementation((v) => Promise.resolve(v));
describe('happy path', () => {
beforeEach(() => {
mockEsClient.security.hasPrivileges.mockResolvedValue({
has_all_requested: true,
} as SecurityHasPrivilegesResponse);
});

it('resets all associated resources', async () => {
const slo = createSLO({ id: 'irrelevant', version: 1 });
mockRepository.findById.mockResolvedValueOnce(slo);
mockRepository.update.mockImplementation((v) => Promise.resolve(v));

await resetSLO.execute(slo.id);

// delete existing resources and data
expect(mockSummaryTransformManager.stop).toMatchSnapshot();
expect(mockSummaryTransformManager.uninstall).toMatchSnapshot();

await resetSLO.execute(slo.id);
expect(mockTransformManager.stop).toMatchSnapshot();
expect(mockTransformManager.uninstall).toMatchSnapshot();

// delete existing resources and data
expect(mockSummaryTransformManager.stop).toMatchSnapshot();
expect(mockSummaryTransformManager.uninstall).toMatchSnapshot();
expect(mockEsClient.deleteByQuery).toMatchSnapshot();

expect(mockTransformManager.stop).toMatchSnapshot();
expect(mockTransformManager.uninstall).toMatchSnapshot();
// install resources
expect(mockSummaryTransformManager.install).toMatchSnapshot();
expect(mockSummaryTransformManager.start).toMatchSnapshot();

expect(mockEsClient.deleteByQuery).toMatchSnapshot();
expect(mockScopedClusterClient.asSecondaryAuthUser.ingest.putPipeline).toMatchSnapshot();

// install resources
expect(mockSummaryTransformManager.install).toMatchSnapshot();
expect(mockSummaryTransformManager.start).toMatchSnapshot();
expect(mockTransformManager.install).toMatchSnapshot();
expect(mockTransformManager.start).toMatchSnapshot();

expect(mockScopedClusterClient.asSecondaryAuthUser.ingest.putPipeline).toMatchSnapshot();
expect(mockEsClient.index).toMatchSnapshot();

expect(mockTransformManager.install).toMatchSnapshot();
expect(mockTransformManager.start).toMatchSnapshot();
expect(mockRepository.update).toHaveBeenCalledWith({
...slo,
version: SLO_MODEL_VERSION,
updatedAt: expect.anything(),
});
});
});

describe('unhappy path', () => {
beforeEach(() => {
mockEsClient.security.hasPrivileges.mockResolvedValue({
has_all_requested: false,
} as SecurityHasPrivilegesResponse);
});

expect(mockEsClient.index).toMatchSnapshot();
it('throws a SecurityException error when the user does not have the required privileges', async () => {
const slo = createSLO({ id: 'irrelevant', version: 1 });
mockRepository.findById.mockResolvedValueOnce(slo);

expect(mockRepository.update).toHaveBeenCalledWith({
...slo,
version: SLO_MODEL_VERSION,
updatedAt: expect.anything(),
await expect(resetSLO.execute(slo.id)).rejects.toThrowError(
"Missing ['read', 'view_index_metadata'] privileges on the source index [metrics-apm*]"
);
});
});
});
13 changes: 8 additions & 5 deletions x-pack/plugins/observability_solution/slo/server/services/reset_slo.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,24 +5,25 @@
* 2.0.
*/

import { ElasticsearchClient, IBasePath, Logger, IScopedClusterClient } from '@kbn/core/server';
import { ElasticsearchClient, IBasePath, IScopedClusterClient, Logger } from '@kbn/core/server';
import { resetSLOResponseSchema } from '@kbn/slo-schema';
import {
getSLOPipelineId,
getSLOSummaryPipelineId,
getSLOSummaryTransformId,
getSLOTransformId,
SLO_DESTINATION_INDEX_PATTERN,
SLO_MODEL_VERSION,
SLO_SUMMARY_DESTINATION_INDEX_PATTERN,
SLO_SUMMARY_TEMP_INDEX_NAME,
getSLOPipelineId,
getSLOSummaryPipelineId,
getSLOSummaryTransformId,
getSLOTransformId,
} from '../../common/constants';
import { getSLOPipelineTemplate } from '../assets/ingest_templates/slo_pipeline_template';
import { getSLOSummaryPipelineTemplate } from '../assets/ingest_templates/slo_summary_pipeline_template';
import { retryTransientEsErrors } from '../utils/retry';
import { SLORepository } from './slo_repository';
import { createTempSummaryDocument } from './summary_transform_generator/helpers/create_temp_summary';
import { TransformManager } from './transform_manager';
import { assertExpectedIndicatorSourceIndexPrivileges } from './utils/assert_expected_indicator_source_index_privileges';

export class ResetSLO {
constructor(
Expand All @@ -39,6 +40,8 @@ export class ResetSLO {
public async execute(sloId: string) {
const slo = await this.repository.findById(sloId);

await assertExpectedIndicatorSourceIndexPrivileges(slo, this.esClient);

const summaryTransformId = getSLOSummaryTransformId(slo.id, slo.revision);
await this.summaryTransformManager.stop(summaryTransformId);
await this.summaryTransformManager.uninstall(summaryTransformId);
Expand Down
Loading

0 comments on commit da85efe

Please sign in to comment.