WIP: Improve Windows (Server) version detection #7743

dnsmichi · 2020-01-08T13:37:19Z

Rationale: I was looking into additional Windows environment specifics and thought it would be a good idea to evaluate this again. With the recent VS compiler update, we can also detected Windows 10 (yay), available since the VS2015 SDK.

Detecting the servers is actually fun with the build numbers, especially to distinguish between Server 2016 and 2019.

If this PR works out, @widhalmt should be even happier.

Resources

dnsmichi · 2020-02-07T15:09:52Z

This PR is needed to improve the output in #7819.

This is required as otherwise IsWindows10OrGreater() still returns false. https://docs.microsoft.com/en-us/windows/win32/sysinfo/targeting-your-application-at-windows-8-1

dnsmichi · 2020-02-10T14:20:22Z

IsWindows10OrGreater() still doesn't work, since we do not target that specifically. And even with the set target, it doesn't work.

https://docs.microsoft.com/en-us/windows/win32/sysinfo/operating-system-version

For applications that have been manifested for Windows 8.1 or Windows 10. Applications not manifested for Windows 8.1 or Windows 10 will return the Windows 8 OS version value (6.2). To manifest your applications for Windows 8.1 or Windows 10, refer to Targeting your application for Windows.

https://docs.microsoft.com/en-us/windows/win32/sysinfo/targeting-your-application-at-windows-8-1

In Windows 8.1 and Windows 10, the GetVersion and GetVersionEx functions have been deprecated. In Windows 10, the VerifyVersionInfo function has also been deprecated. While you can still call the deprecated functions, if your application does not specifically target Windows 8.1 or Windows 10, the functions will return the Windows 8 version (6.2).

So we need an app.manifest which is compiled into the final icinga2.exe. I'm playing around with that.

julianbrost · 2021-09-09T10:34:00Z

So if I understand correctly, Icinga at the moment displays some imprecise version on any recent version of Windows? If so, I think it might be worth looking into this. But I can't believe that there's no way to just get some version string in Windows that we could use.

Al2Klimov · 2023-01-20T11:17:53Z

Any news here?

julianbrost · 2023-01-20T12:54:31Z

Not from my side. Wanted to have a look at it but you know, there's always something more important to do.

Al2Klimov · 2023-01-30T14:54:10Z

I can't believe that there's no way to just get some version string in Windows that we could use.

https://learn.microsoft.com/de-de/windows/win32/sysinfo/getting-the-system-version?redirectedfrom=MSDN

Al2Klimov

This BREAKS CMake < v3.4 on Windows. At least this works at all.

cla-bot · 2023-02-01T15:56:27Z

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Michael Friedrich.
This is most likely caused by a git client misconfiguration; please make sure to:

check if your git client is configured with an email to sign commits git config --list | grep email
If not, set it up using git config --global user.email [email protected]
Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

Al2Klimov

Much better!

PS C:\Users\aklimov\icinga2> C:\Users\aklimov\icinga2\build\Bin\Release\Debug\icinga2.exe --version
(...)
System information:
  Platform: Windows
  Platform version: 10 or greater
  Kernel: Windows
  Kernel version: 10.0
(...)

julianbrost · 2023-02-01T16:08:41Z

Is there a list of potentially breaking changes between these versions? They probably don't emulate the old version just for the purpose of making it harder for you to figure out the version you're actually running on.

Al2Klimov · 2023-02-01T16:10:10Z

Good question. Very good question.

Al2Klimov · 2023-02-01T16:10:26Z

CC @LordHepipud

Al2Klimov · 2023-02-01T16:11:34Z

Shouldn't this basically just be a question of compiles/runs yes/no?

Al2Klimov · 2023-02-01T16:39:39Z

At least it works for me.

LordHepipud · 2023-02-14T16:13:43Z

Packages is working on Windows 2012 R2. Output is also correct now:

System Information:
  Platforn: Windows
  Platform version: Server 2012 R2
  Kernel: Windows
  Kernel version: 6.3-9600
  Architecture: x86_64

julianbrost · 2023-02-15T11:05:33Z

Is there a list of potentially breaking changes between these versions?

Answering my own question:

I haven't looked through this in detail, but there seems to be nothing that will obviously be a problem.

julianbrost

There is also a commit in this PR where the commit message is just "WIP".

julianbrost · 2023-02-15T09:28:35Z

lib/base/utility.cpp

+				*platformVersion = "Server 2008 (EOL)";
+			}
+
+		} else {
+			if (IsWindows10OrGreater())
+				*platformVersion = "10 or greater";
+			else if (IsWindows8Point1OrGreater())
+				*platformVersion = "8.1";
+			else if (IsWindows8OrGreater())
+				*platformVersion = "8";
+			else if (IsWindows7SP1OrGreater())
+				*platformVersion = "7 SP1 (EOL)";
+			else if (IsWindows7OrGreater())
+				*platformVersion = "7 (EOL)";
+			else
+				*platformVersion = "Vista (EOL)";


I would not put any EOL information in here. It's just a maintenance burden. It already seems to be outdated.

julianbrost · 2023-02-15T09:29:42Z

icinga-app/app.manifest

@@ -0,0 +1,19 @@
+<?xml version="1.0" encoding="utf-8"?>
+<asmv1:assembly manifestVersion="1.0" xmlns="urn:schemas-microsoft-com:asm.v1" xmlns:asmv1="urn:schemas-microsoft-com:asm.v1" xmlns:asmv2="urn:schemas-microsoft-com:asm.v2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
+  <assemblyIdentity version="1.0.0.0" name="MyApplication.app"/>


MyApplication.app looks like a placeholder. Shouldn't this be replaced with something more meaningful?

…name

Al2Klimov · 2023-02-15T11:26:42Z

It indeed was work in progress and now I've finished it.

julianbrost · 2023-02-16T09:44:37Z

lib/base/utility.cpp

+		// https://stackoverflow.com/questions/53393150/c-how-to-detect-windows-server-2019
+		// https://techthoughts.info/windows-version-numbers/
+		if (IsWindowsServer()) {
+			// 2019 Server +
+			if (IsWindowsVersionOrGreater(10, 0, 1803)) {
+				*platformVersion = "Server 2019 or greater";
+			// 2016 Server
+			} else if (IsWindowsVersionOrGreater(10, 0, 1607)) {
+				*platformVersion = "Server 2016";
+			// 2012 R2
+			} else if (IsWindowsVersionOrGreater(6, 3, 0)) {
+				*platformVersion = "Server 2012 R2";
+			// 2012
+			} else if (IsWindowsVersionOrGreater(6, 2, 0)) {
+				*platformVersion = "Server 2012";
+			} else {
+				*platformVersion = "Server 2008";
+			}


What versions besides 2012 R2 was this tested on? According to the documentation of IsWindowsVersionOrGreater, the third parameter is named wServicePackMajor and to me it's unclear if that would refer to the version number like 1607 or a build number like 14393 or something completely different like 2 as in SP2.

So this should be tested on two 10.0 version, i.e. 2016 and 2019 to ensure that this parameter isn't totally wrong.

julianbrost · 2023-02-16T09:52:15Z

lib/base/utility.cpp

+			if (IsWindowsVersionOrGreater(10, 0, 1803)) {
+				*platformVersion = "Server 2019 or greater";


The source given in the comment right above lists 1803 twice, for both a 2016 and a 2019 version. On the other hand, Wikipedia lists 1809 as the first 2019 version.

According to the Microsoft documentation, the check functions for desktop versions can be used in conjunction with IsWindowsServer():

For example, a call to IsWindowsXPSP3OrGreater will return true on Windows Server 2008. Applications that need to distinguish between server and client versions of Windows should call IsWindowsServer.

Would this be good enough for detecting the server release on the granularity we need? Or is there a version that we couldn't distinguish that way? Would at least save us from dealing with the magic numbers involved in this super-intuitive versioning scheme.

julianbrost · 2023-02-16T10:02:44Z

icinga-app/app.manifest

@@ -0,0 +1,19 @@
+<?xml version="1.0" encoding="utf-8"?>
+<asmv1:assembly manifestVersion="1.0" xmlns="urn:schemas-microsoft-com:asm.v1" xmlns:asmv1="urn:schemas-microsoft-com:asm.v1" xmlns:asmv2="urn:schemas-microsoft-com:asm.v2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
+  <assemblyIdentity version="1.0.0.0" name="Icinga 2"/>


Documentation for name:

Uniquely names the application or assembly. Use the following format for the name: Organization.Division.Name. For example Microsoft.Windows.mysampleApp. Required.

So doesn't look like a name for human consumption but rather like a scoped identifier. Also not sure if you're supposed to use spaces in there. There's also an example with a 2 level name, so my suggestion for the value would be Icinga.Icinga2.

dnsmichi added area/cli Command line helpers area/windows Windows agent and plugins labels Jan 8, 2020

CLI: Improve Windows (Server) version detection

ff135b6

dnsmichi force-pushed the feature/windows-version-detection branch from 4e16e5b to ff135b6 Compare January 8, 2020 16:38

dnsmichi mentioned this pull request Jan 14, 2020

Make OS facts available in the icinga check #7751

Open

Michael Friedrich added 2 commits February 10, 2020 13:39

Windows: Specifically target 8, 8.1, 10

604ce59

This is required as otherwise IsWindows10OrGreater() still returns false. https://docs.microsoft.com/en-us/windows/win32/sysinfo/targeting-your-application-at-windows-8-1

Windows/CMake: Add app.manifest into icinga2.exe

a2b7c69

dnsmichi changed the title ~~CLI: Improve Windows (Server) version detection~~ WIP: CLI: Improve Windows (Server) version detection Feb 10, 2020

dnsmichi changed the title ~~WIP: CLI: Improve Windows (Server) version detection~~ WIP: Improve Windows (Server) version detection Feb 10, 2020

dnsmichi self-assigned this Feb 10, 2020

WIP

f6b8069

Al2Klimov marked this pull request as draft June 29, 2020 10:08

Al2Klimov added this to the 2.13.0 milestone Nov 23, 2020

julianbrost removed this from the 2.13.0 milestone May 31, 2021

julianbrost assigned julianbrost and unassigned dnsmichi Sep 9, 2021

julianbrost removed their assignment Jan 20, 2023

Al2Klimov requested changes Jan 30, 2023

View reviewed changes

Let CMake handle icinga-app/app.manifest by itself

40619fb

This BREAKS CMake < v3.4 on Windows. At least this works at all.

Al2Klimov added the cla/signed label Feb 1, 2023

Al2Klimov previously approved these changes Feb 1, 2023

View reviewed changes

Al2Klimov marked this pull request as ready for review February 1, 2023 15:58

Al2Klimov requested a review from julianbrost February 15, 2023 09:08

julianbrost requested changes Feb 15, 2023

View reviewed changes

Al2Klimov added 2 commits February 15, 2023 12:15

icinga2 --version: don't tell whether a Windows version is EOL

eeb972a

icinga-app/app.manifest: replace placeholder with actual application …

be0419c

…name

Al2Klimov dismissed their stale review via be0419c February 15, 2023 11:25

cla-bot bot removed the cla/signed label Feb 15, 2023

Icinga deleted a comment from cla-bot bot Feb 15, 2023

Al2Klimov added the cla/signed label Feb 15, 2023

Al2Klimov requested a review from julianbrost February 15, 2023 11:26

julianbrost reviewed Feb 16, 2023

View reviewed changes

Al2Klimov mentioned this pull request Feb 17, 2023

Utility::GetPlatform*(): on Windows query the registry for version info #9699

Open

2 tasks

Al2Klimov self-assigned this Feb 17, 2023

Al2Klimov marked this pull request as draft February 17, 2023 12:30

		if (IsWindowsVersionOrGreater(10, 0, 1803)) {
		*platformVersion = "Server 2019 or greater";

WIP: Improve Windows (Server) version detection #7743

Are you sure you want to change the base?

WIP: Improve Windows (Server) version detection #7743

Uh oh!

Conversation

dnsmichi commented Jan 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Resources

Uh oh!

dnsmichi commented Feb 7, 2020

Uh oh!

dnsmichi commented Feb 10, 2020

Uh oh!

julianbrost commented Sep 9, 2021

Uh oh!

Al2Klimov commented Jan 20, 2023

Uh oh!

julianbrost commented Jan 20, 2023

Uh oh!

Al2Klimov commented Jan 30, 2023

Uh oh!

Al2Klimov left a comment

Choose a reason for hiding this comment

Uh oh!

cla-bot bot commented Feb 1, 2023

Uh oh!

Al2Klimov left a comment

Choose a reason for hiding this comment

Uh oh!

julianbrost commented Feb 1, 2023

Uh oh!

Al2Klimov commented Feb 1, 2023

Uh oh!

Al2Klimov commented Feb 1, 2023

Uh oh!

Al2Klimov commented Feb 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Al2Klimov commented Feb 1, 2023

Uh oh!

LordHepipud commented Feb 14, 2023

Uh oh!

julianbrost commented Feb 15, 2023

Uh oh!

julianbrost left a comment

Choose a reason for hiding this comment

Uh oh!

julianbrost Feb 15, 2023

Choose a reason for hiding this comment

Uh oh!

julianbrost Feb 15, 2023

Choose a reason for hiding this comment

Uh oh!

Al2Klimov commented Feb 15, 2023

Uh oh!

julianbrost Feb 16, 2023

Choose a reason for hiding this comment

Uh oh!

julianbrost Feb 16, 2023

Choose a reason for hiding this comment

Uh oh!

julianbrost Feb 16, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dnsmichi commented Jan 8, 2020 •

edited

Loading

Al2Klimov commented Feb 1, 2023 •

edited

Loading