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)