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

os/user: add users groups iteration functionality #1

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

mjonaitis1
Copy link
Owner

@mjonaitis1 mjonaitis1 commented Aug 18, 2021

Proposal link: golang#47907

Go native users/groups iteration functionality.

Go standard library has os/user package which allows to lookup user
or group records via either user/group name or id. This pull request
extends capabilities of os/user package by introducing iteration
functionality for users and groups.
Users and groups iteration functionality might be useful in cases where
a full or partial list of all available users/groups is required in an
application.

On unix, iteration functionality relies
on two internal implementations: one that does not use cgo and parses
contents of /etc/passwd and /etc/group files for users and groups
respectively, and another one that does use cgo and utilizes POSIX
compliant libc library routines getpwent and getgrent for users and
groups respectively. On windows,
HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows NT\CurrentVersion\ProfileList
registry key is iterated for SID values, each SID value represents either
a user or a group.

Tested on Debian GNU/Linux 10 (buster), macOS 11.5.1, freebsd 13.0,
plan9 fourth edition, windows 10 version 21H1.

@mjonaitis1 mjonaitis1 changed the title Add users groups iteration functionality to os/user os/user: add users groups iteration functionality Aug 18, 2021
@mjonaitis1 mjonaitis1 force-pushed the os-user-iteration branch 5 times, most recently from 589be3e to 8e513f5 Compare August 20, 2021 10:09
src/os/user/iterate_cgo.go Outdated Show resolved Hide resolved
src/os/user/iterate_cgo.go Outdated Show resolved Hide resolved
src/os/user/iterate_cgo.go Outdated Show resolved Hide resolved
src/os/user/iterate_cgo.go Outdated Show resolved Hide resolved
src/os/user/iterate_cgo_bsd_test.go Outdated Show resolved Hide resolved
src/os/user/iterate_windows.go Outdated Show resolved Hide resolved
src/os/user/iterate_windows.go Outdated Show resolved Hide resolved
src/os/user/iterate_windows.go Outdated Show resolved Hide resolved
src/os/user/listgroups_solaris.go Outdated Show resolved Hide resolved
src/os/user/read_colon_file.go Show resolved Hide resolved
@motiejus
Copy link

It would be also helpful if you listed the exact OS versions you have tested this with, and which commands did you run when testing.

For bonus points, write a small application in Go compatible to getent passwd and getent group and compare how they perform:

  • is the output identical?
  • duration.
  • memory usage.

@mjonaitis1 mjonaitis1 force-pushed the os-user-iteration branch 2 times, most recently from ca4f26b to 2806be2 Compare August 24, 2021 07:06
Copy link

@motiejus motiejus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

like

src/internal/syscall/windows/registry/registry_test.go Outdated Show resolved Hide resolved
src/os/user/lookup_unix.go Outdated Show resolved Hide resolved
src/os/user/iterate.go Outdated Show resolved Hide resolved
src/os/user/iterate_cgo.go Outdated Show resolved Hide resolved
src/os/user/iterate_cgo.go Outdated Show resolved Hide resolved
src/os/user/iterate_windows.go Outdated Show resolved Hide resolved
src/os/user/iterate_windows.go Show resolved Hide resolved
src/os/user/listgroups_solaris.go Outdated Show resolved Hide resolved
src/os/user/user.go Outdated Show resolved Hide resolved
src/os/user/user.go Outdated Show resolved Hide resolved
@mjonaitis1 mjonaitis1 force-pushed the os-user-iteration branch 7 times, most recently from cec9b7a to 6ed0451 Compare August 27, 2021 11:30
In order to add users and groups iterators for os/user,
ReadSubKeyNames must be changed. Instead of returning
a full slice of all subkey names, it can be changed to
accept callback function, which receives sequentially
iterated subkey names. As such, ReadSubKeyNames becomes
an iterator and will use less memory for larger sets of
data.
Plan9 operating system stores users and groups information in
/adm/users file. This file is similar in strucuture to unix
based /etc/passwd and /etc/group. Since readColonFile resides in
lookup_unix.go file, extracting it makes it possible to reuse the
functionality across all os targets that can can utilize the function.
@Mantelijo Mantelijo marked this pull request as ready for review August 27, 2021 15:28
Copy link

@witriew witriew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does upstream know about these new API proposals?

A ListUsers type API might be more amenable than a callback-based API

src/internal/syscall/windows/registry/key.go Show resolved Hide resolved
src/internal/syscall/windows/registry/key.go Show resolved Hide resolved
// iterateSIDS iterates through _profileListKey sub keys and calls provided
// fn with enumerated sub key name as parameter. If fn returns non-nil error,
// iteration is terminated.
func iterateSIDS(fn func(string) error) error {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iterateSIDs should probably accept a callback that accepts the SID.

func iterateSIDs(fn func(*syscall.SID) error) error

// IterateUsers iterates over user entries. For each retrieved *User entry provided NextUserFunc is called.
//
// On UNIX, if CGO is enabled, getpwent(3) is used in the underlying implementation. Since getpwent(3) is not thread-safe,
// locking is strongly advised.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What needs locking in this case? Maybe provide some examples or checks to make sure it is locked?

src/os/user/iterate_cgo.go Show resolved Hide resolved
Comment on lines +77 to +88
result, err := userIterator.get()

// If result is nil - getpwent iterated through entire users database or there was an error
if result == nil {
return err
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The return of err should be based off of what err actually is instead of the value, result.

		result, err := userIterator.get()

		// If result is nil - getpwent iterated through entire users database or there was an error
		if err != nil {
			return err
		}
		
		...
		if result == nil {
		        ...
		}

Comment on lines +76 to +82
if user, ok := v.(*User); ok {
err := fn(user)
if err != nil {
return nil, err
}
}
return nil, nil
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if user, ok := v.(*User); ok {
err := fn(user)
if err != nil {
return nil, err
}
}
return nil, nil
if user, ok := v.(*User); !ok {
return nil, nil
}
if err := fn(user); err != nil {
return nil, err
}

src/os/user/iterate_plan9.go Show resolved Hide resolved
src/os/user/iterate_plan9.go Show resolved Hide resolved
Go standard library has os/user package which allows to lookup user
or group records via either user/group name or id. This commit
extends capabilities of os/user package by introducing iteration
functionality for users and groups.

Users and groups iteration functionality might be useful in cases where
a full or partial list of all available users/groups is required in an
application.
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

Successfully merging this pull request may close these issues.

4 participants