[ARVADOS] updated: 1.3.0-2187-g85cbbadc8
Git user
git at public.arvados.org
Mon Mar 2 22:52:25 UTC 2020
Summary of changes:
.../app/controllers/application_controller.rb | 1 +
.../app/controllers/collections_controller.rb | 2 +-
.../app/controllers/virtual_machines_controller.rb | 4 +-
.../application/{error.html.erb => error.text.erb} | 8 +-
.../app/views/users/_virtual_machines.html.erb | 2 +-
.../controllers/collections_controller_test.rb | 4 +-
build/run-tests.sh | 5 +-
lib/config/config.default.yml | 21 ++++
lib/config/export.go | 1 +
lib/config/generated_config.go | 21 ++++
lib/controller/federation/conn.go | 26 +++++
lib/controller/federation/login_test.go | 34 +++++++
lib/controller/handler.go | 1 +
lib/controller/handler_test.go | 47 +++++++++
lib/controller/localdb/conn.go | 9 ++
lib/controller/localdb/login.go | 12 +++
lib/controller/localdb/login_test.go | 6 ++
lib/controller/router/router.go | 7 ++
lib/controller/rpc/conn.go | 8 ++
lib/controller/rpc/conn_test.go | 10 ++
sdk/cwl/arvados_cwl/__init__.py | 37 +++++++
sdk/cwl/setup.py | 7 +-
.../sub.cwl => 16169-no-listing-hint.cwl} | 19 ++--
sdk/cwl/tests/arvados-tests.yml | 12 +++
.../tests/wf/{listing_none.cwl => 16169-step.cwl} | 8 +-
sdk/go/arvados/api.go | 7 ++
sdk/go/arvados/config.go | 1 +
sdk/go/arvados/login.go | 9 ++
sdk/go/arvadostest/api.go | 4 +
sdk/go/httpserver/logger.go | 10 +-
sdk/pam/setup.py | 4 +-
sdk/python/arvados/api.py | 1 +
sdk/python/arvados/commands/keepdocker.py | 2 +-
sdk/python/arvados/util.py | 8 ++
sdk/python/tests/run_test_server.py | 2 +-
services/fuse/arvados_fuse/__init__.py | 2 +-
services/fuse/arvados_fuse/command.py | 2 +-
services/fuse/arvados_fuse/fusedir.py | 113 ++++++++++++++-------
services/fuse/tests/test_mount.py | 76 ++++++++++++--
.../keep-web}/fpm-info.sh | 5 +-
services/keep-web/handler_test.go | 95 +++++++++++++----
services/keep-web/main.go | 5 +
services/nodemanager/setup.py | 4 +-
tools/crunchstat-summary/setup.py | 4 +-
44 files changed, 562 insertions(+), 104 deletions(-)
copy apps/workbench/app/views/application/{error.html.erb => error.text.erb} (59%)
copy sdk/cwl/tests/{secondary/sub.cwl => 16169-no-listing-hint.cwl} (58%)
copy sdk/cwl/tests/wf/{listing_none.cwl => 16169-step.cwl} (70%)
copy {tools/crunchstat-summary => services/keep-web}/fpm-info.sh (62%)
via 85cbbadc846bca172398a8be42f49ff8de91d2e9 (commit)
via b6ad6faad9a0750538cdd6ed68862d6c7d772b2d (commit)
via eb444cd34bf9b57a2844fe7ca4482f50ca094ee9 (commit)
via 0fd5ff742d62382275f1f74d663cfc00d5e0298d (commit)
via 77f2e73c03d263e1cda8ca2f07a35dbc53f6dd90 (commit)
via c5f7bfacd8d7b5e33239434fee3e98b7c364f49b (commit)
via c1777f201a5dee0f69f063dfb3a2287ffd789c97 (commit)
via 97302d14f8ace76ab6abb04f50d8952330b65cea (commit)
via cac986324a271a0e82cfb1e2bb51bafc9504eae2 (commit)
via 8da425a41233ce425f84e0e78166ac97358a5417 (commit)
via c229744b5941ca76138d527578dcc20cd98ce1c6 (commit)
via ff0bf85aeaf3eeaec8465394fb3748e06cfc2ac4 (commit)
via da2385723280b6deb1fcb58ecfbbf7cf952e930e (commit)
via a4b2ddad996a6ef81dcbc852bb03d319422f93b3 (commit)
via 137f2e31166c454d4536ba713347e8af3eb0176b (commit)
via cfd64e9f9714d755a80e96b71b7ab796c0710872 (commit)
via 2ad995236f3584635b7a2e80c62a31323b9e65b8 (commit)
via be568b1c98420f920c9f602d7e1aa47d401bcd39 (commit)
via 8a7a749b027a115c2cef269ab7fdfc85f40c2f63 (commit)
via 503160684305dccec44ad4cf309893404bf817e1 (commit)
from d6cac9806211dc5321cdc4fc24583f71e504cc84 (commit)
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 85cbbadc846bca172398a8be42f49ff8de91d2e9
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Fri Feb 28 12:07:35 2020 -0500
16169: Monkey patch load_tool.resolve_and_validate_document to fix bug
There is a bug in upstream cwltool where the version updater needs to
replace the document fragments in the loader index with the updated
ones, but actually it only does it for the root document. Normally we
just fix the bug in upstream but that's challenging because current
cwltool dropped support for Python 2.7 and we're still supporting py2
in Arvados 2.0 (although py2 support will most likely be dropped in
Arvados 2.1). Making a bugfix fork comes with its own
complications (it would need to be added to PyPi) so monkey patching
is the least disruptive fix (and is relatively safe because our
cwltool dependency is pinned to a specific version). This
should be removed as soon as a bugfix goes into upstream cwltool and
we upgrade to it.
diff --git a/sdk/cwl/arvados_cwl/__init__.py b/sdk/cwl/arvados_cwl/__init__.py
index 3dd04040a..2b2acd568 100644
--- a/sdk/cwl/arvados_cwl/__init__.py
+++ b/sdk/cwl/arvados_cwl/__init__.py
@@ -16,6 +16,43 @@ import sys
import re
import pkg_resources # part of setuptools
+### begin monkey patch ###
+# Monkey patch solution for bug #16169
+#
+# There is a bug in upstream cwltool where the version updater needs
+# to replace the document fragments in the loader index with the
+# updated ones, but actually it only does it for the root document.
+# Normally we just fix the bug in upstream but that's challenging
+# because current cwltool dropped support for Python 2.7 and we're
+# still supporting py2 in Arvados 2.0 (although py2 support will most
+# likely be dropped in Arvados 2.1). Making a bugfix fork comes with
+# its own complications (it would need to be added to PyPi) so monkey
+# patching is the least disruptive fix (and is relatively safe because
+# our cwltool dependency is pinned to a specific version). This
+# should be removed as soon as a bugfix goes into upstream cwltool and
+# we upgrade to it.
+#
+import cwltool.load_tool
+from cwltool.utils import visit_class
+from six.moves import urllib
+original_resolve_and_validate_document = cwltool.load_tool.resolve_and_validate_document
+def wrapped_resolve_and_validate_document(
+ loadingContext, # type: LoadingContext
+ workflowobj, # type: Union[CommentedMap, CommentedSeq]
+ uri, # type: Text
+ preprocess_only=False, # type: bool
+ skip_schemas=None, # type: Optional[bool]
+ ):
+ loadingContext, uri = original_resolve_and_validate_document(loadingContext, workflowobj, uri, preprocess_only, skip_schemas)
+ if loadingContext.do_update in (True, None):
+ fileuri = urllib.parse.urldefrag(uri)[0]
+ def update_index(pr):
+ loadingContext.loader.idx[pr["id"]] = pr
+ visit_class(loadingContext.loader.idx[fileuri], ("CommandLineTool", "Workflow", "ExpressionTool"), update_index)
+ return loadingContext, uri
+cwltool.load_tool.resolve_and_validate_document = wrapped_resolve_and_validate_document
+### end monkey patch ###
+
from schema_salad.sourceline import SourceLine
import schema_salad.validate as validate
import cwltool.main
diff --git a/sdk/cwl/tests/wf/16169-step.cwl b/sdk/cwl/tests/wf/16169-step.cwl
index 8386df517..ce6f2c0c9 100644
--- a/sdk/cwl/tests/wf/16169-step.cwl
+++ b/sdk/cwl/tests/wf/16169-step.cwl
@@ -6,10 +6,12 @@ class: CommandLineTool
cwlVersion: v1.0
requirements:
InlineJavascriptRequirement: {}
+ DockerRequirement:
+ dockerPull: debian:stretch-slim
inputs:
d: Directory
outputs:
out: stdout
stdout: output.txt
arguments:
- [echo, "${if(inputs.d.listing === undefined) {return 'true';} else {return 'false';}}"]
\ No newline at end of file
+ [echo, "${if(inputs.d.listing === undefined) {return 'true';} else {return 'false';}}"]
diff --git a/sdk/python/arvados/commands/keepdocker.py b/sdk/python/arvados/commands/keepdocker.py
index c89fa644c..6673888ab 100644
--- a/sdk/python/arvados/commands/keepdocker.py
+++ b/sdk/python/arvados/commands/keepdocker.py
@@ -390,7 +390,7 @@ def main(arguments=None, stdout=sys.stdout, install_sig_handlers=True, api=None)
try:
image_hash = find_one_image_hash(args.image, args.tag)
except DockerError as error:
- logger.error(error.message)
+ logger.error(str(error))
sys.exit(1)
if not docker_image_compatible(api, image_hash):
commit b6ad6faad9a0750538cdd6ed68862d6c7d772b2d
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date: Wed Feb 26 20:58:59 2020 -0300
16169: Adds test exposing the requirement propagation bug.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>
diff --git a/sdk/cwl/tests/16169-no-listing-hint.cwl b/sdk/cwl/tests/16169-no-listing-hint.cwl
new file mode 100644
index 000000000..fe4f991ff
--- /dev/null
+++ b/sdk/cwl/tests/16169-no-listing-hint.cwl
@@ -0,0 +1,24 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: Apache-2.0
+
+cwlVersion: v1.0
+class: Workflow
+$namespaces:
+ arv: "http://arvados.org/cwl#"
+ cwltool: "http://commonwl.org/cwltool#"
+requirements:
+ cwltool:LoadListingRequirement:
+ loadListing: no_listing
+inputs:
+ d: Directory
+steps:
+ step1:
+ in:
+ d: d
+ out: [out]
+ run: wf/16169-step.cwl
+outputs:
+ out:
+ type: File
+ outputSource: step1/out
diff --git a/sdk/cwl/tests/arvados-tests.yml b/sdk/cwl/tests/arvados-tests.yml
index 99aee3795..df9fac842 100644
--- a/sdk/cwl/tests/arvados-tests.yml
+++ b/sdk/cwl/tests/arvados-tests.yml
@@ -310,3 +310,15 @@
should_fail: true
tool: 15295-bad-keep-ref.cwl
doc: Test checking for invalid keepref
+
+- job: listing-job.yml
+ output: {
+ "out": {
+ "class": "File",
+ "location": "output.txt",
+ "size": 5,
+ "checksum": "sha1$724ba28f4a9a1b472057ff99511ed393a45552e1"
+ }
+ }
+ tool: 16169-no-listing-hint.cwl
+ doc: "Test cwltool:LoadListingRequirement propagation"
diff --git a/sdk/cwl/tests/wf/16169-step.cwl b/sdk/cwl/tests/wf/16169-step.cwl
new file mode 100644
index 000000000..8386df517
--- /dev/null
+++ b/sdk/cwl/tests/wf/16169-step.cwl
@@ -0,0 +1,15 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: Apache-2.0
+
+class: CommandLineTool
+cwlVersion: v1.0
+requirements:
+ InlineJavascriptRequirement: {}
+inputs:
+ d: Directory
+outputs:
+ out: stdout
+stdout: output.txt
+arguments:
+ [echo, "${if(inputs.d.listing === undefined) {return 'true';} else {return 'false';}}"]
\ No newline at end of file
commit eb444cd34bf9b57a2844fe7ca4482f50ca094ee9
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Thu Feb 27 12:21:34 2020 -0500
16177: Fix tests, test server config now has TrustAllContent: false
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/apps/workbench/test/controllers/collections_controller_test.rb b/apps/workbench/test/controllers/collections_controller_test.rb
index 4fce54a8a..a95b64994 100644
--- a/apps/workbench/test/controllers/collections_controller_test.rb
+++ b/apps/workbench/test/controllers/collections_controller_test.rb
@@ -532,7 +532,7 @@ class CollectionsControllerTest < ActionController::TestCase
end
test "Redirect to keep_web_url via #{id_type} when trust_all_content enabled" do
- Rails.configuration.Workbench.TrustAllContent = true
+ Rails.configuration.Collections.TrustAllContent = true
setup_for_keep_web('https://collections.example',
'https://download.example')
tok = api_token('active')
@@ -583,7 +583,7 @@ class CollectionsControllerTest < ActionController::TestCase
[false, true].each do |trust_all_content|
test "Redirect preview to keep_web_download_url when preview is disabled and trust_all_content is #{trust_all_content}" do
- Rails.configuration.Workbench.TrustAllContent = trust_all_content
+ Rails.configuration.Collections.TrustAllContent = trust_all_content
setup_for_keep_web "", 'https://download.example/'
tok = api_token('active')
id = api_fixture('collections')['w_a_z_file']['uuid']
diff --git a/sdk/python/tests/run_test_server.py b/sdk/python/tests/run_test_server.py
index 9e9b12f98..f5528081f 100644
--- a/sdk/python/tests/run_test_server.py
+++ b/sdk/python/tests/run_test_server.py
@@ -737,7 +737,7 @@ def setup_config():
},
"Collections": {
"BlobSigningKey": "zfhgfenhffzltr9dixws36j1yhksjoll2grmku38mi7yxd66h5j4q9w4jzanezacp8s6q0ro3hxakfye02152hncy6zml2ed0uc",
- "TrustAllContent": True,
+ "TrustAllContent": False,
"ForwardSlashNameSubstitution": "/",
},
"Git": {
commit 0fd5ff742d62382275f1f74d663cfc00d5e0298d
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Wed Feb 26 15:31:58 2020 -0500
16177: Use correct config item Collections.TrustAllContent
Also added fallback error handler, if view or download link doesn't
work, render error text (otherwise you get a very confusing Rails
crash page).
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/apps/workbench/app/controllers/application_controller.rb b/apps/workbench/app/controllers/application_controller.rb
index 540770061..8d6f897bb 100644
--- a/apps/workbench/app/controllers/application_controller.rb
+++ b/apps/workbench/app/controllers/application_controller.rb
@@ -62,6 +62,7 @@ class ApplicationController < ActionController::Base
# the browser can't.
f.json { render opts.merge(json: {success: false, errors: @errors}) }
f.html { render({action: 'error'}.merge(opts)) }
+ f.all { render({action: 'error', formats: 'text'}.merge(opts)) }
end
end
diff --git a/apps/workbench/app/controllers/collections_controller.rb b/apps/workbench/app/controllers/collections_controller.rb
index 10e026ae6..9073d06c1 100644
--- a/apps/workbench/app/controllers/collections_controller.rb
+++ b/apps/workbench/app/controllers/collections_controller.rb
@@ -343,7 +343,7 @@ class CollectionsController < ApplicationController
# Prefer the attachment-only-host when we want an attachment
# (and when there is no preview link configured)
tmpl = Rails.configuration.Services.WebDAVDownload.ExternalURL.to_s
- elsif not Rails.configuration.Workbench.TrustAllContent
+ elsif not Rails.configuration.Collections.TrustAllContent
check_uri = URI.parse(tmpl.sub("*", munged_id))
if opts[:query_token] and
(check_uri.host.nil? or (
diff --git a/apps/workbench/app/views/application/error.text.erb b/apps/workbench/app/views/application/error.text.erb
new file mode 100644
index 000000000..103518274
--- /dev/null
+++ b/apps/workbench/app/views/application/error.text.erb
@@ -0,0 +1,11 @@
+<%# Copyright (C) The Arvados Authors. All rights reserved.
+
+SPDX-License-Identifier: AGPL-3.0 %>
+
+Oh... fiddlesticks.
+
+Sorry, I had some trouble handling your request.
+
+<% if @errors.is_a? Array then @errors.each do |error| %>
+<%= error %>
+<% end end %>
commit 77f2e73c03d263e1cda8ca2f07a35dbc53f6dd90
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Wed Feb 26 16:12:22 2020 -0500
16202: Add Workbench.SSHHelpHostSuffix
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/apps/workbench/app/views/users/_virtual_machines.html.erb b/apps/workbench/app/views/users/_virtual_machines.html.erb
index 38458593f..e2ce5b39b 100644
--- a/apps/workbench/app/views/users/_virtual_machines.html.erb
+++ b/apps/workbench/app/views/users/_virtual_machines.html.erb
@@ -85,7 +85,7 @@ SPDX-License-Identifier: AGPL-3.0 %>
<td style="word-break:break-all;">
<% if @my_vm_logins[vm[:uuid]] %>
<% @my_vm_logins[vm[:uuid]].each do |login| %>
- <code>ssh <%= login %>@<%= vm[:hostname] %></code>
+ <code>ssh <%= login %>@<%= vm[:hostname] %><%=Rails.configuration.Workbench.SSHHelpHostSuffix%></code>
<% end %>
<% end %>
</td>
diff --git a/lib/config/config.default.yml b/lib/config/config.default.yml
index 41af15073..3750adcab 100644
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -1169,6 +1169,27 @@ Clusters:
<a href="https://doc.arvados.org/user/getting_started/ssh-access-unix.html">Accessing an Arvados VM with SSH</a> (generic instructions).
Site configurations vary. Contact your local cluster administrator if you have difficulty accessing an Arvados shell node.
+ # Sample text if you are using a "switchyard" ssh proxy.
+ # Replace "zzzzz" with your Cluster ID.
+ #SSHHelpPageHTML: |
+ # <p>Add a section like this to your SSH configuration file ( <i>~/.ssh/config</i>):</p>
+ # <pre>Host *.zzzzz
+ # TCPKeepAlive yes
+ # ServerAliveInterval 60
+ # ProxyCommand ssh -p2222 turnout at switchyard.zzzzz.arvadosapi.com -x -a $SSH_PROXY_FLAGS %h
+ # </pre>
+
+ # If you are using a switchyard ssh proxy, shell node hostnames
+ # may require a special hostname suffix. In the sample ssh
+ # configuration above, this would be ".zzzzz"
+ # This is added to the hostname in the "command line" column
+ # the Workbench "shell VMs" page.
+ #
+ # If your shell nodes are directly accessible by users without a
+ # proxy and have fully qualified host names, you should leave
+ # this blank.
+ SSHHelpHostSuffix: ""
+
# Bypass new (Arvados 1.5) API implementations, and hand off
# requests directly to Rails instead. This can provide a temporary
# workaround for clients that are incompatible with the new API
diff --git a/lib/config/export.go b/lib/config/export.go
index 44c69b6e2..88794140a 100644
--- a/lib/config/export.go
+++ b/lib/config/export.go
@@ -215,6 +215,7 @@ var whitelist = map[string]bool{
"Workbench.WelcomePageHTML": true,
"Workbench.InactivePageHTML": true,
"Workbench.SSHHelpPageHTML": true,
+ "Workbench.SSHHelpHostSuffix": true,
}
func redactUnsafe(m map[string]interface{}, mPrefix, lookupPrefix string) error {
diff --git a/lib/config/generated_config.go b/lib/config/generated_config.go
index 25fa89394..6813bee40 100644
--- a/lib/config/generated_config.go
+++ b/lib/config/generated_config.go
@@ -1175,6 +1175,27 @@ Clusters:
<a href="https://doc.arvados.org/user/getting_started/ssh-access-unix.html">Accessing an Arvados VM with SSH</a> (generic instructions).
Site configurations vary. Contact your local cluster administrator if you have difficulty accessing an Arvados shell node.
+ # Sample text if you are using a "switchyard" ssh proxy.
+ # Replace "zzzzz" with your Cluster ID.
+ #SSHHelpPageHTML: |
+ # <p>Add a section like this to your SSH configuration file ( <i>~/.ssh/config</i>):</p>
+ # <pre>Host *.zzzzz
+ # TCPKeepAlive yes
+ # ServerAliveInterval 60
+ # ProxyCommand ssh -p2222 turnout at switchyard.zzzzz.arvadosapi.com -x -a $SSH_PROXY_FLAGS %h
+ # </pre>
+
+ # If you are using a switchyard ssh proxy, shell node hostnames
+ # may require a special hostname suffix. In the sample ssh
+ # configuration above, this would be ".zzzzz"
+ # This is added to the hostname in the "command line" column
+ # the Workbench "shell VMs" page.
+ #
+ # If your shell nodes are directly accessible by users without a
+ # proxy and have fully qualified host names, you should leave
+ # this blank.
+ SSHHelpHostSuffix: ""
+
# Bypass new (Arvados 1.5) API implementations, and hand off
# requests directly to Rails instead. This can provide a temporary
# workaround for clients that are incompatible with the new API
diff --git a/sdk/go/arvados/config.go b/sdk/go/arvados/config.go
index 176f1dd2a..a70980cbd 100644
--- a/sdk/go/arvados/config.go
+++ b/sdk/go/arvados/config.go
@@ -215,6 +215,7 @@ type Cluster struct {
WelcomePageHTML string
InactivePageHTML string
SSHHelpPageHTML string
+ SSHHelpHostSuffix string
}
ForceLegacyAPI14 bool
commit c5f7bfacd8d7b5e33239434fee3e98b7c364f49b
Author: Ward Vandewege <ward at jhvc.com>
Date: Wed Feb 26 13:49:28 2020 -0500
Fix web shell.
refs #16203
Arvados-DCO-1.1-Signed-off-by: Ward Vandewege <ward at jhvc.com>
diff --git a/apps/workbench/app/controllers/virtual_machines_controller.rb b/apps/workbench/app/controllers/virtual_machines_controller.rb
index 1427e3cc7..c74377314 100644
--- a/apps/workbench/app/controllers/virtual_machines_controller.rb
+++ b/apps/workbench/app/controllers/virtual_machines_controller.rb
@@ -25,8 +25,8 @@ class VirtualMachinesController < ApplicationController
end
def webshell
- return render_not_found if Rails.configuration.Workbench.ShellInABoxURL == URI("")
- webshell_url = URI(Rails.configuration.Workbench.ShellInABoxURL)
+ return render_not_found if Rails.configuration.Services.WebShell.ExternalURL == URI("")
+ webshell_url = URI(Rails.configuration.Services.WebShell.ExternalURL)
if webshell_url.host.index("*") != nil
webshell_url.host = webshell_url.host.sub("*", @object.hostname)
else
commit c1777f201a5dee0f69f063dfb3a2287ffd789c97
Author: Tom Clegg <tom at tomclegg.ca>
Date: Tue Feb 25 12:14:32 2020 -0500
15954: Propagate remote param.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>
diff --git a/lib/controller/handler_test.go b/lib/controller/handler_test.go
index 6b1fa1a0b..f09203f72 100644
--- a/lib/controller/handler_test.go
+++ b/lib/controller/handler_test.go
@@ -19,6 +19,7 @@ import (
"git.arvados.org/arvados.git/sdk/go/arvados"
"git.arvados.org/arvados.git/sdk/go/arvadostest"
+ "git.arvados.org/arvados.git/sdk/go/auth"
"git.arvados.org/arvados.git/sdk/go/ctxlog"
"git.arvados.org/arvados.git/sdk/go/httpserver"
"github.com/prometheus/client_golang/prometheus"
@@ -229,6 +230,26 @@ func (s *HandlerSuite) TestValidateV2APIToken(c *check.C) {
c.Check(user.Authorization.TokenV2(), check.Equals, arvadostest.ActiveTokenV2)
}
+func (s *HandlerSuite) TestValidateRemoteToken(c *check.C) {
+ saltedToken, err := auth.SaltToken(arvadostest.ActiveTokenV2, "abcde")
+ c.Assert(err, check.IsNil)
+ for _, trial := range []struct {
+ code int
+ token string
+ }{
+ {http.StatusOK, saltedToken},
+ {http.StatusUnauthorized, "bogus"},
+ } {
+ req := httptest.NewRequest("GET", "https://0.0.0.0:1/arvados/v1/users/current?remote=abcde", nil)
+ req.Header.Set("Authorization", "Bearer "+trial.token)
+ resp := httptest.NewRecorder()
+ s.handler.ServeHTTP(resp, req)
+ if !c.Check(resp.Code, check.Equals, trial.code) {
+ c.Logf("HTTP %d: %s", resp.Code, resp.Body.String())
+ }
+ }
+}
+
func (s *HandlerSuite) TestCreateAPIToken(c *check.C) {
req := httptest.NewRequest("GET", "/arvados/v1/users/current", nil)
auth, err := s.handler.(*Handler).createAPItoken(req, arvadostest.ActiveUserUUID, nil)
diff --git a/sdk/go/arvados/api.go b/sdk/go/arvados/api.go
index 5c8d4f629..0c5d32e8b 100644
--- a/sdk/go/arvados/api.go
+++ b/sdk/go/arvados/api.go
@@ -64,6 +64,7 @@ type GetOptions struct {
Select []string `json:"select"`
IncludeTrash bool `json:"include_trash"`
ForwardedFor string `json:"forwarded_for"`
+ Remote string `json:"remote"`
}
type UntrashOptions struct {
commit 97302d14f8ace76ab6abb04f50d8952330b65cea
Author: Tom Clegg <tom at tomclegg.ca>
Date: Thu Feb 20 15:43:50 2020 -0500
16101: Redirect logout to Workbench if return_to param missing.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>
diff --git a/lib/controller/federation/login_test.go b/lib/controller/federation/login_test.go
index 3cc5cb842..1d6e12e01 100644
--- a/lib/controller/federation/login_test.go
+++ b/lib/controller/federation/login_test.go
@@ -41,26 +41,30 @@ func (s *LoginSuite) TestDeferToLoginCluster(c *check.C) {
}
func (s *LoginSuite) TestLogout(c *check.C) {
+ s.cluster.Services.Workbench1.ExternalURL = arvados.URL{Scheme: "https", Host: "workbench1.example.com"}
+ s.cluster.Services.Workbench2.ExternalURL = arvados.URL{Scheme: "https", Host: "workbench2.example.com"}
s.cluster.Login.GoogleClientID = "zzzzzzzzzzzzzz"
s.addHTTPRemote(c, "zhome", &arvadostest.APIStub{})
s.cluster.Login.LoginCluster = "zhome"
returnTo := "https://app.example.com/foo?bar"
for _, trial := range []struct {
- token string
- target string
+ token string
+ returnTo string
+ target string
}{
- {token: "", target: returnTo},
- {token: "zzzzzzzzzzzzzzzzzzzzz", target: returnTo},
- {token: "v2/zzzzz-aaaaa-aaaaaaaaaaaaaaa/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", target: returnTo},
- {token: "v2/zhome-aaaaa-aaaaaaaaaaaaaaa/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", target: "http://" + s.cluster.RemoteClusters["zhome"].Host + "/logout?" + url.Values{"return_to": {returnTo}}.Encode()},
+ {token: "", returnTo: "", target: s.cluster.Services.Workbench2.ExternalURL.String()},
+ {token: "", returnTo: returnTo, target: returnTo},
+ {token: "zzzzzzzzzzzzzzzzzzzzz", returnTo: returnTo, target: returnTo},
+ {token: "v2/zzzzz-aaaaa-aaaaaaaaaaaaaaa/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", returnTo: returnTo, target: returnTo},
+ {token: "v2/zhome-aaaaa-aaaaaaaaaaaaaaa/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", returnTo: returnTo, target: "http://" + s.cluster.RemoteClusters["zhome"].Host + "/logout?" + url.Values{"return_to": {returnTo}}.Encode()},
} {
c.Logf("trial %#v", trial)
ctx := context.Background()
if trial.token != "" {
ctx = auth.NewContext(ctx, &auth.Credentials{Tokens: []string{trial.token}})
}
- resp, err := s.fed.Logout(ctx, arvados.LogoutOptions{ReturnTo: returnTo})
+ resp, err := s.fed.Logout(ctx, arvados.LogoutOptions{ReturnTo: trial.returnTo})
c.Assert(err, check.IsNil)
c.Logf(" RedirectLocation %q", resp.RedirectLocation)
target, err := url.Parse(resp.RedirectLocation)
diff --git a/lib/controller/localdb/login.go b/lib/controller/localdb/login.go
index e96b940ef..2e50b84f4 100644
--- a/lib/controller/localdb/login.go
+++ b/lib/controller/localdb/login.go
@@ -53,7 +53,15 @@ func (ctrl *googleLoginController) getProvider() (*oidc.Provider, error) {
}
func (ctrl *googleLoginController) Logout(ctx context.Context, cluster *arvados.Cluster, railsproxy *railsProxy, opts arvados.LogoutOptions) (arvados.LogoutResponse, error) {
- return arvados.LogoutResponse{RedirectLocation: opts.ReturnTo}, nil
+ target := opts.ReturnTo
+ if target == "" {
+ if cluster.Services.Workbench2.ExternalURL.Host != "" {
+ target = cluster.Services.Workbench2.ExternalURL.String()
+ } else {
+ target = cluster.Services.Workbench1.ExternalURL.String()
+ }
+ }
+ return arvados.LogoutResponse{RedirectLocation: target}, nil
}
func (ctrl *googleLoginController) Login(ctx context.Context, cluster *arvados.Cluster, railsproxy *railsProxy, opts arvados.LoginOptions) (arvados.LoginResponse, error) {
commit cac986324a271a0e82cfb1e2bb51bafc9504eae2
Author: Tom Clegg <tom at tomclegg.ca>
Date: Mon Feb 17 13:55:27 2020 -0500
16101: Handle logout without sso-provider.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>
diff --git a/lib/controller/federation/conn.go b/lib/controller/federation/conn.go
index 42083cb83..56f117ee7 100644
--- a/lib/controller/federation/conn.go
+++ b/lib/controller/federation/conn.go
@@ -222,6 +222,32 @@ func (conn *Conn) Login(ctx context.Context, options arvados.LoginOptions) (arva
}
}
+func (conn *Conn) Logout(ctx context.Context, options arvados.LogoutOptions) (arvados.LogoutResponse, error) {
+ // If the logout request comes with an API token from a known
+ // remote cluster, redirect to that cluster's logout handler
+ // so it has an opportunity to clear sessions, expire tokens,
+ // etc. Otherwise use the local endpoint.
+ reqauth, ok := auth.FromContext(ctx)
+ if !ok || len(reqauth.Tokens) == 0 || len(reqauth.Tokens[0]) < 8 || !strings.HasPrefix(reqauth.Tokens[0], "v2/") {
+ return conn.local.Logout(ctx, options)
+ }
+ id := reqauth.Tokens[0][3:8]
+ if id == conn.cluster.ClusterID {
+ return conn.local.Logout(ctx, options)
+ }
+ remote, ok := conn.remotes[id]
+ if !ok {
+ return conn.local.Logout(ctx, options)
+ }
+ baseURL := remote.BaseURL()
+ target, err := baseURL.Parse(arvados.EndpointLogout.Path)
+ if err != nil {
+ return arvados.LogoutResponse{}, fmt.Errorf("internal error getting redirect target: %s", err)
+ }
+ target.RawQuery = url.Values{"return_to": {options.ReturnTo}}.Encode()
+ return arvados.LogoutResponse{RedirectLocation: target.String()}, nil
+}
+
func (conn *Conn) CollectionGet(ctx context.Context, options arvados.GetOptions) (arvados.Collection, error) {
if len(options.UUID) == 27 {
// UUID is really a UUID
diff --git a/lib/controller/federation/login_test.go b/lib/controller/federation/login_test.go
index ab39619c7..3cc5cb842 100644
--- a/lib/controller/federation/login_test.go
+++ b/lib/controller/federation/login_test.go
@@ -10,6 +10,7 @@ import (
"git.arvados.org/arvados.git/sdk/go/arvados"
"git.arvados.org/arvados.git/sdk/go/arvadostest"
+ "git.arvados.org/arvados.git/sdk/go/auth"
check "gopkg.in/check.v1"
)
@@ -38,3 +39,32 @@ func (s *LoginSuite) TestDeferToLoginCluster(c *check.C) {
c.Check(remotePresent, check.Equals, remote != "")
}
}
+
+func (s *LoginSuite) TestLogout(c *check.C) {
+ s.cluster.Login.GoogleClientID = "zzzzzzzzzzzzzz"
+ s.addHTTPRemote(c, "zhome", &arvadostest.APIStub{})
+ s.cluster.Login.LoginCluster = "zhome"
+
+ returnTo := "https://app.example.com/foo?bar"
+ for _, trial := range []struct {
+ token string
+ target string
+ }{
+ {token: "", target: returnTo},
+ {token: "zzzzzzzzzzzzzzzzzzzzz", target: returnTo},
+ {token: "v2/zzzzz-aaaaa-aaaaaaaaaaaaaaa/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", target: returnTo},
+ {token: "v2/zhome-aaaaa-aaaaaaaaaaaaaaa/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", target: "http://" + s.cluster.RemoteClusters["zhome"].Host + "/logout?" + url.Values{"return_to": {returnTo}}.Encode()},
+ } {
+ c.Logf("trial %#v", trial)
+ ctx := context.Background()
+ if trial.token != "" {
+ ctx = auth.NewContext(ctx, &auth.Credentials{Tokens: []string{trial.token}})
+ }
+ resp, err := s.fed.Logout(ctx, arvados.LogoutOptions{ReturnTo: returnTo})
+ c.Assert(err, check.IsNil)
+ c.Logf(" RedirectLocation %q", resp.RedirectLocation)
+ target, err := url.Parse(resp.RedirectLocation)
+ c.Check(err, check.IsNil)
+ c.Check(target.String(), check.Equals, trial.target)
+ }
+}
diff --git a/lib/controller/handler.go b/lib/controller/handler.go
index 935a1b6cb..e3869261a 100644
--- a/lib/controller/handler.go
+++ b/lib/controller/handler.go
@@ -86,6 +86,7 @@ func (h *Handler) setup() {
mux.Handle("/arvados/v1/users", rtr)
mux.Handle("/arvados/v1/users/", rtr)
mux.Handle("/login", rtr)
+ mux.Handle("/logout", rtr)
}
hs := http.NotFoundHandler()
diff --git a/lib/controller/handler_test.go b/lib/controller/handler_test.go
index d54f50cf1..6b1fa1a0b 100644
--- a/lib/controller/handler_test.go
+++ b/lib/controller/handler_test.go
@@ -180,6 +180,32 @@ func (s *HandlerSuite) TestProxyRedirect(c *check.C) {
c.Check(resp.Header().Get("Location"), check.Matches, `(https://0.0.0.0:1)?/auth/joshid\?return_to=%2Cfoo&?`)
}
+func (s *HandlerSuite) TestLogoutSSO(c *check.C) {
+ s.cluster.Login.ProviderAppID = "test"
+ req := httptest.NewRequest("GET", "https://0.0.0.0:1/logout?return_to=https://example.com/foo", nil)
+ resp := httptest.NewRecorder()
+ s.handler.ServeHTTP(resp, req)
+ if !c.Check(resp.Code, check.Equals, http.StatusFound) {
+ c.Log(resp.Body.String())
+ }
+ c.Check(resp.Header().Get("Location"), check.Equals, "http://localhost:3002/users/sign_out?"+url.Values{"redirect_uri": {"https://example.com/foo"}}.Encode())
+}
+
+func (s *HandlerSuite) TestLogoutGoogle(c *check.C) {
+ if s.cluster.ForceLegacyAPI14 {
+ // Google login N/A
+ return
+ }
+ s.cluster.Login.GoogleClientID = "test"
+ req := httptest.NewRequest("GET", "https://0.0.0.0:1/logout?return_to=https://example.com/foo", nil)
+ resp := httptest.NewRecorder()
+ s.handler.ServeHTTP(resp, req)
+ if !c.Check(resp.Code, check.Equals, http.StatusFound) {
+ c.Log(resp.Body.String())
+ }
+ c.Check(resp.Header().Get("Location"), check.Equals, "https://example.com/foo")
+}
+
func (s *HandlerSuite) TestValidateV1APIToken(c *check.C) {
req := httptest.NewRequest("GET", "/arvados/v1/users/current", nil)
user, ok, err := s.handler.(*Handler).validateAPItoken(req, arvadostest.ActiveToken)
diff --git a/lib/controller/localdb/conn.go b/lib/controller/localdb/conn.go
index 4139b270c..ac092382d 100644
--- a/lib/controller/localdb/conn.go
+++ b/lib/controller/localdb/conn.go
@@ -29,6 +29,15 @@ func NewConn(cluster *arvados.Cluster) *Conn {
}
}
+func (conn *Conn) Logout(ctx context.Context, opts arvados.LogoutOptions) (arvados.LogoutResponse, error) {
+ if conn.cluster.Login.ProviderAppID != "" {
+ // Proxy to RailsAPI, which hands off to sso-provider.
+ return conn.railsProxy.Logout(ctx, opts)
+ } else {
+ return conn.googleLoginController.Logout(ctx, conn.cluster, conn.railsProxy, opts)
+ }
+}
+
func (conn *Conn) Login(ctx context.Context, opts arvados.LoginOptions) (arvados.LoginResponse, error) {
wantGoogle := conn.cluster.Login.GoogleClientID != ""
wantSSO := conn.cluster.Login.ProviderAppID != ""
diff --git a/lib/controller/localdb/login.go b/lib/controller/localdb/login.go
index b1ebb27e4..e96b940ef 100644
--- a/lib/controller/localdb/login.go
+++ b/lib/controller/localdb/login.go
@@ -52,6 +52,10 @@ func (ctrl *googleLoginController) getProvider() (*oidc.Provider, error) {
return ctrl.provider, nil
}
+func (ctrl *googleLoginController) Logout(ctx context.Context, cluster *arvados.Cluster, railsproxy *railsProxy, opts arvados.LogoutOptions) (arvados.LogoutResponse, error) {
+ return arvados.LogoutResponse{RedirectLocation: opts.ReturnTo}, nil
+}
+
func (ctrl *googleLoginController) Login(ctx context.Context, cluster *arvados.Cluster, railsproxy *railsProxy, opts arvados.LoginOptions) (arvados.LoginResponse, error) {
provider, err := ctrl.getProvider()
if err != nil {
diff --git a/lib/controller/localdb/login_test.go b/lib/controller/localdb/login_test.go
index 9f3267cef..4fb0fbcee 100644
--- a/lib/controller/localdb/login_test.go
+++ b/lib/controller/localdb/login_test.go
@@ -163,6 +163,12 @@ func (s *LoginSuite) TearDownTest(c *check.C) {
s.railsSpy.Close()
}
+func (s *LoginSuite) TestGoogleLogout(c *check.C) {
+ resp, err := s.localdb.Logout(context.Background(), arvados.LogoutOptions{ReturnTo: "https://foo.example.com/bar"})
+ c.Check(err, check.IsNil)
+ c.Check(resp.RedirectLocation, check.Equals, "https://foo.example.com/bar")
+}
+
func (s *LoginSuite) TestGoogleLogin_Start_Bogus(c *check.C) {
resp, err := s.localdb.Login(context.Background(), arvados.LoginOptions{})
c.Check(err, check.IsNil)
diff --git a/lib/controller/router/router.go b/lib/controller/router/router.go
index c41f103dc..69d707703 100644
--- a/lib/controller/router/router.go
+++ b/lib/controller/router/router.go
@@ -54,6 +54,13 @@ func (rtr *router) addRoutes() {
return rtr.fed.Login(ctx, *opts.(*arvados.LoginOptions))
},
},
+ {
+ arvados.EndpointLogout,
+ func() interface{} { return &arvados.LogoutOptions{} },
+ func(ctx context.Context, opts interface{}) (interface{}, error) {
+ return rtr.fed.Logout(ctx, *opts.(*arvados.LogoutOptions))
+ },
+ },
{
arvados.EndpointCollectionCreate,
func() interface{} { return &arvados.CreateOptions{} },
diff --git a/lib/controller/rpc/conn.go b/lib/controller/rpc/conn.go
index bf6166f44..4b143b770 100644
--- a/lib/controller/rpc/conn.go
+++ b/lib/controller/rpc/conn.go
@@ -145,6 +145,14 @@ func (conn *Conn) Login(ctx context.Context, options arvados.LoginOptions) (arva
return resp, err
}
+func (conn *Conn) Logout(ctx context.Context, options arvados.LogoutOptions) (arvados.LogoutResponse, error) {
+ ep := arvados.EndpointLogout
+ var resp arvados.LogoutResponse
+ err := conn.requestAndDecode(ctx, &resp, ep, nil, options)
+ resp.RedirectLocation = conn.relativeToBaseURL(resp.RedirectLocation)
+ return resp, err
+}
+
// If the given location is a valid URL and its origin is the same as
// conn.baseURL, return it as a relative URL. Otherwise, return it
// unmodified.
diff --git a/lib/controller/rpc/conn_test.go b/lib/controller/rpc/conn_test.go
index 83a80e878..b97c0f87b 100644
--- a/lib/controller/rpc/conn_test.go
+++ b/lib/controller/rpc/conn_test.go
@@ -51,6 +51,16 @@ func (s *RPCSuite) TestLogin(c *check.C) {
c.Check(resp.RedirectLocation, check.Equals, "/auth/joshid?return_to="+url.QueryEscape(","+opts.ReturnTo))
}
+func (s *RPCSuite) TestLogout(c *check.C) {
+ s.ctx = context.Background()
+ opts := arvados.LogoutOptions{
+ ReturnTo: "https://foo.example.com/bar",
+ }
+ resp, err := s.conn.Logout(s.ctx, opts)
+ c.Check(err, check.IsNil)
+ c.Check(resp.RedirectLocation, check.Equals, "http://localhost:3002/users/sign_out?redirect_uri="+url.QueryEscape(opts.ReturnTo))
+}
+
func (s *RPCSuite) TestCollectionCreate(c *check.C) {
coll, err := s.conn.CollectionCreate(s.ctx, arvados.CreateOptions{Attrs: map[string]interface{}{
"owner_uuid": arvadostest.ActiveUserUUID,
diff --git a/sdk/go/arvados/api.go b/sdk/go/arvados/api.go
index aa670c539..5c8d4f629 100644
--- a/sdk/go/arvados/api.go
+++ b/sdk/go/arvados/api.go
@@ -19,6 +19,7 @@ type APIEndpoint struct {
var (
EndpointConfigGet = APIEndpoint{"GET", "arvados/v1/config", ""}
EndpointLogin = APIEndpoint{"GET", "login", ""}
+ EndpointLogout = APIEndpoint{"GET", "logout", ""}
EndpointCollectionCreate = APIEndpoint{"POST", "arvados/v1/collections", "collection"}
EndpointCollectionUpdate = APIEndpoint{"PATCH", "arvados/v1/collections/{uuid}", "collection"}
EndpointCollectionGet = APIEndpoint{"GET", "arvados/v1/collections/{uuid}", ""}
@@ -140,9 +141,14 @@ type LoginOptions struct {
State string `json:"state,omitempty"` // OAuth2 callback state
}
+type LogoutOptions struct {
+ ReturnTo string `json:"return_to"` // Redirect to this URL after logging out
+}
+
type API interface {
ConfigGet(ctx context.Context) (json.RawMessage, error)
Login(ctx context.Context, options LoginOptions) (LoginResponse, error)
+ Logout(ctx context.Context, options LogoutOptions) (LogoutResponse, error)
CollectionCreate(ctx context.Context, options CreateOptions) (Collection, error)
CollectionUpdate(ctx context.Context, options UpdateOptions) (Collection, error)
CollectionGet(ctx context.Context, options GetOptions) (Collection, error)
diff --git a/sdk/go/arvados/login.go b/sdk/go/arvados/login.go
index 7107ac57a..75ebc81c1 100644
--- a/sdk/go/arvados/login.go
+++ b/sdk/go/arvados/login.go
@@ -24,3 +24,12 @@ func (resp LoginResponse) ServeHTTP(w http.ResponseWriter, req *http.Request) {
w.Write(resp.HTML.Bytes())
}
}
+
+type LogoutResponse struct {
+ RedirectLocation string
+}
+
+func (resp LogoutResponse) ServeHTTP(w http.ResponseWriter, req *http.Request) {
+ w.Header().Set("Location", resp.RedirectLocation)
+ w.WriteHeader(http.StatusFound)
+}
diff --git a/sdk/go/arvadostest/api.go b/sdk/go/arvadostest/api.go
index b5cea5c18..9019d33cf 100644
--- a/sdk/go/arvadostest/api.go
+++ b/sdk/go/arvadostest/api.go
@@ -37,6 +37,10 @@ func (as *APIStub) Login(ctx context.Context, options arvados.LoginOptions) (arv
as.appendCall(as.Login, ctx, options)
return arvados.LoginResponse{}, as.Error
}
+func (as *APIStub) Logout(ctx context.Context, options arvados.LogoutOptions) (arvados.LogoutResponse, error) {
+ as.appendCall(as.Logout, ctx, options)
+ return arvados.LogoutResponse{}, as.Error
+}
func (as *APIStub) CollectionCreate(ctx context.Context, options arvados.CreateOptions) (arvados.Collection, error) {
as.appendCall(as.CollectionCreate, ctx, options)
return arvados.Collection{}, as.Error
commit 8da425a41233ce425f84e0e78166ac97358a5417
Author: Tom Clegg <tom at tomclegg.ca>
Date: Sat Feb 15 16:47:34 2020 -0500
16100: Move test to integration suite to avoid logging errors.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>
diff --git a/services/keep-web/handler_test.go b/services/keep-web/handler_test.go
index 488c91cdc..f6f3de887 100644
--- a/services/keep-web/handler_test.go
+++ b/services/keep-web/handler_test.go
@@ -41,24 +41,6 @@ func (s *UnitSuite) SetUpTest(c *check.C) {
s.Config = cfg
}
-func (s *UnitSuite) TestKeepClientBlockCache(c *check.C) {
- cfg := newConfig(s.Config)
- cfg.cluster.Collections.WebDAVCache.MaxBlockEntries = 42
- h := handler{Config: cfg}
- c.Check(keepclient.DefaultBlockCache.MaxBlocks, check.Not(check.Equals), cfg.cluster.Collections.WebDAVCache.MaxBlockEntries)
- u := mustParseURL("http://keep-web.example/c=" + arvadostest.FooCollection + "/t=" + arvadostest.ActiveToken + "/foo")
- req := &http.Request{
- Method: "GET",
- Host: u.Host,
- URL: u,
- RequestURI: u.RequestURI(),
- }
- resp := httptest.NewRecorder()
- h.ServeHTTP(resp, req)
- c.Check(resp.Code, check.Equals, http.StatusOK)
- c.Check(keepclient.DefaultBlockCache.MaxBlocks, check.Equals, cfg.cluster.Collections.WebDAVCache.MaxBlockEntries)
-}
-
func (s *UnitSuite) TestCORSPreflight(c *check.C) {
h := handler{Config: newConfig(s.Config)}
u := mustParseURL("http://keep-web.example/c=" + arvadostest.FooCollection + "/foo")
@@ -994,6 +976,22 @@ func (s *IntegrationSuite) TestFileContentType(c *check.C) {
}
}
+func (s *IntegrationSuite) TestKeepClientBlockCache(c *check.C) {
+ s.testServer.Config.cluster.Collections.WebDAVCache.MaxBlockEntries = 42
+ c.Check(keepclient.DefaultBlockCache.MaxBlocks, check.Not(check.Equals), 42)
+ u := mustParseURL("http://keep-web.example/c=" + arvadostest.FooCollection + "/t=" + arvadostest.ActiveToken + "/foo")
+ req := &http.Request{
+ Method: "GET",
+ Host: u.Host,
+ URL: u,
+ RequestURI: u.RequestURI(),
+ }
+ resp := httptest.NewRecorder()
+ s.testServer.Handler.ServeHTTP(resp, req)
+ c.Check(resp.Code, check.Equals, http.StatusOK)
+ c.Check(keepclient.DefaultBlockCache.MaxBlocks, check.Equals, 42)
+}
+
func copyHeader(h http.Header) http.Header {
hc := http.Header{}
for k, v := range h {
commit c229744b5941ca76138d527578dcc20cd98ce1c6
Author: Tom Clegg <tom at tomclegg.ca>
Date: Fri Feb 14 13:40:01 2020 -0500
16100: Add package dependency for /etc/mime.types.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>
diff --git a/services/keep-web/fpm-info.sh b/services/keep-web/fpm-info.sh
new file mode 100644
index 000000000..6bcbf67fe
--- /dev/null
+++ b/services/keep-web/fpm-info.sh
@@ -0,0 +1,12 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+case "$TARGET" in
+ centos*)
+ fpm_depends+=(mailcap)
+ ;;
+ debian* | ubuntu*)
+ fpm_depends+=(mime-support)
+ ;;
+esac
commit ff0bf85aeaf3eeaec8465394fb3748e06cfc2ac4
Author: Tom Clegg <tom at tomclegg.ca>
Date: Fri Feb 14 13:39:38 2020 -0500
16100: Log a warning if /etc/mime.types is missing.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>
diff --git a/services/keep-web/main.go b/services/keep-web/main.go
index f028cca22..e4028842f 100644
--- a/services/keep-web/main.go
+++ b/services/keep-web/main.go
@@ -7,6 +7,7 @@ package main
import (
"flag"
"fmt"
+ "mime"
"os"
"git.arvados.org/arvados.git/lib/config"
@@ -104,6 +105,10 @@ func main() {
log.Printf("keep-web %s started", version)
+ if ext := ".txt"; mime.TypeByExtension(ext) == "" {
+ log.Warnf("cannot look up MIME type for %q -- this probably means /etc/mime.types is missing -- clients will see incorrect content types", ext)
+ }
+
os.Setenv("ARVADOS_API_HOST", cfg.cluster.Services.Controller.ExternalURL.Host)
srv := &server{Config: cfg}
if err := srv.Start(); err != nil {
commit da2385723280b6deb1fcb58ecfbbf7cf952e930e
Author: Tom Clegg <tom at tomclegg.ca>
Date: Fri Feb 14 13:25:30 2020 -0500
16100: Test content-type detection.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>
diff --git a/services/keep-web/handler_test.go b/services/keep-web/handler_test.go
index 29bcdac1b..488c91cdc 100644
--- a/services/keep-web/handler_test.go
+++ b/services/keep-web/handler_test.go
@@ -19,6 +19,7 @@ import (
"git.arvados.org/arvados.git/lib/config"
"git.arvados.org/arvados.git/sdk/go/arvados"
+ "git.arvados.org/arvados.git/sdk/go/arvadosclient"
"git.arvados.org/arvados.git/sdk/go/arvadostest"
"git.arvados.org/arvados.git/sdk/go/auth"
"git.arvados.org/arvados.git/sdk/go/ctxlog"
@@ -933,6 +934,66 @@ func (s *IntegrationSuite) TestHealthCheckPing(c *check.C) {
c.Check(resp.Body.String(), check.Matches, `{"health":"OK"}\n`)
}
+func (s *IntegrationSuite) TestFileContentType(c *check.C) {
+ s.testServer.Config.cluster.Services.WebDAVDownload.ExternalURL.Host = "download.example.com"
+
+ client := s.testServer.Config.Client
+ client.AuthToken = arvadostest.ActiveToken
+ arv, err := arvadosclient.New(&client)
+ c.Assert(err, check.Equals, nil)
+ kc, err := keepclient.MakeKeepClient(arv)
+ c.Assert(err, check.Equals, nil)
+
+ fs, err := (&arvados.Collection{}).FileSystem(&client, kc)
+ c.Assert(err, check.IsNil)
+
+ trials := []struct {
+ filename string
+ content string
+ contentType string
+ }{
+ {"picture.txt", "BMX bikes are small this year\n", "text/plain; charset=utf-8"},
+ {"picture.bmp", "BMX bikes are small this year\n", "image/x-ms-bmp"},
+ {"picture.jpg", "BMX bikes are small this year\n", "image/jpeg"},
+ {"picture1", "BMX bikes are small this year\n", "image/bmp"}, // content sniff; "BM" is the magic signature for .bmp
+ {"picture2", "Cars are small this year\n", "text/plain; charset=utf-8"}, // content sniff
+ }
+ for _, trial := range trials {
+ f, err := fs.OpenFile(trial.filename, os.O_CREATE|os.O_WRONLY, 0777)
+ c.Assert(err, check.IsNil)
+ _, err = f.Write([]byte(trial.content))
+ c.Assert(err, check.IsNil)
+ c.Assert(f.Close(), check.IsNil)
+ }
+ mtxt, err := fs.MarshalManifest(".")
+ c.Assert(err, check.IsNil)
+ var coll arvados.Collection
+ err = client.RequestAndDecode(&coll, "POST", "arvados/v1/collections", nil, map[string]interface{}{
+ "collection": map[string]string{
+ "manifest_text": mtxt,
+ },
+ })
+ c.Assert(err, check.IsNil)
+
+ for _, trial := range trials {
+ u, _ := url.Parse("http://download.example.com/by_id/" + coll.UUID + "/" + trial.filename)
+ req := &http.Request{
+ Method: "GET",
+ Host: u.Host,
+ URL: u,
+ RequestURI: u.RequestURI(),
+ Header: http.Header{
+ "Authorization": {"Bearer " + client.AuthToken},
+ },
+ }
+ resp := httptest.NewRecorder()
+ s.testServer.Handler.ServeHTTP(resp, req)
+ c.Check(resp.Code, check.Equals, http.StatusOK)
+ c.Check(resp.Header().Get("Content-Type"), check.Equals, trial.contentType)
+ c.Check(resp.Body.String(), check.Equals, trial.content)
+ }
+}
+
func copyHeader(h http.Header) http.Header {
hc := http.Header{}
for k, v := range h {
commit a4b2ddad996a6ef81dcbc852bb03d319422f93b3
Author: Tom Clegg <tom at tomclegg.ca>
Date: Thu Feb 13 02:29:22 2020 -0500
16100: Fix bogus timing stats for empty responses.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>
diff --git a/sdk/go/httpserver/logger.go b/sdk/go/httpserver/logger.go
index 30f5e2612..8886f9517 100644
--- a/sdk/go/httpserver/logger.go
+++ b/sdk/go/httpserver/logger.go
@@ -68,10 +68,16 @@ func logRequest(w *responseTimer, req *http.Request, lgr *logrus.Entry) {
func logResponse(w *responseTimer, req *http.Request, lgr *logrus.Entry) {
if tStart, ok := req.Context().Value(&requestTimeContextKey).(time.Time); ok {
tDone := time.Now()
+ writeTime := w.writeTime
+ if !w.wrote {
+ // Empty response body. Header was sent when
+ // handler exited.
+ writeTime = tDone
+ }
lgr = lgr.WithFields(logrus.Fields{
"timeTotal": stats.Duration(tDone.Sub(tStart)),
- "timeToStatus": stats.Duration(w.writeTime.Sub(tStart)),
- "timeWriteBody": stats.Duration(tDone.Sub(w.writeTime)),
+ "timeToStatus": stats.Duration(writeTime.Sub(tStart)),
+ "timeWriteBody": stats.Duration(tDone.Sub(writeTime)),
})
}
respCode := w.WroteStatus()
commit 137f2e31166c454d4536ba713347e8af3eb0176b
Author: Tom Clegg <tom at tomclegg.ca>
Date: Wed Feb 12 21:50:07 2020 -0500
16039: Touch python clients so they run tests with the latest SDK.
Otherwise, "pip install" (during "install services/nodemanager", for
example) downgrades the SDK from the current version to the latest
version published on pip, and all test suites run with that version,
instead of the version we think we're testing.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>
diff --git a/sdk/cwl/setup.py b/sdk/cwl/setup.py
index 62ceab2fa..d4bb6d102 100644
--- a/sdk/cwl/setup.py
+++ b/sdk/cwl/setup.py
@@ -36,7 +36,8 @@ setup(name='arvados-cwl-runner',
'bin/arvados-cwl-runner',
],
# Note that arvados/build/run-build-packages.sh looks at this
- # file to determine what version of cwltool and schema-salad to build.
+ # file to determine what version of cwltool and schema-salad to
+ # build.
install_requires=[
'cwltool==1.0.20190831161204',
'schema-salad==4.5.20190815125611',
@@ -63,5 +64,5 @@ setup(name='arvados-cwl-runner',
'mock>=1.0,<4',
'subprocess32>=3.5.1',
],
- zip_safe=True
- )
+ zip_safe=True,
+)
diff --git a/sdk/pam/setup.py b/sdk/pam/setup.py
index af00142a0..59b49a19f 100755
--- a/sdk/pam/setup.py
+++ b/sdk/pam/setup.py
@@ -53,5 +53,5 @@ setup(name='arvados-pam',
],
test_suite='tests',
tests_require=['pbr<1.7.0', 'mock>=1.0', 'python-pam'],
- zip_safe=False
- )
+ zip_safe=False,
+)
diff --git a/services/nodemanager/setup.py b/services/nodemanager/setup.py
index a2b9a0ca9..75e8f85fb 100644
--- a/services/nodemanager/setup.py
+++ b/services/nodemanager/setup.py
@@ -56,5 +56,5 @@ setup(name='arvados-node-manager',
'apache-libcloud==2.5.0',
'subprocess32>=3.5.1',
],
- zip_safe=False
- )
+ zip_safe=False,
+)
diff --git a/tools/crunchstat-summary/setup.py b/tools/crunchstat-summary/setup.py
index 40c5a2f9a..557b6d3f4 100755
--- a/tools/crunchstat-summary/setup.py
+++ b/tools/crunchstat-summary/setup.py
@@ -42,5 +42,5 @@ setup(name='crunchstat_summary',
],
test_suite='tests',
tests_require=['pbr<1.7.0', 'mock>=1.0'],
- zip_safe=False
- )
+ zip_safe=False,
+)
commit cfd64e9f9714d755a80e96b71b7ab796c0710872
Author: Tom Clegg <tom at tomclegg.ca>
Date: Wed Feb 12 21:47:44 2020 -0500
16039: Install missing python dep.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>
diff --git a/build/run-tests.sh b/build/run-tests.sh
index 43dd3f4eb..c4c533559 100755
--- a/build/run-tests.sh
+++ b/build/run-tests.sh
@@ -647,8 +647,8 @@ install_env() {
. "$VENVDIR/bin/activate"
# Needed for run_test_server.py which is used by certain (non-Python) tests.
- pip install --no-cache-dir PyYAML future \
- || fatal "pip install PyYAML failed"
+ pip install --no-cache-dir PyYAML future httplib2 \
+ || fatal "`pip install PyYAML future httplib2` failed"
# Preinstall libcloud if using a fork; otherwise nodemanager "pip
# install" won't pick it up by default.
commit 2ad995236f3584635b7a2e80c62a31323b9e65b8
Author: Tom Clegg <tom at tomclegg.ca>
Date: Wed Feb 12 13:49:50 2020 -0500
16039: Use current python sdk in py3 tests.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>
diff --git a/build/run-tests.sh b/build/run-tests.sh
index f21861762..43dd3f4eb 100755
--- a/build/run-tests.sh
+++ b/build/run-tests.sh
@@ -1099,6 +1099,7 @@ install_deps() {
do_install sdk/cli
do_install sdk/perl
do_install sdk/python pip
+ do_install sdk/python pip3
do_install sdk/ruby
do_install services/api
do_install services/arv-git-httpd go
commit be568b1c98420f920c9f602d7e1aa47d401bcd39
Author: Tom Clegg <tom at tomclegg.ca>
Date: Wed Feb 12 10:48:05 2020 -0500
16039: Fix & add test for sanitized/unsanitized name conflict.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>
diff --git a/services/fuse/arvados_fuse/fusedir.py b/services/fuse/arvados_fuse/fusedir.py
index 3f6430973..8b12f73e8 100644
--- a/services/fuse/arvados_fuse/fusedir.py
+++ b/services/fuse/arvados_fuse/fusedir.py
@@ -941,7 +941,7 @@ class ProjectDirectory(Directory):
namefilter],
limit=2).execute(num_retries=self.num_retries)["items"]
if contents:
- if len(contents) > 1 and contents[1].name == k:
+ if len(contents) > 1 and contents[1]['name'] == k:
# If "foo/bar" and "foo[SUBST]bar" both exist, use
# "foo[SUBST]bar".
contents = [contents[1]]
diff --git a/services/fuse/tests/test_mount.py b/services/fuse/tests/test_mount.py
index 1e63b9f4d..593d945cf 100644
--- a/services/fuse/tests/test_mount.py
+++ b/services/fuse/tests/test_mount.py
@@ -1239,3 +1239,17 @@ class SlashSubstitutionTest(IntegrationTest):
def checkContents(self):
self.assertRegexpMatches(self.api.collections().get(uuid=self.testcoll['uuid']).execute()['manifest_text'], ' acbd18db') # md5(foo)
self.assertRegexpMatches(self.api.collections().get(uuid=self.testcolleasy['uuid']).execute()['manifest_text'], ' f561aaf6') # md5(xxx)
+
+ @IntegrationTest.mount(argv=mnt_args)
+ @mock.patch('arvados.util.get_config_once')
+ def test_slash_substitution_conflict(self, get_config_once):
+ self.testcollconflict = self.api.collections().create(body={"name": self.fusename}).execute()
+ get_config_once.return_value = {"Collections": {"ForwardSlashNameSubstitution": "[SLASH]"}}
+ self.pool_test(os.path.join(self.mnt, 'zzz'), self.fusename)
+ self.assertRegexpMatches(self.api.collections().get(uuid=self.testcollconflict['uuid']).execute()['manifest_text'], ' acbd18db') # md5(foo)
+ # foo/bar/baz collection unchanged, because it is masked by foo[SLASH]bar[SLASH]baz
+ self.assertEqual(self.api.collections().get(uuid=self.testcoll['uuid']).execute()['manifest_text'], '')
+ @staticmethod
+ def _test_slash_substitution_conflict(self, tmpdir, fusename):
+ with open(os.path.join(tmpdir, fusename, 'waz'), 'w') as f:
+ f.write('foo')
commit 8a7a749b027a115c2cef269ab7fdfc85f40c2f63
Author: Tom Clegg <tom at tomclegg.ca>
Date: Wed Feb 12 10:37:48 2020 -0500
16039: Accommodate API servers with no config export.
Add more checks in test cases.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>
diff --git a/sdk/python/arvados/util.py b/sdk/python/arvados/util.py
index b27b64e59..9e0a31783 100644
--- a/sdk/python/arvados/util.py
+++ b/sdk/python/arvados/util.py
@@ -421,6 +421,9 @@ def new_request_id():
return rid
def get_config_once(svc):
+ if not svc._rootDesc.get('resources')['configs']:
+ # Old API server version, no config export endpoint
+ return {}
if not hasattr(svc, '_cached_config'):
svc._cached_config = svc.configs().get().execute()
return svc._cached_config
diff --git a/services/fuse/arvados_fuse/fusedir.py b/services/fuse/arvados_fuse/fusedir.py
index 9853f6ac2..3f6430973 100644
--- a/services/fuse/arvados_fuse/fusedir.py
+++ b/services/fuse/arvados_fuse/fusedir.py
@@ -55,10 +55,24 @@ class Directory(FreshBase):
self._entries = {}
self._mtime = time.time()
+ def forward_slash_subst(self):
+ if not hasattr(self, '_fsns'):
+ self._fsns = None
+ config = self.apiconfig()
+ try:
+ self._fsns = config["Collections"]["ForwardSlashNameSubstitution"]
+ except KeyError:
+ # old API server with no FSNS config
+ self._fsns = '_'
+ else:
+ if self._fsns == '' or self._fsns == '/':
+ self._fsns = None
+ return self._fsns
+
def unsanitize_filename(self, incoming):
"""Replace ForwardSlashNameSubstitution value with /"""
- fsns = self.apiconfig()["Collections"]["ForwardSlashNameSubstitution"]
- if isinstance(fsns, str) and fsns != '' and fsns != '/':
+ fsns = self.forward_slash_subst()
+ if isinstance(fsns, str):
return incoming.replace(fsns, '/')
else:
return incoming
@@ -76,8 +90,8 @@ class Directory(FreshBase):
elif dirty == '..':
return '__'
else:
- fsns = self.apiconfig()["Collections"]["ForwardSlashNameSubstitution"]
- if isinstance(fsns, str) and fsns != '':
+ fsns = self.forward_slash_subst()
+ if isinstance(fsns, str):
dirty = dirty.replace('/', fsns)
return _disallowed_filename_characters.sub('_', dirty)
diff --git a/services/fuse/tests/test_mount.py b/services/fuse/tests/test_mount.py
index 3ec03fa22..1e63b9f4d 100644
--- a/services/fuse/tests/test_mount.py
+++ b/services/fuse/tests/test_mount.py
@@ -1214,10 +1214,11 @@ class SlashSubstitutionTest(IntegrationTest):
def test_slash_substitution_before_listing(self, get_config_once):
get_config_once.return_value = {"Collections": {"ForwardSlashNameSubstitution": "[SLASH]"}}
self.pool_test(os.path.join(self.mnt, 'zzz'), self.fusename)
+ self.checkContents()
@staticmethod
def _test_slash_substitution_before_listing(self, tmpdir, fusename):
with open(os.path.join(tmpdir, 'foo-bar-baz', 'waz'), 'w') as f:
- f.write('foo')
+ f.write('xxx')
with open(os.path.join(tmpdir, fusename, 'waz'), 'w') as f:
f.write('foo')
@@ -1226,10 +1227,15 @@ class SlashSubstitutionTest(IntegrationTest):
def test_slash_substitution_after_listing(self, get_config_once):
get_config_once.return_value = {"Collections": {"ForwardSlashNameSubstitution": "[SLASH]"}}
self.pool_test(os.path.join(self.mnt, 'zzz'), self.fusename)
+ self.checkContents()
@staticmethod
def _test_slash_substitution_after_listing(self, tmpdir, fusename):
with open(os.path.join(tmpdir, 'foo-bar-baz', 'waz'), 'w') as f:
- f.write('foo')
+ f.write('xxx')
os.listdir(tmpdir)
with open(os.path.join(tmpdir, fusename, 'waz'), 'w') as f:
f.write('foo')
+
+ def checkContents(self):
+ self.assertRegexpMatches(self.api.collections().get(uuid=self.testcoll['uuid']).execute()['manifest_text'], ' acbd18db') # md5(foo)
+ self.assertRegexpMatches(self.api.collections().get(uuid=self.testcolleasy['uuid']).execute()['manifest_text'], ' f561aaf6') # md5(xxx)
commit 503160684305dccec44ad4cf309893404bf817e1
Author: Tom Clegg <tom at tomclegg.ca>
Date: Tue Feb 11 10:58:28 2020 -0500
16039: Obey ForwardSlashNameSubstitution config in arv-mount.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>
diff --git a/sdk/python/arvados/api.py b/sdk/python/arvados/api.py
index b18ce25fd..ae687c50b 100644
--- a/sdk/python/arvados/api.py
+++ b/sdk/python/arvados/api.py
@@ -237,6 +237,7 @@ def api(version=None, cache=True, host=None, token=None, insecure=False,
svc.api_token = token
svc.insecure = insecure
svc.request_id = request_id
+ svc.config = lambda: util.get_config_once(svc)
kwargs['http'].max_request_size = svc._rootDesc.get('maxRequestSize', 0)
kwargs['http'].cache = None
kwargs['http']._request_id = lambda: svc.request_id or util.new_request_id()
diff --git a/sdk/python/arvados/util.py b/sdk/python/arvados/util.py
index fd29a3dc1..b27b64e59 100644
--- a/sdk/python/arvados/util.py
+++ b/sdk/python/arvados/util.py
@@ -419,3 +419,8 @@ def new_request_id():
rid += chr(c+ord('a')-10)
n = n // 36
return rid
+
+def get_config_once(svc):
+ if not hasattr(svc, '_cached_config'):
+ svc._cached_config = svc.configs().get().execute()
+ return svc._cached_config
diff --git a/services/fuse/arvados_fuse/__init__.py b/services/fuse/arvados_fuse/__init__.py
index 0944a3187..3a0316cf9 100644
--- a/services/fuse/arvados_fuse/__init__.py
+++ b/services/fuse/arvados_fuse/__init__.py
@@ -98,7 +98,7 @@ else:
LLFUSE_VERSION_0 = llfuse.__version__.startswith('0')
-from .fusedir import sanitize_filename, Directory, CollectionDirectory, TmpCollectionDirectory, MagicDirectory, TagsDirectory, ProjectDirectory, SharedDirectory, CollectionDirectoryBase
+from .fusedir import Directory, CollectionDirectory, TmpCollectionDirectory, MagicDirectory, TagsDirectory, ProjectDirectory, SharedDirectory, CollectionDirectoryBase
from .fusefile import StringFile, FuseArvadosFile
_logger = logging.getLogger('arvados.arvados_fuse')
diff --git a/services/fuse/arvados_fuse/command.py b/services/fuse/arvados_fuse/command.py
index 528336753..7bef8a269 100644
--- a/services/fuse/arvados_fuse/command.py
+++ b/services/fuse/arvados_fuse/command.py
@@ -301,7 +301,7 @@ class Mount(object):
return
e = self.operations.inodes.add_entry(Directory(
- llfuse.ROOT_INODE, self.operations.inodes))
+ llfuse.ROOT_INODE, self.operations.inodes, self.api.config))
dir_args[0] = e.inode
for name in self.args.mount_by_id:
diff --git a/services/fuse/arvados_fuse/fusedir.py b/services/fuse/arvados_fuse/fusedir.py
index 328765744..9853f6ac2 100644
--- a/services/fuse/arvados_fuse/fusedir.py
+++ b/services/fuse/arvados_fuse/fusedir.py
@@ -33,20 +33,6 @@ _logger = logging.getLogger('arvados.arvados_fuse')
# appear as underscores in the fuse mount.)
_disallowed_filename_characters = re.compile('[\x00/]')
-# '.' and '..' are not reachable if API server is newer than #6277
-def sanitize_filename(dirty):
- """Replace disallowed filename characters with harmless "_"."""
- if dirty is None:
- return None
- elif dirty == '':
- return '_'
- elif dirty == '.':
- return '_'
- elif dirty == '..':
- return '__'
- else:
- return _disallowed_filename_characters.sub('_', dirty)
-
class Directory(FreshBase):
"""Generic directory object, backed by a dict.
@@ -55,7 +41,7 @@ class Directory(FreshBase):
and the value referencing a File or Directory object.
"""
- def __init__(self, parent_inode, inodes):
+ def __init__(self, parent_inode, inodes, apiconfig):
"""parent_inode is the integer inode number"""
super(Directory, self).__init__()
@@ -65,11 +51,39 @@ class Directory(FreshBase):
raise Exception("parent_inode should be an int")
self.parent_inode = parent_inode
self.inodes = inodes
+ self.apiconfig = apiconfig
self._entries = {}
self._mtime = time.time()
- # Overriden by subclasses to implement logic to update the entries dict
- # when the directory is stale
+ def unsanitize_filename(self, incoming):
+ """Replace ForwardSlashNameSubstitution value with /"""
+ fsns = self.apiconfig()["Collections"]["ForwardSlashNameSubstitution"]
+ if isinstance(fsns, str) and fsns != '' and fsns != '/':
+ return incoming.replace(fsns, '/')
+ else:
+ return incoming
+
+ def sanitize_filename(self, dirty):
+ """Replace disallowed filename characters according to
+ ForwardSlashNameSubstitution in self.api_config."""
+ # '.' and '..' are not reachable if API server is newer than #6277
+ if dirty is None:
+ return None
+ elif dirty == '':
+ return '_'
+ elif dirty == '.':
+ return '_'
+ elif dirty == '..':
+ return '__'
+ else:
+ fsns = self.apiconfig()["Collections"]["ForwardSlashNameSubstitution"]
+ if isinstance(fsns, str) and fsns != '':
+ dirty = dirty.replace('/', fsns)
+ return _disallowed_filename_characters.sub('_', dirty)
+
+
+ # Overridden by subclasses to implement logic to update the
+ # entries dict when the directory is stale
@use_counter
def update(self):
pass
@@ -138,7 +152,7 @@ class Directory(FreshBase):
self._entries = {}
changed = False
for i in items:
- name = sanitize_filename(fn(i))
+ name = self.sanitize_filename(fn(i))
if name:
if name in oldentries and same(oldentries[name], i):
# move existing directory entry over
@@ -246,12 +260,13 @@ class CollectionDirectoryBase(Directory):
"""
- def __init__(self, parent_inode, inodes, collection):
- super(CollectionDirectoryBase, self).__init__(parent_inode, inodes)
+ def __init__(self, parent_inode, inodes, apiconfig, collection):
+ super(CollectionDirectoryBase, self).__init__(parent_inode, inodes, apiconfig)
+ self.apiconfig = apiconfig
self.collection = collection
def new_entry(self, name, item, mtime):
- name = sanitize_filename(name)
+ name = self.sanitize_filename(name)
if hasattr(item, "fuse_entry") and item.fuse_entry is not None:
if item.fuse_entry.dead is not True:
raise Exception("Can only reparent dead inode entry")
@@ -260,7 +275,7 @@ class CollectionDirectoryBase(Directory):
item.fuse_entry.dead = False
self._entries[name] = item.fuse_entry
elif isinstance(item, arvados.collection.RichCollectionBase):
- self._entries[name] = self.inodes.add_entry(CollectionDirectoryBase(self.inode, self.inodes, item))
+ self._entries[name] = self.inodes.add_entry(CollectionDirectoryBase(self.inode, self.inodes, self.apiconfig, item))
self._entries[name].populate(mtime)
else:
self._entries[name] = self.inodes.add_entry(FuseArvadosFile(self.inode, item, mtime))
@@ -268,7 +283,7 @@ class CollectionDirectoryBase(Directory):
def on_event(self, event, collection, name, item):
if collection == self.collection:
- name = sanitize_filename(name)
+ name = self.sanitize_filename(name)
_logger.debug("collection notify %s %s %s %s", event, collection, name, item)
with llfuse.lock:
if event == arvados.collection.ADD:
@@ -357,7 +372,7 @@ class CollectionDirectory(CollectionDirectoryBase):
"""Represents the root of a directory tree representing a collection."""
def __init__(self, parent_inode, inodes, api, num_retries, collection_record=None, explicit_collection=None):
- super(CollectionDirectory, self).__init__(parent_inode, inodes, None)
+ super(CollectionDirectory, self).__init__(parent_inode, inodes, api.config, None)
self.api = api
self.num_retries = num_retries
self.collection_record_file = None
@@ -548,7 +563,7 @@ class TmpCollectionDirectory(CollectionDirectoryBase):
keep_client=api_client.keep,
num_retries=num_retries)
super(TmpCollectionDirectory, self).__init__(
- parent_inode, inodes, collection)
+ parent_inode, inodes, api_client.config, collection)
self.collection_record_file = None
self.populate(self.mtime())
@@ -625,7 +640,7 @@ and the directory will appear if it exists.
""".lstrip()
def __init__(self, parent_inode, inodes, api, num_retries, pdh_only=False):
- super(MagicDirectory, self).__init__(parent_inode, inodes)
+ super(MagicDirectory, self).__init__(parent_inode, inodes, api.config)
self.api = api
self.num_retries = num_retries
self.pdh_only = pdh_only
@@ -660,6 +675,7 @@ and the directory will appear if it exists.
e = self.inodes.add_entry(ProjectDirectory(
self.inode, self.inodes, self.api, self.num_retries, project[u'items'][0]))
else:
+ import sys
e = self.inodes.add_entry(CollectionDirectory(
self.inode, self.inodes, self.api, self.num_retries, k))
@@ -696,7 +712,7 @@ class TagsDirectory(Directory):
"""A special directory that contains as subdirectories all tags visible to the user."""
def __init__(self, parent_inode, inodes, api, num_retries, poll_time=60):
- super(TagsDirectory, self).__init__(parent_inode, inodes)
+ super(TagsDirectory, self).__init__(parent_inode, inodes, api.config)
self.api = api
self.num_retries = num_retries
self._poll = True
@@ -753,7 +769,7 @@ class TagDirectory(Directory):
def __init__(self, parent_inode, inodes, api, num_retries, tag,
poll=False, poll_time=60):
- super(TagDirectory, self).__init__(parent_inode, inodes)
+ super(TagDirectory, self).__init__(parent_inode, inodes, api.config)
self.api = api
self.num_retries = num_retries
self.tag = tag
@@ -783,7 +799,7 @@ class ProjectDirectory(Directory):
def __init__(self, parent_inode, inodes, api, num_retries, project_object,
poll=False, poll_time=60):
- super(ProjectDirectory, self).__init__(parent_inode, inodes)
+ super(ProjectDirectory, self).__init__(parent_inode, inodes, api.config)
self.api = api
self.num_retries = num_retries
self.project_object = project_object
@@ -897,16 +913,25 @@ class ProjectDirectory(Directory):
elif self._full_listing or super(ProjectDirectory, self).__contains__(k):
return super(ProjectDirectory, self).__getitem__(k)
with llfuse.lock_released:
+ k2 = self.unsanitize_filename(k)
+ if k2 == k:
+ namefilter = ["name", "=", k]
+ else:
+ namefilter = ["name", "in", [k, k2]]
contents = self.api.groups().list(filters=[["owner_uuid", "=", self.project_uuid],
["group_class", "=", "project"],
- ["name", "=", k]],
- limit=1).execute(num_retries=self.num_retries)["items"]
+ namefilter],
+ limit=2).execute(num_retries=self.num_retries)["items"]
if not contents:
contents = self.api.collections().list(filters=[["owner_uuid", "=", self.project_uuid],
- ["name", "=", k]],
- limit=1).execute(num_retries=self.num_retries)["items"]
+ namefilter],
+ limit=2).execute(num_retries=self.num_retries)["items"]
if contents:
- name = sanitize_filename(self.namefn(contents[0]))
+ if len(contents) > 1 and contents[1].name == k:
+ # If "foo/bar" and "foo[SUBST]bar" both exist, use
+ # "foo[SUBST]bar".
+ contents = [contents[1]]
+ name = self.sanitize_filename(self.namefn(contents[0]))
if name != k:
raise KeyError(k)
return self._add_entry(contents[0], name)
@@ -995,8 +1020,8 @@ class ProjectDirectory(Directory):
new_attrs = properties.get("new_attributes") or {}
old_attrs["uuid"] = ev["object_uuid"]
new_attrs["uuid"] = ev["object_uuid"]
- old_name = sanitize_filename(self.namefn(old_attrs))
- new_name = sanitize_filename(self.namefn(new_attrs))
+ old_name = self.sanitize_filename(self.namefn(old_attrs))
+ new_name = self.sanitize_filename(self.namefn(new_attrs))
# create events will have a new name, but not an old name
# delete events will have an old name, but not a new name
@@ -1038,7 +1063,7 @@ class SharedDirectory(Directory):
def __init__(self, parent_inode, inodes, api, num_retries, exclude,
poll=False, poll_time=60):
- super(SharedDirectory, self).__init__(parent_inode, inodes)
+ super(SharedDirectory, self).__init__(parent_inode, inodes, api.config)
self.api = api
self.num_retries = num_retries
self.current_user = api.users().current().execute(num_retries=num_retries)
diff --git a/services/fuse/tests/test_mount.py b/services/fuse/tests/test_mount.py
index f539b3f7d..3ec03fa22 100644
--- a/services/fuse/tests/test_mount.py
+++ b/services/fuse/tests/test_mount.py
@@ -20,6 +20,7 @@ import arvados
import arvados_fuse as fuse
from . import run_test_server
+from .integration_test import IntegrationTest
from .mount_test_base import MountTestBase
logger = logging.getLogger('arvados.arv-mount')
@@ -1098,8 +1099,9 @@ class MagicDirApiError(FuseMagicTest):
llfuse.listdir(os.path.join(self.mounttmp, self.testcollection))
-class FuseUnitTest(unittest.TestCase):
+class SanitizeFilenameTest(MountTestBase):
def test_sanitize_filename(self):
+ pdir = fuse.ProjectDirectory(1, {}, self.api, 0, project_object=self.api.users().current().execute())
acceptable = [
"foo.txt",
".foo",
@@ -1119,15 +1121,15 @@ class FuseUnitTest(unittest.TestCase):
"//",
]
for f in acceptable:
- self.assertEqual(f, fuse.sanitize_filename(f))
+ self.assertEqual(f, pdir.sanitize_filename(f))
for f in unacceptable:
- self.assertNotEqual(f, fuse.sanitize_filename(f))
+ self.assertNotEqual(f, pdir.sanitize_filename(f))
# The sanitized filename should be the same length, though.
- self.assertEqual(len(f), len(fuse.sanitize_filename(f)))
+ self.assertEqual(len(f), len(pdir.sanitize_filename(f)))
# Special cases
- self.assertEqual("_", fuse.sanitize_filename(""))
- self.assertEqual("_", fuse.sanitize_filename("."))
- self.assertEqual("__", fuse.sanitize_filename(".."))
+ self.assertEqual("_", pdir.sanitize_filename(""))
+ self.assertEqual("_", pdir.sanitize_filename("."))
+ self.assertEqual("__", pdir.sanitize_filename(".."))
class FuseMagicTestPDHOnly(MountTestBase):
@@ -1191,3 +1193,43 @@ class FuseMagicTestPDHOnly(MountTestBase):
def test_with_default_by_id(self):
self.verify_pdh_only(skip_pdh_only=True)
+
+
+class SlashSubstitutionTest(IntegrationTest):
+ mnt_args = [
+ '--read-write',
+ '--mount-home', 'zzz',
+ ]
+
+ def setUp(self):
+ super(SlashSubstitutionTest, self).setUp()
+ self.api = arvados.safeapi.ThreadSafeApiCache(arvados.config.settings())
+ self.api.config = lambda: {"Collections": {"ForwardSlashNameSubstitution": "[SLASH]"}}
+ self.testcoll = self.api.collections().create(body={"name": "foo/bar/baz"}).execute()
+ self.testcolleasy = self.api.collections().create(body={"name": "foo-bar-baz"}).execute()
+ self.fusename = 'foo[SLASH]bar[SLASH]baz'
+
+ @IntegrationTest.mount(argv=mnt_args)
+ @mock.patch('arvados.util.get_config_once')
+ def test_slash_substitution_before_listing(self, get_config_once):
+ get_config_once.return_value = {"Collections": {"ForwardSlashNameSubstitution": "[SLASH]"}}
+ self.pool_test(os.path.join(self.mnt, 'zzz'), self.fusename)
+ @staticmethod
+ def _test_slash_substitution_before_listing(self, tmpdir, fusename):
+ with open(os.path.join(tmpdir, 'foo-bar-baz', 'waz'), 'w') as f:
+ f.write('foo')
+ with open(os.path.join(tmpdir, fusename, 'waz'), 'w') as f:
+ f.write('foo')
+
+ @IntegrationTest.mount(argv=mnt_args)
+ @mock.patch('arvados.util.get_config_once')
+ def test_slash_substitution_after_listing(self, get_config_once):
+ get_config_once.return_value = {"Collections": {"ForwardSlashNameSubstitution": "[SLASH]"}}
+ self.pool_test(os.path.join(self.mnt, 'zzz'), self.fusename)
+ @staticmethod
+ def _test_slash_substitution_after_listing(self, tmpdir, fusename):
+ with open(os.path.join(tmpdir, 'foo-bar-baz', 'waz'), 'w') as f:
+ f.write('foo')
+ os.listdir(tmpdir)
+ with open(os.path.join(tmpdir, fusename, 'waz'), 'w') as f:
+ f.write('foo')
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list