Bug #16778

Cannot set up federated user with a VM with LoginCluster

Added by Peter Amstutz about 1 year ago. Updated about 1 year ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
API
Target version:
Start date:
09/03/2020
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
-
Release relationship:
Auto

Description

Follow-up from comment https://dev.arvados.org/issues/16775#note-1

User "setup" on a federate which has a LoginCluster gets forwarded to the central cluster. If the setup request includes a VM, the user should be set up with that VM. However, the VM exists on the federate, not the LoginCluster, so setup fails because it doesn't recognize that the VM exists. Also, it won't work to create a permission link on the login cluster, the permission link needs to be created on the federate.

Proposed solution:

Set up the user on the LoginCluster first with "vm" uuid removed, if that succeeds, then invoke setup on the local cluster? (what about git repos?)


Subtasks

Task #16786: Review 16778-setup-fed-userResolvedPeter Amstutz


Related issues

Related to Arvados - Bug #16775: [umbrella ticket] problems with virtual machine permissions in a federated clusterResolved

Associated revisions

Revision 49a89ce9
Added by Peter Amstutz about 1 year ago

Merge branch '16778-setup-fed-user' refs #16778

Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <>

History

#1 Updated by Peter Amstutz about 1 year ago

  • Description updated (diff)

#2 Updated by Peter Amstutz about 1 year ago

  • Subject changed from Cannot set up federated user with a VM to Cannot set up federated user with a VM with LoginCluster

#3 Updated by Peter Amstutz about 1 year ago

  • Related to Bug #16775: [umbrella ticket] problems with virtual machine permissions in a federated cluster added

#4 Updated by Peter Amstutz about 1 year ago

  • Description updated (diff)

#5 Updated by Peter Amstutz about 1 year ago

  • Assigned To set to Peter Amstutz

#6 Updated by Peter Amstutz about 1 year ago

  • Category changed from Workbench to API

#7 Updated by Peter Amstutz about 1 year ago

16778-setup-fed-user @ 51d7a5b2a23074a130aa6dd74cbaf5f335920769

https://ci.arvados.org/view/Developer/job/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

#8 Updated by Tom Clegg about 1 year ago

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)

#9 Updated by Peter Amstutz about 1 year ago

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.

https://ci.arvados.org/view/Developer/job/developer-run-tests/2069/

#10 Updated by Tom Clegg about 1 year ago

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!

#11 Updated by Peter Amstutz about 1 year ago

  • Status changed from New to Resolved

#12 Updated by Peter Amstutz about 1 year ago

  • Release set to 25

Also available in: Atom PDF