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

introduce run --cap-add to run maintenance commands using service image #10669

Merged
merged 1 commit into from
Jun 19, 2023

Conversation

ndeloof
Copy link
Contributor

@ndeloof ndeloof commented Jun 8, 2023

What I did
introduce docker run --cap-add flag so user can get extra privileges to run maintenance commands without the need for an init container (which doesn't yet have a user-friendly support in compose file format)

services:
  test:
    image: alpine
    command: ls -al /volume
    user: "1001"
    volumes:
      - volume:/volume

volumes:
  volume: {}
 $ docker compose run --cap-add CAP_CHOWN --user root test chown 1001:1001 -R /volume
[+] Creating 2/0
 ✔ Network machin_default  Created                                         0.0s 
 ✔ Volume "machin_volume"  Created                                         0.0s 

$ docker compose up
[+] Running 1/0
 ✔ Container machin-test-1  Created                                        0.0s 
Attaching to machin-test-1
machin-test-1  | total 8
machin-test-1  | drwxr-xr-x    2 1001     1001          4096 Jun  8 09:26 .
machin-test-1  | drwxr-xr-x    1 root     root          4096 Jun  8 09:26 ..
machin-test-1 exited with code 0

Related issue
fixed #10655

(not mandatory) A picture of a cute animal, if possible in relation to what you did

@codecov
Copy link

codecov bot commented Jun 8, 2023

Codecov Report

Patch coverage: 75.32% and project coverage change: +0.03 🎉

Comparison is base (2efea2e) 58.89% compared to head (1f4341e) 58.92%.

Additional details and impacted files
@@            Coverage Diff             @@
##               v2   #10669      +/-   ##
==========================================
+ Coverage   58.89%   58.92%   +0.03%     
==========================================
  Files         112      112              
  Lines        9735     9749      +14     
==========================================
+ Hits         5733     5745      +12     
- Misses       3413     3414       +1     
- Partials      589      590       +1     
Impacted Files Coverage Δ
internal/tracing/docker_context.go 23.43% <0.00%> (-1.16%) ⬇️
internal/tracing/mux.go 0.00% <0.00%> (ø)
pkg/api/api.go 43.39% <ø> (ø)
pkg/compose/run.go 53.33% <0.00%> (-4.40%) ⬇️
cmd/compose/run.go 69.47% <89.23%> (+0.99%) ⬆️

... and 5 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@ndeloof ndeloof requested review from a team, nicksieger, StefanScherer, ulyssessouza, glours, milas and laurazard and removed request for a team June 8, 2023 09:36
@ndeloof ndeloof force-pushed the run_cap_add branch 2 times, most recently from 75cd559 to aae5d87 Compare June 8, 2023 09:41
Copy link
Contributor

@glours glours left a comment

Choose a reason for hiding this comment

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

LGTM

@thaJeztah
Copy link
Member

fixed #10655

Note that CAP_CHOWN is part of the default capabilities, so I don't think this PR is needed for that linked one; https://github.com/moby/moby/blob/1aae94074e63736a4b419f08cddc1baaaa717453/oci/caps/defaults.go#L3C1-L21

// DefaultCapabilities returns a Linux kernel default capabilities
func DefaultCapabilities() []string {
	return []string{
		"CAP_CHOWN",
		"CAP_DAC_OVERRIDE",
		"CAP_FSETID",
		"CAP_FOWNER",
		"CAP_MKNOD",
		"CAP_NET_RAW",
		"CAP_SETGID",
		"CAP_SETUID",
		"CAP_SETFCAP",
		"CAP_SETPCAP",
		"CAP_NET_BIND_SERVICE",
		"CAP_SYS_CHROOT",
		"CAP_KILL",
		"CAP_AUDIT_WRITE",
	}
}

@thaJeztah
Copy link
Member

If we add this for parity with docker run, should we add --cap-drop as well?

@ndeloof
Copy link
Contributor Author

ndeloof commented Jun 8, 2023

sure we could. There's no obvious use-case for it, but that would make sense in terms of CLI homogeneity

@ndeloof ndeloof force-pushed the run_cap_add branch 2 times, most recently from 8cac5f2 to c8bceb4 Compare June 8, 2023 11:30
@oliv3r
Copy link

oliv3r commented Jun 8, 2023

fixed #10655

Note that CAP_CHOWN is part of the default capabilities, so I don't think this PR is needed for that linked one; https://github.com/moby/moby/blob/1aae94074e63736a4b419f08cddc1baaaa717453/oci/caps/defaults.go#L3C1-L21

// DefaultCapabilities returns a Linux kernel default capabilities
func DefaultCapabilities() []string {
	return []string{
		"CAP_CHOWN",
		"CAP_DAC_OVERRIDE",
		"CAP_FSETID",
		"CAP_FOWNER",
		"CAP_MKNOD",
		"CAP_NET_RAW",
		"CAP_SETGID",
		"CAP_SETUID",
		"CAP_SETFCAP",
		"CAP_SETPCAP",
		"CAP_NET_BIND_SERVICE",
		"CAP_SYS_CHROOT",
		"CAP_KILL",
		"CAP_AUDIT_WRITE",
	}
}

sure, but if you have cap_drop: all in your compose file, you have to add this special capability :)

@ndeloof
Copy link
Contributor Author

ndeloof commented Jun 8, 2023

I need to check how this interact with existing cap set in service, i.e. have service set with cap_drop: XX and we run docker compose run --cap-add XX.

@oliv3r
Copy link

oliv3r commented Jun 8, 2023

I need to check how this interact with existing cap set in service, i.e. have service set with cap_drop: XX and we run docker compose run --cap-add XX.

I would expect that commandline > compose file.

@ndeloof ndeloof merged commit c61b8aa into docker:v2 Jun 19, 2023
@ndeloof ndeloof deleted the run_cap_add branch June 19, 2023 12:20
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.

Add possibility to override capabilities (--cap-add)
5 participants