Skip to content

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

Merged
ndeloof merged 1 commit intodocker:v2from
ndeloof:run_cap_add
Jun 19, 2023
Merged

introduce run --cap-add to run maintenance commands using service image#10669
ndeloof merged 1 commit intodocker:v2from
ndeloof:run_cap_add

Conversation

@ndeloof
Copy link
Copy Markdown
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

@ndeloof ndeloof mentioned this pull request Jun 8, 2023
Closed
@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 8, 2023 •
edited
Loading

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, StefanScherer, glours, laurazard, milas, nicksieger and ulyssessouza 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
glours
glours approved these changes Jun 8, 2023
View reviewed changes
Copy link
Copy Markdown
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
Copy Markdown
Member

thaJeztah 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",
	}
}

@thaJeztah
Copy link
Copy Markdown
Member

thaJeztah commented Jun 8, 2023

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

@ndeloof
Copy link
Copy Markdown
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
Copy Markdown

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
Copy Markdown
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
Copy Markdown

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

Reviewers

@glours glours glours approved these changes

@nicksieger nicksieger Awaiting requested review from nicksieger nicksieger was automatically assigned from docker/compose-maintainers

@StefanScherer StefanScherer Awaiting requested review from StefanScherer StefanScherer was automatically assigned from docker/compose-maintainers

@ulyssessouza ulyssessouza Awaiting requested review from ulyssessouza ulyssessouza was automatically assigned from docker/compose-maintainers

@laurazard laurazard Awaiting requested review from laurazard laurazard was automatically assigned from docker/compose-maintainers

+1 more reviewer

@milas milas milas approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

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

5 participants

@ndeloof @thaJeztah @oliv3r @glours @milas