[PATCH manager/storage v2 00/50] Fix #2884: Implement Subdirectory Scanning for Dir-Based Storage Types

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

Fix #2884: Implement Subdirectory Scanning for Dir-Based Storage Types - v2
===========================================================================

Basically what the title says. Implement subdirectory scanning for
directory-based storage types, which includes directories (duh), NFS,
CIFS, CephFS, and BTRFS. This fixes #2884 [2884] and the more
narrowly-scoped #623 [623].

See the initial series's cover letter [init-cover] for the entire
context.

*Huge* thanks to @Wolfgang for the thorough review on v1, and also for
the off-list correspondence regarding this! :) This refresh incorporates
all of his feedback.

Changes Since v1
----------------

Since this series is quite massive, I decided to:
* track all changes here, not just the notable ones
* track changes to patches' index numbers
* add references to each suggestion relevant for the change, if
  applicable / mentioned over the mailing list

While this took a bit of extra time, it should hopefully alleviate some
tracking work on the maintainers' end.

- Clarify that `PVE::Storage::Common::plugin_get_default_vtype_subdirs()`
  returns an absolute path in its docstring (patch #6).

- Do not call if-elsif chains "needless" in the patches that break them
  up anymore (patch #8 and #14).

  Ref.: https://lore.proxmox.com/pve-devel/w44tdwzbsu3bh4xggcss6b3ukrv7imddbzackyf3qhsfv2j5mn@bogediza5n5g/

- Use a lookahead for a slash (`(?=/)`) after the `\E` in
  `get_subdir_files() in patch #9.

  Ref.: https://lore.proxmox.com/pve-devel/ex6gdtlicrwrxpw6bmz5awhw5bnnt4g5uokuajq4fn3b2kkkd6@mjkvz3oppvzi/

- Use a lookahead for a slash (`(?=/)`) after the `\E` in
  `path_to_volume_id()` in patch #16.

  Ref.: https://lore.proxmox.com/pve-devel/j5ygsulsykpaevszwpuv2726r7ogzz6wd2346telm7szlqmbwl@opm2banmchnp/

- Fix filtering using `grep` in patch #17 that made the entire check a
  no-op, which could have affected security.

  Ref.: https://lore.proxmox.com/pve-devel/hwb7vmxog4p3v3iqe74ptettlev3nk4vcgaqdrs67nnoohmh72@2wncblbtmcij/

- Do not use a hash in the `split_leading_dir_from_path()` helper in
  patch #22 anymore.

  Ref.: https://lore.proxmox.com/pve-devel/jfa5fql3svnils2dlklyz4vrgyo4jmn24tuv2wa7uu7lf2wgzb@7arsjxlzdivs/

- Rename the subs added in patch #22 to better distinguish between those
  that take a relative path and those take an absolute path.

  Ref.: https://lore.proxmox.com/pve-devel/jfa5fql3svnils2dlklyz4vrgyo4jmn24tuv2wa7uu7lf2wgzb@7arsjxlzdivs/

- Clarify in the docstrings of certain subs added in patch #22 that we
  return `undef` in scalar context, and an empty list in list context.

  Ref.: https://lore.proxmox.com/pve-devel/jfa5fql3svnils2dlklyz4vrgyo4jmn24tuv2wa7uu7lf2wgzb@7arsjxlzdivs/

- Improve the docstrings of the parsing subs in patch #22 by using
  examples for expected outputs and by making them a little less
  verbose.

- Document further special cases in every patch that introduces them
  -- patches #24, #25, #28, and also #44-46 (formerly patches #48-50
  in v1).

- Fold patches #29 - #32 of v1 into patch #29 of this series, since a
  separate patch for each site was a bit *too* granular.

- Do not replace string interpolation with concatenation in patch #7
  anymore.

  Ref.: https://lore.proxmox.com/pve-devel/ehmk4jjaekoqsxf2pqy6imbfouvaeuwdkzc2odvlskf72yl7yp@3lfm4ihnutiw/

- Do not return volumes of undeclared content types anymore instead of
  just documenting that behavior using test cases in patch #34 (formerly
  patch #38 in v1). Instead, return an empty list if the volumes of an
  undeclared content type are queried.

  Ref.: https://lore.proxmox.com/pve-devel/fgno5lzql7h2jrnyfcmn2usahi5e2chteiwfw543ak7mahglvq@ph2sigxrk2ty/

- Use greedy matching instead of non-greedy matching for the
  subdirectory component regex in patch #44 (formerly patch #48 in v1).

  Ref.: https://lore.proxmox.com/pve-devel/cvjtmro3odmdgh574wnfql3nlaezezukokbsuxdnmycadfcuah@4ckwm5hn2bci/

- Add API deprecations within the commits that actually deprecate things
  instead of bumping the APIVER + APIAGE twice throughout the series.

  This means that if e.g. one commit makes a regex in `PVE::Storage`
  obsolete, the deprecation notice in `ApiChangeLog` and TODO comment
  for that regex are added right away.

  Ref.: https://lore.proxmox.com/pve-devel/xapbgpwdqk7h6uebzkgxei7v7gamzbhyin2blbohd2bt4kyrr4@ogniag55fdai/

- Add patch #50 that replaces the usage of `$plugin->get_subdir()` with
  `PVE::Storage::Common::plugin_get_vtype_subdir()`, since the latter is
  now deprecated.

  Ref.: https://lore.proxmox.com/pve-devel/dzlh5oyu73idd2s6e3ovzrivdhhvxdmkiumwjvzq6nbjotidst@2i25ysrie7xt/

Additional Notes
----------------

Since this series also touches the compression extensions and moves them
to the new parsing module it introduces, I considered expanding upon
that, cleaning up our compression-related code as well.

However, given that this series is already large enough, I consider this
out of scope for now and will probably follow up on this in a separate
series.

Testing
-------

If anyone could give this series a spin, I'd be most grateful!

Here are some interesting things you could check out (non-exhaustive):
- Configuring the new 'max-scan-depth' property in the UI
- Adding subdirectories in your ISO, LXC template and snippets dirs
  (and populating those dirs afterwards)
- Checking whether the depth limit is honored
  --> 0 is the default, which retains the current behavior of not
      scanning through any subdirs
- Checking whether imports (.ova files etc) still work as expected
- ISO / CT template upload / deletion
- ...

Also note that I ran the tests in the repository for every single patch
that I added; if you want to do this for yourself as a sanity check, try
the following:

    git rebase -i --autostash --autosquash origin/master -x 'cd src && make test'

References
----------

[2884]: https://bugzilla.proxmox.com/show_bug.cgi?id=2884
[623]: https://bugzilla.proxmox.com/show_bug.cgi?id=623
[init-cover]: https://lore.proxmox.com/pve-devel/20260422111322.257380-1-m.carrara@proxmox.com/

Summary of Changes
------------------


pve-storage:

Max R. Carrara (48):
  test: plugin tests: run tests with at most 4 jobs
  plugin, common: remove superfluous use of =pod command paragraph
  common: add POD headings for groups of helpers
  common: use Exporter module for PVE::Storage::Common
  plugin: make get_subdir_files a proper subroutine and update style
  plugin api: replace helpers w/ standalone subs, bump API version & age
  common: prevent autovivification in plugin_get_vtype_subdir helper
  plugin: break up if-elsif chain into separate if-blocks
  plugin: adapt get_subdir_files helper of list_volumes API method
  plugin: update code style of list_volumes plugin API method
  plugin: use closure for obtaining raw volume data in list_volumes
  plugin: use closure for inner loop logic in list_volumes
  storage: update code style in function path_to_volume_id
  storage: break up if-elsif chain in path_to_volume_id
  storage: heave vtype file path parsing logic inside loop into helper
  storage: clean up code that was moved into helper in path_to_volume_id
  api: status: move content type assert for up-/downloads into helper
  api: status: use helper from common module to get content directory
  api: status: move up-/download file path parsing code into helper
  api: status: simplify file content assertion logic for up-/download
  test: guest import: add tests for PVE::GuestImport
  tree-wide: introduce parsing module and replace usages of ISO_EXT_RE_0
  common: test: set up parser testing code, add tests for 'iso' vtype
  tree-wide: replace usages of VZTMPL_EXT_RE_1 with parsing functions
  tree-wide: replace usages of BACKUP_EXT_RE_2 with parsing functions
  tree-wide: replace usages of inline regexes for snippets with parsers
  tree-wide: partially replace usages of regexes for 'import' vtype
  tree-wide: replace remaining usages of regexes for 'import' vtype
  tree-wide: simplify recently refactored parsing logic
  test: list volumes: reorganize and modernize test running code
  test: list volumes: fix broken test checking for vmlist modifications
  test: list volumes: introduce new format for test cases
  test: list volumes: remove legacy code and migrate cases to new format
  plugin: do not return volumes of undeclared content types anymore
  plugin: correct comment in get_subdir_files helper
  test: parse volname: modernize code
  test: parse volname: adapt tests regarding 'import' volume type
  test: parse volname: move VM disk test creation into separate block
  test: parse volname: move backup file test creation into sep. block
  test: parse volname: parameterize test case creation for some vtypes
  test: volume id: modernize code
  test: volume id: rename 'volname' test case parameter to 'file'
  test: filesystem path: modernize code
  fix #2884: implement nested subdir scanning and support 'iso' vtype
  fix #2884: support nested subdir scanning for 'vztmpl' volume type
  fix #2884: support nested subdir scanning for 'snippets' vtype
  test: add more tests for 'import' vtype & guard against nested subdirs
  test: add tests guarding against subdir scanning for vtypes

 ApiChangeLog                                |   45 +
 debian/control                              |    1 +
 src/PVE/API2/Storage/Status.pm              |  198 +-
 src/PVE/BackupProvider/Plugin/Base.pm       |    6 +-
 src/PVE/GuestImport.pm                      |   28 +-
 src/PVE/GuestImport/OVF.pm                  |   24 +-
 src/PVE/Makefile                            |    1 +
 src/PVE/Storage.pm                          |  118 +-
 src/PVE/Storage/BTRFSPlugin.pm              |   16 +-
 src/PVE/Storage/CephFSPlugin.pm             |    1 +
 src/PVE/Storage/Common.pm                   |   91 +-
 src/PVE/Storage/Common/Makefile             |    5 +
 src/PVE/Storage/Common/Parse.pm             |  674 ++++++
 src/PVE/Storage/Common/test/Makefile        |    6 +
 src/PVE/Storage/Common/test/parser_tests.pl | 1134 ++++++++++
 src/PVE/Storage/Common/test/run_tests.pl    |   25 +
 src/PVE/Storage/DirPlugin.pm                |    1 +
 src/PVE/Storage/ESXiPlugin.pm               |    6 -
 src/PVE/Storage/Makefile                    |    4 +
 src/PVE/Storage/Plugin.pm                   |  368 ++--
 src/test/filesystem_path_test.pm            |  109 +-
 src/test/get_subdir_test.pm                 |   12 +-
 src/test/guest_import_test.pl               |  948 +++++++++
 src/test/list_volumes_test.pm               | 2095 +++++++++++++++----
 src/test/parse_volname_test.pm              |  696 ++++--
 src/test/path_to_volume_id_test.pm          |  201 +-
 src/test/run_plugin_tests.pl                |   18 +-
 src/test/run_volume_access_tests.pl         |    5 +-
 28 files changed, 5833 insertions(+), 1003 deletions(-)
 create mode 100644 src/PVE/Storage/Common/Parse.pm
 create mode 100644 src/PVE/Storage/Common/test/Makefile
 create mode 100755 src/PVE/Storage/Common/test/parser_tests.pl
 create mode 100755 src/PVE/Storage/Common/test/run_tests.pl
 create mode 100755 src/test/guest_import_test.pl


pve-manager:

Max R. Carrara (2):
  fix #2884: ui: storage: add field for 'max-scan-depth' property
  pve8to9: use helper from common storage module to get vtype subdir

 PVE/CLI/pve8to9.pm           |  5 +++--
 www/manager6/storage/Base.js | 14 ++++++++++++++
 2 files changed, 17 insertions(+), 2 deletions(-)


Summary over all repositories:
  30 files changed, 5850 insertions(+), 1005 deletions(-)

-- 
Generated by murpp 0.12.0