[8.6] [Infrastructure UI] Improve metrics settings error handling (#1…

…46272) (#146675) # Backport This will backport the following commits from `main` to `8.6`: - [[Infrastructure UI] Improve metrics settings error handling (#146272)](#146272)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Marco Antonio Ghiani <[email protected]>
elastic · Nov 30, 2022 · 28991d4 · 28991d4
1 parent c9fe5ac
commit 28991d4
Show file tree

Hide file tree

Showing 8 changed files with 161 additions and 50 deletions.
diff --git a/packages/kbn-io-ts-utils/index.ts b/packages/kbn-io-ts-utils/index.ts
@@ -6,9 +6,11 @@
  * Side Public License, v 1.
  */
 
+export type { IndexPatternType } from './src/index_pattern_rt';
 export type { NonEmptyStringBrand } from './src/non_empty_string_rt';
 
 export { deepExactRt } from './src/deep_exact_rt';
+export { indexPatternRt } from './src/index_pattern_rt';
 export { jsonRt } from './src/json_rt';
 export { mergeRt } from './src/merge_rt';
 export { strictKeysRt } from './src/strict_keys_rt';

diff --git a/packages/kbn-io-ts-utils/src/index_pattern_rt/index.test.ts b/packages/kbn-io-ts-utils/src/index_pattern_rt/index.test.ts
@@ -0,0 +1,35 @@
+/*
+ * 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 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+import { indexPatternRt } from '.';
+import { isRight } from 'fp-ts/lib/Either';
+
+describe('indexPatternRt', () => {
+  test('passes on valid index pattern strings', () => {
+    expect(isRight(indexPatternRt.decode('logs-*'))).toBe(true);
+    expect(isRight(indexPatternRt.decode('logs-*,filebeat-*'))).toBe(true);
+  });
+
+  test('fails if the pattern is an empty string', () => {
+    expect(isRight(indexPatternRt.decode(''))).toBe(false);
+  });
+
+  test('fails if the pattern contains empty spaces', () => {
+    expect(isRight(indexPatternRt.decode(' '))).toBe(false);
+    expect(isRight(indexPatternRt.decode(' logs-*'))).toBe(false);
+    expect(isRight(indexPatternRt.decode('logs-* '))).toBe(false);
+    expect(isRight(indexPatternRt.decode('logs-*, filebeat-*'))).toBe(false);
+  });
+
+  test('fails if the pattern contains empty comma-separated entries', () => {
+    expect(isRight(indexPatternRt.decode(',logs-*'))).toBe(false);
+    expect(isRight(indexPatternRt.decode('logs-*,'))).toBe(false);
+    expect(isRight(indexPatternRt.decode('logs-*,,filebeat-*'))).toBe(false);
+    expect(isRight(indexPatternRt.decode('logs-*,,,filebeat-*'))).toBe(false);
+  });
+});
diff --git a/packages/kbn-io-ts-utils/src/index_pattern_rt/index.ts b/packages/kbn-io-ts-utils/src/index_pattern_rt/index.ts
@@ -0,0 +1,36 @@
+/*
+ * 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 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+import * as rt from 'io-ts';
+
+export const isEmptyString = (value: string) => value === '';
+
+export const containsSpaces = (value: string) => value.includes(' ');
+
+export const containsEmptyEntries = (value: string) => value.split(',').some(isEmptyString);
+
+export const validateIndexPattern = (indexPattern: string) => {
+  return (
+    !isEmptyString(indexPattern) &&
+    !containsSpaces(indexPattern) &&
+    !containsEmptyEntries(indexPattern)
+  );
+};
+
+export interface IndexPatternBrand {
+  readonly IndexPattern: unique symbol;
+}
+
+type IndexPattern = rt.Branded<string, IndexPatternBrand>;
+
+export const indexPatternRt = rt.brand(
+  rt.string,
+  (pattern): pattern is IndexPattern => validateIndexPattern(pattern),
+  'IndexPattern'
+);
+
+export type IndexPatternType = rt.TypeOf<typeof indexPatternRt>;
diff --git a/x-pack/plugins/infra/common/metrics_sources/index.ts b/x-pack/plugins/infra/common/metrics_sources/index.ts
@@ -5,6 +5,7 @@
  * 2.0.
  */
 
+import { indexPatternRt } from '@kbn/io-ts-utils';
 import * as rt from 'io-ts';
 import {
   SourceConfigurationRT,
@@ -28,6 +29,11 @@ export type MetricsSourceConfigurationProperties = rt.TypeOf<
   typeof metricsSourceConfigurationPropertiesRT
 >;
 
+export const partialMetricsSourceConfigurationReqPayloadRT = rt.partial({
+  ...metricsSourceConfigurationPropertiesRT.type.props,
+  metricAlias: indexPatternRt,
+});
+
 export const partialMetricsSourceConfigurationPropertiesRT = rt.partial({
   ...metricsSourceConfigurationPropertiesRT.type.props,
 });

diff --git a/x-pack/plugins/infra/public/pages/metrics/settings/indices_configuration_form_state.ts b/x-pack/plugins/infra/public/pages/metrics/settings/indices_configuration_form_state.ts
@@ -6,9 +6,13 @@
  */
 
 import { ReactNode, useCallback, useMemo, useState } from 'react';
+
 import {
+  aggregateValidationErrors,
   createInputFieldProps,
   createInputRangeFieldProps,
+  validateInputFieldHasNotEmptyEntries,
+  validateInputFieldHasNotEmptySpaces,
   validateInputFieldNotEmpty,
 } from './input_fields';
 
@@ -41,7 +45,7 @@ export const useIndicesConfigurationFormState = ({
   const nameFieldProps = useMemo(
     () =>
       createInputFieldProps({
-        errors: validateInputFieldNotEmpty(formState.name),
+        errors: aggregateValidationErrors<string>(validateInputFieldNotEmpty)(formState.name),
         name: 'name',
         onChange: (name) => setFormStateChanges((changes) => ({ ...changes, name })),
         value: formState.name,
@@ -51,7 +55,11 @@ export const useIndicesConfigurationFormState = ({
   const metricAliasFieldProps = useMemo(
     () =>
       createInputFieldProps({
-        errors: validateInputFieldNotEmpty(formState.metricAlias),
+        errors: aggregateValidationErrors<string>(
+          validateInputFieldNotEmpty,
+          validateInputFieldHasNotEmptyEntries,
+          validateInputFieldHasNotEmptySpaces
+        )(formState.metricAlias),
         name: 'metricAlias',
         onChange: (metricAlias) => setFormStateChanges((changes) => ({ ...changes, metricAlias })),
         value: formState.metricAlias,
@@ -62,7 +70,7 @@ export const useIndicesConfigurationFormState = ({
   const anomalyThresholdFieldProps = useMemo(
     () =>
       createInputRangeFieldProps({
-        errors: validateInputFieldNotEmpty(formState.anomalyThreshold),
+        errors: aggregateValidationErrors(validateInputFieldNotEmpty)(formState.anomalyThreshold),
         name: 'anomalyThreshold',
         onChange: (anomalyThreshold) =>
           setFormStateChanges((changes) => ({ ...changes, anomalyThreshold })),

diff --git a/x-pack/plugins/infra/public/pages/metrics/settings/input_fields.tsx b/x-pack/plugins/infra/public/pages/metrics/settings/input_fields.tsx
@@ -22,6 +22,10 @@ export interface InputFieldProps<
 
 export type FieldErrorMessage = string | JSX.Element;
 
+export type ValidationHandlerList<ValueType> = Array<
+  (value: ValueType) => FieldErrorMessage | false
+>;
+
 export const createInputFieldProps = <
   Value extends string = string,
   FieldElement extends HTMLInputElement = HTMLInputElement
@@ -83,12 +87,39 @@ export const createInputRangeFieldProps = <
   value,
 });
 
-export const validateInputFieldNotEmpty = (value: React.ReactText) =>
-  value === ''
-    ? [
-        <FormattedMessage
-          id="xpack.infra.sourceConfiguration.fieldEmptyErrorMessage"
-          defaultMessage="The field must not be empty"
-        />,
-      ]
-    : [];
+export const aggregateValidationErrors =
+  <ValueType extends ReactText = ReactText>(
+    ...validationHandlers: ValidationHandlerList<ValueType>
+  ) =>
+  (value: ValueType) =>
+    validationHandlers.map((validator) => validator(value)).filter(Boolean) as FieldErrorMessage[];
+
+const isEmptyString = (value: ReactText) => value === '';
+
+const containsSpaces = (value: string) => value.includes(' ');
+
+const containsEmptyEntries = (value: string) => value.split(',').some(isEmptyString);
+
+export const validateInputFieldNotEmpty = (value: ReactText) =>
+  isEmptyString(value) && (
+    <FormattedMessage
+      id="xpack.infra.sourceConfiguration.fieldEmptyErrorMessage"
+      defaultMessage="The field must not be empty."
+    />
+  );
+
+export const validateInputFieldHasNotEmptyEntries = (value: string) =>
+  containsEmptyEntries(value) && (
+    <FormattedMessage
+      id="xpack.infra.sourceConfiguration.fieldContainEmptyEntryErrorMessage"
+      defaultMessage="The field must not include empty comma-separated values."
+    />
+  );
+
+export const validateInputFieldHasNotEmptySpaces = (value: string) =>
+  containsSpaces(value) && (
+    <FormattedMessage
+      id="xpack.infra.sourceConfiguration.fieldContainSpacesErrorMessage"
+      defaultMessage="The field must not include spaces."
+    />
+  );
diff --git a/x-pack/plugins/infra/public/pages/metrics/settings/source_configuration_settings.tsx b/x-pack/plugins/infra/public/pages/metrics/settings/source_configuration_settings.tsx
@@ -15,7 +15,7 @@ import {
 } from '@elastic/eui';
 import { i18n } from '@kbn/i18n';
 import { FormattedMessage } from '@kbn/i18n-react';
-import React, { useCallback, useMemo } from 'react';
+import React, { useCallback } from 'react';
 import { Prompt } from '@kbn/observability-plugin/public';
 import { SourceLoadingPage } from '../../../components/source_loading_page';
 import { useSourceContext } from '../../../containers/metrics_source';
@@ -75,19 +75,13 @@ export const SourceConfigurationSettings = ({
     formStateChanges,
   ]);
 
-  const isWriteable = useMemo(
-    () => shouldAllowEdit && source && source.origin !== 'internal',
-    [shouldAllowEdit, source]
-  );
+  const isWriteable = shouldAllowEdit && (!Boolean(source) || source?.origin !== 'internal');
 
   const { hasInfraMLCapabilities } = useInfraMLCapabilitiesContext();
 
   if ((isLoading || isUninitialized) && !source) {
     return <SourceLoadingPage />;
   }
-  if (!source?.configuration) {
-    return null;
-  }
 
   return (
     <MetricsPageTemplate

diff --git a/x-pack/plugins/infra/server/routes/metrics_sources/index.ts b/x-pack/plugins/infra/server/routes/metrics_sources/index.ts
@@ -13,9 +13,9 @@ import { hasData } from '../../lib/sources/has_data';
 import { createSearchClient } from '../../lib/create_search_client';
 import { AnomalyThresholdRangeError } from '../../lib/sources/errors';
 import {
-  partialMetricsSourceConfigurationPropertiesRT,
   metricsSourceConfigurationResponseRT,
   MetricsSourceStatus,
+  partialMetricsSourceConfigurationReqPayloadRT,
 } from '../../../common/metrics_sources';
 
 export const initMetricsSourceConfigurationRoutes = (libs: InfraBackendLibs) => {
@@ -35,24 +35,33 @@ export const initMetricsSourceConfigurationRoutes = (libs: InfraBackendLibs) =>
       const { sourceId } = request.params;
       const soClient = (await requestContext.core).savedObjects.client;
 
-      const [source, metricIndicesExist, indexFields] = await Promise.all([
-        libs.sources.getSourceConfiguration(soClient, sourceId),
-        libs.sourceStatus.hasMetricIndices(requestContext, sourceId),
-        libs.fields.getFields(requestContext, sourceId, 'METRICS'),
-      ]);
+      try {
+        const [source, metricIndicesExist, indexFields] = await Promise.all([
+          libs.sources.getSourceConfiguration(soClient, sourceId),
+          libs.sourceStatus.hasMetricIndices(requestContext, sourceId),
+          libs.fields.getFields(requestContext, sourceId, 'METRICS'),
+        ]);
 
-      if (!source) {
-        return response.notFound();
-      }
+        if (!source) {
+          return response.notFound();
+        }
 
-      const status: MetricsSourceStatus = {
-        metricIndicesExist,
-        indexFields,
-      };
+        const status: MetricsSourceStatus = {
+          metricIndicesExist,
+          indexFields,
+        };
 
-      return response.ok({
-        body: metricsSourceConfigurationResponseRT.encode({ source: { ...source, status } }),
-      });
+        return response.ok({
+          body: metricsSourceConfigurationResponseRT.encode({ source: { ...source, status } }),
+        });
+      } catch (error) {
+        return response.customError({
+          statusCode: error.statusCode ?? 500,
+          body: {
+            message: error.message ?? 'An unexpected error occurred',
+          },
+        });
+      }
     }
   );
 
@@ -64,13 +73,13 @@ export const initMetricsSourceConfigurationRoutes = (libs: InfraBackendLibs) =>
         params: schema.object({
           sourceId: schema.string(),
         }),
-        body: createValidationFunction(partialMetricsSourceConfigurationPropertiesRT),
+        body: createValidationFunction(partialMetricsSourceConfigurationReqPayloadRT),
       },
     },
     framework.router.handleLegacyErrors(async (requestContext, request, response) => {
       const { sources } = libs;
       const { sourceId } = request.params;
-      const patchedSourceConfigurationProperties = request.body;
+      const sourceConfigurationPayload = request.body;
 
       try {
         const soClient = (await requestContext.core).savedObjects.client;
@@ -84,18 +93,8 @@ export const initMetricsSourceConfigurationRoutes = (libs: InfraBackendLibs) =>
 
         const sourceConfigurationExists = sourceConfiguration.origin === 'stored';
         const patchedSourceConfiguration = await (sourceConfigurationExists
-          ? sources.updateSourceConfiguration(
-              soClient,
-              sourceId,
-              // @ts-ignore
-              patchedSourceConfigurationProperties
-            )
-          : sources.createSourceConfiguration(
-              soClient,
-              sourceId,
-              // @ts-ignore
-              patchedSourceConfigurationProperties
-            ));
+          ? sources.updateSourceConfiguration(soClient, sourceId, sourceConfigurationPayload)
+          : sources.createSourceConfiguration(soClient, sourceId, sourceConfigurationPayload));
 
         const [metricIndicesExist, indexFields] = await Promise.all([
           libs.sourceStatus.hasMetricIndices(requestContext, sourceId),