Add user to crc-users and hyperv admins group in a single powershell admin call

this is done to avoid getting two UAC prompts during crc setup command

by combining both in a single call to powershell admin session we are losing
inidividual error messages to determine which action failed but in this case
its an acceptable trade-off, since the two actions are doing similar thing of
adding current member to a group, so its easy to narrow down the issue

Author: anjannath

pkg/crc/preflight/preflight_checks_windows.go

@@ -99,40 +99,21 @@ func checkHyperVServiceRunning() error {
 	return nil
 }
 
-func checkIfUserPartOfHyperVAdmins() error {
-	// https://support.microsoft.com/en-us/help/243330/well-known-security-identifiers-in-windows-operating-systems
-	// BUILTIN\Hyper-V Administrators => S-1-5-32-578
-
-	checkIfMemberOfHyperVAdmins :=
-		`$sid = New-Object System.Security.Principal.SecurityIdentifier("S-1-5-32-578")
-	@([Security.Principal.WindowsPrincipal][Security.Principal.WindowsIdentity]::GetCurrent()).IsInRole($sid)`
-	stdOut, _, err := powershell.Execute(checkIfMemberOfHyperVAdmins)
-	if err != nil {
-		logging.Debug(err.Error())
-		return fmt.Errorf("Failed checking if user is part of hyperv admins group")
-	}
-	if !strings.Contains(stdOut, "True") {
-		return fmt.Errorf("User is not a member of the Hyper-V administrators group")
-	}
-
-	return nil
-}
-
-func fixUserPartOfHyperVAdmins() error {
-	_, _, err := powershell.ExecuteAsAdmin("adding current user to Hyper-V administrator group", fmt.Sprintf("Add-LocalGroupMember -SID 'S-1-5-32-578' -Member '%s'", username()))
+func checkUserPartOfCrcUsersAndHypervAdminsGroup() error {
+	_, _, err := powershell.Execute(fmt.Sprintf("Get-LocalGroupMember -Group 'crc-users' -Member '%s'", username()))
 	if err != nil {
 		return err
 	}
-	return errReboot
-}
 
-func checkUserPartOfCrcUsers() error {
-	_, _, err := powershell.Execute(fmt.Sprintf("Get-LocalGroupMember -Group 'crc-users' -Member '%s'", username()))
+	// https://support.microsoft.com/en-us/help/243330/well-known-security-identifiers-in-windows-operating-systems
+	// BUILTIN\Hyper-V Administrators => S-1-5-32-578
+	_, _, err = powershell.Execute(fmt.Sprintf("Get-LocalGroupMember -SID 'S-1-5-32-578' -Member '%s'", username()))
 	return err
 }
 
-func fixUserPartOfCrcUsers() error {
-	_, _, err := powershell.ExecuteAsAdmin("adding current user to crc-users group", fmt.Sprintf("Add-LocalGroupMember -Group 'crc-users' -Member '%s'", username()))
+func fixUserPartOfCrcUsersAndHypervAdminsGroup() error {
+	cmd := `Add-LocalGroupMember -Group 'crc-users' -Member '%s';Add-LocalGroupMember -SID 'S-1-5-32-578' -Member '%s'`
+	_, _, err := powershell.ExecuteAsAdmin("adding current user to crc-users and Hyper-V admins group", fmt.Sprintf(cmd, username(), username()))
 	if err != nil {
 		return err
 	}

pkg/crc/preflight/preflight_windows.go

@@ -47,28 +47,6 @@ var hypervPreflightChecks = []Check{
 
 		labels: labels{Os: Windows},
 	},
-	{
-		configKeySuffix:  "check-crc-users-group-exists",
-		checkDescription: "Checking if crc-users group exists",
-		check: func() error {
-			if _, _, err := powershell.Execute("Get-LocalGroup -Name crc-users"); err != nil {
-				return fmt.Errorf("'crc-users' group does not exist: %v", err)
-			}
-			return nil
-		},
-		flags: StartUpOnly,
-
-		labels: labels{Os: Windows},
-	},
-	{
-		configKeySuffix:  "check-user-in-hyperv-group",
-		checkDescription: "Checking if current user is in Hyper-V Admins group",
-		check:            checkIfUserPartOfHyperVAdmins,
-		fixDescription:   "Adding current user to Hyper-V Admins group",
-		fix:              fixUserPartOfHyperVAdmins,
-
-		labels: labels{Os: Windows},
-	},
 	{
 		configKeySuffix:  "check-hyperv-service-running",
 		checkDescription: "Checking if Hyper-V service is enabled",
@@ -147,12 +125,29 @@ var adminHelperServiceCheks = []Check{
 	},
 }
 
-var userPartOfCrcUsersGroupCheck = Check{
-	configKeySuffix:  "check-user-in-crc-users-group",
-	checkDescription: "Checking if current user is in crc-users group",
-	check:            checkUserPartOfCrcUsers,
-	fixDescription:   "Adding logon user to crc-users group",
-	fix:              fixUserPartOfCrcUsers,
+// 'crc-user' group is supposed to be created by the msi or chocolatey
+// this check makes sure that was done before stating crc, it does not
+// have a fix function since this'll be handled by the msi or choco
+var crcUsersGroupExistsCheck = Check{
+	configKeySuffix:  "check-crc-users-group-exists",
+	checkDescription: "Checking if crc-users group exists",
+	check: func() error {
+		if _, _, err := powershell.Execute("Get-LocalGroup -Name crc-users"); err != nil {
+			return fmt.Errorf("'crc-users' group does not exist: %v", err)
+		}
+		return nil
+	},
+	flags: StartUpOnly,
+
+	labels: labels{Os: Windows},
+}
+
+var userPartOfCrcUsersAndHypervAdminsGroupCheck = Check{
+	configKeySuffix:  "check-user-in-crc-users-and-hyperv-admins-group",
+	checkDescription: "Checking if current user is in crc-users and Hyper-V admins group",
+	check:            checkUserPartOfCrcUsersAndHypervAdminsGroup,
+	fixDescription:   "Adding logon user to crc-users and Hyper-V admins group",
+	fix:              fixUserPartOfCrcUsersAndHypervAdminsGroup,
 
 	labels: labels{Os: Windows},
 }
@@ -199,7 +194,8 @@ func getAllPreflightChecks() []Check {
 func getChecks(bundlePath string, preset crcpreset.Preset) []Check {
 	checks := []Check{}
 	checks = append(checks, hypervPreflightChecks...)
-	checks = append(checks, userPartOfCrcUsersGroupCheck)
+	checks = append(checks, crcUsersGroupExistsCheck)
+	checks = append(checks, userPartOfCrcUsersAndHypervAdminsGroupCheck)
 	checks = append(checks, vsockChecks...)
 	checks = append(checks, bundleCheck(bundlePath, preset))
 	checks = append(checks, genericCleanupChecks...)

pkg/crc/preflight/preflight_windows_test.go

@@ -13,13 +13,13 @@ import (
 func TestCountConfigurationOptions(t *testing.T) {
 	cfg := config.New(config.NewEmptyInMemoryStorage(), config.NewEmptyInMemorySecretStorage())
 	RegisterSettings(cfg)
-	assert.Len(t, cfg.AllConfigs(), 13)
+	assert.Len(t, cfg.AllConfigs(), 12)
 }
 
 func TestCountPreflights(t *testing.T) {
-	assert.Len(t, getPreflightChecks(false, network.SystemNetworkingMode, constants.GetDefaultBundlePath(preset.OpenShift), preset.OpenShift), 19)
-	assert.Len(t, getPreflightChecks(true, network.SystemNetworkingMode, constants.GetDefaultBundlePath(preset.OpenShift), preset.OpenShift), 19)
+	assert.Len(t, getPreflightChecks(false, network.SystemNetworkingMode, constants.GetDefaultBundlePath(preset.OpenShift), preset.OpenShift), 18)
+	assert.Len(t, getPreflightChecks(true, network.SystemNetworkingMode, constants.GetDefaultBundlePath(preset.OpenShift), preset.OpenShift), 18)
 
-	assert.Len(t, getPreflightChecks(false, network.UserNetworkingMode, constants.GetDefaultBundlePath(preset.OpenShift), preset.OpenShift), 18)
-	assert.Len(t, getPreflightChecks(true, network.UserNetworkingMode, constants.GetDefaultBundlePath(preset.OpenShift), preset.OpenShift), 18)
+	assert.Len(t, getPreflightChecks(false, network.UserNetworkingMode, constants.GetDefaultBundlePath(preset.OpenShift), preset.OpenShift), 17)
+	assert.Len(t, getPreflightChecks(true, network.UserNetworkingMode, constants.GetDefaultBundlePath(preset.OpenShift), preset.OpenShift), 17)
 }