Skip to content
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

Merged
merged 29 commits into from
Apr 15, 2024
Merged

Conversation

grahamc
Copy link
Member

@grahamc grahamc commented Apr 1, 2024

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
  • Formatted with cargo fmt
  • Built with nix build
  • Ran flake checks with nix flake check
  • Added or updated relevant tests (leave unchecked if not applicable)
  • Added or updated relevant documentation (leave unchecked if not applicable)
  • Linked to related issues (leave unchecked if not applicable)
Validating with install.determinate.systems

If a maintainer has added the upload to s3 label to this PR, it will become available for installation via install.determinate.systems:

curl --proto '=https' --tlsv1.2 -sSf -L https://install.determinate.systems/nix/pr/$PR_NUMBER | sh -s -- install

@grahamc grahamc added the upload to s3 The labeled PR is allowed to upload its artifacts to S3 for easy testing label Apr 1, 2024
Copy link
Member

@flexiondotorg flexiondotorg left a 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 👍

@flexiondotorg
Copy link
Member

When the public Nix installer for macOS is run with --nix-enterprise, it will fail as the public version doesn't know how to activate an enterprise install. In this event, it should sign-post the appropriate page on https://determinate.systems as a potential sales funnel.

cole-h
cole-h previously requested changes Apr 2, 2024
src/action/common/configure_init_service.rs Outdated Show resolved Hide resolved
src/action/common/configure_init_service.rs Outdated Show resolved Hide resolved
src/action/common/configure_init_service.rs Outdated Show resolved Hide resolved
src/action/macos/create_nix_volume.rs Outdated Show resolved Hide resolved
@grahamc grahamc changed the title Support targeting installs to Determinate's enterprise offering via Determinate Nix for macOS. Support targeting installs to Determinate Nix Enterprise Edition. Apr 4, 2024
src/planner/macos.rs Outdated Show resolved Hide resolved
src/action/common/configure_init_service.rs Outdated Show resolved Hide resolved
tests/fixtures/macos/macos.json Outdated Show resolved Hide resolved
}

#[tracing::instrument(level = "debug", skip_all)]
async fn execute(&mut self) -> Result<(), ActionError> {
Copy link
Member

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 {
Copy link
Member

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)

Copy link
Member Author

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())
Copy link
Member

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.

Copy link
Member Author

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
upload to s3 The labeled PR is allowed to upload its artifacts to S3 for easy testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants