18858: Don't immediately exit on existing accounts with empty user IDs.

Instead, record the UUIDs and report them as an error after processing
the rest.

Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima@curii.com>

diff --git a/tools/sync-users/sync-users.go b/tools/sync-users/sync-users.go
index 37b94a9f44b636fcd80ce9632ba271c519d4e2b3..f9f8b6046cd376d7e3b59d002168fe237066088c 100644 (file)
--- a/tools/sync-users/sync-users.go
+++ b/tools/sync-users/sync-users.go
@@ -222,6 +222,7 @@ func doMain(cfg *ConfigParams) error {
        allUsers := make(map[string]arvados.User)
        userIDToUUID := make(map[string]string) // Index by email or username
        dupedEmails := make(map[string][]arvados.User)
        allUsers := make(map[string]arvados.User)
        userIDToUUID := make(map[string]string) // Index by email or username
        dupedEmails := make(map[string][]arvados.User)

+       emptyUserIDs := []string{}

        processedUsers := make(map[string]bool)
        results, err := GetAll(cfg.Client, "users", arvados.ResourceListParams{}, &UserList{})
        if err != nil {
        processedUsers := make(map[string]bool)
        results, err := GetAll(cfg.Client, "users", arvados.ResourceListParams{}, &UserList{})
        if err != nil {


@@ -246,7 +247,9 @@ func doMain(cfg *ConfigParams) error {
                        return err
                }
                if uID == "" {
                        return err
                }
                if uID == "" {

-                       return fmt.Errorf("%s is empty for user with uuid %q", cfg.UserID, u.UUID)
+                       emptyUserIDs = append(emptyUserIDs, u.UUID)
+                       log.Printf("Empty %s found in user %s - ignoring", cfg.UserID, u.UUID)
+                       continue

                }
                if cfg.CaseInsensitive {
                        uID = strings.ToLower(uID)
                }
                if cfg.CaseInsensitive {
                        uID = strings.ToLower(uID)


@@ -327,7 +330,7 @@ func doMain(cfg *ConfigParams) error {
 
        log.Printf("User update successes: %d, skips: %d, failures: %d", len(updatesSucceeded), len(updatesSkipped), len(updatesFailed))
 
 
        log.Printf("User update successes: %d, skips: %d, failures: %d", len(updatesSucceeded), len(updatesSkipped), len(updatesFailed))
 

-       // Report duplicated emails detection
+       var errors []string

        if len(dupedEmails) > 0 {
                emails := make([]string, len(dupedEmails))
                i := 0
        if len(dupedEmails) > 0 {
                emails := make([]string, len(dupedEmails))
                i := 0


@@ -335,7 +338,13 @@ func doMain(cfg *ConfigParams) error {
                        emails[i] = e
                        i++
                }
                        emails[i] = e
                        i++
                }

-               return fmt.Errorf("skipped %d duplicated email address(es) in the cluster's local user list: %v", len(dupedEmails), emails)
+               errors = append(errors, fmt.Sprintf("skipped %d duplicated email address(es) in the cluster's local user list: %v", len(dupedEmails), emails))
+       }
+       if len(emptyUserIDs) > 0 {
+               errors = append(errors, fmt.Sprintf("skipped %d user account(s) with empty %s: %v", len(emptyUserIDs), cfg.UserID, emptyUserIDs))
+       }
+       if len(errors) > 0 {
+               return fmt.Errorf("%s", strings.Join(errors, "\n"))

        }
 
        return nil
        }
 
        return nil

diff --git a/tools/sync-users/sync-users_test.go b/tools/sync-users/sync-users_test.go
index 8b5385a32110786c0125896f5507e07a0309b8f5..1e5f688b05e6aee95e1fcee7e29abfde61f3f9cc 100644 (file)
--- a/tools/sync-users/sync-users_test.go
+++ b/tools/sync-users/sync-users_test.go
@@ -434,3 +434,39 @@ func (s *TestSuite) TestFailOnDuplicatedEmails(c *C) {
        c.Assert(err, NotNil)
        c.Assert(err, ErrorMatches, "skipped.*duplicated email address.*")
 }
        c.Assert(err, NotNil)
        c.Assert(err, ErrorMatches, "skipped.*duplicated email address.*")
 }

+
+func (s *TestSuite) TestFailOnEmptyUsernames(c *C) {
+       for i := range []int{1, 2} {
+               var user arvados.User
+               err := CreateUser(s.cfg.Client, &user, map[string]string{
+                       "email":      fmt.Sprintf("johndoe%d@example.com", i),
+                       "username":   "",
+                       "first_name": "John",
+                       "last_name":  "Doe",
+                       "is_active":  "true",
+                       "is_admin":   "false",
+               })
+               c.Assert(err, IsNil)
+               c.Assert(user.Username, Equals, fmt.Sprintf("johndoe%d", i))
+               if i == 1 {
+                       err = UpdateUser(s.cfg.Client, user.UUID, &user, map[string]string{
+                               "username": "",
+                       })
+                       c.Assert(err, IsNil)
+                       c.Assert(user.Username, Equals, "")
+               }
+       }
+
+       s.cfg.Verbose = true
+       data := [][]string{
+               {"johndoe0", "John", "Doe", "0", "0"},
+       }
+       tmpfile, err := MakeTempCSVFile(data)
+       c.Assert(err, IsNil)
+       defer os.Remove(tmpfile.Name())
+       s.cfg.Path = tmpfile.Name()
+       s.cfg.UserID = "username"
+       err = doMain(s.cfg)
+       c.Assert(err, NotNil)
+       c.Assert(err, ErrorMatches, "skipped 1 user account.*with empty username.*")
+}