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

[Fleet] Fix fleet_server_hosts value in fleet/settings API #144898

Merged
merged 6 commits into from
Nov 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 4 additions & 12 deletions x-pack/plugins/fleet/server/collectors/fleet_server_collector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@
* 2.0.
*/

import { isBoom } from '@hapi/boom';
import type { SavedObjectsClient, ElasticsearchClient } from '@kbn/core/server';

import { packagePolicyService, settingsService } from '../services';
import { packagePolicyService } from '../services';
import { getAgentStatusForAgentPolicy } from '../services/agents';
import { listFleetServerHosts } from '../services/fleet_server_host';

const DEFAULT_USAGE = {
total_all_statuses: 0,
Expand Down Expand Up @@ -39,16 +39,8 @@ export const getFleetServerUsage = async (
return DEFAULT_USAGE;
}

const numHostsUrls = await settingsService
.getSettings(soClient)
.then((settings) => settings.fleet_server_hosts?.length ?? 0)
.catch((err) => {
if (isBoom(err) && err.output.statusCode === 404) {
return 0;
}

throw err;
});
const fleetServerHosts = await listFleetServerHosts(soClient);
const numHostsUrls = fleetServerHosts.items.flatMap((host) => host.host_urls).length;

// Find all policies with Fleet server than query agent status
let hasMore = true;
Expand Down
5 changes: 4 additions & 1 deletion x-pack/plugins/fleet/server/services/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { GLOBAL_SETTINGS_SAVED_OBJECT_TYPE, GLOBAL_SETTINGS_ID } from '../../com
import type { SettingsSOAttributes, Settings, BaseSettings } from '../../common/types';

import { appContextService } from './app_context';
import { listFleetServerHosts } from './fleet_server_host';

export async function getSettings(soClient: SavedObjectsClientContract): Promise<Settings> {
const res = await soClient.find<SettingsSOAttributes>({
Expand All @@ -23,10 +24,12 @@ export async function getSettings(soClient: SavedObjectsClientContract): Promise
throw Boom.notFound('Global settings not found');
}
const settingsSo = res.saved_objects[0];
const fleetServerHosts = await listFleetServerHosts(soClient);

return {
id: settingsSo.id,
...settingsSo.attributes,
fleet_server_hosts: settingsSo.attributes.fleet_server_hosts || [],
fleet_server_hosts: fleetServerHosts.items.flatMap((item) => item.host_urls),
preconfigured_fields: getConfigFleetServerHosts() ? ['fleet_server_hosts'] : [],
};
}
Expand Down
9 changes: 7 additions & 2 deletions x-pack/test/fleet_api_integration/apis/fleet_telemetry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,14 @@ export default function (providerContext: FtrProviderContext) {
}

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

// Default Fleet Server
Expand Down
56 changes: 56 additions & 0 deletions x-pack/test/fleet_api_integration/apis/settings/get.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import expect from '@kbn/expect';
import { FtrProviderContext } from '../../../api_integration/ftr_provider_context';
import { skipIfNoDockerRegistry } from '../../helpers';
import { setupFleetAndAgents } from '../agents/services';

export default function (providerContext: FtrProviderContext) {
const { getService } = providerContext;
const supertest = getService('supertest');
const esArchiver = getService('esArchiver');
const kibanaServer = getService('kibanaServer');

describe('Settings - get', async function () {
skipIfNoDockerRegistry(providerContext);
before(async () => {
await esArchiver.load('x-pack/test/functional/es_archives/fleet/empty_fleet_server');
});
setupFleetAndAgents(providerContext);

after(async () => {
await kibanaServer.savedObjects.cleanStandardList();
await esArchiver.unload('x-pack/test/functional/es_archives/fleet/empty_fleet_server');
});

it('should respond with fleet_server_hosts', async function () {
// Create a fleet server host
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.com:8080', 'https://test.com:8081'],
})
.expect(200);

// Assert that the hosts appear in the setting response
const response = await supertest
.get(`/api/fleet/settings`)
.set('kbn-xsrf', 'xxxx')
.expect(200);

expect(response.body.item.fleet_server_hosts).to.eql([
'https://test.com:8080',
'https://test.com:8081',
]);
});
});
}
1 change: 1 addition & 0 deletions x-pack/test/fleet_api_integration/apis/settings/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

export default function loadTests({ loadTestFile }) {
describe('Settings Endpoints', () => {
loadTestFile(require.resolve('./get'));
loadTestFile(require.resolve('./update'));
});
}
3 changes: 2 additions & 1 deletion x-pack/test/fleet_api_integration/apis/settings/update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ export default function (providerContext: FtrProviderContext) {
const esClient = getService('es');
const esArchiver = getService('esArchiver');

describe('Settings - update', async function () {
// Skipped as the Fleet Server hosts settings values are no longer used as of https://github.com/elastic/kibana/issues/137785
Copy link
Member Author

Choose a reason for hiding this comment

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

I think this is the right move to skip these tests - these are leftover from when we were using the /settings API to create new Fleet Server hosts. AFAIK this is no longer supported, as any Fleet Server Hosts within the Fleet settings saved object will be ignored. @nchaulet please correct me if I'm wrong here.

We'll probably want to remove this API altogether as part of the GA work.

Copy link
Member

Choose a reason for hiding this comment

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

Yes we probably want to remove these tests, and this API (at least the fleet_server_hosts) part should be removed as part of the GA work (we should already mark them as deprecated in the open API spec and point to new fleet server hosts endpoints)

describe.skip('Settings - update', async function () {
skipIfNoDockerRegistry(providerContext);
before(async () => {
await esArchiver.load('x-pack/test/functional/es_archives/fleet/empty_fleet_server');
Expand Down