Skip to content

Commit

Permalink
[Fleet] Show in UI warning which data streams require root (elastic#1…
Browse files Browse the repository at this point in the history
…84119)

## Summary

Closes elastic/ingest-dev#3357

Updated UI warning to show which data streams require root, if not all.

To verify:
- enroll an unprivileged agent to an agent policy (e.g. using docker)
- add system integration to the agent policy
- when clicking `Save and continue`, verify that the modal shows a
callout saying which data streams require root

<img width="853" alt="image"
src="https://github.com/elastic/kibana/assets/90178898/db8bf26e-de46-4eeb-a471-002021fff3bc">

Updated the Add agent flyout condition as well to show those packages
that has data streams that require root:

To verify:
- create an agent policy with system integration
- open `Add agent` flyout
- verify that the warning callout mentions `System` integration
requiring root

<img width="819" alt="image"
src="https://github.com/elastic/kibana/assets/90178898/abb7e620-f9e4-4662-b0e8-9a16448e52bb">

Open question:
- Do we want to show data stream names that require root on Add agent
flyout too? It is possible but increases the complexity to store data
stream names in package policy SO and populate in API. At this point I
think it's enough to show the list of integrations there.

### Checklist

Delete any items that are not applicable to this PR.

- [x] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [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
juliaElastic authored May 24, 2024
1 parent 23446f2 commit b86039e
Show file tree
Hide file tree
Showing 8 changed files with 173 additions and 23 deletions.
42 changes: 41 additions & 1 deletion x-pack/plugins/fleet/common/services/package_helpers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@
* 2.0.
*/

import { getRootIntegrations, isRootPrivilegesRequired } from './package_helpers';
import {
getRootIntegrations,
getRootPrivilegedDataStreams,
isRootPrivilegesRequired,
} from './package_helpers';

describe('isRootPrivilegesRequired', () => {
it('should return true if root privileges is required at root level', () => {
Expand Down Expand Up @@ -39,6 +43,42 @@ describe('isRootPrivilegesRequired', () => {
});
});

describe('getRootPrivilegedDataStreams', () => {
it('should return empty datastreams if root privileges is required at root level', () => {
const res = getRootPrivilegedDataStreams({
agent: {
privileges: {
root: true,
},
},
} as any);
expect(res).toEqual([]);
});
it('should return datastreams if root privileges is required at datastreams', () => {
const res = getRootPrivilegedDataStreams({
data_streams: [
{
name: 'syslog',
title: 'System syslog logs',
agent: {
privileges: { root: true },
},
},
{
name: 'sysauth',
title: 'System auth logs',
},
],
} as any);
expect(res).toEqual([
{
name: 'syslog',
title: 'System syslog logs',
},
]);
});
});

describe('getRootIntegrations', () => {
it('should return packages that require root', () => {
const res = getRootIntegrations([
Expand Down
13 changes: 13 additions & 0 deletions x-pack/plugins/fleet/common/services/package_helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,19 @@ export function isRootPrivilegesRequired(packageInfo: PackageInfo) {
);
}

export function getRootPrivilegedDataStreams(
packageInfo: PackageInfo
): Array<{ name: string; title: string }> {
if (packageInfo.agent?.privileges?.root) {
return [];
}
return (
packageInfo.data_streams
?.filter((d) => d.agent?.privileges?.root)
.map((ds) => ({ name: ds.name, title: ds.title })) ?? []
);
}

export function getRootIntegrations(
packagePolicies: PackagePolicy[]
): Array<{ name: string; title: string }> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,15 @@ export const ConfirmDeployAgentPolicyModal: React.FunctionComponent<{
agentPolicy: AgentPolicy;
showUnprivilegedAgentsCallout?: boolean;
unprivilegedAgentsCount?: number;
dataStreams?: Array<{ name: string; title: string }>;
}> = ({
onConfirm,
onCancel,
agentCount,
agentPolicy,
showUnprivilegedAgentsCallout = false,
unprivilegedAgentsCount = 0,
dataStreams,
}) => {
return (
<EuiConfirmModal
Expand Down Expand Up @@ -80,6 +82,7 @@ export const ConfirmDeployAgentPolicyModal: React.FunctionComponent<{
<UnprivilegedAgentsCallout
agentPolicyName={agentPolicy.name}
unprivilegedAgentsCount={unprivilegedAgentsCount}
dataStreams={dataStreams ?? []}
/>
</>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,15 @@ export interface UnprivilegedConfirmModalProps {
onCancel: () => void;
agentPolicyName: string;
unprivilegedAgentsCount: number;
dataStreams: Array<{ name: string; title: string }>;
}

export const UnprivilegedConfirmModal: React.FC<UnprivilegedConfirmModalProps> = ({
onConfirm,
onCancel,
agentPolicyName,
unprivilegedAgentsCount,
dataStreams,
}: UnprivilegedConfirmModalProps) => {
return (
<EuiConfirmModal
Expand Down Expand Up @@ -50,6 +52,7 @@ export const UnprivilegedConfirmModal: React.FC<UnprivilegedConfirmModalProps> =
<UnprivilegedAgentsCallout
unprivilegedAgentsCount={unprivilegedAgentsCount}
agentPolicyName={agentPolicyName}
dataStreams={dataStreams}
/>
</EuiConfirmModal>
);
Expand All @@ -58,7 +61,8 @@ export const UnprivilegedConfirmModal: React.FC<UnprivilegedConfirmModalProps> =
export const UnprivilegedAgentsCallout: React.FC<{
agentPolicyName: string;
unprivilegedAgentsCount: number;
}> = ({ agentPolicyName, unprivilegedAgentsCount }) => {
dataStreams: Array<{ name: string; title: string }>;
}> = ({ agentPolicyName, unprivilegedAgentsCount, dataStreams }) => {
return (
<EuiCallOut
color="warning"
Expand All @@ -68,14 +72,32 @@ export const UnprivilegedAgentsCallout: React.FC<{
})}
data-test-subj="unprivilegedAgentsCallout"
>
<FormattedMessage
id="xpack.fleet.addIntegration.confirmModal.unprivilegedAgentsMessage"
defaultMessage="This integration requires Elastic Agents to have root privileges. There {unprivilegedAgentsCount, plural, one {is # agent} other {are # agents}} running in an unprivileged mode using {agentPolicyName}. To ensure that all data required by the integration can be collected, re-enroll the {unprivilegedAgentsCount, plural, one {agent} other {agents}} using an account with root privileges."
values={{
unprivilegedAgentsCount,
agentPolicyName,
}}
/>
{dataStreams.length === 0 ? (
<FormattedMessage
id="xpack.fleet.addIntegration.confirmModal.unprivilegedAgentsMessage"
defaultMessage="This integration requires Elastic Agents to have root privileges. There {unprivilegedAgentsCount, plural, one {is # agent} other {are # agents}} running in an unprivileged mode using {agentPolicyName}. To ensure that all data required by the integration can be collected, re-enroll the {unprivilegedAgentsCount, plural, one {agent} other {agents}} using an account with root privileges."
values={{
unprivilegedAgentsCount,
agentPolicyName,
}}
/>
) : (
<>
<FormattedMessage
id="xpack.fleet.addIntegration.confirmModal.unprivilegedAgentsDataStreamsMessage"
defaultMessage="This integration has the following data streams that require Elastic Agents to have root privileges. There {unprivilegedAgentsCount, plural, one {is # agent} other {are # agents}} running in an unprivileged mode using {agentPolicyName}. To ensure that all data required by the integration can be collected, re-enroll the {unprivilegedAgentsCount, plural, one {agent} other {agents}} using an account with root privileges."
values={{
unprivilegedAgentsCount,
agentPolicyName,
}}
/>
<ul>
{dataStreams.map((item) => (
<li key={item.name}>{item.title}</li>
))}
</ul>
</>
)}
</EuiCallOut>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,10 @@ describe('when on the package policy create page', () => {
</Route>
));

function getMockPackageInfo(requiresRoot?: boolean) {
function getMockPackageInfo(options?: {
requiresRoot?: boolean;
dataStreamRequiresRoot?: boolean;
}) {
return {
data: {
item: {
Expand Down Expand Up @@ -173,6 +176,11 @@ describe('when on the package policy create page', () => {
dataset: 'nginx.access',
title: 'Nginx access logs',
ingest_pipeline: 'default',
agent: {
privileges: {
root: options?.dataStreamRequiresRoot,
},
},
streams: [
{
input: 'logfile',
Expand Down Expand Up @@ -202,7 +210,7 @@ describe('when on the package policy create page', () => {
status: 'not_installed',
agent: {
privileges: {
root: requiresRoot,
root: options?.requiresRoot,
},
},
},
Expand Down Expand Up @@ -300,8 +308,14 @@ describe('when on the package policy create page', () => {
vars: undefined,
};

beforeEach(() => {
(sendCreatePackagePolicy as jest.MockedFunction<any>).mockClear();
});

test('should show unprivileged warning modal on submit if conditions match', async () => {
(useGetPackageInfoByKeyQuery as jest.Mock).mockReturnValue(getMockPackageInfo(true));
(useGetPackageInfoByKeyQuery as jest.Mock).mockReturnValue(
getMockPackageInfo({ requiresRoot: true })
);
await act(async () => {
render('agent-policy-1');
});
Expand Down Expand Up @@ -338,7 +352,9 @@ describe('when on the package policy create page', () => {
});

test('should show unprivileged warning and agents modal on submit if conditions match', async () => {
(useGetPackageInfoByKeyQuery as jest.Mock).mockReturnValue(getMockPackageInfo(true));
(useGetPackageInfoByKeyQuery as jest.Mock).mockReturnValue(
getMockPackageInfo({ requiresRoot: true })
);
(sendGetAgentStatus as jest.MockedFunction<any>).mockResolvedValueOnce({
data: { results: { total: 1 } },
});
Expand Down Expand Up @@ -367,6 +383,48 @@ describe('when on the package policy create page', () => {
expect(sendCreatePackagePolicy as jest.MockedFunction<any>).toHaveBeenCalled();
});

test('should show unprivileged warning modal with data streams on submit if conditions match', async () => {
(useGetPackageInfoByKeyQuery as jest.Mock).mockReturnValue(
getMockPackageInfo({ dataStreamRequiresRoot: true })
);
await act(async () => {
render('agent-policy-1');
});

let saveBtn: HTMLElement;

await waitFor(() => {
saveBtn = renderResult.getByText(/Save and continue/).closest('button')!;
expect(saveBtn).not.toBeDisabled();
});

await act(async () => {
fireEvent.click(saveBtn);
});

await waitFor(() => {
expect(
renderResult.getByText('Unprivileged agents enrolled to the selected policy')
).toBeInTheDocument();
expect(renderResult.getByTestId('unprivilegedAgentsCallout').textContent).toContain(
'This integration has the following data streams that require Elastic Agents to have root privileges. There is 1 agent running in an unprivileged mode using Agent policy 1. To ensure that all data required by the integration can be collected, re-enroll the agent using an account with root privileges.'
);
expect(renderResult.getByTestId('unprivilegedAgentsCallout').textContent).toContain(
'Nginx access logs'
);
});

await waitFor(() => {
saveBtn = renderResult.getByText(/Add integration/).closest('button')!;
});

await act(async () => {
fireEvent.click(saveBtn);
});

expect(sendCreatePackagePolicy as jest.MockedFunction<any>).toHaveBeenCalledTimes(1);
});

test('should create package policy on submit when query param agent policy id is set', async () => {
await act(async () => {
render('agent-policy-1');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,11 @@ import {

import { useCancelAddPackagePolicy } from '../hooks';

import { isRootPrivilegesRequired, splitPkgKey } from '../../../../../../../common/services';
import {
getRootPrivilegedDataStreams,
isRootPrivilegesRequired,
splitPkgKey,
} from '../../../../../../../common/services';
import type { NewAgentPolicy, PackagePolicyEditExtensionComponentProps } from '../../../../types';
import { SetupTechnology } from '../../../../types';
import {
Expand Down Expand Up @@ -438,6 +442,8 @@ export const CreatePackagePolicySinglePage: CreatePackagePolicyParams = ({
);
}

const rootPrivilegedDataStreams = packageInfo ? getRootPrivilegedDataStreams(packageInfo) : [];

return (
<CreatePackagePolicySinglePageLayout {...layoutProps} data-test-subj="createPackagePolicy">
<EuiErrorBoundary>
Expand All @@ -453,6 +459,7 @@ export const CreatePackagePolicySinglePage: CreatePackagePolicyParams = ({
(agentPolicy?.unprivileged_agents ?? 0) > 0
)}
unprivilegedAgentsCount={agentPolicy?.unprivileged_agents ?? 0}
dataStreams={rootPrivilegedDataStreams}
/>
)}
{formState === 'CONFIRM_UNPRIVILEGED' && agentPolicy ? (
Expand All @@ -461,6 +468,7 @@ export const CreatePackagePolicySinglePage: CreatePackagePolicyParams = ({
onConfirm={onSubmit}
unprivilegedAgentsCount={agentPolicy?.unprivileged_agents ?? 0}
agentPolicyName={agentPolicy?.name ?? ''}
dataStreams={rootPrivilegedDataStreams}
/>
) : null}
{formState === 'SUBMITTED_NO_AGENTS' &&
Expand Down
21 changes: 13 additions & 8 deletions x-pack/plugins/fleet/server/services/package_policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import {
isInputOnlyPolicyTemplate,
getNormalizedDataStreams,
getNormalizedInputs,
isRootPrivilegesRequired,
} from '../../common/services';
import {
SO_SEARCH_LIMIT,
Expand Down Expand Up @@ -329,10 +330,11 @@ class PackagePolicyClientImpl implements PackagePolicyClient {
});
}

if (enrichedPackagePolicy.package && pkgInfo?.agent?.privileges?.root) {
const requiresRoot = isRootPrivilegesRequired(pkgInfo);
if (enrichedPackagePolicy.package && requiresRoot) {
enrichedPackagePolicy.package = {
...enrichedPackagePolicy.package,
requires_root: pkgInfo?.agent?.privileges?.root ?? false,
requires_root: requiresRoot,
};
}
}
Expand Down Expand Up @@ -465,10 +467,11 @@ class PackagePolicyClientImpl implements PackagePolicyClient {

elasticsearch = pkgInfo?.elasticsearch;

if (packagePolicy.package && pkgInfo?.agent?.privileges?.root) {
const requiresRoot = isRootPrivilegesRequired(pkgInfo);
if (packagePolicy.package && requiresRoot) {
packagePolicy.package = {
...packagePolicy.package,
requires_root: pkgInfo?.agent?.privileges?.root ?? false,
requires_root: requiresRoot,
};
}
}
Expand Down Expand Up @@ -877,10 +880,11 @@ class PackagePolicyClientImpl implements PackagePolicyClient {
);
elasticsearchPrivileges = pkgInfo.elasticsearch?.privileges;

if (restOfPackagePolicy.package && pkgInfo?.agent?.privileges?.root) {
const requiresRoot = isRootPrivilegesRequired(pkgInfo);
if (restOfPackagePolicy.package && requiresRoot) {
restOfPackagePolicy.package = {
...restOfPackagePolicy.package,
requires_root: pkgInfo?.agent?.privileges?.root ?? false,
requires_root: requiresRoot,
};
}
}
Expand Down Expand Up @@ -1064,10 +1068,11 @@ class PackagePolicyClientImpl implements PackagePolicyClient {
);
elasticsearchPrivileges = pkgInfo.elasticsearch?.privileges;

if (restOfPackagePolicy.package && pkgInfo?.agent?.privileges?.root) {
const requiresRoot = isRootPrivilegesRequired(pkgInfo);
if (restOfPackagePolicy.package && requiresRoot) {
restOfPackagePolicy.package = {
...restOfPackagePolicy.package,
requires_root: pkgInfo?.agent?.privileges?.root ?? false,
requires_root: requiresRoot,
};
}
}
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit b86039e

Please sign in to comment.