Skip to content

Commit

Permalink
[Fleet] Fix error when creating new logstash output (elastic#153752)
Browse files Browse the repository at this point in the history 
Fixes elastic#153622

## Summary
[Fleet] Fix an error when creating new logstash output.

The bug was added with elastic#153226 and
depended on this [else
branch](https://github.com/criamico/kibana/blob/30d3843b142142d19f1fda8f8107f088c03ff1cb/x-pack/plugins/fleet/server/services/output.ts#L309-L312)
that shouldn't have been added.

I also added unit and integration tests to cover for this and other
cases.

### Testing
- Create a new `logstash` output, it can be `default` or not
- The output creation works as expected in both cases

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
(cherry picked from commit 2e08470)
  • Loading branch information
@criamico
criamico committed Mar 28, 2023
1 parent 09abc78 commit 169be89
Show file tree
Hide file tree
Showing 3 changed files with 295 additions and 10 deletions.
64 changes: 62 additions & 2 deletions x-pack/plugins/fleet/server/services/output.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,7 @@ describe('Output Service', () => {
expect(soClient.create).toBeCalled();
});

it('Should update fleet server policies with data_output_id=default_output_id if a default new logstash output is created', async () => {
it('Should update fleet server policies with data_output_id=default_output_id if a new default logstash output is created', async () => {
const soClient = getMockedSoClient({
defaultOutputId: 'output-test',
});
Expand Down Expand Up @@ -434,6 +434,66 @@ describe('Output Service', () => {
{ force: true }
);
});

it('Should allow to create a new logstash output with no errors if is not set as default', async () => {
const soClient = getMockedSoClient({
defaultOutputId: 'output-test',
});
mockedAppContextService.getEncryptedSavedObjectsSetup.mockReturnValue({
canEncrypt: true,
} as any);
mockedAgentPolicyService.list.mockResolvedValue({
items: [
{
name: 'fleet server policy',
id: 'fleet_server_policy',
is_default_fleet_server: true,
package_policies: [
{
name: 'fleet-server-123',
package: {
name: 'fleet_server',
},
},
],
},
{
name: 'agent policy 1',
id: 'agent_policy_1',
is_managed: false,
package_policies: [
{
name: 'nginx',
package: {
name: 'nginx',
},
},
],
},
],
} as unknown as ReturnType<typeof mockedAgentPolicyService.list>);
mockedAgentPolicyService.hasFleetServerIntegration.mockReturnValue(true);

await outputService.create(
soClient,
esClientMock,
{
is_default: false,
is_default_monitoring: false,
name: 'Test',
type: 'logstash',
},
{ id: 'output-1' }
);

expect(mockedAgentPolicyService.update).toBeCalledWith(
expect.anything(),
expect.anything(),
'fleet_server_policy',
{ data_output_id: 'output-test' },
{ force: true }
);
});
});

describe('update', () => {
Expand Down Expand Up @@ -496,7 +556,7 @@ describe('Output Service', () => {
});

// With preconfigured outputs
it('Do not allow to update a preconfigured output outisde from preconfiguration', async () => {
it('Do not allow to update a preconfigured output outside from preconfiguration', async () => {
const soClient = getMockedSoClient();
await expect(
outputService.update(soClient, esClientMock, 'existing-preconfigured-default-output', {
Expand Down
3 changes: 0 additions & 3 deletions x-pack/plugins/fleet/server/services/output.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -306,9 +306,6 @@ class OutputService {
}
);
}
} else {
// prevent changing an ES output to logstash if it's used by fleet server policies
validateLogstashOutputNotUsedInFleetServerPolicy(fleetServerPolicies);
}
}

Expand Down
238 changes: 233 additions & 5 deletions x-pack/test/fleet_api_integration/apis/outputs/crud.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ export default function (providerContext: FtrProviderContext) {
const esArchiver = getService('esArchiver');
const kibanaServer = getService('kibanaServer');

describe('fleet_output_crud', async function () {
let pkgVersion: string;

describe('fleet_outputs_crud', async function () {
skipIfNoDockerRegistry(providerContext);
before(async () => {
await kibanaServer.savedObjects.cleanStandardList();
Expand All @@ -27,6 +29,61 @@ export default function (providerContext: FtrProviderContext) {
let defaultOutputId: string;

before(async function () {
// we must first force install the fleet_server package to override package verification error on policy create
// https://github.com/elastic/kibana/issues/137450
const getPkRes = await supertest
.get(`/api/fleet/epm/packages/fleet_server`)
.set('kbn-xsrf', 'xxxx')
.expect(200);
pkgVersion = getPkRes.body.item.version;

await supertest
.post(`/api/fleet/epm/packages/fleet_server/${pkgVersion}`)
.set('kbn-xsrf', 'xxxx')
.send({ force: true })
.expect(200);

let { body: apiResponse } = await supertest
.post(`/api/fleet/agent_policies`)
.set('kbn-xsrf', 'kibana')
.send({
name: 'Fleet Server policy 1',
namespace: 'default',
has_fleet_server: true,
})
.expect(200);
const fleetServerPolicy = apiResponse.item;

({ body: apiResponse } = await supertest
.post(`/api/fleet/agent_policies`)
.set('kbn-xsrf', 'kibana')
.send({
name: 'Agent policy 1',
namespace: 'default',
})
.expect(200));

const agentPolicy = apiResponse.item;

if (!fleetServerPolicy) {
throw new Error('No Fleet server policy');
}

if (!agentPolicy) {
throw new Error('No agent policy');
}

await supertest
.post(`/api/fleet/fleet_server_hosts`)
.set('kbn-xsrf', 'xxxx')
.send({
id: 'test-default-123',
name: 'Default',
is_default: true,
host_urls: ['https://test.fr:8080'],
})
.expect(200);

const { body: getOutputsRes } = await supertest.get(`/api/fleet/outputs`).expect(200);

const defaultOutput = getOutputsRes.items.find((item: any) => item.is_default);
Expand Down Expand Up @@ -82,10 +139,127 @@ export default function (providerContext: FtrProviderContext) {
.send({ hosts: ['https://test.fr'] })
.expect(404);
});
it('should allow to update an output with the shipper values', async function () {

it('should not allow to update a default ES output to logstash', async function () {
const { body } = await supertest
.put(`/api/fleet/outputs/${defaultOutputId}`)
.set('kbn-xsrf', 'xxxx')
.send({
name: 'My Logstash Output',
type: 'logstash',
hosts: ['test.fr:443'],
ssl: {
certificate: 'CERTIFICATE',
key: 'KEY',
certificate_authorities: ['CA1', 'CA2'],
},
})
.expect(400);
expect(body.message).to.eql(
'Logstash output cannot be used with Fleet Server integration in Fleet Server policy 1. Please create a new ElasticSearch output.'
);
});

it('should allow to update a default ES output keeping it ES', async function () {
await supertest
.put(`/api/fleet/outputs/${defaultOutputId}`)
.set('kbn-xsrf', 'xxxx')
.send({
name: 'Updated Default ES Output',
type: 'elasticsearch',
hosts: ['test.fr:443'],
})
.expect(200);
});

it('should allow to update a non-default ES output to logstash', async function () {
const { body: postResponse } = await supertest
.post(`/api/fleet/outputs`)
.set('kbn-xsrf', 'xxxx')
.send({
name: 'A Logstash Output',
type: 'logstash',
hosts: ['test.fr:443'],
ssl: {
certificate: 'CERTIFICATE',
key: 'KEY',
certificate_authorities: ['CA1', 'CA2'],
},
})
.expect(200);

const { id: logstashOutput1Id } = postResponse.item;
await supertest
.put(`/api/fleet/outputs/${logstashOutput1Id}`)
.set('kbn-xsrf', 'xxxx')
.send({
name: 'A Logstash Output',
type: 'logstash',
hosts: ['test.fr:443'],
ssl: {
certificate: 'CERTIFICATE',
key: 'KEY',
certificate_authorities: ['CA1', 'CA2'],
},
})
.expect(200);
});

it('should allow to update a default logstash output to logstash', async function () {
const { body: postResponse } = await supertest
.post(`/api/fleet/outputs`)
.set('kbn-xsrf', 'xxxx')
.send({
name: 'Logstash Output 1',
type: 'logstash',
hosts: ['test.fr:443'],
ssl: {
certificate: 'CERTIFICATE',
key: 'KEY',
certificate_authorities: ['CA1', 'CA2'],
},
})
.expect(200);
const { id: logstashOutput2Id } = postResponse.item;

await supertest
.put(`/api/fleet/outputs/${logstashOutput2Id}`)
.set('kbn-xsrf', 'xxxx')
.send({
name: 'My Logstash Output',
type: 'logstash',
hosts: ['test.fr:443', 'test.com:443'],
ssl: {
certificate: 'CERTIFICATE',
key: 'KEY',
certificate_authorities: ['CA1', 'CA2'],
},
})
.expect(200);

await supertest.get(`/api/fleet/outputs`).expect(200);
});

it('should allow to update a logstash output with the shipper values', async function () {
const { body: postResponse } = await supertest
.post(`/api/fleet/outputs`)
.set('kbn-xsrf', 'xxxx')
.send({
name: 'My Logstash Output',
type: 'logstash',
hosts: ['test.fr:443'],
ssl: {
certificate: 'CERTIFICATE',
key: 'KEY',
certificate_authorities: ['CA1', 'CA2'],
},
})
.expect(200);
const { id: newOutputId } = postResponse.item;

await supertest
.put(`/api/fleet/outputs/${newOutputId}`)
.set('kbn-xsrf', 'xxxx')
.send({
name: 'My Logstash Output',
type: 'logstash',
Expand All @@ -107,7 +281,7 @@ export default function (providerContext: FtrProviderContext) {
const {
body: { items: outputs },
} = await supertest.get(`/api/fleet/outputs`).expect(200);
const newOutput = outputs.filter((o: any) => o.id === defaultOutputId);
const newOutput = outputs.filter((o: any) => o.id === newOutputId);

expect(newOutput[0].shipper).to.eql({
compression_level: null,
Expand Down Expand Up @@ -148,7 +322,7 @@ export default function (providerContext: FtrProviderContext) {
});

describe('POST /outputs', () => {
it('should allow to create an output ', async function () {
it('should allow to create an ES output ', async function () {
const { body: postResponse } = await supertest
.post(`/api/fleet/outputs`)
.set('kbn-xsrf', 'xxxx')
Expand All @@ -165,7 +339,29 @@ export default function (providerContext: FtrProviderContext) {
});
});

it('should allow to create a logstash output ', async function () {
it('should allow to create a new default ES output ', async function () {
const { body: postResponse } = await supertest
.post(`/api/fleet/outputs`)
.set('kbn-xsrf', 'xxxx')
.send({
name: 'Default ES output',
type: 'elasticsearch',
hosts: ['https://test.fr'],
is_default: true,
})
.expect(200);

const { id: _, ...itemWithoutId } = postResponse.item;
expect(itemWithoutId).to.eql({
name: 'Default ES output',
type: 'elasticsearch',
hosts: ['https://test.fr:443'],
is_default: true,
is_default_monitoring: false,
});
});

it('should allow to create a logstash output', async function () {
const { body: postResponse } = await supertest
.post(`/api/fleet/outputs`)
.set('kbn-xsrf', 'xxxx')
Expand Down Expand Up @@ -196,6 +392,38 @@ export default function (providerContext: FtrProviderContext) {
});
});

it('should allow to create a new logstash default output', async function () {
const { body: postResponse } = await supertest
.post(`/api/fleet/outputs`)
.set('kbn-xsrf', 'xxxx')
.send({
name: 'Default Logstash Output',
type: 'logstash',
hosts: ['test.fr:443'],
is_default: true,
ssl: {
certificate: 'CERTIFICATE',
key: 'KEY',
certificate_authorities: ['CA1', 'CA2'],
},
})
.expect(200);

const { id: _, ...itemWithoutId } = postResponse.item;
expect(itemWithoutId).to.eql({
name: 'Default Logstash Output',
type: 'logstash',
hosts: ['test.fr:443'],
is_default: true,
is_default_monitoring: false,
ssl: {
certificate: 'CERTIFICATE',
key: 'KEY',
certificate_authorities: ['CA1', 'CA2'],
},
});
});

it('should not allow to create a logstash output with http hosts ', async function () {
const { body: postResponse } = await supertest
.post(`/api/fleet/outputs`)
Expand Down

0 comments on commit 169be89

Please sign in to comment.