CLDSRV-1067: UpdateNotificationDestination API

lib/api/api.js

@@ -54,6 +54,7 @@ const multipartDelete = require('./multipartDelete');
 const notificationDestinationCreate = require('./notificationDestinationCreate');
 const notificationDestinationGet = require('./notificationDestinationGet');
 const notificationDestinationList = require('./notificationDestinationList');
+const notificationDestinationUpdate = require('./notificationDestinationUpdate');
 const objectCopy = require('./objectCopy');
 const { objectDelete } = require('./objectDelete');
 const objectDeleteTagging = require('./objectDeleteTagging');
@@ -647,6 +648,7 @@ const api = {
     notificationDestinationCreate,
     notificationDestinationGet,
     notificationDestinationList,
+    notificationDestinationUpdate,
     objectDelete,
     objectDeleteTagging,
     objectGet,

lib/api/notificationDestinationUpdate.js

@@ -0,0 +1,101 @@
+const { errorInstances } = require('@scality/arsenal');
+
+const recordStore = require('./apiUtils/notificationDestination/recordStore');
+const parseDestination = require('./apiUtils/notificationDestination/parseDestination');
+const { nameError, recordModelVersion, requesterAccountId } = require('./apiUtils/notificationDestination/request');
+const parseXML = require('../utilities/parseXML');
+const monitoring = require('../utilities/monitoringHandler');
+
+const METRICS_ACTION = 'updateNotificationDestination';
+
+/**
+ * notificationDestinationUpdate - replace a bucket notification destination
+ * of the account of the requester with the body, keeping its name, account,
+ * ARN and creation time
+ * @param {AuthInfo} authInfo - Instance of AuthInfo class with requester's info
+ * @param {object} request - http request object
+ * @param {object} log - Werelogs logger
+ * @param {function} callback - callback(err)
+ * @return {undefined}
+ */
+function notificationDestinationUpdate(authInfo, request, log, callback) {
+    log.debug('processing request', { method: 'notificationDestinationUpdate' });
+    const name = request.query.name;
+
+    const invalidName = nameError(name);
+    if (invalidName) {
+        log.debug('invalid destination name', { method: 'notificationDestinationUpdate' });
+        monitoring.promMetrics('POST', request.bucketName, 400, METRICS_ACTION);
+        return process.nextTick(callback, invalidName);
+    }
+    const accountId = requesterAccountId(authInfo);
+    const logFields = { method: 'notificationDestinationUpdate', destinationName: name, accountId };
+
+    return parseXML(request.post, log, (parseErr, parsedXml) => {
+        if (parseErr) {
+            monitoring.promMetrics('POST', request.bucketName, parseErr.code, METRICS_ACTION);
+            return callback(parseErr);
+        }
+        const fields = parseDestination(parsedXml);
+        if (fields.error) {
+            log.debug('invalid destination', { ...logFields, error: fields.error.description });
+            monitoring.promMetrics('POST', request.bucketName, fields.error.code, METRICS_ACTION);
+            return callback(fields.error);
+        }
+        // Read, then write. bucketd has no conditional put. Thus, when an
+        // update reads before a delete and writes after it, the update
+        // answers 200 and restores the record.
+        return recordStore.getDestination(accountId, name, log, (getErr, stored) => {
+            if (getErr) {
+                if (getErr.is?.NoSuchDestination) {
+                    log.debug('destination not found', logFields);
+                } else {
+                    log.error('could not read the destination record', { ...logFields, error: getErr });
+                }
+                monitoring.promMetrics('POST', request.bucketName, getErr.code, METRICS_ACTION);
+                return callback(getErr);
+            }
+            // A record of a newer version may hold fields that this server
+            // would drop. Leave it to a server that knows that version.
+            if (stored.modelVersion > recordModelVersion) {
+                log.debug('destination record of a newer version', {
+                    ...logFields,
+                    modelVersion: stored.modelVersion,
+                });
+                const newerErr = errorInstances.InvalidDestination.customizeDescription(
+                    'This destination was written by a newer server (record version ' +
+                        `${stored.modelVersion}, this server knows ${recordModelVersion}). ` +
+                        'Update it from a server running the newer version.',
+                );
+                monitoring.promMetrics('POST', request.bucketName, newerErr.code, METRICS_ACTION);
+                return callback(newerErr);
+            }
+            // Replace the whole record, in the version that this server
+            // writes. Keep only the identity fields and the creation time
+            // from the stored record.
+            const record = {
+                modelVersion: recordModelVersion,
+                Name: stored.Name,
+                AccountId: stored.AccountId,
+                Arn: stored.Arn,
+                Type: fields.Type,
+                Connection: fields.Connection,
+                Auth: fields.Auth,
+                CreatedAt: stored.CreatedAt,
+                UpdatedAt: new Date().toJSON(),
+            };
+            return recordStore.putDestination(accountId, name, record, log, putErr => {
+                if (putErr) {
+                    log.error('could not write the destination record', { ...logFields, error: putErr });
+                    monitoring.promMetrics('POST', request.bucketName, putErr.code, METRICS_ACTION);
+                    return callback(putErr);
+                }
+                log.debug('destination updated', logFields);
+                monitoring.promMetrics('POST', request.bucketName, '200', METRICS_ACTION);
+                return callback(null);
+            });
+        });
+    });
+}
+
+module.exports = notificationDestinationUpdate;

tests/functional/raw-node/test/routes/notificationDestination.js

@@ -229,6 +229,7 @@ function destinationRequest(method, queryObj, requestBody, credentials = authCre
 
 const createDestination = (name, body, credentials) => destinationRequest('PUT', { name }, body, credentials);
 const getDestination = (name, credentials) => destinationRequest('GET', { name }, undefined, credentials);
+const updateDestination = (name, body, credentials) => destinationRequest('POST', { name }, body, credentials);
 
 const escapeRegExp = text => text.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
 
@@ -558,6 +559,68 @@ describeOn('ListNotificationDestinations', () => {
     });
 });
 
+describeOn('UpdateNotificationDestination', () => {
+    const newBody = saslSslBody
+        .replace(
+            /<Brokers>.*<\/Brokers>/,
+            '<Brokers><Broker>broker7.example.com:9094</Broker><Broker>broker8.example.com:9094</Broker></Brokers>',
+        )
+        .replace('bucket-events', 'updated-events');
+
+    it('should replace the destination, keep CreatedAt, and answer 200 with no body', async () => {
+        const name = uniqueName('update');
+        assert.ifError((await createDestination(name, saslSslBody)).err);
+        const created = await getDestination(name);
+        assert.ifError(created.err);
+        const [createdAt] = elementValues(created.res.body, 'CreatedAt');
+        // UpdatedAt is to the millisecond. Let a few milliseconds pass, so
+        // that UpdatedAt changes.
+        await new Promise(resolve => setTimeout(resolve, 10));
+
+        const { err, res } = await updateDestination(name, newBody);
+        assert.ifError(err);
+        assert.strictEqual(res.statusCode, 200);
+        assert.strictEqual(res.body, '');
+
+        const got = await getDestination(name);
+        assert.ifError(got.err);
+        assert.deepStrictEqual(elementValues(got.res.body, 'Broker'), [
+            'broker7.example.com:9094',
+            'broker8.example.com:9094',
+        ]);
+        assert.deepStrictEqual(elementValues(got.res.body, 'Topic'), ['updated-events']);
+        assert.deepStrictEqual(elementValues(got.res.body, 'CreatedAt'), [createdAt]);
+        const [updatedAt] = elementValues(got.res.body, 'UpdatedAt');
+        assert.match(updatedAt, /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z$/);
+        assert(Date.parse(updatedAt) > Date.parse(createdAt), `${updatedAt} is not after ${createdAt}`);
+    });
+
+    it('should answer NoSuchDestination to an unknown name', async () => {
+        assertError(await updateDestination(uniqueName('unknown'), newBody), 'NoSuchDestination', 404);
+    });
+
+    it('should answer InvalidDestination to an invalid body, and keep the destination', async () => {
+        const name = uniqueName('update-invalid');
+        assert.ifError((await createDestination(name, saslSslBody)).err);
+        const original = await getDestination(name);
+        assert.ifError(original.err);
+        const body = newBody.replace('broker7.example.com:9094', 'broker7.example.com:0');
+        assert.notStrictEqual(body, newBody);
+        assertError(await updateDestination(name, body), 'InvalidDestination', 400);
+        const got = await getDestination(name);
+        assert.ifError(got.err);
+        assert.strictEqual(got.res.body, original.res.body);
+    });
+
+    it('should answer InvalidDestination to a name outside the grammar', async () => {
+        assertError(await updateDestination('bad/name', newBody), 'InvalidDestination', 400);
+    });
+
+    it('should answer AccessDenied to an anonymous request', async () => {
+        assertError(await updateDestination(uniqueName('anonymous'), newBody, null), 'AccessDenied', 403);
+    });
+});
+
 describeIam('CreateNotificationDestination by an IAM user', () => {
     const iamClient = makeIamClient();
     const userName = uniqueName('destination-no-policy');
@@ -662,3 +725,69 @@ describeIam('ListNotificationDestinations by an IAM user', () => {
         assertError(await listAs('one'), 'AccessDenied', 403);
     });
 });
+
+describeIam('UpdateNotificationDestination by an IAM user', () => {
+    const iamClient = makeIamClient();
+    const userName = uniqueName('destination-update-no-policy');
+    let name;
+    let credentials;
+
+    beforeEach(async () => {
+        name = testName('update-denied');
+        assert.ifError((await createDestination(name, saslSslBody)).err);
+        credentials = await createIamCaller(iamClient, userName);
+    });
+
+    afterEach(async () => {
+        await removeIamCaller(iamClient, userName);
+    });
+
+    it('should answer AccessDenied to a user with no policy, and keep the destination', async () => {
+        const body = saslSslBody.replace('bucket-events', 'denied-events');
+        assertError(await updateDestination(name, body, credentials), 'AccessDenied', 403);
+        const got = await getDestination(name);
+        assert.ifError(got.err);
+        assert.deepStrictEqual(elementValues(got.res.body, 'Topic'), ['bucket-events']);
+    });
+});
+
+describeIam('UpdateNotificationDestination by an IAM user allowed on one destination', () => {
+    const iamClient = makeIamClient();
+    const userName = uniqueName('destination-update-one');
+    let allowedName;
+    let otherName;
+    let credentials;
+
+    beforeEach(async () => {
+        allowedName = testName('update-allowed');
+        otherName = testName('update-other');
+        const allowedArn = await createDestinationArn(allowedName);
+        assert.ifError((await createDestination(otherName, saslSslBody)).err);
+        credentials = await createIamCaller(iamClient, userName, {
+            action: 'scality:UpdateBucketNotificationDestination',
+            resource: allowedArn,
+        });
+    });
+
+    afterEach(async () => {
+        await removeIamCaller(iamClient, userName);
+    });
+
+    const newBody = saslSslBody.replace('bucket-events', 'allowed-events');
+
+    it('should update the destination that the policy names', async () => {
+        const { err, res } = await updateDestination(allowedName, newBody, credentials);
+        assert.ifError(err);
+        assert.strictEqual(res.statusCode, 200);
+        const got = await getDestination(allowedName);
+        assert.ifError(got.err);
+        assert.deepStrictEqual(elementValues(got.res.body, 'Topic'), ['allowed-events']);
+    });
+
+    it('should answer AccessDenied to another destination, and keep it', async () => {
+        assertError(await updateDestination(otherName, newBody, credentials), 'AccessDenied', 403);
+        const got = await getDestination(otherName);
+        assert.ifError(got.err);
+        assert.deepStrictEqual(elementValues(got.res.body, 'Topic'), ['bucket-events']);
+    });
+});