[PATCH pve-storage v1 50/54] fix #2884: support nested subdir scanning for 'snippets' vtype

["Max R. Carrara" <m.carrara@proxmox.com>]

Add support for parsing nested subdirectories for the 'snippets'
volume type by adapting its corresponding regex used by parsing
helpers.

Add additional test cases wherever applicable to account for nested
subdirectories for 'snippets' volumes, as was done for the 'iso' and
'vztmpl' volume types.

Originally-by: Noel Ullreich <n.ullreich@proxmox.com>
Signed-off-by: Max R. Carrara <m.carrara@proxmox.com>
---
 src/PVE/Storage/Common/Parse.pm             |  1 +
 src/PVE/Storage/Common/test/parser_tests.pl | 52 ++++++++++++++
 src/PVE/Storage/Plugin.pm                   |  4 +-
 src/test/filesystem_path_test.pm            | 16 +++++
 src/test/list_volumes_test.pm               | 78 +++++++++++++++++++++
 src/test/parse_volname_test.pm              | 52 ++++++++++++++
 src/test/path_to_volume_id_test.pm          |  7 ++
 7 files changed, 208 insertions(+), 2 deletions(-)

diff --git a/src/PVE/Storage/Common/Parse.pm b/src/PVE/Storage/Common/Parse.pm
index 92e5b0fd..55fbc129 100644
--- a/src/PVE/Storage/Common/Parse.pm
+++ b/src/PVE/Storage/Common/Parse.pm
@@ -126,6 +126,7 @@ my $RE_BACKUP_FILE_PATH = qr!

 my $RE_SNIPPETS_FILE_PATH = qr!
     (?<path>
+        (?<dir> $RE_DIRECTORY_COMPONENTS )?
         (?<file> [^/]+ )
     )
 !xn;
diff --git a/src/PVE/Storage/Common/test/parser_tests.pl b/src/PVE/Storage/Common/test/parser_tests.pl
index c80d0d90..38d5ebac 100755
--- a/src/PVE/Storage/Common/test/parser_tests.pl
+++ b/src/PVE/Storage/Common/test/parser_tests.pl
@@ -498,6 +498,57 @@ my $volname_cases_snippets_valid = [
             volname => 'snippets/userconfig.yaml',
         },
     },
+
+    # subdirectories
+    {
+        path => 'subdir/hookscript.pl',
+        expected => {
+            file => 'hookscript.pl',
+            dir => 'subdir',
+            'disk-path' => 'subdir/hookscript.pl',
+            path => 'subdir/hookscript.pl',
+            vtype => 'snippets',
+            volname => 'snippets/subdir/hookscript.pl',
+        },
+    },
+    {
+        path => 'deeply/nested/subdir/hookscript.pl',
+        expected => {
+            file => 'hookscript.pl',
+            dir => 'deeply/nested/subdir',
+            'disk-path' => 'deeply/nested/subdir/hookscript.pl',
+            path => 'deeply/nested/subdir/hookscript.pl',
+            vtype => 'snippets',
+            volname => 'snippets/deeply/nested/subdir/hookscript.pl',
+        },
+    },
+];
+
+my $volname_cases_snippets_invalid = [
+    {
+        description => "Parent dir reference in path (beginning) (snippets)",
+        args => {
+            path => '../hookscript.pl',
+            vtype => 'snippets',
+        },
+        expected => undef,
+    },
+    {
+        description => "Parent dir reference in path (middle) (snippets)",
+        args => {
+            path => 'subdir/../hookscript.pl',
+            vtype => 'snippets',
+        },
+        expected => undef,
+    },
+    {
+        description => "Parent dir reference in path (end) (snippets)",
+        args => {
+            path => 'subdir/hookscript.pl/..',
+            vtype => 'snippets',
+        },
+        expected => undef,
+    },
 ];

 my $volname_cases_import_valid = [
@@ -597,6 +648,7 @@ my $cases_invalid_all = [
     $volname_cases_iso_invalid,
     $volname_cases_vztmpl_invalid,
     $volname_cases_backup_invalid,
+    $volname_cases_snippets_invalid,
     $volname_cases_import_invalid,
 ];

diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
index 2df56e76..3a3e1b62 100644
--- a/src/PVE/Storage/Plugin.pm
+++ b/src/PVE/Storage/Plugin.pm
@@ -235,7 +235,7 @@ my $defaultData = {
         },
         'max-scan-depth' => {
             description => "Maximum depth of subdirectories to traverse when searching for"
-                . " ISOs and container templates in directories.",
+                . " ISOs, container templates and snippets in directories.",
             type => 'integer',
             default => 0,
             minimum => 0,
@@ -1831,7 +1831,7 @@ sub list_volumes {
         }

         if ($type eq 'snippets') {
-            return get_subdir_files($storeid, $scfg, 'snippets', undef);
+            return get_subdir_files($storeid, $scfg, 'snippets', undef, $depth);
         }

         if ($type eq 'import') {
diff --git a/src/test/filesystem_path_test.pm b/src/test/filesystem_path_test.pm
index b5c1ab33..ff6dffe1 100644
--- a/src/test/filesystem_path_test.pm
+++ b/src/test/filesystem_path_test.pm
@@ -82,6 +82,22 @@ my $tests = [
             'backup',
         ],
     },
+    {
+        volname => 'snippets/userconfig.yaml',
+        snapname => undef,
+        expected => ["$DEFAULT_STORAGE_DIR/snippets/userconfig.yaml", undef, 'snippets'],
+    },
+    {
+        volname => 'snippets/hookscript.pl',
+        snapname => undef,
+        expected => ["$DEFAULT_STORAGE_DIR/snippets/hookscript.pl", undef, 'snippets'],
+    },
+    {
+        volname => 'snippets/foo/bar/baz/something.txt',
+        snapname => undef,
+        expected =>
+            ["$DEFAULT_STORAGE_DIR/snippets/foo/bar/baz/something.txt", undef, 'snippets'],
+    },
 ];

 my sub run_tests($tests) {
diff --git a/src/test/list_volumes_test.pm b/src/test/list_volumes_test.pm
index 6bdfa37c..edcf1ba1 100644
--- a/src/test/list_volumes_test.pm
+++ b/src/test/list_volumes_test.pm
@@ -1340,6 +1340,24 @@ my $test_param_list = [
                     "$DEFAULT_STORAGE_PATH/template/cache/1/2/3/4/5/some-lxc-template.tar.gz",
                 expected => undef,
             },
+            {
+                file => "$DEFAULT_STORAGE_PATH/snippets/some-hookscript.pl",
+                expected => {
+                    content => 'snippets',
+                    ctime => $DEFAULT_CTIME,
+                    format => 'snippet',
+                    size => $DEFAULT_SIZE,
+                    volid => 'local:snippets/some-hookscript.pl',
+                },
+            },
+            {
+                file => "$DEFAULT_STORAGE_PATH/snippets/1/some-hookscript.pl",
+                expected => undef,
+            },
+            {
+                file => "$DEFAULT_STORAGE_PATH/snippets/1/2/3/4/5/some-hookscript.pl",
+                expected => undef,
+            },
         ],
     },
     {
@@ -1410,6 +1428,31 @@ my $test_param_list = [
                 file => "$DEFAULT_STORAGE_PATH/template/cache/1/2/some-lxc-template.tar.gz",
                 expected => undef,
             },
+            {
+                file => "$DEFAULT_STORAGE_PATH/snippets/some-hookscript.pl",
+                expected => {
+                    content => 'snippets',
+                    ctime => $DEFAULT_CTIME,
+                    format => 'snippet',
+                    size => $DEFAULT_SIZE,
+                    volid => 'local:snippets/some-hookscript.pl',
+                },
+            },
+            {
+                file => "$DEFAULT_STORAGE_PATH/snippets/1/some-hookscript.pl",
+                expected => {
+                    content => 'snippets',
+                    ctime => $DEFAULT_CTIME,
+                    format => 'snippet',
+                    size => $DEFAULT_SIZE,
+                    volid => 'local:snippets/1/some-hookscript.pl',
+                },
+            },
+            {
+                # Exceeds max-scan-depth
+                file => "$DEFAULT_STORAGE_PATH/snippets/1/2/some-hookscript.pl",
+                expected => undef,
+            },
         ],
     },
     {
@@ -1502,6 +1545,41 @@ my $test_param_list = [
                     "$DEFAULT_STORAGE_PATH/template/cache/1/2/3/4/5/6/some-lxc-template.tar.gz",
                 expected => undef,
             },
+            {
+                file => "$DEFAULT_STORAGE_PATH/snippets/some-hookscript.pl",
+                expected => {
+                    content => 'snippets',
+                    ctime => $DEFAULT_CTIME,
+                    format => 'snippet',
+                    size => $DEFAULT_SIZE,
+                    volid => 'local:snippets/some-hookscript.pl',
+                },
+            },
+            {
+                file => "$DEFAULT_STORAGE_PATH/snippets/1/some-hookscript.pl",
+                expected => {
+                    content => 'snippets',
+                    ctime => $DEFAULT_CTIME,
+                    format => 'snippet',
+                    size => $DEFAULT_SIZE,
+                    volid => 'local:snippets/1/some-hookscript.pl',
+                },
+            },
+            {
+                file => "$DEFAULT_STORAGE_PATH/snippets/1/2/3/4/5/some-hookscript.pl",
+                expected => {
+                    content => 'snippets',
+                    ctime => $DEFAULT_CTIME,
+                    format => 'snippet',
+                    size => $DEFAULT_SIZE,
+                    volid => 'local:snippets/1/2/3/4/5/some-hookscript.pl',
+                },
+            },
+            {
+                # Exceeds max-scan-depth
+                file => "$DEFAULT_STORAGE_PATH/snippets/1/2/3/4/5/6/some-hookscript.pl",
+                expected => undef,
+            },
         ],
     },
 ];
diff --git a/src/test/parse_volname_test.pm b/src/test/parse_volname_test.pm
index b90815c2..0a425ee2 100644
--- a/src/test/parse_volname_test.pm
+++ b/src/test/parse_volname_test.pm
@@ -372,10 +372,62 @@ my $tests = [
                 volname => "snippets/$file_name",
                 expected => ['snippets', $file_name, undef, undef, undef, undef, 'raw'],
             },
+            {
+                description => "Snippets, $file_name, subdirectory",
+                volname => "snippets/foo/$file_name",
+                expected => ['snippets', "foo/$file_name", undef, undef, undef, undef, 'raw'],
+            },
+            {
+                description => "Snippets, $file_name, nested subdirectories",
+                volname => "snippets/foo/bar/baz/$file_name",
+                expected =>
+                    ['snippets', "foo/bar/baz/$file_name", undef, undef, undef, undef, 'raw'],
+            },
+            {
+                description => "Snippets, $file_name, subdirectory with same name as file",
+                volname => "snippets/$file_name/$file_name",
+                expected =>
+                    ['snippets', "$file_name/$file_name", undef, undef, undef, undef, 'raw'],
+            },
         );

         push($tests->@*, @extra_tests);
     }
+
+    # Failed tests
+    {
+        my $file_name = "some-file.txt";
+
+        my @extra_failed_tests = (
+            {
+                description =>
+                    "Snippets, $file_name, parent directory reference before volume type prefix",
+                volname => "../snippets/$file_name",
+                expected => "unable to parse directory volume name '../snippets/$file_name'\n",
+            },
+            {
+                description =>
+                    "Snippets, $file_name, parent directory reference at beginning of volume path",
+                volname => "snippets/../$file_name",
+                expected => "unable to parse directory volume name 'snippets/../$file_name'\n",
+            },
+            {
+                description =>
+                    "Snippets, $file_name, parent directory reference at end of volume path",
+                volname => "snippets/$file_name/..",
+                expected => "unable to parse directory volume name 'snippets/$file_name/..'\n",
+            },
+            {
+                description =>
+                    "Snippets, $file_name, parent directory reference between dir components of volume path",
+                volname => "snippets/foo/../bar/$file_name",
+                expected =>
+                    "unable to parse directory volume name 'snippets/foo/../bar/$file_name'\n",
+            },
+        );
+
+        push($tests->@*, @extra_failed_tests);
+    }
 }

 # Test cases for import files
diff --git a/src/test/path_to_volume_id_test.pm b/src/test/path_to_volume_id_test.pm
index 4dfc68e1..d22f6272 100644
--- a/src/test/path_to_volume_id_test.pm
+++ b/src/test/path_to_volume_id_test.pm
@@ -181,6 +181,13 @@ my $tests = [
             'snippets', 'local:snippets/hookscript.pl',
         ],
     },
+    {
+        description => 'Snippets, arbitrary file, nested subdirectories',
+        file => "$DEFAULT_STORAGE_DIR/snippets/foo/bar/baz/something.txt",
+        expected => [
+            'snippets', 'local:snippets/foo/bar/baz/something.txt',
+        ],
+    },
     {
         description => 'CT template, tar.xz',
         file => "$DEFAULT_STORAGE_DIR/template/cache/debian-10.0-standard_10.0-1_amd64.tar.xz",
--
2.47.3