[ARVADOS] updated: 7327a28eb1d90b161708e9e4e855cf80f41f20ae

Git user git at public.curoverse.com
Mon Oct 30 10:37:03 EDT 2017


Summary of changes:
 tools/arv-sync-groups/arv-sync-groups.go      |  64 ++++--
 tools/arv-sync-groups/arv-sync-groups_test.go | 308 +++++++++++++++++++++++++-
 2 files changed, 337 insertions(+), 35 deletions(-)

       via  7327a28eb1d90b161708e9e4e855cf80f41f20ae (commit)
       via  2da11acebf60b8f7237bb62533a2f5671b90915c (commit)
       via  a442ad28ac9bcbe782d0e1488a4b38ab0ae7076e (commit)
       via  03e0ebb3be64fd3a3f2e1dbfe312f458d755ba44 (commit)
      from  f38cea6fa1ac48dd50789d9e2ec653b6d961c461 (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 7327a28eb1d90b161708e9e4e855cf80f41f20ae
Author: Lucas Di Pentima <ldipentima at veritasgenetics.com>
Date:   Mon Oct 30 11:35:58 2017 -0300

    12018: Fixed positional argument handling.
    Fixed & added tests.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <ldipentima at veritasgenetics.com>

diff --git a/tools/arv-sync-groups/arv-sync-groups.go b/tools/arv-sync-groups/arv-sync-groups.go
index 60fc51b..1e1306d 100644
--- a/tools/arv-sync-groups/arv-sync-groups.go
+++ b/tools/arv-sync-groups/arv-sync-groups.go
@@ -153,6 +153,7 @@ func ParseFlags(config *ConfigParams) error {
 		flags.PrintDefaults()
 	}
 
+	// Set up option flags
 	userID := flags.String(
 		"user-id",
 		"email",
@@ -173,11 +174,11 @@ func ParseFlags(config *ConfigParams) error {
 	if flags.NArg() == 0 {
 		return fmt.Errorf("please provide a path to an input file")
 	}
-	srcPath := &os.Args[1]
+	srcPath := &os.Args[flags.NFlag()+1]
 
 	// Validations
 	if *srcPath == "" {
-		return fmt.Errorf("please provide a path to an input file")
+		return fmt.Errorf("input file path invalid")
 	}
 	if !userIDOpts[*userID] {
 		var options []string
@@ -613,11 +614,11 @@ func RemoveMemberFromGroup(cfg *ConfigParams, user arvados.User, group arvados.G
 	}
 	for _, item := range links {
 		link := item.(Link)
+		userID, _ := GetUserID(user, cfg.UserID)
 		if cfg.Verbose {
-			log.Printf("Removing %q permission link for %q on group %q", link.Name, user.Email, group.Name)
+			log.Printf("Removing %q permission link for %q on group %q", link.Name, userID, group.Name)
 		}
 		if err := DeleteLink(cfg, link.UUID); err != nil {
-			userID, _ := GetUserID(user, cfg.UserID)
 			return fmt.Errorf("error removing user %q from group %q: %s", userID, group.Name, err)
 		}
 	}
@@ -636,7 +637,7 @@ func AddMemberToGroup(cfg *ConfigParams, user arvados.User, group arvados.Group)
 	}
 	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", userID, user.Email, err)
+		return fmt.Errorf("error adding group %q -> user %q read permission: %s", group.Name, userID, err)
 	}
 	linkData = map[string]string{
 		"owner_uuid": cfg.SysUserUUID,
diff --git a/tools/arv-sync-groups/arv-sync-groups_test.go b/tools/arv-sync-groups/arv-sync-groups_test.go
index 8356f69..d4dd854 100644
--- a/tools/arv-sync-groups/arv-sync-groups_test.go
+++ b/tools/arv-sync-groups/arv-sync-groups_test.go
@@ -5,10 +5,14 @@
 package main
 
 import (
+	"fmt"
+	"io/ioutil"
 	"os"
+	"strings"
 	"testing"
 
 	"git.curoverse.com/arvados.git/sdk/go/arvados"
+	"git.curoverse.com/arvados.git/sdk/go/arvadostest"
 	. "gopkg.in/check.v1"
 )
 
@@ -17,20 +21,175 @@ func Test(t *testing.T) {
 	TestingT(t)
 }
 
-type TestSuite struct{}
+type TestSuite struct {
+	cfg   *ConfigParams
+	users map[string]arvados.User
+}
+
+func (s *TestSuite) SetUpSuite(c *C) {
+	ac := arvados.NewClientFromEnv()
+	u, err := ac.CurrentUser()
+	c.Assert(err, IsNil)
+	// Check that the parent group doesn't exist
+	sysUserUUID := u.UUID[:12] + "000000000000000"
+	gl := arvados.GroupList{}
+	params := arvados.ResourceListParams{
+		Filters: []arvados.Filter{{
+			Attr:     "owner_uuid",
+			Operator: "=",
+			Operand:  sysUserUUID,
+		}, {
+			Attr:     "name",
+			Operator: "=",
+			Operand:  "Externally synchronized groups",
+		}},
+	}
+	ac.RequestAndDecode(&gl, "GET", "/arvados/v1/groups", nil, params)
+	c.Assert(gl.ItemsAvailable, Equals, 0)
+	// Set up config
+	os.Args = []string{"cmd", "somefile.csv"}
+	config, err := GetConfig()
+	c.Assert(err, IsNil)
+	// Confirm that the parent group was created
+	gl = arvados.GroupList{}
+	ac.RequestAndDecode(&gl, "GET", "/arvados/v1/groups", nil, params)
+	c.Assert(gl.ItemsAvailable, Equals, 1)
+	// Config set up complete, save config for further testing
+	s.cfg = &config
+
+	// Fetch current user list
+	ul := arvados.UserList{}
+	params = arvados.ResourceListParams{
+		Filters: []arvados.Filter{{
+			Attr:     "uuid",
+			Operator: "!=",
+			Operand:  s.cfg.SysUserUUID,
+		}},
+	}
+	ac.RequestAndDecode(&ul, "GET", "/arvados/v1/users", nil, params)
+	c.Assert(ul.ItemsAvailable, Not(Equals), 0)
+	s.users = make(map[string]arvados.User)
+	for _, u := range ul.Items {
+		s.users[u.UUID] = u
+	}
+	c.Assert(len(s.users), Not(Equals), 0)
+}
 
 var _ = Suite(&TestSuite{})
 
-func (s *TestSuite) TestParseFlagsWithPath(c *C) {
+// MakeTempCVSFile 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_remote_groups")
+	if err != nil {
+		return
+	}
+	for _, line := range data {
+		fmt.Fprintf(f, "%s\n", strings.Join(line, ","))
+	}
+	err = f.Close()
+	return
+}
+
+// GroupMembershipExists checks that both needed links exist between user and group
+func GroupMembershipExists(ac *arvados.Client, userUUID string, groupUUID string) bool {
+	ll := LinkList{}
+	// Check Group -> User can_read permission
+	params := arvados.ResourceListParams{
+		Filters: []arvados.Filter{{
+			Attr:     "link_class",
+			Operator: "=",
+			Operand:  "permission",
+		}, {
+			Attr:     "tail_uuid",
+			Operator: "=",
+			Operand:  groupUUID,
+		}, {
+			Attr:     "name",
+			Operator: "=",
+			Operand:  "can_read",
+		}, {
+			Attr:     "head_uuid",
+			Operator: "=",
+			Operand:  userUUID,
+		}},
+	}
+	ac.RequestAndDecode(&ll, "GET", "/arvados/v1/links", nil, params)
+	if ll.Len() != 1 {
+		return false
+	}
+	// Check User -> Group can_write permission
+	params = arvados.ResourceListParams{
+		Filters: []arvados.Filter{{
+			Attr:     "link_class",
+			Operator: "=",
+			Operand:  "permission",
+		}, {
+			Attr:     "head_uuid",
+			Operator: "=",
+			Operand:  groupUUID,
+		}, {
+			Attr:     "name",
+			Operator: "=",
+			Operand:  "can_write",
+		}, {
+			Attr:     "tail_uuid",
+			Operator: "=",
+			Operand:  userUUID,
+		}},
+	}
+	ac.RequestAndDecode(&ll, "GET", "/arvados/v1/links", nil, params)
+	if ll.Len() != 1 {
+		return false
+	}
+	return true
+}
+
+// If named group exists, return its UUID
+func RemoteGroupExists(cfg *ConfigParams, groupName string) (uuid string, err error) {
+	gl := arvados.GroupList{}
+	params := arvados.ResourceListParams{
+		Filters: []arvados.Filter{{
+			Attr:     "name",
+			Operator: "=",
+			Operand:  groupName,
+		}, {
+			Attr:     "owner_uuid",
+			Operator: "=",
+			Operand:  cfg.ParentGroupUUID,
+		}, {
+			Attr:     "group_class",
+			Operator: "=",
+			Operand:  "role",
+		}},
+	}
+	err = cfg.Client.RequestAndDecode(&gl, "GET", "/arvados/v1/groups", nil, params)
+	if err != nil {
+		return "", err
+	}
+	if gl.ItemsAvailable == 0 {
+		// No group with this name
+		uuid = ""
+	} else if gl.ItemsAvailable == 1 {
+		// Group found
+		uuid = gl.Items[0].UUID
+	} else {
+		// This should never happen
+		uuid = ""
+		err = fmt.Errorf("more than 1 group found with the same name and parent")
+	}
+	return
+}
+
+func (s *TestSuite) TestParseFlagsWithPositionalArgument(c *C) {
 	cfg := ConfigParams{}
-	os.Args = []string{"cmd", "-path", "/tmp/somefile.csv", "-verbose"}
+	os.Args = []string{"cmd", "-verbose", "/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.Check(cfg.Path, Equals, "/tmp/somefile.csv")
+	c.Check(cfg.Verbose, Equals, true)
 }
 
-func (s *TestSuite) TestParseFlagsWithoutPath(c *C) {
+func (s *TestSuite) TestParseFlagsWithoutPositionalArgument(c *C) {
 	os.Args = []string{"cmd", "-verbose"}
 	err := ParseFlags(&ConfigParams{})
 	c.Assert(err, NotNil)
@@ -43,17 +202,142 @@ func (s *TestSuite) TestGetUserID(c *C) {
 	}
 	email, err := GetUserID(u, "email")
 	c.Assert(err, IsNil)
-	c.Assert(email, Equals, "testuser at example.com")
+	c.Check(email, Equals, "testuser at example.com")
 	_, err = GetUserID(u, "bogus")
 	c.Assert(err, NotNil)
 }
 
 func (s *TestSuite) TestGetConfig(c *C) {
-	os.Args = []string{"cmd", "-path", "/tmp/somefile.csv"}
+	os.Args = []string{"cmd", "/tmp/somefile.csv"}
 	cfg, err := GetConfig()
 	c.Assert(err, IsNil)
-	c.Assert(cfg.SysUserUUID, NotNil)
-	c.Assert(cfg.Client, NotNil)
-	c.Assert(cfg.ParentGroupUUID, NotNil)
-	c.Assert(cfg.ParentGroupName, Equals, "Externally synchronized groups")
+	c.Check(cfg.SysUserUUID, NotNil)
+	c.Check(cfg.Client, NotNil)
+	c.Check(cfg.ParentGroupUUID, NotNil)
+	c.Check(cfg.ParentGroupName, Equals, "Externally synchronized groups")
+}
+
+// Ignore leading & trailing spaces on group & users names
+func (s *TestSuite) TestIgnoreSpaces(c *C) {
+	activeUserEmail := s.users[arvadostest.ActiveUserUUID].Email
+	activeUserUUID := s.users[arvadostest.ActiveUserUUID].UUID
+	// Confirm that the groups don't exist
+	for _, groupName := range []string{"TestGroup1", "TestGroup2", "Test Group 3"} {
+		groupUUID, err := RemoteGroupExists(s.cfg, groupName)
+		c.Assert(err, IsNil)
+		c.Assert(groupUUID, Equals, "")
+	}
+	data := [][]string{
+		{" TestGroup1", activeUserEmail},
+		{"TestGroup2 ", " " + activeUserEmail},
+		{" Test Group 3 ", activeUserEmail + " "},
+	}
+	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)
+	// Check that 3 groups were created correctly, and have the active user as
+	// a member.
+	for _, groupName := range []string{"TestGroup1", "TestGroup2", "Test Group 3"} {
+		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)
+	}
+}
+
+// The absence of a user membership on the CSV file implies its removal
+func (s *TestSuite) TestMembershipRemoval(c *C) {
+	activeUserEmail := s.users[arvadostest.ActiveUserUUID].Email
+	activeUserUUID := s.users[arvadostest.ActiveUserUUID].UUID
+	data := [][]string{
+		{"TestGroup1", activeUserEmail},
+		{"TestGroup2", activeUserEmail},
+	}
+	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)
+	// Confirm that memberships exist
+	for _, groupName := range []string{"TestGroup1", "TestGroup2"} {
+		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)
+	}
+	// New CSV with one previous membership missing
+	data = [][]string{
+		{"TestGroup1", activeUserEmail},
+	}
+	tmpfile2, err := MakeTempCSVFile(data)
+	c.Assert(err, IsNil)
+	defer os.Remove(tmpfile2.Name()) // clean up
+	s.cfg.Path = tmpfile2.Name()
+	err = doMain(s.cfg)
+	c.Assert(err, IsNil)
+	// Confirm TestGroup1 membership still exist
+	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)
+	// Confirm TestGroup2 membership was removed
+	groupUUID, err = RemoteGroupExists(s.cfg, "TestGroup2")
+	c.Assert(err, IsNil)
+	c.Assert(groupUUID, Not(Equals), "")
+	c.Assert(GroupMembershipExists(s.cfg.Client, activeUserUUID, groupUUID), Equals, false)
+}
+
+// If a group doesn't exist on the system, create it before adding users
+func (s *TestSuite) TestAutoCreateGroupWhenNotExisting(c *C) {
+	groupName := "Testers"
+	// Confirm that group doesn't exist
+	groupUUID, err := RemoteGroupExists(s.cfg, groupName)
+	c.Assert(err, IsNil)
+	c.Assert(groupUUID, Equals, "")
+	// Make a tmp CSV file
+	data := [][]string{
+		{groupName, s.users[arvadostest.ActiveUserUUID].Email},
+	}
+	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)
+	// "Testers" group should now exist
+	groupUUID, err = RemoteGroupExists(s.cfg, groupName)
+	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)
+}
+
+// Users listed on the file that don't exist on the system are ignored
+func (s *TestSuite) TestIgnoreNonexistantUsers(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, "")
+	// Create file & run command
+	data := [][]string{
+		{"TestGroup4", "nonexistantuser at unknowndomain.com"}, // Processed first
+		{"TestGroup4", activeUserEmail},
+	}
+	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)
+	// Confirm that memberships exist
+	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)
 }

commit 2da11acebf60b8f7237bb62533a2f5671b90915c
Author: Lucas Di Pentima <ldipentima at veritasgenetics.com>
Date:   Thu Oct 26 17:31:26 2017 -0300

    12018: Changed the input file parameter to be positional.
    Enhanced usage message.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <ldipentima at veritasgenetics.com>

diff --git a/tools/arv-sync-groups/arv-sync-groups.go b/tools/arv-sync-groups/arv-sync-groups.go
index b21d22e..60fc51b 100644
--- a/tools/arv-sync-groups/arv-sync-groups.go
+++ b/tools/arv-sync-groups/arv-sync-groups.go
@@ -19,8 +19,6 @@ import (
 	"git.curoverse.com/arvados.git/sdk/go/arvados"
 )
 
-// const remoteGroupParentName string = "Externally synchronized groups"
-
 type resourceList interface {
 	Len() int
 	GetItems() []interface{}
@@ -112,7 +110,13 @@ func (l LinkList) GetItems() (out []interface{}) {
 }
 
 func main() {
-	if err := doMain(); err != nil {
+	// Parse & validate arguments, set up arvados client.
+	cfg, err := GetConfig()
+	if err != nil {
+		log.Fatalf("%v", err)
+	}
+
+	if err := doMain(&cfg); err != nil {
 		log.Fatalf("%v", err)
 	}
 }
@@ -135,11 +139,20 @@ func ParseFlags(config *ConfigParams) error {
 		"email":    true, // default
 		"username": true,
 	}
+
 	flags := flag.NewFlagSet("arv-sync-groups", flag.ExitOnError)
-	srcPath := flags.String(
-		"path",
-		"",
-		"Local file path containing a CSV format: GroupName,UserID")
+
+	// 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`
+		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")
+		flags.PrintDefaults()
+	}
+
 	userID := flags.String(
 		"user-id",
 		"email",
@@ -156,6 +169,12 @@ func ParseFlags(config *ConfigParams) error {
 	// Parse args; omit the first arg which is the command name
 	flags.Parse(os.Args[1:])
 
+	// Input file as a required positional argument
+	if flags.NArg() == 0 {
+		return fmt.Errorf("please provide a path to an input file")
+	}
+	srcPath := &os.Args[1]
+
 	// Validations
 	if *srcPath == "" {
 		return fmt.Errorf("please provide a path to an input file")
@@ -260,13 +279,7 @@ func GetConfig() (config ConfigParams, err error) {
 	return config, nil
 }
 
-func doMain() error {
-	// Parse & validate arguments, set up arvados client.
-	cfg, err := GetConfig()
-	if err != nil {
-		return err
-	}
-
+func doMain(cfg *ConfigParams) error {
 	// Try opening the input file early, just in case there's a problem.
 	f, err := os.Open(cfg.Path)
 	if err != nil {
@@ -298,7 +311,7 @@ func doMain() error {
 	}
 
 	// Get remote groups and their members
-	remoteGroups, groupNameToUUID, err := GetRemoteGroups(&cfg, allUsers)
+	remoteGroups, groupNameToUUID, err := GetRemoteGroups(cfg, allUsers)
 	if err != nil {
 		return err
 	}
@@ -307,7 +320,7 @@ func doMain() error {
 	membershipsRemoved := 0
 
 	// Read the CSV file
-	groupsCreated, membershipsAdded, membershipsSkipped, err := ProcessFile(&cfg, f, userIDToUUID, groupNameToUUID, remoteGroups, allUsers)
+	groupsCreated, membershipsAdded, membershipsSkipped, err := ProcessFile(cfg, f, userIDToUUID, groupNameToUUID, remoteGroups, allUsers)
 	if err != nil {
 		return err
 	}
@@ -321,7 +334,7 @@ func doMain() error {
 			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 err := RemoveMemberFromGroup(cfg, allUsers[userIDToUUID[evictedUser]], gi.Group); err != nil {
 				return err
 			}
 			membershipsRemoved++

commit a442ad28ac9bcbe782d0e1488a4b38ab0ae7076e
Author: Lucas Di Pentima <ldipentima at veritasgenetics.com>
Date:   Thu Oct 26 15:21:36 2017 -0300

    12018: Create synchronized remote groups as "role" groups.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <ldipentima at veritasgenetics.com>

diff --git a/tools/arv-sync-groups/arv-sync-groups.go b/tools/arv-sync-groups/arv-sync-groups.go
index 0f70256..b21d22e 100644
--- a/tools/arv-sync-groups/arv-sync-groups.go
+++ b/tools/arv-sync-groups/arv-sync-groups.go
@@ -364,8 +364,9 @@ func ProcessFile(cfg *ConfigParams, f *os.File, userIDToUUID map[string]string,
 			}
 			var newGroup arvados.Group
 			groupData := map[string]string{
-				"name":       groupName,
-				"owner_uuid": cfg.ParentGroupUUID,
+				"name":        groupName,
+				"owner_uuid":  cfg.ParentGroupUUID,
+				"group_class": "role",
 			}
 			if e := CreateGroup(cfg, &newGroup, groupData); e != nil {
 				err = fmt.Errorf("error creating group named %q: %s", groupName, err)

commit 03e0ebb3be64fd3a3f2e1dbfe312f458d755ba44
Author: Lucas Di Pentima <ldipentima at veritasgenetics.com>
Date:   Thu Oct 26 13:25:37 2017 -0300

    12018: Fixed membership removal call. Added input validation.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <ldipentima at veritasgenetics.com>

diff --git a/tools/arv-sync-groups/arv-sync-groups.go b/tools/arv-sync-groups/arv-sync-groups.go
index 56541d3..0f70256 100644
--- a/tools/arv-sync-groups/arv-sync-groups.go
+++ b/tools/arv-sync-groups/arv-sync-groups.go
@@ -321,7 +321,7 @@ func doMain() error {
 			log.Printf("Removing %d users from group %q", len(evictedMembers), groupName)
 		}
 		for evictedUser := range evictedMembers {
-			if err := RemoveMemberFromGroup(&cfg, allUsers[evictedUser], gi.Group); err != nil {
+			if err := RemoveMemberFromGroup(&cfg, allUsers[userIDToUUID[evictedUser]], gi.Group); err != nil {
 				return err
 			}
 			membershipsRemoved++
@@ -600,7 +600,7 @@ func RemoveMemberFromGroup(cfg *ConfigParams, user arvados.User, group arvados.G
 	for _, item := range links {
 		link := item.(Link)
 		if cfg.Verbose {
-			log.Printf("Removing permission link for %q on group %q", user.Email, group.Name)
+			log.Printf("Removing %q permission link for %q on group %q", link.Name, user.Email, group.Name)
 		}
 		if err := DeleteLink(cfg, link.UUID); err != nil {
 			userID, _ := GetUserID(user, cfg.UserID)
@@ -655,6 +655,9 @@ func CreateLink(cfg *ConfigParams, dst *Link, linkData map[string]string) error
 
 // DeleteLink deletes a link by its UUID
 func DeleteLink(cfg *ConfigParams, linkUUID string) error {
+	if linkUUID == "" {
+		return fmt.Errorf("cannot delete link with invalid UUID: %q", linkUUID)
+	}
 	return cfg.Client.RequestAndDecode(&Link{}, "DELETE", "/arvados/v1/links/"+linkUUID, nil, nil)
 }
 

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list