Arvados

While trying to reproduce this error, I kept getting another error message when asking tordo for its user's list:

$ arv user list
Error: error updating local user records: //railsapi.internal/arvados/v1/users/batch_update: 422 Unprocessable Entity: #<ActiveRecord::RecordInvalid: Validation failed: Username has already been taken> (req-1iksissnw490d1thn1q9)

After re-requesting the list with -b (to bypass federation) and comparing users with ce8i5, I realized an issue with a couple of users sharing the same ward2 username. The one cached on tordo's side doesn't exist on ce8i5 (even though its UUID is from that cluster), so when tordo's controller attempts to refresh the local cache, the username is already taken by a user record that won't get updated anymore.

For the "Login as <user>" case, the issue is that wb1 tries to create an ApiClientAuthorization resource owned by the target user, and that fails because the current api client record isn't trusted because:

$ arv user current
{
 "created_at":"2020-01-21T14:39:17.181611000Z",
 "first_name":"Lucas ",
 "full_name":"Lucas  Di Pentima",
 "identity_url":"",
 "is_active":true,
 "is_admin":true,
 "is_invited":true,
 "kind":"arvados#user",
 "last_name":"Di Pentima",
 "uuid":"ce8i5-tpzed-o4njwilpp4ov286",
[...]
}
$ arv api_client_authorization current
{
 "href":"/api_client_authorizations/ce8i5-gj3su-vobk909hrbmmsrl",
 "uuid":"ce8i5-gj3su-vobk909hrbmmsrl",
 "owner_uuid":"ce8i5-tpzed-o4njwilpp4ov286",
 "api_token":"xxxxtokenxxxx",
 "api_client_id":0,
[...]
}

Updates at deca285 - branch 17785-federated-token-regression
Test run: developer-run-tests-remainder: #2789

• Adds an integration test that exposes the issue.

For the arvados-login-sync case, maybe what's needed is to set up the LOGINCLUSTER_ARVADOS_API_HOST and LOGINCLUSTER_ARVADOS_API_TOKEN envvars so that the api client authorization creation is done on the LoginCluster?

Updates at a98916d - branch 17785-federated-token-regression
Test run: developer-run-tests: #2812

• Migrates the api_client_authorization handling to controller's "new code path", forwarding CREATE and UPDATE calls to remote clusters if needed.

This fixes the test commented on #17785-28

• The *string fields should be string since there is no semantic difference between "" and NULL
• I don't think we need the Href field. It looks like we are currently using two different approaches for this. Container and Group have Href fields, but User and Collection don't -- instead "href" is exempt from the response-comparison test for User and Collection in lib/controller/handler_test.go. Unless something actually needs this, I think we should drop it.

• Looks like this could be simplified
  

  -       for cls := range s.testClusters {
  -               if cls == "z1111" || cls == "z3333" {
  +       for _, cls := range []string{"z1111", "z3333"} {
  

• After creating the "log in as..." token, might as well check that it works... if resp is an APIClientAuthorization then resp.TokenV2() might be convenient.

Updates at 47982d3
Test run: developer-run-tests: #2814

• Enhances IntegrationSuite.TestFederatedApiClientAuthHandling test by confirming that the created token does work.
• Fixes arvados.ApiClientAuthorization's member types. Updates tests & related code.

LGTM, thanks!