[Infrastructure UI] Improve metrics settings error handling

x-pack/plugins/infra/common/source_configuration/validate_index_pattern.test.ts

@@ -0,0 +1,33 @@
+/*
+ * 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 { validateIndexPattern } from './validate_index_pattern';
+
+describe('validateIndexPattern', () => {
+  test('should return true if the value is a valid settings index pattern', () => {
+    expect(validateIndexPattern('logs-*')).toBe(true);
+    expect(validateIndexPattern('logs-*,filebeat-*')).toBe(true);
+  });
+
+  test('should return false if the index pattern is an empty string', () => {
+    expect(validateIndexPattern('')).toBe(false);
+  });
+
+  test('should return false if the index pattern contains empty spaces', () => {
+    expect(validateIndexPattern(' ')).toBe(false);
+    expect(validateIndexPattern(' logs-*')).toBe(false);
+    expect(validateIndexPattern('logs-* ')).toBe(false);
+    expect(validateIndexPattern('logs-*, filebeat-*')).toBe(false);
+  });
+
+  test('should return false if the index pattern contains empty comma-separated entries', () => {
+    expect(validateIndexPattern(',logs-*')).toBe(false);
+    expect(validateIndexPattern('logs-*,')).toBe(false);
+    expect(validateIndexPattern('logs-*,,filebeat-*')).toBe(false);
+    expect(validateIndexPattern('logs-*,,,filebeat-*')).toBe(false);
+  });
+});

x-pack/plugins/infra/common/source_configuration/validate_index_pattern.ts

@@ -0,0 +1,28 @@
+/*
+ * 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.
+ */
+
+export const validateIndexPattern = (indexPattern?: string) => {
+  if (indexPattern === undefined) return true;
+
+  return (
+    !isEmptyString(indexPattern) &&
+    !containsEmptyEntries(indexPattern) &&
+    !containsSpaces(indexPattern)
+  );
+};
+
+export const isEmptyString = (value: string) => {
+  return value === '';
+};
+
+export const containsEmptyEntries = (value: string) => {
+  return value.split(',').some(isEmptyString);
+};
+
+export const containsSpaces = (value: string) => {
+  return value.includes(' ');
+};

x-pack/plugins/infra/public/pages/metrics/settings/indices_configuration_form_state.ts

@@ -7,8 +7,11 @@
 
 import { ReactNode, useCallback, useMemo, useState } from 'react';
 import {
+  aggregateValidationErrors,
   createInputFieldProps,
   createInputRangeFieldProps,
+  validateInputFieldHasNotEmptyEntries,
+  validateInputFieldHasNotEmptySpaces,
   validateInputFieldNotEmpty,
 } from './input_fields';
 
@@ -41,7 +44,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 +54,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 +69,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 })),

x-pack/plugins/infra/public/pages/metrics/settings/input_fields.tsx

@@ -8,6 +8,10 @@
 import React, { ReactText } from 'react';
 
 import { FormattedMessage } from '@kbn/i18n-react';
+import {
+  containsEmptyEntries,
+  containsSpaces,
+} from '../../../../common/source_configuration/validate_index_pattern';
 
 export interface InputFieldProps<
   Value extends string = string,
@@ -22,6 +26,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 +91,33 @@ 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[];
+
+export const validateInputFieldNotEmpty = (value: ReactText) =>
+  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."
+    />
+  );

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

x-pack/plugins/infra/server/lib/sources/errors.ts

@@ -28,3 +28,11 @@ export class SavedObjectReferenceResolutionError extends Error {
     this.name = 'SavedObjectReferenceResolutionError';
   }
 }
+
+export class InvalidMetricIndicesError extends Error {
+  constructor(message?: string) {
+    super(message);
+    Object.setPrototypeOf(this, new.target.prototype);
+    this.name = 'InvalidMetricIndicesError';
+  }
+}

x-pack/plugins/infra/server/routes/metrics_sources/index.ts

@@ -11,12 +11,13 @@ import { createValidationFunction } from '../../../common/runtime_types';
 import { InfraBackendLibs } from '../../lib/infra_types';
 import { hasData } from '../../lib/sources/has_data';
 import { createSearchClient } from '../../lib/create_search_client';
-import { AnomalyThresholdRangeError } from '../../lib/sources/errors';
+import { AnomalyThresholdRangeError, InvalidMetricIndicesError } from '../../lib/sources/errors';
 import {
   partialMetricsSourceConfigurationPropertiesRT,
   metricsSourceConfigurationResponseRT,
   MetricsSourceStatus,
 } from '../../../common/metrics_sources';
+import { assertMetricAlias } from './lib/validate_metric_alias';
 
 export const initMetricsSourceConfigurationRoutes = (libs: InfraBackendLibs) => {
   const { framework } = libs;
@@ -35,24 +36,28 @@ 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 (err) {
+        return response.notFound();
+      }
     }
   );
 
@@ -70,9 +75,13 @@ export const initMetricsSourceConfigurationRoutes = (libs: InfraBackendLibs) =>
     framework.router.handleLegacyErrors(async (requestContext, request, response) => {
       const { sources } = libs;
       const { sourceId } = request.params;
-      const patchedSourceConfigurationProperties = request.body;
+      const sourceConfigurationPayload = request.body;
 
       try {
+        if (sourceConfigurationPayload.metricAlias) {
+          assertMetricAlias(sourceConfigurationPayload.metricAlias);
+        }
+
         const soClient = (await requestContext.core).savedObjects.client;
         const sourceConfiguration = await sources.getSourceConfiguration(soClient, sourceId);
 
@@ -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),
@@ -117,6 +116,10 @@ export const initMetricsSourceConfigurationRoutes = (libs: InfraBackendLibs) =>
           throw error;
         }
 
+        if (error instanceof InvalidMetricIndicesError) {
+          return response.badRequest({ body: error.message });
+        }
+
         if (error instanceof AnomalyThresholdRangeError) {
           return response.customError({
             statusCode: 400,

x-pack/plugins/infra/server/routes/metrics_sources/lib/validate_metric_alias.ts

@@ -0,0 +1,19 @@
+/*
+ * 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 { validateIndexPattern } from '../../../../common/source_configuration/validate_index_pattern';
+import { InvalidMetricIndicesError } from '../../../lib/sources/errors';
+
+export function assertMetricAlias(metricAlias?: string) {
+  const isValidMetricAlias = validateIndexPattern(metricAlias);
+
+  if (!isValidMetricAlias) {
+    throw new InvalidMetricIndicesError(
+      'The metric indices value contains invalid characters (empty values, spaces).'
+    );
+  }
+}