-
Notifications
You must be signed in to change notification settings - Fork 2k
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
docker: fix delimiter for selinux label for read-only volumes #23750
Conversation
629c4b8
to
cebf14a
Compare
7a7169d
to
f51ae9b
Compare
91dc067
to
e57e7ba
Compare
e57e7ba
to
1be928f
Compare
1be928f
to
f0419f6
Compare
Tested end-to-end on Amazon Linux (which has SELinux) with the following agent configuration: agent.hclbind_addr = "0.0.0.0"
enable_debug = true
log_level = "debug"
data_dir = "/var/run/nomad"
server {
enabled = true
}
client {
enabled = true
}
plugin "docker" {
config {
allow_privileged = true
volumes {
enabled = true
selinuxlabel = "z"
}
gc {
image = false
}
}
} and the following jobspec: example.nomad.hcljob "example" {
group "group" {
task "task" {
driver = "docker"
config {
image = "busybox:1"
command = "httpd"
args = ["-vv", "-f", "-p", "8001", "-h", "/local"]
volumes = ["/etc/ssl/certs:/etc/ssl/certs:ro"]
}
resources {
cpu = 50
memory = 50
}
}
}
} Logs from the client:
And resulting mounts on the Docker container:
|
f0419f6
to
9e9610c
Compare
The Docker driver's `volume` field to specify bind-mounts takes a list of strings that consist of three `:`-delimited fields: source, destination, and options. We append the SELinux label from the plugin configuration as the third field. But when the user has already specified the volume is read-only with `:ro`, we're incorrectly appending the SELinux label with another `:` instead of the required `,`. Combine the options into a single field value before appending them to the bind mounts configuration. Updated the tests to split out Windows behavior (which doesn't accept options) and to ensure the test task has the expected environment for bind mounts. Fixes: #23690
9e9610c
to
c75ea1d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
The Docker driver's
volume
field to specify bind-mounts takes a list of strings that consist of three:
-delimited fields: source, destination, and options. We append the SELinux label from the plugin configuration as the third field. But when the user has already specified the volume is read-only with:ro
, we're incorrectly appending the SELinux label with another:
instead of the required,
.Combine the options into a single field value before appending them to the bind mounts configuration. Updated the tests to split out Windows behavior (which has a different volume specification format) and to ensure the test task has the expected environment for bind mounts.
Fixes: #23690