16263: Generalize "local_user_list" flag to "no_federation"

Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz@curii.com>

diff --git a/lib/controller/federation/conn.go b/lib/controller/federation/conn.go
index 519ea21b9f91a61669895296a45a65e64cfe8944..f67ea6713ec6a8bca926c939defdb141867735e9 100644 (file)
--- a/lib/controller/federation/conn.go
+++ b/lib/controller/federation/conn.go
@@ -365,64 +365,76 @@ var userAttrsCachedFromLoginCluster = map[string]bool{
        "writable_by":             false,
 }
 
-func (conn *Conn) UserList(ctx context.Context, options arvados.ListOptions) (arvados.UserList, error) {
+func (conn *Conn) batchUpdateUsers(ctx context.Context,
+       options arvados.ListOptions,
+       items []arvados.User) (err error) {
+
+       id := conn.cluster.Login.LoginCluster
        logger := ctxlog.FromContext(ctx)
-       if id := conn.cluster.Login.LoginCluster; id != "" && id != conn.cluster.ClusterID && !options.LocalUserList {
-               resp, err := conn.chooseBackend(id).UserList(ctx, options)
-               if err != nil {
-                       return resp, err
+       batchOpts := arvados.UserBatchUpdateOptions{Updates: map[string]map[string]interface{}{}}
+       for _, user := range items {
+               if !strings.HasPrefix(user.UUID, id) {
+                       continue
+               }
+               logger.Debugf("cache user info for uuid %q", user.UUID)
+
+               // If the remote cluster has null timestamps
+               // (e.g., test server with incomplete
+               // fixtures) use dummy timestamps (instead of
+               // the zero time, which causes a Rails API
+               // error "year too big to marshal: 1 UTC").
+               if user.ModifiedAt.IsZero() {
+                       user.ModifiedAt = time.Now()
+               }
+               if user.CreatedAt.IsZero() {
+                       user.CreatedAt = time.Now()
                }
-               batchOpts := arvados.UserBatchUpdateOptions{Updates: map[string]map[string]interface{}{}}
-               for _, user := range resp.Items {
-                       if !strings.HasPrefix(user.UUID, id) {
-                               continue
-                       }
-                       logger.Debugf("cache user info for uuid %q", user.UUID)
-
-                       // If the remote cluster has null timestamps
-                       // (e.g., test server with incomplete
-                       // fixtures) use dummy timestamps (instead of
-                       // the zero time, which causes a Rails API
-                       // error "year too big to marshal: 1 UTC").
-                       if user.ModifiedAt.IsZero() {
-                               user.ModifiedAt = time.Now()
-                       }
-                       if user.CreatedAt.IsZero() {
-                               user.CreatedAt = time.Now()
-                       }
 
-                       var allFields map[string]interface{}
-                       buf, err := json.Marshal(user)
-                       if err != nil {
-                               return arvados.UserList{}, fmt.Errorf("error encoding user record from remote response: %s", err)
-                       }
-                       err = json.Unmarshal(buf, &allFields)
-                       if err != nil {
-                               return arvados.UserList{}, fmt.Errorf("error transcoding user record from remote response: %s", err)
-                       }
-                       updates := allFields
-                       if len(options.Select) > 0 {
-                               updates = map[string]interface{}{}
-                               for _, k := range options.Select {
-                                       if v, ok := allFields[k]; ok && userAttrsCachedFromLoginCluster[k] {
-                                               updates[k] = v
-                                       }
+               var allFields map[string]interface{}
+               buf, err := json.Marshal(user)
+               if err != nil {
+                       return fmt.Errorf("error encoding user record from remote response: %s", err)
+               }
+               err = json.Unmarshal(buf, &allFields)
+               if err != nil {
+                       return fmt.Errorf("error transcoding user record from remote response: %s", err)
+               }
+               updates := allFields
+               if len(options.Select) > 0 {
+                       updates = map[string]interface{}{}
+                       for _, k := range options.Select {
+                               if v, ok := allFields[k]; ok && userAttrsCachedFromLoginCluster[k] {
+                                       updates[k] = v
                                }
-                       } else {
-                               for k := range updates {
-                                       if !userAttrsCachedFromLoginCluster[k] {
-                                               delete(updates, k)
-                                       }
+                       }
+               } else {
+                       for k := range updates {
+                               if !userAttrsCachedFromLoginCluster[k] {
+                                       delete(updates, k)
                                }
                        }
-                       batchOpts.Updates[user.UUID] = updates
                }
-               if len(batchOpts.Updates) > 0 {
-                       ctxRoot := auth.NewContext(ctx, &auth.Credentials{Tokens: []string{conn.cluster.SystemRootToken}})
-                       _, err = conn.local.UserBatchUpdate(ctxRoot, batchOpts)
-                       if err != nil {
-                               return arvados.UserList{}, fmt.Errorf("error updating local user records: %s", err)
-                       }
+               batchOpts.Updates[user.UUID] = updates
+       }
+       if len(batchOpts.Updates) > 0 {
+               ctxRoot := auth.NewContext(ctx, &auth.Credentials{Tokens: []string{conn.cluster.SystemRootToken}})
+               _, err = conn.local.UserBatchUpdate(ctxRoot, batchOpts)
+               if err != nil {
+                       return fmt.Errorf("error updating local user records: %s", err)
+               }
+       }
+       return nil
+}
+
+func (conn *Conn) UserList(ctx context.Context, options arvados.ListOptions) (arvados.UserList, error) {
+       if id := conn.cluster.Login.LoginCluster; id != "" && id != conn.cluster.ClusterID && !options.NoFederation {
+               resp, err := conn.chooseBackend(id).UserList(ctx, options)
+               if err != nil {
+                       return resp, err
+               }
+               err = conn.batchUpdateUsers(ctx, options, resp.Items)
+               if err != nil {
+                       return arvados.UserList{}, err
                }
                return resp, nil
        } else {
@@ -439,7 +451,7 @@ func (conn *Conn) UserUpdate(ctx context.Context, options arvados.UpdateOptions)
 }
 
 func (conn *Conn) UserUpdateUUID(ctx context.Context, options arvados.UpdateUUIDOptions) (arvados.User, error) {
-       return conn.chooseBackend(options.UUID).UserUpdateUUID(ctx, options)
+       return conn.local.UserUpdateUUID(ctx, options)
 }
 
 func (conn *Conn) UserMerge(ctx context.Context, options arvados.UserMergeOptions) (arvados.User, error) {

diff --git a/lib/controller/federation/list.go b/lib/controller/federation/list.go
index 6ee813317417cd012d829387e10e04f2ea1dad17..d1d52cfc6cd54db52db52b9d4123d403f81b7ab2 100644 (file)
--- a/lib/controller/federation/list.go
+++ b/lib/controller/federation/list.go
@@ -106,6 +106,13 @@ func (conn *Conn) generated_CollectionList(ctx context.Context, options arvados.
 // corresponding options argument suitable for sending to that
 // backend.
 func (conn *Conn) splitListRequest(ctx context.Context, opts arvados.ListOptions, fn func(context.Context, string, arvados.API, arvados.ListOptions) ([]string, error)) error {
+
+       if opts.NoFederation {
+               // Client requested no federation.  Pass through.
+               _, err := fn(ctx, conn.cluster.ClusterID, conn.local, opts)
+               return err
+       }
+
        cannotSplit := false
        var matchAllFilters map[string]bool
        for _, f := range opts.Filters {

diff --git a/sdk/go/arvados/api.go b/sdk/go/arvados/api.go
index 3b20955a1678c6819921dbf9d718db217e11e2e7..a30da62425883988f52f71aa9b48d5495df54f72 100644 (file)
--- a/sdk/go/arvados/api.go
+++ b/sdk/go/arvados/api.go
@@ -84,7 +84,7 @@ type ListOptions struct {
        Count              string                 `json:"count"`
        IncludeTrash       bool                   `json:"include_trash"`
        IncludeOldVersions bool                   `json:"include_old_versions"`
-       LocalUserList      bool                   `json:"local_user_list"`
+       NoFederation       bool                   `json:"no_federation"`
 }
 
 type CreateOptions struct {

diff --git a/sdk/python/arvados/commands/federation_migrate.py b/sdk/python/arvados/commands/federation_migrate.py
index 344390b48c920016c1b914f37de9482baa8161b7..e1b8ee7d8402f5834ba9391c6fddb78b5d79ef14 100755 (executable)
--- a/sdk/python/arvados/commands/federation_migrate.py
+++ b/sdk/python/arvados/commands/federation_migrate.py
@@ -98,7 +98,7 @@ def fetch_users(clusters, loginCluster):
     users = []
     for c, arv in clusters.items():
         print("Getting user list from %s" % c)
-        ul = arvados.util.list_all(arv.users().list, local_user_list=True)
+        ul = arvados.util.list_all(arv.users().list, no_federation=True)
         for l in ul:
             if l["uuid"].startswith(c):
                 users.append(l)
@@ -171,7 +171,7 @@ def update_username(args, email, user_uuid, username, migratecluster, migratearv
     print("(%s) Updating username of %s to '%s' on %s" % (email, user_uuid, username, migratecluster))
     if not args.dry_run:
         try:
-            conflicts = migratearv.users().list(filters=[["username", "=", username]], local_user_list=True).execute()
+            conflicts = migratearv.users().list(filters=[["username", "=", username]], no_federation=True).execute()
             if conflicts["items"]:
                 migratearv.users().update(uuid=conflicts["items"][0]["uuid"], body={"user": {"username": username+"migrate"}}).execute()
             migratearv.users().update(uuid=user_uuid, body={"user": {"username": username}}).execute()
@@ -204,7 +204,7 @@ def choose_new_user(args, by_email, email, userhome, username, old_user_uuid, cl
             user = None
             try:
                 olduser = oldhomearv.users().get(uuid=old_user_uuid).execute()
-                conflicts = homearv.users().list(filters=[["username", "=", username]], local_user_list=True).execute()
+                conflicts = homearv.users().list(filters=[["username", "=", username]], no_federation=True).execute()
                 if conflicts["items"]:
                     homearv.users().update(uuid=conflicts["items"][0]["uuid"], body={"user": {"username": username+"migrate"}}).execute()
                 user = homearv.users().create(body={"user": {"email": email, "username": username, "is_active": olduser["is_active"]}}).execute()
@@ -241,10 +241,16 @@ def activate_remote_user(args, email, homearv, migratearv, old_user_uuid, new_us
         return None
 
     try:
-        olduser = migratearv.users().get(uuid=old_user_uuid).execute()
+        findolduser = migratearv.users().list(filters=[["uuid", "=", old_user_uuid]], no_federation=True).execute()
+        if len(findolduser["items"]) == 0:
+            return False
+        if len(findolduser["items"]) == 1:
+            olduser = findolduser["items"][0]
+        else:
+            print("(%s) Unexpected result" % (email))
+            return None
     except arvados.errors.ApiError as e:
-        if e.resp.status != 404:
-            print("(%s) Could not retrieve user %s from %s, user may have already been migrated: %s" % (email, old_user_uuid, migratecluster, e))
+        print("(%s) Could not retrieve user %s from %s, user may have already been migrated: %s" % (email, old_user_uuid, migratecluster, e))
         return None
 
     salted = 'v2/' + newtok["uuid"] + '/' + hmac.new(newtok["api_token"].encode(),
@@ -351,10 +357,10 @@ def main():
             if new_user_uuid is None:
                 continue
 
-            # cluster where the migration is happening
             remote_users = {}
             got_error = False
             for migratecluster in clusters:
+                # cluster where the migration is happening
                 migratearv = clusters[migratecluster]
 
                 # the user's new home cluster
@@ -370,6 +376,8 @@ def main():
                 for migratecluster in clusters:
                     migratearv = clusters[migratecluster]
                     newuser = remote_users[migratecluster]
+                    if newuser is False:
+                        continue
 
                     print("(%s) Migrating %s to %s on %s" % (email, old_user_uuid, new_user_uuid, migratecluster))
 

diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index 7b82cdb61952888fdb1b924d99e9005c1b2f42dd..68fa7d88015d3ff3b081c9a185e6db7aa5ba1b64 100644 (file)
--- a/services/api/app/controllers/application_controller.rb
+++ b/services/api/app/controllers/application_controller.rb
@@ -656,6 +656,11 @@ class ApplicationController < ActionController::Base
         location: "query",
         required: false,
       },
+      no_federation: {
+        type: 'boolean',
+        required: false,
+        description: 'bypass federation behavior, list items from local instance database only'
+      }
     }
   end
 

diff --git a/services/api/app/controllers/arvados/v1/users_controller.rb b/services/api/app/controllers/arvados/v1/users_controller.rb
index 9439493df4171aa533a29e97ecbd1926ee29ea01..289e82567a67f7eb4f1c8223153f2bc9c76f439a 100644 (file)
--- a/services/api/app/controllers/arvados/v1/users_controller.rb
+++ b/services/api/app/controllers/arvados/v1/users_controller.rb
@@ -257,12 +257,6 @@ class Arvados::V1::UsersController < ApplicationController
     }
   end
 
-  def self._index_requires_parameters
-    super.merge(
-      { local_user_list: {required: false, type: 'boolean',
-                          description: 'only list users from local database, no effect if LoginCluster is not set'} })
-  end
-
   def apply_filters(model_class=nil)
     return super if @read_users.any?(&:is_admin)
     if params[:uuid] != current_user.andand.uuid