Skip to content
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ import {
expectError,
createBadRequestErrorPayload,
expectUpdateResult,
MULTI_NAMESPACE_TYPE,
} from '../../test_helpers/repository.test.common';
import type { ISavedObjectsSecurityExtension } from '@kbn/core-saved-objects-server';
import { savedObjectsExtensionsMock } from '../../mocks/saved_objects_extensions.mock';
Expand Down Expand Up @@ -620,6 +621,74 @@ describe('#bulkUpdate', () => {
2
);
});

it('migrates single namespace objects using the object namespace', async () => {
const modifiedObj2 = {
...obj2,
coreMigrationVersion: '8.0.0',
namespace: 'test',
};
const objects = [modifiedObj2];
migrator.migrateDocument.mockImplementationOnce((doc) => ({ ...doc, migrated: true }));

await bulkUpdateSuccess(client, repository, registry, objects);

expect(migrator.migrateDocument).toHaveBeenCalledTimes(2);
expectMigrationArgs(
{
id: modifiedObj2.id,
namespace: 'test',
},
true,
2
);
});

it('migrates multiple namespace objects using the object namespaces', async () => {
const modifiedObj2 = {
...obj2,
type: MULTI_NAMESPACE_TYPE,
coreMigrationVersion: '8.0.0',
namespace: 'test',
};
const objects = [modifiedObj2];
migrator.migrateDocument.mockImplementationOnce((doc) => ({ ...doc, migrated: true }));

await bulkUpdateSuccess(client, repository, registry, objects);

expect(migrator.migrateDocument).toHaveBeenCalledTimes(2);
expectMigrationArgs(
{
id: modifiedObj2.id,
namespaces: ['test'],
},
true,
2
);
});

it('migrates namespace agnsostic objects', async () => {
const modifiedObj2 = {
...obj2,
type: NAMESPACE_AGNOSTIC_TYPE,
coreMigrationVersion: '8.0.0',
namespace: 'test', // specify a namespace, but it should be ignored
};
const objects = [modifiedObj2];
migrator.migrateDocument.mockImplementationOnce((doc) => ({ ...doc, migrated: true }));

await bulkUpdateSuccess(client, repository, registry, objects);

expect(migrator.migrateDocument).toHaveBeenCalledTimes(2);
expectMigrationArgs(
{
id: modifiedObj2.id,
namespaces: [],
},
true,
2
);
});
});

describe('returns', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,17 +150,11 @@ export const performBulkUpdate = async <T>(
};
}

// `objectNamespace` is a namespace string, while `namespace` is a namespace ID.
// The object namespace string, if defined, will supersede the operation's namespace ID.
const namespaceString = SavedObjectsUtils.namespaceIdToString(namespace);

const getNamespaceId = (objectNamespace?: string) =>
objectNamespace !== undefined
? SavedObjectsUtils.namespaceStringToId(objectNamespace)
: namespace;

const getNamespaceString = (objectNamespace?: string) => objectNamespace ?? namespaceString;

const bulkGetDocs = validObjects.map(({ value: { type, id, objectNamespace } }) => ({
_id: serializer.generateRawId(getNamespaceId(objectNamespace), type, id),
_index: commonHelper.getIndexForType(type),
Expand Down Expand Up @@ -235,7 +229,6 @@ export const performBulkUpdate = async <T>(
mergeAttributes,
} = expectedBulkGetResult.value;

let namespaces: string[] | undefined;
const versionProperties = getExpectedVersionProperties(version);
const indexFound = bulkGetResponse?.statusCode !== 404;
const actualResult = indexFound ? bulkGetResponse?.body.docs[esRequestIndex] : undefined;
Expand All @@ -258,15 +251,18 @@ export const performBulkUpdate = async <T>(
});
}

let savedObjectNamespace: string | undefined;
let savedObjectNamespaces: string[] | undefined;

if (isMultiNS) {
// @ts-expect-error MultiGetHit is incorrectly missing _id, _source
namespaces = actualResult!._source.namespaces ?? [
savedObjectNamespaces = actualResult!._source.namespaces ?? [
// @ts-expect-error MultiGetHit is incorrectly missing _id, _source
SavedObjectsUtils.namespaceIdToString(actualResult!._source.namespace),
];
} else if (registry.isSingleNamespace(type)) {
// if `objectNamespace` is undefined, fall back to `options.namespace`
namespaces = [getNamespaceString(objectNamespace)];
savedObjectNamespace = objectNamespace ?? namespace;
}

const document = getSavedObjectFromSource<T>(
Expand Down Expand Up @@ -310,8 +306,8 @@ export const performBulkUpdate = async <T>(
...migrated!,
id,
type,
namespace,
namespaces,
...(savedObjectNamespace && { namespace: savedObjectNamespace }),
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't comment on the right place, but in line 297 should we fallback to namespace or namespaceString for the encryption helper?

Copy link
Contributor

@jeramysoucy jeramysoucy Jun 13, 2025

Choose a reason for hiding this comment

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

It actually doesn't matter if we pass a namespace ID or a namespace string to the encryption extension - it will always get "normalized" and used appropriately. However, you brought to my attention that the parameters might have always been wrong here.

The object should be encrypted based on its descriptor, not the current namespace or the provided object namespace. So... let me think about this...

Disregard...I forgot that we do not use namespace in the descriptor for multi-namespace ESOs.

...(savedObjectNamespaces && { namespaces: savedObjectNamespaces }),
attributes: updatedAttributes,
updated_at: time,
updated_by: updatedBy,
Expand All @@ -321,6 +317,9 @@ export const performBulkUpdate = async <T>(
migratedUpdatedSavedObjectDoc as SavedObjectSanitizedDoc
);

const namespaces =
savedObjectNamespaces ?? (savedObjectNamespace ? [savedObjectNamespace] : []);

const expectedResult = {
type,
id,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -546,9 +546,13 @@ describe('#update', () => {
namespace: 'default',
});
expect(client.index).toHaveBeenCalledWith(
expect.objectContaining({ id: expect.stringMatching(`${type}:${id}`) }),
expect.objectContaining({
id: expect.stringMatching(`${type}:${id}`),
}),
expect.anything()
);
// Assert that 'namespace' does not exist at all
expect(client.index.mock.calls[0][0]).not.toHaveProperty('namespace');
});

it(`doesn't prepend namespace to the id when using agnostic-namespace type`, async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -716,7 +716,7 @@ export const expectUpdateResult = ({
attributes,
references,
version: mockVersion,
namespaces: ['default'],
namespaces: [],
...mockTimestampFields,
});

Expand Down