5416: Disable repository browsing (and skip tests) if git version is suspected unreli...

diff --git a/apps/workbench/app/controllers/repositories_controller.rb b/apps/workbench/app/controllers/repositories_controller.rb
index c5b3501b328e1214cc00c292c19f61d0be07312f..89dd96bc6e4264fe2b258583adb1c1ca1708d67a 100644 (file)
--- a/apps/workbench/app/controllers/repositories_controller.rb
+++ b/apps/workbench/app/controllers/repositories_controller.rb
@@ -1,5 +1,8 @@
 class RepositoriesController < ApplicationController
   before_filter :set_share_links, if: -> { defined? @object }
+  if Repository.disable_repository_browsing?
+    before_filter :render_browsing_disabled, only: [:show_tree, :show_blob, :show_commit]
+  end
 
   def index_pane_list
     %w(recent help)
@@ -32,4 +35,10 @@ class RepositoriesController < ApplicationController
   def show_commit
     @commit = params[:commit]
   end
+
+  protected
+
+  def render_browsing_disabled
+    render_not_found ActionController::RoutingError.new("Repository browsing features disabled")
+  end
 end

diff --git a/apps/workbench/app/models/repository.rb b/apps/workbench/app/models/repository.rb
index aa7791388f72f4c50f8ccbc8a9392e698fa792d3..28f8f0cc91124ceafd3dfce682b140c1a740659f 100644 (file)
--- a/apps/workbench/app/models/repository.rb
+++ b/apps/workbench/app/models/repository.rb
@@ -48,6 +48,15 @@ class Repository < ArvadosBase
     subtree
   end
 
+  # git 2.1.4 does not use credential helpers reliably, see #5416
+  def self.disable_repository_browsing?
+    return false if Rails.configuration.use_git2_despite_bug_risk
+    if @buggy_git_version.nil?
+      @buggy_git_version = /git version 2/ =~ `git version`
+    end
+    @buggy_git_version
+  end
+
   protected
 
   # refresh fetches the latest repository content into the local

diff --git a/apps/workbench/app/views/pipeline_instances/_running_component.html.erb b/apps/workbench/app/views/pipeline_instances/_running_component.html.erb
index 2ab8da1f4ce2d258d8f098c5c78175d6fbcdf713..80210c43600bdccc1c2c3d9343bf998760f60e1b 100644 (file)
--- a/apps/workbench/app/views/pipeline_instances/_running_component.html.erb
+++ b/apps/workbench/app/views/pipeline_instances/_running_component.html.erb
@@ -99,7 +99,8 @@
               <% # link to repo tree/file only if the repo is readable
                  # and the commit is a sha1...
                  repo =
-                 (/^[0-9a-f]{40}$/ =~ current_component[:script_version] and
+                 (not Repository.disable_repository_browsing? and
+                 /^[0-9a-f]{40}$/ =~ current_component[:script_version] and
                  Repository.where(name: current_component[:repository]).first)
 
                  # ...and the api server provides an http:// or https:// url

diff --git a/apps/workbench/config/application.default.yml b/apps/workbench/config/application.default.yml
index 4061ee83d89ae83a0ef363e1e43571326ca870e6..e28f76ae29f26e5f51a655c2f747cd270c9673d8 100644 (file)
--- a/apps/workbench/config/application.default.yml
+++ b/apps/workbench/config/application.default.yml
@@ -211,3 +211,9 @@ common:
 
   # Enable response payload compression in Arvados API requests.
   include_accept_encoding_header_in_api_requests: true
+
+  # Enable repository browsing even if git2 is installed. Repository
+  # browsing requires credential helpers, which do not work reliably
+  # as of git version 2.1.4. If you have git version 2.* and you want
+  # to use it anyway, change this to true.
+  use_git2_despite_bug_risk: false

diff --git a/apps/workbench/test/controllers/repositories_controller_test.rb b/apps/workbench/test/controllers/repositories_controller_test.rb
index 25bf55768529f58327e7f8861835516e0b0f5f06..852a602d526ee70120b68bde1af1caf4f48b113d 100644 (file)
--- a/apps/workbench/test/controllers/repositories_controller_test.rb
+++ b/apps/workbench/test/controllers/repositories_controller_test.rb
@@ -69,6 +69,7 @@ class RepositoriesControllerTest < ActionController::TestCase
 
   [:active, :spectator].each do |user|
     test "show tree to #{user}" do
+      skip "git2 is unreliable" if Repository.disable_repository_browsing?
       reset_api_fixtures_after_test false
       sha1, _, _ = stub_repo_content
       get :show_tree, {
@@ -85,6 +86,7 @@ class RepositoriesControllerTest < ActionController::TestCase
     end
 
     test "show commit to #{user}" do
+      skip "git2 is unreliable" if Repository.disable_repository_browsing?
       reset_api_fixtures_after_test false
       sha1, commit, _ = stub_repo_content
       get :show_commit, {
@@ -96,6 +98,7 @@ class RepositoriesControllerTest < ActionController::TestCase
     end
 
     test "show blob to #{user}" do
+      skip "git2 is unreliable" if Repository.disable_repository_browsing?
       reset_api_fixtures_after_test false
       sha1, _, filedata = stub_repo_content filename: 'COPYING'
       get :show_blob, {
@@ -110,6 +113,7 @@ class RepositoriesControllerTest < ActionController::TestCase
 
   ['', '/'].each do |path|
     test "show tree with path '#{path}'" do
+      skip "git2 is unreliable" if Repository.disable_repository_browsing?
       reset_api_fixtures_after_test false
       sha1, _, _ = stub_repo_content filename: 'COPYING'
       get :show_tree, {

diff --git a/apps/workbench/test/integration/repositories_browse_test.rb b/apps/workbench/test/integration/repositories_browse_test.rb
index a6a85b5e4db04705604df37c0a1ffd7e24c28e85..d936877ae28a96ab530a9ad6df8e845e4480a2b7 100644 (file)
--- a/apps/workbench/test/integration/repositories_browse_test.rb
+++ b/apps/workbench/test/integration/repositories_browse_test.rb
@@ -13,6 +13,7 @@ class RepositoriesTest < ActionDispatch::IntegrationTest
   end
 
   test "browse repository from jobs#show" do
+    skip "git2 is unreliable" if Repository.disable_repository_browsing?
     sha1 = api_fixture('jobs')['running']['script_version']
     _, fakecommit, fakefile =
       stub_repo_content sha1: sha1, filename: 'crunch_scripts/hash'
@@ -36,6 +37,7 @@ class RepositoriesTest < ActionDispatch::IntegrationTest
   end
 
   test "browse using arv-git-http" do
+    skip "git2 is unreliable" if Repository.disable_repository_browsing?
     repo = api_fixture('repositories')['foo']
     portfile =
       File.expand_path('../../../../../tmp/arv-git-httpd-ssl.port', __FILE__)