From 3614f217e6be0ec606156750f188adb7677f92bb Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Mon, 7 Jan 2019 15:54:24 +0000 Subject: [PATCH] add enableConversions flag to disable conversions which we can't do on k8 --- .../app/coffee/ImageOptimiser.coffee | 4 +++ services/filestore/app/coffee/SafeExec.coffee | 5 +++ .../filestore/config/settings.defaults.coffee | 1 + .../unit/coffee/ImageOptimiserTests.coffee | 34 +++++++++++++------ .../{SafeExec.coffee => SafeExecTests.coffee} | 10 +++++- 5 files changed, 43 insertions(+), 11 deletions(-) rename services/filestore/test/unit/coffee/{SafeExec.coffee => SafeExecTests.coffee} (81%) diff --git a/services/filestore/app/coffee/ImageOptimiser.coffee b/services/filestore/app/coffee/ImageOptimiser.coffee index 4888e00224..4c4a353f21 100644 --- a/services/filestore/app/coffee/ImageOptimiser.coffee +++ b/services/filestore/app/coffee/ImageOptimiser.coffee @@ -1,5 +1,6 @@ exec = require('child_process').exec logger = require("logger-sharelatex") +Settings = require "settings-sharelatex" module.exports = @@ -10,6 +11,9 @@ module.exports = opts = timeout: 30 * 1000 killSignal: "SIGKILL" + if !Settings.enableConversions + error = new Error("Image conversions are disabled") + return callback(error) exec args, opts,(err, stdout, stderr)-> if err? and err.signal == 'SIGKILL' logger.warn {err: err, stderr: stderr, localPath: localPath}, "optimiser timeout reached" diff --git a/services/filestore/app/coffee/SafeExec.coffee b/services/filestore/app/coffee/SafeExec.coffee index aa8121a360..3559d35c95 100644 --- a/services/filestore/app/coffee/SafeExec.coffee +++ b/services/filestore/app/coffee/SafeExec.coffee @@ -1,6 +1,7 @@ _ = require("underscore") logger = require("logger-sharelatex") child_process = require('child_process') +Settings = require "settings-sharelatex" # execute a command in the same way as 'exec' but with a timeout that # kills all child processes @@ -9,6 +10,10 @@ child_process = require('child_process') # group, then we can kill everything in that process group. module.exports = (command, options, callback = (err, stdout, stderr) ->) -> + if !Settings.enableConversions + error = new Error("Image conversions are disabled") + return callback(error) + # options are {timeout: number-of-milliseconds, killSignal: signal-name} [cmd, args...] = command diff --git a/services/filestore/config/settings.defaults.coffee b/services/filestore/config/settings.defaults.coffee index e04c86c6ea..7c5b01ef90 100644 --- a/services/filestore/config/settings.defaults.coffee +++ b/services/filestore/config/settings.defaults.coffee @@ -50,6 +50,7 @@ settings = # Any commands to wrap the convert utility in, for example ["nice"], or ["firejail", "--profile=/etc/firejail/convert.profile"] convertCommandPrefix: [] + enableConversions: false # Filestore health check # ---------------------- diff --git a/services/filestore/test/unit/coffee/ImageOptimiserTests.coffee b/services/filestore/test/unit/coffee/ImageOptimiserTests.coffee index e16d8e0917..398eacf70f 100644 --- a/services/filestore/test/unit/coffee/ImageOptimiserTests.coffee +++ b/services/filestore/test/unit/coffee/ImageOptimiserTests.coffee @@ -11,13 +11,16 @@ describe "ImageOptimiser", -> beforeEach -> @child_process = exec : sinon.stub() - + @settings = + enableConversions:true @optimiser = SandboxedModule.require modulePath, requires: 'child_process': @child_process "logger-sharelatex": log:-> err:-> warn:-> + "settings-sharelatex": @settings + @sourcePath = "/this/path/here.eps" @error = "Error" @@ -33,18 +36,29 @@ describe "ImageOptimiser", -> done() - it "should return the errro the file", (done)-> + it "should return the error", (done)-> @child_process.exec.callsArgWith(2, @error) @optimiser.compressPng @sourcePath, (err)=> err.should.equal @error done() - describe 'when optimiser is sigkilled', -> + describe 'when enableConversions is disabled', -> - it 'should not produce an error', (done) -> - @error = new Error('woops') - @error.signal = 'SIGKILL' - @child_process.exec.callsArgWith(2, @error) - @optimiser.compressPng @sourcePath, (err)=> - expect(err).to.equal(null) - done() + it 'should produce an error', (done) -> + @settings.enableConversions = false + @child_process.exec.callsArgWith(2) + @optimiser.compressPng @sourcePath, (err)=> + @child_process.exec.called.should.equal false + expect(err).to.exist + done() + + + describe 'when optimiser is sigkilled', -> + + it 'should not produce an error', (done) -> + @error = new Error('woops') + @error.signal = 'SIGKILL' + @child_process.exec.callsArgWith(2, @error) + @optimiser.compressPng @sourcePath, (err)=> + expect(err).to.equal(null) + done() diff --git a/services/filestore/test/unit/coffee/SafeExec.coffee b/services/filestore/test/unit/coffee/SafeExecTests.coffee similarity index 81% rename from services/filestore/test/unit/coffee/SafeExec.coffee rename to services/filestore/test/unit/coffee/SafeExecTests.coffee index 10d920df11..1be22f3993 100644 --- a/services/filestore/test/unit/coffee/SafeExec.coffee +++ b/services/filestore/test/unit/coffee/SafeExecTests.coffee @@ -9,11 +9,13 @@ SandboxedModule = require('sandboxed-module') describe "SafeExec", -> beforeEach -> - + @settings = + enableConversions:true @safe_exec = SandboxedModule.require modulePath, requires: "logger-sharelatex": log:-> err:-> + "settings-sharelatex": @settings @options = {timeout: 10*1000, killSignal: "SIGTERM" } describe "safe_exec", -> @@ -24,6 +26,12 @@ describe "SafeExec", -> should.not.exist(err) done() + it "should error when conversions are disabled", (done) -> + @settings.enableConversions = false + @safe_exec ["/bin/echo", "hello"], @options, (err, stdout, stderr) => + expect(err).to.exist + done() + it "should execute a command with non-zero exit status", (done) -> @safe_exec ["/usr/bin/env", "false"], @options, (err, stdout, stderr) => stdout.should.equal ""