Skip to content

Commit

Permalink
## [Security Solution] fixes Data Quality dashboard errors when a `ba…
Browse files Browse the repository at this point in the history
…sePath` is configured

This PR implements a fix for an [issue](#156231) where the Data Quality dashboard displays errors when  a `basePath` is configured, preventing indices from being checked.

### Desk testing

- Verify the fix per the reporduction steps in <#156231>
- Also verify the page still behaves correctly when Kibana is started with no base path, via:

```
yarn start --no-base-path
```

### Before / after screenshots

**Before:**

![before](https://user-images.githubusercontent.com/4459398/235273609-952fd7e4-0a22-4344-b1e4-48411c6d0e33.png)

_Above: Before the fix, errors occur when a `basePath` is configured_

**After:**

![after](https://user-images.githubusercontent.com/4459398/235276257-c62feb05-699a-418c-8da2-b90b6683e5b4.png)

_Above: After the fix, errors do NOT occur_
  • Loading branch information
andrew-goldstein committed Apr 29, 2023
1 parent ec76672 commit f891f21
Show file tree
Hide file tree
Showing 27 changed files with 1,050 additions and 177 deletions.
Expand Up @@ -7,7 +7,7 @@

import { DARK_THEME } from '@elastic/charts';
import numeral from '@elastic/numeral';
import { render, screen } from '@testing-library/react';
import { render, screen, waitFor } from '@testing-library/react';
import React from 'react';

import { EMPTY_STAT } from '../../../helpers';
Expand Down Expand Up @@ -66,14 +66,16 @@ const defaultProps: Props = {

describe('DataQualityDetails', () => {
describe('when ILM phases are provided', () => {
beforeEach(() => {
beforeEach(async () => {
jest.clearAllMocks();

render(
<TestProviders>
<DataQualityDetails {...defaultProps} />
</TestProviders>
);

await waitFor(() => {});
});

test('it renders the storage details', () => {
Expand Down
Expand Up @@ -7,7 +7,7 @@

import { DARK_THEME } from '@elastic/charts';
import numeral from '@elastic/numeral';
import { render, screen } from '@testing-library/react';
import { render, screen, waitFor } from '@testing-library/react';
import React from 'react';

import { EMPTY_STAT } from '../../../../helpers';
Expand Down Expand Up @@ -67,14 +67,16 @@ const defaultProps: Props = {
};

describe('IndicesDetails', () => {
beforeEach(() => {
beforeEach(async () => {
jest.clearAllMocks();

render(
<TestProviders>
<IndicesDetails {...defaultProps} />
</TestProviders>
);

await waitFor(() => {});
});

describe('rendering patterns', () => {
Expand Down
Expand Up @@ -7,7 +7,7 @@

import { DARK_THEME } from '@elastic/charts';
import numeral from '@elastic/numeral';
import { render, screen } from '@testing-library/react';
import { render, screen, waitFor } from '@testing-library/react';
import React from 'react';

import { EMPTY_STAT } from '../../helpers';
Expand All @@ -25,7 +25,7 @@ const formatNumber = (value: number | undefined) =>
const ilmPhases: string[] = ['hot', 'warm', 'unmanaged'];

describe('IndexInvalidValues', () => {
test('it renders the data quality summary', () => {
test('it renders the data quality summary', async () => {
render(
<TestProviders>
<Body
Expand All @@ -44,14 +44,16 @@ describe('IndexInvalidValues', () => {
</TestProviders>
);

expect(screen.getByTestId('dataQualitySummary')).toBeInTheDocument();
await waitFor(() => {
expect(screen.getByTestId('dataQualitySummary')).toBeInTheDocument();
});
});

describe('patterns', () => {
const patterns = ['.alerts-security.alerts-default', 'auditbeat-*', 'logs-*', 'packetbeat-*'];

patterns.forEach((pattern) => {
test(`it renders the '${pattern}' pattern`, () => {
test(`it renders the '${pattern}' pattern`, async () => {
render(
<TestProviders>
<Body
Expand All @@ -70,7 +72,9 @@ describe('IndexInvalidValues', () => {
</TestProviders>
);

expect(screen.getByTestId(`${pattern}PatternPanel`)).toBeInTheDocument();
await waitFor(() => {
expect(screen.getByTestId(`${pattern}PatternPanel`)).toBeInTheDocument();
});
});
});

Expand All @@ -94,7 +98,9 @@ describe('IndexInvalidValues', () => {
);

const items = await screen.findAllByTestId('bodyPatternSpacer');
expect(items).toHaveLength(patterns.length - 1);
await waitFor(() => {
expect(items).toHaveLength(patterns.length - 1);
});
});
});
});
@@ -0,0 +1,40 @@
/*
* 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 { renderHook } from '@testing-library/react-hooks';
import React from 'react';

import { DataQualityProvider, useDataQualityContext } from '.';

const mockHttpFetch = jest.fn();
const ContextWrapper: React.FC = ({ children }) => (
<DataQualityProvider httpFetch={mockHttpFetch}>{children}</DataQualityProvider>
);

describe('DataQualityContext', () => {
beforeEach(() => {
jest.clearAllMocks();
});

test('it throws an error when useDataQualityContext hook is used without a DataQualityContext', () => {
const { result } = renderHook(useDataQualityContext);

expect(result.error).toEqual(
new Error('useDataQualityContext must be used within a DataQualityProvider')
);
});

test('it should return the httpFetch function', async () => {
const { result } = renderHook(useDataQualityContext, { wrapper: ContextWrapper });
const httpFetch = await result.current.httpFetch;

const path = '/path/to/resource';
httpFetch(path);

expect(mockHttpFetch).toBeCalledWith(path);
});
});
@@ -0,0 +1,39 @@
/*
* 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 type { HttpHandler } from '@kbn/core-http-browser';
import React, { useMemo } from 'react';

interface DataQualityProviderProps {
httpFetch: HttpHandler;
}

const DataQualityContext = React.createContext<DataQualityProviderProps | undefined>(undefined);

export const DataQualityProvider: React.FC<DataQualityProviderProps> = ({
children,
httpFetch,
}) => {
const value = useMemo(
() => ({
httpFetch,
}),
[httpFetch]
);

return <DataQualityContext.Provider value={value}>{children}</DataQualityContext.Provider>;
};

export const useDataQualityContext = () => {
const context = React.useContext(DataQualityContext);

if (context == null) {
throw new Error('useDataQualityContext must be used within a DataQualityProvider');
}

return context;
};
Expand Up @@ -87,6 +87,7 @@ describe('checkIndex', () => {

const indexName = 'auditbeat-custom-index-1';
const pattern = 'auditbeat-*';
const httpFetch = jest.fn();

describe('happy path', () => {
const onCheckCompleted = jest.fn();
Expand All @@ -99,6 +100,7 @@ describe('checkIndex', () => {
ecsMetadata,
formatBytes,
formatNumber,
httpFetch,
indexName,
onCheckCompleted,
pattern,
Expand Down Expand Up @@ -145,6 +147,7 @@ describe('checkIndex', () => {
ecsMetadata,
formatBytes,
formatNumber,
httpFetch,
indexName,
onCheckCompleted,
pattern,
Expand All @@ -166,6 +169,7 @@ describe('checkIndex', () => {
ecsMetadata: null, // <--
formatBytes,
formatNumber,
httpFetch,
indexName,
onCheckCompleted,
pattern,
Expand Down Expand Up @@ -218,6 +222,7 @@ describe('checkIndex', () => {
ecsMetadata,
formatBytes,
formatNumber,
httpFetch,
indexName,
onCheckCompleted,
pattern,
Expand All @@ -226,7 +231,7 @@ describe('checkIndex', () => {
});

test('it invokes onCheckCompleted with the expected `error`', () => {
expect(onCheckCompleted.mock.calls[0][0].error).toEqual(`Error: ${error}`);
expect(onCheckCompleted.mock.calls[0][0].error).toEqual(error);
});

test('it invokes onCheckCompleted with the expected `indexName`', () => {
Expand Down Expand Up @@ -268,6 +273,7 @@ describe('checkIndex', () => {
ecsMetadata,
formatBytes,
formatNumber,
httpFetch,
indexName,
onCheckCompleted,
pattern,
Expand Down Expand Up @@ -326,6 +332,7 @@ describe('checkIndex', () => {
ecsMetadata,
formatBytes,
formatNumber,
httpFetch,
indexName,
onCheckCompleted,
pattern,
Expand Down
Expand Up @@ -5,6 +5,7 @@
* 2.0.
*/

import type { HttpHandler } from '@kbn/core-http-browser';
import { getUnallowedValueRequestItems } from '../../../allowed_values/helpers';
import {
getMappingsProperties,
Expand All @@ -27,6 +28,7 @@ export async function checkIndex({
ecsMetadata,
formatBytes,
formatNumber,
httpFetch,
indexName,
onCheckCompleted,
pattern,
Expand All @@ -36,13 +38,18 @@ export async function checkIndex({
ecsMetadata: Record<string, EcsMetadata> | null;
formatBytes: (value: number | undefined) => string;
formatNumber: (value: number | undefined) => string;
httpFetch: HttpHandler;
indexName: string;
onCheckCompleted: OnCheckCompleted;
pattern: string;
version: string;
}) {
try {
const indexes = await fetchMappings({ abortController, patternOrIndexName: indexName });
const indexes = await fetchMappings({
abortController,
httpFetch,
patternOrIndexName: indexName,
});

const requestItems = getUnallowedValueRequestItems({
ecsMetadata,
Expand All @@ -51,6 +58,7 @@ export async function checkIndex({

const searchResults = await fetchUnallowedValues({
abortController,
httpFetch,
indexName,
requestItems,
});
Expand Down Expand Up @@ -87,7 +95,7 @@ export async function checkIndex({
} catch (error) {
if (!abortController.signal.aborted) {
onCheckCompleted({
error: error != null ? error.toString() : i18n.AN_ERROR_OCCURRED_CHECKING_INDEX(indexName),
error: error != null ? error.message : i18n.AN_ERROR_OCCURRED_CHECKING_INDEX(indexName),
formatBytes,
formatNumber,
indexName,
Expand Down
Expand Up @@ -12,6 +12,7 @@ import React, { useCallback, useEffect, useRef, useState } from 'react';
import styled from 'styled-components';

import { checkIndex } from './check_index';
import { useDataQualityContext } from '../../../data_quality_context';
import { getAllIndicesToCheck } from './helpers';
import * as i18n from '../../../../translations';
import type { EcsMetadata, IndexToCheck, OnCheckCompleted } from '../../../../types';
Expand Down Expand Up @@ -58,6 +59,7 @@ const CheckAllComponent: React.FC<Props> = ({
setCheckAllTotalIndiciesToCheck,
setIndexToCheck,
}) => {
const { httpFetch } = useDataQualityContext();
const abortController = useRef(new AbortController());
const [isRunning, setIsRunning] = useState<boolean>(false);

Expand Down Expand Up @@ -91,6 +93,7 @@ const CheckAllComponent: React.FC<Props> = ({
ecsMetadata: EcsFlat as unknown as Record<string, EcsMetadata>,
formatBytes,
formatNumber,
httpFetch,
indexName,
onCheckCompleted,
pattern,
Expand Down Expand Up @@ -121,6 +124,7 @@ const CheckAllComponent: React.FC<Props> = ({
cancelIfRunning,
formatBytes,
formatNumber,
httpFetch,
incrementCheckAllIndiciesChecked,
isRunning,
onCheckCompleted,
Expand Down

0 comments on commit f891f21

Please sign in to comment.