[RAM] Fix maintenance window update not updating removal of fields (#155431)

## Summary

Fix a bug where when we remove fields from `rRule` during a maintenance
window update, the removed fields are not removed because of how ES does
partial updates.

Due to the complexity of the `rRule` schema, we decided to simply delete
and re-create the maintenance window with the same ID, otherwise, we
would have to diff and null each of the removed fields specifically.

### Checklist
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

x-pack/plugins/alerting/server/maintenance_window_client/methods/update.test.ts

@@ -9,7 +9,7 @@ import moment from 'moment-timezone';
 import { RRule } from 'rrule';
 import { update } from './update';
 import { savedObjectsClientMock, loggingSystemMock } from '@kbn/core/server/mocks';
-import { SavedObjectsUpdateResponse, SavedObject } from '@kbn/core/server';
+import { SavedObject } from '@kbn/core/server';
 import {
   MaintenanceWindowClientContext,
   MAINTENANCE_WINDOW_SAVED_OBJECT_TYPE,
@@ -72,14 +72,14 @@ describe('MaintenanceWindowClient - update', () => {
       id: 'test-id',
     } as unknown as SavedObject);
 
-    savedObjectsClient.update.mockResolvedValueOnce({
+    savedObjectsClient.create.mockResolvedValueOnce({
       attributes: {
         ...mockMaintenanceWindow,
         ...updatedAttributes,
         ...updatedMetadata,
       },
       id: 'test-id',
-    } as unknown as SavedObjectsUpdateResponse);
+    } as unknown as SavedObject);
 
     jest.useFakeTimers().setSystemTime(new Date(secondTimestamp));
 
@@ -92,9 +92,8 @@ describe('MaintenanceWindowClient - update', () => {
       MAINTENANCE_WINDOW_SAVED_OBJECT_TYPE,
       'test-id'
     );
-    expect(savedObjectsClient.update).toHaveBeenLastCalledWith(
+    expect(savedObjectsClient.create).toHaveBeenLastCalledWith(
       MAINTENANCE_WINDOW_SAVED_OBJECT_TYPE,
-      'test-id',
       {
         ...updatedAttributes,
         events: [
@@ -104,7 +103,7 @@ describe('MaintenanceWindowClient - update', () => {
         expirationDate: moment(new Date(secondTimestamp)).tz('UTC').add(1, 'year').toISOString(),
         ...updatedMetadata,
       },
-      { version: '123' }
+      { id: 'test-id' }
     );
     // Only these 3 properties are worth asserting since the rest come from mocks
     expect(result).toEqual(
@@ -141,25 +140,24 @@ describe('MaintenanceWindowClient - update', () => {
       id: 'test-id',
     } as unknown as SavedObject);
 
-    savedObjectsClient.update.mockResolvedValue({
+    savedObjectsClient.create.mockResolvedValue({
       attributes: {
         ...mockMaintenanceWindow,
         ...updatedAttributes,
         ...updatedMetadata,
       },
       id: 'test-id',
-    } as unknown as SavedObjectsUpdateResponse);
+    } as unknown as SavedObject);
 
     // Update without changing duration or rrule
     await update(mockContext, { id: 'test-id' });
     // Events keep the previous modified events, but adds on the new events
-    expect(savedObjectsClient.update).toHaveBeenLastCalledWith(
+    expect(savedObjectsClient.create).toHaveBeenLastCalledWith(
       MAINTENANCE_WINDOW_SAVED_OBJECT_TYPE,
-      'test-id',
       expect.objectContaining({
         events: [...modifiedEvents, expect.any(Object), expect.any(Object), expect.any(Object)],
       }),
-      { version: '123' }
+      { id: 'test-id' }
     );
 
     // Update with changing rrule
@@ -173,16 +171,15 @@ describe('MaintenanceWindowClient - update', () => {
       },
     });
     // All events are regenerated
-    expect(savedObjectsClient.update).toHaveBeenLastCalledWith(
+    expect(savedObjectsClient.create).toHaveBeenLastCalledWith(
       MAINTENANCE_WINDOW_SAVED_OBJECT_TYPE,
-      'test-id',
       expect.objectContaining({
         events: [
           { gte: '2023-03-26T00:00:00.000Z', lte: '2023-03-26T01:00:00.000Z' },
           { gte: '2023-04-01T23:00:00.000Z', lte: '2023-04-02T00:00:00.000Z' },
         ],
       }),
-      { version: '123' }
+      { id: 'test-id' }
     );
   });
 

x-pack/plugins/alerting/server/maintenance_window_client/methods/update.ts

@@ -51,14 +51,11 @@ async function updateWithOCC(
   const { id, title, enabled, duration, rRule } = params;
 
   try {
-    const {
-      attributes,
-      version,
-      id: fetchedId,
-    } = await savedObjectsClient.get<MaintenanceWindowSOAttributes>(
-      MAINTENANCE_WINDOW_SAVED_OBJECT_TYPE,
-      id
-    );
+    const { attributes, id: fetchedId } =
+      await savedObjectsClient.get<MaintenanceWindowSOAttributes>(
+        MAINTENANCE_WINDOW_SAVED_OBJECT_TYPE,
+        id
+      );
 
     if (moment.utc(attributes.expirationDate).isBefore(new Date())) {
       throw Boom.badRequest('Cannot edit archived maintenance windows');
@@ -77,29 +74,32 @@ async function updateWithOCC(
       events = mergeEvents({ oldEvents: attributes.events, newEvents: events });
     }
 
-    const result = await savedObjectsClient.update<MaintenanceWindowSOAttributes>(
+    await savedObjectsClient.delete(MAINTENANCE_WINDOW_SAVED_OBJECT_TYPE, fetchedId);
+
+    const updatedAttributes = {
+      ...attributes,
+      ...(title ? { title } : {}),
+      ...(rRule ? { rRule } : {}),
+      ...(typeof duration === 'number' ? { duration } : {}),
+      ...(typeof enabled === 'boolean' ? { enabled } : {}),
+      expirationDate,
+      events,
+      ...modificationMetadata,
+    };
+
+    // We are deleting and then creating rather than updating because SO.update
+    // performs a partial update on the rRule, we would need to null out all of the fields
+    // that are removed from a new rRule if that were the case.
+    const result = await savedObjectsClient.create<MaintenanceWindowSOAttributes>(
       MAINTENANCE_WINDOW_SAVED_OBJECT_TYPE,
-      fetchedId,
+      updatedAttributes,
       {
-        ...attributes,
-        title,
-        enabled: typeof enabled === 'boolean' ? enabled : attributes.enabled,
-        expirationDate,
-        duration,
-        rRule,
-        events,
-        ...modificationMetadata,
-      },
-      {
-        version,
+        id: fetchedId,
       }
     );
 
     return getMaintenanceWindowFromRaw({
-      attributes: {
-        ...attributes,
-        ...result.attributes,
-      },
+      attributes: result.attributes,
       id: result.id,
     });
   } catch (e) {

x-pack/test/alerting_api_integration/security_and_spaces/group3/tests/maintenance_window/update_maintenance_window.ts

@@ -5,6 +5,7 @@
  * 2.0.
  */
 
+import moment from 'moment';
 import expect from '@kbn/expect';
 import { UserAtSpaceScenarios } from '../../../scenarios';
 import { getUrlPrefix, ObjectRemover } from '../../../../common/lib';
@@ -88,5 +89,54 @@ export default function updateMaintenanceWindowTests({ getService }: FtrProvider
         });
       });
     }
+
+    it('should update RRule correctly when removing fields', async () => {
+      const { body: createdMaintenanceWindow } = await supertest
+        .post(`${getUrlPrefix('space1')}/internal/alerting/rules/maintenance_window`)
+        .set('kbn-xsrf', 'foo')
+        .send({
+          ...createParams,
+          r_rule: {
+            ...createParams.r_rule,
+            count: 1,
+            until: moment.utc().add(1, 'week').toISOString(),
+          },
+        })
+        .expect(200);
+
+      objectRemover.add(
+        'space1',
+        createdMaintenanceWindow.id,
+        'rules/maintenance_window',
+        'alerting',
+        true
+      );
+
+      const updatedRRule = {
+        ...createParams.r_rule,
+        count: 2,
+      };
+
+      await supertest
+        .post(
+          `${getUrlPrefix('space1')}/internal/alerting/rules/maintenance_window/${
+            createdMaintenanceWindow.id
+          }`
+        )
+        .set('kbn-xsrf', 'foo')
+        .send({
+          ...createParams,
+          r_rule: updatedRRule,
+        })
+        .expect(200);
+
+      const response = await supertest
+        .get(`${getUrlPrefix('space1')}/internal/alerting/rules/maintenance_window/_find`)
+        .set('kbn-xsrf', 'foo')
+        .send({});
+
+      expect(response.body.data[0].id).to.eql(createdMaintenanceWindow.id);
+      expect(response.body.data[0].r_rule).to.eql(updatedRRule);
+    });
   });
 }