diff --git a/.changes/v2.15.0/439-improvements.md b/.changes/v2.15.0/439-improvements.md index a02cb192d..6cd81592a 100644 --- a/.changes/v2.15.0/439-improvements.md +++ b/.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] diff --git a/govcd/group.go b/govcd/group.go index 263dd418f..6b64b3add 100644 --- a/govcd/group.go +++ b/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, + } +} diff --git a/govcd/group_test.go b/govcd/group_test.go index cd7815287..902e4d83b 100644 --- a/govcd/group_test.go +++ b/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)