diff --git a/services/filestore/app/coffee/S3PersistorManager.coffee b/services/filestore/app/coffee/S3PersistorManager.coffee index ca74bdb013..89522a4643 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") @@ -16,10 +17,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,33 +33,51 @@ 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 + endpoint = URL.parse(settings.filestore.s3.endpoint) + options.endpoint = settings.filestore.s3.endpoint + options.sslEnabled = endpoint.protocol == 'https' + + 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) + 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) @@ -68,6 +92,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" @@ -95,9 +121,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. @@ -107,13 +133,16 @@ 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) - 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") @@ -171,10 +200,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 +226,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" 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..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: @@ -25,9 +25,20 @@ 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 + 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 + mongo: + condition: service_healthy + redis: + condition: service_healthy + fakes3: + condition: service_healthy user: node command: npm run test:acceptance:_run @@ -46,3 +57,11 @@ services: mongo: image: mongo:3.4 + + fakes3: + 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 b7fd2afc71..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: @@ -31,10 +31,21 @@ 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 + 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 + mongo: + condition: service_healthy + redis: + condition: service_healthy + fakes3: + condition: service_healthy command: npm run test:acceptance @@ -53,4 +64,11 @@ services: mongo: image: mongo:3.4 + fakes3: + 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/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 b77afb866b..4e9443fd88 100644 --- a/services/filestore/test/acceptance/coffee/SendingFileTest.coffee +++ b/services/filestore/test/acceptance/coffee/SendingFileTest.coffee @@ -9,9 +9,16 @@ fs = require("fs") request = require("request") settings = require("settings-sharelatex") FilestoreApp = require "./FilestoreApp" +async = require('async') + + +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" @@ -22,15 +29,26 @@ 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 => - fs.unlink @localFileWritePath, -> - 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)-> @@ -59,6 +77,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) => @@ -72,7 +95,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 @@ -82,6 +112,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 = @@ -128,11 +169,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) 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)