⚠️ run BMO ironic deployment as non-root #1231
Conversation
metal3-io-bot
added
do-not-merge/work-in-progress
tuminoid
force-pushed
the
tuomo/run-ironic-non-root
branch
from
9320b5a
to
c843753
Compare
tuminoid
changed the title
|
/cc @elfosardo @dtantsur This is still WIP, but I'd appreciate early comments. |
tuminoid
force-pushed
the
tuomo/run-ironic-non-root
branch
from
c843753
to
11f4c06
Compare
Testing thes two together: metal3-io/ironic-image#410 metal3-io/baremetal-operator#1231
Testing thes two together: metal3-io/ironic-image#410 metal3-io/baremetal-operator#1231
Testing thes two together: metal3-io/ironic-image#410 metal3-io/baremetal-operator#1231
|
/test-centos-e2e-integration-main |
Testing thes two together: metal3-io/ironic-image#410 metal3-io/baremetal-operator#1231
As expected, this fails. This must run on top of Ironic image change. |
tuminoid
force-pushed
the
tuomo/run-ironic-non-root
branch
2 times, most recently
from
6ebd6aa
to
88997cc
Compare
|
The title is misleading: you're not updating just BMO. Let's maybe start with BMO itself since it is an easy win? And finish with dnsmasq which is the hardest. |
All the technical issues are solved, I just keep to update the pidfile location, and as discussed in Ironic-image PR, I will reuse existing users instead of nonroot and update the UID in this one. Combined BMO and Ironic-image test is running at metal3-io/metal3-dev-env#1172 and were passing already. I will fix the commit message, title and description of the PR when I get those sorted out, and remove the WIP when its ready for review and merge. |
tuminoid
force-pushed
the
tuomo/run-ironic-non-root
branch
2 times, most recently
from
8d8c0bf
to
c59d776
Compare
metal3-io-bot
removed
the
do-not-merge/work-in-progress
Testing thes two together: metal3-io/ironic-image#410 metal3-io/baremetal-operator#1231
|
/hold No longer WIP, but needs metal3-io/ironic-image#410 to be merged first to test. |
metal3-io-bot
added
the
do-not-merge/hold
Do not merge this. Testing thes two together: metal3-io/ironic-image#410 metal3-io/baremetal-operator#1231
tuminoid
changed the title
tuminoid
force-pushed
the
tuomo/run-ironic-non-root
branch
from
c59d776
to
c40cc69
Compare
|
As requested on the ironic-image PR, I changed inspector container to run as ironic-inspector user, and keepalived back to nonroot as it is not sharing the filesystem with other containers. |
Do not merge this. Testing thes two together: metal3-io/ironic-image#410 metal3-io/baremetal-operator#1231
Do not merge this. Testing thes two together: metal3-io/ironic-image#410 metal3-io/baremetal-operator#1231
BMO ironic has no reason to run as root. Make it run as "ironic" user. dnsmasq requires elevated capabiities. k8s is missing the feature of ambient capabilities, so it requires us to setcap the binaries with expected capabilities and container must be running with "allowPrivilegeEscalation: true" in the manifest to allow elevation. Read the ambient capabilities KEP for more details: https://github.com/kubernetes/enhancements/blob/master/keps/sig-security/2763-ambient-capabilities/README.md Add securityContext to BMO deployment manifest and keepalived component, with correct UIDs and GIDs. This is important to be able to share files via /shared. Modify keepalived image to run as ironic user, which we use the same UID and GID as the ironic-image. This commit requires ironic-image with PR metal3-io/ironic-image#410 to be merged to work.
tuminoid
force-pushed
the
tuomo/run-ironic-non-root
branch
from
c40cc69
to
4908f57
Compare
|
Let's see if it picks up the new ironic-image now. /test-centos-e2e-integration-main |
Do not merge this. Testing thes two together: metal3-io/ironic-image#410 metal3-io/baremetal-operator#1231
|
OK, we're passing tests here and in dev-env after fixing CI flake (corrupted node image was built). All comments should be addressed. Can I get LGTM and approve for this as well? @zaneb @dtantsur @elfosardo |
|
/unhold |
metal3-io-bot
removed
the
do-not-merge/hold
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kashifest The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
metal3-io-bot
added
the
approved
|
Ow it had lgtm already. Didnt notice. |
BMO ironic has no reason to run as root. Make it run as "ironic" user.
dnsmasq requires elevated capabiities. k8s is missing the feature of ambient capabilities, so it requires us to setcap the binaries with expected capabilities and container must be running with "allowPrivilegeEscalation: true" in the manifest to allow elevation.
Read the ambient capabilities KEP for more details: https://github.com/kubernetes/enhancements/blob/master/keps/sig-security/2763-ambient-capabilities/README.md
Add securityContext to BMO deployment manifest and keepalived component, with correct UIDs and GIDs. This is important to be able to share files via /shared.
Modify keepalived image to run as ironic user, which we use the same UID and GID as the ironic-image.
This commit requires ironic-image with PR metal3-io/ironic-image#410 to be merged to work.