[ARVADOS] created: 2.1.0-1292-g7edaf0605
Git user
git at public.arvados.org
Mon Sep 6 19:44:38 UTC 2021
at 7edaf06056ef65fad632955bf647f6c36024c1bc (commit)
commit 7edaf06056ef65fad632955bf647f6c36024c1bc
Author: Lucas Di Pentima <lucas.dipentima at curii.com>
Date: Mon Sep 6 16:41:40 2021 -0300
18076: Fixes the bug by assigning a different (and random) username.
If the renamed user does exist on the LoginCluster, it'll eventually be named
correctly, and it is a stale record, it'll get a username that gives the admin
a clue about what happened to it.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima at curii.com>
diff --git a/services/api/app/controllers/arvados/v1/users_controller.rb b/services/api/app/controllers/arvados/v1/users_controller.rb
index 82594c1eb..ff4aecac4 100644
--- a/services/api/app/controllers/arvados/v1/users_controller.rb
+++ b/services/api/app/controllers/arvados/v1/users_controller.rb
@@ -38,8 +38,9 @@ class Arvados::V1::UsersController < ApplicationController
# A cached user record from the LoginCluster is stale, reset its username
# and retry the update operation.
if local_user.andand.uuid[0..4] == loginCluster && local_user.uuid != u.uuid
- Rails.logger.warn("cached username '#{needupdate[:username]}' collision with user '#{local_user.uuid}' - resetting")
- local_user.update_attributes!({username: nil})
+ new_username = "#{needupdate[:username]}conflict#{rand(99999999)}"
+ Rails.logger.warn("cached username '#{needupdate[:username]}' collision with user '#{local_user.uuid}' - renaming to '#{new_username}' before retrying")
+ local_user.update_attributes!({username: new_username})
retry
end
end
commit d545cb6a969d00ae9a5aa083713f328c1fd441c9
Author: Lucas Di Pentima <lucas.dipentima at curii.com>
Date: Mon Sep 6 16:28:42 2021 -0300
18076: Expands test case exposing a related issue with users with a repository.
The previous fix set the conflicting user's username to null but this isn't
possible when the user record is linked to a repository.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima at curii.com>
diff --git a/lib/controller/integration_test.go b/lib/controller/integration_test.go
index 7cad7d5f7..8a23bccfb 100644
--- a/lib/controller/integration_test.go
+++ b/lib/controller/integration_test.go
@@ -665,6 +665,7 @@ func (s *IntegrationSuite) TestIntermediateCluster(c *check.C) {
// Test for bug #18076
func (s *IntegrationSuite) TestStaleCachedUserRecord(c *check.C) {
rootctx1, _, _ := s.testClusters["z1111"].RootClients()
+ _, rootclnt3, _ := s.testClusters["z3333"].RootClients()
conn1 := s.testClusters["z1111"].Conn()
conn3 := s.testClusters["z3333"].Conn()
@@ -678,69 +679,92 @@ func (s *IntegrationSuite) TestStaleCachedUserRecord(c *check.C) {
}
}
- // Create some users, request them on the federated cluster so they're cached.
- var users []arvados.User
- for userNr := 0; userNr < 2; userNr++ {
- _, _, _, user := s.testClusters["z1111"].UserClients(
- rootctx1,
- c,
- conn1,
- fmt.Sprintf("user%d at example.com", userNr),
- true)
- c.Assert(user.Username, check.Not(check.Equals), "")
- users = append(users, user)
-
- lst, err := conn3.UserList(rootctx1, arvados.ListOptions{Limit: -1})
- c.Assert(err, check.Equals, nil)
- userFound := false
- for _, fedUser := range lst.Items {
- if fedUser.UUID == user.UUID {
- c.Assert(fedUser.Username, check.Equals, user.Username)
- userFound = true
- break
+ for testCaseNr, testCase := range []struct {
+ name string
+ withRepository bool
+ }{
+ {"User without local repository", false},
+ {"User with local repository", true},
+ } {
+ c.Log(c.TestName() + " " + testCase.name)
+ // Create some users, request them on the federated cluster so they're cached.
+ var users []arvados.User
+ for userNr := 0; userNr < 2; userNr++ {
+ _, _, _, user := s.testClusters["z1111"].UserClients(
+ rootctx1,
+ c,
+ conn1,
+ fmt.Sprintf("user%d%d at example.com", testCaseNr, userNr),
+ true)
+ c.Assert(user.Username, check.Not(check.Equals), "")
+ users = append(users, user)
+
+ lst, err := conn3.UserList(rootctx1, arvados.ListOptions{Limit: -1})
+ c.Assert(err, check.Equals, nil)
+ userFound := false
+ for _, fedUser := range lst.Items {
+ if fedUser.UUID == user.UUID {
+ c.Assert(fedUser.Username, check.Equals, user.Username)
+ userFound = true
+ break
+ }
+ }
+ c.Assert(userFound, check.Equals, true)
+
+ if testCase.withRepository {
+ var repo interface{}
+ err = rootclnt3.RequestAndDecode(
+ &repo, "POST", "arvados/v1/repositories", nil,
+ map[string]interface{}{
+ "repository": map[string]string{
+ "name": fmt.Sprintf("%s/test", user.Username),
+ "owner_uuid": user.UUID,
+ },
+ },
+ )
+ c.Assert(err, check.IsNil)
}
}
- c.Assert(userFound, check.Equals, true)
- }
- // Swap the usernames
- _, err := conn1.UserUpdate(rootctx1, arvados.UpdateOptions{
- UUID: users[0].UUID,
- Attrs: map[string]interface{}{
- "username": "",
- },
- })
- c.Assert(err, check.Equals, nil)
- _, err = conn1.UserUpdate(rootctx1, arvados.UpdateOptions{
- UUID: users[1].UUID,
- Attrs: map[string]interface{}{
- "username": users[0].Username,
- },
- })
- c.Assert(err, check.Equals, nil)
- _, err = conn1.UserUpdate(rootctx1, arvados.UpdateOptions{
- UUID: users[0].UUID,
- Attrs: map[string]interface{}{
- "username": users[1].Username,
- },
- })
- c.Assert(err, check.Equals, nil)
+ // Swap the usernames
+ _, err := conn1.UserUpdate(rootctx1, arvados.UpdateOptions{
+ UUID: users[0].UUID,
+ Attrs: map[string]interface{}{
+ "username": "",
+ },
+ })
+ c.Assert(err, check.Equals, nil)
+ _, err = conn1.UserUpdate(rootctx1, arvados.UpdateOptions{
+ UUID: users[1].UUID,
+ Attrs: map[string]interface{}{
+ "username": users[0].Username,
+ },
+ })
+ c.Assert(err, check.Equals, nil)
+ _, err = conn1.UserUpdate(rootctx1, arvados.UpdateOptions{
+ UUID: users[0].UUID,
+ Attrs: map[string]interface{}{
+ "username": users[1].Username,
+ },
+ })
+ c.Assert(err, check.Equals, nil)
- // Re-request the list on the federated cluster & check for updates
- lst, err := conn3.UserList(rootctx1, arvados.ListOptions{Limit: -1})
- c.Assert(err, check.Equals, nil)
- var user0Found, user1Found bool
- for _, user := range lst.Items {
- if user.UUID == users[0].UUID {
- user0Found = true
- c.Assert(user.Username, check.Equals, users[1].Username)
- } else if user.UUID == users[1].UUID {
- user1Found = true
- c.Assert(user.Username, check.Equals, users[0].Username)
+ // Re-request the list on the federated cluster & check for updates
+ lst, err := conn3.UserList(rootctx1, arvados.ListOptions{Limit: -1})
+ c.Assert(err, check.Equals, nil)
+ var user0Found, user1Found bool
+ for _, user := range lst.Items {
+ if user.UUID == users[0].UUID {
+ user0Found = true
+ c.Assert(user.Username, check.Equals, users[1].Username)
+ } else if user.UUID == users[1].UUID {
+ user1Found = true
+ c.Assert(user.Username, check.Equals, users[0].Username)
+ }
}
+ c.Assert(user0Found, check.Equals, true)
+ c.Assert(user1Found, check.Equals, true)
}
- c.Assert(user0Found, check.Equals, true)
- c.Assert(user1Found, check.Equals, true)
}
// Test for bug #16263
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list