Skip to content
This repository has been archived by the owner on Jul 1, 2021. It is now read-only.

Update ServiceInstance.rb #37

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Update ServiceInstance.rb #37

wants to merge 2 commits into from

Conversation

iarebatman
Copy link

Related 'bug': http://projects.theforeman.org/issues/5006.
Not sure this is the proper approach to fixing the solution, but it allows us to properly retrieve the datacenter in our environment. Feedback welcome.

Related 'bug': http://projects.theforeman.org/issues/5006.
Not sure this is the proper approach to fixing the solution, but it allows us to properly retrieve the datacenter in our environment.  Feedback welcome.
@cdickmann
Copy link
Contributor

As this is just a convenience method, I would rather add a new method called find_datacenter_by_searchindex or something

Revert change to find_datacenter and add new helper method.
@iarebatman
Copy link
Author

I have no issue with doing that - could you comment on the method in which
I get the datacenter though? I'm worried about datacenters nested in
folders and such - I don't really have a way to test that what I changed
still works for those cases.

On Wed, Apr 2, 2014 at 9:58 AM, cdickmann notifications@github.com wrote:

As this is just a convenience method, I would rather add a new method
called find_datacenter_by_searchindex or something

Reply to this email directly or view it on GitHubhttps://github.com//pull/37#issuecomment-39340463
.

@cdickmann
Copy link
Contributor

Thats my point. If you just add a new method, it is not a big deal if it has a bug in some cases. I have larger deployments, so if you tell me what to test, I can test it.

@iarebatman
Copy link
Author

Alright, well I've already moved it to a new method. I believe the
important thing to test in this case would be:
Given: A path such as /A/B/C/DEV_DATACENTER, where DEV_DATACENTER is the
datacenter and A, B, and C are nested folders.
When: find_datacenter_by_searchindex is called with the path parameter set
to DEV_DATACENTER
That: A valid instance of DEV_DATACENTER is returned to the caller.

On Wed, Apr 2, 2014 at 10:30 AM, cdickmann notifications@github.com wrote:

Thats my point. If you just add a new method, it is not a big deal if it
has a bug in some cases. I have larger deployments, so if you tell me what
to test, I can test it.

Reply to this email directly or view it on GitHubhttps://github.com//pull/37#issuecomment-39344660
.

@mkuzmin
Copy link

mkuzmin commented May 6, 2014

Oops, I'm dealing with the same bug in #39.

But it looks both our patches are wrong.
Instead of changing find_datacenter method, or introducing find_datacenter_working_edition one, we'll probably have to fix this bug right within traverse method of RbVmomi::VIM::Folder class.

@mkuzmin
Copy link

mkuzmin commented May 8, 2014

Well, I think I've figured this issue out. This is not a bug, but a misconfiguration of our vCenter instances.
VMware could design vCenter permission model better, and maybe its web services API could be improved. But anyway, there is nothing to fix in rbvmomi.

When you grant permissions on specifiс datacenters, users implicitly get read-only permissions on all upper-level folders. And this is enough for vShere Client to work properly.
But vSphere Web Services API (that's used by rbvmomi) requires to have these permissions granted explicitly on a root folder, and all nested folders in a datacenter hierarchy.
Permissions

Pay attention to Propogate to Child Objects option. If you hide some datacenters from your users, then it should be unchecked, otherwise users get access to all other items. But if you are using nested folders, then the permission should be individually applied to each folder.

This should not affect security anyhow, your users already have these permissions anyway.

@hartsock hartsock modified the milestone: v1.8.3 Sep 23, 2014
@hartsock hartsock modified the milestones: v1.8.2, v1.8.3 Jul 27, 2016
@hartsock hartsock self-assigned this Jul 27, 2016
@hartsock hartsock modified the milestones: v1.8.2, v1.8.3 Jul 29, 2016
@jrgarcia jrgarcia removed this from the v1.8.3 milestone May 8, 2019
@jrgarcia jrgarcia removed their assignment Apr 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants