Skip to content

Commit

Permalink
Issue crc-org#570 Print a reason why root access is needed before usi…
Browse files Browse the repository at this point in the history
…ng it

The RunWithPrivilege func now takes a string as its first argument
which is a 'reason' why root access is needed, this change is made
so that users are not  scared by seeing unreasonable prompt asking
them to enter their password by sudo.

In windows the ExecuteAsAdmin and ShellExecuteAsAdmin funcs take a
first argument which is the reason why it needs to run as admin.

Since the crc setup o/p  is slightly changed, the integration test
are updated to match the new o/p.
  • Loading branch information
anjannath committed Sep 13, 2019
1 parent 7901366 commit 6793033
Show file tree
Hide file tree
Showing 10 changed files with 86 additions and 37 deletions.
16 changes: 8 additions & 8 deletions pkg/crc/preflight/preflight_checks_darwin.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,15 +58,15 @@ func fixVirtualBoxInstallation() (bool, error) {
}
defer os.Remove(tempFilePath)
logging.Debug("Installing VirtualBox")
stdOut, stdErr, err := crcos.RunWithPrivilege("hdiutil", "attach", tempFilePath)
stdOut, stdErr, err := crcos.RunWithPrivilege("mount VirtualBox disk image", "hdiutil", "attach", tempFilePath)
if err != nil {
return false, fmt.Errorf("Could not mount the virtualbox.dmg file: %s %v: %s", stdOut, err, stdErr)
}
stdOut, stdErr, err = crcos.RunWithPrivilege("installer", "-package", virtualBoxPkgLocation, "-target", "/")
stdOut, stdErr, err = crcos.RunWithPrivilege("run VirtualBox installation", "installer", "-package", virtualBoxPkgLocation, "-target", "/")
if err != nil {
return false, fmt.Errorf("Could not install VirtualBox.pkg: %s %v: %s", stdOut, err, stdErr)
}
stdOut, stdErr, err = crcos.RunWithPrivilege("hdiutil", "detach", virtualBoxMountLocation)
stdOut, stdErr, err = crcos.RunWithPrivilege("unmount VirtualBox disk image", "hdiutil", "detach", virtualBoxMountLocation)
if err != nil {
return false, fmt.Errorf("Could not install VirtualBox.pkg: %s %v: %s", stdOut, err, stdErr)
}
Expand Down Expand Up @@ -124,14 +124,14 @@ func download(url string, destDir string, mode os.FileMode) (string, error) {
func setSuid(path string) error {
logging.Debugf("Making %s suid", path)

stdOut, stdErr, err := crcos.RunWithPrivilege("chown", "root:wheel", path)
stdOut, stdErr, err := crcos.RunWithPrivilege(fmt.Sprintf("change ownership of %s", path), "chown", "root:wheel", path)
if err != nil {
return fmt.Errorf("Unable to set ownership of %s to root:wheel: %s %v: %s",
path, stdOut, err, stdErr)
}

/* Can't do this before the chown as the chown will reset the suid bit */
stdOut, stdErr, err = crcos.RunWithPrivilege("chmod", "u+s", path)
stdOut, stdErr, err = crcos.RunWithPrivilege(fmt.Sprintf("set suid for %s", path), "chmod", "u+s", path)
if err != nil {
return fmt.Errorf("Unable to set suid bit on %s: %s %v: %s", path, stdOut, err, stdErr)
}
Expand Down Expand Up @@ -222,13 +222,13 @@ func fixResolverFilePermissions() (bool, error) {
// Check if resolver directory available or not
if _, err := os.Stat(resolverDir); os.IsNotExist(err) {
logging.Debugf("Creating %s directory", resolverDir)
stdOut, stdErr, err := crcos.RunWithPrivilege("mkdir", resolverDir)
stdOut, stdErr, err := crcos.RunWithPrivilege(fmt.Sprintf("create dir %s", resolverDir), "mkdir", resolverDir)
if err != nil {
return false, fmt.Errorf("Unable to create the resolver Dir: %s %v: %s", stdOut, err, stdErr)
}
}
logging.Debugf("Making %s readable/writable by the current user", resolverFile)
stdOut, stdErr, err := crcos.RunWithPrivilege("touch", resolverFile)
stdOut, stdErr, err := crcos.RunWithPrivilege(fmt.Sprintf("create file %s", resolverFile), "touch", resolverFile)
if err != nil {
return false, fmt.Errorf("Unable to create the resolver file: %s %v: %s", stdOut, err, stdErr)
}
Expand Down Expand Up @@ -259,7 +259,7 @@ func addFileWritePermissionToUser(filename string) (bool, error) {
return false, err
}

stdOut, stdErr, err := crcos.RunWithPrivilege("chown", currentUser.Username, filename)
stdOut, stdErr, err := crcos.RunWithPrivilege(fmt.Sprintf("change ownership of %s", filename), "chown", currentUser.Username, filename)
if err != nil {
return false, fmt.Errorf("Unable to change ownership of the filename: %s %v: %s", stdOut, err, stdErr)
}
Expand Down
36 changes: 19 additions & 17 deletions pkg/crc/preflight/preflight_checks_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ func checkLibvirtInstalled() (bool, error) {

func fixLibvirtInstalled() (bool, error) {
logging.Debug("Trying to install libvirt")
stdOut, stdErr, err := crcos.RunWithPrivilege("yum", "install", "-y", "libvirt", "libvirt-daemon-kvm", "qemu-kvm")
stdOut, stdErr, err := crcos.RunWithPrivilege("install virtualization related packages", "yum", "install", "-y", "libvirt", "libvirt-daemon-kvm", "qemu-kvm")
if err != nil {
return false, fmt.Errorf("Could not install required packages: %s %v: %s", stdOut, err, stdErr)
}
Expand Down Expand Up @@ -138,7 +138,7 @@ func fixLibvirtEnabled() (bool, error) {
if err != nil {
return false, err
}
stdOut, stdErr, err := crcos.RunWithPrivilege(path, "enable", "libvirtd")
stdOut, stdErr, err := crcos.RunWithPrivilege("enable libvirtd service", path, "enable", "libvirtd")
if err != nil {
return false, fmt.Errorf("%s, %v : %s", stdOut, err, stdErr)
}
Expand Down Expand Up @@ -175,7 +175,7 @@ func fixUserPartOfLibvirtGroup() (bool, error) {
if err != nil {
return false, err
}
stdOut, stdErr, err := crcos.RunWithPrivilege("usermod", "-a", "-G", "libvirt", currentUser.Username)
stdOut, stdErr, err := crcos.RunWithPrivilege("add user to libvirt group", "usermod", "-a", "-G", "libvirt", currentUser.Username)
if err != nil {
return false, fmt.Errorf("%s %v : %s", stdOut, err, stdErr)
}
Expand Down Expand Up @@ -206,7 +206,7 @@ func fixLibvirtServiceRunning() (bool, error) {
if err != nil {
return false, err
}
stdOut, stdErr, err := crcos.RunWithPrivilege(path, "start", "libvirtd")
stdOut, stdErr, err := crcos.RunWithPrivilege("start libvirtd service", path, "start", "libvirtd")
if err != nil {
return false, fmt.Errorf("%s %v : %s", stdOut, err, stdErr)
}
Expand Down Expand Up @@ -263,7 +263,7 @@ func checkOldMachineDriverLibvirtInstalled() (bool, error) {
func fixOldMachineDriverLibvirtInstalled() (bool, error) {
oldLibvirtDriverPath := filepath.Join("/usr/local/bin/", libvirtDriverCommand)
logging.Debugf("Removing %s", oldLibvirtDriverPath)
_, _, err := crcos.RunWithPrivilege("rm", "-f", oldLibvirtDriverPath)
_, _, err := crcos.RunWithPrivilege("remove old libvirt driver", "rm", "-f", oldLibvirtDriverPath)
if err != nil {
logging.Debugf("Removal of %s failed", oldLibvirtDriverPath)
/* Ignoring error, an obsolete file being still present is not a fatal error */
Expand Down Expand Up @@ -394,12 +394,13 @@ func checkCrcDnsmasqConfigFile() (bool, error) {

func fixCrcDnsmasqConfigFile() (bool, error) {
logging.Debug("Fixing dnsmasq configuration")
cmd := exec.Command("sudo", "tee", crcDnsmasqConfigPath)
cmd.Stdin = strings.NewReader(crcDnsmasqConfig)
buf := new(bytes.Buffer)
cmd.Stderr = buf
if err := cmd.Run(); err != nil {
return false, fmt.Errorf("Failed to write dnsmasq config file: %s: %s: %v", crcDnsmasqConfigPath, buf.String(), err)
err := crcos.WriteToFileAsRoot(
fmt.Sprintf("write dnsmasq configuration in %s", crcDnsmasqConfigPath),
crcDnsmasqConfig,
crcDnsmasqConfigPath,
)
if err != nil {
return false, fmt.Errorf("Failed to write dnsmasq config file: %s: %v", crcDnsmasqConfigPath, err)
}

logging.Debug("Reloading NetworkManager")
Expand Down Expand Up @@ -430,12 +431,13 @@ func checkCrcNetworkManagerConfig() (bool, error) {
}
func fixCrcNetworkManagerConfig() (bool, error) {
logging.Debug("Fixing NetworkManager configuration")
cmd := exec.Command("sudo", "tee", crcNetworkManagerConfigPath)
cmd.Stdin = strings.NewReader(crcNetworkManagerConfig)
buf := new(bytes.Buffer)
cmd.Stderr = buf
if err := cmd.Run(); err != nil {
return false, fmt.Errorf("Failed to write NetworkManager config file: %s: %s: %v", crcNetworkManagerConfigPath, buf.String(), err)
err := crcos.WriteToFileAsRoot(
fmt.Sprintf("write NetworkManager config in %s", crcNetworkManagerConfigPath),
crcNetworkManagerConfig,
crcNetworkManagerConfigPath,
)
if err != nil {
return false, fmt.Errorf("Failed to write NetworkManager config file: %s: %v", crcNetworkManagerConfigPath, err)
}

logging.Debug("Reloading NetworkManager")
Expand Down
4 changes: 2 additions & 2 deletions pkg/crc/preflight/preflight_checks_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func checkHyperVInstalled() (bool, error) {
//
func fixHyperVInstalled() (bool, error) {
enableHyperVCommand := `Enable-WindowsOptionalFeature -Online -FeatureName Microsoft-Hyper-V -All`
_, _, err := powershell.ExecuteAsAdmin(enableHyperVCommand)
_, _, err := powershell.ExecuteAsAdmin("enable Hyper-V", enableHyperVCommand)

if err != nil {
return false, errors.New("Error occured installing Hyper-V")
Expand Down Expand Up @@ -96,7 +96,7 @@ func fixUserPartOfHyperVAdmins() (bool, error) {
username := os.Getenv("USERNAME")

netCmdArgs := fmt.Sprintf(`([adsi]"WinNT://./%s,group").Add("WinNT://%s,user")`, groupName, username)
_, _, err = powershell.ExecuteAsAdmin(netCmdArgs)
_, _, err = powershell.ExecuteAsAdmin("add user to hyperv admins group", netCmdArgs)
if err != nil {
return false, errors.New("Unable to get user name")
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/crc/services/dns/dns_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func setInterfaceNameserverValue(iface string, address string) {
exe := "netsh"
args := fmt.Sprintf(`interface ip set dns "%s" static %s primary`, iface, address)

win32.ShellExecuteAsAdmin(win32.HWND_DESKTOP, exe, args, "", 0)
win32.ShellExecuteAsAdmin(fmt.Sprintf("add dns server address to interface %s", iface), win32.HWND_DESKTOP, exe, args, "", 0)
}

func getMainInterface() string {
Expand Down
4 changes: 2 additions & 2 deletions pkg/crc/systemd/host.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func (c HostSystemdCommander) Disable(name string) (bool, error) {
}

func (c HostSystemdCommander) DaemonReload() (bool, error) {
stdOut, stdErr, err := crcos.RunWithPrivilege("systemctl", "daemon-reload")
stdOut, stdErr, err := crcos.RunWithPrivilege("execute systemctl daemon-reload command", "systemctl", "daemon-reload")
if err != nil {
return false, fmt.Errorf("Executing systemctl daemon-reload failed: %s %v: %s", stdOut, err, stdErr)
}
Expand Down Expand Up @@ -83,7 +83,7 @@ func (c HostSystemdCommander) Status(name string) (states.State, error) {
}

func (c HostSystemdCommander) service(name string, action actions.Action) (states.State, error) {
stdOut, stdErr, err := crcos.RunWithPrivilege("systemctl", action.String(), name)
stdOut, stdErr, err := crcos.RunWithPrivilege("execute systemctl stop/start command", "systemctl", action.String(), name)
if err != nil {
return states.Error, fmt.Errorf("Executing systemctl action failed: %s %v: %s", stdOut, err, stdErr)
}
Expand Down
6 changes: 5 additions & 1 deletion pkg/os/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,14 @@ package os

import (
"bytes"
"github.com/code-ready/crc/pkg/crc/logging"
"os"
"os/exec"
)

func RunWithPrivilege(cmdAndArgs ...string) (string, string, error) {
// RunWithPrivilege executes a command using sudo
// provide a reason why root is needed as the first argument
func RunWithPrivilege(reason string, cmdAndArgs ...string) (string, string, error) {
sudo, err := exec.LookPath("sudo")
if err != nil {
return "", "", err
Expand All @@ -16,6 +19,7 @@ func RunWithPrivilege(cmdAndArgs ...string) (string, string, error) {
stdErr := new(bytes.Buffer)
cmd.Stdout = stdOut
cmd.Stderr = stdErr
logging.Infof("Will use root access: %s", reason)
err = cmd.Run()
return stdOut.String(), stdErr.String(), err
}
Expand Down
22 changes: 22 additions & 0 deletions pkg/os/util_linux.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package os

import (
"bytes"
"fmt"
"os/exec"
"strings"

"github.com/code-ready/crc/pkg/crc/logging"
)

func WriteToFileAsRoot(reason, content, filepath string) error {
logging.Infof("Will use root: %s", reason)
cmd := exec.Command("sudo", "tee", filepath)
cmd.Stdin = strings.NewReader(content)
buf := new(bytes.Buffer)
cmd.Stderr = buf
if err := cmd.Run(); err != nil {
return fmt.Errorf("Failed writing to file as root: %s: %s: %v", filepath, buf.String(), err)
}
return nil
}
5 changes: 4 additions & 1 deletion pkg/os/windows/powershell/powershell_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
"syscall"

"os/exec"

"github.com/code-ready/crc/pkg/crc/logging"
)

var (
Expand Down Expand Up @@ -66,7 +68,7 @@ func Execute(args ...string) (stdOut string, stdErr string, err error) {
return
}

func ExecuteAsAdmin(cmd string) (stdOut string, stdErr string, err error) {
func ExecuteAsAdmin(reason, cmd string) (stdOut string, stdErr string, err error) {
scriptContent := strings.Join(append(runAsCmds, cmd), "\n")

tempDir, _ := ioutil.TempDir("", "crcScripts")
Expand All @@ -88,6 +90,7 @@ func ExecuteAsAdmin(cmd string) (stdOut string, stdErr string, err error) {
return "", "", err
}
psFile.Close()
logging.Infof("Will run as admin: %s", reason)

return Execute(psFile.Name())

Expand Down
4 changes: 3 additions & 1 deletion pkg/os/windows/win32/shell32_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package win32
import (
"errors"
"fmt"
"github.com/code-ready/crc/pkg/crc/logging"
"syscall"
"unsafe"
)
Expand All @@ -13,7 +14,8 @@ var (
)

// Uses "runas" as verb to execute as Elevated privileges
func ShellExecuteAsAdmin(hwnd HWND, file, parameters, directory string, showCmd int) error {
func ShellExecuteAsAdmin(reason string, hwnd HWND, file, parameters, directory string, showCmd int) error {
logging.Infof("Will run as admin: %s", reason)
return ShellExecute(hwnd, "runas", file, parameters, directory, showCmd)
}

Expand Down
24 changes: 20 additions & 4 deletions test/integration/features/basic.feature
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,33 @@ Feature: Basic test
@linux
Scenario: CRC setup on Linux
When executing "crc setup" succeeds
Then stdout should contain "Caching oc binary"
And stdout should contain "Starting libvirt 'crc' network"
And stdout should contain "Setting up virtualization"
Then stdout should contain "Checking if running as root"
And stdout should contain "Caching oc binary"
And stdout should contain "Setting up virtualization"
And stdout should contain "Setting up KVM"
And stdout should contain "Installing libvirt"
And stdout should contain "Installing libvirt service and dependencies"
And stdout should contain "Adding user to libvirt group"
And stdout should contain "Will use root access: add user to libvirt group"
And stdout should contain "Enabling libvirt"
And stdout should contain "Starting libvirt service"
And stdout should contain "Will use root access: start libvirtd service"
And stdout should contain "Installing crc-driver-libvirt"
And stdout should contain "Removing older system-wide crc-driver-libvirt"
And stdout should contain "Setting up libvirt 'crc' network"
And stdout should contain "Starting libvirt 'crc' network"
And stdout should contain "Checking if NetworkManager is installed"
And stdout should contain "Checking if NetworkManager service is running"
And stdout should contain "Writing Network Manager config for crc"
And stdout should contain "Will use root access: write NetworkManager config in /etc/NetworkManager/conf.d/crc-nm-dnsmasq.conf"
And stdout should contain "Will use root access: execute systemctl daemon-reload command"
And stdout should contain "Will use root access: execute systemctl stop/start command"
And stdout should contain "Writing dnsmasq config for crc"
And stdout should contain "Will use root access: write dnsmasq configuration in /etc/NetworkManager/dnsmasq.d/crc.conf"
And stdout should contain "Will use root access: execute systemctl daemon-reload command"
And stdout should contain "Will use root access: execute systemctl stop/start command"
And stdout should contain "Unpacking bundle from the CRC binary"
And stdout should contain "CRC bundle is not embedded in the binary"
And stdout should contain "Setup is complete, you can now run 'crc start -b $bundlename' to start a CodeReady Containers instance"

@darwin
Scenario: CRC setup on Mac
Expand Down

0 comments on commit 6793033

Please sign in to comment.