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.

Sign up for GitHub

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

Jump to bottom

⭐️ support Windows 2025 on arm via SSH #4808

Merged
merged 1 commit into from
Nov 5, 2024
Merged

⭐️ support Windows 2025 on arm via SSH #4808

merged 1 commit into from
Nov 5, 2024

Conversation

chris-rock
Copy link
Member 

cnquery shell ssh [email protected] --ask-pass
→ connected to Windows Server 2025 Datacenter
  ___ _ __   __ _ _   _  ___ _ __ _   _ 
 / __| '_ \ / _` | | | |/ _ \ '__| | | |
| (__| | | | (_| | |_| |  __/ |  | |_| |
 \___|_| |_|\__, |\__,_|\___|_|   \__, |
  mondoo™      |_|                |___/  interactive shell

cnquery> asset { platform version build labels arch  }
asset: {
  build: "5000"
  version: "26311"
  arch: "ARM64"
  labels: {
    windows.mondoo.com/display-version: "24H2"
    windows.mondoo.com/product-type: "3"
  }
  platform: "windows"
}
cnquery>

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Nov 2, 2024

Test Results

3 135 tests   3 134 ✅  1m 26s ⏱️
  371 suites      1 💤
   28 files        0 ❌

Results for commit 8d7b70c.

philipbalinov
philipbalinov reviewed Nov 4, 2024
View reviewed changes
providers/os/detector/testdata/detect-windows2019.toml
@@ -3,11 +3,16 @@ stdout = """Node,BootDevice,BuildNumber,BuildType,Caption,CodeSet,CountryCode,Cr
VAGRANT,\\Device\\HarddiskVolume1,17763,Multiprocessor Free,Microsoft Windows Server 2019 Datacenter Evaluation,1252,1,Win32_OperatingSystem,Win32_ComputerSystem,,VAGRANT,-420,TRUE,TRUE,TRUE,3,FALSE,,FALSE,256,2,721716,979372,1922780,20190906065515.000000-420,,20190908011749.580533-420,20190908042731.608000-420,0409,Microsoft Corporation,4294967295,137438953344,{en-US},Microsoft Windows Server 2019 Datacenter Evaluation|C:\\Windows|\\Device\\Harddisk0\\Partition2,0,69,1,80,Vagrant,64-bit,1033,400,18,,,,,FALSE,TRUE,3,,00431-20000-00000-AA838,0,0,1179648,OK,400,\\Device\\HarddiskVolume2,C:\\Windows\\system32,C:,,3276340,2096692,10.0.17763,C:\\Windows
"""

[commands."powershell -c \"Get-ItemProperty -Path 'HKLM:\\SOFTWARE\\Microsoft\\Windows NT\\CurrentVersion' -Name CurrentBuild, UBR, EditionID | ConvertTo-Json\""]
[commands."55dbc0e9b838caa11145eed07f6e73644bda27bf65a0c58a52291f9a18384481"]
Copy link

Choose a reason for hiding this comment

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

why are we replacing the verbose subsection name with a sha?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because this is a longer script now, we use the script hash.

preslavgerchev
preslavgerchev approved these changes Nov 5, 2024
View reviewed changes
providers/os/detector/detector_win.go
var productType string
switch current.ProductType {
case "WinNT":
productType = "1" // Workstation
Copy link
Contributor

Choose a reason for hiding this comment

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

is there any reason why we use the digits here and not Workstation, Server, etc?

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 think it makes sense to change this, but this way it is compatible to the current implementation. We need to check and adjust policies like https://github.com/mondoohq/cnspec-policies/blob/main/core/mondoo-windows-11-compatibility.mql.yaml#L22-L23 to use a new label based approach.

Copy link
Contributor

Choose a reason for hiding this comment

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

Same question, specially since PS command returns a string as a product type and here we are using a number as a string? 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's do this with #4815

providers/os/detector/windows/build_version.go
pscommand := "Get-ItemProperty -Path 'HKLM:\\SOFTWARE\\Microsoft\\Windows NT\\CurrentVersion' -Name CurrentBuild, UBR, EditionID | ConvertTo-Json"
cmd, err := conn.RunCommand(powershell.Wrap(pscommand))
pscommand := `
$info = Get-ItemProperty -Path 'HKLM:\SYSTEM\CurrentControlSet\Control\ProductOptions' -Name ProductType
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: why not use our existing way of fetching registry keys to build this? the only thing thats extra here seems to be the processor architecture which can be a separate smaller pwsh cmd to fetch

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 was just adjusting the current implementation with minimal change. I agree, that we should use the new registry key implementation for this. Since I have no time to do this, I recommend we do this in a follow up PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's do this with #4815

afiune
afiune reviewed Nov 5, 2024
View reviewed changes
providers/os/detector/detector_win.go

// try to get build + ubr number (win 10+, 2019+)
current, err := win.GetWindowsOSBuild(conn)
if err == nil && current.UBR > 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be nice to log this error even though we are falling back to wmi

Copy link
Member Author

Choose a reason for hiding this comment

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

Feel free to add this to the PR.

providers/os/detector/detector_win.go
var productType string
switch current.ProductType {
case "WinNT":
productType = "1" // Workstation
Copy link
Contributor

Choose a reason for hiding this comment

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

Same question, specially since PS command returns a string as a product type and here we are using a number as a string? 🤔

@afiune afiune mentioned this pull request Nov 5, 2024
Open
@preslavgerchev preslavgerchev merged commit df77016 into main Nov 5, 2024
15 checks passed
@preslavgerchev preslavgerchev deleted the chris-rock/windows-2025-arm branch November 5, 2024 09:02
@github-actions github-actions bot locked and limited conversation to collaborators Nov 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@afiune afiune afiune left review comments

@philipbalinov philipbalinov philipbalinov left review comments

@preslavgerchev preslavgerchev preslavgerchev approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@chris-rock @philipbalinov @afiune @preslavgerchev