MSSQL Server Instance Custom Name not inventory

[jcervantes-sipecom]

Bug reporting acknowledgment

Yes, I read it

Professional support

Still not applicable

Describe the bug

When inventory MSSQL Server Instance, the agent just inventory when the instance name is "MSSQLSERVER", if the instance has a custom name, or there is more than one instance with different names, the agent doesn't inventory it.

To reproduce

1. Install 2 SQL Server and change the instance name to a new custom instance name, anyone instead MSSQLSERVER.
2. Execute an Inventory
3. Search in /front/computer.form.php > "Database Instances" the missing instance names.

Expected behavior

That the agent will inventory any SQL Server instance without matter any custom instance name, or other than MSSQLSERVER.

Operating system

Windows

GLPI Agent version

1.8

GLPI version

10.0.15

GLPIInventory plugin or other plugin version

GLPI Inventory v1.3.5

Additional context

No response

[g-bougard]

Hi @jcervantes-sipecom

by default, for MSSQL, glpi-agent tries to run sqlcmd with few SQL commands to inventory locally installed instance.

Here you seem to have installed more than one instance. I really don't know if there's a way to detect more than one instance is installed. Have you some well-known command or a full process to detect installed instances ?

Also you should know you can use databaseinventory GLPI Plugin to specify credentials so you can tell the agent where to find databases.

[jcervantes-sipecom]

Hi @jcervantes-sipecom

by default, for MSSQL, glpi-agent tries to run sqlcmd with few SQL commands to inventory locally installed instance.

Here you seem to have installed more than one instance. I really don't know if there's a way to detect more than one instance is installed. Have you some well-known command or a full process to detect installed instances ?

Also you should know you can use databaseinventory GLPI Plugin to specify credentials so you can tell the agent where to find databases.

Hi @g-bougard thanks for your response, actually I just know by powershell, not with perl:

PS C:\Windows\system32> $SQLInstances = Invoke-Command  {
>>  (Get-ItemProperty 'HKLM:\SOFTWARE\Microsoft\Microsoft SQL Server').InstalledInstances
>>  }
>>     foreach ($sql in $SQLInstances) {
>>        [PSCustomObject]@{
>>            ServerName = $sql.PSComputerName
>>            InstanceName = $sql
>>        }
>>        }
>>

ServerName                                                  InstanceName
----------                                                  ------------
                                                            MSSQLSERVER
                                                            MSSQLSERVER_2017

I try to check and debug in perl but I don't know how to do, so I don't know exactly where and how the service is checking or inventory the SQL Instance.

I will check the plugin you said.

[jcervantes-sipecom]

Once knowing the instance name the sqlcmd can login:

I just find this:

In this file, but nothing about auto discovery instance name:

Also I tried to use the plugin you suggested but it can't work, I review the tutorial but nothing, I think maybe it's a bug but I cannot report it, it supposed that in "use":[ ]should be the params credentials for the plugin but it's empty:

But I already has configured the Task, Credential, and Computer:

But it works with sqlcmd:

[g-bougard]

Hi @jcervantes-sipecom

so the question now is to know how to discover installed instance. For that, I need a command or registry datas. In your PS snippet, I see HKLM:\SOFTWARE\Microsoft\Microsoft SQL Server. Does this mean SQL Server instance are registered there ? If yes, can you provide an export of that registry key ?

[jcervantes-sipecom]

Hi @g-bougard, actually I don't know nothing about perl, I'm not a programmer/developer, I know a bit about coding, but I have read and investigated this morning I got something that retrieving from Regedit/Registry Windows the Values of SQL Instances Installed:

#use Data::Dumper;
use Win32::TieRegistry;

my $key = new Win32::TieRegistry
  'HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Microsoft SQL Server\Instance Names\SQL',
  { Access=>Win32::TieRegistry::KEY_READ()|0x0100 };

my @valnames = $key->ValueNames();
#print Dumper @valnames;

foreach my $sqlinstance (@valnames) {
    print "$sqlinstance \n";
}

But I don't know how to add it in the PM files of the agent to get SQL Instaces invetoried.

But I guess in some way could be here something like:

[jcervantes-sipecom]

Hi, I did these modifications in MSSQL.pm:

And this works! At least for me, because I don't know how this can conflict with the ForEach Credentials so you developers can test it:

The MSSQL.pm in ZIP: MSSQL.zip

Or in TXT: MSSQL.pm.txt

[g-bougard]

Hi @jcervantes-sipecom

thank for your sharing, but I can't take it by itself as it is windows OS specific and this module is not OS dependent (it supports MSSQL on linux). I can add specific code for the windows case to discover instance and this is why I requested a registry export. With an export (use regedit for that), I can create a dedicated unittest. If you find sensible datas in that export, you can remove them, I think the most important data is the instance names, but sometime more datas can help to find issues solution.

Be sure I'll reuse your work and credit your efforts as you managed to obtain the expected result.

[keguira]

Hi @jcervantes-sipecom

thank for your sharing, but I can't take it by itself as it is windows OS specific and this module is not OS dependent (it supports MSSQL on linux). I can add specific code for the windows case to discover instance and this is why I requested a registry export. With an export (use regedit for that), I can create a dedicated unittest. If you find sensible datas in that export, you can remove them, I think the most important data is the instance names, but sometime more datas can help to find issues solution.

Be sure I'll reuse your work and credit your efforts as you managed to obtain the expected result.

Multiple instances at the same time on a single server is not possible on linux (you have to use containers) such as, indeed, this feature will be Windows specific. I do not have this king of use-case, on my servers I think today but I can provide you with a single-instance registry export. I will see if I can create a temp server with multiple instances and with different versions of sql server.

single-instance.reg.txt

[g-bougard]

Hi @keguira

thank you for your submission.

With @jcervantes-sipecom work and you sharing, I'm confident the databaseinventory plugin could be used to add credentials for each instance to inventory. For that, I think you just need to select 'login_password' type and set socket to .\MSSQLSERVER for the default instance without login and password. And then add other credentials with the instance name in place of "MSSQLSERVER". This should reproduce the same command that @jcervantes-sipecom uses in his code. And you need to set as much credentials as required for a server.

This is a way of doing, but you need to know which instances you want to inventory and of course this is better to discover available instances.

So inspired by @jcervantes-sipecom code, I think the following module should do the job: MSSQL.pm.zip

@jcervantes-sipecom can you test it ? @keguira can you also test it at least in single instance case so we are sure it doesn't break the current default logic ?

In my version, I choose to add credentials for instances other than the default one with a private _discovered_instance type to setup the command as you do. Here is the diff of my version:

diff --git a/lib/GLPI/Agent/Task/Inventory/Generic/Databases/MSSQL.pm b/lib/GLPI/Agent/Task/Inventory/Generic/Databases/MSSQL.pm
index 5b0924ddb..2bd394131 100644
--- a/lib/GLPI/Agent/Task/Inventory/Generic/Databases/MSSQL.pm
+++ b/lib/GLPI/Agent/Task/Inventory/Generic/Databases/MSSQL.pm
@@ -5,6 +5,8 @@ use English qw(-no_match_vars);
 use strict;
 use warnings;

+use UNIVERSAL::require;
+
 use parent 'GLPI::Agent::Task::Inventory::Generic::Databases';

 use GLPI::Agent::Tools;
@@ -39,8 +41,27 @@ sub _getDatabaseService {
     my $credentials = delete $params{credentials};
     return [] unless $credentials && ref($credentials) eq 'ARRAY';

-    # Add SQLExpress default credential when trying default credential
+    # Handle default credentials case
     if (@{$credentials} == 1 && !keys(%{$credentials->[0]})) {
+        # On windows, we can discover instance names in registry
+        if (OSNAME eq 'MSWin32') {
+            GLPI::Agent::Tools::Win32->require();
+            my $instances = getRegistryKey(
+                path => 'HKEY_LOCAL_MACHINE/SOFTWARE/Microsoft/Microsoft SQL Server/Instance Names/SQL',
+            );
+            foreach my $key (%{$instances}) {
+                # Only consider valuename keys
+                my ($instance) = $key =~ m{^/(.+)$}
+                    or next;
+                # Default credentials will still match MSSQLSERVER instance
+                next if $instance eq 'MSSQLSERVER';
+                push @{$credentials}, {
+                    type        => "_discovered_instance",
+                    instance    => $instance,
+                };
+            }
+        }
+        # Add SQLExpress default credential when trying default credential
         push @{$credentials}, {
             type    => "login_password",
             socket  => "localhost\\SQLExpress",
@@ -197,6 +218,8 @@ sub _mssqlOptions {
             $credential->{password} =~ s/"/\\"/g;
             $options .= ' -P "'.$credential->{password}.'"' ;
         }
+    } elsif ($credential->{type} eq "_discovered_instance" && $credential->{instance}) {
+        $options .= " -S .\\$credential->{instance}" ;
     }

     return $options;

[jcervantes-sipecom]

Hi @g-bougard so sorry for my misunderstood, I guess you were asking me for the code to export the registry, I didn't get it that you were ask for the Regedit Key export, my bad.

This is a registry key that contains 3 instances names, I don't know which format do you need so I'll zip in some formats:

InstancesNamesSQL.zip

And yes you were right @g-bougard I know the code is for Windows specific, I guess this not work for linux, so thank you for search a solution that works for both.

Something you should know is, if the Instance name is: MSSQLSERVER the sqlcmd cannot connect in this way: sqlcmd -l 5 -S .\MSSQLSERVER -X1 -t 30 -K ReadOnly -r1 -W -h -1 -s ";" -Q "SELECT SERVERPROPERTY('productversion')", at least in my case.

That's way I did a condition if the instance name is MSSQLSERVER so try to connect without the instance name.

[jcervantes-sipecom]

Hi @keguira

thank you for your submission.

With @jcervantes-sipecom work and you sharing, I'm confident the databaseinventory plugin could be used to add credentials for each instance to inventory. For that, I think you just need to select 'login_password' type and set socket to .\MSSQLSERVER for the default instance without login and password. And then add other credentials with the instance name in place of "MSSQLSERVER". This should reproduce the same command that @jcervantes-sipecom uses in his code. And you need to set as much credentials as required for a server.

This is a way of doing, but you need to know which instances you want to inventory and of course this is better to discover available instances.

So inspired by @jcervantes-sipecom code, I think the following module should do the job: MSSQL.pm.zip

@jcervantes-sipecom can you test it ? @keguira can you also test it at least in single instance case so we are sure it doesn't break the current default logic ?

In my version, I choose to add credentials for instances other than the default one with a private _discovered_instance type to setup the command as you do. Here is the diff of my version:

diff --git a/lib/GLPI/Agent/Task/Inventory/Generic/Databases/MSSQL.pm b/lib/GLPI/Agent/Task/Inventory/Generic/Databases/MSSQL.pm
index 5b0924ddb..2bd394131 100644
--- a/lib/GLPI/Agent/Task/Inventory/Generic/Databases/MSSQL.pm
+++ b/lib/GLPI/Agent/Task/Inventory/Generic/Databases/MSSQL.pm
@@ -5,6 +5,8 @@ use English qw(-no_match_vars);
 use strict;
 use warnings;

+use UNIVERSAL::require;
+
 use parent 'GLPI::Agent::Task::Inventory::Generic::Databases';

 use GLPI::Agent::Tools;
@@ -39,8 +41,27 @@ sub _getDatabaseService {
     my $credentials = delete $params{credentials};
     return [] unless $credentials && ref($credentials) eq 'ARRAY';

-    # Add SQLExpress default credential when trying default credential
+    # Handle default credentials case
     if (@{$credentials} == 1 && !keys(%{$credentials->[0]})) {
+        # On windows, we can discover instance names in registry
+        if (OSNAME eq 'MSWin32') {
+            GLPI::Agent::Tools::Win32->require();
+            my $instances = getRegistryKey(
+                path => 'HKEY_LOCAL_MACHINE/SOFTWARE/Microsoft/Microsoft SQL Server/Instance Names/SQL',
+            );
+            foreach my $key (%{$instances}) {
+                # Only consider valuename keys
+                my ($instance) = $key =~ m{^/(.+)$}
+                    or next;
+                # Default credentials will still match MSSQLSERVER instance
+                next if $instance eq 'MSSQLSERVER';
+                push @{$credentials}, {
+                    type        => "_discovered_instance",
+                    instance    => $instance,
+                };
+            }
+        }
+        # Add SQLExpress default credential when trying default credential
         push @{$credentials}, {
             type    => "login_password",
             socket  => "localhost\\SQLExpress",
@@ -197,6 +218,8 @@ sub _mssqlOptions {
             $credential->{password} =~ s/"/\\"/g;
             $options .= ' -P "'.$credential->{password}.'"' ;
         }
+    } elsif ($credential->{type} eq "_discovered_instance" && $credential->{instance}) {
+        $options .= " -S .\\$credential->{instance}" ;
     }

     return $options;

Sorry I didn't see you last response, sure I will test it in a Windows with multiple instances.

[g-bougard]

No problem, the reg export is perfect.

So I see 3 entries in that case, MSSQLSERVER2016, MSSQLSERVER2017 & MSSQLSERVER2019. With my code, agent will try default credentials without the -S option and 3 others credentials with -S .\MSSQLSERVER2016, -S .\MSSQLSERVER2017 & -S .\MSSQLSERVER2019. Is this correct ? Or does one of the instances will still answer to the default credentials ?

For the database-inventory plugin, so a login_password credentials without anything set should reproduce the default credentials on the server.

[jcervantes-sipecom]

@g-bougard, I replaced the file but the code is not scanning the SQL Instances, it's not executing the sqlcmd:

I tried with 2 computers, one with 2 SQL Instance: MSSQLSERVER, MSSQLSERVER_2017, and the other one with MSSQLSERVER2016, MSSQLSERVER2017, MSSQLSERVER2019, and in a GLPI instance without the plugin DatabaseInventory or is imperative that the plugin should be active?

[g-bougard]

Sorry, I forgot to include the GLPI::Agent::Tools::Win32:: prefix onto the getRegistryKey() call.

Here is a fixed version that should work: MSSQL.pm-2.zip

[jcervantes-sipecom]

Thank you for your help @g-bougard, now the agent is inventory all the SQL instances installed:

I thought to open a new ticket because the size of some databases were not inventorying well:

But I found that is necessary that in SQL Security logins, the user NT AUTHORITY\SYSTEM has permission to all databases in the instances, after that the size are well inventorying:

[g-bougard]

Thank you for this sharing.

So in the case with no MSSQLSERVER instance in registry, no instance is answering on default credentials.

I will merge my code today so next nightly build will include it.

[g-bougard]

Next nightly build will include the update.