Fix group.Update() when users list is populated (#449)

When group.Update() is called, users list is set to nil to avoid an error 400 on VCD side

.changes/v2.15.0/439-improvements.md

@@ -1,2 +1,3 @@
 * Add support for `User` entities imported from LDAP, with `IsExternal` property [GH-439]
 * Add support for users list attribute for `Group` [GH-439]
+* Improve `group.Update()` to avoid sending the users list to VCD to avoid unwanted errors [GH-449]

govcd/group.go

@@ -131,7 +131,7 @@ func (group *OrgGroup) Update() error {
 	util.Logger.Printf("[TRACE] Url for updating group : %s and name: %s", groupHREF.String(), group.Group.Name)
 
 	_, err = group.client.ExecuteRequest(groupHREF.String(), http.MethodPut,
-		types.MimeAdminGroup, "error updating group : %s", group.Group, nil)
+		types.MimeAdminGroup, "error updating group : %s", copyWithoutUserList(group.Group), nil)
 	return err
 }
 
@@ -183,3 +183,21 @@ func validateDeleteGroup(group *types.Group) error {
 
 	return nil
 }
+
+// copyWithoutUserList returns a copy of the given group, with the UserList attribute set to nil.
+// This can and should be used to interact with VCD after a group read from the LDAP,
+// as having this list populated will return an error 400 as VCD doesn't expect this list to be updatable.
+func copyWithoutUserList(group *types.Group) *types.Group {
+	return &types.Group{
+		XMLName:      group.XMLName,
+		Xmlns:        group.Xmlns,
+		ID:           group.ID,
+		Href:         group.Href,
+		Type:         group.Type,
+		Description:  group.Description,
+		Name:         group.Name,
+		ProviderType: group.ProviderType,
+		Role:         group.Role,
+		UsersList:    nil,
+	}
+}

govcd/group_test.go

@@ -222,9 +222,22 @@ func (vcd *TestVCD) test_GroupUserListIsPopulated(check *C) {
 	check.Assert(grp.Group.UsersList, NotNil)
 	check.Assert(grp.Group.UsersList.UserReference[0], NotNil)
 
+	// We check here that usersList doesn't make VCD fail, they should be sent as nil
+	err = grp.Update()
+	check.Assert(err, IsNil)
+
 	user, err = adminOrg.GetUserByHref(grp.Group.UsersList.UserReference[0].HREF)
 	check.Assert(err, IsNil)
 	check.Assert(user.User.Name, Equals, userName)
+	check.Assert(len(user.User.GroupReferences.GroupReference), Equals, 1)
+
+	// We check here that the user used for update is the same as we had originally, except the user list
+	grp.Group.UsersList = nil
+	check.Assert(copyWithoutUserList(grp.Group), DeepEquals, grp.Group)
+
+	// We check here that groupReferences doesn't make VCD fail, they should be sent as nil
+	err = user.Update()
+	check.Assert(err, IsNil)
 
 	// Cleanup
 	err = user.Delete(false)