[chore] Migrate MSI build to WiX v6 #6861
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6861 +/- ##
==========================================
+ Coverage 38.00% 38.01% +0.01%
==========================================
Files 367 367
Lines 25711 25711
==========================================
+ Hits 9771 9774 +3
+ Misses 15127 15125 -2
+ Partials 813 812 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
pjanotti
changed the title
There was a problem hiding this comment.
Pull Request Overview
This PR migrates the MSI build system from WiX Toolset v3.14 to WiX v6, modernizing the Windows installer infrastructure.
Key changes:
- Updated WiX XML schema from v3 to v4 format with attribute name changes (BinaryKey → BinaryRef, Condition moved to attributes)
- Replaced WiX installation from Chocolatey to .NET tool via dotnet-tools.json
- Simplified build process by consolidating candle/light commands into single
dotnet wix buildcommand
Reviewed Changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/msi/msi_test.go | Added UTF-16 encoding support for MSI log files to improve error output readability |
| tests/go.mod | Promoted golang.org/x/text from indirect to direct dependency for UTF-16 encoding |
| packaging/msi/splunk-otel-collector.wxs | Migrated WiX schema from v3 to v4 with updated element structure and attribute names |
| packaging/msi/msi-builder/build.sh | Replaced candle/light commands with unified dotnet wix build command |
| packaging/msi/build.sh | Updated WiX version verification from v3.14.0 to v6.x |
| packaging/msi/SplunkCustomActions/src/SplunkCustomActions.csproj | Replaced WiX v3.14 package with WixToolset.Dtf.CustomAction v6.0.2 |
| packaging/msi/SplunkCustomActions/src/CustomActions.cs | Updated namespace from Microsoft.Deployment.WindowsInstaller to WixToolset.Dtf.WindowsInstaller |
| packaging/msi/SplunkCustomActions/src/CustomAction.config | Fixed formatting (removed extra space in startup tag) |
| dotnet-tools.json | Added WiX v6.0.0 as .NET tool |
| .gitlab-ci.yml | Simplified CI by consolidating custom action jobs and using dotnet tool restore |
| .github/workflows/win-package-test.yml | Added tests/msi/** to trigger paths |
| .github/workflows/reusable-msi-custom-actions.yml | Removed WiX 3.14 Chocolatey installation steps, simplified packaging |
| .github/workflows/reusable-msi-build.yml | Replaced Chocolatey WiX installation with dotnet tool restore |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 12 out of 13 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 12 out of 13 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <ComponentRef Id="RegistryComponent" /> | ||
| </Feature> | ||
| <ComponentRef Id="RegistryComponent"/> | ||
| <ComponentRef Id="JmxMetricsJarComponent"/> | ||
| <!-- list of config files auto-generated by heat at build time --> | ||
| <ComponentGroupRef Id="ConfigFiles"/> | ||
| <ComponentRef Id="JmxMetricsJarComponent" /> |
There was a problem hiding this comment.
The RegistryComponent is referenced twice in the same feature (lines 94 and 96). This is redundant and could cause issues. Remove one of the duplicate ComponentRef entries.
| t.Logf("Install command: %s", installCmd.SysProcAttr.CmdLine) | ||
| err := installCmd.Run() | ||
| if err != nil { |
There was a problem hiding this comment.
[nitpick] The log command has been moved before the error check, which means it will execute even when installation succeeds. Consider keeping the log statement after the error check to avoid unnecessary logging in successful cases, or add a separate log statement for successful installations.
| t.Logf("Install command: %s", installCmd.SysProcAttr.CmdLine) | |
| err := installCmd.Run() | |
| if err != nil { | |
| err := installCmd.Run() | |
| if err != nil { | |
| t.Logf("Install command: %s", installCmd.SysProcAttr.CmdLine) |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 13 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <MultiStringValue Value="SPLUNK_ACCESS_TOKEN=[SPLUNK_ACCESS_TOKEN]" /> | ||
| <MultiStringValue Value="SPLUNK_API_URL=[SPLUNK_API_URL]" /> | ||
| <MultiStringValue Value="SPLUNK_BUNDLE_DIR=[SPLUNK_BUNDLE_DIR]" /> | ||
| <MultiStringValue Value="SPLUNK_CONFIG=[SPLUNK_CONFIG]" /> | ||
| <MultiStringValue Value="SPLUNK_HEC_TOKEN=[SPLUNK_HEC_TOKEN]" /> |
There was a problem hiding this comment.
Sensitive access tokens are written in cleartext into the registry under the service Environment values (SPLUNK_ACCESS_TOKEN and SPLUNK_HEC_TOKEN). Local users or processes with registry read access can retrieve these tokens and misuse them to send data or impersonate the collector. Store tokens in a protected secure store (e.g., DPAPI-protected secrets, Windows Credential Manager, or a file with strict ACLs) and avoid placing them in HKLM; if environment variables are required, derive them at runtime from a protected location rather than persisting secrets in the registry.
| <MultiStringValue Value="SPLUNK_ACCESS_TOKEN=[SPLUNK_ACCESS_TOKEN]" /> | |
| <MultiStringValue Value="SPLUNK_API_URL=[SPLUNK_API_URL]" /> | |
| <MultiStringValue Value="SPLUNK_BUNDLE_DIR=[SPLUNK_BUNDLE_DIR]" /> | |
| <MultiStringValue Value="SPLUNK_CONFIG=[SPLUNK_CONFIG]" /> | |
| <MultiStringValue Value="SPLUNK_HEC_TOKEN=[SPLUNK_HEC_TOKEN]" /> | |
| <MultiStringValue Value="SPLUNK_API_URL=[SPLUNK_API_URL]" /> | |
| <MultiStringValue Value="SPLUNK_BUNDLE_DIR=[SPLUNK_BUNDLE_DIR]" /> | |
| <MultiStringValue Value="SPLUNK_CONFIG=[SPLUNK_CONFIG]" /> |
1 participant
Migrating to the latest WiX toolset, main changes:
.wxsfile to match the latest.Collateral: