Arvados

16778-setup-fed-user @ 51d7a5b2a23074a130aa6dd74cbaf5f335920769

developer-run-tests: #2068

• When LoginCluster is in effect, setup call goes to both the LoginCluster and the local cluster, so that local cluster can set up VM and git repo for the user.
• Also tweak "choose backend" behavior for setup, unsetup, and activate

This line in (*federation.Conn)UserSetup() looks like it will crash if options.VMUUID or options.UUID is "foo". It seems like we don't care what happens if VMUUID matches neither local nor logincluster, so perhaps this condition should be just strings.HasPrefix(options.VMUUID, conn.cluster.ClusterID)?

+               if options.VMUUID != "" && options.VMUUID[0:5] != options.UUID[0:5] {

Setup does things other than VMs and repos, so I'm wondering if it would be better to always call local user setup after the remote setup, even if there's no local VM or repo involved. Perhaps it would be a little easier to follow this way, too:

if logincluster != "" {
  optsRemote := options
  optsRemote.RepoName = "" // repo is always ours
  if strings.HasPrefix(options.VMUUID, conn.cluster.ClusterID) {
    optsRemote.VMUUID = "" // VM is ours
  } else {
    options.VMUUID = "" // VM is theirs
  }
  ret, err := conn.chooseBackend(conn.cluster.Login.LoginCluster).UserSetup(ctx, optsRemote)
  if err != nil { return ret, err }
}
return conn.local.UserSetup(ctx, options)

"Is LoginCluster in use?" conditions should treat LoginCluster==ClusterID the same as LoginCluster="" so things don't get weird with a template-driven config used across the whole federation (like z1111 in the integration test). Currently Login() has this

        if id := conn.cluster.Login.LoginCluster; id != "" && id != conn.cluster.ClusterID {

...but rather than repeat that everywhere, maybe we could rename localOrLoginCluster to loginCluster, reduce it to return conn.chooseBackend(conn.cluster.Login.LoginCluster), and change the "logincluster is remote?" conditions to conn.loginCluster() != conn.local?

In TestSetupUserWithVM, this should be just outAuth.TokenV2()

fmt.Sprintf("v2/%v/%v", outAuth.UUID, outAuth.APIToken)

16778-setup-fed-user @ 47aa52f1b343c93e09908b69d40bf8b389e8b15c

Simplified the logic.

I don't think we need to support the case where the user is set up on a federate but given access to a VM on the LoginCluster. You can just set up the user on the LoginCluster directly for that.

developer-run-tests: #2069

Nit: Making a block-scoped copy of options with upstreamOptions := options (or even options := options) would be less clunky than using local vars to stash/restore the blanked fields.

LGTM, thanks!