diff --git a/app/models/docker_builder_service.rb b/app/models/docker_builder_service.rb index 8a8a3bdbda..7e70195124 100644 --- a/app/models/docker_builder_service.rb +++ b/app/models/docker_builder_service.rb @@ -45,14 +45,21 @@ def run!(image_name: nil, push: false, tag_as_latest: false) @output = execution.output repository.executor = execution.executor - if build.kubernetes_job - run_build_image_job(job, image_name, push: push, tag_as_latest: tag_as_latest) - elsif build_image(tmp_dir) - ret = true - ret = push_image(image_name, tag_as_latest: tag_as_latest) if push - build.docker_image.remove(force: true) unless ENV["DOCKER_KEEP_BUILT_IMGS"] == "1" - ret + success = + if build.kubernetes_job + run_build_image_job(job, image_name, push: push, tag_as_latest: tag_as_latest) + elsif build_image(tmp_dir) + ret = true + ret = push_image(image_name, tag_as_latest: tag_as_latest) if push + build.docker_image.remove(force: true) unless ENV["DOCKER_KEEP_BUILT_IMGS"] == "1" + ret + end + + if success + Samson::Clair.append_job_with_scan(job, docker_image_ref(image_name, build)) end + + success end job_execution.on_complete { send_after_notifications } diff --git a/lib/samson/clair.rb b/lib/samson/clair.rb new file mode 100644 index 0000000000..f7fa441903 --- /dev/null +++ b/lib/samson/clair.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true +module Samson + # Hyperclair will pull the image from registry and run scan with Clair scanner + # TODO: should check based on docker_repo_digest not tag + # TODO: this should be a plugin instead and use hooks + module Clair + class << self + def append_job_with_scan(job, docker_tag) + return unless clair = ENV['HYPERCLAIR_PATH'] + + Thread.new do + sleep 0.1 if Rails.env.test? # in test we reuse the same connection, so we cannot use it at the same time + success, output, time = scan(clair, job.project, docker_tag) + status = (success ? "success" : "errored or vulnerabilities found") + output = "### Clair scan: #{status} in #{time}s\n#{output}" + job.reload + job.update_column(:output, job.output + output) + end + end + + private + + def scan(executable, project, docker_ref) + with_time do + Samson::CommandExecutor.execute( + executable, + *project.docker_repo.split('/', 2), + docker_ref, + whitelist_env: ['DOCKER_REGISTRY_USER', 'DOCKER_REGISTRY_PASS', 'PATH'], + timeout: 60 * 60 + ) + end + end + + def with_time + result = [] + time = Benchmark.realtime { result = yield } + result << time + end + end + end +end diff --git a/lib/samson/command_executor.rb b/lib/samson/command_executor.rb new file mode 100644 index 0000000000..d177a8dfc2 --- /dev/null +++ b/lib/samson/command_executor.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true +module Samson + # TODO: reuse in git_repo ? + # safe command execution that makes sure to use timeouts for everything and cleans up dead sub processes + module CommandExecutor + class << self + # timeout could be done more reliably with timeout(1) from gnu coreutils ... but that would add another dependency + def execute(*command, timeout:, whitelist_env: []) + raise ArgumentError, "Positive timeout required" if timeout <= 0 + output = "ABORTED" + pid = nil + + wait = Thread.new do + begin + IO.popen(ENV.to_h.slice(*whitelist_env), command, unsetenv_others: true, err: [:child, :out]) do |io| + pid = io.pid + output = io.read + end + $?&.success? || false + rescue Errno::ENOENT + output = "No such file or directory - #{command.first}" + false + end + end + success = Timeout.timeout(timeout) { wait.value } # using timeout in a blocking thread never interrupts + + return success, output + rescue Timeout::Error + kill_process pid if pid + return false, $!.message + end + + private + + # timeout or parent process interrupted by user with Interrupt or SystemExit + def kill_process(pid) + Process.kill :INT, pid # tell it to stop + sleep 1 # give it a second to clean up + Process.kill :KILL, pid # kill it + Process.wait pid # prevent zombie processes + rescue Errno::ESRCH, Errno::ECHILD # kill or wait failing because pid was already gone + nil + end + end + end +end diff --git a/plugins/kubernetes/app/models/kubernetes/build_job_executor.rb b/plugins/kubernetes/app/models/kubernetes/build_job_executor.rb index a41030bb99..f6e077f8bb 100644 --- a/plugins/kubernetes/app/models/kubernetes/build_job_executor.rb +++ b/plugins/kubernetes/app/models/kubernetes/build_job_executor.rb @@ -34,10 +34,6 @@ def execute!(build, project, docker_ref:, push: false, tag_as_latest: false) message = (success ? "completed successfully" : "failed or timed out") @output.puts "### Remote build job #{job_name} #{message}" - if success && clair = ENV['HYPERCLAIR_PATH'] - Thread.new { scan_with_clair(clair, project, docker_ref) } - end - return success, job_log ensure # Jobs will still be there regardless of their statuses @@ -84,7 +80,7 @@ def fill_job_details(k8s_job, build, project, docker_ref:, push: false, tag_as_l # Pass all necessary information so that remote container can build the image container_params = { - env: [{name: 'DOCKER_REGISTRY', value: @registry[:serveraddress] }], + env: [{name: 'DOCKER_REGISTRY', value: @registry.fetch(:serveraddress) }], args: [ project.repository_url, build.git_sha, project.docker_repo, docker_ref, push ? "yes" : "no", @@ -94,55 +90,6 @@ def fill_job_details(k8s_job, build, project, docker_ref:, push: false, tag_as_l k8s_job[:spec][:template][:spec][:containers][0].update(container_params) end - # Run a shell script that wraps hyperclair. - # Hyperclair will pull the image from registry and run scan with Clair scanner - def scan_with_clair(script, project, git_ref) - sleep 0.1 if Rails.env.test? # in test we reuse the same connection, so we cannot use it at the same time - project_param = project.permalink.tr('_', '-') - success, output, time = execute_command( - script, - @registry.fetch(:serveraddress), - project_param, - git_ref, - timeout: 60 * 60 - ) - status = (success ? "success" : "errored or vulnerabilities found") - output = "### Clair scan: #{status} in #{time}s\n#{output}" - @job.reload - @job.update_column(:output, @job.output + output) - end - - # timeout could be done more reliably with timeout(1) from gnu coreutils ... but that would add another dependency - def execute_command(*command, timeout:) - output = "ABORTED" - - time = Benchmark.realtime do - IO.popen(command, unsetenv_others: true, err: [:child, :out]) do |io| - output = Timeout.timeout(timeout) { kill_child_on_error(io) { io.read } } - end - end.round - success = $?.success? - - return success, output, time - rescue Errno::ENOENT, Timeout::Error - return false, $!.message, 0 - end - - # timeout or parent process interrupted by user with Interrupt or SystemExit - def kill_child_on_error(io) - yield - rescue Exception => e # rubocop:disable Lint/RescueException - begin - Process.kill :INT, io.pid # tell it to stop - sleep 1 # give it a second to clean up - Process.kill :KILL, io.pid # kill it - Process.wait io.pid # prevent zombie processes - rescue Errno::ESRCH # rubocop:disable Lint/HandleExceptions - # pid was already gone - end - raise e - end - def job_config(build, project, docker_ref:, push: false, tag_as_latest: false) # Read the external config path and create a new job config instance contents = File.read(build_job_config_path) diff --git a/plugins/kubernetes/test/models/kubernetes/build_job_executor_test.rb b/plugins/kubernetes/test/models/kubernetes/build_job_executor_test.rb index bc216ad364..45e5ea6dd7 100644 --- a/plugins/kubernetes/test/models/kubernetes/build_job_executor_test.rb +++ b/plugins/kubernetes/test/models/kubernetes/build_job_executor_test.rb @@ -150,87 +150,6 @@ def execute!(push: false) assert success assert_equal(job_pod_log.join("\n") << "\n", job_log) end - - describe "clair" do - before { ActiveRecord::Base.stubs(connection: ActiveRecord::Base.connection) } # we update in another thread - - around do |t| - Tempfile.open('clair') do |f| - f.write("#!/bin/bash\necho HELLO\nexit 0") - f.close - File.chmod 0o755, f.path - with_env(HYPERCLAIR_PATH: f.path, &t) - end - end - - it "runs clair and reports success to the database" do - success, job_log = execute! - job_log.wont_include "Clair" - assert success - - wait_for_threads - - job = Job.first - job.output.must_include "Clair scan: success" - job.output.must_include "HELLO" - end - - it "does not run clair when build failed" do - job_api_obj.stubs(:failure?).returns true - success, job_log = execute! - job_log.wont_include "Clair" - refute success - - wait_for_threads # just in case something goes wrong / to keep tests symmetric - - job = Job.first - job.output.wont_include "Clair scan" - end - - it "runs clair and reports missing script to the database" do - File.unlink ENV['HYPERCLAIR_PATH'] - - success, job_log = execute! - job_log.wont_include "Clair" - assert success - - wait_for_threads - - job = Job.first - job.output.must_include "Clair scan: errored" - job.output.must_include "No such file or directory" - end - - it "runs clair and reports failed script to the database" do - File.write ENV['HYPERCLAIR_PATH'], "#!/bin/bash\necho WORLD\nexit 1" - - success, job_log = execute! - job_log.wont_include "Clair" - assert success - - wait_for_threads - - job = Job.first - job.output.must_include "Clair scan: errored" - job.output.must_include "WORLD" - end - - it "runs clair and reports timed out script to the database" do - IO.any_instance.expects(:read).raises(Timeout::Error) - Process.expects(:kill).times(2) - Process.expects(:wait) - - success, job_log = execute! - job_log.wont_include "Clair" - assert success - - wait_for_threads - - job = Job.first - job.output.must_include "Clair scan: errored" - job.output.must_include "Timeout::Error" - end - end end end end diff --git a/test/lib/samson/clair_test.rb b/test/lib/samson/clair_test.rb new file mode 100644 index 0000000000..0a95a31ed6 --- /dev/null +++ b/test/lib/samson/clair_test.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true +require_relative '../../test_helper' + +SingleCov.covered! + +describe Samson::Clair do + def execute! + Samson::Clair.append_job_with_scan(job, 'latest') + end + + let(:job) { jobs(:succeeded_test) } + + before { ActiveRecord::Base.stubs(connection: ActiveRecord::Base.connection) } # we update in another thread + + around do |t| + Tempfile.open('clair') do |f| + f.write("#!/bin/bash\necho HELLO\nexit 0") + f.close + File.chmod 0o755, f.path + with_env(HYPERCLAIR_PATH: f.path, DOCKER_REGISTRY: 'my.registry', &t) + end + end + + it "runs clair and reports success to the database" do + execute! + + wait_for_threads + + job.reload + job.output.must_include "Clair scan: success" + job.output.must_include "HELLO" + end + + it "runs clair and reports missing script to the database" do + File.unlink ENV['HYPERCLAIR_PATH'] + + execute! + + wait_for_threads + + job.reload + job.output.must_include "Clair scan: errored" + job.output.must_include "No such file or directory" + end + + it "runs clair and reports failed script to the database" do + File.write ENV['HYPERCLAIR_PATH'], "#!/bin/bash\necho WORLD\nexit 1" + + execute! + + wait_for_threads + + job.reload + job.output.must_include "Clair scan: errored" + job.output.must_include "WORLD" + end +end diff --git a/test/lib/samson/command_executor_test.rb b/test/lib/samson/command_executor_test.rb new file mode 100644 index 0000000000..c3b6cb6bd2 --- /dev/null +++ b/test/lib/samson/command_executor_test.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true +require_relative '../../test_helper' + +SingleCov.covered! + +describe Samson::CommandExecutor do + describe "#execute" do + it "runs" do + Samson::CommandExecutor.execute("echo", "hello", timeout: 1).must_equal [true, "hello\n"] + end + + it "fails" do + Samson::CommandExecutor.execute("foo", "bar", timeout: 1).must_equal [false, "No such file or directory - foo"] + end + + it "times out and cleans up" do + command = ["sleep", "15"] + Samson::CommandExecutor.expects(:sleep) # waiting after kill ... no need to make this test slow + time = Benchmark.realtime do + Samson::CommandExecutor.execute(*command, timeout: 0.1).must_equal [false, "execution expired"] + end + time.must_be :<, 0.2 + `ps -ef`.wont_include(command.join(" ")) + end + + it "does not fail when pid was already gone" do + Process.expects(:kill).raises(Errno::ESRCH) # simulate that pid was gone and kill failed + Samson::CommandExecutor.execute("sleep", "0.2", timeout: 0.1).must_equal [false, "execution expired"] + sleep 0.2 # do not leave process thread hanging + end + + it "waits for zombie processes" do + Samson::CommandExecutor.expects(:sleep) # waiting after kill ... we ignore it in this test + Process.expects(:kill).twice # simulate that process could not be killed with :KILL + time = Benchmark.realtime do + Samson::CommandExecutor.execute("sleep", "0.5", timeout: 0.1).must_equal [false, "execution expired"] + end + time.must_be :>, 0.5 + end + + it "complains about infinite timeout" do + assert_raises ArgumentError do + Samson::CommandExecutor.execute("sleep", "5", timeout: 0) + end + end + + it "does not allow injection" do + Samson::CommandExecutor.execute("echo", "hel << lo | ;", timeout: 1).must_equal [true, "hel << lo | ;\n"] + end + + it "does not allow env access" do + with_env FOO: 'bar' do + Samson::CommandExecutor.execute("printenv", "FOO", timeout: 1).must_equal [false, ""] + end + end + + it "allows whitelisted env access" do + with_env FOO: 'bar' do + Samson::CommandExecutor.execute("printenv", "FOO", timeout: 1, whitelist_env: ["FOO"]). + must_equal [true, "bar\n"] + end + end + end +end diff --git a/test/models/docker_builder_service_test.rb b/test/models/docker_builder_service_test.rb index 1996e5b2e9..f813420d24 100644 --- a/test/models/docker_builder_service_test.rb +++ b/test/models/docker_builder_service_test.rb @@ -10,10 +10,8 @@ let(:project_repo_url) { repo_temp_dir } let(:git_tag) { 'v123' } let(:project) { projects(:test).tap { |p| p.repository_url = project_repo_url } } - let(:build) { project.builds.create!(git_ref: git_tag, git_sha: 'a' * 40) } let(:service) { DockerBuilderService.new(build) } - let(:docker_image_id) { '2d2b0b3204b0166435c3d96d0b27d0ad2083e5e040192632c58eeb9491d6bfaa' } let(:docker_image_json) do { @@ -64,7 +62,7 @@ def execute_job end it "does not remove when DOCKER_KEEP_BUILT_IMGS is set" do - with_env "DOCKER_KEEP_BUILT_IMGS" => "1" do + with_env DOCKER_KEEP_BUILT_IMGS: "1" do run!(push: false) service.expects(:build_image).returns(true) # simulate that build worked @@ -76,7 +74,7 @@ def execute_job end it "returns push_image result when it pushes" do - with_env "DOCKER_KEEP_BUILT_IMGS" => "1" do + with_env DOCKER_KEEP_BUILT_IMGS: "1" do run!(push: true) # simulate that build worked @@ -99,6 +97,24 @@ def execute_job execute_job.must_equal(123) end end + + describe "with clair" do + with_env HYPERCLAIR_PATH: 'foobar', DOCKER_KEEP_BUILT_IMGS: "1" + + it "runs clair" do + Samson::Clair.expects(:append_job_with_scan).with(instance_of(Job), 'latest') + service.expects(:build_image).returns(true) + run! + execute_job + end + + it "does not run clair when build failed" do + Samson::Clair.expects(:append_job_with_scan).never + service.expects(:build_image).returns(false) + run! + execute_job + end + end end describe "#run_build_image_job" do