-
Notifications
You must be signed in to change notification settings - Fork 56
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
Support targeting installs to Determinate Nix Enterprise Edition. #905
Conversation
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: I've seen the run-through, this is a great step forward to deliver custom Nix on macOS for enterprise customers 👍
When the public Nix installer for macOS is run with |
} | ||
|
||
#[tracing::instrument(level = "debug", skip_all)] | ||
async fn execute(&mut self) -> Result<(), ActionError> { |
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.
It would be nice to share more code with configure_init_service
, but we can do that in a follow-up PR.
@@ -57,7 +59,11 @@ impl EncryptApfsVolume { | |||
// The user has a password matching what we would create. | |||
if planned_create_apfs_volume.state == ActionState::Completed { |
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.
I wonder if this is still true in the case of this "enterprise edition" -- since we add specific access to another binary, isn't it possible that if we take the current volume as-is, we'll not have that access and things will break?
(I think all of these completed
branches need to be audited to ensure that this is OK)
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.
All the other branches look good to me ... one option is to unconditionally add our program to the access list. I don't know how else we can detect a fault here.
impl ConfigureEnterpriseEditionInitService { | ||
#[tracing::instrument(level = "debug", skip_all)] | ||
pub async fn plan(start_daemon: bool) -> Result<StatefulAction<Self>, ActionError> { | ||
Ok(Self { start_daemon }.into()) |
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.
These EE-specific steps should probably ensure that the "enablement" binary is present, or otherwise mark it as Skipped
(or error). Additionally, this seems to be causing some issues in CI where it just.... hangs at some point (why is the CI test trying to enable EE mode? surely that's wrong). That hang will probably rear its ugly head outside of CI too.
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.
I had accidentally inverted the bool in the planner: 045f388
The root macos planner does make the check that the binary is present. I'm not sure we should duplicate the behavior in the actions ... what do you think?
…ur planning requires no changes on other targets at the moment.
…t across tools and products.
…otherwise no-op enterprise_editition encryption case
Description
Passing
--enterprise
will disable the upstream launchd daemons and use ours. Requires the user separately coordinate the installation of Determinate Nix for macOS ahead of time.Checklist
cargo fmt
nix build
nix flake check
Validating with
install.determinate.systems
If a maintainer has added the
upload to s3
label to this PR, it will become available for installation viainstall.determinate.systems
: