-
Notifications
You must be signed in to change notification settings - Fork 75
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 access control calls #329
Conversation
* Added methods Client.GetAccessControl, Client.SetAccessControl * Added methods VApp.GetAccessControl, VApp.SetAccessControl, VApp.RemoveAccessControl, VApp.IsShared * Added methods AdminCatalog.GetAccessControl, AdminCatalog.SetAccessControl, AdminCatalog.RemoveAccessControl, AdminCatalog.IsShared * Added methods Catalog.GetAccessControl, Catalog.SetAccessControl, Catalog.RemoveAccessControl, Catalog.IsShared * Added methods Vdc.GetVappControlAccess, AdminOrg.GetCatalogControlAccess, Org.GetCatalogControlAccess
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From my perspective, a very clean PR. Thanks! Just a few remarks. Hopefully, others will find more :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pass one. Looks good but I need to play with it. This one contains things I noticed while reading it through.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work!
@@ -2,6 +2,13 @@ | |||
|
|||
* Improved testing tags isolation [#320](https://github.com/vmware/go-vcloud-director/pull/320) | |||
* Added command `make tagverify` to check tags isolation tests [#320](https://github.com/vmware/go-vcloud-director/pull/320) | |||
* Added methods `Client.GetAccessControl`, `Client.SetAccessControl`[#329](https://github.com/vmware/go-vcloud-director/pull/329) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe worth adding a brief mention about useTenantContext
feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few possible improvements left. Overall LGTM
Introduce access control management for vApps and Catalogs
Client.GetAccessControl
,Client.SetAccessControl
VApp.GetAccessControl
,VApp.SetAccessControl
,VApp.RemoveAccessControl
,VApp.IsShared
AdminCatalog.GetAccessControl
,AdminCatalog.SetAccessControl
,AdminCatalog.RemoveAccessControl
,AdminCatalog.IsShared
Catalog.GetAccessControl
,Catalog.SetAccessControl
,Catalog.RemoveAccessControl
,Catalog.IsShared
Vdc.GetVappAccessControl
,AdminOrg.GetCatalogAccessControl
,Org.GetCatalogAccessControl
Vdc.QueryVappList
,Vdc.GetVappList
,AdminVdc.GetVappList
,client.GetQueryType