S3_store_handler: works w/ S3 implementations w/o versioning

lib/routes/S3_store_handler.js

@@ -11,7 +11,7 @@ const {
   CreateBucketCommand,
   PutBucketVersioningCommand, PutBucketLifecycleConfigurationCommand,
   ListObjectVersionsCommand,
-  DeleteBucketCommand, GetBucketVersioningCommand, ListBucketsCommand
+  DeleteBucketCommand, GetBucketVersioningCommand, ListBucketsCommand, ListObjectsV2Command, DeleteObjectsCommand
 } = require('@aws-sdk/client-s3');
 const { NodeHttpHandler } = require('@smithy/node-http-handler');
 const { EMAIL_PATTERN, EMAIL_ERROR, PASSWORD_ERROR, PASSWORD_PATTERN } = require('../util/validations');
@@ -259,6 +259,7 @@ module.exports = function ({ endPoint = 'play.min.io', accessKey = 'Q3AM3UQ867SP
    * @param {string} params.username
    * @param {string} params.email
    * @param {string} params.password
+   * @param {Set} logNotes: strings for the notes field in the log
    * @returns {Promise<string>} name of bucket
    */
   router.createUser = async function createUser (params, logNotes) {
@@ -289,7 +290,7 @@ module.exports = function ({ endPoint = 'play.min.io', accessKey = 'Q3AM3UQ867SP
 
       return bucketName;
     } catch (err) {
-      logNotes.push('while creating user auth or metadata:');
+      logNotes.add('while creating user auth or metadata:');
       throw err;
     }
   };
@@ -309,114 +310,128 @@ module.exports = function ({ endPoint = 'play.min.io', accessKey = 'Q3AM3UQ867SP
 
     try {
       await s3client.send(new CreateBucketCommand({ Bucket: bucketName }));
+    } catch (err) {
+      if (err.name === 'BucketAlreadyOwnedByYou') {
+        throw new Error(`Username “${username}” is already taken`, { cause: err });
+      } else {
+        logNotes.add('while creating or configuring bucket:');
+        throw err;
+      }
+    }
 
+    try {
       await s3client.send(new PutBucketVersioningCommand({
         Bucket: bucketName,
         VersioningConfiguration: { Status: 'Enabled' }
       }));
+    } catch (err) {
+      errToMessages(err, logNotes.add('Couldn\'t set bucket to version blobs:'));
+    }
 
-      try {
-        await s3client.send(new PutBucketLifecycleConfigurationCommand({
-          Bucket: bucketName,
-          LifecycleConfiguration: {
-            Rules: [{
-              ID: '35days+2',
-              Filter: { Prefix: BLOB_PREFIX + '/' },
-              NoncurrentVersionExpiration: { NoncurrentDays: 35, NewerNoncurrentVersions: 2 },
-              AbortIncompleteMultipartUpload: { DaysAfterInitiation: 1 },
-              Status: 'Enabled'
-            }, {
-              ID: 'auth+10',
-              Filter: { Prefix: AUTH_PREFIX + '/' },
-              NoncurrentVersionExpiration: { NoncurrentDays: 1, NewerNoncurrentVersions: 10 },
-              AbortIncompleteMultipartUpload: { DaysAfterInitiation: 1 },
-              Status: 'Enabled'
-            }]
-          }
-        }));
-      } catch (err) {
-        errToMessages(err, logNotes.add('Couldn\'t set bucket to expire old blob versions:'));
-      }
+    try {
+      await s3client.send(new PutBucketLifecycleConfigurationCommand({
+        Bucket: bucketName,
+        LifecycleConfiguration: {
+          Rules: [{
+            ID: '35days+2',
+            Filter: { Prefix: BLOB_PREFIX + '/' },
+            NoncurrentVersionExpiration: { NoncurrentDays: 35, NewerNoncurrentVersions: 2 },
+            AbortIncompleteMultipartUpload: { DaysAfterInitiation: 1 },
+            Status: 'Enabled'
+          }, {
+            ID: 'auth+10',
+            Filter: { Prefix: AUTH_PREFIX + '/' },
+            NoncurrentVersionExpiration: { NoncurrentDays: 1, NewerNoncurrentVersions: 10 },
+            AbortIncompleteMultipartUpload: { DaysAfterInitiation: 1 },
+            Status: 'Enabled'
+          }]
+        }
+      }));
     } catch (err) {
-      if (err.name === 'BucketAlreadyOwnedByYou') {
-        throw new Error(`Username “${username}” is already taken`, { cause: err });
-      } else {
-        logNotes.push('while creating or configuring bucket:');
-        throw err;
-      }
+      errToMessages(err, logNotes.add('Couldn\'t set bucket to expire old blob versions:'));
     }
 
     return bucketName;
   };
 
   /**
    * Deletes all of user's documents & folders and the bucket. NOT REVERSIBLE.
-   * @param username
-   * @returns {Promise<number>} number of document & folder versions deleted
+   * @param {string} username
+   * @param {Set} logNotes: strings for the notes field in the log
+   * @returns {[Number, Number, Number]} number of successful deletions, number of errors, number of passes used
    */
   router.deleteUser = async function deleteUser (username, logNotes) {
-    let numObjectsDeleted = 0;
-    let numObjectsFailedToDelete = 0;
+    const bucketName = calcBucketName(username);
+    let numDeletions = 0;
+    let numErrors = 0;
+    let numPasses = 0;
 
-    const promise = new Promise((resolve, reject) => {
-      const bucketName = calcBucketName(username);
-      let pass = 0;
-
-      const deleteItems = async items => {
-        for (const item of items) {
-          try {
-            /* const { DeleteMarker } = */ await s3client.send(new DeleteObjectCommand({ Bucket: bucketName, Key: item.Key, VersionId: item.VersionId }));
-            ++numObjectsDeleted;
-          } catch (err) {
-            errToMessages(err, logNotes.add('while deleting:').add(bucketName).add(item.Key).add(item.VersionId));
-            ++numObjectsFailedToDelete;
+    try {
+      const versioningResult = await s3client.send(new GetBucketVersioningCommand({ Bucket: bucketName }));
+      if (['Enabled', 'Suspended'].includes(versioningResult?.Status)) {
+        let KeyMarker, VersionIdMarker;
+        do {
+          const { Versions, DeleteMarkers, IsTruncated, NextKeyMarker, NextVersionIdMarker } = await s3client.send(new ListObjectVersionsCommand({ Bucket: bucketName, ...(KeyMarker ? { KeyMarker } : null), ...(VersionIdMarker ? { VersionIdMarker } : null) }));
+
+          if (!(Versions?.length) && !(DeleteMarkers?.length)) { break; }
+
+          if (Versions?.length > 0) {
+            const { Deleted, Errors } = await s3client.send(new DeleteObjectsCommand({ Bucket: bucketName, Delete: { Objects: Versions } }));
+            numDeletions += Deleted?.length || 0;
+            numErrors += Errors?.length || 0;
           }
-        }
-      };
 
-      const pageObjectVersions = async (KeyMarker) => {
-        try {
-          const { Versions, DeleteMarkers, IsTruncated, NextKeyMarker } = await s3client.send(new ListObjectVersionsCommand({ Bucket: bucketName, ...(KeyMarker ? { KeyMarker } : null) }));
-
-          if (typeof Versions?.[Symbol.iterator] === 'function') {
-            await deleteItems(Versions);
-          }
-          if (typeof DeleteMarkers?.[Symbol.iterator] === 'function') {
-            await deleteItems(DeleteMarkers);
+          if (DeleteMarkers?.length > 0) {
+            const { Deleted, Errors } = await s3client.send(new DeleteObjectsCommand({
+              Bucket: bucketName,
+              Delete: { Objects: DeleteMarkers }
+            }));
+            numDeletions += Deleted?.length || 0;
+            numErrors += Errors?.length || 0;
           }
 
-          if (IsTruncated) {
-            pageObjectVersions(NextKeyMarker).catch(reject);
-          } else if (Versions?.length > 0 || DeleteMarkers?.length > 0) {
-            if (++pass <= 100) {
-              pageObjectVersions(undefined).catch(reject);
-            } else {
-              reject(new Error(`for user “${username}” couldn't delete all object versions after ${pass - 1} passes`));
-            }
-          } else {
-            await s3client.send(new DeleteBucketCommand({ Bucket: bucketName }));
-            resolve([numObjectsDeleted, numObjectsFailedToDelete]);
-          }
-        } catch (err) {
-          if (err.name === 'NoSuchBucket') {
-            resolve([numObjectsDeleted, numObjectsFailedToDelete]);
-          } else {
-            reject(err);
+          KeyMarker = IsTruncated ? NextKeyMarker : undefined;
+          VersionIdMarker = IsTruncated ? NextVersionIdMarker : undefined;
+
+          if (!IsTruncated && ++numPasses >= 100) {
+            throw new Error(`for user “${username}” couldn't delete all blobs after ${numPasses} passes`);
           }
-        }
-      };
+        } while (true);
+      } else {
+        let ContinuationToken;
+        do {
+          const { Contents, IsTruncated, NextContinuationToken } = await s3client.send(new ListObjectsV2Command({ Bucket: bucketName, ...(ContinuationToken ? { ContinuationToken } : null) }));
 
-      pageObjectVersions(undefined).catch(reject);
-    });
+          if (!Contents?.length) { break; }
+
+          const { Deleted, Errors } = await s3client.send(new DeleteObjectsCommand({ Bucket: bucketName, Delete: { Objects: Contents } }));
+          numDeletions += Deleted?.length || 0;
+          numErrors += Errors?.length || 0;
+
+          ContinuationToken = IsTruncated ? NextContinuationToken : undefined;
+          if (!IsTruncated && ++numPasses >= 100) {
+            throw new Error(`for user “${username}” couldn't delete all blobs after ${numPasses} passes`);
+          }
+        } while (true);
+      }
 
-    return promise;
+      await s3client.send(new DeleteBucketCommand({ Bucket: bucketName }));
+      return [numDeletions, numErrors, numPasses];
+    } catch (err) {
+      if (err.Code === 'NoSuchBucket') {
+        return [numDeletions, numErrors, numPasses];
+      } else {
+        throw err;
+      }
+    }
   };
 
   /**
    * Checks password against stored credentials
    * @param {String} username
    * @param {String} email
    * @param {String} password
+   * @param {Set} logNotes: strings for the notes field in the log
    * @returns {Promise<boolean>} true if correct
    * @throws if user doesn't exist, password doesn't match, or any error
    */

notes/S3-streaming-store.md

@@ -1,29 +1,44 @@
 # S3-compatible Streaming Store
 
-Streaming Stores can only be used with the modular server.
+Note: S3-compatible storage trades the strong consistency of a file system for higher performance and multi-datacenter capability, so this store only offers eventual consistency.
 
-You should be able to connect to any S3-compatible service that supports versioning. Tested services include:
+Streaming Stores like this can only be used with the modular server.
 
-Fully working implementations:
+## Compatible S3 Implementations
 
-* AWS S3
+Tested services include:
 
-Works, but web console can't be used with this:
+### AWS S3
 
-* min.io (both self-hosted and cloud)
+* Fully working
 
-Implementations with bugs that can't be worked around:
+### Garage
 
-* OpenIO [simultaneous delete]
+* doesn't implement versioning
+* doesn't implement If-Match for GET, which is not yet used but will be required to support Range requests
 
+### min.io (both self-hosted and cloud)
 
-Configure the store by passing to the constructor the endpoint (host name, and port if not 9000), access key (admin user name) and secret key (password). (If you don't pass any arguments, S3 will use the public account on `play.min.io`, where the documents & folders can be **read, altered and deleted** by anyone in the world. Also, the Min.IO browser can't list your documents or folders.) If you're using a AWS and a region other than `us-east-1`, include that as a fourth argument.  You can provide these however you like, but typically they are stored in these environment variables:
+* web console can't be used with this, and probably won't ever
+
+
+### OpenIO
+Disrecommended — bugs can't be worked around
+
+*  fails simultaneous delete test
+* doesn't implement DeleteObjectsCommand
+
+
+## Configuration
+
+Configure the store by passing to the constructor the endpoint (host name, and port if not 9000), access key (admin user name) and secret key (password). (If you don't pass any arguments, S3 will use the public account on `play.min.io`, where the documents & folders can be **read, altered and deleted** by anyone in the world! Also, the Min.IO browser can't list your documents or folders.) If you're using a AWS and a region other than `us-east-1`, include that as a fourth argument.  You can provide these however you like, but typically they are stored in these environment variables:
 
 * S3_ENDPOINT
 * S3_ACCESS_KEY
 * S3_SECRET_KEY
+* S3_REGION
 
-For AWS, you must also pass a fifth argument — a user name suffix so bucket names don't collide with other users. By default, this is a dash plus `conf.host_identity`, but you can set `conf.user_name_suffix` to override.
+For AWS, you must also pass a fifth argument — a user name suffix so bucket names don't collide with other users. By default, this is a hyphen plus `conf.host_identity`, but you can set `conf.user_name_suffix` to override.
 
 Creating an app server then resembles:
 
@@ -36,13 +51,13 @@ const s3handler = new S3Handler({
 const app = require('../../lib/appFactory')({account: s3handler, store: s3handler, ...});
 ```
 
-Https is used if the endpoint is not localhost.  If you must use http, you can include the scheme in the endpoint: `http://myhost.example.org`.
+HTTPS is used if the endpoint is not localhost.  If you must use http, you can include the scheme in the endpoint: `http://myhost.example.org`.
 
 This one access key is used to create a bucket for each user.
-The bucket name is the username.
-If other buckets are created at that endpoint, those bucket names will be unavailable as usernames.
+The bucket name is the username plus the suffix, if any.
+If other non-remoteStorage buckets are created at that endpoint, those bucket names will be unavailable as usernames.
 Buckets can be administered using the service's tools, such as a webapp console or command-line tools.
-The bucket **MAY** contain non-remoteStorage blobs outside these prefixes:
+The bucket **SHOULD NOT** contain non-RS blobs with these prefixes:
 
 * remoteStorageBlob/
 * remoteStorageAuth/

spec/account.spec.js

@@ -53,22 +53,49 @@ module.exports.shouldCreateDeleteAndReadAccounts = function () {
   describe('deleteUser', function () {
     before(function () {
       this.username2 = 'automated-test-' + Math.round(Math.random() * Number.MAX_SAFE_INTEGER);
+      this.username3 = 'automated-test-' + Math.round(Math.random() * Number.MAX_SAFE_INTEGER);
+      this.username4 = 'automated-test-' + Math.round(Math.random() * Number.MAX_SAFE_INTEGER);
     });
 
-    after(async function () {
+    it('deletes a user', async function () {
       this.timeout(10_000);
-      await this.store.deleteUser(this.username2, new Set());
+      const params = { username: this.username2, email: 'a@b.c', password: 'swordfish' };
+      await expect(this.store.createUser(params, new Set())).to.eventually.eql(this.username2 + this.USER_NAME_SUFFIX);
+
+      const logNotes = new Set();
+      const result = await this.store.deleteUser(this.username2, logNotes);
+      expect(result?.[0]).to.be.greaterThanOrEqual(2);
+      expect(result?.[1]).to.equal(0);
+      expect(result?.[2]).to.equal(1);
+      expect(logNotes.size).to.equal(0);
     });
 
-    it('deletes a user', async function () {
+    it('returns normally when user deleted twice at the same time', async function () {
+      this.timeout(10_000);
+      const params = { username: this.username3, email: 'a@b.c', password: 'swordfish' };
+      await expect(this.store.createUser(params, new Set())).to.eventually.eql(this.username3 + this.USER_NAME_SUFFIX);
+
+      const logNotes = new Set();
+      const results = await Promise.all(
+        [this.store.deleteUser(this.username3, logNotes), this.store.deleteUser(this.username3, logNotes)]);
+      expect(results[0]?.[0]).to.be.greaterThanOrEqual(0);
+      expect(results[0]?.[1]).to.equal(0);
+      expect(results[0]?.[2]).to.be.within(0, 1);
+      expect(results[1]?.[0]).to.be.greaterThanOrEqual(0);
+      expect(results[1]?.[1]).to.equal(0);
+      expect(results[1]?.[2]).to.be.within(0, 1);
+      expect(logNotes.size).to.equal(0);
+    });
+
+    it('returns normally when user doesn\'t exist', async function () {
       this.timeout(10_000);
 
-      const params = { username: this.username2, email: 'a@b.c', password: 'swordfish' };
       const logNotes = new Set();
-      await expect(this.store.createUser(params, logNotes)).to.eventually.eql(this.username2 + this.USER_NAME_SUFFIX);
-      const result = await this.store.deleteUser(this.username2, new Set());
-      await expect(result?.[0]).to.be.greaterThanOrEqual(2);
-      await expect(result?.[1]).to.equal(0);
+      const result = await this.store.deleteUser(this.username4, logNotes);
+      expect(result?.[0]).to.equal(0);
+      expect(result?.[1]).to.equal(0);
+      expect(result?.[2]).to.equal(0);
+      expect(logNotes.size).to.equal(0);
     });
   });
 

spec/store_handlers/S3_store_handler.spec.js

@@ -11,10 +11,10 @@ const { shouldCreateDeleteAndReadAccounts } = require('../account.spec');
 
 describe('S3 store handler', function () {
   before(function () {
-    configureLogger({ stdout: [], log_dir: './test-log', log_files: ['debug'] });
+    configureLogger({ stdout: ['notice'], log_dir: './test-log', log_files: ['debug'] });
     this.USER_NAME_SUFFIX = '-java.extraordinary.org';
     // If the environment variables aren't set, tests are run using a shared public account on play.min.io
-    this.handler = s3storeHandler({ endPoint: process.env.S3_ENDPOINT, accessKey: process.env.S3_ACCESS_KEY, secretKey: process.env.S3_SECRET_KEY, userNameSuffix: this.USER_NAME_SUFFIX });
+    this.handler = s3storeHandler({ endPoint: process.env.S3_ENDPOINT, accessKey: process.env.S3_ACCESS_KEY, secretKey: process.env.S3_SECRET_KEY, region: process.env.S3_REGION || 'us-east-1', userNameSuffix: this.USER_NAME_SUFFIX });
     this.store = this.handler;
   });