Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ftr] migrating 'management/ingest_pipelines' API integration test to deployment-agnostic one #192063

1 change: 0 additions & 1 deletion x-pack/test/api_integration/apis/management/index.js
Original file line number Diff line number Diff line change
@@ -13,7 +13,6 @@ export default function ({ loadTestFile }) {
loadTestFile(require.resolve('./rollup'));
loadTestFile(require.resolve('./index_management'));
loadTestFile(require.resolve('./index_lifecycle_management'));
loadTestFile(require.resolve('./ingest_pipelines'));
loadTestFile(require.resolve('./snapshot_restore'));
});
}
Original file line number Diff line number Diff line change
@@ -5,10 +5,10 @@
* 2.0.
*/

import { FtrProviderContext } from '../../../ftr_provider_context';
import { DeploymentAgnosticFtrProviderContext } from '../../ftr_provider_context';

export default function ({ loadTestFile }: FtrProviderContext) {
describe('Ingest Pipelines', () => {
export default function ({ loadTestFile }: DeploymentAgnosticFtrProviderContext) {
describe('Management', () => {
loadTestFile(require.resolve('./ingest_pipelines'));
});
}
Original file line number Diff line number Diff line change
@@ -6,27 +6,33 @@
*/

import expect from '@kbn/expect';

import { IngestPutPipelineRequest } from '@elastic/elasticsearch/lib/api/types';
import { DeploymentAgnosticFtrProviderContext } from '../../ftr_provider_context';
import { SupertestWithRoleScopeType } from '../../services';

import { FtrProviderContext } from '../../../ftr_provider_context';

export default function ({ getService }: FtrProviderContext) {
const supertest = getService('supertest');
export default function ({ getService }: DeploymentAgnosticFtrProviderContext) {
const ingestPipelines = getService('ingestPipelines');
const roleScopedSupertest = getService('roleScopedSupertest');
const log = getService('log');
let supertestWithAdminScope: SupertestWithRoleScopeType;

describe('Ingest Pipelines', function () {
before(async () => {
supertestWithAdminScope = await roleScopedSupertest.getSupertestWithRoleScope('admin', {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this have the same privileges as supertest from the original test? Just wanted to make sure we don't give additional privileges that are not needed.

Copy link
Member Author

@dmlemeshko dmlemeshko Sep 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

supertest is inited with system_indices_superuser role, which in stateful grants absolutely "everything". In the past it hided some bugs, and generally it is not recommended to use that role for API calls testing, but create something scoped to the particular functionality.

You can see both role definitions here https://github.com/elastic/kibana/blob/main/packages/kbn-es/src/stateful_resources/roles.yml#L99-L130 , admin is a default Cloud role (Cloud Org admin) and should be able to perform all the possible operations with Kibana.

But if you think ingesting pipeline should be available with lower permissions (editor) , we should definitely change it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for clarifying! I think leaving it with admin role is okay given that the privileges in the original test were even higher. cc: @alisonelizabeth

withInternalHeaders: true,
});
});

describe('Pipelines', function () {
after(async () => {
await ingestPipelines.api.deletePipelines();
await supertestWithAdminScope.destroy();
});

describe('Create', () => {
it('should create a pipeline', async () => {
const pipelineRequestBody = ingestPipelines.fixtures.createPipelineBody();
const { body } = await supertest
const { body } = await supertestWithAdminScope
.post(ingestPipelines.fixtures.apiBasePath)
.set('kbn-xsrf', 'xxx')
.send(pipelineRequestBody)
.expect(200);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we keep the .expect(200) part?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!


@@ -37,10 +43,8 @@ export default function ({ getService }: FtrProviderContext) {

it('should create a pipeline with only required fields', async () => {
const pipelineRequestBody = ingestPipelines.fixtures.createPipelineBodyWithRequiredFields(); // Includes name and processors[] only

const { body } = await supertest
const { body } = await supertestWithAdminScope
.post(ingestPipelines.fixtures.apiBasePath)
.set('kbn-xsrf', 'xxx')
.send(pipelineRequestBody)
.expect(200);

@@ -51,16 +55,12 @@ export default function ({ getService }: FtrProviderContext) {

it('should not allow creation of an existing pipeline', async () => {
const pipelineRequestBody = ingestPipelines.fixtures.createPipelineBodyWithRequiredFields(); // Includes name and processors[] only

const { name, ...esPipelineRequestBody } = pipelineRequestBody;

// First, create a pipeline using the ES API
await ingestPipelines.api.createPipeline({ id: name, ...esPipelineRequestBody });

// Then, create a pipeline with our internal API
const { body } = await supertest
const { body } = await supertestWithAdminScope
.post(ingestPipelines.fixtures.apiBasePath)
.set('kbn-xsrf', 'xxx')
.send(pipelineRequestBody)
.expect(409);

@@ -75,9 +75,8 @@ export default function ({ getService }: FtrProviderContext) {
const pipelineRequestBody = {
name: 'testtesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttesttest1',
};
await supertest
await supertestWithAdminScope
.post(ingestPipelines.fixtures.apiBasePath)
.set('kbn-xsrf', 'xxx')
.send(pipelineRequestBody)
.expect(400);
});
@@ -105,10 +104,8 @@ export default function ({ getService }: FtrProviderContext) {

it('should allow an existing pipeline to be updated', async () => {
const uri = `${ingestPipelines.fixtures.apiBasePath}/${pipelineName}`;

const { body } = await supertest
const { body } = await supertestWithAdminScope
.put(uri)
.set('kbn-xsrf', 'xxx')
.send({
...pipeline,
description: 'updated test pipeline description',
@@ -126,10 +123,8 @@ export default function ({ getService }: FtrProviderContext) {

it('should allow optional fields to be removed', async () => {
const uri = `${ingestPipelines.fixtures.apiBasePath}/${pipelineName}`;

const { body } = await supertest
const { body } = await supertestWithAdminScope
.put(uri)
.set('kbn-xsrf', 'xxx')
.send({
// removes description, version, on_failure, and _meta
processors: pipeline.processors,
@@ -143,10 +138,8 @@ export default function ({ getService }: FtrProviderContext) {

it('should not allow a non-existing pipeline to be updated', async () => {
const uri = `${ingestPipelines.fixtures.apiBasePath}/pipeline_does_not_exist`;

const { body } = await supertest
const { body } = await supertestWithAdminScope
.put(uri)
.set('kbn-xsrf', 'xxx')
.send({
...pipeline,
description: 'updated test pipeline description',
@@ -188,9 +181,8 @@ export default function ({ getService }: FtrProviderContext) {

describe('all pipelines', () => {
it('should return an array of pipelines', async () => {
const { body } = await supertest
const { body } = await supertestWithAdminScope
.get(ingestPipelines.fixtures.apiBasePath)
.set('kbn-xsrf', 'xxx')
.expect(200);

expect(Array.isArray(body)).to.be(true);
@@ -210,8 +202,7 @@ export default function ({ getService }: FtrProviderContext) {
describe('one pipeline', () => {
it('should return a single pipeline', async () => {
const uri = `${ingestPipelines.fixtures.apiBasePath}/${pipelineName}`;

const { body } = await supertest.get(uri).set('kbn-xsrf', 'xxx').expect(200);
const { body } = await supertestWithAdminScope.get(uri).expect(200);

expect(body).to.eql({
...pipeline,
@@ -246,10 +237,8 @@ export default function ({ getService }: FtrProviderContext) {

it('should delete a pipeline', async () => {
const pipelineA = pipelineIds[0];

const uri = `${ingestPipelines.fixtures.apiBasePath}/${pipelineA}`;

const { body } = await supertest.delete(uri).set('kbn-xsrf', 'xxx').expect(200);
const { body } = await supertestWithAdminScope.delete(uri).expect(200);

expect(body).to.eql({
itemsDeleted: [pipelineA],
@@ -260,11 +249,10 @@ export default function ({ getService }: FtrProviderContext) {
it('should delete multiple pipelines', async () => {
const pipelineB = pipelineIds[1];
const pipelineC = pipelineIds[2];
const uri = `${ingestPipelines.fixtures.apiBasePath}/${pipelineIds[1]},${pipelineIds[2]}`;

const uri = `${ingestPipelines.fixtures.apiBasePath}/${pipelineB},${pipelineC}`;
const {
body: { itemsDeleted, errors },
} = await supertest.delete(uri).set('kbn-xsrf', 'xxx').expect(200);
} = await supertestWithAdminScope.delete(uri).expect(200);

expect(errors).to.eql([]);

@@ -277,10 +265,8 @@ export default function ({ getService }: FtrProviderContext) {
it('should return an error for any pipelines not sucessfully deleted', async () => {
const PIPELINE_DOES_NOT_EXIST = 'pipeline_does_not_exist';
const pipelineD = pipelineIds[3];

const uri = `${ingestPipelines.fixtures.apiBasePath}/${pipelineD},${PIPELINE_DOES_NOT_EXIST}`;

const { body } = await supertest.delete(uri).set('kbn-xsrf', 'xxx').expect(200);
const { body } = await supertestWithAdminScope.delete(uri).expect(200);

expect(body).to.eql({
itemsDeleted: [pipelineD],
@@ -308,9 +294,8 @@ export default function ({ getService }: FtrProviderContext) {
it('should successfully simulate a pipeline', async () => {
const { name, ...pipeline } = ingestPipelines.fixtures.createPipelineBody();
const documents = ingestPipelines.fixtures.createDocuments();
const { body } = await supertest
const { body } = await supertestWithAdminScope
.post(`${ingestPipelines.fixtures.apiBasePath}/simulate`)
.set('kbn-xsrf', 'xxx')
.send({
pipeline,
documents,
@@ -326,9 +311,8 @@ export default function ({ getService }: FtrProviderContext) {
const { name, ...pipeline } =
ingestPipelines.fixtures.createPipelineBodyWithRequiredFields();
const documents = ingestPipelines.fixtures.createDocuments();
const { body } = await supertest
const { body } = await supertestWithAdminScope
.post(`${ingestPipelines.fixtures.apiBasePath}/simulate`)
.set('kbn-xsrf', 'xxx')
.send({
pipeline,
documents,
@@ -370,8 +354,7 @@ export default function ({ getService }: FtrProviderContext) {

it('should return a document', async () => {
const uri = `${ingestPipelines.fixtures.apiBasePath}/documents/${INDEX}/${DOCUMENT_ID}`;

const { body } = await supertest.get(uri).set('kbn-xsrf', 'xxx').expect(200);
const { body } = await supertestWithAdminScope.get(uri).expect(200);

expect(body).to.eql({
_index: INDEX,
@@ -382,8 +365,7 @@ export default function ({ getService }: FtrProviderContext) {

it('should return an error if the document does not exist', async () => {
const uri = `${ingestPipelines.fixtures.apiBasePath}/documents/${INDEX}/2`; // Document 2 does not exist

const { body } = await supertest.get(uri).set('kbn-xsrf', 'xxx').expect(404);
const { body } = await supertestWithAdminScope.get(uri).expect(404);

expect(body).to.eql({
error: 'Not Found',
@@ -398,9 +380,8 @@ export default function ({ getService }: FtrProviderContext) {
it('should map to a pipeline', async () => {
const validCsv =
'source_field,copy_action,format_action,timestamp_format,destination_field,Notes\nsrcip,,,,source.address,Copying srcip to source.address';
const { body } = await supertest
const { body } = await supertestWithAdminScope
.post(`${ingestPipelines.fixtures.apiBasePath}/parse_csv`)
.set('kbn-xsrf', 'xxx')
.send({
copyAction: 'copy',
file: validCsv,
Original file line number Diff line number Diff line change
@@ -11,6 +11,7 @@ export default function ({ loadTestFile }: DeploymentAgnosticFtrProviderContext)
// load new oblt and platform deployment-agnostic test here
loadTestFile(require.resolve('../../apis/console'));
loadTestFile(require.resolve('../../apis/core'));
loadTestFile(require.resolve('../../apis/management'));
loadTestFile(require.resolve('../../apis/painless_lab'));
loadTestFile(require.resolve('../../apis/observability/alerting'));
});
Original file line number Diff line number Diff line change
@@ -11,5 +11,6 @@ export default function ({ loadTestFile }: DeploymentAgnosticFtrProviderContext)
// load new search and platform deployment-agnostic test here
loadTestFile(require.resolve('../../apis/console'));
loadTestFile(require.resolve('../../apis/core'));
loadTestFile(require.resolve('../../apis/management'));
});
}
Original file line number Diff line number Diff line change
@@ -11,6 +11,7 @@ export default function ({ loadTestFile }: DeploymentAgnosticFtrProviderContext)
// load new security and platform deployment-agnostic test here
loadTestFile(require.resolve('../../apis/console'));
loadTestFile(require.resolve('../../apis/core'));
loadTestFile(require.resolve('../../apis/management'));
loadTestFile(require.resolve('../../apis/painless_lab'));
});
}
Original file line number Diff line number Diff line change
@@ -12,6 +12,7 @@ export default function ({ loadTestFile }: DeploymentAgnosticFtrProviderContext)
// load new platform deployment-agnostic test here
loadTestFile(require.resolve('../../apis/console'));
loadTestFile(require.resolve('../../apis/core'));
loadTestFile(require.resolve('../../apis/management'));
loadTestFile(require.resolve('../../apis/painless_lab'));
});
}
Original file line number Diff line number Diff line change
@@ -5,11 +5,8 @@
* 2.0.
*/

import { FtrProviderContext } from '../ftr_provider_context';
import {
IngestPipelinesAPIProvider,
IngestPipelinesFixturesProvider,
} from '../apis/management/ingest_pipelines/lib';
import { FtrProviderContext } from '../../ftr_provider_context';
import { IngestPipelinesAPIProvider, IngestPipelinesFixturesProvider } from './lib';

export function IngestPipelinesProvider(context: FtrProviderContext) {
const api = IngestPipelinesAPIProvider(context);
Original file line number Diff line number Diff line change
@@ -7,8 +7,7 @@

import { IngestPutPipelineRequest } from '@elastic/elasticsearch/lib/api/types';
import expect from '@kbn/expect';

import { FtrProviderContext } from '../../../../ftr_provider_context';
import { FtrProviderContext } from '../../../ftr_provider_context';

export function IngestPipelinesAPIProvider({ getService }: FtrProviderContext) {
const es = getService('es');
Original file line number Diff line number Diff line change
@@ -11,7 +11,6 @@ export default function ({ loadTestFile }: FtrProviderContext) {
describe('Management', function () {
this.tags(['esGate']);

loadTestFile(require.resolve('./ingest_pipelines'));
loadTestFile(require.resolve('./rollups'));
loadTestFile(require.resolve('./scripted_fields'));
loadTestFile(require.resolve('./spaces'));

This file was deleted.