Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 7 commits into from
Mar 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .changes/v2.15.0/439-improvements.md
Original file line number Diff line number Diff line change
@@ -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]
20 changes: 19 additions & 1 deletion govcd/group.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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 {
Didainius marked this conversation as resolved.
Show resolved Hide resolved
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,
}
}
13 changes: 13 additions & 0 deletions govcd/group_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down