[Ingest Manager] Implement concurrency control for package configs (#70680)

* Send SO version field as part of package configs, enforce it during package config update

* Fix typings, extend response error to include optional status code

* Revert unnecessary version fields in tests, fix schema

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

x-pack/plugins/ingest_manager/common/types/models/package_config.ts

@@ -55,9 +55,14 @@ export interface NewPackageConfig {
   inputs: NewPackageConfigInput[];
 }
 
+export interface UpdatePackageConfig extends NewPackageConfig {
+  version?: string;
+}
+
 export interface PackageConfig extends Omit<NewPackageConfig, 'inputs'> {
   id: string;
   inputs: PackageConfigInput[];
+  version?: string;
   revision: number;
   updated_at: string;
   updated_by: string;
@@ -65,4 +70,4 @@ export interface PackageConfig extends Omit<NewPackageConfig, 'inputs'> {
   created_by: string;
 }
 
-export type PackageConfigSOAttributes = Omit<PackageConfig, 'id'>;
+export type PackageConfigSOAttributes = Omit<PackageConfig, 'id' | 'version'>;

x-pack/plugins/ingest_manager/common/types/rest_spec/package_config.ts

@@ -3,7 +3,7 @@
  * or more contributor license agreements. Licensed under the Elastic License;
  * you may not use this file except in compliance with the Elastic License.
  */
-import { PackageConfig, NewPackageConfig } from '../models';
+import { PackageConfig, NewPackageConfig, UpdatePackageConfig } from '../models';
 
 export interface GetPackageConfigsRequest {
   query: {
@@ -42,7 +42,7 @@ export interface CreatePackageConfigResponse {
 }
 
 export type UpdatePackageConfigRequest = GetOnePackageConfigRequest & {
-  body: NewPackageConfig;
+  body: UpdatePackageConfig;
 };
 
 export type UpdatePackageConfigResponse = CreatePackageConfigResponse;

x-pack/plugins/ingest_manager/public/applications/ingest_manager/hooks/use_request/use_request.ts

@@ -17,33 +17,39 @@ let httpClient: HttpSetup;
 
 export type UseRequestConfig = _UseRequestConfig;
 
+interface RequestError extends Error {
+  statusCode?: number;
+}
+
 export const setHttpClient = (client: HttpSetup) => {
   httpClient = client;
 };
 
-export const sendRequest = <D = any>(
+export const sendRequest = <D = any, E = RequestError>(
   config: SendRequestConfig
-): Promise<SendRequestResponse<D>> => {
+): Promise<SendRequestResponse<D, E>> => {
   if (!httpClient) {
     throw new Error('sendRequest has no http client set');
   }
-  return _sendRequest<D>(httpClient, config);
+  return _sendRequest<D, E>(httpClient, config);
 };
 
-export const useRequest = <D = any>(config: UseRequestConfig) => {
+export const useRequest = <D = any, E = RequestError>(config: UseRequestConfig) => {
   if (!httpClient) {
     throw new Error('sendRequest has no http client set');
   }
-  return _useRequest<D>(httpClient, config);
+  return _useRequest<D, E>(httpClient, config);
 };
 
 export type SendConditionalRequestConfig =
   | (SendRequestConfig & { shouldSendRequest: true })
   | (Partial<SendRequestConfig> & { shouldSendRequest: false });
 
-export const useConditionalRequest = <D = any>(config: SendConditionalRequestConfig) => {
+export const useConditionalRequest = <D = any, E = RequestError>(
+  config: SendConditionalRequestConfig
+) => {
   const [state, setState] = useState<{
-    error: Error | null;
+    error: RequestError | null;
     data: D | null;
     isLoading: boolean;
   }>({
@@ -70,7 +76,7 @@ export const useConditionalRequest = <D = any>(config: SendConditionalRequestCon
         isLoading: true,
         error: null,
       });
-      const res = await sendRequest<D>({
+      const res = await sendRequest<D, E>({
         method: config.method,
         path: config.path,
         query: config.query,

x-pack/plugins/ingest_manager/public/applications/ingest_manager/sections/agent_config/edit_package_config_page/index.tsx

@@ -16,7 +16,7 @@ import {
   EuiFlexItem,
   EuiSpacer,
 } from '@elastic/eui';
-import { AgentConfig, PackageInfo, NewPackageConfig } from '../../../types';
+import { AgentConfig, PackageInfo, UpdatePackageConfig } from '../../../types';
 import {
   useLink,
   useBreadcrumbs,
@@ -72,7 +72,7 @@ export const EditPackageConfigPage: React.FunctionComponent = () => {
   const [loadingError, setLoadingError] = useState<Error>();
   const [agentConfig, setAgentConfig] = useState<AgentConfig>();
   const [packageInfo, setPackageInfo] = useState<PackageInfo>();
-  const [packageConfig, setPackageConfig] = useState<NewPackageConfig>({
+  const [packageConfig, setPackageConfig] = useState<UpdatePackageConfig>({
     name: '',
     description: '',
     namespace: '',
@@ -80,6 +80,7 @@ export const EditPackageConfigPage: React.FunctionComponent = () => {
     enabled: true,
     output_id: '',
     inputs: [],
+    version: '',
   });
 
   // Retrieve agent config, package, and package config info
@@ -160,7 +161,7 @@ export const EditPackageConfigPage: React.FunctionComponent = () => {
   const hasErrors = validationResults ? validationHasErrors(validationResults) : false;
 
   // Update package config method
-  const updatePackageConfig = (updatedFields: Partial<NewPackageConfig>) => {
+  const updatePackageConfig = (updatedFields: Partial<UpdatePackageConfig>) => {
     const newPackageConfig = {
       ...packageConfig,
       ...updatedFields,
@@ -178,7 +179,7 @@ export const EditPackageConfigPage: React.FunctionComponent = () => {
     }
   };
 
-  const updatePackageConfigValidation = (newPackageConfig?: NewPackageConfig) => {
+  const updatePackageConfigValidation = (newPackageConfig?: UpdatePackageConfig) => {
     if (packageInfo) {
       const newValidationResult = validatePackageConfig(
         newPackageConfig || packageConfig,
@@ -234,9 +235,31 @@ export const EditPackageConfigPage: React.FunctionComponent = () => {
             : undefined,
       });
     } else {
-      notifications.toasts.addError(error, {
-        title: 'Error',
-      });
+      if (error.statusCode === 409) {
+        notifications.toasts.addError(error, {
+          title: i18n.translate('xpack.ingestManager.editPackageConfig.failedNotificationTitle', {
+            defaultMessage: `Error updating '{packageConfigName}'`,
+            values: {
+              packageConfigName: packageConfig.name,
+            },
+          }),
+          toastMessage: i18n.translate(
+            'xpack.ingestManager.editPackageConfig.failedConflictNotificationMessage',
+            {
+              defaultMessage: `Data is out of date. Refresh the page to get the latest configuration.`,
+            }
+          ),
+        });
+      } else {
+        notifications.toasts.addError(error, {
+          title: i18n.translate('xpack.ingestManager.editPackageConfig.failedNotificationTitle', {
+            defaultMessage: `Error updating '{packageConfigName}'`,
+            values: {
+              packageConfigName: packageConfig.name,
+            },
+          }),
+        });
+      }
       setFormState('VALID');
     }
   };

x-pack/plugins/ingest_manager/public/applications/ingest_manager/types/index.ts

@@ -15,6 +15,7 @@ export {
   EnrollmentAPIKey,
   PackageConfig,
   NewPackageConfig,
+  UpdatePackageConfig,
   PackageConfigInput,
   PackageConfigInputStream,
   PackageConfigConfigRecordEntry,

x-pack/plugins/ingest_manager/server/routes/package_config/handlers.ts

@@ -178,7 +178,7 @@ export const updatePackageConfigHandler: RequestHandler<
     });
   } catch (e) {
     return response.customError({
-      statusCode: 500,
+      statusCode: e.statusCode || 500,
       body: { message: e.message },
     });
   }

x-pack/plugins/ingest_manager/server/services/package_config.ts

@@ -15,6 +15,7 @@ import {
 import { PACKAGE_CONFIG_SAVED_OBJECT_TYPE } from '../constants';
 import {
   NewPackageConfig,
+  UpdatePackageConfig,
   PackageConfig,
   ListWithKuery,
   PackageConfigSOAttributes,
@@ -60,6 +61,7 @@ class PackageConfigService {
 
     return {
       id: newSo.id,
+      version: newSo.version,
       ...newSo.attributes,
     };
   }
@@ -71,7 +73,7 @@ class PackageConfigService {
     options?: { user?: AuthenticatedUser }
   ): Promise<PackageConfig[]> {
     const isoDate = new Date().toISOString();
-    const { saved_objects: newSos } = await soClient.bulkCreate<Omit<PackageConfig, 'id'>>(
+    const { saved_objects: newSos } = await soClient.bulkCreate<PackageConfigSOAttributes>(
       packageConfigs.map((packageConfig) => ({
         type: SAVED_OBJECT_TYPE,
         attributes: {
@@ -98,6 +100,7 @@ class PackageConfigService {
 
     return newSos.map((newSo) => ({
       id: newSo.id,
+      version: newSo.version,
       ...newSo.attributes,
     }));
   }
@@ -117,6 +120,7 @@ class PackageConfigService {
 
     return {
       id: packageConfigSO.id,
+      version: packageConfigSO.version,
       ...packageConfigSO.attributes,
     };
   }
@@ -137,6 +141,7 @@ class PackageConfigService {
 
     return packageConfigSO.saved_objects.map((so) => ({
       id: so.id,
+      version: so.version,
       ...so.attributes,
     }));
   }
@@ -163,8 +168,9 @@ class PackageConfigService {
     });
 
     return {
-      items: packageConfigs.saved_objects.map<PackageConfig>((packageConfigSO) => ({
+      items: packageConfigs.saved_objects.map((packageConfigSO) => ({
         id: packageConfigSO.id,
+        version: packageConfigSO.version,
         ...packageConfigSO.attributes,
       })),
       total: packageConfigs.total,
@@ -176,21 +182,29 @@ class PackageConfigService {
   public async update(
     soClient: SavedObjectsClientContract,
     id: string,
-    packageConfig: NewPackageConfig,
+    packageConfig: UpdatePackageConfig,
     options?: { user?: AuthenticatedUser }
   ): Promise<PackageConfig> {
     const oldPackageConfig = await this.get(soClient, id);
+    const { version, ...restOfPackageConfig } = packageConfig;
 
     if (!oldPackageConfig) {
       throw new Error('Package config not found');
     }
 
-    await soClient.update<PackageConfigSOAttributes>(SAVED_OBJECT_TYPE, id, {
-      ...packageConfig,
-      revision: oldPackageConfig.revision + 1,
-      updated_at: new Date().toISOString(),
-      updated_by: options?.user?.username ?? 'system',
-    });
+    await soClient.update<PackageConfigSOAttributes>(
+      SAVED_OBJECT_TYPE,
+      id,
+      {
+        ...restOfPackageConfig,
+        revision: oldPackageConfig.revision + 1,
+        updated_at: new Date().toISOString(),
+        updated_by: options?.user?.username ?? 'system',
+      },
+      {
+        version,
+      }
+    );
 
     // Bump revision of associated agent config
     await agentConfigService.bumpRevision(soClient, packageConfig.config_id, {

x-pack/plugins/ingest_manager/server/types/index.tsx

@@ -21,6 +21,7 @@ export {
   PackageConfigInput,
   PackageConfigInputStream,
   NewPackageConfig,
+  UpdatePackageConfig,
   PackageConfigSOAttributes,
   FullAgentConfigInput,
   FullAgentConfig,

x-pack/plugins/ingest_manager/server/types/models/package_config.ts

@@ -66,7 +66,13 @@ export const NewPackageConfigSchema = schema.object({
   ...PackageConfigBaseSchema,
 });
 
+export const UpdatePackageConfigSchema = schema.object({
+  ...PackageConfigBaseSchema,
+  version: schema.maybe(schema.string()),
+});
+
 export const PackageConfigSchema = schema.object({
   ...PackageConfigBaseSchema,
   id: schema.string(),
+  version: schema.maybe(schema.string()),
 });

x-pack/plugins/ingest_manager/server/types/rest_spec/package_config.ts

@@ -4,7 +4,7 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 import { schema } from '@kbn/config-schema';
-import { NewPackageConfigSchema } from '../models';
+import { NewPackageConfigSchema, UpdatePackageConfigSchema } from '../models';
 import { ListWithKuerySchema } from './index';
 
 export const GetPackageConfigsRequestSchema = {
@@ -23,7 +23,7 @@ export const CreatePackageConfigRequestSchema = {
 
 export const UpdatePackageConfigRequestSchema = {
   ...GetOnePackageConfigRequestSchema,
-  body: NewPackageConfigSchema,
+  body: UpdatePackageConfigSchema,
 };
 
 export const DeletePackageConfigsRequestSchema = {