[ARVADOS] updated: 1.3.0-2264-gb405ab07d

Git user git at public.arvados.org
Mon Jun 8 18:58:33 UTC 2020


Summary of changes:
 ... => test-package-python3-arvados-cwl-runner.sh} |   0
 build/run-library.sh                               |  13 +-
 doc/architecture/index.html.textile.liquid         |   2 +-
 ...onfigure-azure-blob-storage.html.textile.liquid |   4 +-
 ...configure-s3-object-storage.html.textile.liquid |   4 +-
 doc/install/install-keepstore.html.textile.liquid  |   4 +-
 .../topics/arvados-sync-groups.html.textile.liquid |   4 +-
 lib/controller/fed_containers.go                   |  12 +-
 lib/controller/federation.go                       |  13 +-
 sdk/cwl/fpm-info.sh                                |   3 +
 sdk/go/arvados/container.go                        |   2 +
 sdk/python/arvados/keep.py                         |   4 +
 sdk/python/arvados/util.py                         |   5 +-
 .../app/controllers/arvados/v1/users_controller.rb |   6 +-
 .../api/app/models/api_client_authorization.rb     |   4 +
 services/api/app/models/user.rb                    |  10 +-
 services/api/config/initializers/time_zone.rb      |  15 ++
 services/api/lib/db_current_time.rb                |   8 +-
 services/api/test/unit/time_zone_test.rb           |  15 ++
 services/keepproxy/keepproxy.go                    |   2 +-
 tools/sync-groups/sync-groups.go                   | 249 ++++++++++++++-------
 tools/sync-groups/sync-groups_test.go              | 138 ++++++++++--
 22 files changed, 387 insertions(+), 130 deletions(-)
 copy build/package-testing/{test-package-python27-python-arvados-cwl-runner.sh => test-package-python3-arvados-cwl-runner.sh} (100%)
 create mode 100644 services/api/config/initializers/time_zone.rb
 create mode 100644 services/api/test/unit/time_zone_test.rb

       via  b405ab07d342827e2bcb70ab4072d30eded90537 (commit)
       via  a788b2a6c38b0a5d5887f44f45ff44d258f1b723 (commit)
       via  04e3809b09008c4009a9bc74120952d753244884 (commit)
       via  15f61e8bce0d67014f02852a05bc462e09b80435 (commit)
       via  cc6d74141a399b9f8401e1388bf6913114cad5bc (commit)
       via  1334a03fb592d13f72ee5379cc4ac1199240d2e7 (commit)
       via  cfa70fa206cef453a736187c1b23c8360f010cb8 (commit)
       via  6260e1b0d7778d247880357556debe51acf71cf4 (commit)
       via  e58c5263c3529a9f0fe586852612c991b5a8ceec (commit)
       via  0267966c0452f398b6f56077e426584446ad403e (commit)
       via  5c2cfee366a58d54fc39f47ccd84a5db5d96d856 (commit)
       via  e94ed22c9f40ecafd3cb1ee1349eaee018b4d883 (commit)
       via  d7597b98e702ad5ab9c059ee3c3528b9d3bda292 (commit)
       via  6b11d6756340eb9ba557b4148c4e5df8d46660b9 (commit)
       via  26f06e7a69aaf7c84995d452c16f7a97ddf6032d (commit)
       via  1b9451fc5c3765191cb17bfb4ed4468ecbf077aa (commit)
       via  3889fe0ff14d710817f53ba4d517445df89c2342 (commit)
       via  2ea16ba491194afca89500fcee80e91dac0d3d95 (commit)
       via  5958df985bfc8293dc94f06b2c23824cb4c8f41b (commit)
       via  8b6836581b1ce61075f6e9020d18acd858a86cdf (commit)
       via  ccc6ca8fb944d45c94f2ae994d3c0f0567c1b91b (commit)
       via  c6523d3fdf49b9d7b0fc998f64473725ad9346fa (commit)
       via  4506c6105f716cceb091954d8589dbea12696bea (commit)
      from  f9fe0abf384122c003cac247840f2113fccec59b (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 b405ab07d342827e2bcb70ab4072d30eded90537
Author: Ward Vandewege <ward at jhvc.com>
Date:   Fri May 1 16:55:01 2020 -0400

    16393: keepproxy used cluster.API.KeepServiceRequestTimeout (defaults to
    15s) as the timeout on its connection to the keepstores. When a slow client
    sends blocks to keepproxy, they get streamed through to keepstore, and
    the upload can take more than cluster.API.KeepServiceRequestTimeout
    seconds. Update keepproxy to use keepclient.DefaultProxyRequestTimeout
    (300s) instead when it connects to the keepstores.
    
    Arvados-DCO-1.1-Signed-off-by: Ward Vandewege <ward at jhvc.com>

diff --git a/services/keepproxy/keepproxy.go b/services/keepproxy/keepproxy.go
index 58e4a8534..79aa992aa 100644
--- a/services/keepproxy/keepproxy.go
+++ b/services/keepproxy/keepproxy.go
@@ -157,7 +157,7 @@ func run(logger log.FieldLogger, cluster *arvados.Cluster) error {
 	signal.Notify(term, syscall.SIGINT)
 
 	// Start serving requests.
-	router = MakeRESTRouter(kc, time.Duration(cluster.API.KeepServiceRequestTimeout), cluster.SystemRootToken)
+	router = MakeRESTRouter(kc, time.Duration(keepclient.DefaultProxyRequestTimeout), cluster.ManagementToken)
 	return http.Serve(listener, httpserver.AddRequestIDs(httpserver.LogRequests(router)))
 }
 

commit a788b2a6c38b0a5d5887f44f45ff44d258f1b723
Author: Tom Clegg <tom at tomclegg.ca>
Date:   Wed Apr 29 15:37:32 2020 -0400

    16349: Set time zone to UTC at db connection level.
    
    This covers the trash time comparisons in materialized_permission_view
    as well as explicit uses of CURRENT_TIMESTAMP in Rails queries.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>

diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index c7d7ad814..83a233cd5 100644
--- a/services/api/app/controllers/application_controller.rb
+++ b/services/api/app/controllers/application_controller.rb
@@ -386,8 +386,8 @@ class ApplicationController < ActionController::Base
       @read_auths += ApiClientAuthorization
         .includes(:user)
         .where('api_token IN (?) AND
-                (expires_at IS NULL OR expires_at > CURRENT_TIMESTAMP AT TIME ZONE ?)',
-               secrets, 'UTC')
+                (expires_at IS NULL OR expires_at > CURRENT_TIMESTAMP)',
+               secrets)
         .to_a
     end
     @read_auths.select! { |auth| auth.scopes_allow_request? request }
diff --git a/services/api/app/models/api_client_authorization.rb b/services/api/app/models/api_client_authorization.rb
index a4bf0cd5b..503dfc50d 100644
--- a/services/api/app/models/api_client_authorization.rb
+++ b/services/api/app/models/api_client_authorization.rb
@@ -158,7 +158,7 @@ class ApiClientAuthorization < ArvadosModel
       # fast path: look up the token in the local database
       auth = ApiClientAuthorization.
              includes(:user, :api_client).
-             where('uuid=? and (expires_at is null or expires_at > CURRENT_TIMESTAMP AT TIME ZONE ?)', token_uuid, 'UTC').
+             where('uuid=? and (expires_at is null or expires_at > CURRENT_TIMESTAMP)', token_uuid).
              first
       if auth && auth.user &&
          (secret == auth.api_token ||
@@ -284,7 +284,7 @@ class ApiClientAuthorization < ArvadosModel
       # token is not a 'v2' token
       auth = ApiClientAuthorization.
                includes(:user, :api_client).
-               where('api_token=? and (expires_at is null or expires_at > CURRENT_TIMESTAMP AT TIME ZONE ?)', token, 'UTC').
+               where('api_token=? and (expires_at is null or expires_at > CURRENT_TIMESTAMP)', token).
                first
       if auth && auth.user
         return auth
diff --git a/services/api/config/initializers/time_zone.rb b/services/api/config/initializers/time_zone.rb
new file mode 100644
index 000000000..cedd8f3e4
--- /dev/null
+++ b/services/api/config/initializers/time_zone.rb
@@ -0,0 +1,15 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+ActiveRecord::Base.connection.class.set_callback :checkout, :after do
+  # If the database connection is in a time zone other than UTC,
+  # "timestamp" values don't behave as desired.
+  #
+  # For example, ['select now() > ?', Time.now] returns true in time
+  # zones +0100 and UTC (which makes sense since Time.now is evaluated
+  # before now()), but false in time zone -0100 (now() returns an
+  # earlier clock time, and its time zone is dropped when comparing to
+  # a "timestamp without time zone").
+  raw_connection.sync_exec("SET TIME ZONE 'UTC'")
+end
diff --git a/services/api/lib/create_superuser_token.rb b/services/api/lib/create_superuser_token.rb
index c1530162e..57eac048a 100755
--- a/services/api/lib/create_superuser_token.rb
+++ b/services/api/lib/create_superuser_token.rb
@@ -40,7 +40,7 @@ module CreateSuperUserToken
           where(user_id: system_user.id).
           where(api_client_id: apiClient.id).
           where_serialized(:scopes, ['all']).
-          where('(expires_at IS NULL OR expires_at > CURRENT_TIMESTAMP AT TIME ZONE ?)', 'UTC').
+          where('(expires_at IS NULL OR expires_at > CURRENT_TIMESTAMP)').
           first
 
         # none exist; create one with the supplied token
diff --git a/services/api/test/unit/time_zone_test.rb b/services/api/test/unit/time_zone_test.rb
new file mode 100644
index 000000000..60ca6b50b
--- /dev/null
+++ b/services/api/test/unit/time_zone_test.rb
@@ -0,0 +1,15 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+require 'test_helper'
+
+class TimeZoneTest < ActiveSupport::TestCase
+  test "Database connection time zone" do
+    # This is pointless if the testing host is already using the UTC
+    # time zone.  But if not, the test confirms that
+    # config/initializers/time_zone.rb has successfully changed the
+    # database connection time zone to UTC.
+    assert_equal('UTC', ActiveRecord::Base.connection.select_value("show timezone"))
+  end
+end

commit 04e3809b09008c4009a9bc74120952d753244884
Author: Tom Clegg <tom at tomclegg.ca>
Date:   Mon Apr 27 16:57:55 2020 -0400

    16349: Skip converting DB timestamps to & from localtime.
    
    The previous code already returned the correct time. This change
    merely avoids converting the time to the local time zone in
    PostgreSQL, and back to UTC again in Rails.
    
    This might also avoid rare problems in unusual edge cases where Rails
    and PostgreSQL session use different time zones, like a configuration
    mishap or a race during a daylight savings time transition.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>

diff --git a/services/api/lib/db_current_time.rb b/services/api/lib/db_current_time.rb
index fdb664152..5e1634ecb 100644
--- a/services/api/lib/db_current_time.rb
+++ b/services/api/lib/db_current_time.rb
@@ -3,9 +3,13 @@
 # SPDX-License-Identifier: AGPL-3.0
 
 module DbCurrentTime
-  CURRENT_TIME_SQL = "SELECT clock_timestamp()"
+  CURRENT_TIME_SQL = "SELECT clock_timestamp() AT TIME ZONE 'UTC'"
 
   def db_current_time
-    Time.parse(ActiveRecord::Base.connection.select_value(CURRENT_TIME_SQL)).to_time
+    Time.parse(ActiveRecord::Base.connection.select_value(CURRENT_TIME_SQL) + " +0000")
+  end
+
+  def db_transaction_time
+    Time.parse(ActiveRecord::Base.connection.select_value("SELECT current_timestamp AT TIME ZONE 'UTC'") + " +0000")
   end
 end

commit 15f61e8bce0d67014f02852a05bc462e09b80435
Author: Tom Clegg <tom at tomclegg.ca>
Date:   Mon Apr 27 16:37:49 2020 -0400

    16349: Fix TZ-sensitive comparison in token expiry checks.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>

diff --git a/lib/controller/federation.go b/lib/controller/federation.go
index c0d127284..ac239fb9b 100644
--- a/lib/controller/federation.go
+++ b/lib/controller/federation.go
@@ -166,7 +166,7 @@ func (h *Handler) validateAPItoken(req *http.Request, token string) (*CurrentUse
 	}
 	user.Authorization.APIToken = token
 	var scopes string
-	err = db.QueryRowContext(req.Context(), `SELECT api_client_authorizations.uuid, api_client_authorizations.scopes, users.uuid FROM api_client_authorizations JOIN users on api_client_authorizations.user_id=users.id WHERE api_token=$1 AND (expires_at IS NULL OR expires_at > current_timestamp) LIMIT 1`, token).Scan(&user.Authorization.UUID, &scopes, &user.UUID)
+	err = db.QueryRowContext(req.Context(), `SELECT api_client_authorizations.uuid, api_client_authorizations.scopes, users.uuid FROM api_client_authorizations JOIN users on api_client_authorizations.user_id=users.id WHERE api_token=$1 AND (expires_at IS NULL OR expires_at > current_timestamp AT TIME ZONE 'UTC') LIMIT 1`, token).Scan(&user.Authorization.UUID, &scopes, &user.UUID)
 	if err == sql.ErrNoRows {
 		ctxlog.FromContext(req.Context()).Debugf("validateAPItoken(%s): not found in database", token)
 		return nil, false, nil
@@ -214,9 +214,9 @@ func (h *Handler) createAPItoken(req *http.Request, userUUID string, scopes []st
 (uuid, api_token, expires_at, scopes,
 user_id,
 api_client_id, created_at, updated_at)
-VALUES ($1, $2, CURRENT_TIMESTAMP + INTERVAL '2 weeks', $3,
+VALUES ($1, $2, CURRENT_TIMESTAMP AT TIME ZONE 'UTC' + INTERVAL '2 weeks', $3,
 (SELECT id FROM users WHERE users.uuid=$4 LIMIT 1),
-0, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)`,
+0, CURRENT_TIMESTAMP AT TIME ZONE 'UTC', CURRENT_TIMESTAMP AT TIME ZONE 'UTC')`,
 		uuid, token, string(scopesjson), userUUID)
 
 	if err != nil {
diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index 83a233cd5..c7d7ad814 100644
--- a/services/api/app/controllers/application_controller.rb
+++ b/services/api/app/controllers/application_controller.rb
@@ -386,8 +386,8 @@ class ApplicationController < ActionController::Base
       @read_auths += ApiClientAuthorization
         .includes(:user)
         .where('api_token IN (?) AND
-                (expires_at IS NULL OR expires_at > CURRENT_TIMESTAMP)',
-               secrets)
+                (expires_at IS NULL OR expires_at > CURRENT_TIMESTAMP AT TIME ZONE ?)',
+               secrets, 'UTC')
         .to_a
     end
     @read_auths.select! { |auth| auth.scopes_allow_request? request }
diff --git a/services/api/app/models/api_client_authorization.rb b/services/api/app/models/api_client_authorization.rb
index 503dfc50d..a4bf0cd5b 100644
--- a/services/api/app/models/api_client_authorization.rb
+++ b/services/api/app/models/api_client_authorization.rb
@@ -158,7 +158,7 @@ class ApiClientAuthorization < ArvadosModel
       # fast path: look up the token in the local database
       auth = ApiClientAuthorization.
              includes(:user, :api_client).
-             where('uuid=? and (expires_at is null or expires_at > CURRENT_TIMESTAMP)', token_uuid).
+             where('uuid=? and (expires_at is null or expires_at > CURRENT_TIMESTAMP AT TIME ZONE ?)', token_uuid, 'UTC').
              first
       if auth && auth.user &&
          (secret == auth.api_token ||
@@ -284,7 +284,7 @@ class ApiClientAuthorization < ArvadosModel
       # token is not a 'v2' token
       auth = ApiClientAuthorization.
                includes(:user, :api_client).
-               where('api_token=? and (expires_at is null or expires_at > CURRENT_TIMESTAMP)', token).
+               where('api_token=? and (expires_at is null or expires_at > CURRENT_TIMESTAMP AT TIME ZONE ?)', token, 'UTC').
                first
       if auth && auth.user
         return auth
diff --git a/services/api/lib/create_superuser_token.rb b/services/api/lib/create_superuser_token.rb
index 57eac048a..c1530162e 100755
--- a/services/api/lib/create_superuser_token.rb
+++ b/services/api/lib/create_superuser_token.rb
@@ -40,7 +40,7 @@ module CreateSuperUserToken
           where(user_id: system_user.id).
           where(api_client_id: apiClient.id).
           where_serialized(:scopes, ['all']).
-          where('(expires_at IS NULL OR expires_at > CURRENT_TIMESTAMP)').
+          where('(expires_at IS NULL OR expires_at > CURRENT_TIMESTAMP AT TIME ZONE ?)', 'UTC').
           first
 
         # none exist; create one with the supplied token

commit cc6d74141a399b9f8401e1388bf6913114cad5bc
Author: Tom Clegg <tom at tomclegg.ca>
Date:   Wed Apr 22 16:50:06 2020 -0400

    16343: Fix handling of local CR creation when LoginCluster is used.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>

diff --git a/lib/controller/fed_containers.go b/lib/controller/fed_containers.go
index a923f757f..c62cea116 100644
--- a/lib/controller/fed_containers.go
+++ b/lib/controller/fed_containers.go
@@ -42,13 +42,11 @@ func remoteContainerRequestCreate(
 		return true
 	}
 
-	if *clusterId == "" {
-		*clusterId = h.handler.Cluster.ClusterID
-	}
-
-	if strings.HasPrefix(currentUser.Authorization.UUID, h.handler.Cluster.ClusterID) &&
-		*clusterId == h.handler.Cluster.ClusterID {
-		// local user submitting container request to local cluster
+	if *clusterId == "" || *clusterId == h.handler.Cluster.ClusterID {
+		// Submitting container request to local cluster. No
+		// need to set a runtime_token (rails api will create
+		// one when the container runs) or do a remote cluster
+		// request.
 		return false
 	}
 

commit 1334a03fb592d13f72ee5379cc4ac1199240d2e7
Author: Tom Clegg <tom at tomclegg.ca>
Date:   Wed Apr 22 16:31:06 2020 -0400

    16343: Add debug logs in token checking code.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>

diff --git a/lib/controller/federation.go b/lib/controller/federation.go
index 674183dcc..c0d127284 100644
--- a/lib/controller/federation.go
+++ b/lib/controller/federation.go
@@ -19,6 +19,7 @@ import (
 
 	"git.arvados.org/arvados.git/sdk/go/arvados"
 	"git.arvados.org/arvados.git/sdk/go/auth"
+	"git.arvados.org/arvados.git/sdk/go/ctxlog"
 	"github.com/jmcvetta/randutil"
 )
 
@@ -153,6 +154,7 @@ func (h *Handler) validateAPItoken(req *http.Request, token string) (*CurrentUse
 	user := CurrentUser{Authorization: arvados.APIClientAuthorization{APIToken: token}}
 	db, err := h.db(req)
 	if err != nil {
+		ctxlog.FromContext(req.Context()).WithError(err).Debugf("validateAPItoken(%s): database error", token)
 		return nil, false, err
 	}
 
@@ -166,18 +168,23 @@ func (h *Handler) validateAPItoken(req *http.Request, token string) (*CurrentUse
 	var scopes string
 	err = db.QueryRowContext(req.Context(), `SELECT api_client_authorizations.uuid, api_client_authorizations.scopes, users.uuid FROM api_client_authorizations JOIN users on api_client_authorizations.user_id=users.id WHERE api_token=$1 AND (expires_at IS NULL OR expires_at > current_timestamp) LIMIT 1`, token).Scan(&user.Authorization.UUID, &scopes, &user.UUID)
 	if err == sql.ErrNoRows {
+		ctxlog.FromContext(req.Context()).Debugf("validateAPItoken(%s): not found in database", token)
 		return nil, false, nil
 	} else if err != nil {
+		ctxlog.FromContext(req.Context()).WithError(err).Debugf("validateAPItoken(%s): database error", token)
 		return nil, false, err
 	}
 	if uuid != "" && user.Authorization.UUID != uuid {
 		// secret part matches, but UUID doesn't -- somewhat surprising
+		ctxlog.FromContext(req.Context()).Debugf("validateAPItoken(%s): secret part found, but with different UUID: %s", token, user.Authorization.UUID)
 		return nil, false, nil
 	}
 	err = json.Unmarshal([]byte(scopes), &user.Authorization.Scopes)
 	if err != nil {
+		ctxlog.FromContext(req.Context()).WithError(err).Debugf("validateAPItoken(%s): error parsing scopes from db", token)
 		return nil, false, err
 	}
+	ctxlog.FromContext(req.Context()).Debugf("validateAPItoken(%s): ok", token)
 	return &user, true, nil
 }
 
diff --git a/services/api/app/models/api_client_authorization.rb b/services/api/app/models/api_client_authorization.rb
index 77fc0a45a..503dfc50d 100644
--- a/services/api/app/models/api_client_authorization.rb
+++ b/services/api/app/models/api_client_authorization.rb
@@ -164,6 +164,9 @@ class ApiClientAuthorization < ArvadosModel
          (secret == auth.api_token ||
           secret == OpenSSL::HMAC.hexdigest('sha1', auth.api_token, remote))
         # found it
+        if token_uuid[0..4] != Rails.configuration.ClusterID
+          Rails.logger.debug "found cached remote token #{token_uuid} with secret #{secret} in local db"
+        end
         return auth
       end
 
@@ -274,6 +277,7 @@ class ApiClientAuthorization < ArvadosModel
                                 api_token: secret,
                                 api_client_id: 0,
                                 expires_at: Time.now + Rails.configuration.Login.RemoteTokenRefresh)
+        Rails.logger.debug "cached remote token #{token_uuid} with secret #{secret} in local db"
       end
       return auth
     else

commit cfa70fa206cef453a736187c1b23c8360f010cb8
Author: Peter Amstutz <peter.amstutz at curii.com>
Date:   Tue Apr 28 16:44:30 2020 -0400

    16366: "InternalURLs" and "AccessViaHosts" are consistent in install
    
    refs #16366
    
    Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>

diff --git a/doc/install/configure-azure-blob-storage.html.textile.liquid b/doc/install/configure-azure-blob-storage.html.textile.liquid
index 8c5098abe..2ccec586e 100644
--- a/doc/install/configure-azure-blob-storage.html.textile.liquid
+++ b/doc/install/configure-azure-blob-storage.html.textile.liquid
@@ -67,8 +67,8 @@ Volumes are configured in the @Volumes@ section of the cluster configuration fil
           # If the AccessViaHosts section is empty or omitted, all
           # keepstore servers will have read/write access to the
           # volume.
-          "http://<span class="userinput">keep0.ClusterID.example.com</span>:25107/": {}
-          "http://<span class="userinput">keep1.ClusterID.example.com</span>:25107/": {ReadOnly: true}
+          "http://<span class="userinput">keep0.ClusterID.example.com</span>:25107": {}
+          "http://<span class="userinput">keep1.ClusterID.example.com</span>:25107": {ReadOnly: true}
 
         Driver: <span class="userinput">Azure</span>
         DriverParameters:
diff --git a/doc/install/configure-s3-object-storage.html.textile.liquid b/doc/install/configure-s3-object-storage.html.textile.liquid
index 9708ea5cd..e953f660f 100644
--- a/doc/install/configure-s3-object-storage.html.textile.liquid
+++ b/doc/install/configure-s3-object-storage.html.textile.liquid
@@ -25,8 +25,8 @@ Volumes are configured in the @Volumes@ section of the cluster configuration fil
           # If the AccessViaHosts section is empty or omitted, all
           # keepstore servers will have read/write access to the
           # volume.
-          "http://<span class="userinput">keep0.ClusterID.example.com</span>:25107/": {}
-          "http://<span class="userinput">keep1.ClusterID.example.com</span>:25107/": {ReadOnly: true}
+          "http://<span class="userinput">keep0.ClusterID.example.com</span>:25107": {}
+          "http://<span class="userinput">keep1.ClusterID.example.com</span>:25107": {ReadOnly: true}
 
         Driver: <span class="userinput">S3</span>
         DriverParameters:
diff --git a/doc/install/install-keepstore.html.textile.liquid b/doc/install/install-keepstore.html.textile.liquid
index fedeb3c3f..869ca15d9 100644
--- a/doc/install/install-keepstore.html.textile.liquid
+++ b/doc/install/install-keepstore.html.textile.liquid
@@ -61,8 +61,8 @@ Add each keepstore server to the @Services.Keepstore@ section of @/etc/arvados/c
       Keepstore:
         # No ExternalURL because they are only accessed by the internal subnet.
         InternalURLs:
-          "http://<span class="userinput">keep0.ClusterID.example.com</span>:25107/": {}
-          "http://<span class="userinput">keep1.ClusterID.example.com</span>:25107/": {}
+          "http://<span class="userinput">keep0.ClusterID.example.com</span>:25107": {}
+          "http://<span class="userinput">keep1.ClusterID.example.com</span>:25107": {}
           # and so forth
 </code></pre>
 </notextile>

commit 6260e1b0d7778d247880357556debe51acf71cf4
Author: Ward Vandewege <ward at jhvc.com>
Date:   Thu May 14 20:39:13 2020 -0400

    16434: the python3-arvados-cwl-runner packages on Debian 10 and Ubuntu
    18.04 (and later, presumably) have a dependency on the
    python3-distutils package.
    
    Arvados-DCO-1.1-Signed-off-by: Ward Vandewege <ward at jhvc.com>

diff --git a/sdk/cwl/fpm-info.sh b/sdk/cwl/fpm-info.sh
index 5c47532db..977f53da6 100644
--- a/sdk/cwl/fpm-info.sh
+++ b/sdk/cwl/fpm-info.sh
@@ -9,6 +9,9 @@ case "$TARGET" in
     debian* | ubuntu*)
         fpm_depends+=(libcurl3-gnutls libpython2.7)
         ;;
+    debian* | ubuntu*)
+        fpm_depends+=(libcurl3-gnutls python3-distutils)
+        ;;
 esac
 
 fpm_args+=(--conflicts=python-cwltool --conflicts=cwltool)

commit e58c5263c3529a9f0fe586852612c991b5a8ceec
Author: Ward Vandewege <ward at jhvc.com>
Date:   Thu May 14 20:40:14 2020 -0400

    Fix comment typos in run-library.sh
    
    No issue #
    
    Arvados-DCO-1.1-Signed-off-by: Ward Vandewege <ward at jhvc.com>

diff --git a/build/run-library.sh b/build/run-library.sh
index eb6ee1b1f..de51d19bc 100755
--- a/build/run-library.sh
+++ b/build/run-library.sh
@@ -465,7 +465,7 @@ fpm_build_virtualenv () {
   fi
 
   # arvados-python-client sdist should always be built, to be available
-  # for other dependant packages.
+  # for other dependent packages.
   if [[ -n "$ONLY_BUILD" ]] && [[ "arvados-python-client" != "$PKG" ]] && [[ "$PYTHON_PKG" != "$ONLY_BUILD" ]] && [[ "$PKG" != "$ONLY_BUILD" ]]; then
     return 0
   fi
@@ -613,7 +613,7 @@ fpm_build_virtualenv () {
   # 12271 - As FPM-generated packages don't include scripts by default, the
   # packages cleanup on upgrade depends on files being listed on the %files
   # section in the generated SPEC files. To remove DIRECTORIES, they need to
-  # be listed in that sectiontoo, so we need to add this parameter to properly
+  # be listed in that section too, so we need to add this parameter to properly
   # remove lingering dirs. But this only works for python2: if used on
   # python33, it includes dirs like /opt/rh/python33 that belong to
   # other packages.

commit 0267966c0452f398b6f56077e426584446ad403e
Author: Ward Vandewege <ward at jhvc.com>
Date:   Thu May 14 20:40:55 2020 -0400

    16434: add a basic execution test for the python3-arvados-cwl-runner packages
    
    Arvados-DCO-1.1-Signed-off-by: Ward Vandewege <ward at jhvc.com>

diff --git a/build/package-testing/test-package-python3-arvados-cwl-runner.sh b/build/package-testing/test-package-python3-arvados-cwl-runner.sh
new file mode 100755
index 000000000..99327c016
--- /dev/null
+++ b/build/package-testing/test-package-python3-arvados-cwl-runner.sh
@@ -0,0 +1,8 @@
+#!/bin/sh
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+set -e
+
+arvados-cwl-runner --version

commit 5c2cfee366a58d54fc39f47ccd84a5db5d96d856
Author: Tom Clegg <tom at tomclegg.ca>
Date:   Wed May 6 15:39:30 2020 -0400

    16387: Allow setting is_active=false only on LoginCluster users.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>

diff --git a/services/api/app/controllers/arvados/v1/users_controller.rb b/services/api/app/controllers/arvados/v1/users_controller.rb
index 647b62fea..ede7572e6 100644
--- a/services/api/app/controllers/arvados/v1/users_controller.rb
+++ b/services/api/app/controllers/arvados/v1/users_controller.rb
@@ -51,9 +51,10 @@ class Arvados::V1::UsersController < ApplicationController
       @object = current_user
     end
     if not @object.is_active
-      if @object.uuid[0..4] != Rails.configuration.ClusterID
-        logger.warn "Remote user #{@object.uuid} called users.activate"
-        raise ArgumentError.new "cannot activate remote account"
+      if @object.uuid[0..4] == Rails.configuration.Login.LoginCluster &&
+         @object.uuid[0..4] != Rails.configuration.ClusterID
+        logger.warn "Local user #{@object.uuid} called users#activate but only LoginCluster can do that"
+        raise ArgumentError.new "cannot activate user #{@object.uuid} here, only the #{@object.uuid[0..4]} cluster can do that"
       elsif not (current_user.is_admin or @object.is_invited)
         logger.warn "User #{@object.uuid} called users.activate " +
           "but is not invited"
diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index 3f0a97062..609e26413 100644
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -238,10 +238,15 @@ class User < ArvadosModel
   end
 
   def must_unsetup_to_deactivate
-    if self.is_active_changed? &&
+    if !self.new_record? &&
+       self.uuid[0..4] == Rails.configuration.Login.LoginCluster &&
+       self.uuid[0..4] != Rails.configuration.ClusterID
+      # OK to update our local record to whatever the LoginCluster
+      # reports, because self-activate is not allowed.
+      return
+    elsif self.is_active_changed? &&
        self.is_active_was &&
-       !self.is_active &&
-       self.uuid[0..4] == Rails.configuration.ClusterID
+       !self.is_active
 
       group = Group.where(name: 'All users').select do |g|
         g[:uuid].match(/-f+$/)

commit e94ed22c9f40ecafd3cb1ee1349eaee018b4d883
Author: Tom Clegg <tom at tomclegg.ca>
Date:   Mon May 4 10:53:21 2020 -0400

    16387: Allow batch update to set is_active=false for a remote user.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>

diff --git a/services/api/app/controllers/arvados/v1/users_controller.rb b/services/api/app/controllers/arvados/v1/users_controller.rb
index 6a5fbbc50..647b62fea 100644
--- a/services/api/app/controllers/arvados/v1/users_controller.rb
+++ b/services/api/app/controllers/arvados/v1/users_controller.rb
@@ -51,7 +51,10 @@ class Arvados::V1::UsersController < ApplicationController
       @object = current_user
     end
     if not @object.is_active
-      if not (current_user.is_admin or @object.is_invited)
+      if @object.uuid[0..4] != Rails.configuration.ClusterID
+        logger.warn "Remote user #{@object.uuid} called users.activate"
+        raise ArgumentError.new "cannot activate remote account"
+      elsif not (current_user.is_admin or @object.is_invited)
         logger.warn "User #{@object.uuid} called users.activate " +
           "but is not invited"
         raise ArgumentError.new "Cannot activate without being invited."
diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index 310c2ca69..3f0a97062 100644
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -239,8 +239,9 @@ class User < ArvadosModel
 
   def must_unsetup_to_deactivate
     if self.is_active_changed? &&
-       self.is_active_was == true &&
-       !self.is_active
+       self.is_active_was &&
+       !self.is_active &&
+       self.uuid[0..4] == Rails.configuration.ClusterID
 
       group = Group.where(name: 'All users').select do |g|
         g[:uuid].match(/-f+$/)

commit d7597b98e702ad5ab9c059ee3c3528b9d3bda292
Author: Tom Clegg <tom at tomclegg.ca>
Date:   Fri May 1 15:58:58 2020 -0400

    Add StartedAt and FinishedAt fields to arvados.Container.
    
    No issue #
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>

diff --git a/sdk/go/arvados/container.go b/sdk/go/arvados/container.go
index 653312d86..fddea2c00 100644
--- a/sdk/go/arvados/container.go
+++ b/sdk/go/arvados/container.go
@@ -28,6 +28,8 @@ type Container struct {
 	SchedulingParameters SchedulingParameters   `json:"scheduling_parameters"`
 	ExitCode             int                    `json:"exit_code"`
 	RuntimeStatus        map[string]interface{} `json:"runtime_status"`
+	StartedAt            *time.Time             `json:"started_at"`  // nil if not yet started
+	FinishedAt           *time.Time             `json:"finished_at"` // nil if not yet finished
 }
 
 // Container is an arvados#container resource.

commit 6b11d6756340eb9ba557b4148c4e5df8d46660b9
Author: Ward Vandewege <ward at jhvc.com>
Date:   Thu May 14 20:57:49 2020 -0400

    bugfix: fix arvados-node-manager package build
    
    refs #16373
    
    Arvados-DCO-1.1-Signed-off-by: Ward Vandewege <ward at jhvc.com>

diff --git a/build/run-library.sh b/build/run-library.sh
index 3a1ed1c1a..eb6ee1b1f 100755
--- a/build/run-library.sh
+++ b/build/run-library.sh
@@ -673,9 +673,9 @@ fpm_build_virtualenv () {
   done
 
   # make sure the systemd service file ends up in the right place
-  # currently only used by arvados-docker-cleaner
+  # used by arvados-docker-cleaner and arvados-node-manager
   if [[ -e "${systemd_unit}" ]]; then
-    COMMAND_ARR+=("usr/share/python3/dist/$PKG/share/doc/$PKG/$PKG.service=/lib/systemd/system/$PKG.service")
+    COMMAND_ARR+=("usr/share/$python/dist/$PKG/share/doc/$PKG/$PKG.service=/lib/systemd/system/$PKG.service")
   fi
 
   COMMAND_ARR+=("${fpm_args[@]}")

commit 26f06e7a69aaf7c84995d452c16f7a97ddf6032d
Author: Ward Vandewege <ward at jhvc.com>
Date:   Thu May 7 10:20:50 2020 -0400

    Bugfix: make the arvados-docker-cleaner packages install their systemd
    unit file in the correct directory.
    
    closes #16373
    
    Arvados-DCO-1.1-Signed-off-by: Ward Vandewege <ward at jhvc.com>

diff --git a/build/run-library.sh b/build/run-library.sh
index ac5dc718b..3a1ed1c1a 100755
--- a/build/run-library.sh
+++ b/build/run-library.sh
@@ -638,7 +638,8 @@ fpm_build_virtualenv () {
   COMMAND_ARR+=('-n' "$PYTHON_PKG")
   COMMAND_ARR+=('-C' "build")
 
-  if [[ -e "$WORKSPACE/$PKG_DIR/$PKG.service" ]]; then
+  systemd_unit="$WORKSPACE/$PKG_DIR/$PKG.service"
+  if [[ -e "${systemd_unit}" ]]; then
     COMMAND_ARR+=('--after-install' "${WORKSPACE}/build/go-python-package-scripts/postinst")
     COMMAND_ARR+=('--before-remove' "${WORKSPACE}/build/go-python-package-scripts/prerm")
   fi
@@ -671,6 +672,12 @@ fpm_build_virtualenv () {
     COMMAND_ARR+=('--depends' "$i")
   done
 
+  # make sure the systemd service file ends up in the right place
+  # currently only used by arvados-docker-cleaner
+  if [[ -e "${systemd_unit}" ]]; then
+    COMMAND_ARR+=("usr/share/python3/dist/$PKG/share/doc/$PKG/$PKG.service=/lib/systemd/system/$PKG.service")
+  fi
+
   COMMAND_ARR+=("${fpm_args[@]}")
 
   # Make sure to install all our package binaries in /usr/bin.

commit 1b9451fc5c3765191cb17bfb4ed4468ecbf077aa
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date:   Thu May 21 20:08:40 2020 -0300

    16435: Updates the documentation.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>

diff --git a/doc/architecture/index.html.textile.liquid b/doc/architecture/index.html.textile.liquid
index c7ea3268e..705048cd6 100644
--- a/doc/architecture/index.html.textile.liquid
+++ b/doc/architecture/index.html.textile.liquid
@@ -56,4 +56,4 @@ table(table table-bordered table-condensed).
 |keep-block-check|Given a list of keep block locators, check that each block exists on one of the configured keepstore servers and verify the block hash.|
 |keep-exercise|Benchmarking tool to test throughput and reliability of keepstores under various usage patterns.|
 |keep-rsync|Get lists of blocks from two clusters, copy blocks which exist on source cluster but are missing from destination cluster.|
-|sync-groups|Take a CSV file listing (group, username) pairs and synchronize membership in Arvados groups.|
+|sync-groups|Take a CSV file listing with (group, user, permission) records and synchronize membership in Arvados groups.|
diff --git a/doc/user/topics/arvados-sync-groups.html.textile.liquid b/doc/user/topics/arvados-sync-groups.html.textile.liquid
index 9a609039b..7d831bf04 100644
--- a/doc/user/topics/arvados-sync-groups.html.textile.liquid
+++ b/doc/user/topics/arvados-sync-groups.html.textile.liquid
@@ -15,10 +15,12 @@ h1. Using arvados-sync-groups
 
 This tool reads a CSV (comma-separated values) file having information about external groups and their members. When running it for the first time, it'll create a special group named 'Externally synchronized groups' meant to be the parent of all the remote groups.
 
-Every line on the file should have 2 values: a group name and a local user identifier, meaning that the named user is a member of the group. The tool will create the group if it doesn't exist, and add the user to it. If group member is not present on the input file, the account will be removed from the group.
+Every line on the file should have 3 values: a group name, a local user identifier and a permission level, meaning that the named user is a member of the group with the provided permission. The tool will create the group if it doesn't exist, and add the user to it. If any group member is not present on the input file, it will be removed from the group.
 
 Users can be identified by their email address or username: the tool will check if every user exist on the system, and report back when not found. Groups on the other hand, are identified by their name.
 
+Permission level can be one of the following: @can_read@, @can_write@ or @can_manage@, giving the group member read, read/write or managing privileges on the group. For backwards compatibility purposes, if any record omits the third (permission) field, it will default to @can_write@ permission. You can read more about permissions on the "group management admin guide":/admin/group-management.html.
+
 This tool is designed to be run periodically reading a file created by a remote auth system (ie: LDAP) dump script, applying what's included on the file as the source of truth.
 
 

commit 3889fe0ff14d710817f53ba4d517445df89c2342
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date:   Thu May 21 19:00:37 2020 -0300

    16435: Adds & updates tests.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>

diff --git a/tools/sync-groups/sync-groups_test.go b/tools/sync-groups/sync-groups_test.go
index 77d9756ff..9eec6b6d9 100644
--- a/tools/sync-groups/sync-groups_test.go
+++ b/tools/sync-groups/sync-groups_test.go
@@ -106,7 +106,7 @@ func MakeTempCSVFile(data [][]string) (f *os.File, err error) {
 }
 
 // GroupMembershipExists checks that both needed links exist between user and group
-func GroupMembershipExists(ac *arvados.Client, userUUID string, groupUUID string) bool {
+func GroupMembershipExists(ac *arvados.Client, userUUID string, groupUUID string, perm string) bool {
 	ll := LinkList{}
 	// Check Group -> User can_read permission
 	params := arvados.ResourceListParams{
@@ -145,7 +145,7 @@ func GroupMembershipExists(ac *arvados.Client, userUUID string, groupUUID string
 		}, {
 			Attr:     "name",
 			Operator: "=",
-			Operand:  "can_write",
+			Operand:  perm,
 		}, {
 			Attr:     "tail_uuid",
 			Operator: "=",
@@ -259,7 +259,7 @@ func (s *TestSuite) TestIgnoreSpaces(c *C) {
 		groupUUID, err := RemoteGroupExists(s.cfg, groupName)
 		c.Assert(err, IsNil)
 		c.Assert(groupUUID, Not(Equals), "")
-		c.Assert(GroupMembershipExists(s.cfg.Client, activeUserUUID, groupUUID), Equals, true)
+		c.Assert(GroupMembershipExists(s.cfg.Client, activeUserUUID, groupUUID, "can_write"), Equals, true)
 	}
 }
 
@@ -279,6 +279,83 @@ func (s *TestSuite) TestWrongNumberOfFields(c *C) {
 	}
 }
 
+// Check different membership permissions
+func (s *TestSuite) TestMembershipLevels(c *C) {
+	userEmail := s.users[arvadostest.ActiveUserUUID].Email
+	userUUID := s.users[arvadostest.ActiveUserUUID].UUID
+	data := [][]string{
+		{"TestGroup1", userEmail, "can_read"},
+		{"TestGroup2", userEmail, "can_write"},
+		{"TestGroup3", userEmail, "can_manage"},
+		{"TestGroup4", userEmail, "invalid_permission"},
+	}
+	tmpfile, err := MakeTempCSVFile(data)
+	c.Assert(err, IsNil)
+	defer os.Remove(tmpfile.Name()) // clean up
+	s.cfg.Path = tmpfile.Name()
+	err = doMain(s.cfg)
+	c.Assert(err, IsNil)
+	for _, record := range data {
+		groupName := record[0]
+		permLevel := record[2]
+		if permLevel != "invalid_permission" {
+			groupUUID, err := RemoteGroupExists(s.cfg, groupName)
+			c.Assert(err, IsNil)
+			c.Assert(groupUUID, Not(Equals), "")
+			c.Assert(GroupMembershipExists(s.cfg.Client, userUUID, groupUUID, permLevel), Equals, true)
+		} else {
+			groupUUID, err := RemoteGroupExists(s.cfg, groupName)
+			c.Assert(err, IsNil)
+			c.Assert(groupUUID, Equals, "")
+		}
+	}
+}
+
+// Check membership level change
+func (s *TestSuite) TestMembershipLevelUpdate(c *C) {
+	userEmail := s.users[arvadostest.ActiveUserUUID].Email
+	userUUID := s.users[arvadostest.ActiveUserUUID].UUID
+	groupName := "TestGroup1"
+	// Give read permissions
+	tmpfile, err := MakeTempCSVFile([][]string{{groupName, userEmail, "can_read"}})
+	c.Assert(err, IsNil)
+	defer os.Remove(tmpfile.Name()) // clean up
+	s.cfg.Path = tmpfile.Name()
+	err = doMain(s.cfg)
+	c.Assert(err, IsNil)
+	// Check permissions
+	groupUUID, err := RemoteGroupExists(s.cfg, groupName)
+	c.Assert(err, IsNil)
+	c.Assert(groupUUID, Not(Equals), "")
+	c.Assert(GroupMembershipExists(s.cfg.Client, userUUID, groupUUID, "can_read"), Equals, true)
+	c.Assert(GroupMembershipExists(s.cfg.Client, userUUID, groupUUID, "can_write"), Equals, false)
+	c.Assert(GroupMembershipExists(s.cfg.Client, userUUID, groupUUID, "can_manage"), Equals, false)
+
+	// Give write permissions
+	tmpfile, err = MakeTempCSVFile([][]string{{groupName, userEmail, "can_write"}})
+	c.Assert(err, IsNil)
+	defer os.Remove(tmpfile.Name()) // clean up
+	s.cfg.Path = tmpfile.Name()
+	err = doMain(s.cfg)
+	c.Assert(err, IsNil)
+	// Check permissions
+	c.Assert(GroupMembershipExists(s.cfg.Client, userUUID, groupUUID, "can_read"), Equals, false)
+	c.Assert(GroupMembershipExists(s.cfg.Client, userUUID, groupUUID, "can_write"), Equals, true)
+	c.Assert(GroupMembershipExists(s.cfg.Client, userUUID, groupUUID, "can_manage"), Equals, false)
+
+	// Give manage permissions
+	tmpfile, err = MakeTempCSVFile([][]string{{groupName, userEmail, "can_manage"}})
+	c.Assert(err, IsNil)
+	defer os.Remove(tmpfile.Name()) // clean up
+	s.cfg.Path = tmpfile.Name()
+	err = doMain(s.cfg)
+	c.Assert(err, IsNil)
+	// Check permissions
+	c.Assert(GroupMembershipExists(s.cfg.Client, userUUID, groupUUID, "can_read"), Equals, false)
+	c.Assert(GroupMembershipExists(s.cfg.Client, userUUID, groupUUID, "can_write"), Equals, false)
+	c.Assert(GroupMembershipExists(s.cfg.Client, userUUID, groupUUID, "can_manage"), Equals, true)
+}
+
 // The absence of a user membership on the CSV file implies its removal
 func (s *TestSuite) TestMembershipRemoval(c *C) {
 	localUserEmail := s.users[arvadostest.ActiveUserUUID].Email
@@ -302,8 +379,8 @@ func (s *TestSuite) TestMembershipRemoval(c *C) {
 		groupUUID, err := RemoteGroupExists(s.cfg, groupName)
 		c.Assert(err, IsNil)
 		c.Assert(groupUUID, Not(Equals), "")
-		c.Assert(GroupMembershipExists(s.cfg.Client, localUserUUID, groupUUID), Equals, true)
-		c.Assert(GroupMembershipExists(s.cfg.Client, remoteUserUUID, groupUUID), Equals, true)
+		c.Assert(GroupMembershipExists(s.cfg.Client, localUserUUID, groupUUID, "can_write"), Equals, true)
+		c.Assert(GroupMembershipExists(s.cfg.Client, remoteUserUUID, groupUUID, "can_write"), Equals, true)
 	}
 	// New CSV with some previous membership missing
 	data = [][]string{
@@ -320,14 +397,14 @@ func (s *TestSuite) TestMembershipRemoval(c *C) {
 	groupUUID, err := RemoteGroupExists(s.cfg, "TestGroup1")
 	c.Assert(err, IsNil)
 	c.Assert(groupUUID, Not(Equals), "")
-	c.Assert(GroupMembershipExists(s.cfg.Client, localUserUUID, groupUUID), Equals, true)
-	c.Assert(GroupMembershipExists(s.cfg.Client, remoteUserUUID, groupUUID), Equals, false)
+	c.Assert(GroupMembershipExists(s.cfg.Client, localUserUUID, groupUUID, "can_write"), Equals, true)
+	c.Assert(GroupMembershipExists(s.cfg.Client, remoteUserUUID, groupUUID, "can_write"), Equals, false)
 	// Confirm TestGroup1 memberships
 	groupUUID, err = RemoteGroupExists(s.cfg, "TestGroup2")
 	c.Assert(err, IsNil)
 	c.Assert(groupUUID, Not(Equals), "")
-	c.Assert(GroupMembershipExists(s.cfg.Client, localUserUUID, groupUUID), Equals, false)
-	c.Assert(GroupMembershipExists(s.cfg.Client, remoteUserUUID, groupUUID), Equals, true)
+	c.Assert(GroupMembershipExists(s.cfg.Client, localUserUUID, groupUUID, "can_write"), Equals, false)
+	c.Assert(GroupMembershipExists(s.cfg.Client, remoteUserUUID, groupUUID, "can_write"), Equals, true)
 }
 
 // If a group doesn't exist on the system, create it before adding users
@@ -352,7 +429,7 @@ func (s *TestSuite) TestAutoCreateGroupWhenNotExisting(c *C) {
 	c.Assert(err, IsNil)
 	c.Assert(groupUUID, Not(Equals), "")
 	// active user should be a member
-	c.Assert(GroupMembershipExists(s.cfg.Client, arvadostest.ActiveUserUUID, groupUUID), Equals, true)
+	c.Assert(GroupMembershipExists(s.cfg.Client, arvadostest.ActiveUserUUID, groupUUID, "can_write"), Equals, true)
 }
 
 // Users listed on the file that don't exist on the system are ignored
@@ -378,7 +455,7 @@ func (s *TestSuite) TestIgnoreNonexistantUsers(c *C) {
 	groupUUID, err = RemoteGroupExists(s.cfg, "TestGroup4")
 	c.Assert(err, IsNil)
 	c.Assert(groupUUID, Not(Equals), "")
-	c.Assert(GroupMembershipExists(s.cfg.Client, activeUserUUID, groupUUID), Equals, true)
+	c.Assert(GroupMembershipExists(s.cfg.Client, activeUserUUID, groupUUID, "can_write"), Equals, true)
 }
 
 // Users listed on the file that don't exist on the system are ignored
@@ -386,13 +463,16 @@ func (s *TestSuite) TestIgnoreEmptyFields(c *C) {
 	activeUserEmail := s.users[arvadostest.ActiveUserUUID].Email
 	activeUserUUID := s.users[arvadostest.ActiveUserUUID].UUID
 	// Confirm that group doesn't exist
-	groupUUID, err := RemoteGroupExists(s.cfg, "TestGroup4")
-	c.Assert(err, IsNil)
-	c.Assert(groupUUID, Equals, "")
+	for _, groupName := range []string{"TestGroup4", "TestGroup5"} {
+		groupUUID, err := RemoteGroupExists(s.cfg, groupName)
+		c.Assert(err, IsNil)
+		c.Assert(groupUUID, Equals, "")
+	}
 	// Create file & run command
 	data := [][]string{
-		{"", activeUserEmail}, // Empty field
-		{"TestGroup5", ""},    // Empty field
+		{"", activeUserEmail},               // Empty field
+		{"TestGroup5", ""},                  // Empty field
+		{"TestGroup5", activeUserEmail, ""}, // Empty 3rd field: is optional but cannot be empty
 		{"TestGroup4", activeUserEmail},
 	}
 	tmpfile, err := MakeTempCSVFile(data)
@@ -401,11 +481,15 @@ func (s *TestSuite) TestIgnoreEmptyFields(c *C) {
 	s.cfg.Path = tmpfile.Name()
 	err = doMain(s.cfg)
 	c.Assert(err, IsNil)
-	// Confirm that memberships exist
+	// Confirm that records about TestGroup5 were skipped
+	groupUUID, err := RemoteGroupExists(s.cfg, "TestGroup5")
+	c.Assert(err, IsNil)
+	c.Assert(groupUUID, Equals, "")
+	// Confirm that membership exists
 	groupUUID, err = RemoteGroupExists(s.cfg, "TestGroup4")
 	c.Assert(err, IsNil)
 	c.Assert(groupUUID, Not(Equals), "")
-	c.Assert(GroupMembershipExists(s.cfg.Client, activeUserUUID, groupUUID), Equals, true)
+	c.Assert(GroupMembershipExists(s.cfg.Client, activeUserUUID, groupUUID, "can_write"), Equals, true)
 }
 
 // Instead of emails, use username as identifier
@@ -432,5 +516,5 @@ func (s *TestSuite) TestUseUsernames(c *C) {
 	groupUUID, err = RemoteGroupExists(s.cfg, "TestGroup1")
 	c.Assert(err, IsNil)
 	c.Assert(groupUUID, Not(Equals), "")
-	c.Assert(GroupMembershipExists(s.cfg.Client, activeUserUUID, groupUUID), Equals, true)
+	c.Assert(GroupMembershipExists(s.cfg.Client, activeUserUUID, groupUUID, "can_write"), Equals, true)
 }

commit 2ea16ba491194afca89500fcee80e91dac0d3d95
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date:   Thu May 21 18:11:10 2020 -0300

    16435: Avoids creating duplicated group->user links.
    
    When a user needs a permission change, the g->u link already exists.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>

diff --git a/tools/sync-groups/sync-groups.go b/tools/sync-groups/sync-groups.go
index 2ffd61fec..9e2307b7a 100644
--- a/tools/sync-groups/sync-groups.go
+++ b/tools/sync-groups/sync-groups.go
@@ -450,8 +450,13 @@ func ProcessFile(
 			if cfg.Verbose {
 				log.Printf("Adding %q to group %q", groupMember, groupName)
 			}
-			// User wasn't a member, but should be.
-			if e := AddMemberToGroup(cfg, allUsers[userIDToUUID[groupMember]], gi.Group, groupPermission); e != nil {
+			// User permissionwasn't there, but should be. Avoid duplicating the
+			// group->user link when necessary.
+			createG2ULink := true
+			if _, ok := gi.PreviousMembers[groupMember]; ok {
+				createG2ULink = false // User is already member of the group
+			}
+			if e := AddMemberToGroup(cfg, allUsers[userIDToUUID[groupMember]], gi.Group, groupPermission, createG2ULink); e != nil {
 				err = e
 				return
 			}
@@ -498,7 +503,7 @@ func subtract(setA map[string]GroupPermissions, setB map[string]GroupPermissions
 		} else {
 			for perm := range setA[element] {
 				if _, ok := setB[element][perm]; !ok {
-					result[element][perm] = true
+					result[element] = GroupPermissions{perm: true}
 				}
 			}
 		}
@@ -716,18 +721,21 @@ func RemoveMemberLinksFromGroup(cfg *ConfigParams, user arvados.User, linkNames
 }
 
 // AddMemberToGroup create membership links
-func AddMemberToGroup(cfg *ConfigParams, user arvados.User, group arvados.Group, perm string) error {
+func AddMemberToGroup(cfg *ConfigParams, user arvados.User, group arvados.Group, perm string, createG2ULink bool) error {
 	var newLink arvados.Link
-	linkData := map[string]string{
-		"owner_uuid": cfg.SysUserUUID,
-		"link_class": "permission",
-		"name":       "can_read",
-		"tail_uuid":  group.UUID,
-		"head_uuid":  user.UUID,
-	}
-	if err := CreateLink(cfg, &newLink, linkData); err != nil {
-		userID, _ := GetUserID(user, cfg.UserID)
-		return fmt.Errorf("error adding group %q -> user %q read permission: %s", group.Name, userID, err)
+	var linkData map[string]string
+	if createG2ULink {
+		linkData = map[string]string{
+			"owner_uuid": cfg.SysUserUUID,
+			"link_class": "permission",
+			"name":       "can_read",
+			"tail_uuid":  group.UUID,
+			"head_uuid":  user.UUID,
+		}
+		if err := CreateLink(cfg, &newLink, linkData); err != nil {
+			userID, _ := GetUserID(user, cfg.UserID)
+			return fmt.Errorf("error adding group %q -> user %q read permission: %s", group.Name, userID, err)
+		}
 	}
 	linkData = map[string]string{
 		"owner_uuid": cfg.SysUserUUID,

commit 5958df985bfc8293dc94f06b2c23824cb4c8f41b
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date:   Thu May 21 16:21:12 2020 -0300

    16435: Adds support for different permission levels: read, write & manage.
    
    If the 3rd field isn't present on any record, it will fallback to 'can_write'
    to maintain backwards compatibility.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>

diff --git a/tools/sync-groups/sync-groups.go b/tools/sync-groups/sync-groups.go
index b77d8ccdd..2ffd61fec 100644
--- a/tools/sync-groups/sync-groups.go
+++ b/tools/sync-groups/sync-groups.go
@@ -26,11 +26,14 @@ type resourceList interface {
 	GetItems() []interface{}
 }
 
-// GroupInfo tracks previous and current members of a particular Group
+// GroupPermissions maps permission levels on groups (can_read, can_write, can_manage)
+type GroupPermissions map[string]bool
+
+// GroupInfo tracks previous and current member's permissions on a particular Group
 type GroupInfo struct {
 	Group           arvados.Group
-	PreviousMembers map[string]bool
-	CurrentMembers  map[string]bool
+	PreviousMembers map[string]GroupPermissions
+	CurrentMembers  map[string]GroupPermissions
 }
 
 // GetUserID returns the correct user id value depending on the selector
@@ -137,7 +140,7 @@ func ParseFlags(config *ConfigParams) error {
 		usageStr := `Synchronize remote groups into Arvados from a CSV format file with 3 columns:
   * 1st: Group name
   * 2nd: User identifier
-  * 3rd (Optional): User permission on the group: read, write or manage. (Default: write)`
+  * 3rd (Optional): User permission on the group: can_read, can_write or can_manage. (Default: can_write)`
 		fmt.Fprintf(os.Stderr, "%s\n\n", usageStr)
 		fmt.Fprintf(os.Stderr, "Usage:\n%s [OPTIONS] <input-file.csv>\n\n", os.Args[0])
 		fmt.Fprintf(os.Stderr, "Options:\n")
@@ -335,16 +338,30 @@ func doMain(cfg *ConfigParams) error {
 	// Remove previous members not listed on this run
 	for groupUUID := range remoteGroups {
 		gi := remoteGroups[groupUUID]
-		evictedMembers := subtract(gi.PreviousMembers, gi.CurrentMembers)
+		evictedMemberPerms := subtract(gi.PreviousMembers, gi.CurrentMembers)
 		groupName := gi.Group.Name
-		if len(evictedMembers) > 0 {
-			log.Printf("Removing %d users from group %q", len(evictedMembers), groupName)
-		}
-		for evictedUser := range evictedMembers {
-			if err := RemoveMemberFromGroup(cfg, allUsers[userIDToUUID[evictedUser]], gi.Group); err != nil {
+		if len(evictedMemberPerms) > 0 {
+			log.Printf("Removing permissions from %d users on group %q", len(evictedMemberPerms), groupName)
+		}
+		for member := range evictedMemberPerms {
+			var perms []string
+			completeMembershipRemoval := false
+			if _, ok := gi.CurrentMembers[member]; !ok {
+				completeMembershipRemoval = true
+				membershipsRemoved++
+			} else {
+				// Collect which user->group permission links should be removed
+				for p := range evictedMemberPerms[member] {
+					if evictedMemberPerms[member][p] {
+						perms = append(perms, p)
+					}
+				}
+				membershipsRemoved += len(perms)
+			}
+			if err := RemoveMemberLinksFromGroup(cfg, allUsers[userIDToUUID[member]],
+				perms, completeMembershipRemoval, gi.Group); err != nil {
 				return err
 			}
-			membershipsRemoved++
 		}
 	}
 	log.Printf("Groups created: %d. Memberships added: %d, removed: %d, skipped: %d", groupsCreated, membershipsAdded, membershipsRemoved, membershipsSkipped)
@@ -382,8 +399,17 @@ func ProcessFile(
 		}
 		groupName := strings.TrimSpace(record[0])
 		groupMember := strings.TrimSpace(record[1]) // User ID (username or email)
-		if groupName == "" || groupMember == "" {
-			log.Printf("Warning: CSV record has at least one empty field (%s, %s). Skipping", groupName, groupMember)
+		groupPermission := "can_write"
+		if len(record) == 3 {
+			groupPermission = strings.ToLower(record[2])
+		}
+		if groupName == "" || groupMember == "" || groupPermission == "" {
+			log.Printf("Warning: CSV record has at least one empty field (%s, %s, %s). Skipping", groupName, groupMember, groupPermission)
+			membersSkipped++
+			continue
+		}
+		if !(groupPermission == "can_read" || groupPermission == "can_write" || groupPermission == "can_manage") {
+			log.Printf("Warning: 3rd field should be 'can_read', 'can_write' or 'can_manage'. Found: %q at line %d, skipping.", groupPermission, lineNo)
 			membersSkipped++
 			continue
 		}
@@ -412,26 +438,31 @@ func ProcessFile(
 			groupNameToUUID[groupName] = newGroup.UUID
 			remoteGroups[newGroup.UUID] = &GroupInfo{
 				Group:           newGroup,
-				PreviousMembers: make(map[string]bool), // Empty set
-				CurrentMembers:  make(map[string]bool), // Empty set
+				PreviousMembers: make(map[string]GroupPermissions),
+				CurrentMembers:  make(map[string]GroupPermissions),
 			}
 			groupsCreated++
 		}
 		// Both group & user exist, check if user is a member
 		groupUUID := groupNameToUUID[groupName]
 		gi := remoteGroups[groupUUID]
-		if !gi.PreviousMembers[groupMember] && !gi.CurrentMembers[groupMember] {
+		if !gi.PreviousMembers[groupMember][groupPermission] && !gi.CurrentMembers[groupMember][groupPermission] {
 			if cfg.Verbose {
 				log.Printf("Adding %q to group %q", groupMember, groupName)
 			}
 			// User wasn't a member, but should be.
-			if e := AddMemberToGroup(cfg, allUsers[userIDToUUID[groupMember]], gi.Group); e != nil {
+			if e := AddMemberToGroup(cfg, allUsers[userIDToUUID[groupMember]], gi.Group, groupPermission); e != nil {
 				err = e
 				return
 			}
 			membersAdded++
 		}
-		gi.CurrentMembers[groupMember] = true
+		if _, ok := gi.CurrentMembers[groupMember]; ok {
+			gi.CurrentMembers[groupMember][groupPermission] = true
+		} else {
+			gi.CurrentMembers[groupMember] = GroupPermissions{groupPermission: true}
+		}
+
 	}
 	return
 }
@@ -459,11 +490,17 @@ func GetAll(c *arvados.Client, res string, params arvados.ResourceListParams, pa
 	return allItems, nil
 }
 
-func subtract(setA map[string]bool, setB map[string]bool) map[string]bool {
-	result := make(map[string]bool)
+func subtract(setA map[string]GroupPermissions, setB map[string]GroupPermissions) map[string]GroupPermissions {
+	result := make(map[string]GroupPermissions)
 	for element := range setA {
-		if !setB[element] {
-			result[element] = true
+		if _, ok := setB[element]; !ok {
+			result[element] = setA[element]
+		} else {
+			for perm := range setA[element] {
+				if _, ok := setB[element][perm]; !ok {
+					result[element][perm] = true
+				}
+			}
 		}
 	}
 	return result
@@ -533,8 +570,8 @@ func GetRemoteGroups(cfg *ConfigParams, allUsers map[string]arvados.User) (remot
 				Operand:  "permission",
 			}, {
 				Attr:     "name",
-				Operator: "=",
-				Operand:  "can_write",
+				Operator: "in",
+				Operand:  []string{"can_read", "can_write", "can_manage"},
 			}, {
 				Attr:     "head_uuid",
 				Operator: "=",
@@ -547,18 +584,23 @@ func GetRemoteGroups(cfg *ConfigParams, allUsers map[string]arvados.User) (remot
 		}
 		g2uLinks, err := GetAll(cfg.Client, "links", g2uFilter, &LinkList{})
 		if err != nil {
-			return remoteGroups, groupNameToUUID, fmt.Errorf("error getting member (can_read) links for group %q: %s", group.Name, err)
+			return remoteGroups, groupNameToUUID, fmt.Errorf("error getting group->user 'can_read' links for group %q: %s", group.Name, err)
 		}
 		u2gLinks, err := GetAll(cfg.Client, "links", u2gFilter, &LinkList{})
 		if err != nil {
-			return remoteGroups, groupNameToUUID, fmt.Errorf("error getting member (can_write) links for group %q: %s", group.Name, err)
+			return remoteGroups, groupNameToUUID, fmt.Errorf("error getting user->group links for group %q: %s", group.Name, err)
 		}
-		// Build a list of user ids (email or username) belonging to this group
-		membersSet := make(map[string]bool)
-		u2gLinkSet := make(map[string]bool)
+		// Build a list of user ids (email or username) belonging to this group.
+		membersSet := make(map[string]GroupPermissions)
+		u2gLinkSet := make(map[string]GroupPermissions)
 		for _, l := range u2gLinks {
-			linkedMemberUUID := l.(arvados.Link).TailUUID
-			u2gLinkSet[linkedMemberUUID] = true
+			link := l.(arvados.Link)
+			// Also save the member's group access level.
+			if _, ok := u2gLinkSet[link.TailUUID]; ok {
+				u2gLinkSet[link.TailUUID][link.Name] = true
+			} else {
+				u2gLinkSet[link.TailUUID] = GroupPermissions{link.Name: true}
+			}
 		}
 		for _, item := range g2uLinks {
 			link := item.(arvados.Link)
@@ -576,55 +618,81 @@ func GetRemoteGroups(cfg *ConfigParams, allUsers map[string]arvados.User) (remot
 			if err != nil {
 				return remoteGroups, groupNameToUUID, err
 			}
-			membersSet[memberID] = true
+			membersSet[memberID] = u2gLinkSet[link.HeadUUID]
 		}
 		remoteGroups[group.UUID] = &GroupInfo{
 			Group:           group,
 			PreviousMembers: membersSet,
-			CurrentMembers:  make(map[string]bool), // Empty set
+			CurrentMembers:  make(map[string]GroupPermissions),
 		}
 		groupNameToUUID[group.Name] = group.UUID
 	}
 	return remoteGroups, groupNameToUUID, nil
 }
 
-// RemoveMemberFromGroup remove all links related to the membership
-func RemoveMemberFromGroup(cfg *ConfigParams, user arvados.User, group arvados.Group) error {
+// RemoveMemberLinksFromGroup remove all links related to the membership
+func RemoveMemberLinksFromGroup(cfg *ConfigParams, user arvados.User, linkNames []string, completeRemoval bool, group arvados.Group) error {
 	if cfg.Verbose {
 		log.Printf("Getting group membership links for user %q (%s) on group %q (%s)", user.Username, user.UUID, group.Name, group.UUID)
 	}
 	var links []interface{}
-	// Search for all group<->user links (both ways)
-	for _, filterset := range [][]arvados.Filter{
-		// Group -> User
-		{{
-			Attr:     "link_class",
-			Operator: "=",
-			Operand:  "permission",
-		}, {
-			Attr:     "tail_uuid",
-			Operator: "=",
-			Operand:  group.UUID,
-		}, {
-			Attr:     "head_uuid",
-			Operator: "=",
-			Operand:  user.UUID,
-		}},
-		// Group <- User
-		{{
-			Attr:     "link_class",
-			Operator: "=",
-			Operand:  "permission",
-		}, {
-			Attr:     "tail_uuid",
-			Operator: "=",
-			Operand:  user.UUID,
-		}, {
-			Attr:     "head_uuid",
-			Operator: "=",
-			Operand:  group.UUID,
-		}},
-	} {
+	var filters [][]arvados.Filter
+	if completeRemoval {
+		// Search for all group<->user links (both ways)
+		filters = [][]arvados.Filter{
+			// Group -> User
+			{{
+				Attr:     "link_class",
+				Operator: "=",
+				Operand:  "permission",
+			}, {
+				Attr:     "tail_uuid",
+				Operator: "=",
+				Operand:  group.UUID,
+			}, {
+				Attr:     "head_uuid",
+				Operator: "=",
+				Operand:  user.UUID,
+			}},
+			// Group <- User
+			{{
+				Attr:     "link_class",
+				Operator: "=",
+				Operand:  "permission",
+			}, {
+				Attr:     "tail_uuid",
+				Operator: "=",
+				Operand:  user.UUID,
+			}, {
+				Attr:     "head_uuid",
+				Operator: "=",
+				Operand:  group.UUID,
+			}},
+		}
+	} else {
+		// Search only for the requested Group <- User permission links
+		filters = [][]arvados.Filter{
+			{{
+				Attr:     "link_class",
+				Operator: "=",
+				Operand:  "permission",
+			}, {
+				Attr:     "tail_uuid",
+				Operator: "=",
+				Operand:  user.UUID,
+			}, {
+				Attr:     "head_uuid",
+				Operator: "=",
+				Operand:  group.UUID,
+			}, {
+				Attr:     "name",
+				Operator: "in",
+				Operand:  linkNames,
+			}},
+		}
+	}
+
+	for _, filterset := range filters {
 		l, err := GetAll(cfg.Client, "links", arvados.ResourceListParams{Filters: filterset}, &LinkList{})
 		if err != nil {
 			userID, _ := GetUserID(user, cfg.UserID)
@@ -648,7 +716,7 @@ func RemoveMemberFromGroup(cfg *ConfigParams, user arvados.User, group arvados.G
 }
 
 // AddMemberToGroup create membership links
-func AddMemberToGroup(cfg *ConfigParams, user arvados.User, group arvados.Group) error {
+func AddMemberToGroup(cfg *ConfigParams, user arvados.User, group arvados.Group, perm string) error {
 	var newLink arvados.Link
 	linkData := map[string]string{
 		"owner_uuid": cfg.SysUserUUID,
@@ -664,13 +732,13 @@ func AddMemberToGroup(cfg *ConfigParams, user arvados.User, group arvados.Group)
 	linkData = map[string]string{
 		"owner_uuid": cfg.SysUserUUID,
 		"link_class": "permission",
-		"name":       "can_write",
+		"name":       perm,
 		"tail_uuid":  user.UUID,
 		"head_uuid":  group.UUID,
 	}
 	if err := CreateLink(cfg, &newLink, linkData); err != nil {
 		userID, _ := GetUserID(user, cfg.UserID)
-		return fmt.Errorf("error adding user %q -> group %q write permission: %s", userID, group.Name, err)
+		return fmt.Errorf("error adding user %q -> group %q %s permission: %s", userID, group.Name, perm, err)
 	}
 	return nil
 }

commit 8b6836581b1ce61075f6e9020d18acd858a86cdf
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date:   Wed May 20 18:23:08 2020 -0300

    16435: Allows 2 or 3 fields per record on the CSV file.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>

diff --git a/tools/sync-groups/sync-groups.go b/tools/sync-groups/sync-groups.go
index 7c5cd0558..b77d8ccdd 100644
--- a/tools/sync-groups/sync-groups.go
+++ b/tools/sync-groups/sync-groups.go
@@ -134,9 +134,10 @@ func ParseFlags(config *ConfigParams) error {
 
 	// Set up usage message
 	flags.Usage = func() {
-		usageStr := `Synchronize remote groups into Arvados from a CSV format file with 2 columns:
-  * 1st column: Group name
-  * 2nd column: User identifier`
+		usageStr := `Synchronize remote groups into Arvados from a CSV format file with 3 columns:
+  * 1st: Group name
+  * 2nd: User identifier
+  * 3rd (Optional): User permission on the group: read, write or manage. (Default: write)`
 		fmt.Fprintf(os.Stderr, "%s\n\n", usageStr)
 		fmt.Fprintf(os.Stderr, "Usage:\n%s [OPTIONS] <input-file.csv>\n\n", os.Args[0])
 		fmt.Fprintf(os.Stderr, "Options:\n")
@@ -362,7 +363,8 @@ func ProcessFile(
 ) (groupsCreated, membersAdded, membersSkipped int, err error) {
 	lineNo := 0
 	csvReader := csv.NewReader(f)
-	csvReader.FieldsPerRecord = 2
+	// Allow variable number of fields.
+	csvReader.FieldsPerRecord = -1
 	for {
 		record, e := csvReader.Read()
 		if e == io.EOF {
@@ -373,6 +375,11 @@ func ProcessFile(
 			err = fmt.Errorf("error parsing %q, line %d", cfg.Path, lineNo)
 			return
 		}
+		// Only allow 2 or 3 fields per record for backwards compatibility.
+		if len(record) < 2 || len(record) > 3 {
+			err = fmt.Errorf("error parsing %q, line %d: found %d fields but only 2 or 3 are allowed", cfg.Path, lineNo, len(record))
+			return
+		}
 		groupName := strings.TrimSpace(record[0])
 		groupMember := strings.TrimSpace(record[1]) // User ID (username or email)
 		if groupName == "" || groupMember == "" {
diff --git a/tools/sync-groups/sync-groups_test.go b/tools/sync-groups/sync-groups_test.go
index 3ef360079..77d9756ff 100644
--- a/tools/sync-groups/sync-groups_test.go
+++ b/tools/sync-groups/sync-groups_test.go
@@ -263,6 +263,22 @@ func (s *TestSuite) TestIgnoreSpaces(c *C) {
 	}
 }
 
+// Error out when records have <2 or >3 records
+func (s *TestSuite) TestWrongNumberOfFields(c *C) {
+	for _, testCase := range [][][]string{
+		{{"field1"}},
+		{{"field1", "field2", "field3", "field4"}},
+		{{"field1", "field2", "field3", "field4", "field5"}},
+	} {
+		tmpfile, err := MakeTempCSVFile(testCase)
+		c.Assert(err, IsNil)
+		defer os.Remove(tmpfile.Name())
+		s.cfg.Path = tmpfile.Name()
+		err = doMain(s.cfg)
+		c.Assert(err, Not(IsNil))
+	}
+}
+
 // The absence of a user membership on the CSV file implies its removal
 func (s *TestSuite) TestMembershipRemoval(c *C) {
 	localUserEmail := s.users[arvadostest.ActiveUserUUID].Email

commit ccc6ca8fb944d45c94f2ae994d3c0f0567c1b91b
Author: Peter Amstutz <peter.amstutz at curii.com>
Date:   Thu May 21 09:51:03 2020 -0400

    16419: Use CAINFO instead of CAPATH
    
    Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>

diff --git a/sdk/python/arvados/keep.py b/sdk/python/arvados/keep.py
index 9601601d4..bc43b849c 100644
--- a/sdk/python/arvados/keep.py
+++ b/sdk/python/arvados/keep.py
@@ -376,7 +376,7 @@ class KeepClient(object):
                     if self.insecure:
                         curl.setopt(pycurl.SSL_VERIFYPEER, 0)
                     else:
-                        curl.setopt(pycurl.CAPATH,os.path.dirname(arvados.util.ca_certs_path()))
+                        curl.setopt(pycurl.CAINFO, arvados.util.ca_certs_path())
                     if method == "HEAD":
                         curl.setopt(pycurl.NOBODY, True)
                     self._setcurltimeouts(curl, timeout, method=="HEAD")
@@ -476,7 +476,7 @@ class KeepClient(object):
                     if self.insecure:
                         curl.setopt(pycurl.SSL_VERIFYPEER, 0)
                     else:
-                        curl.setopt(pycurl.CAPATH,os.path.dirname(arvados.util.ca_certs_path()))
+                        curl.setopt(pycurl.CAINFO, arvados.util.ca_certs_path())
                     self._setcurltimeouts(curl, timeout)
                     try:
                         curl.perform()

commit c6523d3fdf49b9d7b0fc998f64473725ad9346fa
Author: Pjotr Prins <pjotr.public01 at thebird.nl>
Date:   Wed May 20 14:35:54 2020 -0500

    keep.py: python-api https certificate align pycurl with httplib2 certificate finder
    
    Arvados-DCO-1.1-Signed-off-by: Pjotr Prins <pjotr.public01 at thebird.nl>

diff --git a/sdk/python/arvados/keep.py b/sdk/python/arvados/keep.py
index 86a28f54c..9601601d4 100644
--- a/sdk/python/arvados/keep.py
+++ b/sdk/python/arvados/keep.py
@@ -375,6 +375,8 @@ class KeepClient(object):
                     curl.setopt(pycurl.HEADERFUNCTION, self._headerfunction)
                     if self.insecure:
                         curl.setopt(pycurl.SSL_VERIFYPEER, 0)
+                    else:
+                        curl.setopt(pycurl.CAPATH,os.path.dirname(arvados.util.ca_certs_path()))
                     if method == "HEAD":
                         curl.setopt(pycurl.NOBODY, True)
                     self._setcurltimeouts(curl, timeout, method=="HEAD")
@@ -473,6 +475,8 @@ class KeepClient(object):
                     curl.setopt(pycurl.HEADERFUNCTION, self._headerfunction)
                     if self.insecure:
                         curl.setopt(pycurl.SSL_VERIFYPEER, 0)
+                    else:
+                        curl.setopt(pycurl.CAPATH,os.path.dirname(arvados.util.ca_certs_path()))
                     self._setcurltimeouts(curl, timeout)
                     try:
                         curl.perform()

commit 4506c6105f716cceb091954d8589dbea12696bea
Author: Pjotr Prins <pjotr.public01 at thebird.nl>
Date:   Wed May 20 11:12:20 2020 -0500

    util.py: python-api https certificate openssl override as is used in GNU Guix
    
    Arvados-DCO-1.1-Signed-off-by: Pjotr Prins <pjotr.public01 at thebird.nl>

diff --git a/sdk/python/arvados/util.py b/sdk/python/arvados/util.py
index dcc0417c1..6c9822e9f 100644
--- a/sdk/python/arvados/util.py
+++ b/sdk/python/arvados/util.py
@@ -396,6 +396,9 @@ def ca_certs_path(fallback=httplib2.CA_CERTS):
     it returns the value of `fallback` (httplib2's CA certs by default).
     """
     for ca_certs_path in [
+        # SSL_CERT_FILE and SSL_CERT_DIR are openssl overrides - note
+        # that httplib2 itself also supports HTTPLIB2_CA_CERTS.
+        os.environ.get('SSL_CERT_FILE'),
         # Arvados specific:
         '/etc/arvados/ca-certificates.crt',
         # Debian:
@@ -403,7 +406,7 @@ def ca_certs_path(fallback=httplib2.CA_CERTS):
         # Red Hat:
         '/etc/pki/tls/certs/ca-bundle.crt',
         ]:
-        if os.path.exists(ca_certs_path):
+        if ca_certs_path and os.path.exists(ca_certs_path):
             return ca_certs_path
     return fallback
 

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list