Reduce customisations

Dockerfile

@@ -18,8 +18,6 @@ RUN \
     npm && \
   rm -rf /var/lib/apt/lists/*
 
-RUN gem update --system 3.3.25
-
 RUN curl -fsSL https://get.docker.com | bash -
 
 WORKDIR /app

app/models/git_repository.rb

@@ -185,15 +185,13 @@ def instance_cache(key)
   # error: nil
   def capture_stdout(*command, dir: repo_cache_dir)
     ::Rails.logger.info("Running command #{command}")
-    result = Samson::CommandExecutor.execute(
+    success, output, error = Samson::CommandExecutor.execute(
       *command,
       whitelist_env: ['HOME', 'PATH'],
       timeout: 30.minutes,
       dir: dir
     )
-    unless result.status
-      ::Rails.logger.error("Failed to run command #{command}: #{result.error}")
-    end
-    result.output.strip if result.status
+    ::Rails.logger.error("Failed to run command #{command}: #{error}") unless success
+    output.strip if success
   end
 end

app/models/job_execution.rb

@@ -181,7 +181,6 @@ def resolve_ref_to_commit(source)
       found
     end
     tag = @repository.fuzzy_tag_from_ref(source)
-
     if commit
       @job.update_git_references!(commit: commit, tag: tag)
 

lib/samson/command_executor.rb

@@ -1,5 +1,6 @@
 # frozen_string_literal: true
-require 'tempfile'
+
+require "stringio"
 
 module Samson
   # safe command execution that makes sure to use timeouts for everything and cleans up dead sub processes
@@ -12,7 +13,7 @@ def execute(*command, timeout:, whitelist_env: [], env: {}, dir: nil)
         raise ArgumentError, "Positive timeout required" if timeout <= 0
         env = ENV.to_h.slice(*whitelist_env).merge(env)
         pio = nil
-        stderr = Tempfile.new('stderr')
+        stderr = StringIO.new
         popen_options = {unsetenv_others: true, err: stderr}
         popen_options[:chdir] = dir if dir
 
@@ -22,28 +23,14 @@ def execute(*command, timeout:, whitelist_env: [], env: {}, dir: nil)
               pio = IO.popen(env, command.map(&:to_s), popen_options)
               output = pio.read
               pio.close
-              result = OpenStruct.new(
-                output: output,
-                error: File.read(stderr),
-                status: $?.success?
-              )
-              pio.close
-              result
+              [$?.success?, output, stderr.string]
             rescue Errno::ENOENT
-              OpenStruct.new(
-                error: "No such file or directory - #{command.first}",
-                status: false,
-                output: ""
-              )
+              [false, "", "No such file or directory - #{command.first}"]
             end
           end
         end
       rescue Timeout::Error
-        OpenStruct.new(
-          error: $!.message,
-          status: false,
-          output: ""
-        )
+        [false, "", $!.message]
       ensure
         if pio && !pio.closed?
           kill_process pio.pid

plugins/gcloud/app/controllers/gcloud_controller.rb

@@ -27,10 +27,10 @@ def update_build(build)
     command = [
       "gcloud", "container", "builds", "describe", build.gcr_id, "--format", "json", *SamsonGcloud.cli_options
     ]
-    result = Samson::CommandExecutor.execute(*command, timeout: 30, whitelist_env: ["PATH"])
-    return "Failed to execute gcloud command: #{result.output}" unless result.status
+    success, output, error = Samson::CommandExecutor.execute(*command, timeout: 30, whitelist_env: ["PATH"])
+    return "Failed to execute gcloud command: #{output} #{error}" unless success
 
-    response = JSON.parse(result.output)
+    response = JSON.parse(output)
     build.external_status = STATUSMAP.fetch(response.fetch("status"))
 
     if build.external_status == "succeeded"

plugins/gcloud/app/controllers/gke_clusters_controller.rb

@@ -34,16 +34,16 @@ def create
       "gcloud", "container", "clusters", "get-credentials", "--zone", zone, cluster,
       *SamsonGcloud.cli_options(project: project)
     ]
-    result = Samson::CommandExecutor.execute(
+    success, content, error = Samson::CommandExecutor.execute(
       *command,
       whitelist_env: ["PATH"],
       timeout: 10,
       env: {"KUBECONFIG" => path, "CLOUDSDK_CONTAINER_USE_CLIENT_CERTIFICATE" => "True"}
     )
 
-    unless result.status
+    unless success
       flash.now[:alert] = "Failed to execute (make sure container.cluster.getCredentials permissions are granted): " \
-        "#{command.join(" ")} #{result.output}"
+        "#{command.join(" ")} #{content} #{error}"
       return render :new
     end
 

plugins/gcloud/lib/samson_gcloud/image_builder.rb

@@ -88,12 +88,12 @@ def prevent_upload_of_ignored_files(dir, build)
       # NOTE: not using executor since it does not return output
       def image_exists_in_gcloud?(repo_digest)
         image, digest = repo_digest.split('@')
-        result = Samson::CommandExecutor.execute(
+        output = Samson::CommandExecutor.execute(
           "gcloud", "container", "images", "list-tags", image,
           "--format", "get(digest)", "--filter", "digest=#{digest}", *SamsonGcloud.cli_options,
           timeout: 10
-        )
-        result.output.strip == digest
+        ).second
+        output.strip == digest
       end
     end
   end

plugins/gcloud/lib/samson_gcloud/image_tagger.rb

@@ -33,14 +33,14 @@ def tag_image(tag, base, digest, job_output)
         command = [
           "gcloud", "container", "images", "add-tag", digest, "#{base}:#{tag}", "--quiet", *SamsonGcloud.cli_options
         ]
-        result = Samson::CommandExecutor.execute(*command, timeout: 10, whitelist_env: ["PATH"])
+        success, output = Samson::CommandExecutor.execute(*command, timeout: 10, whitelist_env: ["PATH"])
         job_output.write <<~TEXT
           #{Samson::OutputUtils.timestamp} Tagging GCR image:
           #{command.join(" ")}
-          #{result.output.strip}
+          #{output.strip}
         TEXT
-        job_output.puts "FAILED" unless result.status
-        result.status
+        job_output.puts "FAILED" unless success
+        success
       end
 
       def cache_last_tagged(key, value)

plugins/gcloud/lib/samson_gcloud/tag_resolver.rb

@@ -10,14 +10,15 @@ def resolve_docker_image_tag(image)
         return unless SamsonGcloud.gcr? image
         return if image.match? Build::DIGEST_REGEX
 
-        result = Samson::CommandExecutor.execute(
+        success, json, error = Samson::CommandExecutor.execute(
           "gcloud", "container", "images", "describe", image, "--format", "json",
           *SamsonGcloud.cli_options,
+          err: '/dev/null',
           timeout: 10,
           whitelist_env: ["PATH"]
         )
-        raise "GCLOUD ERROR: unable to resolve #{image}\n#{result.error}" unless result.status
-        digest = JSON.parse(result.output).dig_fetch("image_summary", "digest")
+        raise "GCLOUD ERROR: unable to resolve #{image}\n#{error}" unless success
+        digest = JSON.parse(json).dig_fetch("image_summary", "digest")
 
         base = image.split(":", 2).first
         "#{base}@#{digest}"

plugins/gcloud/test/controllers/gcloud_controller_test.rb

@@ -30,7 +30,7 @@ def do_sync
       end
 
       it "can sync" do
-        Samson::CommandExecutor.expects(:execute).returns(OpenStruct.new(status: true, output: result.to_json))
+        Samson::CommandExecutor.expects(:execute).returns([true, result.to_json, ""])
         do_sync
         assert flash[:notice]
         build.reload
@@ -40,13 +40,13 @@ def do_sync
 
       it "can sync images with a tag" do
         result[:results][:images][1][:name] += ":foo"
-        Samson::CommandExecutor.expects(:execute).returns(OpenStruct.new(status: true, output: result.to_json))
+        Samson::CommandExecutor.expects(:execute).returns([true, result.to_json, ""])
         do_sync
         build.reload.docker_repo_digest.must_equal repo_digest
       end
 
       it "fails when gcloud cli fails" do
-        Samson::CommandExecutor.expects(:execute).returns(OpenStruct.new(status: false, output: result.to_json))
+        Samson::CommandExecutor.expects(:execute).returns([false, result.to_json, ""])
         do_sync
         assert flash[:alert]
       end
@@ -55,7 +55,7 @@ def do_sync
         let(:image_name) { "gcr.io/foo*baz+bing/#{build.image_name}" }
 
         it "fails when digest does not pass validations" do
-          Samson::CommandExecutor.expects(:execute).returns(OpenStruct.new(status: true, output: result.to_json))
+          Samson::CommandExecutor.expects(:execute).returns([true, result.to_json, ""])
 
           do_sync
 
@@ -66,7 +66,7 @@ def do_sync
 
       it "fails when image is not found" do
         result[:results][:images].last[:name] = "gcr.io/other"
-        Samson::CommandExecutor.expects(:execute).returns(OpenStruct.new(status: true, output: result.to_json))
+        Samson::CommandExecutor.expects(:execute).returns([true, result.to_json, ""])
 
         do_sync
 
@@ -77,7 +77,7 @@ def do_sync
       it "can store failures" do
         result[:status] = "QUEUED"
         result.delete(:results)
-        Samson::CommandExecutor.expects(:execute).returns(OpenStruct.new(status: true, output: result.to_json))
+        Samson::CommandExecutor.expects(:execute).returns([true, result.to_json, ""])
 
         do_sync
 

plugins/gcloud/test/controllers/gke_clusters_controller_test.rb

@@ -38,7 +38,7 @@ def do_create(gcp_project: 'pp', cluster_name: 'cc', zone: 'zz')
           timeout: 10,
           env: {'KUBECONFIG' => expected_file, "CLOUDSDK_CONTAINER_USE_CLIENT_CERTIFICATE" => "True"},
           whitelist_env: ['PATH']
-        ).returns(OpenStruct.new(status: true, output: "foo"))
+        ).returns([true, "foo", ""])
         do_create
         assert_redirected_to(
           "/kubernetes/clusters/new?kubernetes_cluster%5Bconfig_filepath%5D=#{CGI.escape(expected_file)}"
@@ -48,7 +48,7 @@ def do_create(gcp_project: 'pp', cluster_name: 'cc', zone: 'zz')
       end
 
       it "shows errors when command fails" do
-        Samson::CommandExecutor.expects(:execute).returns(OpenStruct.new(status: false, output: "foo"))
+        Samson::CommandExecutor.expects(:execute).returns([false, "foo", ""])
         do_create
         assert_response :success
         flash[:alert].must_include(

plugins/gcloud/test/samson_gcloud/image_builder_test.rb

@@ -46,7 +46,7 @@ def build_image(tag_as_latest: false, cache_from: nil)
     end
 
     it "can use cache" do
-      Samson::CommandExecutor.expects(:execute).returns(OpenStruct.new(status: true, output: "sha256:abc\n"))
+      Samson::CommandExecutor.expects(:execute).returns([true, "sha256:abc\n", ""])
       old = 'gcr.io/something-old@sha256:abc'
       build_image(cache_from: old)
       File.read("some-dir/cloudbuild.yml").must_equal <<~YML
@@ -63,7 +63,7 @@ def build_image(tag_as_latest: false, cache_from: nil)
     end
 
     it "does not use cache when image is not available" do
-      Samson::CommandExecutor.expects(:execute).returns(OpenStruct.new(status: true, output: "\n"))
+      Samson::CommandExecutor.expects(:execute).returns([true, "\n", ""])
       build_image(cache_from: 'gcr.io/something-old')
       File.read("some-dir/cloudbuild.yml").must_equal <<~YML
         steps:

plugins/gcloud/test/samson_gcloud/image_tagger_test.rb

@@ -16,7 +16,7 @@ def assert_tagged_with(tag, opts: [])
       Samson::CommandExecutor.expects(:execute).with(
         'gcloud', 'container', 'images', 'add-tag', 'gcr.io/sdfsfsdf@some-sha', "gcr.io/sdfsfsdf:#{tag}",
         '--quiet', *opts, *auth_options, anything
-      ).returns(OpenStruct.new(status: true, output: "OUT"))
+      ).returns([true, "OUT", ""])
     end
 
     with_env DOCKER_FEATURE: 'true', GCLOUD_PROJECT: '123', GCLOUD_ACCOUNT: 'acc'
@@ -53,7 +53,7 @@ def assert_tagged_with(tag, opts: [])
     end
 
     it "tries again when tagging failed" do
-      Samson::CommandExecutor.expects(:execute).returns(OpenStruct.new(status: false, output: 'x')).times(2)
+      Samson::CommandExecutor.expects(:execute).returns([false, 'x', ""]).times(2)
       2.times { tag }
     end
 
@@ -72,7 +72,7 @@ def assert_tagged_with(tag, opts: [])
 
     it "tags other regions" do
       build.update_column(:docker_repo_digest, 'asia.gcr.io/sdfsfsdf@some-sha')
-      Samson::CommandExecutor.expects(:execute).returns(OpenStruct.new(status: true, output: "OUT"))
+      Samson::CommandExecutor.expects(:execute).returns([true, "OUT", ""])
       tag
     end
 
@@ -89,7 +89,7 @@ def assert_tagged_with(tag, opts: [])
     end
 
     it "shows tagging errors" do
-      Samson::CommandExecutor.expects(:execute).returns(OpenStruct.new(status: false, output: "NOPE"))
+      Samson::CommandExecutor.expects(:execute).returns([false, "NOPE", ""])
       tag
       output_serialized.must_include "NOPE"
     end

plugins/gcloud/test/samson_gcloud/tag_resolver_test.rb

@@ -12,13 +12,13 @@
 
     it "resolves latest" do
       Samson::CommandExecutor.expects(:execute).
-        returns(OpenStruct.new(status: true, output: {image_summary: {digest: digest}}.to_json))
+        returns([true, {image_summary: {digest: digest}}.to_json, ""])
       SamsonGcloud::TagResolver.resolve_docker_image_tag(image).must_equal "#{image}@#{digest}"
     end
 
     it "resolves custom tag" do
       Samson::CommandExecutor.expects(:execute).
-        returns(OpenStruct.new(status: true, output: {image_summary: {digest: digest}}.to_json))
+        returns([true, {image_summary: {digest: digest}}.to_json, ""])
       SamsonGcloud::TagResolver.resolve_docker_image_tag("#{image}:foo").must_equal "#{image}@#{digest}"
     end
 
@@ -32,7 +32,7 @@
 
     it "raises on gcloud error" do
       Samson::CommandExecutor.expects(:execute).
-        returns(OpenStruct.new(status: false, output: ""))
+        returns([false, "", ""])
       assert_raises RuntimeError do
         SamsonGcloud::TagResolver.resolve_docker_image_tag(image)
       end

test/integration/cleanliness_test.rb

@@ -236,6 +236,10 @@ def assert_content(files)
     values.uniq.size.must_equal 1, "Expected all places to use consistent PERIODICAL value, but found #{values.inspect}"
   end
 
+  it "has gitignore and dockerignore in sync" do
+    File.read(".dockerignore").must_include File.read(".gitignore")
+  end
+
   it "explicity defines what should happen to dependencies" do
     bad = all_models.flat_map do |model|
       model.reflect_on_all_associations.map do |association|