fix: Cleanup server url in log and server vardir with automatic migra…

…tion

Changes

@@ -24,6 +24,7 @@ core:
 * Refacto: Add Logger api to register a callback permitting to collect journaling events
 * Fix ToolBox job events support
 * Support ssl-keystore=system-ssl-ca on MacOSX to force using system known public CAs
+* Cleanup server url in log and server vardir with automatic migration
 
 inventory:
 * Fix rare windows perl error during drives, ipv6 network or videos inventory

lib/GLPI/Agent/Storage.pm

@@ -5,6 +5,7 @@ use warnings;
 
 use Config;
 use English qw(-no_match_vars);
+use File::Find;
 use File::Path qw(mkpath);
 use File::stat;
 use Storable;
@@ -29,6 +30,11 @@ sub new {
             mkpath($params{directory});
         };
         die "Can't create $params{directory}: $EVAL_ERROR" if $EVAL_ERROR;
+
+        # Migrate files from oldvardir if exists
+        if ($params{oldvardir} && -d $params{oldvardir}) {
+            _migrateVarDir($params{oldvardir}, $params{directory});
+        }
     }
 
     if (! -w $params{directory} && !$params{read_only}) {
@@ -47,6 +53,40 @@ sub new {
     return $self;
 }
 
+# Migrate vardir content tree
+sub _migrateVarDir {
+    my ($from, $to) = @_;
+
+    return unless $from && -d $from && $to && -d $to;
+
+    my $path_offset = length($from);
+    my @deletedir = ($from);
+
+    File::Find::find(
+        {
+            wanted => sub {
+                if (-l) {
+                    unlink $_;
+                    return;
+                }
+                return if $_ eq $from;
+                my $dest = $to.substr($File::Find::name, $path_offset);
+                if (-d) {
+                    mkdir $dest unless -d $dest;
+                    unshift @deletedir, $_;
+                } else {
+                    rename $_, $dest;
+                }
+            },
+            no_chdir => 1,
+        },
+        $from
+    );
+
+    # Recursively delete old dirs
+    map { rmdir $_ } @deletedir;
+}
+
 sub getDirectory {
     my ($self) = @_;
 

lib/GLPI/Agent/Target.pm

@@ -51,6 +51,7 @@ sub _init {
 
     $self->{storage} = GLPI::Agent::Storage->new(
         logger    => $self->{logger},
+        oldvardir => $params{oldvardir} // "",
         directory => $params{vardir}
     );
 

lib/GLPI/Agent/Target/Server.pm

@@ -22,13 +22,25 @@ sub new {
     $self->{url} = _getCanonicalURL($params{url});
 
     # compute storage subdirectory from url
-    my $subdir = $self->{url};
+    my $url = ref($self->{url}) eq "URI" ? $self->{url}->clone : URI->new($self->{url});
+    $url->userinfo(undef);
+    my $subdir = $url->as_string;
     $subdir =~ s/\//_/g;
     $subdir =~ s/:/../g if $OSNAME eq 'MSWin32';
+    # Remove any trailing underscore
+    $subdir =~ s/_+$//;
+
+    # Provide oldvardir to eventually migrate it to newer clean version
+    my $oldvardir = ref($self->{url}) eq "URI" ? $self->{url}->as_string : $self->{url};
+    $oldvardir =~ s/\//_/g;
+    $oldvardir =~ s/:/../g if $OSNAME eq 'MSWin32';
+    # But leave it empty if not changed
+    $oldvardir = "" if $subdir eq $oldvardir;
 
     $self->_init(
-        id     => 'server' . $count++,
-        vardir => $params{basevardir} . '/' . $subdir
+        id        => 'server' . $count++,
+        vardir    => $params{basevardir} . '/' . $subdir,
+        oldvardir => $oldvardir ? $params{basevardir} . '/' . $oldvardir : ""
     );
 
     return $self;
@@ -74,7 +86,10 @@ sub getUrl {
 sub getName {
     my ($self) = @_;
 
-    return $self->{url};
+    my $url = ref($self->{url}) eq "URI" ? $self->{url}->clone : URI->new($self->{url});
+    $url->userinfo(undef);
+
+    return $url->as_string;
 }
 
 sub getType {

t/agent/http/server/proxy.t

@@ -320,7 +320,7 @@ subtest "failing to pass inventory to server" => sub {
     check_error(500, "Inventory not sent to server0");
 };
 
-$glpi->{url} = "http://glpi-project.test/glpi?test=sent";
+$glpi->{url} = URI->new("http://glpi-project.test/glpi?test=sent");
 _request();
 subtest "send inventory to server" => sub {
     check_error(200, { REPLY => "" }, "Inventory sent to server0", "xml");
@@ -363,7 +363,7 @@ subtest "Supported xml PROLOG query" => sub {
 
 # Same request but with a server set
 $proxy->config("only_local_store", 0);
-$glpi->{url} = "http://glpi-project.test/glpi";
+$glpi->{url} = URI->new("http://glpi-project.test/glpi");
 $glpi->isGlpiServer(1);
 $agent->{targets} = [ $glpi ];
 _request();
@@ -433,15 +433,15 @@ SKIP: {
 }
 
 $proxy->config("only_local_store", 0);
-$glpi->{url} = "http://glpi-project.test/glpi?test=noserver";
+$glpi->{url} = URI->new("http://glpi-project.test/glpi?test=noserver");
 _request();
 subtest "JSON inventory pending request but ko" => sub {
     check_error(202, { status => "pending", expiration => "10s" }, "JSON inventory action stored", "json");
 };
 like(shift @events, qr/^PROXYREQ,[0-9A-F]{8},.*"status":"pending"/, "Pending inventory event");
 like(shift @events, qr/^PROXYREQ,[0-9A-F]{8},.*"message":"server0 forward failure"/, "Pending inventory event not sent");
 
-$glpi->{url} = "http://glpi-project.test/glpi?test=sent";
+$glpi->{url} = URI->new("http://glpi-project.test/glpi?test=sent");
 _request();
 subtest "JSON inventory pending request and ok" => sub {
     check_error(202, { status => "pending", expiration => "10s" }, "JSON inventory action stored", "json");

t/agent/target.t

@@ -37,8 +37,8 @@ lives_ok {
 } 'instanciation: ok';
 
 my $storage_dir = $OSNAME eq 'MSWin32' ?
-    "$basevardir/http..__my.domain.tld_" :
-    "$basevardir/http:__my.domain.tld_" ;
+    "$basevardir/http..__my.domain.tld" :
+    "$basevardir/http:__my.domain.tld" ;
 ok(-d $storage_dir, "storage directory creation");
 is($target->{id}, 'server0', "identifier");