[ARVADOS] created: 1.3.0-2590-ga76593348
Git user
git at public.arvados.org
Thu May 21 23:09:03 UTC 2020
at a7659334804bdbfe705f93309a55462e82320ef0 (commit)
commit a7659334804bdbfe705f93309a55462e82320ef0
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date: Thu May 21 20:08:40 2020 -0300
16435: Updates the documentation.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>
diff --git a/doc/architecture/index.html.textile.liquid b/doc/architecture/index.html.textile.liquid
index c7ea3268e..705048cd6 100644
--- a/doc/architecture/index.html.textile.liquid
+++ b/doc/architecture/index.html.textile.liquid
@@ -56,4 +56,4 @@ table(table table-bordered table-condensed).
|keep-block-check|Given a list of keep block locators, check that each block exists on one of the configured keepstore servers and verify the block hash.|
|keep-exercise|Benchmarking tool to test throughput and reliability of keepstores under various usage patterns.|
|keep-rsync|Get lists of blocks from two clusters, copy blocks which exist on source cluster but are missing from destination cluster.|
-|sync-groups|Take a CSV file listing (group, username) pairs and synchronize membership in Arvados groups.|
+|sync-groups|Take a CSV file listing with (group, user, permission) records and synchronize membership in Arvados groups.|
diff --git a/doc/user/topics/arvados-sync-groups.html.textile.liquid b/doc/user/topics/arvados-sync-groups.html.textile.liquid
index 9a609039b..7d831bf04 100644
--- a/doc/user/topics/arvados-sync-groups.html.textile.liquid
+++ b/doc/user/topics/arvados-sync-groups.html.textile.liquid
@@ -15,10 +15,12 @@ h1. Using arvados-sync-groups
This tool reads a CSV (comma-separated values) file having information about external groups and their members. When running it for the first time, it'll create a special group named 'Externally synchronized groups' meant to be the parent of all the remote groups.
-Every line on the file should have 2 values: a group name and a local user identifier, meaning that the named user is a member of the group. The tool will create the group if it doesn't exist, and add the user to it. If group member is not present on the input file, the account will be removed from the group.
+Every line on the file should have 3 values: a group name, a local user identifier and a permission level, meaning that the named user is a member of the group with the provided permission. The tool will create the group if it doesn't exist, and add the user to it. If any group member is not present on the input file, it will be removed from the group.
Users can be identified by their email address or username: the tool will check if every user exist on the system, and report back when not found. Groups on the other hand, are identified by their name.
+Permission level can be one of the following: @can_read@, @can_write@ or @can_manage@, giving the group member read, read/write or managing privileges on the group. For backwards compatibility purposes, if any record omits the third (permission) field, it will default to @can_write@ permission. You can read more about permissions on the "group management admin guide":/admin/group-management.html.
+
This tool is designed to be run periodically reading a file created by a remote auth system (ie: LDAP) dump script, applying what's included on the file as the source of truth.
commit 5c2b2811fb3646bb42fb6a0154a79e3791afc73f
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date: Thu May 21 19:00:37 2020 -0300
16435: Adds & updates tests.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>
diff --git a/tools/sync-groups/sync-groups_test.go b/tools/sync-groups/sync-groups_test.go
index 77d9756ff..9eec6b6d9 100644
--- a/tools/sync-groups/sync-groups_test.go
+++ b/tools/sync-groups/sync-groups_test.go
@@ -106,7 +106,7 @@ func MakeTempCSVFile(data [][]string) (f *os.File, err error) {
}
// GroupMembershipExists checks that both needed links exist between user and group
-func GroupMembershipExists(ac *arvados.Client, userUUID string, groupUUID string) bool {
+func GroupMembershipExists(ac *arvados.Client, userUUID string, groupUUID string, perm string) bool {
ll := LinkList{}
// Check Group -> User can_read permission
params := arvados.ResourceListParams{
@@ -145,7 +145,7 @@ func GroupMembershipExists(ac *arvados.Client, userUUID string, groupUUID string
}, {
Attr: "name",
Operator: "=",
- Operand: "can_write",
+ Operand: perm,
}, {
Attr: "tail_uuid",
Operator: "=",
@@ -259,7 +259,7 @@ func (s *TestSuite) TestIgnoreSpaces(c *C) {
groupUUID, err := RemoteGroupExists(s.cfg, groupName)
c.Assert(err, IsNil)
c.Assert(groupUUID, Not(Equals), "")
- c.Assert(GroupMembershipExists(s.cfg.Client, activeUserUUID, groupUUID), Equals, true)
+ c.Assert(GroupMembershipExists(s.cfg.Client, activeUserUUID, groupUUID, "can_write"), Equals, true)
}
}
@@ -279,6 +279,83 @@ func (s *TestSuite) TestWrongNumberOfFields(c *C) {
}
}
+// Check different membership permissions
+func (s *TestSuite) TestMembershipLevels(c *C) {
+ userEmail := s.users[arvadostest.ActiveUserUUID].Email
+ userUUID := s.users[arvadostest.ActiveUserUUID].UUID
+ data := [][]string{
+ {"TestGroup1", userEmail, "can_read"},
+ {"TestGroup2", userEmail, "can_write"},
+ {"TestGroup3", userEmail, "can_manage"},
+ {"TestGroup4", userEmail, "invalid_permission"},
+ }
+ tmpfile, err := MakeTempCSVFile(data)
+ c.Assert(err, IsNil)
+ defer os.Remove(tmpfile.Name()) // clean up
+ s.cfg.Path = tmpfile.Name()
+ err = doMain(s.cfg)
+ c.Assert(err, IsNil)
+ for _, record := range data {
+ groupName := record[0]
+ permLevel := record[2]
+ if permLevel != "invalid_permission" {
+ groupUUID, err := RemoteGroupExists(s.cfg, groupName)
+ c.Assert(err, IsNil)
+ c.Assert(groupUUID, Not(Equals), "")
+ c.Assert(GroupMembershipExists(s.cfg.Client, userUUID, groupUUID, permLevel), Equals, true)
+ } else {
+ groupUUID, err := RemoteGroupExists(s.cfg, groupName)
+ c.Assert(err, IsNil)
+ c.Assert(groupUUID, Equals, "")
+ }
+ }
+}
+
+// Check membership level change
+func (s *TestSuite) TestMembershipLevelUpdate(c *C) {
+ userEmail := s.users[arvadostest.ActiveUserUUID].Email
+ userUUID := s.users[arvadostest.ActiveUserUUID].UUID
+ groupName := "TestGroup1"
+ // Give read permissions
+ tmpfile, err := MakeTempCSVFile([][]string{{groupName, userEmail, "can_read"}})
+ c.Assert(err, IsNil)
+ defer os.Remove(tmpfile.Name()) // clean up
+ s.cfg.Path = tmpfile.Name()
+ err = doMain(s.cfg)
+ c.Assert(err, IsNil)
+ // Check permissions
+ groupUUID, err := RemoteGroupExists(s.cfg, groupName)
+ c.Assert(err, IsNil)
+ c.Assert(groupUUID, Not(Equals), "")
+ c.Assert(GroupMembershipExists(s.cfg.Client, userUUID, groupUUID, "can_read"), Equals, true)
+ c.Assert(GroupMembershipExists(s.cfg.Client, userUUID, groupUUID, "can_write"), Equals, false)
+ c.Assert(GroupMembershipExists(s.cfg.Client, userUUID, groupUUID, "can_manage"), Equals, false)
+
+ // Give write permissions
+ tmpfile, err = MakeTempCSVFile([][]string{{groupName, userEmail, "can_write"}})
+ c.Assert(err, IsNil)
+ defer os.Remove(tmpfile.Name()) // clean up
+ s.cfg.Path = tmpfile.Name()
+ err = doMain(s.cfg)
+ c.Assert(err, IsNil)
+ // Check permissions
+ c.Assert(GroupMembershipExists(s.cfg.Client, userUUID, groupUUID, "can_read"), Equals, false)
+ c.Assert(GroupMembershipExists(s.cfg.Client, userUUID, groupUUID, "can_write"), Equals, true)
+ c.Assert(GroupMembershipExists(s.cfg.Client, userUUID, groupUUID, "can_manage"), Equals, false)
+
+ // Give manage permissions
+ tmpfile, err = MakeTempCSVFile([][]string{{groupName, userEmail, "can_manage"}})
+ c.Assert(err, IsNil)
+ defer os.Remove(tmpfile.Name()) // clean up
+ s.cfg.Path = tmpfile.Name()
+ err = doMain(s.cfg)
+ c.Assert(err, IsNil)
+ // Check permissions
+ c.Assert(GroupMembershipExists(s.cfg.Client, userUUID, groupUUID, "can_read"), Equals, false)
+ c.Assert(GroupMembershipExists(s.cfg.Client, userUUID, groupUUID, "can_write"), Equals, false)
+ c.Assert(GroupMembershipExists(s.cfg.Client, userUUID, groupUUID, "can_manage"), Equals, true)
+}
+
// The absence of a user membership on the CSV file implies its removal
func (s *TestSuite) TestMembershipRemoval(c *C) {
localUserEmail := s.users[arvadostest.ActiveUserUUID].Email
@@ -302,8 +379,8 @@ func (s *TestSuite) TestMembershipRemoval(c *C) {
groupUUID, err := RemoteGroupExists(s.cfg, groupName)
c.Assert(err, IsNil)
c.Assert(groupUUID, Not(Equals), "")
- c.Assert(GroupMembershipExists(s.cfg.Client, localUserUUID, groupUUID), Equals, true)
- c.Assert(GroupMembershipExists(s.cfg.Client, remoteUserUUID, groupUUID), Equals, true)
+ c.Assert(GroupMembershipExists(s.cfg.Client, localUserUUID, groupUUID, "can_write"), Equals, true)
+ c.Assert(GroupMembershipExists(s.cfg.Client, remoteUserUUID, groupUUID, "can_write"), Equals, true)
}
// New CSV with some previous membership missing
data = [][]string{
@@ -320,14 +397,14 @@ func (s *TestSuite) TestMembershipRemoval(c *C) {
groupUUID, err := RemoteGroupExists(s.cfg, "TestGroup1")
c.Assert(err, IsNil)
c.Assert(groupUUID, Not(Equals), "")
- c.Assert(GroupMembershipExists(s.cfg.Client, localUserUUID, groupUUID), Equals, true)
- c.Assert(GroupMembershipExists(s.cfg.Client, remoteUserUUID, groupUUID), Equals, false)
+ c.Assert(GroupMembershipExists(s.cfg.Client, localUserUUID, groupUUID, "can_write"), Equals, true)
+ c.Assert(GroupMembershipExists(s.cfg.Client, remoteUserUUID, groupUUID, "can_write"), Equals, false)
// Confirm TestGroup1 memberships
groupUUID, err = RemoteGroupExists(s.cfg, "TestGroup2")
c.Assert(err, IsNil)
c.Assert(groupUUID, Not(Equals), "")
- c.Assert(GroupMembershipExists(s.cfg.Client, localUserUUID, groupUUID), Equals, false)
- c.Assert(GroupMembershipExists(s.cfg.Client, remoteUserUUID, groupUUID), Equals, true)
+ c.Assert(GroupMembershipExists(s.cfg.Client, localUserUUID, groupUUID, "can_write"), Equals, false)
+ c.Assert(GroupMembershipExists(s.cfg.Client, remoteUserUUID, groupUUID, "can_write"), Equals, true)
}
// If a group doesn't exist on the system, create it before adding users
@@ -352,7 +429,7 @@ func (s *TestSuite) TestAutoCreateGroupWhenNotExisting(c *C) {
c.Assert(err, IsNil)
c.Assert(groupUUID, Not(Equals), "")
// active user should be a member
- c.Assert(GroupMembershipExists(s.cfg.Client, arvadostest.ActiveUserUUID, groupUUID), Equals, true)
+ c.Assert(GroupMembershipExists(s.cfg.Client, arvadostest.ActiveUserUUID, groupUUID, "can_write"), Equals, true)
}
// Users listed on the file that don't exist on the system are ignored
@@ -378,7 +455,7 @@ func (s *TestSuite) TestIgnoreNonexistantUsers(c *C) {
groupUUID, err = RemoteGroupExists(s.cfg, "TestGroup4")
c.Assert(err, IsNil)
c.Assert(groupUUID, Not(Equals), "")
- c.Assert(GroupMembershipExists(s.cfg.Client, activeUserUUID, groupUUID), Equals, true)
+ c.Assert(GroupMembershipExists(s.cfg.Client, activeUserUUID, groupUUID, "can_write"), Equals, true)
}
// Users listed on the file that don't exist on the system are ignored
@@ -386,13 +463,16 @@ func (s *TestSuite) TestIgnoreEmptyFields(c *C) {
activeUserEmail := s.users[arvadostest.ActiveUserUUID].Email
activeUserUUID := s.users[arvadostest.ActiveUserUUID].UUID
// Confirm that group doesn't exist
- groupUUID, err := RemoteGroupExists(s.cfg, "TestGroup4")
- c.Assert(err, IsNil)
- c.Assert(groupUUID, Equals, "")
+ for _, groupName := range []string{"TestGroup4", "TestGroup5"} {
+ groupUUID, err := RemoteGroupExists(s.cfg, groupName)
+ c.Assert(err, IsNil)
+ c.Assert(groupUUID, Equals, "")
+ }
// Create file & run command
data := [][]string{
- {"", activeUserEmail}, // Empty field
- {"TestGroup5", ""}, // Empty field
+ {"", activeUserEmail}, // Empty field
+ {"TestGroup5", ""}, // Empty field
+ {"TestGroup5", activeUserEmail, ""}, // Empty 3rd field: is optional but cannot be empty
{"TestGroup4", activeUserEmail},
}
tmpfile, err := MakeTempCSVFile(data)
@@ -401,11 +481,15 @@ func (s *TestSuite) TestIgnoreEmptyFields(c *C) {
s.cfg.Path = tmpfile.Name()
err = doMain(s.cfg)
c.Assert(err, IsNil)
- // Confirm that memberships exist
+ // Confirm that records about TestGroup5 were skipped
+ groupUUID, err := RemoteGroupExists(s.cfg, "TestGroup5")
+ c.Assert(err, IsNil)
+ c.Assert(groupUUID, Equals, "")
+ // Confirm that membership exists
groupUUID, err = RemoteGroupExists(s.cfg, "TestGroup4")
c.Assert(err, IsNil)
c.Assert(groupUUID, Not(Equals), "")
- c.Assert(GroupMembershipExists(s.cfg.Client, activeUserUUID, groupUUID), Equals, true)
+ c.Assert(GroupMembershipExists(s.cfg.Client, activeUserUUID, groupUUID, "can_write"), Equals, true)
}
// Instead of emails, use username as identifier
@@ -432,5 +516,5 @@ func (s *TestSuite) TestUseUsernames(c *C) {
groupUUID, err = RemoteGroupExists(s.cfg, "TestGroup1")
c.Assert(err, IsNil)
c.Assert(groupUUID, Not(Equals), "")
- c.Assert(GroupMembershipExists(s.cfg.Client, activeUserUUID, groupUUID), Equals, true)
+ c.Assert(GroupMembershipExists(s.cfg.Client, activeUserUUID, groupUUID, "can_write"), Equals, true)
}
commit 84a1f53eb78e189d37897627aed7c0213d1b13ee
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date: Thu May 21 18:11:10 2020 -0300
16435: Avoids creating duplicated group->user links.
When a user needs a permission change, the g->u link already exists.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>
diff --git a/tools/sync-groups/sync-groups.go b/tools/sync-groups/sync-groups.go
index 2ffd61fec..9e2307b7a 100644
--- a/tools/sync-groups/sync-groups.go
+++ b/tools/sync-groups/sync-groups.go
@@ -450,8 +450,13 @@ func ProcessFile(
if cfg.Verbose {
log.Printf("Adding %q to group %q", groupMember, groupName)
}
- // User wasn't a member, but should be.
- if e := AddMemberToGroup(cfg, allUsers[userIDToUUID[groupMember]], gi.Group, groupPermission); e != nil {
+ // User permissionwasn't there, but should be. Avoid duplicating the
+ // group->user link when necessary.
+ createG2ULink := true
+ if _, ok := gi.PreviousMembers[groupMember]; ok {
+ createG2ULink = false // User is already member of the group
+ }
+ if e := AddMemberToGroup(cfg, allUsers[userIDToUUID[groupMember]], gi.Group, groupPermission, createG2ULink); e != nil {
err = e
return
}
@@ -498,7 +503,7 @@ func subtract(setA map[string]GroupPermissions, setB map[string]GroupPermissions
} else {
for perm := range setA[element] {
if _, ok := setB[element][perm]; !ok {
- result[element][perm] = true
+ result[element] = GroupPermissions{perm: true}
}
}
}
@@ -716,18 +721,21 @@ func RemoveMemberLinksFromGroup(cfg *ConfigParams, user arvados.User, linkNames
}
// AddMemberToGroup create membership links
-func AddMemberToGroup(cfg *ConfigParams, user arvados.User, group arvados.Group, perm string) error {
+func AddMemberToGroup(cfg *ConfigParams, user arvados.User, group arvados.Group, perm string, createG2ULink bool) error {
var newLink arvados.Link
- linkData := map[string]string{
- "owner_uuid": cfg.SysUserUUID,
- "link_class": "permission",
- "name": "can_read",
- "tail_uuid": group.UUID,
- "head_uuid": user.UUID,
- }
- if err := CreateLink(cfg, &newLink, linkData); err != nil {
- userID, _ := GetUserID(user, cfg.UserID)
- return fmt.Errorf("error adding group %q -> user %q read permission: %s", group.Name, userID, err)
+ var linkData map[string]string
+ if createG2ULink {
+ linkData = map[string]string{
+ "owner_uuid": cfg.SysUserUUID,
+ "link_class": "permission",
+ "name": "can_read",
+ "tail_uuid": group.UUID,
+ "head_uuid": user.UUID,
+ }
+ if err := CreateLink(cfg, &newLink, linkData); err != nil {
+ userID, _ := GetUserID(user, cfg.UserID)
+ return fmt.Errorf("error adding group %q -> user %q read permission: %s", group.Name, userID, err)
+ }
}
linkData = map[string]string{
"owner_uuid": cfg.SysUserUUID,
commit ffd61b1958372df2b28821910ab8c08ac53ce327
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date: Thu May 21 16:21:12 2020 -0300
16435: Adds support for different permission levels: read, write & manage.
If the 3rd field isn't present on any record, it will fallback to 'can_write'
to maintain backwards compatibility.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>
diff --git a/tools/sync-groups/sync-groups.go b/tools/sync-groups/sync-groups.go
index b77d8ccdd..2ffd61fec 100644
--- a/tools/sync-groups/sync-groups.go
+++ b/tools/sync-groups/sync-groups.go
@@ -26,11 +26,14 @@ type resourceList interface {
GetItems() []interface{}
}
-// GroupInfo tracks previous and current members of a particular Group
+// GroupPermissions maps permission levels on groups (can_read, can_write, can_manage)
+type GroupPermissions map[string]bool
+
+// GroupInfo tracks previous and current member's permissions on a particular Group
type GroupInfo struct {
Group arvados.Group
- PreviousMembers map[string]bool
- CurrentMembers map[string]bool
+ PreviousMembers map[string]GroupPermissions
+ CurrentMembers map[string]GroupPermissions
}
// GetUserID returns the correct user id value depending on the selector
@@ -137,7 +140,7 @@ func ParseFlags(config *ConfigParams) error {
usageStr := `Synchronize remote groups into Arvados from a CSV format file with 3 columns:
* 1st: Group name
* 2nd: User identifier
- * 3rd (Optional): User permission on the group: read, write or manage. (Default: write)`
+ * 3rd (Optional): User permission on the group: can_read, can_write or can_manage. (Default: can_write)`
fmt.Fprintf(os.Stderr, "%s\n\n", usageStr)
fmt.Fprintf(os.Stderr, "Usage:\n%s [OPTIONS] <input-file.csv>\n\n", os.Args[0])
fmt.Fprintf(os.Stderr, "Options:\n")
@@ -335,16 +338,30 @@ func doMain(cfg *ConfigParams) error {
// Remove previous members not listed on this run
for groupUUID := range remoteGroups {
gi := remoteGroups[groupUUID]
- evictedMembers := subtract(gi.PreviousMembers, gi.CurrentMembers)
+ evictedMemberPerms := subtract(gi.PreviousMembers, gi.CurrentMembers)
groupName := gi.Group.Name
- if len(evictedMembers) > 0 {
- log.Printf("Removing %d users from group %q", len(evictedMembers), groupName)
- }
- for evictedUser := range evictedMembers {
- if err := RemoveMemberFromGroup(cfg, allUsers[userIDToUUID[evictedUser]], gi.Group); err != nil {
+ if len(evictedMemberPerms) > 0 {
+ log.Printf("Removing permissions from %d users on group %q", len(evictedMemberPerms), groupName)
+ }
+ for member := range evictedMemberPerms {
+ var perms []string
+ completeMembershipRemoval := false
+ if _, ok := gi.CurrentMembers[member]; !ok {
+ completeMembershipRemoval = true
+ membershipsRemoved++
+ } else {
+ // Collect which user->group permission links should be removed
+ for p := range evictedMemberPerms[member] {
+ if evictedMemberPerms[member][p] {
+ perms = append(perms, p)
+ }
+ }
+ membershipsRemoved += len(perms)
+ }
+ if err := RemoveMemberLinksFromGroup(cfg, allUsers[userIDToUUID[member]],
+ perms, completeMembershipRemoval, gi.Group); err != nil {
return err
}
- membershipsRemoved++
}
}
log.Printf("Groups created: %d. Memberships added: %d, removed: %d, skipped: %d", groupsCreated, membershipsAdded, membershipsRemoved, membershipsSkipped)
@@ -382,8 +399,17 @@ func ProcessFile(
}
groupName := strings.TrimSpace(record[0])
groupMember := strings.TrimSpace(record[1]) // User ID (username or email)
- if groupName == "" || groupMember == "" {
- log.Printf("Warning: CSV record has at least one empty field (%s, %s). Skipping", groupName, groupMember)
+ groupPermission := "can_write"
+ if len(record) == 3 {
+ groupPermission = strings.ToLower(record[2])
+ }
+ if groupName == "" || groupMember == "" || groupPermission == "" {
+ log.Printf("Warning: CSV record has at least one empty field (%s, %s, %s). Skipping", groupName, groupMember, groupPermission)
+ membersSkipped++
+ continue
+ }
+ if !(groupPermission == "can_read" || groupPermission == "can_write" || groupPermission == "can_manage") {
+ log.Printf("Warning: 3rd field should be 'can_read', 'can_write' or 'can_manage'. Found: %q at line %d, skipping.", groupPermission, lineNo)
membersSkipped++
continue
}
@@ -412,26 +438,31 @@ func ProcessFile(
groupNameToUUID[groupName] = newGroup.UUID
remoteGroups[newGroup.UUID] = &GroupInfo{
Group: newGroup,
- PreviousMembers: make(map[string]bool), // Empty set
- CurrentMembers: make(map[string]bool), // Empty set
+ PreviousMembers: make(map[string]GroupPermissions),
+ CurrentMembers: make(map[string]GroupPermissions),
}
groupsCreated++
}
// Both group & user exist, check if user is a member
groupUUID := groupNameToUUID[groupName]
gi := remoteGroups[groupUUID]
- if !gi.PreviousMembers[groupMember] && !gi.CurrentMembers[groupMember] {
+ if !gi.PreviousMembers[groupMember][groupPermission] && !gi.CurrentMembers[groupMember][groupPermission] {
if cfg.Verbose {
log.Printf("Adding %q to group %q", groupMember, groupName)
}
// User wasn't a member, but should be.
- if e := AddMemberToGroup(cfg, allUsers[userIDToUUID[groupMember]], gi.Group); e != nil {
+ if e := AddMemberToGroup(cfg, allUsers[userIDToUUID[groupMember]], gi.Group, groupPermission); e != nil {
err = e
return
}
membersAdded++
}
- gi.CurrentMembers[groupMember] = true
+ if _, ok := gi.CurrentMembers[groupMember]; ok {
+ gi.CurrentMembers[groupMember][groupPermission] = true
+ } else {
+ gi.CurrentMembers[groupMember] = GroupPermissions{groupPermission: true}
+ }
+
}
return
}
@@ -459,11 +490,17 @@ func GetAll(c *arvados.Client, res string, params arvados.ResourceListParams, pa
return allItems, nil
}
-func subtract(setA map[string]bool, setB map[string]bool) map[string]bool {
- result := make(map[string]bool)
+func subtract(setA map[string]GroupPermissions, setB map[string]GroupPermissions) map[string]GroupPermissions {
+ result := make(map[string]GroupPermissions)
for element := range setA {
- if !setB[element] {
- result[element] = true
+ if _, ok := setB[element]; !ok {
+ result[element] = setA[element]
+ } else {
+ for perm := range setA[element] {
+ if _, ok := setB[element][perm]; !ok {
+ result[element][perm] = true
+ }
+ }
}
}
return result
@@ -533,8 +570,8 @@ func GetRemoteGroups(cfg *ConfigParams, allUsers map[string]arvados.User) (remot
Operand: "permission",
}, {
Attr: "name",
- Operator: "=",
- Operand: "can_write",
+ Operator: "in",
+ Operand: []string{"can_read", "can_write", "can_manage"},
}, {
Attr: "head_uuid",
Operator: "=",
@@ -547,18 +584,23 @@ func GetRemoteGroups(cfg *ConfigParams, allUsers map[string]arvados.User) (remot
}
g2uLinks, err := GetAll(cfg.Client, "links", g2uFilter, &LinkList{})
if err != nil {
- return remoteGroups, groupNameToUUID, fmt.Errorf("error getting member (can_read) links for group %q: %s", group.Name, err)
+ return remoteGroups, groupNameToUUID, fmt.Errorf("error getting group->user 'can_read' links for group %q: %s", group.Name, err)
}
u2gLinks, err := GetAll(cfg.Client, "links", u2gFilter, &LinkList{})
if err != nil {
- return remoteGroups, groupNameToUUID, fmt.Errorf("error getting member (can_write) links for group %q: %s", group.Name, err)
+ return remoteGroups, groupNameToUUID, fmt.Errorf("error getting user->group links for group %q: %s", group.Name, err)
}
- // Build a list of user ids (email or username) belonging to this group
- membersSet := make(map[string]bool)
- u2gLinkSet := make(map[string]bool)
+ // Build a list of user ids (email or username) belonging to this group.
+ membersSet := make(map[string]GroupPermissions)
+ u2gLinkSet := make(map[string]GroupPermissions)
for _, l := range u2gLinks {
- linkedMemberUUID := l.(arvados.Link).TailUUID
- u2gLinkSet[linkedMemberUUID] = true
+ link := l.(arvados.Link)
+ // Also save the member's group access level.
+ if _, ok := u2gLinkSet[link.TailUUID]; ok {
+ u2gLinkSet[link.TailUUID][link.Name] = true
+ } else {
+ u2gLinkSet[link.TailUUID] = GroupPermissions{link.Name: true}
+ }
}
for _, item := range g2uLinks {
link := item.(arvados.Link)
@@ -576,55 +618,81 @@ func GetRemoteGroups(cfg *ConfigParams, allUsers map[string]arvados.User) (remot
if err != nil {
return remoteGroups, groupNameToUUID, err
}
- membersSet[memberID] = true
+ membersSet[memberID] = u2gLinkSet[link.HeadUUID]
}
remoteGroups[group.UUID] = &GroupInfo{
Group: group,
PreviousMembers: membersSet,
- CurrentMembers: make(map[string]bool), // Empty set
+ CurrentMembers: make(map[string]GroupPermissions),
}
groupNameToUUID[group.Name] = group.UUID
}
return remoteGroups, groupNameToUUID, nil
}
-// RemoveMemberFromGroup remove all links related to the membership
-func RemoveMemberFromGroup(cfg *ConfigParams, user arvados.User, group arvados.Group) error {
+// RemoveMemberLinksFromGroup remove all links related to the membership
+func RemoveMemberLinksFromGroup(cfg *ConfigParams, user arvados.User, linkNames []string, completeRemoval bool, group arvados.Group) error {
if cfg.Verbose {
log.Printf("Getting group membership links for user %q (%s) on group %q (%s)", user.Username, user.UUID, group.Name, group.UUID)
}
var links []interface{}
- // Search for all group<->user links (both ways)
- for _, filterset := range [][]arvados.Filter{
- // Group -> User
- {{
- Attr: "link_class",
- Operator: "=",
- Operand: "permission",
- }, {
- Attr: "tail_uuid",
- Operator: "=",
- Operand: group.UUID,
- }, {
- Attr: "head_uuid",
- Operator: "=",
- Operand: user.UUID,
- }},
- // Group <- User
- {{
- Attr: "link_class",
- Operator: "=",
- Operand: "permission",
- }, {
- Attr: "tail_uuid",
- Operator: "=",
- Operand: user.UUID,
- }, {
- Attr: "head_uuid",
- Operator: "=",
- Operand: group.UUID,
- }},
- } {
+ var filters [][]arvados.Filter
+ if completeRemoval {
+ // Search for all group<->user links (both ways)
+ filters = [][]arvados.Filter{
+ // Group -> User
+ {{
+ Attr: "link_class",
+ Operator: "=",
+ Operand: "permission",
+ }, {
+ Attr: "tail_uuid",
+ Operator: "=",
+ Operand: group.UUID,
+ }, {
+ Attr: "head_uuid",
+ Operator: "=",
+ Operand: user.UUID,
+ }},
+ // Group <- User
+ {{
+ Attr: "link_class",
+ Operator: "=",
+ Operand: "permission",
+ }, {
+ Attr: "tail_uuid",
+ Operator: "=",
+ Operand: user.UUID,
+ }, {
+ Attr: "head_uuid",
+ Operator: "=",
+ Operand: group.UUID,
+ }},
+ }
+ } else {
+ // Search only for the requested Group <- User permission links
+ filters = [][]arvados.Filter{
+ {{
+ Attr: "link_class",
+ Operator: "=",
+ Operand: "permission",
+ }, {
+ Attr: "tail_uuid",
+ Operator: "=",
+ Operand: user.UUID,
+ }, {
+ Attr: "head_uuid",
+ Operator: "=",
+ Operand: group.UUID,
+ }, {
+ Attr: "name",
+ Operator: "in",
+ Operand: linkNames,
+ }},
+ }
+ }
+
+ for _, filterset := range filters {
l, err := GetAll(cfg.Client, "links", arvados.ResourceListParams{Filters: filterset}, &LinkList{})
if err != nil {
userID, _ := GetUserID(user, cfg.UserID)
@@ -648,7 +716,7 @@ func RemoveMemberFromGroup(cfg *ConfigParams, user arvados.User, group arvados.G
}
// AddMemberToGroup create membership links
-func AddMemberToGroup(cfg *ConfigParams, user arvados.User, group arvados.Group) error {
+func AddMemberToGroup(cfg *ConfigParams, user arvados.User, group arvados.Group, perm string) error {
var newLink arvados.Link
linkData := map[string]string{
"owner_uuid": cfg.SysUserUUID,
@@ -664,13 +732,13 @@ func AddMemberToGroup(cfg *ConfigParams, user arvados.User, group arvados.Group)
linkData = map[string]string{
"owner_uuid": cfg.SysUserUUID,
"link_class": "permission",
- "name": "can_write",
+ "name": perm,
"tail_uuid": user.UUID,
"head_uuid": group.UUID,
}
if err := CreateLink(cfg, &newLink, linkData); err != nil {
userID, _ := GetUserID(user, cfg.UserID)
- return fmt.Errorf("error adding user %q -> group %q write permission: %s", userID, group.Name, err)
+ return fmt.Errorf("error adding user %q -> group %q %s permission: %s", userID, group.Name, perm, err)
}
return nil
}
commit a0d5cb5096652c99f746bb9207bbf0f4220cb79d
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date: Wed May 20 18:23:08 2020 -0300
16435: Allows 2 or 3 fields per record on the CSV file.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>
diff --git a/tools/sync-groups/sync-groups.go b/tools/sync-groups/sync-groups.go
index 7c5cd0558..b77d8ccdd 100644
--- a/tools/sync-groups/sync-groups.go
+++ b/tools/sync-groups/sync-groups.go
@@ -134,9 +134,10 @@ func ParseFlags(config *ConfigParams) error {
// Set up usage message
flags.Usage = func() {
- usageStr := `Synchronize remote groups into Arvados from a CSV format file with 2 columns:
- * 1st column: Group name
- * 2nd column: User identifier`
+ usageStr := `Synchronize remote groups into Arvados from a CSV format file with 3 columns:
+ * 1st: Group name
+ * 2nd: User identifier
+ * 3rd (Optional): User permission on the group: read, write or manage. (Default: write)`
fmt.Fprintf(os.Stderr, "%s\n\n", usageStr)
fmt.Fprintf(os.Stderr, "Usage:\n%s [OPTIONS] <input-file.csv>\n\n", os.Args[0])
fmt.Fprintf(os.Stderr, "Options:\n")
@@ -362,7 +363,8 @@ func ProcessFile(
) (groupsCreated, membersAdded, membersSkipped int, err error) {
lineNo := 0
csvReader := csv.NewReader(f)
- csvReader.FieldsPerRecord = 2
+ // Allow variable number of fields.
+ csvReader.FieldsPerRecord = -1
for {
record, e := csvReader.Read()
if e == io.EOF {
@@ -373,6 +375,11 @@ func ProcessFile(
err = fmt.Errorf("error parsing %q, line %d", cfg.Path, lineNo)
return
}
+ // Only allow 2 or 3 fields per record for backwards compatibility.
+ if len(record) < 2 || len(record) > 3 {
+ err = fmt.Errorf("error parsing %q, line %d: found %d fields but only 2 or 3 are allowed", cfg.Path, lineNo, len(record))
+ return
+ }
groupName := strings.TrimSpace(record[0])
groupMember := strings.TrimSpace(record[1]) // User ID (username or email)
if groupName == "" || groupMember == "" {
diff --git a/tools/sync-groups/sync-groups_test.go b/tools/sync-groups/sync-groups_test.go
index 3ef360079..77d9756ff 100644
--- a/tools/sync-groups/sync-groups_test.go
+++ b/tools/sync-groups/sync-groups_test.go
@@ -263,6 +263,22 @@ func (s *TestSuite) TestIgnoreSpaces(c *C) {
}
}
+// Error out when records have <2 or >3 records
+func (s *TestSuite) TestWrongNumberOfFields(c *C) {
+ for _, testCase := range [][][]string{
+ {{"field1"}},
+ {{"field1", "field2", "field3", "field4"}},
+ {{"field1", "field2", "field3", "field4", "field5"}},
+ } {
+ 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, Not(IsNil))
+ }
+}
+
// The absence of a user membership on the CSV file implies its removal
func (s *TestSuite) TestMembershipRemoval(c *C) {
localUserEmail := s.users[arvadostest.ActiveUserUUID].Email
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list