Skip to content

Fixed macOS MSI package -- using local wine and wix#16307

Merged
getvictor merged 16 commits intomainfrom
victor/15463-msi-on-mac
Jan 30, 2024
Merged

Fixed macOS MSI package -- using local wine and wix#16307
getvictor merged 16 commits intomainfrom
victor/15463-msi-on-mac

Conversation

@getvictor
Copy link
Copy Markdown
Member

@getvictor getvictor commented Jan 23, 2024

New flow for fleetctl --package --type=msi on macOS using arm64 processor (M1, M2, etc.)

PR for docs: #16459

Checklist for submitter

If some of the following don't apply, delete the relevant line.

  • Changes file added for user-visible changes in changes/ or orbit/changes/.
    See Changes files for more information.
  • Manual QA for all new/changed functionality

@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 23, 2024

Codecov Report

Attention: 123 lines in your changes are missing coverage. Please review.

Comparison is base (576405b) 65.46% compared to head (079499f) 65.53%.
Report is 65 commits behind head on main.

Files Patch % Lines
orbit/pkg/packaging/windows.go 25.60% 78 Missing and 15 partials ⚠️
orbit/pkg/packaging/wix/wix.go 0.00% 27 Missing ⚠️
cmd/fleetctl/package.go 40.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #16307      +/-   ##
==========================================
+ Coverage   65.46%   65.53%   +0.07%     
==========================================
  Files        1124     1127       +3     
  Lines       97575    98765    +1190     
  Branches     2413     2413              
==========================================
+ Hits        63875    64729     +854     
- Misses      28918    29167     +249     
- Partials     4782     4869      +87     
Flag Coverage Δ
backend 66.58% <22.15%> (+0.06%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@getvictor getvictor changed the title Added another way to build MSI on macOS, using --local-wix-dir Fixed macOS MSI package -- using local wine and wix Jan 24, 2024
@getvictor getvictor marked this pull request as ready for review January 24, 2024 17:48
@getvictor getvictor requested a review from a team as a code owner January 24, 2024 17:48
@getvictor
Copy link
Copy Markdown
Member Author

@noahtalerman FYI.

The new flow for fleetctl --package --type=msi on macOS using arm64 processors (M1, M2, etc.) will require the user to install wine. If they do not have it, the error message will point them to our install script on main. Currently, the script is only this branch: https://github.com/fleetdm/fleet/blob/victor/15463-msi-on-mac/orbit/tools/build/install-wine-macos.sh

The change is that this flow will no longer use docker, and will use a local build flow.

Copy link
Copy Markdown
Contributor

@mostlikelee mostlikelee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some initial questions, primarily for Product:

  • is it reasonable to ask customers to install homebrew, wget, and wine?

Comment thread cmd/fleetctl/package.go
Comment thread orbit/pkg/packaging/windows.go Outdated
Comment thread orbit/pkg/packaging/wix/wix.go
Comment thread orbit/tools/build/install-wine-macos.sh
Comment thread orbit/pkg/packaging/windows.go
Comment thread orbit/pkg/packaging/windows.go Outdated
Comment thread orbit/pkg/packaging/windows.go Outdated
Comment thread orbit/pkg/packaging/windows.go Outdated
Comment thread orbit/pkg/packaging/windows.go Outdated
Comment thread changes/15463-msi-on-mac Outdated
Copy link
Copy Markdown
Member

@lucasmrod lucasmrod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, left a couple of comments.

Comment on lines +471 to +476
for _, archiveReader := range zipReader.File {
err = extractZipFile(archiveReader, destPath)
if err != nil {
return err
}
}

Check failure

Code scanning / CodeQL

Arbitrary file access during archive extraction ("Zip Slip")

Unsanitized archive entry, which may contain '..', is used in a [file system operation](1). Unsanitized archive entry, which may contain '..', is used in a [file system operation](2). Unsanitized archive entry, which may contain '..', is used in a [file system operation](3).
@noahtalerman
Copy link
Copy Markdown
Member

is it reasonable to ask customers to install homebrew, wget, and wine?

@getvictor and @mostlikelee what are the required tools today v. what they will be with this change?

@getvictor is this PR also a fix for "fleetctl on Apple Silicon" (#9047) ? If so, what are the required tools v. what they will be after this change?

Trying to get a feel for whether this is a step in the right direction: fewer dependencies = better UX

@lucasmrod
Copy link
Copy Markdown
Member

We may need similar docs here:

### Generating Windows installers using local WiX toolset
`Applies only to Fleet Premium`
When creating a fleetd installer for Windows hosts (**.msi**) on a Windows machine, you can tell `fleetctl package` to
use local installations of the 3 WiX v3 binaries used by this command (`heat.exe`, `candle.exe`, and
`light.exe`) instead of those in a pre-configured container, which is the default behavior. To do
so:
1. Install the WiX v3 binaries. To install, you can download them
[here](https://github.com/wixtoolset/wix3/releases/download/wix3112rtm/wix311-binaries.zip), then unzip the downloaded file.
2. Find the absolute filepath of the directory containing your local WiX v3 binaries. This will be wherever you saved the unzipped package contents.
3. Run `fleetctl package`, and pass the absolute path above as the string argument to the
`--local-wix-dir` flag. For example:
```
fleetctl package --type msi --fleet-url=[YOUR FLEET URL] --enroll-secret=[YOUR ENROLLMENT SECRET] --local-wix-dir "\Users\me\AppData\Local\Temp\wix311-binaries"
```
If the provided path doesn't contain all 3 binaries, the command will fail.

@getvictor
Copy link
Copy Markdown
Member Author

@noahtalerman The current requirement is to have docker installed. The new requirement is to have wine installed. We need this fix, since the current flow fails half the time, and always prints error messages such as:

=================================================================
	Native Crash Reporting
=================================================================
Got a UNKNOWN while executing native code. This usually indicates
a fatal error in the mono runtime or one of the native libraries
used by your application.

The fix for #9047 has already been merged into main.

Comment thread orbit/pkg/packaging/windows.go Outdated
Co-authored-by: Noah Talerman <47070608+noahtalerman@users.noreply.github.com>
@getvictor
Copy link
Copy Markdown
Member Author

@lucasmrod Please re-review. I met with Noah to review this PR and made some changes.

Comment thread orbit/pkg/packaging/windows.go
Copy link
Copy Markdown
Member

@lucasmrod lucasmrod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Left a small comment around checking the response status.

Also, should we add docs here?
https://github.com/fleetdm/fleet/blob/main/docs/Using%20Fleet/enroll-hosts.md?plain=1#L297-L313

@getvictor
Copy link
Copy Markdown
Member Author

Looks good. Left a small comment around checking the response status.

Also, should we add docs here? https://github.com/fleetdm/fleet/blob/main/docs/Using%20Fleet/enroll-hosts.md?plain=1#L297-L313

Docs added in this PR: #16459

@getvictor getvictor merged commit ed7ab1e into main Jan 30, 2024
@getvictor getvictor deleted the victor/15463-msi-on-mac branch January 30, 2024 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants