🐛 install location + re-enable arm64 MSI builds #490

philipbalinov · 2024-10-31T13:08:25Z

after enabling arm64 support for the mondoo.MSI, the install location on amd64 suddenly changed to "Program Files (x64)" which breaks the product.

This change aims to pin the installation path on all platforms to "Program Files"

Changes

Fixed amd64 package configuration to install to "Program Files" dir
Added a blocking test step before publishing the package that installs the amd64 package and verifies cnspec/cnquery are operational
- added signature validation for the package
- run cnquery with simple mql rule
- login to edge with cnspec and run basic policy

Testing Done

Manually tested on local arm64 Windows11 VM
Manually tested on windows 2022 ec2 instance
Added automated e2e tests

Notes

This reverts and emergency revert that was done; this is the fix compared to the original commit for arm64 support

chris-rock · 2024-11-01T11:28:53Z

.github/workflows/pkg_msi.yaml

+          if (-not $match) {
+            exit 1
+          }
+      - name: Run a basic cnquery sanity check


Do you mean to test cnspec here?

yes, fixed in #491

chris-rock · 2024-11-01T11:32:20Z

packages/archlinux/.vscode/launch.json

@@ -0,0 +1,17 @@
+{


lets not add /DS_Store files. We should add this one to gitignore to prevent this from future commits.

whoops - added those two patterns to .gitignore

chris-rock

Great improvement @philipbalinov

chris-rock · 2024-11-01T11:33:02Z

.github/workflows/pkg_msi.yaml

-          curl -sL --head --fail https://github.com/mondoohq/cnquery/releases/download/v${{ steps.version.outputs.version }}/cnquery_${{ steps.version.outputs.version }}_windows_amd64.zip
-          curl -sL --head --fail https://github.com/mondoohq/cnspec/releases/download/v${{ steps.version.outputs.version }}/cnspec_${{ steps.version.outputs.version }}_windows_amd64.zip
-
+          curl -sL --head --fail https://github.com/mondoohq/cnquery/releases/download/v${{ steps.version.outputs.version }}/cnquery_${{ steps.version.outputs.version }}_windows_amd64.zip \


nice simplification

chris-rock · 2024-11-01T11:33:10Z

.github/workflows/pkg_msi.yaml

-          curl -sSL -O https://github.com/mondoohq/cnquery/releases/download/v${VERSION}/cnquery_${VERSION}_windows_amd64.zip
-          unzip cnquery_${VERSION}_windows_amd64.zip
-          rm cnquery_${VERSION}_windows_amd64.zip
+          mkdir -p dist/${{ matrix.arch }} && cd dist/${{ matrix.arch }}


I like the matrix here

chris-rock · 2024-11-01T11:33:33Z

.github/workflows/pkg_msi.yaml

+          & 'C:\Program Files\Mondoo\cnquery.exe' run -c "os.base.packages.where(name == 'Mondoo') { name }"
+      - name: Login to edge with cnspec
+        run: |
+          & 'C:\Program Files\Mondoo\cnspec.exe' login -t "${{ secrets.INSTALL_TEST_MONDOO_REGISTRATION_TOKEN }}" --config C:\ProgramData\Mondoo\mondoo.yml


This is great! I think we should also run the logout.

added logout

chris-rock · 2024-11-01T11:35:08Z

packages/msi/msi/Product.wxs

@@ -6,13 +6,23 @@

 <?define UpgradeCodeStandard = "b0fee933-ccd2-467c-8fe4-bb0ac6a099c8" ?>
 <?define UpgradeCodeEnterprise = "4ABDD5C7-E1E1-41A6-8119-DCE65634A6CC" ?>


Not required as this PR but we should remove the enterprise package since that is something we do not do anymore. Essentially we have the same package, just for amd64 and arm64. From what I see in the build pipeline we do not even build enterprise anymore.

just a note on this - we need to at least keep the UpgradeCodeEnterprise and OtherSKU logic, as that protects us from installing the standard package side-by-side with the enterprise one

Great idea.

chris-rock · 2024-11-01T11:35:45Z

packages/msi/msi/Product.wxs

+<?if $(var.arch) = "arm64"?>
+<?define UpgradeCode = "$(var.UpgradeCodeArm64)"?>
+<?define InstallerVersion="500"?>
+<?define ProductName = "Mondoo"?>


Why do we need the product name in the if condition?

moved all common elements out of the conditional block.

chris-rock · 2024-11-01T11:35:59Z

packages/msi/msi/Product.wxs

+<?define UpgradeCodeArm64 = "090cfb7d-c00c-4d36-94fe-f649a4b29c91" ?>
+<?if $(var.arch) = "arm64"?>
+<?define UpgradeCode = "$(var.UpgradeCodeArm64)"?>
+<?define InstallerVersion="500"?>


Can we add comment what installer version 500 / 200 mean?

added comments with the meaning of these versions

chris-rock · 2024-11-01T11:37:52Z

packages/msi/package.ps1

@@ -3,9 +3,12 @@

 # use: ./package.ps1 -version 0.32.0
 param (
-    [string]$version = 'x.xx.x'
+    [string]$version = 'x.xx.x',
+    [string]$arch = 'amd64|arm64'


afiune

Can't test but it looks good to me.

- Revert "Revert "Build arm64 MSI packages additionally to the current amd64 ones (#482)" (#486)" - This reverts commit 843a783. - Revert the `Platform` attribute in the Package definition, as that appears to dictate which install location the package should use i.e. "Program Files" vs "Program Files (x64)" - Some minor improvements

philipbalinov changed the title ~~Philip/fix install location to be "Program Files"~~ Fix install location to be "Program Files" Oct 31, 2024

philipbalinov marked this pull request as ready for review October 31, 2024 13:26

philipbalinov requested a review from chris-rock October 31, 2024 13:53

philipbalinov marked this pull request as draft October 31, 2024 15:19

philipbalinov force-pushed the philip/fix-install-location-to-be-programfiles branch 15 times, most recently from 8b0a8ca to e393b2a Compare October 31, 2024 17:25

philipbalinov changed the title ~~Fix install location to be "Program Files"~~ re-enable arm64 MSI builds + fix install location to be "Program Files" Oct 31, 2024

philipbalinov marked this pull request as ready for review October 31, 2024 17:36

philipbalinov force-pushed the philip/fix-install-location-to-be-programfiles branch 4 times, most recently from d63164d to 8eb0c59 Compare November 1, 2024 09:50

chris-rock reviewed Nov 1, 2024

View reviewed changes

philipbalinov force-pushed the philip/fix-install-location-to-be-programfiles branch from 373d298 to a3382cc Compare November 1, 2024 14:55

philipbalinov requested a review from chris-rock November 4, 2024 09:37

benr approved these changes Nov 5, 2024

View reviewed changes

afiune approved these changes Nov 5, 2024

View reviewed changes

afiune changed the title ~~re-enable arm64 MSI builds + fix install location to be "Program Files"~~ 🐛 install location + re-enable arm64 MSI builds Nov 5, 2024

philipbalinov added 3 commits November 5, 2024 16:32

Fix: add logout step to cnspec test

39d9470

Correct msi filename in test block

a9b0ecd

philipbalinov force-pushed the philip/fix-install-location-to-be-programfiles branch from e3b39e8 to a9b0ecd Compare November 5, 2024 14:35

philipbalinov merged commit 1af15a5 into main Nov 8, 2024
16 checks passed

philipbalinov deleted the philip/fix-install-location-to-be-programfiles branch November 8, 2024 12:38

github-actions bot locked and limited conversation to collaborators Nov 8, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🐛 install location + re-enable arm64 MSI builds #490

🐛 install location + re-enable arm64 MSI builds #490

philipbalinov commented Oct 31, 2024 •

edited

Loading

chris-rock Nov 1, 2024

philipbalinov Nov 1, 2024

chris-rock Nov 1, 2024

philipbalinov Nov 1, 2024

chris-rock left a comment

chris-rock Nov 1, 2024

chris-rock Nov 1, 2024

chris-rock Nov 1, 2024

philipbalinov Nov 1, 2024

chris-rock Nov 1, 2024

philipbalinov Nov 1, 2024 •

edited

Loading

chris-rock Nov 5, 2024

chris-rock Nov 1, 2024

philipbalinov Nov 1, 2024

chris-rock Nov 1, 2024

philipbalinov Nov 1, 2024

chris-rock Nov 1, 2024

afiune left a comment

		@@ -6,13 +6,23 @@

		<?define UpgradeCodeStandard = "b0fee933-ccd2-467c-8fe4-bb0ac6a099c8" ?>
		<?define UpgradeCodeEnterprise = "4ABDD5C7-E1E1-41A6-8119-DCE65634A6CC" ?>

🐛 install location + re-enable arm64 MSI builds #490

🐛 install location + re-enable arm64 MSI builds #490

Conversation

philipbalinov commented Oct 31, 2024 • edited Loading

Changes

Testing Done

Notes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chris-rock left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

philipbalinov Nov 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

afiune left a comment

Choose a reason for hiding this comment

philipbalinov commented Oct 31, 2024 •

edited

Loading

philipbalinov Nov 1, 2024 •

edited

Loading