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

FYI #20

Open
kevinburke opened this issue Aug 25, 2017 · 12 comments
Open

FYI #20

kevinburke opened this issue Aug 25, 2017 · 12 comments

Comments

@kevinburke
Copy link

kevinburke commented Aug 25, 2017

Go 1.9 ships with os/user.LookupId, os/user.Lookup, os/user.LookupGroup, os/user.LookupGroupId that work without cgo. If you compile without cgo enabled, those functions will read /etc/passwd and /etc/group respectively to find answers to the questions you ask. Which may be good enough for your use case.

(I wrote the functions in question, and I'm curious how well they work for folks. I would appreciate feedback on how well they work or don't. If they don't work, I'd love to help fix them. Thanks!)

@Ilyes512
Copy link

Would be nice if we could create tests for both to see if there would be situations that os/user.Lookup* would return something else.

@joonas-fi
Copy link

@kevinburke: for some reason I got user: Current not implemented on linux/amd64 (compiled using go version go1.11.1 linux/amd64). I'm running inside a container, if that makes a difference.

Here's my code:

usr, err := user.Current()
if err != nil {
	panic(err)
}

fmt.Printf("homedir = %s\n", usr.HomeDir)

This /etc/passwd parsing is total overkill, since all I want is a cross-platform way to resolve user's homedir via the ENV variables. That's why I'm having to resort into using this external dependency.

@Ilyes512
Copy link

Ilyes512 commented Dec 1, 2018

@joonas-fi Are you using FROM scratch perhaps? Do you got the Dockerfile of the image available somewhere to see?

@kevinburke
Copy link
Author

kevinburke commented Dec 1, 2018

here's the logic in go 1.11 that leads to an error

	u := &User{
		Uid:      currentUID(),
		Gid:      currentGID(),
		Username: os.Getenv("USER"),
		Name:     "", // ignored
		HomeDir:  os.Getenv("HOME"),
	}
	// On NaCL and Android, return a dummy user instead of failing.
	switch runtime.GOOS {
	case "nacl":
		if u.Uid == "" {
			u.Uid = "1"
		}
		if u.Username == "" {
			u.Username = "nacl"
		}
		if u.HomeDir == "" {
			u.HomeDir = "/"
		}
	case "android":
		if u.Uid == "" {
			u.Uid = "1"
		}
		if u.Username == "" {
			u.Username = "android"
		}
		if u.HomeDir == "" {
			u.HomeDir = "/sdcard"
		}
	}
	// cgo isn't available, but if we found the minimum information
	// without it, use it:
	if u.Uid != "" && u.Username != "" && u.HomeDir != "" {
		return u, nil
	}
	return u, fmt.Errorf("user: Current not implemented on %s/%s", runtime.GOOS, runtime.GOARCH)

note there is a patch to this logic incoming in go1.12, the patch is here: golang/go@3a18f0ecb57

the logic in go 1.12 will look like:

	uid := currentUID()
	// $USER and /etc/passwd may disagree; prefer the latter if we can get it.
	// See issue 27524 for more information.
	u, err := lookupUserId(uid)
	if err == nil {
		return u, nil
	}
	u = &User{
		Uid:      uid,
		Gid:      currentGID(),
		Username: os.Getenv("USER"),
		Name:     "", // ignored
		HomeDir:  os.Getenv("HOME"),
	}
	// On NaCL and Android, return a dummy user instead of failing.
	switch runtime.GOOS {
	case "nacl":
		if u.Uid == "" {
			u.Uid = "1"
		}
		if u.Username == "" {
			u.Username = "nacl"
		}
		if u.HomeDir == "" {
			u.HomeDir = "/"
		}
	case "android":
		if u.Uid == "" {
			u.Uid = "1"
		}
		if u.Username == "" {
			u.Username = "android"
		}
		if u.HomeDir == "" {
			u.HomeDir = "/sdcard"
		}
	}
	// cgo isn't available, but if we found the minimum information
	// without it, use it:
	if u.Uid != "" && u.Username != "" && u.HomeDir != "" {
		return u, nil
	}
	return u, fmt.Errorf("user: Current not implemented on %s/%s", runtime.GOOS, runtime.GOARCH)

@joonas-fi
Copy link

joonas-fi commented Dec 1, 2018

@Ilyes512 unfortunately my image is not public, but my image's first public parent is FROM golang:1.11.1

@kevinburke thanks for responding so quickly, and with the code. The issue seems to be here:

$ whoami
root
$ env | grep USER
$ env | grep HOME
HOME=/root

The code you shared seems to require that USER env is defined. In my case it is not defined. I am not sure if this is common behaviour inside Docker containers.

The upcoming patch doesn't seem to mitigate this exact case.

EDIT: Here's a repro:

$ docker run --rm -it golang:1.11.1 bash
root@b2434c121eba:/go# whoami
root
root@b2434c121eba:/go# env | egrep 'USER|HOME'
HOME=/root
$ 

@leonklingele
Copy link

Go 1.12 supports os.UserHomeDir().

@mitchellh
Copy link
Owner

I noted this somewhere else but can't find it... but the Go stdlib implementation is quite simplistic. It only checks env vars. This will work in most cases (90+%?) so if it works for you that's great and you can remove a dep!

In the nearly decade (!!!) we've had some of our software like Vagrant, there has been enough situations where that is not enough so we have extra logic here in go-homedir to try other ways to detect the home directory. We haven't had a bug reported in home directory detection in a long time...

So if you want something more robust then keep using this lib, otherwise you can use the stdlib!

@leonklingele
Copy link

Correct me if I'm mistaken, but having an HOME env var is required for POSIX compliance. If it is unset for whatever reason, that is an issue which needs to be fixed.
@mitchellh can you point to a specific case where the HOME env var is missing?

@mitchellh
Copy link
Owner

No, its non-standard. Just the realities of delivering software to many people... you hit some truly “wtf” environments.

@jhulten
Copy link

jhulten commented Jul 18, 2019

The bit I hit just now, @mitchellh, is the difference between %HOME% and %USERPROFILE% on windows. AWS appears to specifically use %USERPROFILE% which is fine in most cases, but if %HOME% is defined as well, go-homedir prefers it.

@mitchellh
Copy link
Owner

Hey @jhulten. Its been long enough that I don't have full context here, but if there is a well established practice that USERPROFILE is preferred, then we should/can switch to that as a preference for Windosws.

@jhulten
Copy link

jhulten commented Jul 18, 2019

Probably would be a breaking change for a lot of people, but the ability to provide lookup order instead of it being hardcoded would be a plus.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants