From 86b9e4b53a349a808755b155b5050743db9127c1 Mon Sep 17 00:00:00 2001
From: Simon Detheridge <s@sd.ai>
Date: Thu, 5 Dec 2019 13:51:27 +0000
Subject: [PATCH 1/9] Rename request -> s3Request to prevent overwriting main
 import

---
 services/filestore/app/coffee/S3PersistorManager.coffee | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/services/filestore/app/coffee/S3PersistorManager.coffee b/services/filestore/app/coffee/S3PersistorManager.coffee
index ca74bdb013..4c33fd84d2 100644
--- a/services/filestore/app/coffee/S3PersistorManager.coffee
+++ b/services/filestore/app/coffee/S3PersistorManager.coffee
@@ -95,9 +95,9 @@ module.exports =
 		}
 		if opts.start? and opts.end?
 			s3Params['Range'] = "bytes=#{opts.start}-#{opts.end}"
-		request = s3.getObject(s3Params)
+		s3Request = s3.getObject(s3Params)
 
-		request.on 'httpHeaders', (statusCode, headers, response, statusMessage) =>
+		s3Request.on 'httpHeaders', (statusCode, headers, response, statusMessage) =>
 			if statusCode in [403, 404]
 				# S3 returns a 403 instead of a 404 when the user doesn't have
 				# permission to list the bucket contents.
@@ -109,11 +109,11 @@ module.exports =
 			stream = response.httpResponse.createUnbufferedStream()
 			callback(null, stream)
 
-		request.on 'error', (err) =>
+		s3Request.on 'error', (err) =>
 			logger.err({ err: err, bucketName: bucketName, key: key }, "error getting file stream from s3")
 			callback(err)
 
-		request.send()
+		s3Request.send()
 
 	getFileSize: (bucketName, key, callback) ->
 		logger.log({ bucketName: bucketName, key: key }, "getting file size from S3")

From 96457597ac30dc23cc723072ae0fbb45c8701f81 Mon Sep 17 00:00:00 2001
From: Simon Detheridge <s@sd.ai>
Date: Thu, 5 Dec 2019 13:55:08 +0000
Subject: [PATCH 2/9] Add fake s3 server and initial config

---
 services/filestore/config/settings.defaults.coffee |  1 +
 services/filestore/docker-compose.ci.yml           | 11 +++++++++++
 services/filestore/docker-compose.yml              | 12 ++++++++++++
 3 files changed, 24 insertions(+)

diff --git a/services/filestore/config/settings.defaults.coffee b/services/filestore/config/settings.defaults.coffee
index 550cfd2694..29010e85ee 100644
--- a/services/filestore/config/settings.defaults.coffee
+++ b/services/filestore/config/settings.defaults.coffee
@@ -16,6 +16,7 @@ settings =
 			s3:
 				key: process.env['AWS_KEY']
 				secret: process.env['AWS_SECRET']
+				endpoint: process.env['AWS_S3_ENDPOINT']
 			stores:
 				user_files: process.env['AWS_S3_USER_FILES_BUCKET_NAME']
 				template_files: process.env['AWS_S3_TEMPLATE_FILES_BUCKET_NAME']
diff --git a/services/filestore/docker-compose.ci.yml b/services/filestore/docker-compose.ci.yml
index e5557b7e91..790109f70b 100644
--- a/services/filestore/docker-compose.ci.yml
+++ b/services/filestore/docker-compose.ci.yml
@@ -25,9 +25,16 @@ services:
       ENABLE_CONVERSIONS: "true"
       MOCHA_GREP: ${MOCHA_GREP}
       NODE_ENV: test
+      AWS_KEY: fake
+      AWS_SECRET: fake
+      AWS_S3_USER_FILES_BUCKET_NAME: fake_user_files
+      AWS_S3_TEMPLATE_FILES_BUCKET_NAME: fake_template_files
+      AWS_S3_PUBLIC_FILES_BUCKET_NAME: fake_public_files
+      AWS_S3_ENDPOINT: http://fakes3:9090
     depends_on:
       - mongo
       - redis
+      - fakes3
     user: node
     command: npm run test:acceptance:_run
 
@@ -46,3 +53,7 @@ services:
 
   mongo:
     image: mongo:3.4
+
+  fakes3:
+    image: adobe/s3mock
+    command: --initialBuckets=fake_user_files,fake_template_files,fake_public_files
diff --git a/services/filestore/docker-compose.yml b/services/filestore/docker-compose.yml
index b7fd2afc71..cd03a9daf4 100644
--- a/services/filestore/docker-compose.yml
+++ b/services/filestore/docker-compose.yml
@@ -31,10 +31,17 @@ services:
       ENABLE_CONVERSIONS: "true"
       LOG_LEVEL: ERROR
       NODE_ENV: test
+      AWS_KEY: fake
+      AWS_SECRET: fake
+      AWS_S3_USER_FILES_BUCKET_NAME: fake_user_files
+      AWS_S3_TEMPLATE_FILES_BUCKET_NAME: fake_template_files
+      AWS_S3_PUBLIC_FILES_BUCKET_NAME: fake_public_files
+      AWS_S3_ENDPOINT: http://fakes3:9090
     user: node
     depends_on:
       - mongo
       - redis
+      - fakes3
     command: npm run test:acceptance
 
 
@@ -53,4 +60,9 @@ services:
   mongo:
     image: mongo:3.4
 
+  fakes3:
+    image: adobe/s3mock
+    environment:
+      - initialBuckets=fake_user_files,fake_template_files,fake_public_files
+
 

From c01603b1e71afedeb2471c358e296c3677412a56 Mon Sep 17 00:00:00 2001
From: Simon Detheridge <s@sd.ai>
Date: Thu, 5 Dec 2019 14:25:25 +0000
Subject: [PATCH 3/9] Support custom S3 endpoints

---
 .../app/coffee/S3PersistorManager.coffee      | 60 ++++++++++++-------
 1 file changed, 37 insertions(+), 23 deletions(-)

diff --git a/services/filestore/app/coffee/S3PersistorManager.coffee b/services/filestore/app/coffee/S3PersistorManager.coffee
index 4c33fd84d2..5ee9e25865 100644
--- a/services/filestore/app/coffee/S3PersistorManager.coffee
+++ b/services/filestore/app/coffee/S3PersistorManager.coffee
@@ -16,10 +16,15 @@ LocalFileWriter = require("./LocalFileWriter")
 Errors = require("./Errors")
 _ = require("underscore")
 awsS3 = require "aws-sdk/clients/s3"
+URL = require('url')
 
 thirtySeconds = 30 * 1000
 
 buildDefaultOptions = (bucketName, method, key)->
+	if settings.filestore.s3.endpoint
+		endpoint = "#{settings.filestore.s3.endpoint}/#{bucketName}"
+	else
+		endpoint = "https://#{bucketName}.s3.amazonaws.com"
 	return {
 			aws:
 				key: settings.filestore.s3.key
@@ -27,32 +32,47 @@ buildDefaultOptions = (bucketName, method, key)->
 				bucket: bucketName
 			method: method
 			timeout: thirtySeconds
-			uri:"https://#{bucketName}.s3.amazonaws.com/#{key}"
+			uri:"#{endpoint}/#{key}"
 	}
 
-defaultS3Client = new awsS3({
-	credentials:
-		accessKeyId: settings.filestore.s3.key,
-		secretAccessKey: settings.filestore.s3.secret
-})
+getS3Options = (credentials) ->
+	options =
+		credentials:
+			accessKeyId: credentials.auth_key
+			secretAccessKey: credentials.auth_secret
+
+	if settings.filestore.s3.endpoint
+		options.endpoint = settings.filestore.s3.endpoint
+		options.sslEnabled = false
+
+	return options
+
+defaultS3Client = new awsS3(getS3Options({
+	auth_key: settings.filestore.s3.key,
+	auth_secret: settings.filestore.s3.secret
+}))
 
 getS3Client = (credentials) ->
 	if credentials?
-		return new awsS3({
-			credentials:
-				accessKeyId: credentials.auth_key
-				secretAccessKey: credentials.auth_secret
-		})
+		return new awsS3(getS3Options(credentials))
 	else
 		return defaultS3Client
 
+getKnoxClient = (bucketName) =>
+	options =
+		key: settings.filestore.s3.key
+		secret: settings.filestore.s3.secret
+		bucket: bucketName
+	if settings.filestore.s3.endpoint
+		endpoint = URL.parse(settings.filestore.s3.endpoint)
+		options.endpoint = endpoint.hostname
+		options.port = endpoint.port
+	return knox.createClient(options)
+
 module.exports =
 
 	sendFile: (bucketName, key, fsPath, callback)->
-		s3Client = knox.createClient
-			key: settings.filestore.s3.key
-			secret: settings.filestore.s3.secret
-			bucket: bucketName
+		s3Client = getKnoxClient(bucketName)
 		putEventEmiter = s3Client.putFile fsPath, key, (err, res)->
 			if err?
 				logger.err err:err,  bucketName:bucketName, key:key, fsPath:fsPath,"something went wrong uploading file to s3"
@@ -171,10 +191,7 @@ module.exports =
 			_callback = () ->
 
 		logger.log key: key, bucketName: bucketName, "deleting directory"
-		s3Client = knox.createClient
-			key: settings.filestore.s3.key
-			secret: settings.filestore.s3.secret
-			bucket: bucketName
+		s3Client = getKnoxClient(bucketName)
 		s3Client.list prefix:key, (err, data)->
 			if err?
 				logger.err err:err, bucketName:bucketName, key:key, "something went wrong listing prefix in aws"
@@ -200,10 +217,7 @@ module.exports =
 
 	directorySize:(bucketName, key, callback)->
 		logger.log bucketName:bucketName, key:key, "get project size in s3"
-		s3Client = knox.createClient
-			key: settings.filestore.s3.key
-			secret: settings.filestore.s3.secret
-			bucket: bucketName
+		s3Client = getKnoxClient(bucketName)
 		s3Client.list prefix:key, (err, data)->
 			if err?
 				logger.err err:err, bucketName:bucketName, key:key, "something went wrong listing prefix in aws"

From 1d1106bc679e344653c5feef96dca4a3124d6a38 Mon Sep 17 00:00:00 2001
From: Simon Detheridge <s@sd.ai>
Date: Thu, 5 Dec 2019 15:25:35 +0000
Subject: [PATCH 4/9] Add metric for s3 egress

---
 .../app/coffee/S3PersistorManager.coffee      |  5 +++
 services/filestore/docker-compose.ci.yml      |  1 +
 services/filestore/docker-compose.yml         |  1 +
 .../acceptance/coffee/SendingFileTest.coffee  | 33 ++++++++++++++-----
 4 files changed, 32 insertions(+), 8 deletions(-)

diff --git a/services/filestore/app/coffee/S3PersistorManager.coffee b/services/filestore/app/coffee/S3PersistorManager.coffee
index 5ee9e25865..14ab1bffe5 100644
--- a/services/filestore/app/coffee/S3PersistorManager.coffee
+++ b/services/filestore/app/coffee/S3PersistorManager.coffee
@@ -9,6 +9,7 @@ https.globalAgent.maxSockets = 300
 settings = require("settings-sharelatex")
 request = require("request")
 logger = require("logger-sharelatex")
+metrics = require("metrics-sharelatex")
 fs = require("fs")
 knox = require("knox")
 path = require("path")
@@ -73,7 +74,9 @@ module.exports =
 
 	sendFile: (bucketName, key, fsPath, callback)->
 		s3Client = getKnoxClient(bucketName)
+		uploaded = 0
 		putEventEmiter = s3Client.putFile fsPath, key, (err, res)->
+			metrics.count 's3.egress', uploaded
 			if err?
 				logger.err err:err,  bucketName:bucketName, key:key, fsPath:fsPath,"something went wrong uploading file to s3"
 				return callback(err)
@@ -88,6 +91,8 @@ module.exports =
 		putEventEmiter.on "error", (err)->
 			logger.err err:err,  bucketName:bucketName, key:key, fsPath:fsPath, "error emmited on put of file"
 			callback err
+		putEventEmiter.on "progress", (progress)->
+			uploaded = progress.written
 
 	sendStream: (bucketName, key, readStream, callback)->
 		logger.log bucketName:bucketName, key:key, "sending file to s3"
diff --git a/services/filestore/docker-compose.ci.yml b/services/filestore/docker-compose.ci.yml
index 790109f70b..d7b97ac011 100644
--- a/services/filestore/docker-compose.ci.yml
+++ b/services/filestore/docker-compose.ci.yml
@@ -25,6 +25,7 @@ services:
       ENABLE_CONVERSIONS: "true"
       MOCHA_GREP: ${MOCHA_GREP}
       NODE_ENV: test
+      USE_PROM_METRICS: "true"
       AWS_KEY: fake
       AWS_SECRET: fake
       AWS_S3_USER_FILES_BUCKET_NAME: fake_user_files
diff --git a/services/filestore/docker-compose.yml b/services/filestore/docker-compose.yml
index cd03a9daf4..c2c2ed565d 100644
--- a/services/filestore/docker-compose.yml
+++ b/services/filestore/docker-compose.yml
@@ -31,6 +31,7 @@ services:
       ENABLE_CONVERSIONS: "true"
       LOG_LEVEL: ERROR
       NODE_ENV: test
+      USE_PROM_METRICS: "true"
       AWS_KEY: fake
       AWS_SECRET: fake
       AWS_S3_USER_FILES_BUCKET_NAME: fake_user_files
diff --git a/services/filestore/test/acceptance/coffee/SendingFileTest.coffee b/services/filestore/test/acceptance/coffee/SendingFileTest.coffee
index b77afb866b..fb03697cb0 100644
--- a/services/filestore/test/acceptance/coffee/SendingFileTest.coffee
+++ b/services/filestore/test/acceptance/coffee/SendingFileTest.coffee
@@ -10,8 +10,14 @@ request = require("request")
 settings = require("settings-sharelatex")
 FilestoreApp = require "./FilestoreApp"
 
-describe "Filestore", ->
 
+getMetric = (filestoreUrl, metric, cb) ->
+	request.get "#{filestoreUrl}/metrics", (err, res) ->
+		expect(res.statusCode).to.equal 200
+		metricRegex = new RegExp("^#{metric}{[^}]+} ([0-9]+)$", "m")
+		cb(parseInt(metricRegex.exec(res.body)?[1] || '0'))
+
+describe "Filestore", ->
 	before (done)->
 		@localFileReadPath = "/tmp/filestore_acceptence_tests_file_read.txt"
 		@localFileWritePath = "/tmp/filestore_acceptence_tests_file_write.txt"
@@ -27,10 +33,10 @@ describe "Filestore", ->
 
 	beforeEach (done)->
 		FilestoreApp.ensureRunning =>
-			fs.unlink @localFileWritePath, ->
-				done()
-
-
+			fs.unlink @localFileWritePath, =>
+				getMetric @filestoreUrl, 's3_egress', (metric) =>
+					@previousEgress = metric
+					done()
 
 	it "should send a 200 for status endpoint", (done)->
 		request "#{@filestoreUrl}/status", (err, response, body)->
@@ -59,6 +65,11 @@ describe "Filestore", ->
 				response.statusCode.should.equal 404
 				done()
 
+		it 'should record an egress metric for the upload', (done) ->
+			getMetric @filestoreUrl, 's3_egress', (metric) =>
+				expect(metric - @previousEgress).to.equal @constantFileContent.length
+				done()
+
 		it "should return the file size on a HEAD request", (done) ->
 			expectedLength = Buffer.byteLength(@constantFileContent)
 			request.head @fileUrl, (err, res) =>
@@ -128,11 +139,17 @@ describe "Filestore", ->
 			@file_id = Math.random()
 			@fileUrl = "#{@filestoreUrl}/project/acceptence_tests/file/#{@file_id}"
 			@localFileReadPath = __dirname + '/../../fixtures/test.pdf'
+			fs.stat @localFileReadPath, (err, stat) =>
+				@localFileSize = stat.size
+				writeStream = request.post(@fileUrl)
 
-			writeStream = request.post(@fileUrl)
+				writeStream.on "end", done
+				fs.createReadStream(@localFileReadPath).pipe writeStream
 
-			writeStream.on "end", done
-			fs.createReadStream(@localFileReadPath).pipe writeStream
+		it 'should record an egress metric for the upload', (done) ->
+			getMetric @filestoreUrl, 's3_egress', (metric) =>
+				expect(metric - @previousEgress).to.equal @localFileSize
+				done()
 
 		it "should be able get the file back", (done)->
 			@timeout(1000 * 10)

From 48aa14159163217deb75181e4d7435f325e163e5 Mon Sep 17 00:00:00 2001
From: Simon Detheridge <s@sd.ai>
Date: Thu, 5 Dec 2019 15:46:14 +0000
Subject: [PATCH 5/9] Add metric for s3 ingress

---
 .../app/coffee/S3PersistorManager.coffee      |  3 ++
 .../acceptance/coffee/SendingFileTest.coffee  | 38 ++++++++++++++++---
 .../coffee/S3PersistorManagerTests.coffee     |  1 +
 3 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/services/filestore/app/coffee/S3PersistorManager.coffee b/services/filestore/app/coffee/S3PersistorManager.coffee
index 14ab1bffe5..b575001fb3 100644
--- a/services/filestore/app/coffee/S3PersistorManager.coffee
+++ b/services/filestore/app/coffee/S3PersistorManager.coffee
@@ -132,6 +132,9 @@ module.exports =
 				logger.log({bucketName: bucketName, key: key }, "error getting file from s3: #{statusCode}")
 				return callback(new Error("Got non-200 response from S3: #{statusCode} #{statusMessage}"), null)
 			stream = response.httpResponse.createUnbufferedStream()
+			stream.on 'data', (data) ->
+				metrics.count 's3.ingress', data.byteLength
+
 			callback(null, stream)
 
 		s3Request.on 'error', (err) =>
diff --git a/services/filestore/test/acceptance/coffee/SendingFileTest.coffee b/services/filestore/test/acceptance/coffee/SendingFileTest.coffee
index fb03697cb0..cd1fa167f5 100644
--- a/services/filestore/test/acceptance/coffee/SendingFileTest.coffee
+++ b/services/filestore/test/acceptance/coffee/SendingFileTest.coffee
@@ -9,6 +9,7 @@ fs = require("fs")
 request = require("request")
 settings = require("settings-sharelatex")
 FilestoreApp = require "./FilestoreApp"
+async = require('async')
 
 
 getMetric = (filestoreUrl, metric, cb) ->
@@ -33,10 +34,19 @@ describe "Filestore", ->
 
 	beforeEach (done)->
 		FilestoreApp.ensureRunning =>
-			fs.unlink @localFileWritePath, =>
-				getMetric @filestoreUrl, 's3_egress', (metric) =>
-					@previousEgress = metric
-					done()
+			async.parallel [
+				(cb) =>
+					fs.unlink @localFileWritePath, () ->
+						cb()
+				(cb) =>
+					getMetric @filestoreUrl, 's3_egress', (metric) =>
+						@previousEgress = metric
+						cb()
+				(cb) =>
+					getMetric @filestoreUrl, 's3_ingress', (metric) =>
+						@previousIngress = metric
+						cb()
+			], done
 
 	it "should send a 200 for status endpoint", (done)->
 		request "#{@filestoreUrl}/status", (err, response, body)->
@@ -83,7 +93,14 @@ describe "Filestore", ->
 				body.should.equal @constantFileContent
 				done()
 
-		it "should be able to get back the first 8 bytes of the file", (done) ->
+		it "should record an ingress metric when downloading the file", (done)->
+			@timeout(1000 * 10)
+			request.get @fileUrl, () =>
+				getMetric @filestoreUrl, 's3_ingress', (metric) =>
+					expect(metric - @previousIngress).to.equal @constantFileContent.length
+					done()
+
+		it "should be able to get back the first 9 bytes of the file", (done) ->
 			@timeout(1000 * 10)
 			options =
 				uri: @fileUrl
@@ -93,6 +110,17 @@ describe "Filestore", ->
 				body.should.equal 'hello wor'
 				done()
 
+		it "should record an ingress metric for a partial download", (done)->
+			@timeout(1000 * 10)
+			options =
+				uri: @fileUrl
+				headers:
+					'Range': 'bytes=0-8'
+			request.get options, ()=>
+				getMetric @filestoreUrl, 's3_ingress', (metric) =>
+					expect(metric - @previousIngress).to.equal 9
+					done()
+
 		it "should be able to get back bytes 4 through 10 of the file", (done) ->
 			@timeout(1000 * 10)
 			options =
diff --git a/services/filestore/test/unit/coffee/S3PersistorManagerTests.coffee b/services/filestore/test/unit/coffee/S3PersistorManagerTests.coffee
index 8a1b2e4d49..a5ab5c2932 100644
--- a/services/filestore/test/unit/coffee/S3PersistorManagerTests.coffee
+++ b/services/filestore/test/unit/coffee/S3PersistorManagerTests.coffee
@@ -62,6 +62,7 @@ describe "S3PersistorManagerTests", ->
 		describe "success", ->
 			beforeEach () ->
 				@expectedStream = { expectedStream: true }
+				@expectedStream.on = sinon.stub()
 				@s3Request.send.callsFake () =>
 					@s3EventHandlers.httpHeaders(200, {}, @s3Response, "OK")
 				@s3Response.httpResponse.createUnbufferedStream.returns(@expectedStream)

From 6f326d56505f53106908b607d139729176fa2c93 Mon Sep 17 00:00:00 2001
From: Simon Detheridge <s@sd.ai>
Date: Mon, 9 Dec 2019 17:41:20 +0000
Subject: [PATCH 6/9] Use SSL setting based on url protocol

---
 services/filestore/app/coffee/S3PersistorManager.coffee | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/services/filestore/app/coffee/S3PersistorManager.coffee b/services/filestore/app/coffee/S3PersistorManager.coffee
index b575001fb3..89522a4643 100644
--- a/services/filestore/app/coffee/S3PersistorManager.coffee
+++ b/services/filestore/app/coffee/S3PersistorManager.coffee
@@ -43,8 +43,9 @@ getS3Options = (credentials) ->
 			secretAccessKey: credentials.auth_secret
 
 	if settings.filestore.s3.endpoint
+		endpoint = URL.parse(settings.filestore.s3.endpoint)
 		options.endpoint = settings.filestore.s3.endpoint
-		options.sslEnabled = false
+		options.sslEnabled = endpoint.protocol == 'https'
 
 	return options
 

From cf684dcd9865bd6f9e49b6b8ceb7c2c2f6aa8d0b Mon Sep 17 00:00:00 2001
From: Simon Detheridge <s@sd.ai>
Date: Tue, 10 Dec 2019 11:38:19 +0000
Subject: [PATCH 7/9] Fix fakes3 configuration in ci yml

---
 services/filestore/docker-compose.ci.yml | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/services/filestore/docker-compose.ci.yml b/services/filestore/docker-compose.ci.yml
index d7b97ac011..c1929c03ca 100644
--- a/services/filestore/docker-compose.ci.yml
+++ b/services/filestore/docker-compose.ci.yml
@@ -57,4 +57,6 @@ services:
 
   fakes3:
     image: adobe/s3mock
-    command: --initialBuckets=fake_user_files,fake_template_files,fake_public_files
+    environment:
+      - initialBuckets=fake_user_files,fake_template_files,fake_public_files
+

From 237c4113cd0b4158b8445d19d4d0d1b4a9fa07af Mon Sep 17 00:00:00 2001
From: Simon Detheridge <s@sd.ai>
Date: Tue, 10 Dec 2019 16:11:44 +0000
Subject: [PATCH 8/9] Ensure fakes3 is healthy before running tests

---
 services/filestore/docker-compose.ci.yml | 13 +++++++++----
 services/filestore/docker-compose.yml    | 13 +++++++++----
 2 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/services/filestore/docker-compose.ci.yml b/services/filestore/docker-compose.ci.yml
index c1929c03ca..42c6ae37b5 100644
--- a/services/filestore/docker-compose.ci.yml
+++ b/services/filestore/docker-compose.ci.yml
@@ -3,7 +3,7 @@
 # https://github.com/sharelatex/sharelatex-dev-environment
 # Version: 1.1.24
 
-version: "2"
+version: "2.1"
 
 services:
   test_unit:
@@ -33,9 +33,12 @@ services:
       AWS_S3_PUBLIC_FILES_BUCKET_NAME: fake_public_files
       AWS_S3_ENDPOINT: http://fakes3:9090
     depends_on:
-      - mongo
-      - redis
-      - fakes3
+      mongo:
+        condition: service_healthy
+      redis:
+        condition: service_healthy
+      fakes3:
+        condition: service_healthy
     user: node
     command: npm run test:acceptance:_run
 
@@ -59,4 +62,6 @@ services:
     image: adobe/s3mock
     environment:
       - initialBuckets=fake_user_files,fake_template_files,fake_public_files
+    healthcheck:
+      test: ["CMD", "curl", "-f", "http://localhost:9090"]
 
diff --git a/services/filestore/docker-compose.yml b/services/filestore/docker-compose.yml
index c2c2ed565d..65f18f4d78 100644
--- a/services/filestore/docker-compose.yml
+++ b/services/filestore/docker-compose.yml
@@ -3,7 +3,7 @@
 # https://github.com/sharelatex/sharelatex-dev-environment
 # Version: 1.1.24
 
-version: "2"
+version: "2.1"
 
 services:
   test_unit:
@@ -40,9 +40,12 @@ services:
       AWS_S3_ENDPOINT: http://fakes3:9090
     user: node
     depends_on:
-      - mongo
-      - redis
-      - fakes3
+      mongo:
+        condition: service_healthy
+      redis:
+        condition: service_healthy
+      fakes3:
+        condition: service_healthy
     command: npm run test:acceptance
 
 
@@ -65,5 +68,7 @@ services:
     image: adobe/s3mock
     environment:
       - initialBuckets=fake_user_files,fake_template_files,fake_public_files
+    healthcheck:
+      test: ["CMD", "curl", "-f", "http://localhost:9090"]
 
 

From 56b38af678c8061eaddc88a5b211c4a5c57537b3 Mon Sep 17 00:00:00 2001
From: Simon Detheridge <s@sd.ai>
Date: Tue, 10 Dec 2019 17:43:34 +0000
Subject: [PATCH 9/9] Explicitly wait for S3 in acceptance tests

---
 .../acceptance/coffee/FilestoreApp.coffee     | 23 ++++++++++++++++++-
 .../acceptance/coffee/SendingFileTest.coffee  |  4 +++-
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/services/filestore/test/acceptance/coffee/FilestoreApp.coffee b/services/filestore/test/acceptance/coffee/FilestoreApp.coffee
index 818e90ec6f..1b4cc38834 100644
--- a/services/filestore/test/acceptance/coffee/FilestoreApp.coffee
+++ b/services/filestore/test/acceptance/coffee/FilestoreApp.coffee
@@ -2,6 +2,9 @@ app = require('../../../app')
 require("logger-sharelatex").logger.level("info")
 logger = require("logger-sharelatex")
 Settings = require("settings-sharelatex")
+request = require('request')
+
+S3_TRIES = 30
 
 module.exports =
 	running: false
@@ -21,4 +24,22 @@ module.exports =
 				logger.log("filestore running in dev mode")
 
 				for callback in @callbacks
-					callback()
\ No newline at end of file
+					callback()
+
+	waitForS3: (callback, tries) ->
+		return callback() unless Settings.filestore.s3?.endpoint
+		tries = 1 unless tries
+
+		request.get "#{Settings.filestore.s3.endpoint}/", (err, response) =>
+			console.log(err, response?.statusCode, tries)
+			if !err && [200, 404].includes(response?.statusCode)
+				return callback()
+
+			if tries == S3_TRIES
+				return callback('timed out waiting for S3')
+
+			setTimeout(
+				() =>
+					@waitForS3 callback, tries + 1
+				1000
+			)
diff --git a/services/filestore/test/acceptance/coffee/SendingFileTest.coffee b/services/filestore/test/acceptance/coffee/SendingFileTest.coffee
index cd1fa167f5..4e9443fd88 100644
--- a/services/filestore/test/acceptance/coffee/SendingFileTest.coffee
+++ b/services/filestore/test/acceptance/coffee/SendingFileTest.coffee
@@ -29,8 +29,10 @@ describe "Filestore", ->
 			"there are 3 lines in all"
 		].join("\n")
 
-		fs.writeFile(@localFileReadPath, @constantFileContent, done)
 		@filestoreUrl = "http://localhost:#{settings.internal.filestore.port}"
+		fs.writeFile @localFileReadPath, @constantFileContent, (err) ->
+			return done(err) if err
+			FilestoreApp.waitForS3(done)
 
 	beforeEach (done)->
 		FilestoreApp.ensureRunning =>