Skip to content

Commit

Permalink
[Fleet] separated showInactive from unenrolled status filter (#187960)
Browse files Browse the repository at this point in the history
## Summary

Closes #186065

The `showInactive:true` flag in query agents logic filtered out inactive
and unenrolled agents.
This is not working correctly in a few places where we only want to
include inactive, not unenrolled: agent count per policy (the linked
reported issue), taking actions.

Instead, changed the meaning of `showInactive:true` to only filter out
inactive agents and filter out `unenrolled` unless the `kuery`
explicitly contains it.
In most functionality we ignore unenrolled agents, only need to query
them when looking at Agent list without any status filters or the UI
filter includes `unenrolled` status.

<img width="1402" alt="image"
src="https://github.com/elastic/kibana/assets/90178898/38bebbc6-db04-45d2-a3a5-fb1c4b94af7e">
<img width="1157" alt="image"
src="https://github.com/elastic/kibana/assets/90178898/ca3c388b-53f0-459d-826e-3f4f9353ccc1">
<img width="1242" alt="image"
src="https://github.com/elastic/kibana/assets/90178898/37ecddc3-c038-4423-9a52-84a14a1ced52">

Agent count includes inactive, but filters out unenrolled.
<img width="1249" alt="image"
src="https://github.com/elastic/kibana/assets/90178898/21e8cf89-4219-48a0-a374-def3dd28d86f">
When clicking on the agent count, it navigates to Agent list, including
inactive.
<img width="1621" alt="image"
src="https://github.com/user-attachments/assets/59c44560-801d-4232-aaff-bb1ee245a153">

<img width="1742" alt="image"
src="https://github.com/user-attachments/assets/57f91bf5-617b-4365-af79-8b0ad3719bc7">
<img width="1789" alt="image"
src="https://github.com/user-attachments/assets/fc857dec-1e39-4e12-b242-441f818b1ae0">


### Checklist

- [ ] [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
  • Loading branch information
juliaElastic committed Jul 18, 2024
1 parent 3ac173e commit 9d4c061
Show file tree
Hide file tree
Showing 7 changed files with 103 additions and 47 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export function useFetchAgentsData() {
const history = useHistory();
const { urlParams, toUrlParams } = useUrlParams();
const defaultKuery: string = (urlParams.kuery as string) || '';
const urlHasInactive = (urlParams.showInactive as string) === 'true';

// Agent data states
const [showUpgradeable, setShowUpgradeable] = useState<boolean>(false);
Expand All @@ -64,12 +65,17 @@ export function useFetchAgentsData() {
'unhealthy',
'updating',
'offline',
...(urlHasInactive ? ['inactive'] : []),
]);

const [selectedTags, setSelectedTags] = useState<string[]>([]);

const showInactive = useMemo(() => {
return selectedStatus.some((status) => status === 'inactive' || status === 'unenrolled');
return selectedStatus.some((status) => status === 'inactive') || selectedStatus.length === 0;
}, [selectedStatus]);

const includeUnenrolled = useMemo(() => {
return selectedStatus.some((status) => status === 'unenrolled') || selectedStatus.length === 0;
}, [selectedStatus]);

const setSearch = useCallback(
Expand All @@ -89,7 +95,7 @@ export function useFetchAgentsData() {
);

// filters kuery
const kuery = useMemo(() => {
let kuery = useMemo(() => {
return getKuery({
search,
selectedAgentPolicies,
Expand All @@ -98,6 +104,9 @@ export function useFetchAgentsData() {
});
}, [search, selectedAgentPolicies, selectedStatus, selectedTags]);

kuery =
includeUnenrolled && kuery ? `status:* AND (${kuery})` : includeUnenrolled ? `status:*` : kuery;

const [agentsOnCurrentPage, setAgentsOnCurrentPage] = useState<Agent[]>([]);
const [agentsStatus, setAgentsStatus] = useState<
{ [key in SimplifiedAgentStatus]: number } | undefined
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export const LinkedAgentCount = memo<
}`;

return count > 0 ? (
<EuiLink {...otherEuiLinkProps} href={getHref('agent_list', { kuery })}>
<EuiLink {...otherEuiLinkProps} href={getHref('agent_list', { kuery, showInactive: true })}>
{displayValue}
</EuiLink>
) : (
Expand Down
11 changes: 10 additions & 1 deletion x-pack/plugins/fleet/public/constants/page_paths.ts
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,16 @@ export const pagePathGetters: {
FLEET_BASE_PATH,
`/policies/${policyId}/upgrade-package-policy/${packagePolicyId}`,
],
agent_list: ({ kuery }) => [FLEET_BASE_PATH, `/agents${kuery ? `?kuery=${kuery}` : ''}`],
agent_list: ({ kuery, showInactive }) => [
FLEET_BASE_PATH,
`/agents${
kuery && showInactive
? `?kuery=${kuery}&showInactive=true`
: showInactive
? '?showInactive=true'
: ''
}`,
],
agent_details: ({ agentId, tabId, logQuery }) => [
FLEET_BASE_PATH,
`/agents/${agentId}${tabId ? `/${tabId}` : ''}${logQuery ? `?_q=${logQuery}` : ''}`,
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/fleet/server/services/agent_policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -999,7 +999,7 @@ class AgentPolicyService {
showInactive: true,
perPage: 0,
page: 1,
kuery: `${AGENTS_PREFIX}.policy_id:${id} and not status: unenrolled`,
kuery: `${AGENTS_PREFIX}.policy_id:${id}`,
});

if (total > 0) {
Expand Down
82 changes: 68 additions & 14 deletions x-pack/plugins/fleet/server/services/agents/crud.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import { errors } from '@elastic/elasticsearch';
import type { ElasticsearchClient } from '@kbn/core/server';
import { elasticsearchServiceMock, savedObjectsClientMock } from '@kbn/core/server/mocks';
import { toElasticsearchQuery } from '@kbn/es-query';

import { AGENTS_INDEX } from '../../constants';
import { createAppContextStartContractMock } from '../../mocks';
Expand All @@ -22,6 +23,7 @@ import {
getAgentTags,
openPointInTime,
updateAgent,
_joinFilters,
} from './crud';

jest.mock('../audit_logging');
Expand Down Expand Up @@ -135,20 +137,6 @@ describe('Agents CRUD test', () => {
expect(searchMock).toHaveBeenCalledWith(
expect.objectContaining({
aggs: { tags: { terms: { field: 'tags', size: 10000 } } },
body: {
query: {
bool: {
minimum_should_match: 1,
should: [
{
match: {
policy_id: '123',
},
},
],
},
},
},
index: '.fleet-agents',
size: 0,
fields: ['status'],
Expand All @@ -157,6 +145,12 @@ describe('Agents CRUD test', () => {
},
})
);

expect(searchMock.mock.calls.at(-1)[0].body.query).toEqual(
toElasticsearchQuery(
_joinFilters(['fleet-agents.policy_id: 123', 'NOT status:unenrolled'])!
)
);
});
});

Expand Down Expand Up @@ -415,6 +409,66 @@ describe('Agents CRUD test', () => {
});
expect(searchMock.mock.calls.at(-1)[0].sort).toEqual([{ policy_id: { order: 'desc' } }]);
});

describe('status filters', () => {
beforeEach(() => {
searchMock.mockImplementationOnce(() => Promise.resolve(getEsResponse([], 0, 'online')));
});
it('should add inactive and unenrolled filter', async () => {
await getAgentsByKuery(esClientMock, soClientMock, {
showInactive: false,
kuery: '',
});

expect(searchMock.mock.calls.at(-1)[0].query).toEqual(
toElasticsearchQuery(_joinFilters(['NOT (status:inactive)', 'NOT status:unenrolled'])!)
);
});

it('should add unenrolled filter', async () => {
await getAgentsByKuery(esClientMock, soClientMock, {
showInactive: true,
kuery: '',
});

expect(searchMock.mock.calls.at(-1)[0].query).toEqual(
toElasticsearchQuery(_joinFilters(['NOT status:unenrolled'])!)
);
});

it('should not add unenrolled filter', async () => {
await getAgentsByKuery(esClientMock, soClientMock, {
showInactive: true,
kuery: 'status:unenrolled',
});

expect(searchMock.mock.calls.at(-1)[0].query).toEqual(
toElasticsearchQuery(_joinFilters(['status:unenrolled'])!)
);
});

it('should add inactive filter', async () => {
await getAgentsByKuery(esClientMock, soClientMock, {
showInactive: false,
kuery: 'status:*',
});

expect(searchMock.mock.calls.at(-1)[0].query).toEqual(
toElasticsearchQuery(_joinFilters(['status:*', 'NOT status:inactive'])!)
);
});

it('should not add inactive filter', async () => {
await getAgentsByKuery(esClientMock, soClientMock, {
showInactive: true,
kuery: 'status:*',
});

expect(searchMock.mock.calls.at(-1)[0].query).toEqual(
toElasticsearchQuery(_joinFilters(['status:*'])!)
);
});
});
});

describe('update', () => {
Expand Down
39 changes: 11 additions & 28 deletions x-pack/plugins/fleet/server/services/agents/crud.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,12 @@ import { searchHitToAgent, agentSOAttributesToFleetServerAgentDoc } from './help
import { buildAgentStatusRuntimeField } from './build_status_runtime_field';
import { getLatestAvailableAgentVersion } from './versions';

const INACTIVE_AGENT_CONDITION = `status:inactive OR status:unenrolled`;
const INACTIVE_AGENT_CONDITION = `status:inactive`;
const ACTIVE_AGENT_CONDITION = `NOT (${INACTIVE_AGENT_CONDITION})`;
const ENROLLED_AGENT_CONDITION = `NOT status:unenrolled`;

const includeUnenrolled = (kuery?: string) =>
kuery?.toLowerCase().includes('status:*') || kuery?.toLowerCase().includes('status:unenrolled');

export function _joinFilters(
filters: Array<string | undefined | KueryNode>
Expand Down Expand Up @@ -157,6 +161,9 @@ export async function getAgentTags(
if (showInactive === false) {
filters.push(ACTIVE_AGENT_CONDITION);
}
if (!includeUnenrolled(kuery)) {
filters.push(ENROLLED_AGENT_CONDITION);
}

const kueryNode = _joinFilters(filters);
const body = kueryNode ? { query: toElasticsearchQuery(kueryNode) } : {};
Expand Down Expand Up @@ -184,33 +191,6 @@ export async function getAgentTags(
}
}

export function getElasticsearchQuery(
kuery: string,
showInactive = false,
includeHosted = false,
hostedPolicies: string[] = [],
extraFilters: string[] = []
): estypes.QueryDslQueryContainer | undefined {
const filters = [];

if (kuery && kuery !== '') {
filters.push(kuery);
}

if (showInactive === false) {
filters.push(ACTIVE_AGENT_CONDITION);
}

if (!includeHosted && hostedPolicies.length > 0) {
filters.push('NOT (policy_id:{policyIds})'.replace('{policyIds}', hostedPolicies.join(',')));
}

filters.push(...extraFilters);

const kueryNode = _joinFilters(filters);
return kueryNode ? toElasticsearchQuery(kueryNode) : undefined;
}

export async function getAgentsByKuery(
esClient: ElasticsearchClient,
soClient: SavedObjectsClientContract,
Expand Down Expand Up @@ -264,6 +244,9 @@ export async function getAgentsByKuery(
if (showInactive === false) {
filters.push(ACTIVE_AGENT_CONDITION);
}
if (!includeUnenrolled(kuery)) {
filters.push(ENROLLED_AGENT_CONDITION);
}

const kueryNode = _joinFilters(filters);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,7 @@ export class TelemetryReceiver implements ITelemetryReceiver {
?.listAgents({
perPage: this.maxRecords,
showInactive: true,
kuery: 'status:*', // include unenrolled agents
sortField: 'enrolled_at',
sortOrder: 'desc',
})
Expand Down

0 comments on commit 9d4c061

Please sign in to comment.