Skip to content

Add a default /etc/passwd with a root user. - #14

Closed
dlorenc wants to merge 1 commit into
GoogleContainerTools:masterfrom
dlorenc:passwd
Closed

Add a default /etc/passwd with a root user.#14
dlorenc wants to merge 1 commit into
GoogleContainerTools:masterfrom
dlorenc:passwd

Conversation

@dlorenc

@dlorenc dlorenc commented May 10, 2017

Copy link
Copy Markdown
Contributor

This adds a default /etc/passwd for a root user.

Without this, a simple Go program to find the current user fails:

package main

import (
	"fmt"
	"os"
	"os/user"
)

func main() {
	u, err := user.Current()
	if err != nil {
		fmt.Println("Error getting user: ", err)
		os.Exit(1)
	}
	fmt.Printf("USER: %s\n", u)
}

with:

Error getting user:  user: lookup userid 0: no such file or directory

With this, the program runs correctly:

USER: &{0 0 root root /root}

We might also want to create the /root directory.

@dlorenc

dlorenc commented May 10, 2017

Copy link
Copy Markdown
Contributor Author

Also, what should we put in for the final field, which is the default shell for the user?

@mattmoor

Copy link
Copy Markdown
Contributor

I have an idea :)

What if we made the user="..." directive on docker_build write this file...

We wouldn't be able to append to it, but there likely shouldn't be multiple users available anyhow.

@dlorenc

dlorenc commented May 10, 2017

Copy link
Copy Markdown
Contributor Author

I have an idea :)

What if we made the user="..." directive on docker_build write this file...

We wouldn't be able to append to it, but there likely shouldn't be multiple users available anyhow.

Good point re no multiple users. We probably want one to exist for root by default.

@mattmoor

Copy link
Copy Markdown
Contributor

@dlorenc Agreed. Wouldn't it be awesome if all it took was user="root" on the base image :D

@dlorenc

dlorenc commented May 11, 2017

Copy link
Copy Markdown
Contributor Author

I have an idea :)

What if we made the user="..." directive on docker_build write this file...

We wouldn't be able to append to it, but there likely shouldn't be multiple users available anyhow.

Actually I'm not sure this would work that well without changes to the user directive. This file needs to map the UID to the name in the USER directive.

Without knowing the UID the user wants, USER isn't very helpful and we wouldn't be able to auto-create this file.

What do you think of another rule to generate this file? Something like:

passwd_file(
  initial_file = ":mypasswd" # Will append into this if specified
  user_entries = [
    "root:x:0:0:root:/root:/myshell",
    "otheruser:x:1:1:otheruser:/home/otheruser:/bin/bash"
  ]
)

Then you could include this via a normal docker_build rule?

@r2d4

r2d4 commented May 11, 2017

Copy link
Copy Markdown
Contributor

I like @dlorenc's syntax but leaving the buildroot syntax here for reference, which is very similar

rkt    -1 rkt    -1 -       /home/rkt    /bin/bash rkt-admin,wheel,vboxsf     -
docker -1 docker -1 =tcuser /home/docker /bin/bash rkt,rkt-admin,wheel,vboxsf -

Docs:
https://buildroot.org/downloads/manual/manual.html#makeuser-syntax

Example:
https://github.com/kubernetes/minikube/blob/master/deploy/iso/minikube-iso/board/coreos/minikube/users

@dlorenc

dlorenc commented May 11, 2017

Copy link
Copy Markdown
Contributor Author

Yeah this is starting to feel a bit overengineered given that I can't imagine a use case of an image needing more than one user. The nice thing about making this into a rule is that we could automate the group creation as well I guess.

@mattmoor

Copy link
Copy Markdown
Contributor

I think that (as you suggest) creating the rule provides an element of encapsulation that could enable us to grow what it can do without crazy changes to the docker_build surface.

If what we're walking about is the creation of a single user, then I'd suggest naming the elements and shedding the passwd syntax:

passwd_file(
   name = "passwd",
   username="mattmoor",
   uid=1, # Optional (default to like 1000, warn when username=root and this isn't 0)
   shell="/bin/bash", # Optional
)

This probably belongs in rules_docker, not here :)

@mattmoor

Copy link
Copy Markdown
Contributor

Another point on encapsulation, if it produces a tarball instead of a single file, then we would accomplish two things:

  1. We gain the capacity to add multiple files in the future.
  2. We can control the file path/headers, which would be easy to get wrong when adding this to docker_build.

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.

3 participants