[ARVADOS] updated: d8626f8ae89b0748bc8ac45c1050c4090a166d7c
git at public.curoverse.com
git at public.curoverse.com
Thu Apr 16 16:32:03 EDT 2015
Summary of changes:
apps/workbench/app/controllers/repositories_controller.rb | 9 +++++++++
apps/workbench/app/models/repository.rb | 9 +++++++++
.../app/views/pipeline_instances/_running_component.html.erb | 3 ++-
apps/workbench/config/application.default.yml | 6 ++++++
apps/workbench/test/controllers/repositories_controller_test.rb | 4 ++++
apps/workbench/test/integration/repositories_browse_test.rb | 2 ++
services/arv-git-httpd/server_test.go | 6 ++----
7 files changed, 34 insertions(+), 5 deletions(-)
discards e1445f72262dffccc3f6f750bc734a3def68e8cd (commit)
via d8626f8ae89b0748bc8ac45c1050c4090a166d7c (commit)
via d88c3bf86de3a57dc95f327290c121f7e65295a9 (commit)
This update added new revisions after undoing existing revisions. That is
to say, the old revision is not a strict subset of the new revision. This
situation occurs when you --force push a change and generate a repository
containing something like this:
* -- * -- B -- O -- O -- O (e1445f72262dffccc3f6f750bc734a3def68e8cd)
\
N -- N -- N (d8626f8ae89b0748bc8ac45c1050c4090a166d7c)
When this happens we assume that you've already had alert emails for all
of the O revisions, and so we here report only the revisions in the N
branch from the common base, B.
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
commit d8626f8ae89b0748bc8ac45c1050c4090a166d7c
Author: Tom Clegg <tom at curoverse.com>
Date: Thu Apr 16 16:31:22 2015 -0400
5416: Disable repository browsing (and skip tests) if git version is suspected unreliable.
diff --git a/apps/workbench/app/controllers/repositories_controller.rb b/apps/workbench/app/controllers/repositories_controller.rb
index c5b3501..89dd96b 100644
--- 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 48c7f9e..bc9f87c 100644
--- 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 af83031..fbe1dbc 100644
--- 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)
%>
<% [:script, :repository, :script_version, :supplied_script_version, :nondeterministic].each do |k| %>
diff --git a/apps/workbench/config/application.default.yml b/apps/workbench/config/application.default.yml
index 5b2391f..4dfc48b 100644
--- a/apps/workbench/config/application.default.yml
+++ b/apps/workbench/config/application.default.yml
@@ -210,3 +210,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 25bf557..852a602 100644
--- 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 a6a85b5..d936877 100644
--- 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__)
commit d88c3bf86de3a57dc95f327290c121f7e65295a9
Author: Tom Clegg <tom at curoverse.com>
Date: Thu Apr 16 15:52:20 2015 -0400
5416: Use http://foo:bar@host:port/ instead of credential helper.
diff --git a/services/arv-git-httpd/auth_handler.go b/services/arv-git-httpd/auth_handler.go
index ef16acb..df2d8d6 100644
--- a/services/arv-git-httpd/auth_handler.go
+++ b/services/arv-git-httpd/auth_handler.go
@@ -52,7 +52,7 @@ func (h *authHandler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) {
w.WriteHeader(statusCode)
w.Write([]byte(statusText))
}
- log.Println(quoteStrings(r.RemoteAddr, username, password, wroteStatus, statusText, repoName, r.URL.Path)...)
+ log.Println(quoteStrings(r.RemoteAddr, username, password, wroteStatus, statusText, repoName, r.Method, r.URL.Path)...)
}()
// HTTP request username is logged, but unused. Password is an
@@ -87,7 +87,7 @@ func (h *authHandler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) {
arv.ApiToken = password
reposFound := arvadosclient.Dict{}
if err := arv.List("repositories", arvadosclient.Dict{
- "filters": [][]string{[]string{"name", "=", repoName}},
+ "filters": [][]string{{"name", "=", repoName}},
}, &reposFound); err != nil {
statusCode, statusText = http.StatusInternalServerError, err.Error()
return
diff --git a/services/arv-git-httpd/server_test.go b/services/arv-git-httpd/server_test.go
index 82d71ae..d773dd9 100644
--- a/services/arv-git-httpd/server_test.go
+++ b/services/arv-git-httpd/server_test.go
@@ -14,6 +14,12 @@ import (
var _ = check.Suite(&IntegrationSuite{})
+const (
+ spectatorToken = "zw2f4gwx8hw8cjre7yp6v1zylhrhn3m5gvjq73rtpwhmknrybu"
+ activeToken = "3kg6k6lzmp9kj5cpkcoxie963cmvjahbt2fod9zru30k1jqdmi"
+ anonymousToken = "4kg6k6lzmp9kj4cpkcoxie964cmvjahbt4fod9zru44k4jqdmi"
+)
+
// IntegrationSuite tests need an API server and an arv-git-httpd server
type IntegrationSuite struct {
tmpRepoRoot string
@@ -23,55 +29,43 @@ type IntegrationSuite struct {
func (s *IntegrationSuite) TestPathVariants(c *check.C) {
s.makeArvadosRepo(c)
- // Spectator token
- os.Setenv("ARVADOS_API_TOKEN", "zw2f4gwx8hw8cjre7yp6v1zylhrhn3m5gvjq73rtpwhmknrybu")
for _, repo := range []string{"active/foo.git", "active/foo/.git", "arvados.git", "arvados/.git"} {
- err := s.runGit(c, "fetch", repo)
+ err := s.runGit(c, spectatorToken, "fetch", repo)
c.Assert(err, check.Equals, nil)
}
}
func (s *IntegrationSuite) TestReadonly(c *check.C) {
- // Spectator token
- os.Setenv("ARVADOS_API_TOKEN", "zw2f4gwx8hw8cjre7yp6v1zylhrhn3m5gvjq73rtpwhmknrybu")
- err := s.runGit(c, "fetch", "active/foo.git")
+ err := s.runGit(c, spectatorToken, "fetch", "active/foo.git")
c.Assert(err, check.Equals, nil)
- err = s.runGit(c, "push", "active/foo.git", "master:newbranchfail")
+ err = s.runGit(c, spectatorToken, "push", "active/foo.git", "master:newbranchfail")
c.Assert(err, check.ErrorMatches, `.*HTTP code = 403.*`)
_, err = os.Stat(s.tmpRepoRoot + "/zzzzz-s0uqq-382brsig8rp3666/.git/refs/heads/newbranchfail")
c.Assert(err, check.FitsTypeOf, &os.PathError{})
}
func (s *IntegrationSuite) TestReadwrite(c *check.C) {
- // Active user token
- os.Setenv("ARVADOS_API_TOKEN", "3kg6k6lzmp9kj5cpkcoxie963cmvjahbt2fod9zru30k1jqdmi")
- err := s.runGit(c, "fetch", "active/foo.git")
+ err := s.runGit(c, activeToken, "fetch", "active/foo.git")
c.Assert(err, check.Equals, nil)
- err = s.runGit(c, "push", "active/foo.git", "master:newbranch")
+ err = s.runGit(c, activeToken, "push", "active/foo.git", "master:newbranch")
c.Assert(err, check.Equals, nil)
_, err = os.Stat(s.tmpRepoRoot + "/zzzzz-s0uqq-382brsig8rp3666/.git/refs/heads/newbranch")
c.Assert(err, check.Equals, nil)
}
func (s *IntegrationSuite) TestNonexistent(c *check.C) {
- // Spectator token
- os.Setenv("ARVADOS_API_TOKEN", "zw2f4gwx8hw8cjre7yp6v1zylhrhn3m5gvjq73rtpwhmknrybu")
- err := s.runGit(c, "fetch", "thisrepodoesnotexist.git")
+ err := s.runGit(c, spectatorToken, "fetch", "thisrepodoesnotexist.git")
c.Assert(err, check.ErrorMatches, `.* not found.*`)
}
func (s *IntegrationSuite) TestMissingGitdirReadableRepository(c *check.C) {
- // Active user token
- os.Setenv("ARVADOS_API_TOKEN", "3kg6k6lzmp9kj5cpkcoxie963cmvjahbt2fod9zru30k1jqdmi")
- err := s.runGit(c, "fetch", "active/foo2.git")
+ err := s.runGit(c, activeToken, "fetch", "active/foo2.git")
c.Assert(err, check.ErrorMatches, `.* not found.*`)
}
func (s *IntegrationSuite) TestNoPermission(c *check.C) {
- // Anonymous token
- os.Setenv("ARVADOS_API_TOKEN", "4kg6k6lzmp9kj4cpkcoxie964cmvjahbt4fod9zru44k4jqdmi")
for _, repo := range []string{"active/foo.git", "active/foo/.git"} {
- err := s.runGit(c, "fetch", repo)
+ err := s.runGit(c, anonymousToken, "fetch", repo)
c.Assert(err, check.ErrorMatches, `.* not found.*`)
}
}
@@ -107,18 +101,7 @@ func (s *IntegrationSuite) SetUpTest(c *check.C) {
// Clear ARVADOS_API_TOKEN after starting up the server, to
// make sure arv-git-httpd doesn't use it.
- os.Setenv("ARVADOS_API_TOKEN", "")
-
- _, err = exec.Command("git", "config",
- "--file", s.tmpWorkdir+"/.git/config",
- "credential.http://"+s.testServer.Addr+"/.helper",
- "!cred(){ echo password=$ARVADOS_API_TOKEN; };cred").Output()
- c.Assert(err, check.Equals, nil)
- _, err = exec.Command("git", "config",
- "--file", s.tmpWorkdir+"/.git/config",
- "credential.http://"+s.testServer.Addr+"/.username",
- "none").Output()
- c.Assert(err, check.Equals, nil)
+ os.Setenv("ARVADOS_API_TOKEN", "unused-token-placates-client-library")
}
func (s *IntegrationSuite) TearDownTest(c *check.C) {
@@ -137,14 +120,14 @@ func (s *IntegrationSuite) TearDownTest(c *check.C) {
}
}
-func (s *IntegrationSuite) runGit(c *check.C, gitCmd, repo string, args ...string) error {
+func (s *IntegrationSuite) runGit(c *check.C, token, gitCmd, repo string, args ...string) error {
cwd, err := os.Getwd()
c.Assert(err, check.Equals, nil)
defer os.Chdir(cwd)
os.Chdir(s.tmpWorkdir)
gitargs := append([]string{
- gitCmd, "http://" + s.testServer.Addr + "/" + repo,
+ gitCmd, "http://none:" + token + "@" + s.testServer.Addr + "/" + repo,
}, args...)
cmd := exec.Command("git", gitargs...)
w, err := cmd.StdinPipe()
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list