-
Notifications
You must be signed in to change notification settings - Fork 18
FreeBSD sandbox module, using jail(8) #156
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
Conversation
| (deps sandbox.runc.ml) | ||
| (target sandbox.ml) | ||
| (enabled_if (<> %{system} macosx)) | ||
| (enabled_if (and (<> %{system} macosx) (<> %{system} freebsd))) |
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.
Given runc is Linux can this rule become (enabled_if (<> %{system} linux))?
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.
That would be (enabled_if (= %{system} linux)), but yes, I suppose this could work.
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.
Well, it turns out making this change breaks a few builds (such as debian 11.4 on arm32 and powerpc64, see build report), so I'll revert that particular change.
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.
The issue is to do with the variable expansion in dune if you add a default sandbox.dummy.ml with
type config = unit
let cmdliner = failwith "Sandbox not available"and setup a copy rule for Win32 that should work. I'm not sure what is happening with the POWER build, the reported value for system is clearly not doing what I expect.
| (deps sandbox.runc.ml) | ||
| (target sandbox.ml) | ||
| (enabled_if (<> %{system} macosx)) | ||
| (enabled_if (and (<> %{system} macosx) (<> %{system} freebsd))) |
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.
The issue is to do with the variable expansion in dune if you add a default sandbox.dummy.ml with
type config = unit
let cmdliner = failwith "Sandbox not available"and setup a copy rule for Win32 that should work. I'm not sure what is happening with the POWER build, the reported value for system is clearly not doing what I expect.
| match config.network with | ||
| | [ "host" ] -> | ||
| "ip4=inherit" :: "ip6=inherit" :: "host=inherit" :: options | ||
| | _ -> options |
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.
Can we restrict the jail networking to only access itself here?
The runc implementation overwrites hosts with 127.0.0.1 localhost builder https://github.com/ocurrent/obuilder/blob/master/lib/sandbox.runc.ml#L283
lib/sandbox.jail.ml
Outdated
| Due to the above mutex, only one jail may be started by a given | ||
| obuilder process, so appending the obuilder pid is enough to | ||
| guarantee uniqueness. *) | ||
| jail_name = "name=obuilder_" ^ (Int.to_string (Unix.getpid ())); |
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.
This needs to handle multiple jails concurrently from the ocluster-worker this will need a different approach to generating the jail name and removing the mutex.
lib/sandbox.jail.ml
Outdated
|
|
||
| let cmdliner : config Term.t = | ||
| let make dummy = { dummy } in | ||
| Term.(const make $ dummy) |
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.
If you make type config = unit then this can just become
let cmdliner : config Term.t =
Term.(const ())| in | ||
| (* At least under FreeBSD, pass the -f option to make sure any dangling | ||
| devfs mount within the filesystem gets automatically unmounted. *) | ||
| let opts = "-f" :: opts in |
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.
This needs validation on linux and macOS ZFS implementations.
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.
man zfs-destroy on macOS says:
-f Forcibly unmount file systems. This option has no effect on non-file systems or unmounted file systems.
|
Rebasquashed, and documentation draft added. |
CHANGES: - Add a Docker backend for Windows and Linux jobs. (@MisterDA ocurrent/obuilder#127 ocurrent/obuilder#75, reviewed by @talex5 and @tmcgilchrist) - Add FreeBSD sandbox backend using jail(8) (@dustanddreams ocurrent/obuilder#156 ocurrent/obuilder#174, reviewed by @tmcgilchrist, @MisterDA, and @mtelvers) - Add Macos ZFS sandbox (@mtelvers ocurrent/obuilder#164, reviewed by @tmcgilchrist) - Support XFS store (@mtelvers ocurrent/obuilder#170, reviewed by @tmcgilchrist) - Search for bash rather than assume it lies in /bin (@dustanddreams ocurrent/obuilder#159, reviewed by @tmcgilchrist) - Prune builds one at a time up to the limit (@mtelvers ocurrent/obuilder#157) - Specify upper bound on number of items in the store (@mtelvers ocurrent/obuilder#158, reviewed by @MisterDA) - Fix case where BTRFS is not fully allocated (@mtelvers ocurrent/obuilder#162) - Avoid pruning parent cache objects (@mtelvers ocurrent/obuilder#176, reviewed by @tmcgilchrist)
CHANGES: - Add a Docker backend for Windows and Linux jobs. (@MisterDA ocurrent/obuilder#127 ocurrent/obuilder#75, reviewed by @talex5 and @tmcgilchrist) - Add FreeBSD sandbox backend using jail(8) (@dustanddreams ocurrent/obuilder#156 ocurrent/obuilder#174, reviewed by @tmcgilchrist, @MisterDA, and @mtelvers) - Add Macos ZFS sandbox (@mtelvers ocurrent/obuilder#164, reviewed by @tmcgilchrist) - Support XFS store (@mtelvers ocurrent/obuilder#170, reviewed by @tmcgilchrist) - Search for bash rather than assume it lies in /bin (@dustanddreams ocurrent/obuilder#159, reviewed by @tmcgilchrist) - Prune builds one at a time up to the limit (@mtelvers ocurrent/obuilder#157) - Specify upper bound on number of items in the store (@mtelvers ocurrent/obuilder#158, reviewed by @MisterDA) - Fix case where BTRFS is not fully allocated (@mtelvers ocurrent/obuilder#162) - Avoid pruning parent cache objects (@mtelvers ocurrent/obuilder#176, reviewed by @tmcgilchrist)
Jail-based sandbox implementation.
Related to #109