Skip to content

Commit

Permalink
[Security Solution] fixes Data Quality dashboard errors when a `baseP…
Browse files Browse the repository at this point in the history
…ath` is configured (elastic#156233)

## [Security Solution] fixes Data Quality dashboard errors when a `basePath` is configured

This PR implements a fix for an [issue](elastic#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 <elastic#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_

(cherry picked from commit 3389a2b)
  • Loading branch information
andrew-goldstein committed May 4, 2023
1 parent e02d94a commit 1e56b5c
Show file tree
Hide file tree
Showing 27 changed files with 957 additions and 182 deletions.
Original file line number Diff line number Diff line change
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(() => {}); // wait for PatternComponent state updates
});

test('it renders the storage details', () => {
Expand Down
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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);
});
});
});
});
Original file line number Diff line number Diff line change
@@ -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);
});
});
Original file line number Diff line number Diff line change
@@ -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;
};
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,9 @@ describe('checkIndex', () => {

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

describe('happy path', () => {
describe('when `checkIndex` successfully completes the check', () => {
const onCheckCompleted = jest.fn();

beforeEach(async () => {
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Loading

0 comments on commit 1e56b5c

Please sign in to comment.