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

Add user list to Group and add IsExternal behaviour to User #439

Merged
merged 12 commits into from
Mar 7, 2022

Conversation

adambarreiro
Copy link
Collaborator

@adambarreiro adambarreiro commented Feb 24, 2022

Signed-off-by: abarreiro abarreiro@vmware.com

Description

This PR modifies some behaviour, described below, of the Group and User resources.

The proposed changes enhance this Group type by adding the UsersList attribute. This attribute contains a list of references to all the users that belong to the group. However, and this is a bit of a quirk, this list is only populated whenever the user is created after the group.
For reference, the behaviour is exactly the same as the attribute GroupReferences from the User type. It's also populated in the same scenario, only.

As a side change, this PR also changes how passwords are managed for Users when they're external, or in other words, when they're imported (using UI terms). When IsExternal is true, this means the user is imported. When it is imported, it doesn't need a password. If a password is specified anyway, it is ignored by VCD. For the sake of simplicity, in the changes proposed we simply remove it when this happens.

Also, when a user is imported, only the username+role is required (+other flags such as IsExternal=true). This matches the Import user UI. The remaining fields are filled by fetching the details from the LDAP.

List of changes

Added UsersList to the Group type

When you import a LDAP group in VCD, and there are some users that belong to this group, this list will be populated.

Added a new test for Groups

It imports a Group from the testing LDAP, imports a user from the testing LDAP, then checks that retrieving the group returns the imported user in the list.

Added IsExternal attribute to the OrgUserConfiguration struct

When a user is imported, and not created, IsExternal must be true. Otherwise, VCD considers it as a local user instead of imported from LDAP. This struct is just used as a simplification of the User XML.

Modified Password behaviour for users

Before, password was mandatory. This is not true for users being created with IsExternal=true, which ignore the password. I took the decision of overriding this field for simplicity, but VCD ignores it if populated anyway.

Signed-off-by: abarreiro <abarreiro@vmware.com>
Signed-off-by: abarreiro <abarreiro@vmware.com>
@adambarreiro adambarreiro self-assigned this Feb 24, 2022
#
Signed-off-by: abarreiro <abarreiro@vmware.com>
@adambarreiro adambarreiro marked this pull request as ready for review February 24, 2022 16:04
abarreiro added 3 commits February 24, 2022 17:08
#
Signed-off-by: abarreiro <abarreiro@vmware.com>
#
Signed-off-by: abarreiro <abarreiro@vmware.com>
#
Signed-off-by: abarreiro <abarreiro@vmware.com>
Copy link
Collaborator

@lvirbalas lvirbalas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a tiny ask.

Signed-off-by: abarreiro <abarreiro@vmware.com>
@adambarreiro adambarreiro force-pushed the add-org-groups-members branch from c24952f to 1409525 Compare February 28, 2022 09:38
Signed-off-by: abarreiro <abarreiro@vmware.com>
@adambarreiro adambarreiro marked this pull request as draft February 28, 2022 14:14
abarreiro added 3 commits February 28, 2022 16:10
Signed-off-by: abarreiro <abarreiro@vmware.com>
Signed-off-by: abarreiro <abarreiro@vmware.com>
#
Signed-off-by: abarreiro <abarreiro@vmware.com>
@adambarreiro adambarreiro changed the title Add user list to organization groups Add user list to Group and add IsExternal behaviour to User Mar 1, 2022
@adambarreiro adambarreiro marked this pull request as ready for review March 2, 2022 16:03
@adambarreiro adambarreiro requested a review from lvirbalas March 2, 2022 16:03
Copy link
Collaborator

@lvirbalas lvirbalas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is the changelog entry?

Signed-off-by: abarreiro <abarreiro@vmware.com>
@adambarreiro
Copy link
Collaborator Author

Where is the changelog entry?

I forgot to add it. I've just pushed it. Thanks for reminding!

@adambarreiro adambarreiro removed the request for review from dataclouder March 3, 2022 11:09
@adambarreiro adambarreiro removed the request for review from vbauzys March 3, 2022 11:09
Copy link
Collaborator

@Didainius Didainius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks

Copy link
Contributor

@mikeletux mikeletux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great @adambarreiro , approved!

@adambarreiro adambarreiro merged commit 7827b70 into vmware:main Mar 7, 2022
@adambarreiro adambarreiro deleted the add-org-groups-members branch March 7, 2022 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants