[arvados] updated: 2.1.0-2576-g00e9a9a5a
git repository hosting
git at public.arvados.org
Fri Jun 24 14:03:09 UTC 2022
Summary of changes:
services/api/test/fixtures/users.yml | 8 +-
tools/sync-users/sync-users.go | 43 ++---
tools/sync-users/sync-users_test.go | 293 +++++++++++++++++++++++++++++++++++
3 files changed, 322 insertions(+), 22 deletions(-)
create mode 100644 tools/sync-users/sync-users_test.go
via 00e9a9a5a38d1ab2f0f75285172ff9ec810189ea (commit)
via b43c42e17ad0e1896abe4169cd57ac078e5ec436 (commit)
via 1733ad88ebd41c3430393411f1edf284f82e13c2 (commit)
via 11bae3ba7281cf0f9da7f682a447e8cf407a5bd5 (commit)
via f65ce8a0e34f80fc32f1403c85c83ba6ce944277 (commit)
from 2489ab043bb41a2b6ba7ef4ef5491bdae4b63584 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
commit 00e9a9a5a38d1ab2f0f75285172ff9ec810189ea
Author: Lucas Di Pentima <lucas.dipentima at curii.com>
Date: Fri Jun 24 11:02:25 2022 -0300
18858: Fixes fixture to avoid having duplicated emails in user accounts.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima at curii.com>
diff --git a/services/api/test/fixtures/users.yml b/services/api/test/fixtures/users.yml
index 495149dc8..1cf79b1a5 100644
--- a/services/api/test/fixtures/users.yml
+++ b/services/api/test/fixtures/users.yml
@@ -171,7 +171,7 @@ spectator:
container_runtime_token_user:
owner_uuid: zzzzz-tpzed-000000000000000
uuid: zzzzz-tpzed-l3skomkti0c4vg4
- email: spectator at arvados.local
+ email: container_runtime_token_user at arvados.local
first_name: Spect
last_name: Ator
identity_url: https://container_runtime_token_user.openid.local
@@ -278,7 +278,7 @@ active_no_prefs:
active_no_prefs_profile_no_getting_started_shown:
owner_uuid: zzzzz-tpzed-000000000000000
uuid: zzzzz-tpzed-a46c98d1td4aoj4
- email: active_no_prefs_profile at arvados.local
+ email: active_no_prefs_profile_no_gs at arvados.local
first_name: HasPrefs
last_name: NoProfile
identity_url: https://active_no_prefs_profile.openid.local
@@ -372,7 +372,7 @@ fuse:
permission_perftest:
owner_uuid: zzzzz-tpzed-000000000000000
uuid: zzzzz-tpzed-permissionptest
- email: fuse at arvados.local
+ email: permission_perftest at arvados.local
first_name: FUSE
last_name: User
identity_url: https://permission_perftest.openid.local
commit b43c42e17ad0e1896abe4169cd57ac078e5ec436
Author: Lucas Di Pentima <lucas.dipentima at curii.com>
Date: Fri Jun 24 10:08:54 2022 -0300
18858: Initial set of tests.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima at curii.com>
diff --git a/tools/sync-users/sync-users_test.go b/tools/sync-users/sync-users_test.go
new file mode 100644
index 000000000..6d10595d4
--- /dev/null
+++ b/tools/sync-users/sync-users_test.go
@@ -0,0 +1,293 @@
+// Copyright (C) The Arvados Authors. All rights reserved.
+//
+// SPDX-License-Identifier: AGPL-3.0
+
+package main
+
+import (
+ "fmt"
+ "io/ioutil"
+ "os"
+ "strings"
+ "testing"
+
+ "git.arvados.org/arvados.git/sdk/go/arvados"
+ . "gopkg.in/check.v1"
+)
+
+// Gocheck boilerplate
+func Test(t *testing.T) {
+ TestingT(t)
+}
+
+type TestSuite struct {
+ cfg *ConfigParams
+ ac *arvados.Client
+ users map[string]arvados.User
+}
+
+func (s *TestSuite) SetUpTest(c *C) {
+ s.ac = arvados.NewClientFromEnv()
+ u, err := s.ac.CurrentUser()
+ c.Assert(err, IsNil)
+ c.Assert(u.IsAdmin, Equals, true)
+
+ s.users = make(map[string]arvados.User)
+ ul := arvados.UserList{}
+ s.ac.RequestAndDecode(&ul, "GET", "/arvados/v1/users", nil, arvados.ResourceListParams{})
+ c.Assert(ul.ItemsAvailable, Not(Equals), 0)
+ s.users = make(map[string]arvados.User)
+ for _, u := range ul.Items {
+ s.users[u.UUID] = u
+ }
+
+ // Set up command config
+ os.Args = []string{"cmd", "somefile.csv"}
+ config, err := GetConfig()
+ c.Assert(err, IsNil)
+ s.cfg = &config
+}
+
+func (s *TestSuite) TearDownTest(c *C) {
+ var dst interface{}
+ // Reset database to fixture state after every test run.
+ err := s.cfg.Client.RequestAndDecode(&dst, "POST", "/database/reset", nil, nil)
+ c.Assert(err, IsNil)
+}
+
+var _ = Suite(&TestSuite{})
+
+// MakeTempCSVFile creates a temp file with data as comma separated values
+func MakeTempCSVFile(data [][]string) (f *os.File, err error) {
+ f, err = ioutil.TempFile("", "test_sync_users")
+ if err != nil {
+ return
+ }
+ for _, line := range data {
+ fmt.Fprintf(f, "%s\n", strings.Join(line, ","))
+ }
+ err = f.Close()
+ return
+}
+
+func ListUsers(ac *arvados.Client) ([]arvados.User, error) {
+ var ul arvados.UserList
+ err := ac.RequestAndDecode(&ul, "GET", "/arvados/v1/users", nil, arvados.ResourceListParams{})
+ if err != nil {
+ return nil, err
+ }
+ return ul.Items, nil
+}
+
+func (s *TestSuite) TestParseFlagsWithoutPositionalArgument(c *C) {
+ os.Args = []string{"cmd", "-verbose"}
+ err := ParseFlags(&ConfigParams{})
+ c.Assert(err, NotNil)
+}
+
+func (s *TestSuite) TestParseFlagsWithPositionalArgument(c *C) {
+ cfg := ConfigParams{}
+ os.Args = []string{"cmd", "/tmp/somefile.csv"}
+ err := ParseFlags(&cfg)
+ c.Assert(err, IsNil)
+ c.Assert(cfg.Path, Equals, "/tmp/somefile.csv")
+ c.Assert(cfg.Verbose, Equals, false)
+ c.Assert(cfg.DeactivateUnlisted, Equals, false)
+}
+
+func (s *TestSuite) TestParseFlagsWithOptionalFlags(c *C) {
+ cfg := ConfigParams{}
+ os.Args = []string{"cmd", "-verbose", "-deactivate-unlisted", "/tmp/somefile.csv"}
+ err := ParseFlags(&cfg)
+ c.Assert(err, IsNil)
+ c.Assert(cfg.Path, Equals, "/tmp/somefile.csv")
+ c.Assert(cfg.Verbose, Equals, true)
+ c.Assert(cfg.DeactivateUnlisted, Equals, true)
+}
+
+func (s *TestSuite) TestGetConfig(c *C) {
+ os.Args = []string{"cmd", "/tmp/somefile.csv"}
+ cfg, err := GetConfig()
+ c.Assert(err, IsNil)
+ c.Assert(cfg.AnonUserUUID, Not(Equals), "")
+ c.Assert(cfg.SysUserUUID, Not(Equals), "")
+ c.Assert(cfg.CurrentUser, Not(Equals), "")
+ c.Assert(cfg.ClusterID, Not(Equals), "")
+ c.Assert(cfg.Client, NotNil)
+}
+
+func (s *TestSuite) TestFailOnEmptyFields(c *C) {
+ records := [][]string{
+ {"", "first-name", "last-name", "1", "0"},
+ {"user at example", "", "last-name", "1", "0"},
+ {"user at example", "first-name", "", "1", "0"},
+ {"user at example", "first-name", "last-name", "", "0"},
+ {"user at example", "first-name", "last-name", "1", ""},
+ }
+ for _, record := range records {
+ data := [][]string{record}
+ tmpfile, err := MakeTempCSVFile(data)
+ c.Assert(err, IsNil)
+ defer os.Remove(tmpfile.Name())
+ s.cfg.Path = tmpfile.Name()
+ err = doMain(s.cfg)
+ c.Assert(err, NotNil)
+ c.Assert(err, ErrorMatches, ".*fields cannot be empty.*")
+ }
+}
+
+func (s *TestSuite) TestIgnoreSpaces(c *C) {
+ // Make sure users aren't already there from fixtures
+ for _, user := range s.users {
+ e := user.Email
+ found := e == "user1 at example.com" || e == "user2 at example.com" || e == "user3 at example.com"
+ c.Assert(found, Equals, false)
+ }
+ // Use CSV data with leading/trailing whitespaces, confirm that they get ignored
+ data := [][]string{
+ {" user1 at example.com", " Example", " User1", "1", "0"},
+ {"user2 at example.com ", "Example ", "User2 ", "1", "0"},
+ {" user3 at example.com ", " Example ", " User3 ", "1", "0"},
+ }
+ tmpfile, err := MakeTempCSVFile(data)
+ c.Assert(err, IsNil)
+ defer os.Remove(tmpfile.Name())
+ s.cfg.Path = tmpfile.Name()
+ err = doMain(s.cfg)
+ c.Assert(err, IsNil)
+ users, err := ListUsers(s.cfg.Client)
+ c.Assert(err, IsNil)
+ for _, userNr := range []int{1, 2, 3} {
+ found := false
+ for _, user := range users {
+ if user.Email == fmt.Sprintf("user%d at example.com", userNr) &&
+ user.LastName == fmt.Sprintf("User%d", userNr) &&
+ user.FirstName == "Example" && user.IsActive == true {
+ found = true
+ break
+ }
+ }
+ c.Assert(found, Equals, true)
+ }
+}
+
+// Error out when records have != 5 records
+func (s *TestSuite) TestWrongNumberOfFields(c *C) {
+ for _, testCase := range [][][]string{
+ {{"user1 at example.com", "Example", "User1", "1"}},
+ {{"user1 at example.com", "Example", "User1", "1", "0", "extra data"}},
+ } {
+ tmpfile, err := MakeTempCSVFile(testCase)
+ c.Assert(err, IsNil)
+ defer os.Remove(tmpfile.Name())
+ s.cfg.Path = tmpfile.Name()
+ err = doMain(s.cfg)
+ c.Assert(err, NotNil)
+ c.Assert(err, ErrorMatches, ".*expected 5 fields, found.*")
+ }
+}
+
+// Error out when records have incorrect data types
+func (s *TestSuite) TestWrongDataFields(c *C) {
+ for _, testCase := range [][][]string{
+ {{"user1 at example.com", "Example", "User1", "yep", "0"}},
+ {{"user1 at example.com", "Example", "User1", "1", "nope"}},
+ } {
+ tmpfile, err := MakeTempCSVFile(testCase)
+ c.Assert(err, IsNil)
+ defer os.Remove(tmpfile.Name())
+ s.cfg.Path = tmpfile.Name()
+ err = doMain(s.cfg)
+ c.Assert(err, NotNil)
+ c.Assert(err, ErrorMatches, ".*parsing error at line.*[active|admin] status not recognized.*")
+ }
+}
+
+// Activate and deactivate users
+func (s *TestSuite) TestUserCreationAndUpdate(c *C) {
+ testCases := []struct {
+ Email string
+ FirstName string
+ LastName string
+ Active bool
+ Admin bool
+ }{{
+ Email: "user1 at example.com",
+ FirstName: "Example",
+ LastName: "User1",
+ Active: true,
+ Admin: false,
+ }, {
+ Email: "admin1 at example.com",
+ FirstName: "Example",
+ LastName: "Admin1",
+ Active: true,
+ Admin: true,
+ }}
+ // Make sure users aren't already there from fixtures
+ for _, user := range s.users {
+ e := user.Email
+ found := e == testCases[0].Email || e == testCases[1].Email
+ c.Assert(found, Equals, false)
+ }
+ // User creation
+ data := [][]string{
+ {testCases[0].Email, testCases[0].FirstName, testCases[0].LastName, fmt.Sprintf("%t", testCases[0].Active), fmt.Sprintf("%t", testCases[0].Admin)},
+ {testCases[1].Email, testCases[1].FirstName, testCases[1].LastName, fmt.Sprintf("%t", testCases[1].Active), fmt.Sprintf("%t", testCases[1].Admin)},
+ }
+ tmpfile, err := MakeTempCSVFile(data)
+ c.Assert(err, IsNil)
+ defer os.Remove(tmpfile.Name())
+ s.cfg.Path = tmpfile.Name()
+ err = doMain(s.cfg)
+ c.Assert(err, IsNil)
+
+ users, err := ListUsers(s.cfg.Client)
+ c.Assert(err, IsNil)
+ for _, tc := range testCases {
+ var foundUser arvados.User
+ for _, user := range users {
+ if user.Email == tc.Email {
+ foundUser = user
+ break
+ }
+ }
+ c.Assert(foundUser, NotNil)
+ c.Logf("Checking recently created user %q", foundUser.Email)
+ c.Assert(foundUser.FirstName, Equals, tc.FirstName)
+ c.Assert(foundUser.LastName, Equals, tc.LastName)
+ c.Assert(foundUser.IsActive, Equals, true)
+ c.Assert(foundUser.IsAdmin, Equals, tc.Admin)
+ }
+ // User deactivation
+ testCases[0].Active = false
+ testCases[1].Active = false
+ data = [][]string{
+ {testCases[0].Email, testCases[0].FirstName, testCases[0].LastName, fmt.Sprintf("%t", testCases[0].Active), fmt.Sprintf("%t", testCases[0].Admin)},
+ {testCases[1].Email, testCases[1].FirstName, testCases[1].LastName, fmt.Sprintf("%t", testCases[1].Active), fmt.Sprintf("%t", testCases[1].Admin)},
+ }
+ tmpfile, err = MakeTempCSVFile(data)
+ c.Assert(err, IsNil)
+ defer os.Remove(tmpfile.Name())
+ s.cfg.Path = tmpfile.Name()
+ err = doMain(s.cfg)
+ c.Assert(err, IsNil)
+
+ users, err = ListUsers(s.cfg.Client)
+ c.Assert(err, IsNil)
+ for _, tc := range testCases {
+ var foundUser arvados.User
+ for _, user := range users {
+ if user.Email == tc.Email {
+ foundUser = user
+ break
+ }
+ }
+ c.Assert(foundUser, NotNil)
+ c.Logf("Checking recently deactivated user %q", foundUser.Email)
+ c.Assert(foundUser.FirstName, Equals, tc.FirstName)
+ c.Assert(foundUser.LastName, Equals, tc.LastName)
+ c.Assert(foundUser.IsActive, Equals, false)
+ c.Assert(foundUser.IsAdmin, Equals, false) // inactive users cannot be admins
+ }
+}
commit 1733ad88ebd41c3430393411f1edf284f82e13c2
Author: Lucas Di Pentima <lucas.dipentima at curii.com>
Date: Thu Jun 23 10:03:57 2022 -0300
18858: Enhances logging a bit.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima at curii.com>
diff --git a/tools/sync-users/sync-users.go b/tools/sync-users/sync-users.go
index 969e101fe..25c759456 100644
--- a/tools/sync-users/sync-users.go
+++ b/tools/sync-users/sync-users.go
@@ -198,7 +198,7 @@ func doMain(cfg *ConfigParams) error {
processedUsers[record.Email] = true
if record.Email == cfg.CurrentUser.Email {
updatesSkipped[record.Email] = true
- log.Printf("Skipping current user %q from processing", record.Email)
+ log.Printf("Skipping current user %q (%s) from processing", record.Email, cfg.CurrentUser.UUID)
continue
}
if updated, err := ProcessRecord(cfg, record, allUsers); err != nil {
@@ -213,6 +213,7 @@ func doMain(cfg *ConfigParams) error {
for email, user := range allUsers {
if shouldSkip(cfg, user) {
updatesSkipped[email] = true
+ log.Printf("Skipping unlisted user %q (%s) from deactivating", user.Email, user.UUID)
continue
}
if !processedUsers[email] && allUsers[email].IsActive {
commit 11bae3ba7281cf0f9da7f682a447e8cf407a5bd5
Author: Lucas Di Pentima <lucas.dipentima at curii.com>
Date: Thu Jun 23 09:22:53 2022 -0300
18858: Fixes user fixture: usernames can only be alphanums.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima at curii.com>
diff --git a/services/api/test/fixtures/users.yml b/services/api/test/fixtures/users.yml
index 14630d9ef..495149dc8 100644
--- a/services/api/test/fixtures/users.yml
+++ b/services/api/test/fixtures/users.yml
@@ -431,4 +431,4 @@ has_can_login_permission:
is_active: true
is_admin: false
modified_at: 2015-03-26 12:34:56.789000000 Z
- username: can-login-user
+ username: canLoginUser
commit f65ce8a0e34f80fc32f1403c85c83ba6ce944277
Author: Lucas Di Pentima <lucas.dipentima at curii.com>
Date: Tue Jun 21 11:16:08 2022 -0300
18858: Fixes unlisted account deactivation logic. Improves logging.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima at curii.com>
diff --git a/tools/sync-users/sync-users.go b/tools/sync-users/sync-users.go
index 23f111817..969e101fe 100644
--- a/tools/sync-users/sync-users.go
+++ b/tools/sync-users/sync-users.go
@@ -190,33 +190,29 @@ func doMain(cfg *ConfigParams) error {
}
log.Printf("Loaded %d records from input file", len(loadedRecords))
- updatesSucceeded, updatesFailed := 0, 0
+ updatesSucceeded := map[string]bool{}
+ updatesFailed := map[string]bool{}
+ updatesSkipped := map[string]bool{}
+
for _, record := range loadedRecords {
+ processedUsers[record.Email] = true
if record.Email == cfg.CurrentUser.Email {
+ updatesSkipped[record.Email] = true
log.Printf("Skipping current user %q from processing", record.Email)
continue
}
if updated, err := ProcessRecord(cfg, record, allUsers); err != nil {
log.Printf("error processing record %q: %s", record.Email, err)
- updatesFailed++
+ updatesFailed[record.Email] = true
} else if updated {
- processedUsers[strings.ToLower(record.Email)] = true
- updatesSucceeded++
+ updatesSucceeded[record.Email] = true
}
}
if cfg.DeactivateUnlisted {
for email, user := range allUsers {
- switch user.UUID {
- case cfg.SysUserUUID, cfg.AnonUserUUID:
- if cfg.Verbose {
- log.Printf("Skipping system user deactivation: %s", user.UUID)
- }
- continue
- case cfg.CurrentUser.UUID:
- if cfg.Verbose {
- log.Printf("Skipping current user deactivation: %s (%s)", user.Email, user.UUID)
- }
+ if shouldSkip(cfg, user) {
+ updatesSkipped[email] = true
continue
}
if !processedUsers[email] && allUsers[email].IsActive {
@@ -226,20 +222,30 @@ func doMain(cfg *ConfigParams) error {
var updatedUser arvados.User
if err := UnsetupUser(cfg.Client, user.UUID, &updatedUser); err != nil {
log.Printf("error deactivating unlisted user %q: %s", user.UUID, err)
- updatesFailed++
+ updatesFailed[email] = true
} else {
allUsers[email] = updatedUser
- updatesSucceeded++
+ updatesSucceeded[email] = true
}
}
}
}
- log.Printf("Updated %d user(s), failed to update %d user(s)", updatesSucceeded, updatesFailed)
+ log.Printf("User update successes: %d, skips: %d, failures: %d", len(updatesSucceeded), len(updatesSkipped), len(updatesFailed))
return nil
}
+func shouldSkip(cfg *ConfigParams, user arvados.User) bool {
+ switch user.UUID {
+ case cfg.SysUserUUID, cfg.AnonUserUUID:
+ return true
+ case cfg.CurrentUser.UUID:
+ return true
+ }
+ return false
+}
+
type userRecord struct {
Email string
FirstName string
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list