Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 4 commits into from
Jul 6, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,19 @@ 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;
created_at: string;
created_by: string;
}

export type PackageConfigSOAttributes = Omit<PackageConfig, 'id'>;
export type PackageConfigSOAttributes = Omit<PackageConfig, 'id' | 'version'>;
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down Expand Up @@ -42,7 +42,7 @@ export interface CreatePackageConfigResponse {
}

export type UpdatePackageConfigRequest = GetOnePackageConfigRequest & {
body: NewPackageConfig;
body: UpdatePackageConfig;
};

export type UpdatePackageConfigResponse = CreatePackageConfigResponse;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}>({
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -72,14 +72,15 @@ 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: '',
config_id: '',
enabled: true,
output_id: '',
inputs: [],
version: '',
});

// Retrieve agent config, package, and package config info
Expand Down Expand Up @@ -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,
Expand All @@ -178,7 +179,7 @@ export const EditPackageConfigPage: React.FunctionComponent = () => {
}
};

const updatePackageConfigValidation = (newPackageConfig?: NewPackageConfig) => {
const updatePackageConfigValidation = (newPackageConfig?: UpdatePackageConfig) => {
if (packageInfo) {
const newValidationResult = validatePackageConfig(
newPackageConfig || packageConfig,
Expand Down Expand Up @@ -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');
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export {
EnrollmentAPIKey,
PackageConfig,
NewPackageConfig,
UpdatePackageConfig,
PackageConfigInput,
PackageConfigInputStream,
PackageConfigConfigRecordEntry,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ export const updatePackageConfigHandler: RequestHandler<
});
} catch (e) {
return response.customError({
statusCode: 500,
statusCode: e.statusCode || 500,
body: { message: e.message },
});
}
Expand Down
32 changes: 23 additions & 9 deletions x-pack/plugins/ingest_manager/server/services/package_config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
import { PACKAGE_CONFIG_SAVED_OBJECT_TYPE } from '../constants';
import {
NewPackageConfig,
UpdatePackageConfig,
PackageConfig,
ListWithKuery,
PackageConfigSOAttributes,
Expand Down Expand Up @@ -60,6 +61,7 @@ class PackageConfigService {

return {
id: newSo.id,
version: newSo.version,
...newSo.attributes,
};
}
Expand All @@ -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: {
Expand All @@ -98,6 +100,7 @@ class PackageConfigService {

return newSos.map((newSo) => ({
id: newSo.id,
version: newSo.version,
...newSo.attributes,
}));
}
Expand All @@ -117,6 +120,7 @@ class PackageConfigService {

return {
id: packageConfigSO.id,
version: packageConfigSO.version,
...packageConfigSO.attributes,
};
}
Expand All @@ -137,6 +141,7 @@ class PackageConfigService {

return packageConfigSO.saved_objects.map((so) => ({
id: so.id,
version: so.version,
...so.attributes,
}));
}
Expand All @@ -161,8 +166,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,
Expand All @@ -174,21 +180,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, {
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/ingest_manager/server/types/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export {
PackageConfigInput,
PackageConfigInputStream,
NewPackageConfig,
UpdatePackageConfig,
PackageConfigSOAttributes,
FullAgentConfigInput,
FullAgentConfig,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
});
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -23,7 +23,7 @@ export const CreatePackageConfigRequestSchema = {

export const UpdatePackageConfigRequestSchema = {
...GetOnePackageConfigRequestSchema,
body: NewPackageConfigSchema,
body: UpdatePackageConfigSchema,
};

export const DeletePackageConfigsRequestSchema = {
Expand Down