adferrand / docker-backuppc

Docker container with BackupPC version 4.x/3.x based on Alpine distribution.
MIT License
72 stars 48 forks source link

BackupPC breaks after using webinterface to modify configuration on backuppc:4.4.0-10 #96

Open anders-larsson opened 10 months ago

anders-larsson commented 10 months ago

Hi,

Updated to backuppc:4.4.0-10 when it was release and everything looked fine. However yesterday after removing a host in the web interface (Edit Hosts -> Delete on host) the web interfaces throws the following error:

Software error:

Not an ARRAY reference at /usr/local/BackupPC/lib/BackupPC/CGI/Lib.pm line 468.

For help, please send mail to this site's webmaster, giving this error message and the time and date of the error. 

It appears to be because of https://github.com/backuppc/backuppc/issues/466. This issue has been fixed in the master branch of BackupPC but no release has been made for it. I was able to get it working again by comparing the config.pl.pre-4.0.0 and the most recent config.pl and changing () to either [] or {} as it was previously configured as mentioned in https://github.com/backuppc/backuppc/issues/472.

Made a patch out of https://github.com/backuppc/backuppc/commit/2c9270b9b849b2c86ae6301dd722c97757bc9256 and modified entrypoint.sh to apply the patch (needed to install patch too) and included both files in the container and it seemed to fix the issue. Not sure if this is the correct way to fix it though.

I should add that this might be a problem in releases before 4.4.0-10 as well. It was introduced in Data::Dumper versions >= 2.182.

dth202 commented 10 months ago

@anders-larsson

to avoid re-inventing the wheel, could you possibly share more details on how you made the patch? I would like to do the same until the issue has been added to the latest release.

Did you create a script to pull in the new files or just download the files and have a copy command install them?

anders-larsson commented 10 months ago

First of all I would suggest you make a copy of config.pl to ensure you have a sane copy to go back to if something goes wrong.

I maintain my own container image based on adferreand/backuppc with a few extra tools (gnupg and fcron) and added the fixes to it1.

Steps (let's assume we're modifying adferrand/backuppc source directly)

Hope it helps.

Patch:

diff --git a/configure.pl b/configure.pl
index 6826ebc..d5deef5 100755
--- a/configure.pl
+++ b/configure.pl
@@ -668,7 +668,7 @@ if ( defined($Conf{CgiUserConfigEdit}) ) {
           if ( defined($Conf{CgiUserConfigEdit}{$p}) );
     }
     $Conf{CgiUserConfigEdit} = $new;
-    my $d = Data::Dumper->new([$new], [*value]);
+    my $d = Data::Dumper->new([$new]);
     $d->Indent(1);
     $d->Terse(1);
     $d->Sortkeys(1);
diff --git a/lib/BackupPC/Storage/Text.pm b/lib/BackupPC/Storage/Text.pm
index e9df664..09fcb24 100644
--- a/lib/BackupPC/Storage/Text.pm
+++ b/lib/BackupPC/Storage/Text.pm
@@ -422,7 +422,7 @@ sub ConfigFileMerge
                 my $var = $1;
                 $skipExpr = "\$fakeVar = $2\n";
                 if ( exists($newConf->{$var}) ) {
-                    my $d = Data::Dumper->new([$newConf->{$var}], [*value]);
+                    my $d = Data::Dumper->new([$newConf->{$var}]);
                     $d->Indent(1);
                     $d->Terse(1);
                     $d->Sortkeys(1);
@@ -454,7 +454,7 @@ sub ConfigFileMerge
     #
     foreach my $var ( sort(keys(%$newConf)) ) {
         next if ( $done->{$var} );
-        my $d = Data::Dumper->new([$newConf->{$var}], [*value]);
+        my $d = Data::Dumper->new([$newConf->{$var}]);
         $d->Indent(1);
         $d->Terse(1);
         $d->Sortkeys(1);

Patch for entrypoint.sh:

index e9f5939..b6262f2 100755
--- a/files/entrypoint.sh
+++ b/files/entrypoint.sh
@@ -43,6 +43,8 @@ if [ -f /firstrun ]; then
        tar xf "BackupPC-$BACKUPPC_VERSION.tar.gz"
        cd "/root/BackupPC-$BACKUPPC_VERSION"

+       patch -p1 < /root/datadumper.patch
+
        # Configure WEB UI access
        configure_admin=""
        if [ ! -f /etc/backuppc/htpasswd ]; then

Patch for Dockerfile:

diff --git a/Dockerfile b/Dockerfile
index ee81e60..fed5bd6 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -21,7 +21,7 @@ RUN apk --no-cache --update add \
  && apk --no-cache --update -X http://dl-cdn.alpinelinux.org/alpine/edge/community add par2cmdline \
 # Install backuppc build dependencies
  && apk --no-cache --update --virtual build-dependencies add \
-        gcc g++ autoconf automake make git perl-dev acl-dev curl \
+        gcc g++ autoconf automake make git perl-dev acl-dev curl patch \
 # Compile and install BackupPC:XS
  && git clone https://github.com/backuppc/backuppc-xs.git /root/backuppc-xs --branch $BACKUPPC_XS_VERSION \
  && cd /root/backuppc-xs \
@@ -45,6 +45,7 @@ RUN apk --no-cache --update add \
  && rm -rf /root/backuppc-xs /root/rsync-bpc /root/par2cmdline \
  && apk del build-dependencies

+COPY files/datadumper.patch /root/datadumper.patch
 COPY files/lighttpd.conf /etc/lighttpd/lighttpd.conf
 COPY files/auth.conf /etc/lighttpd/auth.conf
 COPY files/auth-ldap.conf /etc/lighttpd/auth-ldap.conf
diff --git a/files/entrypoint.sh b/files/entrypoint.sh
index e9f5939..7b7b4b8 100755
--- a/files/entrypoint.sh
+++ b/files/entrypoint.sh
@@ -43,6 +43,8 @@ if [ -f /firstrun ]; then
        tar xf "BackupPC-$BACKUPPC_VERSION.tar.gz"
        cd "/root/BackupPC-$BACKUPPC_VERSION"

+       patch -p1 < /root/datadumper.patch
+
        # Configure WEB UI access
        configure_admin=""
        if [ ! -f /etc/backuppc/htpasswd ]; then
@@ -120,7 +122,7 @@ if [ -f /firstrun ]; then
        fi

        # Clean
-       rm -rf "/root/BackupPC-$BACKUPPC_VERSION.tar.gz" "/root/BackupPC-$BACKUPPC_VERSION" /firstrun
+       rm -rf "/root/BackupPC-$BACKUPPC_VERSION.tar.gz" "/root/BackupPC-$BACKUPPC_VERSION" /firstrun /root/datadumper.patch
 fi

 export BACKUPPC_UUID
adferrand commented 10 months ago

Hello guys ! I forgot to share the update here. I applied this patch with the release 4.4.0-11.

However someone encountered an issue with this new version, and I think it is related to this patch: #97.

Do you encountered any issue on your side ? I am not fluent in perl, so I would gladly accept propositions to fix the problem if it is confirmed that this patch is the issue. I will try on my side the migration path from 4.4.0-10.

As a side note, I really need to add integration tests to validate migration paths ;)

anders-larsson commented 10 months ago

I've replied to the #97. It's not because of the patch but because the patch was missing and some kind of modification was made to BackupPC's configuration while the patch was not applied and it "corrupted" the config file. https://github.com/backuppc/backuppc/issues/472 was mentioned in the initial comment and that is what happened in #97.