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

Fixed Admin Verification for HyperV #8510

Closed
wants to merge 2 commits into from

Conversation

johnrizzo1
Copy link
Contributor

Fixed admin test to verify that you are running in an elevated shell, not that you are in the administrators group since that is not required.

John Rizzo added 2 commits April 21, 2017 17:18
… not that you are in the administrators group since that is not required.
…section of the config. For example

    default.vm.provider 'hyperv' do |provider|
      provider.vswitch = 'DockerNAT'
    end
@DrowningElysium
Copy link
Contributor

I can confirm that this fixes the admin issues I have when trying to create smb shares

@mwrock
Copy link
Contributor

mwrock commented Apr 29, 2017

This does indeed fix the issue where the hyperv provider errors stating that the user lacks admin rights when they in fact do. It currently just checks if the user is a system account and that would be very unlikely unless vagrant is running as a service.

I would remove the second commit from this PR. Its completely unrelated and IMO unnecessary since the bridge setting should accomplish this.

@invisibleaxm
Copy link

fwiw, I stumbled into this issue, manually applied the fix and i can also confirmed that it worked.

@wickedviking
Copy link

wickedviking commented May 1, 2017

Regarding the VMSwitch handling, unless this could be updated to handle mutli-homed scenarios, it would better to leave it as is or update the current network processes to handle mutliple adapters/switches.

User case, setting up a Router VM to bridge two networks (local host private and lab for example) to create an isolated environment that "borrows" internet access from another network.

Also, it's not at all related to the admin verification, so should be on it's own.

@mwrock
Copy link
Contributor

mwrock commented May 1, 2017

It should at least be implemented to not break current Vagrantfiles that use the bridge to specify a switch. For clarity I think having a separate PR is for the best.

@chrisroberts
Copy link
Member

Extracted the admin check to isolate change in #8548. Thank you for the PR!

@johnrizzo1
Copy link
Contributor Author

johnrizzo1 commented May 2, 2017 via email

@ghost
Copy link

ghost commented Apr 2, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants