Improve Mac package for enterprise install scenarios #16073

nstrauss · 2023-10-02T23:52:59Z

Thank you for investing time into a package! This PR suggests a few improvements to use with large scale enterprise deployments.

The Mac package as is assumes there's a logged in console user in order to set permissions on /opt/homebrew and copy cache files to ~/Library/Caches/Homebrew/api. Usually when a person installs the package manually. However, in an enterprise environment that's often not the case.

For organizationally managed Macs using software management tools like Jamf or Munki which initiate installs, it's common for there to be no logged in user or that a user hasn't even been created yet during install. These solutions and similar often run at the login window via a launch daemon before a valid console user has logged in. It's also possible to use Apple's Automated Device Enrollment to deliver packages via MDM before setup assistant is complete. Introducing fallbacks in cases where there's no active console user should cover most enterprise focused edge cases.

When a known system account such loginwindow is the current console user, infer the intended user by looking at the most recently created UID over 500.
Support passing in a HOMEBREW_PKG_USER environment variable for admins to have better control over the target user where required (a lab environment). For example - HOMEBREW_PKG_USER="nathaniel.strauss" installer -pkg Homebrew-4.1.15 -target /
Fix moving /opt/homebrew/cache_api to user directory. This previously did not work.
Bump package required minimum OS to 12.0 to match Homebrew.
Remove sudo from postinstall script. The installer already runs as root.
Remove installing gh from workflow. GitHub CLI is baked into the runner. https://github.com/actions/runner-images/blob/375e9a1c270ec81a3ffd5b429dcdee9508b46124/images/macos/macos-13-Readme.md?plain=1#L57

MikeMcQuaid

Thanks @nstrauss, great work so far ❤️! A bunch of comments here but I think we're pretty close. The TL;DR is that I think we want to avoid as many edge-cases here as possible. I think HOMEBREW_PKG_USER is a good addition but trying to guess the correct user or ending up with a root-owned installation gets a fairly big 👎🏻 from me, I'm afraid.

.github/workflows/build-pkg.yml

package/Distribution.xml

package/scripts/postinstall

package/Distribution.xml

MikeMcQuaid · 2023-10-03T12:07:11Z

Remove installing gh from workflow. GitHub CLI is baked into the runner. https://github.com/actions/runner-images/blob/375e9a1c270ec81a3ffd5b429dcdee9508b46124/images/macos/macos-13-Readme.md?plain=1#L57

We definitely have seen cases where it's not been present 🤷🏻. Would rather just leave this as it guarantees we have the newest version and adds minimal overhead.

For organizationally managed Macs using software management tools like Jamf or Munki which initiate installs, it's common for there to be no logged in user or that a user hasn't even been created yet during install. These solutions and similar often run at the login window via a launch daemon before a valid console user has logged in. It's also possible to use Apple's Automated Device Enrollment to deliver packages via MDM before setup assistant is complete. Introducing fallbacks in cases where there's no active console user should cover most enterprise focused edge cases.

I'm game for some additional configuration e.g. HOMEBREW_PKG_USER but a fall-back that results in a root-owned Homebrew or guesses at the correct user doesn't seem like an acceptable fall-back, I'm afraid.

MikeMcQuaid · 2023-10-03T12:08:39Z

@nstrauss Oh also: just a process note to avoid any surprises: when this PR looks good to go, we probably won't merge it but instead close and cherry-pick to a branch for testing. This isn't our usual process but want to avoid merging something that breaks the installer creation CI and can only test that with a maintainer-created non-fork PR on this repository (so the secrets are exposed and accessible).

nstrauss · 2023-10-03T20:22:05Z

@MikeMcQuaid Thank you for comments and feedback 🙏 Made a few changes in response, but would like more direction on a few other items.

MikeMcQuaid

Thanks for the changes here, good progress!

MikeMcQuaid · 2023-10-04T07:34:58Z

package/scripts/postinstall

+  # get last created account UID over 500
+  logged_in_user=$(dscl . list /Users UniqueID | awk '$2 > 500 { print $1 }' | tail -n 1)


4. Specify a user (HOMEBREW_PKG_USER).

This seems the best option here.

Not everyone will agree.

Yup, but that doesn't mean Homebrew needs to offer multiple options 😁

When Homebrew.pkg is installed at the login window with no console user, it's owned by root or another system account.

Yeh, this should be considered a bug. I agree with detecting these cases and handling it, I just think that we should fail if either a user isn't specified or logged-in.

Considering the vast majority of enterprise managed Macs have a 1:1 relationship with an assigned user, it's generally safe to say UID 501 will be the intended owner.

This feels too much like unexpected magic to me. Requiring an explicit user feels like a better fit here.

package/scripts/postinstall

MikeMcQuaid · 2023-10-04T09:48:10Z

@nstrauss I merged #16077 so I've merged that in here to resolve merge conflicts for you. A few related notes:

you changed my mind on sudo so I've removed all of those (plus the one I got you to 😅)
I removed the unnecessary shell out if HOMEBREW_PKG_USER is passed

Feels like the next step, as discussed would be to:

check for HOMEBREW_PKG_USER or a logged-in user in Distribution.xml or preinstall (depending on what can get the nicest user experience on failure)

MikeMcQuaid · 2023-10-04T16:03:05Z

Support passing in a HOMEBREW_PKG_USER environment variable for admins to have better control over the target user where required (a lab environment). For example - HOMEBREW_PKG_USER="nathaniel.strauss" installer -pkg Homebrew-4.1.15 -target /

@nstrauss I was testing this out and couldn't seem to get it to work. https://stackoverflow.com/questions/55511165/pass-variables-to-the-installer-on-osx-package-installer makes it look like it may be more involved, unfortunately.

nstrauss · 2023-10-04T17:23:07Z

Support passing in a HOMEBREW_PKG_USER environment variable for admins to have better control over the target user where required (a lab environment). For example - HOMEBREW_PKG_USER="nathaniel.strauss" installer -pkg Homebrew-4.1.15 -target /

@nstrauss I was testing this out and couldn't seem to get it to work. https://stackoverflow.com/questions/55511165/pass-variables-to-the-installer-on-osx-package-installer makes it look like it may be more involved, unfortunately.

Tested this myself and the linked article is correct. No longer working as I thought. May have another idea.

nstrauss · 2023-10-04T17:57:56Z

@MikeMcQuaid Alright, thanks for bearing with me. Here's where we're at.

Permissions improvements to postinstall.
A new preinstall to hard fail early when there is no logged in user.
The HOMEBREW_PKG_USER environment variable doesn't work, and probably won't without hacky workarounds. Going to keep thinking about alternatives.

So slightly dismayed the PR has been hollowed out from its original intent. My main goal is still to get login window installations working. Your preference seems to be to explicitly define a user when one is not logged in. I respect that approach as it means admins can bring their own logic for which user to specify instead of Homebrew being heavily opinionated by shelling out to dscl. I'm trying to think up ways to do so outside the non-functional env variable. Open to ideas there. Is there a canonical place Homebrew stores settings?

I'm still of the opinion falling back to first/last user created UID isn't all that bad, but it does mean Homebrew is taking a position, and with no configurable way to define user could end up with unexpected behavior.

nstrauss · 2023-10-04T21:25:35Z

I think I have a better solution in place. 2c57881 adds support for reading a defined user from /var/tmp/.homebrew_pkg_user.plist. I'm open to changing the location and format as long as it exists outside any Homebrew directory. I chose a property list file since it's a commonly used structured data format in the Mac admin community and easy to read/write with defaults. The workflow would be...

Write /var/tmp/.homebrew_pkg_user.plist with a command like defaults write /var/tmp/.homebrew_pkg_user HOMEBREW_PKG_USER penny. A normal thing to do with a Munki preinstall script or Jamf policy. Allows an admin to include whatever logic they choose for the user.
Install the package.
Optionally clean up /var/tmp/.homebrew_pkg_user.plist

I think this meets both of our requirements. Also fixed a few edge bugs in postinstall for Intel machines.

MikeMcQuaid

Looks good! Couple more tiny nits and I think we're good to go with this. As mentioned before: that will likely be me squashing and cherry-picking this into another non-fork branch for testing the CI job with all secrets etc.

Thanks again @nstrauss!

package/scripts/postinstall

nstrauss · 2023-10-05T18:25:14Z

Looks good! Couple more tiny nits and I think we're good to go with this. As mentioned before: that will likely be me squashing and cherry-picking this into another non-fork branch for testing the CI job with all secrets etc.

Thanks again @nstrauss!

@MikeMcQuaid 🙏Thank you for your help and guidance along the way. I'm happy with where it's at now. Will leave the rest up to you. Otherwise let me know how else I can help.

- Allow specifying user through `/var/tmp/.homebrew_pkg_user.plist` - Improve permission handling - Correctly write API cache - Add `preinstall` script to fail if we can't get a valid user Co-authored-by: Mike McQuaid <[email protected]>

MikeMcQuaid · 2023-10-06T11:29:47Z

Thanks so much for your first contribution to Homebrew! Without people like you submitting PRs we couldn't run this project. You rock, @nstrauss!

MikeMcQuaid reviewed Oct 3, 2023

View reviewed changes

nstrauss requested a review from MikeMcQuaid October 3, 2023 20:18

MikeMcQuaid reviewed Oct 4, 2023

View reviewed changes

MikeMcQuaid mentioned this pull request Oct 4, 2023

Improve building Homebrew*.pkg #16077

Merged

nstrauss requested a review from MikeMcQuaid October 4, 2023 21:25

MikeMcQuaid reviewed Oct 5, 2023

View reviewed changes

package/scripts/postinstall Outdated Show resolved Hide resolved

package/scripts/postinstall Show resolved Hide resolved

MikeMcQuaid force-pushed the mac_enterprise_pkg branch from 8560827 to e0ac545 Compare October 6, 2023 10:53

MikeMcQuaid mentioned this pull request Oct 6, 2023

Improve Mac package for enterprise install scenarios #16094

Merged

MikeMcQuaid approved these changes Oct 6, 2023

View reviewed changes

MikeMcQuaid merged commit 7a2425f into Homebrew:master Oct 6, 2023
57 checks passed

github-actions bot added the outdated PR was locked due to age label Nov 6, 2023

github-actions bot locked as resolved and limited conversation to collaborators Nov 6, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve Mac package for enterprise install scenarios #16073

Improve Mac package for enterprise install scenarios #16073

nstrauss commented Oct 2, 2023 •

edited

Loading

MikeMcQuaid left a comment

MikeMcQuaid commented Oct 3, 2023

MikeMcQuaid commented Oct 3, 2023

nstrauss commented Oct 3, 2023

MikeMcQuaid left a comment

MikeMcQuaid Oct 4, 2023

MikeMcQuaid commented Oct 4, 2023

MikeMcQuaid commented Oct 4, 2023

nstrauss commented Oct 4, 2023

nstrauss commented Oct 4, 2023

nstrauss commented Oct 4, 2023

MikeMcQuaid left a comment

nstrauss commented Oct 5, 2023

MikeMcQuaid commented Oct 6, 2023

		# get last created account UID over 500
		logged_in_user=$(dscl . list /Users UniqueID \| awk '$2 > 500 { print $1 }' \| tail -n 1)

Improve Mac package for enterprise install scenarios #16073

Improve Mac package for enterprise install scenarios #16073

Conversation

nstrauss commented Oct 2, 2023 • edited Loading

MikeMcQuaid left a comment

Choose a reason for hiding this comment

MikeMcQuaid commented Oct 3, 2023

MikeMcQuaid commented Oct 3, 2023

nstrauss commented Oct 3, 2023

MikeMcQuaid left a comment

Choose a reason for hiding this comment

MikeMcQuaid Oct 4, 2023

Choose a reason for hiding this comment

MikeMcQuaid commented Oct 4, 2023

MikeMcQuaid commented Oct 4, 2023

nstrauss commented Oct 4, 2023

nstrauss commented Oct 4, 2023

nstrauss commented Oct 4, 2023

MikeMcQuaid left a comment

Choose a reason for hiding this comment

nstrauss commented Oct 5, 2023

MikeMcQuaid commented Oct 6, 2023

nstrauss commented Oct 2, 2023 •

edited

Loading