[8.6] [Infrastructure UI] Improve metrics settings error handling (#146272) (#146675)

# Backport This will backport the following commits from `main` to `8.6`: - [[Infrastructure UI] Improve metrics settings error handling (#146272)](https://github.com/elastic/kibana/pull/146272)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Marco Antonio Ghiani <marcoantonio.ghiani01@gmail.com>
2025-04-23 17:28:26 -04:00 · 2022-11-30 07:00:48 -05:00 · 2022-11-30 07:00:48 -05:00 · 28991d49d8
commit 28991d49d8
parent c9fe5ac74b
8 changed files with 162 additions and 51 deletions
--- 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';
--- 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);
+  });
+});
--- 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>;
--- 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,
 });
--- 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 })),
--- 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."
+    />
+  );
--- 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
--- 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,
+        };
+
+        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',
+          },
+        });
      }
-
-      const status: MetricsSourceStatus = {
-        metricIndicesExist,
-        indexFields,
-      };
-
-      return response.ok({
-        body: metricsSourceConfigurationResponseRT.encode({ source: { ...source, status } }),
-      });
    }
  );

@ -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),