Tweak jaymells nice work to make add-user work on classic

daemon/api.go

@@ -1423,7 +1423,7 @@ func abortChange(c *Command, r *http.Request, user *auth.UserState) Response {
 var (
 	postCreateUserUcrednetGetUID = ucrednetGetUID
 	storeUserInfo                = store.UserInfo
-	osutilAddExtraUser           = osutil.AddExtraUser
+	osutilAddUser                = osutil.AddUser
 )
 
 func postCreateUser(c *Command, r *http.Request, user *auth.UserState) Response {
@@ -1458,7 +1458,13 @@ func postCreateUser(c *Command, r *http.Request, user *auth.UserState) Response
 	}
 
 	gecos := fmt.Sprintf("%s,%s", createData.Email, v.OpenIDIdentifier)
-	if err := osutilAddExtraUser(v.Username, v.SSHKeys, gecos, createData.Sudoer); err != nil {
+	opts := &osutil.AddUserOptions{
+		SshKeys:       v.SSHKeys,
+		Gecos:         gecos,
+		Sudoer:        createData.Sudoer,
+		UseExtraUsers: !release.OnClassic,
+	}
+	if err := osutilAddUser(v.Username, opts); err != nil {
 		return BadRequest("cannot create user %s: %s", v.Username, err)
 	}
 

daemon/api_test.go

@@ -389,7 +389,7 @@ func (s *apiSuite) TestListIncludesAll(c *check.C) {
 		"snapstateTryPath",
 		"snapstateGet",
 		"readSnapInfo",
-		"osutilAddExtraSudoUser",
+		"osutilAddUser",
 		"storeUserInfo",
 		"postCreateUserUcrednetGetUID",
 		"ensureStateSoon",
@@ -3111,19 +3111,19 @@ func (s *apiSuite) TestPostCreateUser(c *check.C) {
 			OpenIDIdentifier: "xxyyzz",
 		}, nil
 	}
-	osutilAddExtraUser = func(username string, sshKeys []string, gecos string, sudoer bool) error {
+	osutilAddUser = func(username string, opts *osutil.AddUserOptions) error {
 		c.Check(username, check.Equals, "karl")
-		c.Check(sshKeys, check.DeepEquals, []string{"ssh1", "ssh2"})
-		c.Check(gecos, check.Equals, "popper@lse.ac.uk,xxyyzz")
-		c.Check(sudoer, check.Equals, false)
+		c.Check(opts.SshKeys, check.DeepEquals, []string{"ssh1", "ssh2"})
+		c.Check(opts.Gecos, check.Equals, "popper@lse.ac.uk,xxyyzz")
+		c.Check(opts.Sudoer, check.Equals, false)
 		return nil
 	}
 
 	postCreateUserUcrednetGetUID = func(string) (uint32, error) {
 		return 0, nil
 	}
 	defer func() {
-		osutilAddExtraUser = osutil.AddExtraUser
+		osutilAddUser = osutil.AddUser
 		postCreateUserUcrednetGetUID = ucrednetGetUID
 	}()
 

osutil/user.go

@@ -28,8 +28,6 @@ import (
 	"regexp"
 	"strconv"
 	"strings"
-
-	"github.com/snapcore/snapd/release"
 )
 
 var userLookup = user.Lookup
@@ -43,7 +41,18 @@ var sudoersTemplate = `
 %[1]s ALL=(ALL) NOPASSWD:ALL
 `
 
-func AddExtraUser(name string, sshKeys []string, gecos string, sudoer bool) error {
+type AddUserOptions struct {
+	Sudoer        bool
+	UseExtraUsers bool
+	Gecos         string
+	SshKeys       []string
+}
+
+func AddUser(name string, opts *AddUserOptions) error {
+	if opts == nil {
+		opts = &AddUserOptions{}
+	}
+
 	// we check the (user)name ourselves, adduser is a bit too
 	// strict (i.e. no `.`) - this regexp is in sync with that SSO
 	// allows as valid usernames
@@ -52,27 +61,23 @@ func AddExtraUser(name string, sshKeys []string, gecos string, sudoer bool) erro
 		return fmt.Errorf("cannot add user %q: name contains invalid characters", name)
 	}
 
-	var cmd *exec.Cmd
-
-	if release.OnClassic {
-		cmd = exec.Command("adduser",
-			"--force-badname",
-			"--gecos", gecos,
-			"--disabled-password",
-			name)
-	} else {
-		cmd = exec.Command("adduser",
-			"--force-badname",
-			"--gecos", gecos,
-			"--extrausers",
-			"--disabled-password",
-			name)
+	cmdStr := []string{
+		"adduser",
+		"--force-badname",
+		"--gecos", opts.Gecos,
+		"--disabled-password",
+	}
+	if opts.UseExtraUsers {
+		cmdStr = append(cmdStr, "--extrausers")
 	}
+	cmdStr = append(cmdStr, name)
+
+	cmd := exec.Command(cmdStr[0], cmdStr[1:]...)
 	if output, err := cmd.CombinedOutput(); err != nil {
 		return fmt.Errorf("adduser failed with %s: %s", err, output)
 	}
 
-	if sudoer {
+	if opts.Sudoer {
 		// Must escape "." as files containing it are ignored in sudoers.d.
 		sudoersFile := filepath.Join(sudoersDotD, "create-user-"+strings.Replace(name, ".", "%2E", -1))
 		if err := AtomicWriteFile(sudoersFile, []byte(fmt.Sprintf(sudoersTemplate, name)), 0400, 0); err != nil {
@@ -99,7 +104,7 @@ func AddExtraUser(name string, sshKeys []string, gecos string, sudoer bool) erro
 		return fmt.Errorf("cannot create %s: %s", sshDir, err)
 	}
 	authKeys := filepath.Join(sshDir, "authorized_keys")
-	authKeysContent := strings.Join(sshKeys, "\n")
+	authKeysContent := strings.Join(opts.SshKeys, "\n")
 	if err := AtomicWriteFileChown(authKeys, []byte(authKeysContent), 0600, 0, uid, gid); err != nil {
 		return fmt.Errorf("cannot write %s: %s", authKeys, err)
 	}

osutil/user_test.go

@@ -27,7 +27,6 @@ import (
 	"gopkg.in/check.v1"
 
 	"github.com/snapcore/snapd/osutil"
-	"github.com/snapcore/snapd/release"
 	"github.com/snapcore/snapd/testutil"
 )
 
@@ -37,7 +36,7 @@ type createUserSuite struct {
 
 var _ = check.Suite(&createUserSuite{})
 
-func (s *createUserSuite) TestAddExtraUserOnClassic(c *check.C) {
+func (s *createUserSuite) TestAddUserOnClassic(c *check.C) {
 	mockHome := c.MkDir()
 	restorer := osutil.MockUserLookup(func(string) (*user.User, error) {
 		current, err := user.Current()
@@ -55,10 +54,12 @@ func (s *createUserSuite) TestAddExtraUserOnClassic(c *check.C) {
 	mockAddUser := testutil.MockCommand(c, "adduser", "true")
 	defer mockAddUser.Restore()
 
-	restore := release.MockOnClassic(true)
-	defer restore()
-
-	err := osutil.AddExtraUser("karl.sagan", []string{"ssh-key1", "ssh-key2"}, "my gecos", false)
+	err := osutil.AddUser("karl.sagan", &osutil.AddUserOptions{
+		SshKeys:       []string{"ssh-key1", "ssh-key2"},
+		Gecos:         "my gecos",
+		Sudoer:        false,
+		UseExtraUsers: false,
+	})
 	c.Assert(err, check.IsNil)
 
 	c.Check(mockAddUser.Calls(), check.DeepEquals, [][]string{
@@ -70,7 +71,7 @@ func (s *createUserSuite) TestAddExtraUserOnClassic(c *check.C) {
 	c.Check(string(sshKeys), check.Equals, "ssh-key1\nssh-key2")
 }
 
-func (s *createUserSuite) TestAddExtraSudoUser(c *check.C) {
+func (s *createUserSuite) TestAddSudoUser(c *check.C) {
 	mockHome := c.MkDir()
 	restorer := osutil.MockUserLookup(func(string) (*user.User, error) {
 		current, err := user.Current()
@@ -91,25 +92,32 @@ func (s *createUserSuite) TestAddExtraSudoUser(c *check.C) {
 	mockAddUser := testutil.MockCommand(c, "adduser", "true")
 	defer mockAddUser.Restore()
 
-	restore := release.MockOnClassic(false)
-	defer restore()
-
-	err := osutil.AddExtraUser("karl.sagan", []string{"ssh-key1", "ssh-key2"}, "my gecos", false)
+	err := osutil.AddUser("karl.sagan", &osutil.AddUserOptions{
+		SshKeys:       []string{"ssh-key1", "ssh-key2"},
+		Gecos:         "my gecos",
+		Sudoer:        false,
+		UseExtraUsers: true,
+	})
 	c.Assert(err, check.IsNil)
 
 	c.Check(mockAddUser.Calls(), check.DeepEquals, [][]string{
-		{"adduser", "--force-badname", "--gecos", "my gecos", "--extrausers", "--disabled-password", "karl.sagan"},
+		{"adduser", "--force-badname", "--gecos", "my gecos", "--disabled-password", "--extrausers", "karl.sagan"},
 	})
 	fs, _ := filepath.Glob(filepath.Join(mockSudoers, "*"))
 	c.Assert(fs, check.HasLen, 0)
 
 	mockAddUser.ForgetCalls()
 
-	err = osutil.AddExtraUser("karl.sagan", []string{"ssh-key1", "ssh-key2"}, "my gecos", true)
+	err = osutil.AddUser("karl.sagan", &osutil.AddUserOptions{
+		SshKeys:       []string{"ssh-key1", "ssh-key2"},
+		Gecos:         "my gecos",
+		Sudoer:        true,
+		UseExtraUsers: true,
+	})
 	c.Assert(err, check.IsNil)
 
 	c.Check(mockAddUser.Calls(), check.DeepEquals, [][]string{
-		{"adduser", "--force-badname", "--gecos", "my gecos", "--extrausers", "--disabled-password", "karl.sagan"},
+		{"adduser", "--force-badname", "--gecos", "my gecos", "--disabled-password", "--extrausers", "karl.sagan"},
 	})
 
 	sshKeys, err := ioutil.ReadFile(filepath.Join(mockHome, ".ssh", "authorized_keys"))
@@ -129,7 +137,7 @@ karl.sagan ALL=(ALL) NOPASSWD:ALL
 `)
 }
 
-func (s *createUserSuite) TestAddExtraSudoUserInvalid(c *check.C) {
-	err := osutil.AddExtraUser("k!", nil, "my gecos", false)
+func (s *createUserSuite) TestAddSudoUserInvalid(c *check.C) {
+	err := osutil.AddUser("k!", nil)
 	c.Assert(err, check.ErrorMatches, `cannot add user "k!": name contains invalid characters`)
 }

release/release.go

@@ -24,6 +24,8 @@ import (
 	"os"
 	"strings"
 	"unicode"
+
+	"github.com/snapcore/snapd/osutil"
 )
 
 // Series holds the Ubuntu Core series for snapd to use.
@@ -127,9 +129,7 @@ func init() {
 	// dpkg status file can be used as an indicator for a classic vs all-snap
 	// system.
 	if ReleaseInfo.ID == "ubuntu" {
-		// using os.Stat directly to avoid import cycle with osutil
-		_, err := os.Stat("/var/lib/dpkg/status")
-		OnClassic = err == nil
+		OnClassic = osutil.FileExists("/var/lib/dpkg/status")
 	}
 }