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

[Saved Objects] Adds managed to import options #155677

Merged
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -293,11 +293,5 @@ export function setManaged({
optionsManaged?: boolean;
objectManaged?: boolean;
}): boolean {
if (optionsManaged !== undefined) {
return optionsManaged;
} else if (optionsManaged === undefined && objectManaged !== undefined) {
return objectManaged;
} else {
return false;
}
return optionsManaged ?? objectManaged ?? false;
}
Original file line number Diff line number Diff line change
Expand Up @@ -59,4 +59,10 @@ export interface LegacyUrlAlias {
* created because of saved object conversion, then we will display a toast telling the user that the object has a new URL.
*/
purpose?: 'savedObjectConversion' | 'savedObjectImport';
/**
* Flag indicating if a saved object is managed by Kibana (default=false).
* Only used when upserting a saved object. If the saved object already
* exist this option has no effect.
*/
managed?: boolean;
Comment on lines +62 to +67
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the purpose of surfacing this managed flag on our legacy url alias definition?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's to set all objects created during import as managed if the option is provided. Legacy alias get created if they need to and the repository will set the default for managed as false if it's not specified.
In short: to bypass adding a default.

}
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ export interface SavedObjectsImportFailure {
* If `overwrite` is specified, an attempt was made to overwrite an existing object.
*/
overwrite?: boolean;
managed?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question here, I don't see it as being a problem, but just so I understand, why do we need to surface the managed flag on SO import failures/successes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a reference, that's all.

error:
| SavedObjectsImportConflictError
| SavedObjectsImportAmbiguousConflictError
Expand Down Expand Up @@ -125,6 +126,14 @@ export interface SavedObjectsImportSuccess {
* If `overwrite` is specified, this object overwrote an existing one (or will do so, in the case of a pending resolution).
*/
overwrite?: boolean;
/**
* Flag indicating if a saved object is managed by Kibana (default=false)
*
* This can be leveraged by applications to e.g. prevent edits to a managed
* saved object. Instead, users can be guided to create a copy first and
* make their edits to the copy.
*/
managed?: boolean;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,12 @@ describe('#importSavedObjectsFromStream', () => {
management: { icon: `${type}-icon` },
} as any),
importHooks = {},
managed,
}: {
createNewCopies?: boolean;
getTypeImpl?: (name: string) => any;
importHooks?: Record<string, SavedObjectsImportHook[]>;
managed?: boolean;
} = {}): ImportSavedObjectsOptions => {
readStream = new Readable();
savedObjectsClient = savedObjectsClientMock.create();
Expand All @@ -98,19 +100,23 @@ describe('#importSavedObjectsFromStream', () => {
namespace,
createNewCopies,
importHooks,
managed,
};
};
const createObject = ({
type = 'foo-type',
title = 'some-title',
}: { type?: string; title?: string } = {}): SavedObject<{
managed = undefined, // explicitly declare undefined so as not set to test against existing objects
}: { type?: string; title?: string; managed?: boolean } = {}): SavedObject<{
title: string;
managed?: boolean;
}> => {
return {
type,
id: uuidv4(),
references: [],
attributes: { title },
managed,
};
};
const createError = (): SavedObjectsImportFailure => {
Expand Down Expand Up @@ -320,6 +326,55 @@ describe('#importSavedObjectsFromStream', () => {
importStateMap,
overwrite,
namespace,
managed: options.managed,
};
expect(mockCreateSavedObjects).toHaveBeenCalledWith(createSavedObjectsParams);
});

test('creates managed saved objects', async () => {
const options = setupOptions({ managed: true });
const collectedObjects = [createObject({ managed: true })];
const filteredObjects = [createObject({ managed: false })];
const errors = [createError(), createError(), createError(), createError()];
mockCollectSavedObjects.mockResolvedValue({
errors: [errors[0]],
collectedObjects,
importStateMap: new Map([
['foo', {}],
['bar', {}],
['baz', { isOnlyReference: true }],
]),
});
mockCheckReferenceOrigins.mockResolvedValue({
importStateMap: new Map([['baz', { isOnlyReference: true, destinationId: 'newId1' }]]),
});
mockValidateReferences.mockResolvedValue([errors[1]]);
mockCheckConflicts.mockResolvedValue({
errors: [errors[2]],
filteredObjects,
importStateMap: new Map([['foo', { destinationId: 'newId2' }]]),
pendingOverwrites: new Set(),
});
mockCheckOriginConflicts.mockResolvedValue({
errors: [errors[3]],
importStateMap: new Map([['bar', { destinationId: 'newId3' }]]),
pendingOverwrites: new Set(),
});

await importSavedObjectsFromStream(options);
const importStateMap = new Map([
['foo', { destinationId: 'newId2' }],
['bar', { destinationId: 'newId3' }],
['baz', { isOnlyReference: true, destinationId: 'newId1' }],
]);
const createSavedObjectsParams = {
objects: collectedObjects,
accumulatedErrors: errors,
savedObjectsClient,
importStateMap,
overwrite,
namespace,
managed: options.managed,
};
expect(mockCreateSavedObjects).toHaveBeenCalledWith(createSavedObjectsParams);
});
Expand Down Expand Up @@ -383,6 +438,118 @@ describe('#importSavedObjectsFromStream', () => {
expect(mockCreateSavedObjects).toHaveBeenCalledWith(createSavedObjectsParams);
});
});

describe('managed option', () => {
test('if not provided, calls create without an override', async () => {
const options = setupOptions({ createNewCopies: true }); // weithout `managed` set
const collectedObjects = [
createObject({ type: 'foo', managed: true }),
createObject({ type: 'bar', title: 'bar-title', managed: false }),
];
const errors = [createError(), createError()];
mockCollectSavedObjects.mockResolvedValue({
errors: [errors[0]],
collectedObjects,
importStateMap: new Map([
['foo', {}],
['bar', { isOnlyReference: true }],
]),
});
mockCheckReferenceOrigins.mockResolvedValue({
importStateMap: new Map([['bar', { isOnlyReference: true, destinationId: 'newId' }]]),
});
mockValidateReferences.mockResolvedValue([errors[1]]);
mockRegenerateIds.mockReturnValue(new Map([['foo', { destinationId: `randomId1` }]]));

await importSavedObjectsFromStream(options);
const importStateMap: ImportStateMap = new Map([
['foo', { destinationId: `randomId1` }],
['bar', { isOnlyReference: true, destinationId: 'newId' }],
]);
const createSavedObjectsParams = {
objects: collectedObjects,
accumulatedErrors: errors,
savedObjectsClient,
importStateMap,
overwrite,
namespace,
managed: undefined,
};
expect(mockCreateSavedObjects).toHaveBeenCalledWith(createSavedObjectsParams);
}); // assert that the call to create will not override the object props.

test('creates managed saved objects, overriding existing `managed` value', async () => {
const options = setupOptions({ createNewCopies: true, managed: true });
const collectedObjects = [createObject({ managed: false })];
const errors = [createError(), createError()];
mockCollectSavedObjects.mockResolvedValue({
errors: [errors[0]],
collectedObjects,
importStateMap: new Map([
['foo', {}],
['bar', { isOnlyReference: true }],
]),
});
mockCheckReferenceOrigins.mockResolvedValue({
importStateMap: new Map([['bar', { isOnlyReference: true, destinationId: 'newId' }]]),
});
mockValidateReferences.mockResolvedValue([errors[1]]);
mockRegenerateIds.mockReturnValue(new Map([['foo', { destinationId: `randomId1` }]]));

await importSavedObjectsFromStream(options);
// assert that the importStateMap is correctly composed of the results from the three modules
const importStateMap: ImportStateMap = new Map([
['foo', { destinationId: `randomId1` }],
['bar', { isOnlyReference: true, destinationId: 'newId' }],
]);
const createSavedObjectsParams = {
objects: collectedObjects,
accumulatedErrors: errors,
savedObjectsClient,
importStateMap,
overwrite,
namespace,
managed: true,
};
expect(mockCreateSavedObjects).toHaveBeenCalledWith(createSavedObjectsParams);
});

test('creates and converts objects from managed to unmanaged', async () => {
const options = setupOptions({ createNewCopies: true, managed: false });
const collectedObjects = [createObject({ managed: true })];
const errors = [createError(), createError()];
mockCollectSavedObjects.mockResolvedValue({
errors: [errors[0]],
collectedObjects,
importStateMap: new Map([
['foo', {}],
['bar', { isOnlyReference: true }],
]),
});
mockCheckReferenceOrigins.mockResolvedValue({
importStateMap: new Map([['bar', { isOnlyReference: true, destinationId: 'newId' }]]),
});
mockValidateReferences.mockResolvedValue([errors[1]]);
mockRegenerateIds.mockReturnValue(new Map([['foo', { destinationId: `randomId1` }]]));

await importSavedObjectsFromStream(options);
// assert that the importStateMap is correctly composed of the results from the three modules
const importStateMap: ImportStateMap = new Map([
['foo', { destinationId: `randomId1` }],
['bar', { isOnlyReference: true, destinationId: 'newId' }],
]);
const createSavedObjectsParams = {
objects: collectedObjects,
accumulatedErrors: errors,
savedObjectsClient,
importStateMap,
overwrite,
namespace,
managed: false,
};
expect(mockCreateSavedObjects).toHaveBeenCalledWith(createSavedObjectsParams);
});
});
});

describe('results', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ export interface ImportSavedObjectsOptions {
* different Kibana versions (e.g. generate legacy URL aliases for all imported objects that have to change IDs).
*/
compatibilityMode?: boolean;
/**
* If provided, Kibana will apply the given option to the `managed` property.
*/
managed?: boolean;
}

/**
Expand All @@ -73,6 +77,7 @@ export async function importSavedObjectsFromStream({
namespace,
refresh,
compatibilityMode,
managed,
}: ImportSavedObjectsOptions): Promise<SavedObjectsImportResponse> {
let errorAccumulator: SavedObjectsImportFailure[] = [];
const supportedTypes = typeRegistry.getImportableAndExportableTypes().map((type) => type.name);
Expand All @@ -82,6 +87,7 @@ export async function importSavedObjectsFromStream({
readStream,
objectLimit,
supportedTypes,
managed,
});
errorAccumulator = [...errorAccumulator, ...collectSavedObjectsResult.errors];
// Map of all IDs for objects that we are attempting to import, and any references that are not included in the read stream;
Expand Down Expand Up @@ -154,12 +160,13 @@ export async function importSavedObjectsFromStream({
namespace,
refresh,
compatibilityMode,
managed,
};
const createSavedObjectsResult = await createSavedObjects(createSavedObjectsParams);
errorAccumulator = [...errorAccumulator, ...createSavedObjectsResult.errors];

const successResults = createSavedObjectsResult.createdObjects.map((createdObject) => {
const { type, id, destinationId, originId } = createdObject;
const { type, id, destinationId, originId, managed: createdObjectManaged } = createdObject;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ts complains about mirrored types

const getTitle = typeRegistry.getType(type)?.management?.getTitle;
const meta = {
title: getTitle ? getTitle(createdObject) : createdObject.attributes.title,
Expand All @@ -170,6 +177,7 @@ export async function importSavedObjectsFromStream({
type,
id,
meta,
managed: createdObjectManaged ?? managed,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: we're passing managed to createSavedObjects, so in theory all the created objects will have correct value set, right? So I think managed: createdObjectManaged would have the same effect?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea, it would.

...(attemptedOverwrite && { overwrite: true }),
...(destinationId && { destinationId }),
...(destinationId && !originId && !createNewCopies && { createNewCopy: true }),
Expand Down
Loading