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 datasource_vcd_org_group; Add group_names, is_external attribute to org_user; Add user_names attribute to org_group #798

Merged
merged 36 commits into from
Mar 24, 2022

Conversation

adambarreiro
Copy link
Collaborator

@adambarreiro adambarreiro commented Mar 2, 2022

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

⚠️ This PR depends on:

Description

This PR modifies some behaviour, described below, of the vcd_org_group and vcd_org_user resources and datasources. It also introduces the new vcd_org_group datasource.

The proposed changes enhance the vcd_org_group type by adding the user_names attribute. This attribute contains a set of names of all the users that belong to the group. However, and this is a bit of a quirk, this set is only populated whenever the user is created after the group.
That's why, if someone wants the set to be populated, she should add:

resource "vcd_org_user" "myUser" {
  ...
  depends_on = [
    vcd_org_group.myGroup,
  ]
}

The behaviour is exactly the same as the attribute group_references from the vcd_org_user resource, which was added in the same way.

As a side change, this PR also adds the scenario where a vcd_org_user is not created but imported from an LDAP ("Import" is a VCD UI term). To import a user from the LDAP, the is_external attribute has been added. To use these kind of users, one must not declare any other attribute except name and is_external=true, they will be fetched from the LDAP.

Last but not least, the default values for deployed_vm_quota and stored_vm_quota from the vcd_org_user have been removed. Reason is that these fields cannot be computed and default'ed at the same time. Now, if not declared, local users will have unlimited quota, and imported users (LDAP users) will have no quota at all.

Detailed changelist

  • vcd/datasource_vcd_org.go: Updated to new function signatures of the Terraform SDK.
  • vcd/datasource_vcd_org_group.go: New data source for LDAP groups.
  • vcd/datasource_vcd_org_user.go:
    • Added is_external attribute to get users that were imported from an LDAP.
    • Added group_names attribute with the set of group names to which the imported user belongs.
    • Updated to new function signatures of the Terraform SDK.
  • vcd/resource_vcd_org.go: Updated to new function signatures of the Terraform SDK.
  • vcd/resource_vcd_org_group.go:
    • Added user_names attribute with the set of user names that belong to this group. This attribute is only filled when user is created after the group.
  • vcd/resource_vcd_org_user.go:
    • Added is_external flag to import users from LDAP. When is_external is true, no password nor other attributes are needed because they're picked from LDAP. This makes other attributes be Computed.
    • Removed defaults from deployed_vm_quota and stored_vm_quota for the reason above.

Signed-off-by: abarreiro <abarreiro@vmware.com>
#
Signed-off-by: abarreiro <abarreiro@vmware.com>
Signed-off-by: abarreiro <abarreiro@vmware.com>
Signed-off-by: abarreiro <abarreiro@vmware.com>
Signed-off-by: abarreiro <abarreiro@vmware.com>
Signed-off-by: abarreiro <abarreiro@vmware.com>
#
Signed-off-by: abarreiro <abarreiro@vmware.com>
#
Signed-off-by: abarreiro <abarreiro@vmware.com>
#
Signed-off-by: abarreiro <abarreiro@vmware.com>
Signed-off-by: abarreiro <abarreiro@vmware.com>
Signed-off-by: abarreiro <abarreiro@vmware.com>
Signed-off-by: abarreiro <abarreiro@vmware.com>
#
Signed-off-by: abarreiro <abarreiro@vmware.com>
#
Signed-off-by: abarreiro <abarreiro@vmware.com>
Signed-off-by: abarreiro <abarreiro@vmware.com>
@adambarreiro adambarreiro marked this pull request as ready for review March 3, 2022 11:09
@adambarreiro adambarreiro requested review from mikeletux and removed request for dataclouder and vbauzys March 3, 2022 11:09
Signed-off-by: abarreiro <abarreiro@vmware.com>
Signed-off-by: abarreiro <abarreiro@vmware.com>
Signed-off-by: abarreiro <abarreiro@vmware.com>
#
Signed-off-by: abarreiro <abarreiro@vmware.com>
Signed-off-by: abarreiro <abarreiro@vmware.com>
Signed-off-by: abarreiro <abarreiro@vmware.com>
Signed-off-by: abarreiro <abarreiro@vmware.com>
Copy link
Contributor

@vbauzys vbauzys left a comment

Choose a reason for hiding this comment

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

LGTM. Needs testing.

Signed-off-by: abarreiro <abarreiro@vmware.com>
Signed-off-by: abarreiro <abarreiro@vmware.com>
Signed-off-by: abarreiro <abarreiro@vmware.com>
Signed-off-by: abarreiro <abarreiro@vmware.com>
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.

Thanks for all these fixes and improvements!
I had another manual run after data source fix and it works!

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

Will do another run of the tests to be sure all is OK before merging

@adambarreiro
Copy link
Collaborator Author

Will do another run of the tests to be sure all is OK before merging

Tests passed :)

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.

Did some through manual testing an it worked very well 💯
Also run acceptance tests and passed!

Great work @adambarreiro ! Keep it up!
Approved!

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

Re-tested with renaming of attributes, all OK.

Signed-off-by: abarreiro <abarreiro@vmware.com>
@adambarreiro adambarreiro merged commit 10048b5 into vmware:main Mar 24, 2022
@adambarreiro adambarreiro deleted the vcd-org-group-datasource branch March 24, 2022 11:21
@adambarreiro adambarreiro changed the title Add list of users to Groups, list of groups to Users, add imported users from LDAP functionality Add datasource_vcd_org_group; Add group_names, is_external attribute to org_user; Add user_names attribute to org_group Mar 24, 2022
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.

None yet

6 participants