Skip to content

Commit

Permalink
[8.18] [Security Assistant] Fix Product documentation installation ba…
Browse files Browse the repository at this point in the history
…nner (#212463) (#213017)

# Backport

This will backport the following commits from `main` to `8.18`:
- [[Security Assistant] Fix Product documentation installation banner
(#212463)](#212463)

<!--- Backport version: 9.6.6 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sorenlouv/backport)

<!--BACKPORT [{"author":{"name":"Patryk
Kopyciński","email":"contact@patrykkopycinski.com"},"sourceCommit":{"committedDate":"2025-03-04T01:28:35Z","message":"[Security
Assistant] Fix Product documentation installation banner (#212463)\n\n##
Summary\n\nFixes logic on fresh cluster where the ELSER was not started
yet, in\nthis case API reports `status` as `uninstalled`, but it doesn't
mean\nthat the Product documentation was actually uninstall, but rather
it's a\ndefault state.\nAdded internal `product_documentation_status` to
KB status API to make\nsure we keep track of the status internally and
present the banner only\nif the docs were intentionally
uninstalled\n\n---------\n\nCo-authored-by: kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"f0d66691b855fe65291fa8f889def3d3f695fe06","branchLabelMapping":{"^v9.1.0$":"main","^v8.19.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:skip","v9.0.0","backport:version","v8.18.0","v9.1.0","v8.19.0"],"title":"[Security
Assistant] Fix Product documentation installation banner
","number":212463,"url":"https://github.com/elastic/kibana/pull/212463","mergeCommit":{"message":"[Security
Assistant] Fix Product documentation installation banner (#212463)\n\n##
Summary\n\nFixes logic on fresh cluster where the ELSER was not started
yet, in\nthis case API reports `status` as `uninstalled`, but it doesn't
mean\nthat the Product documentation was actually uninstall, but rather
it's a\ndefault state.\nAdded internal `product_documentation_status` to
KB status API to make\nsure we keep track of the status internally and
present the banner only\nif the docs were intentionally
uninstalled\n\n---------\n\nCo-authored-by: kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"f0d66691b855fe65291fa8f889def3d3f695fe06"}},"sourceBranch":"main","suggestedTargetBranches":["9.0","8.18","8.x"],"targetPullRequestStates":[{"branch":"9.0","label":"v9.0.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.18","label":"v8.18.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/212463","number":212463,"mergeCommit":{"message":"[Security
Assistant] Fix Product documentation installation banner (#212463)\n\n##
Summary\n\nFixes logic on fresh cluster where the ELSER was not started
yet, in\nthis case API reports `status` as `uninstalled`, but it doesn't
mean\nthat the Product documentation was actually uninstall, but rather
it's a\ndefault state.\nAdded internal `product_documentation_status` to
KB status API to make\nsure we keep track of the status internally and
present the banner only\nif the docs were intentionally
uninstalled\n\n---------\n\nCo-authored-by: kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"f0d66691b855fe65291fa8f889def3d3f695fe06"}},{"branch":"8.x","label":"v8.19.0","branchLabelMappingKey":"^v8.19.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Patryk Kopyciński <contact@patrykkopycinski.com>
  • Loading branch information
kibanamachine and patrykkopycinski authored Mar 4, 2025
1 parent 94bc52a commit d18e6e8
Show file tree
Hide file tree
Showing 18 changed files with 161 additions and 96 deletions.
2 changes: 2 additions & 0 deletions oas_docs/output/kibana.serverless.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -36281,6 +36281,8 @@ paths:
type: boolean
pipeline_exists:
type: boolean
product_documentation_status:
type: string
security_labs_exists:
type: boolean
user_data_exists:
Expand Down
2 changes: 2 additions & 0 deletions oas_docs/output/kibana.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20374,6 +20374,8 @@ paths:
type: boolean
pipeline_exists:
type: boolean
product_documentation_status:
type: string
security_labs_exists:
type: boolean
user_data_exists:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,8 @@ paths:
type: boolean
pipeline_exists:
type: boolean
product_documentation_status:
type: string
security_labs_exists:
type: boolean
user_data_exists:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,8 @@ paths:
type: boolean
pipeline_exists:
type: boolean
product_documentation_status:
type: string
security_labs_exists:
type: boolean
user_data_exists:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,4 +73,5 @@ export const ReadKnowledgeBaseResponse = z.object({
pipeline_exists: z.boolean().optional(),
security_labs_exists: z.boolean().optional(),
user_data_exists: z.boolean().optional(),
product_documentation_status: z.string().optional(),
});
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ paths:
type: boolean
user_data_exists:
type: boolean
product_documentation_status:
type: string
400:
description: Generic Error
content:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import type { IToasts } from '@kbn/core-notifications-browser';
import { i18n } from '@kbn/i18n';
import { useCallback } from 'react';
import { ReadKnowledgeBaseResponse } from '@kbn/elastic-assistant-common';
import { InstallationStatus } from '@kbn/product-doc-base-plugin/common/install_status';
import { getKnowledgeBaseStatus } from './api';

const KNOWLEDGE_BASE_STATUS_QUERY_KEY = ['elastic-assistant', 'knowledge-base-status'];
Expand All @@ -38,7 +39,10 @@ export const useKnowledgeBaseStatus = ({
resource,
toasts,
enabled,
}: UseKnowledgeBaseStatusParams): UseQueryResult<ReadKnowledgeBaseResponse, IHttpFetchError> => {
}: UseKnowledgeBaseStatusParams): UseQueryResult<
ReadKnowledgeBaseResponse & { product_documentation_status: InstallationStatus },
IHttpFetchError
> => {
return useQuery(
KNOWLEDGE_BASE_STATUS_QUERY_KEY,
async ({ signal }) => {
Expand All @@ -49,7 +53,10 @@ export const useKnowledgeBaseStatus = ({
retry: false,
keepPreviousData: true,
// Polling interval for Knowledge Base setup in progress
refetchInterval: (data) => (data?.is_setup_in_progress ? 30000 : false),
refetchInterval: (data) =>
data?.is_setup_in_progress || data?.product_documentation_status === 'installing'
? 30000
: false,
// Deprecated, hoist to `queryCache` w/in `QueryClient. See: https://stackoverflow.com/a/76961109
onError: (error: IHttpFetchError<ResponseErrorBody>) => {
if (error.name !== 'AbortError') {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import { render, screen, fireEvent, waitFor } from '@testing-library/react';
import { ProductDocumentationManagement } from '.';
import * as i18n from './translations';
import { useInstallProductDoc } from '../../api/product_docs/use_install_product_doc';
import { useGetProductDocStatus } from '../../api/product_docs/use_get_product_doc_status';

jest.mock('../../api/product_docs/use_install_product_doc');
jest.mock('../../api/product_docs/use_get_product_doc_status');
Expand All @@ -18,69 +17,64 @@ describe('ProductDocumentationManagement', () => {
const mockInstallProductDoc = jest.fn().mockResolvedValue({});

beforeEach(() => {
(useInstallProductDoc as jest.Mock).mockReturnValue({ mutateAsync: mockInstallProductDoc });
(useGetProductDocStatus as jest.Mock).mockReturnValue({ status: null, isLoading: false });
jest.clearAllMocks();
});

it('renders loading spinner when status is loading', async () => {
(useGetProductDocStatus as jest.Mock).mockReturnValue({
status: { overall: 'not_installed' },
isLoading: true,
(useInstallProductDoc as jest.Mock).mockReturnValue({
mutateAsync: mockInstallProductDoc,
isLoading: false,
isSuccess: false,
});
render(<ProductDocumentationManagement />);
expect(screen.getByTestId('statusLoading')).toBeInTheDocument();
jest.clearAllMocks();
});

it('renders install button when not installed', () => {
(useGetProductDocStatus as jest.Mock).mockReturnValue({
status: { overall: 'not_installed' },
isLoading: false,
});
render(<ProductDocumentationManagement />);
render(<ProductDocumentationManagement status="uninstalled" />);
expect(screen.getByText(i18n.INSTALL)).toBeInTheDocument();
});

it('does not render anything when already installed', () => {
(useGetProductDocStatus as jest.Mock).mockReturnValue({
status: { overall: 'installed' },
const { container } = render(<ProductDocumentationManagement status="installed" />);
expect(container).toBeEmptyDOMElement();
});

it('does not render anything when the installation was started by the plugin', () => {
(useInstallProductDoc as jest.Mock).mockReturnValue({
mutateAsync: mockInstallProductDoc,
isLoading: false,
isSuccess: false,
});
const { container } = render(<ProductDocumentationManagement />);
const { container } = render(<ProductDocumentationManagement status="installing" />);
expect(container).toBeEmptyDOMElement();
});

it('shows installing spinner and text when installing', async () => {
(useGetProductDocStatus as jest.Mock).mockReturnValue({
status: { overall: 'not_installed' },
isLoading: false,
});
render(<ProductDocumentationManagement />);
fireEvent.click(screen.getByText(i18n.INSTALL));
await waitFor(() => {
expect(screen.getByTestId('installing')).toBeInTheDocument();
expect(screen.getByText(i18n.INSTALLING)).toBeInTheDocument();
(useInstallProductDoc as jest.Mock).mockReturnValue({
mutateAsync: mockInstallProductDoc,
isLoading: true,
isSuccess: false,
});
render(<ProductDocumentationManagement status="uninstalled" />);
expect(screen.getByTestId('installing')).toBeInTheDocument();
expect(screen.getByText(i18n.INSTALLING)).toBeInTheDocument();
});

it('sets installed state to true after successful installation', async () => {
(useGetProductDocStatus as jest.Mock).mockReturnValue({
status: { overall: 'not_installed' },
(useInstallProductDoc as jest.Mock).mockReturnValue({
mutateAsync: mockInstallProductDoc,
isLoading: false,
isSuccess: true,
});
mockInstallProductDoc.mockResolvedValueOnce({});
render(<ProductDocumentationManagement />);
fireEvent.click(screen.getByText(i18n.INSTALL));
await waitFor(() => expect(screen.queryByText(i18n.INSTALL)).not.toBeInTheDocument());
render(<ProductDocumentationManagement status="uninstalled" />);
expect(screen.queryByText(i18n.INSTALL)).not.toBeInTheDocument();
});

it('sets installed state to false after failed installation', async () => {
(useGetProductDocStatus as jest.Mock).mockReturnValue({
status: { overall: 'not_installed' },
(useInstallProductDoc as jest.Mock).mockReturnValue({
mutateAsync: mockInstallProductDoc,
isLoading: false,
isSuccess: false,
});
mockInstallProductDoc.mockRejectedValueOnce(new Error('Installation failed'));
render(<ProductDocumentationManagement />);
render(<ProductDocumentationManagement status="uninstalled" />);
fireEvent.click(screen.getByText(i18n.INSTALL));
await waitFor(() => expect(screen.getByText(i18n.INSTALL)).toBeInTheDocument());
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,43 +14,25 @@ import {
EuiSpacer,
EuiText,
} from '@elastic/eui';
import React, { useEffect, useState, useCallback, useMemo } from 'react';
import React, { useCallback, useMemo } from 'react';
import { InstallationStatus } from '@kbn/product-doc-base-plugin/common/install_status';
import { useInstallProductDoc } from '../../api/product_docs/use_install_product_doc';
import { useGetProductDocStatus } from '../../api/product_docs/use_get_product_doc_status';
import * as i18n from './translations';

export const ProductDocumentationManagement: React.FC = React.memo(() => {
const [{ isInstalled, isInstalling }, setState] = useState({
isInstalled: true,
isInstalling: false,
});
export const ProductDocumentationManagement = React.memo<{
status?: InstallationStatus;
}>(({ status }) => {
const {
mutateAsync: installProductDoc,
isSuccess: isInstalled,
isLoading: isInstalling,
} = useInstallProductDoc();

const { mutateAsync: installProductDoc } = useInstallProductDoc();
const { status, isLoading: isStatusLoading } = useGetProductDocStatus();

useEffect(() => {
if (status) {
setState((prevState) => ({
...prevState,
isInstalled: status.overall === 'installed',
}));
}
}, [status]);

const onClickInstall = useCallback(async () => {
setState((prevState) => ({ ...prevState, isInstalling: true }));
try {
await installProductDoc();
setState({ isInstalled: true, isInstalling: false });
} catch {
setState({ isInstalled: false, isInstalling: false });
}
const onClickInstall = useCallback(() => {
installProductDoc();
}, [installProductDoc]);

const content = useMemo(() => {
if (isStatusLoading) {
return <EuiLoadingSpinner data-test-subj="statusLoading" size="m" />;
}
if (isInstalling) {
return (
<EuiFlexGroup justifyContent="flexStart" alignItems="center">
Expand All @@ -72,11 +54,18 @@ export const ProductDocumentationManagement: React.FC = React.memo(() => {
</EuiFlexItem>
</EuiFlexGroup>
);
}, [isInstalling, isStatusLoading, onClickInstall]);
}, [isInstalling, onClickInstall]);

if (isInstalled) {
// The last condition means that the installation was started by the plugin
if (
!status ||
status === 'installed' ||
isInstalled ||
(status === 'installing' && !isInstalling)
) {
return null;
}

return (
<>
<EuiCallOut title={i18n.LABEL} iconType="iInCircle">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ export const KnowledgeBaseSettingsManagement: React.FC<Params> = React.memo(({ d

return (
<>
<ProductDocumentationManagement />
<ProductDocumentationManagement status={kbStatus?.product_documentation_status} />
<EuiPanel hasShadow={false} hasBorder paddingSize="l">
<EuiText size={'m'}>
<FormattedMessage
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ type ConversationsDataClientContract = PublicMethodsOf<AIAssistantConversationsD
export type ConversationsDataClientMock = jest.Mocked<ConversationsDataClientContract>;
type AttackDiscoveryDataClientContract = PublicMethodsOf<AttackDiscoveryDataClient>;
export type AttackDiscoveryDataClientMock = jest.Mocked<AttackDiscoveryDataClientContract>;
type KnowledgeBaseDataClientContract = PublicMethodsOf<AIAssistantKnowledgeBaseDataClient>;
type KnowledgeBaseDataClientContract = PublicMethodsOf<AIAssistantKnowledgeBaseDataClient> & {
isSetupInProgress: AIAssistantKnowledgeBaseDataClient['isSetupInProgress'];
};
export type KnowledgeBaseDataClientMock = jest.Mocked<KnowledgeBaseDataClientContract>;

const createConversationsDataClientMock = () => {
Expand Down Expand Up @@ -73,9 +75,11 @@ const createKnowledgeBaseDataClientMock = () => {
isModelInstalled: jest.fn(),
isSecurityLabsDocsLoaded: jest.fn(),
isSetupAvailable: jest.fn(),
isSetupInProgress: jest.fn().mockReturnValue(false)(),
isUserDataExists: jest.fn(),
setupKnowledgeBase: jest.fn(),
getLoadedSecurityLabsDocsCount: jest.fn(),
getProductDocumentationStatus: jest.fn(),
};
return mocked;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ describe('AIAssistantKnowledgeBaseDataClient', () => {
ml,
getElserId: getElserId.mockResolvedValue('elser-id'),
getIsKBSetupInProgress: mockGetIsKBSetupInProgress.mockReturnValue(false),
getProductDocumentationStatus: jest.fn().mockResolvedValue('installed'),
ingestPipelineResourceName: 'something',
setIsKBSetupInProgress: jest.fn().mockImplementation(() => {}),
manageGlobalKnowledgeBaseAIAssistant: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import {
} from '@kbn/core/server';
import { IndexPatternsFetcher } from '@kbn/data-views-plugin/server';
import { map } from 'lodash';
import { InstallationStatus } from '@kbn/product-doc-base-plugin/common/install_status';
import type { TrainedModelsProvider } from '@kbn/ml-plugin/server/shared_services/providers';
import { getMlNodeCount } from '@kbn/ml-plugin/server/lib/node_utils';
import { AIAssistantDataClient, AIAssistantDataClientParams } from '..';
Expand Down Expand Up @@ -86,6 +87,7 @@ export interface KnowledgeBaseDataClientParams extends AIAssistantDataClientPara
ml: MlPluginSetup;
getElserId: GetElser;
getIsKBSetupInProgress: (spaceId: string) => boolean;
getProductDocumentationStatus: () => Promise<InstallationStatus>;
ingestPipelineResourceName: string;
setIsKBSetupInProgress: (spaceId: string, isInProgress: boolean) => void;
manageGlobalKnowledgeBaseAIAssistant: boolean;
Expand All @@ -100,6 +102,11 @@ export class AIAssistantKnowledgeBaseDataClient extends AIAssistantDataClient {
public get isSetupInProgress() {
return this.options.getIsKBSetupInProgress(this.spaceId);
}

public getProductDocumentationStatus = async () => {
return (await this.options.getProductDocumentationStatus()) ?? 'uninstalled';
};

/**
* Returns whether setup of the Knowledge Base can be performed (essentially an ML features check)
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,11 @@ describe('helpers', () => {
mockProductDocManager.getStatus.mockResolvedValue({ status: 'uninstalled' });
mockProductDocManager.install.mockResolvedValue(null);

await ensureProductDocumentationInstalled(mockProductDocManager, mockLogger);
await ensureProductDocumentationInstalled({
productDocManager: mockProductDocManager,
setIsProductDocumentationInProgress: jest.fn(),
logger: mockLogger,
});

expect(mockProductDocManager.getStatus).toHaveBeenCalled();
expect(mockLogger.debug).toHaveBeenCalledWith(
Expand All @@ -42,7 +46,11 @@ describe('helpers', () => {
it('should not install product documentation if already installed', async () => {
mockProductDocManager.getStatus.mockResolvedValue({ status: 'installed' });

await ensureProductDocumentationInstalled(mockProductDocManager, mockLogger);
await ensureProductDocumentationInstalled({
productDocManager: mockProductDocManager,
setIsProductDocumentationInProgress: jest.fn(),
logger: mockLogger,
});

expect(mockProductDocManager.getStatus).toHaveBeenCalled();
expect(mockProductDocManager.install).not.toHaveBeenCalled();
Expand All @@ -54,7 +62,11 @@ describe('helpers', () => {
mockProductDocManager.getStatus.mockResolvedValue({ status: 'not_installed' });
mockProductDocManager.install.mockRejectedValue(new Error('Install failed'));

await ensureProductDocumentationInstalled(mockProductDocManager, mockLogger);
await ensureProductDocumentationInstalled({
productDocManager: mockProductDocManager,
setIsProductDocumentationInProgress: jest.fn(),
logger: mockLogger,
});

expect(mockProductDocManager.getStatus).toHaveBeenCalled();
expect(mockProductDocManager.install).toHaveBeenCalled();
Expand All @@ -67,7 +79,11 @@ describe('helpers', () => {
it('should log a warning if getStatus fails', async () => {
mockProductDocManager.getStatus.mockRejectedValue(new Error('Status check failed'));

await ensureProductDocumentationInstalled(mockProductDocManager, mockLogger);
await ensureProductDocumentationInstalled({
productDocManager: mockProductDocManager,
setIsProductDocumentationInProgress: jest.fn(),
logger: mockLogger,
});

expect(mockProductDocManager.getStatus).toHaveBeenCalled();
expect(mockLogger.warn).toHaveBeenCalledWith(
Expand Down
Loading

0 comments on commit d18e6e8

Please sign in to comment.