Skip to content

Commit

Permalink
[Defend Workflows][Reusable integrations] View agent policy works w…
Browse files Browse the repository at this point in the history
…ith reusable integrations (elastic#195110)

## Summary

This PR fixes a (future) bug around Endpoint list's _View agent policy_
action. Instead of navigating the user to `package_policy.policy_ids[0]`
which is the assigned integrations first agent policy instead of the one
agent policy the host is enrolled to, it will navigate to
`host.policy_info?.agent.applied.id`.

Modifications:
- navigate user to `host.policy_info?.agent.applied.id`:
633325f
- remove the now unused `serverReturnedEndpointAgentPolicies` dispatch
action and corresponding store item:
15e8023
- refactor around `serverReturnedEndpointNonExistingPolicies` which is
now solely dispatched in the modified function
- a small fix to the previous similar PR
(elastic#193518) to not cause bad
requests in a corner case: 56cf7ac

#### Testing
For testing, assign the same Defend integration to multiple Agent
policies - order of assignment matters, see screenshot:
<img width="1469" alt="image"
src="https://github.com/user-attachments/assets/bb266b25-7523-423b-a5e0-a0840a0af6d4">

On `main`, Endpoint list's _View agent policy_ action will navigate to
`Agent policy 2` in the above example, while _View agent details_ will
show that `cheese` is assigned to `Agent policy 1`. With this fix, _View
agent policy_ should navigate to the correct agent policy.
<img width="1193" alt="image"
src="https://github.com/user-attachments/assets/a78623d9-0b41-4b90-8dec-f91e2ee09493">


### Checklist

Delete any items that are not applicable to this PR.

- [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
  • Loading branch information
gergoabraham authored Oct 8, 2024
1 parent faa74d2 commit 251e289
Show file tree
Hide file tree
Showing 16 changed files with 101 additions and 135 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,6 @@ export interface ServerReturnedEndpointNonExistingPolicies {
payload: EndpointState['nonExistingPolicies'];
}

export interface ServerReturnedEndpointAgentPolicies {
type: 'serverReturnedEndpointAgentPolicies';
payload: EndpointState['agentPolicies'];
}

export interface ServerFinishedInitialization {
type: 'serverFinishedInitialization';
payload: boolean;
Expand Down Expand Up @@ -162,7 +157,6 @@ export type EndpointAction =
| AppRequestedEndpointList
| ServerReturnedEndpointNonExistingPolicies
| ServerReturnedAgenstWithEndpointsTotal
| ServerReturnedEndpointAgentPolicies
| UserUpdatedEndpointListRefreshOptions
| ServerReturnedEndpointsTotal
| ServerFailedToReturnAgenstWithEndpointsTotal
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,7 @@ export const initialEndpointPageState = (): Immutable<EndpointState> => {
selectedPolicyId: undefined,
policyItemsLoading: false,
endpointPackageInfo: createUninitialisedResourceState(),
nonExistingPolicies: {},
agentPolicies: {},
nonExistingPolicies: new Set(),
endpointsExist: true,
patterns: [],
patternsError: undefined,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ describe('EndpointList store concerns', () => {
});

test('it creates default state', () => {
expect(store.getState()).toEqual({
const expectedDefaultState: EndpointState = {
hosts: [],
isInitialized: false,
pageSize: 10,
Expand All @@ -57,8 +57,7 @@ describe('EndpointList store concerns', () => {
endpointPackageInfo: {
type: 'UninitialisedResourceState',
},
nonExistingPolicies: {},
agentPolicies: {},
nonExistingPolicies: new Set(),
endpointsExist: true,
patterns: [],
patternsError: undefined,
Expand All @@ -68,12 +67,13 @@ describe('EndpointList store concerns', () => {
endpointsTotal: 0,
agentsWithEndpointsTotalError: undefined,
endpointsTotalError: undefined,
queryStrategyVersion: undefined,
isolationRequestState: {
type: 'UninitialisedResourceState',
},
metadataTransformStats: createUninitialisedResourceState(),
});
};

expect(store.getState()).toEqual(expectedDefaultState);
});

test('it handles `serverReturnedEndpointList', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,12 @@ import {
sendGetEndpointSecurityPackage,
} from '../../../services/policies/ingest';
import type { GetPolicyListResponse } from '../../policy/types';
import type { EndpointState, PolicyIds, TransformStats, TransformStatsResponse } from '../types';
import type {
EndpointState,
NonExistingPolicies,
TransformStats,
TransformStatsResponse,
} from '../types';
import type { EndpointPackageInfoStateChanged } from './action';
import {
endpointPackageInfo,
Expand Down Expand Up @@ -130,26 +135,24 @@ export const endpointMiddlewareFactory: ImmutableMiddlewareFactory<EndpointState
};
};

const getAgentAndPoliciesForEndpointsList = async (
const getNonExistingPoliciesForEndpointList = async (
http: HttpStart,
hosts: HostResultList['hosts'],
currentNonExistingPolicies: EndpointState['nonExistingPolicies']
): Promise<PolicyIds | undefined> => {
currentNonExistingPolicies: Immutable<NonExistingPolicies>
): Promise<NonExistingPolicies | undefined> => {
if (hosts.length === 0) {
return;
}

// Create an array of unique policy IDs that are not yet known to be non-existing.
const policyIdsToCheck = [
...new Set(
hosts.reduce((acc: string[], host) => {
const appliedPolicyId = host.metadata.Endpoint.policy.applied.id;
if (!currentNonExistingPolicies[appliedPolicyId]) {
acc.push(appliedPolicyId);
}
return acc;
}, [])
),
...hosts.reduce<Set<string>>((acc, host) => {
const appliedPolicyId = host.metadata.Endpoint.policy.applied.id;
if (!currentNonExistingPolicies.has(appliedPolicyId)) {
acc.add(appliedPolicyId);
}
return acc;
}, new Set()),
];

if (policyIdsToCheck.length === 0) {
Expand All @@ -158,37 +161,24 @@ const getAgentAndPoliciesForEndpointsList = async (

const policiesFound = (
await sendBulkGetPackagePolicies(http, policyIdsToCheck)
).items.reduce<PolicyIds>(
(list, packagePolicy) => {
list.packagePolicy[packagePolicy.id as string] = true;
list.agentPolicy[packagePolicy.id as string] = packagePolicy.policy_ids[0]; // TODO

return list;
},
{ packagePolicy: {}, agentPolicy: {} }
);
).items.reduce<NonExistingPolicies>((set, packagePolicy) => set.add(packagePolicy.id), new Set());

// packagePolicy contains non-existing packagePolicy ids whereas agentPolicy contains existing agentPolicy ids
const nonExistingPackagePoliciesAndExistingAgentPolicies = policyIdsToCheck.reduce<PolicyIds>(
(list, policyId: string) => {
if (policiesFound.packagePolicy[policyId as string]) {
list.agentPolicy[policyId as string] = policiesFound.agentPolicy[policyId];
return list;
const nonExistingPackagePolicies = policyIdsToCheck.reduce<NonExistingPolicies>(
(set, policyId) => {
if (!policiesFound.has(policyId)) {
set.add(policyId);
}
list.packagePolicy[policyId as string] = true;
return list;

return set;
},
{ packagePolicy: {}, agentPolicy: {} }
new Set()
);

if (
Object.keys(nonExistingPackagePoliciesAndExistingAgentPolicies.packagePolicy).length === 0 &&
Object.keys(nonExistingPackagePoliciesAndExistingAgentPolicies.agentPolicy).length === 0
) {
if (!nonExistingPackagePolicies.size) {
return;
}

return nonExistingPackagePoliciesAndExistingAgentPolicies;
return nonExistingPackagePolicies;
};

const endpointsTotal = async (http: HttpStart): Promise<number> => {
Expand Down Expand Up @@ -330,7 +320,7 @@ async function endpointListMiddleware({
payload: endpointResponse,
});

dispatchIngestPolicies({ http: coreStart.http, hosts: endpointResponse.data, store });
fetchNonExistingPolicies({ http: coreStart.http, hosts: endpointResponse.data, store });
} catch (error) {
dispatch({
type: 'serverFailedToReturnEndpointList',
Expand Down Expand Up @@ -447,7 +437,7 @@ export async function handleLoadMetadataTransformStats(http: HttpStart, store: E
}
}

async function dispatchIngestPolicies({
async function fetchNonExistingPolicies({
store,
hosts,
http,
Expand All @@ -458,21 +448,15 @@ async function dispatchIngestPolicies({
}) {
const { getState, dispatch } = store;
try {
const ingestPolicies = await getAgentAndPoliciesForEndpointsList(
const missingPolicies = await getNonExistingPoliciesForEndpointList(
http,
hosts,
nonExistingPolicies(getState())
);
if (ingestPolicies?.packagePolicy !== undefined) {
if (missingPolicies !== undefined) {
dispatch({
type: 'serverReturnedEndpointNonExistingPolicies',
payload: ingestPolicies.packagePolicy,
});
}
if (ingestPolicies?.agentPolicy !== undefined) {
dispatch({
type: 'serverReturnedEndpointAgentPolicies',
payload: ingestPolicies.agentPolicy,
payload: missingPolicies,
});
}
} catch (error) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,18 +67,7 @@ export const endpointListReducer: StateReducer = (state = initialEndpointPageSta
} else if (action.type === 'serverReturnedEndpointNonExistingPolicies') {
return {
...state,
nonExistingPolicies: {
...state.nonExistingPolicies,
...action.payload,
},
};
} else if (action.type === 'serverReturnedEndpointAgentPolicies') {
return {
...state,
agentPolicies: {
...state.agentPolicies,
...action.payload,
},
nonExistingPolicies: new Set([...state.nonExistingPolicies, ...action.payload]),
};
} else if (action.type === 'serverReturnedMetadataPatterns') {
// handle an error case
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,13 +167,6 @@ export const nonExistingPolicies: (
state: Immutable<EndpointState>
) => Immutable<EndpointState['nonExistingPolicies']> = (state) => state.nonExistingPolicies;

/**
* returns the list of known existing agent policies
*/
export const agentPolicies: (
state: Immutable<EndpointState>
) => Immutable<EndpointState['agentPolicies']> = (state) => state.agentPolicies;

/**
* Return boolean that indicates whether endpoints exist
* @param state
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,7 @@ export interface EndpointState {
/** Endpoint package info */
endpointPackageInfo: AsyncResourceState<GetInfoResponse['item']>;
/** Tracks the list of policy IDs used in Host metadata that may no longer exist */
nonExistingPolicies: PolicyIds['packagePolicy'];
/** List of Package Policy Ids mapped to an associated Fleet Parent Agent Policy Id*/
agentPolicies: PolicyIds['agentPolicy'];
nonExistingPolicies: NonExistingPolicies;
/** Tracks whether hosts exist and helps control if onboarding should be visible */
endpointsExist: boolean;
/** index patterns for query bar */
Expand Down Expand Up @@ -79,13 +77,9 @@ export interface EndpointState {
export type AgentIdsPendingActions = Map<string, EndpointPendingActions['pending_actions']>;

/**
* packagePolicy contains a list of Package Policy IDs (received via Endpoint metadata policy response) mapped to a boolean whether they exist or not.
* agentPolicy contains a list of existing Package Policy Ids mapped to an associated Fleet parent Agent Config.
* Set containing Package Policy IDs which are used but do not exist anymore
*/
export interface PolicyIds {
packagePolicy: Record<string, boolean>;
agentPolicy: Record<string, string>;
}
export type NonExistingPolicies = Set<string>;

/**
* Query params on the host page parsed from the URL
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,16 @@ import type { EuiContextMenuPanelProps, EuiPopoverProps } from '@elastic/eui';
import { EuiButtonIcon, EuiContextMenuPanel, EuiPopover } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import { ContextMenuItemNavByRouter } from '../../../../components/context_menu_with_router_support/context_menu_item_nav_by_router';
import type { HostMetadata } from '../../../../../../common/endpoint/types';
import type { HostInfo } from '../../../../../../common/endpoint/types';
import { useEndpointActionItems } from '../hooks';

export interface TableRowActionProps {
endpointMetadata: HostMetadata;
endpointInfo: HostInfo;
}

export const TableRowActions = memo<TableRowActionProps>(({ endpointMetadata }) => {
export const TableRowActions = memo<TableRowActionProps>(({ endpointInfo }) => {
const [isOpen, setIsOpen] = useState(false);
const endpointActions = useEndpointActionItems(endpointMetadata, { isEndpointList: true });
const endpointActions = useEndpointActionItems(endpointInfo, { isEndpointList: true });

const handleCloseMenu = useCallback(() => setIsOpen(false), [setIsOpen]);
const handleToggleMenu = useCallback(() => setIsOpen(!isOpen), [isOpen]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ describe('When using the Endpoint Details Actions Menu', () => {
endpointHost.metadata.host.os.name = 'Windows';
// @ts-expect-error TS2540
endpointHost.metadata.agent.version = '7.14.0';
// @ts-expect-error TS2540
endpointHost.policy_info = { agent: { applied: { id: '123' } } };
httpMocks.responseProvider.metadataDetails.mockReturnValue(endpointHost);
};

Expand Down Expand Up @@ -87,7 +89,7 @@ describe('When using the Endpoint Details Actions Menu', () => {
});

render = async () => {
renderResult = mockedContext.render(<ActionsMenu hostMetadata={endpointHost?.metadata} />);
renderResult = mockedContext.render(<ActionsMenu hostInfo={endpointHost} />);
const endpointDetailsActionsButton = renderResult.getByTestId('endpointDetailsActionsButton');
endpointDetailsActionsButton.style.pointerEvents = 'all';
await user.click(endpointDetailsActionsButton);
Expand Down Expand Up @@ -129,10 +131,6 @@ describe('When using the Endpoint Details Actions Menu', () => {
'should navigate via kibana `navigateToApp()` when %s is clicked',
async (_, dataTestSubj) => {
await render();
// TODO middlewareSpy.waitForAction() times out after the upgrade to userEvent v14 https://github.com/elastic/kibana/pull/189949
// await act(async () => {
// await middlewareSpy.waitForAction('serverReturnedEndpointAgentPolicies');
// });

const takeActionMenuItem = renderResult.getByTestId(dataTestSubj);
takeActionMenuItem.style.pointerEvents = 'all';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@
import React, { memo, useCallback, useMemo, useState } from 'react';
import { EuiButton, EuiContextMenuPanel, EuiPopover } from '@elastic/eui';
import { FormattedMessage } from '@kbn/i18n-react';
import type { HostMetadata } from '../../../../../../../common/endpoint/types';
import type { HostInfo } from '../../../../../../../common/endpoint/types';
import { useEndpointActionItems } from '../../hooks';
import { ContextMenuItemNavByRouter } from '../../../../../components/context_menu_with_router_support/context_menu_item_nav_by_router';

export const ActionsMenu = memo<{ hostMetadata: HostMetadata }>(({ hostMetadata }) => {
const menuOptions = useEndpointActionItems(hostMetadata);
export const ActionsMenu = memo<{ hostInfo: HostInfo }>(({ hostInfo }) => {
const menuOptions = useEndpointActionItems(hostInfo);
const [isPopoverOpen, setIsPopoverOpen] = useState(false);

const closePopoverHandler = useCallback(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ export const EndpointDetails = memo(() => {

{showFlyoutFooter && (
<EuiFlyoutFooter className="eui-textRight" data-test-subj="endpointDetailsFlyoutFooter">
<ActionsMenu hostMetadata={hostInfo.metadata} />
<ActionsMenu hostInfo={hostInfo} />
</EuiFlyoutFooter>
)}
</>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ export const EndpointDetailsContent = memo<EndpointDetailsContentProps>(
policyId={hostInfo.metadata.Endpoint.policy.applied.id}
revision={hostInfo.metadata.Endpoint.policy.applied.endpoint_policy_version}
isOutdated={isPolicyOutOfDate(hostInfo.metadata.Endpoint.policy.applied, policyInfo)}
policyExists={!missingPolicies[hostInfo.metadata.Endpoint.policy.applied.id]}
policyExists={!missingPolicies.has(hostInfo.metadata.Endpoint.policy.applied.id)}
data-test-subj="policyDetailsValue"
>
{hostInfo.metadata.Endpoint.policy.applied.name}
Expand Down
Loading

0 comments on commit 251e289

Please sign in to comment.