Support custom docker daemon args #8435

Furisto · 2022-02-24T14:16:44Z

Description

This allows users to add arguments to the docker daemon running in the workspace. As a first step mapping the container user to the gitpod user is supported.

Related Issue(s)

Fixes #8103

How to test

Add environment variable DOCKER_DARGS to your user settings. The content should look like this:
{ "remap-user": "1000" } (Means container id 1000 will be mapped to gitpod user).
Start workspace
mkdir /tmp/me
Run docker image which supports running as unprivileged user e.g. sudo docker run -it -u node -v /tmp/me:/tmp node sh
Create file within container e.g. touch /tmp/foo
Verify that the created file has gitpod:gitpod permissions in the workspace

Hints for reviewers

The uidmap package needs to be installed as docker needs to create multiple user mappings which require newuidmap/newgidmap
The most recent version of runc needs to be installed as otherwise mounting cgroupfs will fail with when starting the container
Both dependencies will only be installed if the DOCKER_DAEMON_ARGS environment variable has been so that the startup performance of the first container will not be impacted for users that do no require a mapping.
This does not allow specifying the dockerd argument (userns-remap) directly as this would mean the user would specify ("userns-remap" : "gitpod"). We already know that the user wants to map the container user to gitpod so this is redundant. What we do not get this way is the container user that should be mapped, which means we cannot setup the /etc/subuid and /etc/subgid files for them and they would have to do it themselves.
Instead they can just specify the user they want to be mapped (e.g. 1000) and we ensure that this will be mapped to the gitpod user.
I considered modifying runc-facade instead of adding this to docker-up, but just adding the user mapping to the spec will cause file system permission errors because the rootfs needs to be chowned. This way docker will do it for us.

Release Notes

Docker in workspaces now allows mapping the user id of a container user to the workspace gitpod user

Documentation

Have not added documentation before, so any advice is appreciated.

codecov · 2022-02-24T14:16:58Z

Codecov Report

Merging #8435 (e9ab07f) into main (149ca15) will increase coverage by 0.79%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #8435      +/-   ##
==========================================
+ Coverage   10.03%   10.82%   +0.79%     
==========================================
  Files          24       18       -6     
  Lines        1944     1025     -919     
==========================================
- Hits          195      111      -84     
+ Misses       1742      912     -830     
+ Partials        7        2       -5

Flag	Coverage Δ
components-gitpod-cli-app	`10.82% <ø> (ø)`
components-local-app-app-darwin-amd64	`?`
components-local-app-app-darwin-arm64	`?`
components-local-app-app-linux-amd64	`?`
components-local-app-app-linux-arm64	`?`
components-local-app-app-windows-386	`?`
components-local-app-app-windows-amd64	`?`
components-local-app-app-windows-arm64	`?`
components-workspacekit-app	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
components/workspacekit/cmd/nsenter.go
components/local-app/pkg/auth/pkce.go
components/local-app/pkg/auth/auth.go
components/workspacekit/cmd/rings.go
components/workspacekit/cmd/lift.go
components/workspacekit/cmd/root.go

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 149ca15...e9ab07f. Read the comment docs.

components/docker-up/dependencies.sh

utam0k · 2022-02-25T00:06:49Z

First of all, thanks for the great PR.
Do you know why? I was a little curious.

The most recent version of runc needs to be installed as otherwise mounting cgroupfs will fail with when starting the container

components/docker-up/docker-up/main.go

utam0k · 2022-02-25T00:34:53Z

components/docker-up/docker-up/main.go

+	"remap-user": "userns-remap",
+}
+
+func setUserArgs(args []string) ([]string, error) {


This function seemed simpler if it was dedicated to crate args from env vars from the user that were converted to dockerd args. In other words, instead of doing both generation and addition, why not just concentrate on generation?

Suggested change

func setUserArgs(args []string) ([]string, error) {

func userArgs() ([]string, error) {

args := []string{}

And,

uargs, err := userArgs() if err != nil { return xerrors.Errorf("cannot add user supplied docker args: %w", err) } args = append(args, uargs...)

components/docker-up/docker-up/main.go

utam0k · 2022-02-25T01:52:15Z

components/docker-up/docker-up/main.go

+	if id != 0 {
+		newfile.WriteString(fmt.Sprintf("gitpod:%d:%d\n", 1, id))
+		newfile.WriteString("gitpod:33333:1\n")
+
+	} else {
+		newfile.WriteString("gitpod:33333:1\n")
+		newfile.WriteString(fmt.Sprintf("gitpod:%d:%d\n", 1, 33332))
+		newfile.WriteString(fmt.Sprintf("gitpod:%d:%d\n", 33334, 32200))
+	}


How about this? I couldn't get the point what 32200 mean. What does it mean?

Suggested change

if id != 0 {

newfile.WriteString(fmt.Sprintf("gitpod:%d:%d\n", 1, id))

newfile.WriteString("gitpod:33333:1\n")

} else {

newfile.WriteString("gitpod:33333:1\n")

newfile.WriteString(fmt.Sprintf("gitpod:%d:%d\n", 1, 33332))

newfile.WriteString(fmt.Sprintf("gitpod:%d:%d\n", 33334, 32200))

}

gitpodUserId := 33333

mappingFmt := func(username string, id int, size int) string { return fmt.Sprintf("%s:%d:%d\n", username, id, size) }

if id != 0 {

newfile.WriteString(mappingFmt("gitpod", 1, id))

newfile.WriteString(mappingFmt("gitpod", gitpodUserId, 1))

} else {

newfile.WriteString(mappingFmt("gitpod", gitpodUserId, 1))

newfile.WriteString(mappingFmt("gitpod", 1, gitpodUserId-1))

newfile.WriteString(mappingFmt("gitpod", gitpodUserId+1, 33200))

}

utam0k · 2022-02-25T01:58:00Z

I agree with you, we have to add the documentation. But I don't know too 😭

Have not added documentation before, so any advice is appreciated.

- Configurable runc version - Change env name - Converter func for user args

Furisto · 2022-02-25T17:51:07Z

@utam0k PTAL

kylos101 · 2022-02-25T20:10:29Z

Hey @mikenikles , if we wanted to document a new capability to our docker in workspaces feature - can you help guide on what is recommended to update related documentation?

mikenikles · 2022-02-26T03:29:59Z

We don't have a page specifically for docker in workspaces, but could add one in the Configure section.

Alternatively, I'd add a note to https://www.gitpod.io/docs/environment-variables#user-specific-environment-variables given this PR introduces a new user-level environment variable.

The corresponding source file is at https://github.com/gitpod-io/website/blob/main/gitpod/docs/environment-variables.md.

Furisto · 2022-02-27T14:30:45Z

First of all, thanks for the great PR. Do you know why? I was a little curious.

The most recent version of runc needs to be installed as otherwise mounting cgroupfs will fail with when starting the container

I did not try to verify which specific change in runc made the difference, although I have some suspicions.

utam0k · 2022-02-28T00:25:05Z

components/docker-up/docker-up/main.go

@@ -191,25 +196,18 @@ func setUserArgs(args []string) ([]string, error) {
 	}

 	for userArg, userValue := range providedDockerArgs {
-		mapped, exists := allowedDockerArgs[userArg]
+		converter, exists := allowedDockerArgs[userArg]


I love this name 👍

utam0k · 2022-02-28T00:26:19Z

components/docker-up/docker-up/main.go

-		newfile.WriteString(fmt.Sprintf("gitpod:%d:%d\n", 33334, 32200))
+		newfile.WriteString(mappingFmt("gitpod", gitpodUserId, 1))
+		newfile.WriteString(mappingFmt("gitpod", 1, gitpodUserId-1))
+		newfile.WriteString(mappingFmt("gitpod", gitpodUserId+1, 32200)) // map rest of user ids in the user namespace


A good comment. Thanks.

map rest of user ids in the user namespace

utam0k

LGTM 💯

iQQBot · 2022-02-28T02:58:46Z

too many commit 😂

shaoye · 2022-06-21T22:03:54Z

1. Add environment variable DOCKER_DAEMON_ARGS to your user settings. The content should look like this:
{ "remap-user": "1000" } (Means container id 1000 will be mapped to gitpod user).
2. Start workspace
3. mkdir /tmp/me
4. Run docker image which supports running as unprivileged user e.g. sudo docker run -it -u node -v /tmp/me:/tmp node sh
5. Create file within container e.g. touch /tmp/foo
6. Verify that the created file has gitpod:gitpod permissions in the workspace

got permission denied at step 5,

touch: cannot touch '/tmp/foo': Permission denied

the node user can't create any files since /tmp is owned by gitpod user

/tmp$ ls -al
total 4
drwxr-xr-x 2 33333 33333   40 Jun 21 21:59 .
drwxr-xr-x 1 root  root  4096 Jun 21 22:00 ..

Any settings I am missing?

utam0k · 2022-06-22T21:54:08Z

@shaoye Thanks for your report. Can I ask you to make sure how under /workspace/* dir is the right behavior?

shaoye · 2022-06-22T22:05:02Z

@shaoye Thanks for your report. Can I ask you to make sure how under /workspace/* dir is the right behavior?

same, still got permission error if I mount /workspace/tmp/me

-v /workspace/tmp/me:/tmp

utam0k · 2022-06-22T22:09:01Z

@shaoye Thanks! Can I ask you to create the issue? If you create it, we can schedule the issue.

shaoye · 2022-06-22T23:11:30Z

@shaoye Thanks! Can I ask you to create the issue? If you create it, we can schedule the issue.

np! #10844

utam0k · 2022-06-23T04:57:08Z

@shaoye Thank you!

Furisto added 8 commits February 10, 2022 16:54

Log env args

32e899e

Install prerequisites for id mapping

5c6bc1a

Ensure newer runc version is not overwritten

3d77668

Map container user to gitpod user

e2773de

Remove logging

00262b8

Enable mapping of root to gitpod user

c8ac691

Map additional ids

5996069

Improve startup performance

5d83a7c

Furisto requested a review from a team February 24, 2022 14:16

roboquat added the release-note label Feb 24, 2022

roboquat added the size/L label Feb 24, 2022

github-actions bot added the team: workspace Issue belongs to the Workspace team label Feb 24, 2022

Furisto mentioned this pull request Feb 24, 2022

Support custom docker daemon args #8361

Closed

utam0k reviewed Feb 24, 2022

View reviewed changes

components/docker-up/dependencies.sh Outdated Show resolved Hide resolved

utam0k reviewed Feb 25, 2022

View reviewed changes

components/docker-up/docker-up/main.go Outdated Show resolved Hide resolved

utam0k reviewed Feb 25, 2022

View reviewed changes

components/docker-up/docker-up/main.go Outdated Show resolved Hide resolved

utam0k reviewed Feb 25, 2022

View reviewed changes

Review feedback

e9ab07f

- Configurable runc version - Change env name - Converter func for user args

sagor999 mentioned this pull request Feb 26, 2022

[image-builder] Increase docker log size #7147

Closed

sagor999 linked an issue Feb 26, 2022 that may be closed by this pull request

[image-builder] Increase docker log size #7147

Closed

utam0k reviewed Feb 28, 2022

View reviewed changes

utam0k approved these changes Feb 28, 2022

View reviewed changes

roboquat merged commit ba168ed into main Feb 28, 2022

roboquat deleted the fo/8103-2 branch February 28, 2022 00:30

utam0k mentioned this pull request Feb 28, 2022

[docker] Support arguments to Docker daemon #8103

Closed

shaoye mentioned this pull request Jun 22, 2022

Custom docker daemon args is not working #10844

Closed

kylos101 removed a link to an issue Jan 10, 2023

[image-builder] Increase docker log size #7147

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support custom docker daemon args #8435

Support custom docker daemon args #8435

Furisto commented Feb 24, 2022 •

edited by utam0k

Loading

codecov bot commented Feb 24, 2022 •

edited

Loading

utam0k commented Feb 25, 2022

utam0k Feb 25, 2022

utam0k Feb 25, 2022

utam0k commented Feb 25, 2022 •

edited

Loading

Furisto commented Feb 25, 2022

kylos101 commented Feb 25, 2022

mikenikles commented Feb 26, 2022

Furisto commented Feb 27, 2022

utam0k Feb 28, 2022 •

edited

Loading

utam0k Feb 28, 2022

utam0k left a comment

iQQBot commented Feb 28, 2022 •

edited by werft-gitpod-dev-com bot

Loading

shaoye commented Jun 21, 2022 •

edited

Loading

utam0k commented Jun 22, 2022

shaoye commented Jun 22, 2022

utam0k commented Jun 22, 2022

shaoye commented Jun 22, 2022

utam0k commented Jun 23, 2022

	func setUserArgs(args []string) ([]string, error) {
	func userArgs() ([]string, error) {
	args := []string{}

Support custom docker daemon args #8435

Support custom docker daemon args #8435

Conversation

Furisto commented Feb 24, 2022 • edited by utam0k Loading

Description

Related Issue(s)

How to test

Hints for reviewers

Release Notes

Documentation

codecov bot commented Feb 24, 2022 • edited Loading

Codecov Report

utam0k commented Feb 25, 2022

utam0k Feb 25, 2022

Choose a reason for hiding this comment

utam0k Feb 25, 2022

Choose a reason for hiding this comment

utam0k commented Feb 25, 2022 • edited Loading

Furisto commented Feb 25, 2022

kylos101 commented Feb 25, 2022

mikenikles commented Feb 26, 2022

Furisto commented Feb 27, 2022

utam0k Feb 28, 2022 • edited Loading

Choose a reason for hiding this comment

utam0k Feb 28, 2022

Choose a reason for hiding this comment

utam0k left a comment

Choose a reason for hiding this comment

iQQBot commented Feb 28, 2022 • edited by werft-gitpod-dev-com bot Loading

shaoye commented Jun 21, 2022 • edited Loading

utam0k commented Jun 22, 2022

shaoye commented Jun 22, 2022

utam0k commented Jun 22, 2022

shaoye commented Jun 22, 2022

utam0k commented Jun 23, 2022

Furisto commented Feb 24, 2022 •

edited by utam0k

Loading

codecov bot commented Feb 24, 2022 •

edited

Loading

utam0k commented Feb 25, 2022 •

edited

Loading

utam0k Feb 28, 2022 •

edited

Loading

iQQBot commented Feb 28, 2022 •

edited by werft-gitpod-dev-com bot

Loading

shaoye commented Jun 21, 2022 •

edited

Loading