Added configuration support for additional restic commands

[jkellerer]

This PR is a cherry-pick from #66 to support configuration of additional commands that manage snapshots (or restore files).

Support for the following additional commands is added: dump, find, ls, restore, stats, tag. All of the commands optionally have host, path and tag filters which can now be set in the profile.

In detail, this PR adds the following:

• Adds config sections for the commands above
• Implements tag and path copy from backup with boolean true for all commands
• Fixes SetHost for the copy section
• Resolves paths for all path parameters in dynamic flags maps (for consistency).
• Changes the defaults of path and tag for retention in config file version 2. Both parameters are copied from backup when not explicitly defined. Was mentioned in Optional: Allow disabling path in retention with 'false' #67 that it would be better to have the same defaults for both filters but couldn't be implemented to not break existing configs.

[codecov]

Codecov Report

Merging #94 (707cc0a) into master (ff13660) will increase coverage by 0.33%.
The diff coverage is 100.00%.

❗ Current head 707cc0a differs from pull request most recent head 1d2b214. Consider uploading reports for the commit 1d2b214 to get more accurate results

@@            Coverage Diff             @@
##           master      #94      +/-   ##
==========================================
+ Coverage   64.62%   64.95%   +0.33%     
==========================================
  Files          64       64              
  Lines        6371     6414      +43     
==========================================
+ Hits         4117     4166      +49     
+ Misses       2050     2047       -3     
+ Partials      204      201       -3     

Impacted Files	Coverage Δ
config/config.go	75.77% <100.00%> (+0.51%)	⬆️
config/profile.go	88.47% <100.00%> (+4.28%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ff13660...1d2b214. Read the comment docs.

[jkellerer]

You might notice, I've removed several unnecessary nil checks to reduce the overall complexity of GetCommandFlags. Also tests on retention are rewritten to allow querying V1 and V2 configs with less code duplications.

[creativeprojects]

Yes, I like the refactoring of the tests 👍🏻

There's something I can't figure out...

In the current version, when I select a backup path of . it keeps it as --path parameter during the retention.
With this PR, it is translated into an absolute path anchored from the config file directory. I wanted to find where it's happening but I can't find it so far... Any idea?

[jkellerer]

My bad. This PR translates all paths (including parameter path) via "fixPath" in SetRootPath. Was just not sure if it makes sense to apply this to path as well since path means a path inside the repository. Thinking about it again it should not be resolved.

[creativeprojects]

Indeed,

also I found another bug: if you select a glob in the backup source it's not resolved when passed as a --path parameter

Sorry 😊

[jkellerer]

Thanks! Looks like we need more I need to write better tests!

[creativeprojects]

Sorry, there's something else I just noticed:

if you use a relative path in the backup source, it needs to be translated into an absolute path in the --path options:

restic is expanding the paths during the backup and writing down the absolute path in the snapshot definition. Then if you run a command selecting these paths (like forget or snapshots) they need to be absolute to match what is recorded in the snapshot.

[jkellerer]

So the prev implementation to resolve path was actually not completely wrong.

I kind of thought that it is like this but wasn’t sure if path always must be absolute or whether there are situations where it can be something else.

Wondering how to fix this, Backup.Source is relative to env and only the absolutePrefix filter creates stable absolute paths that do not depend on current working dir but this logic cannot be used as it differs from sources then.

Log warning when path is not absolute?
Or log a warning if we changed path to an absolute path relative to cwd?

[jkellerer]

Glob pattern in a backup source are another issue. They also let path depend on the env. The usual strategy an admin would apply is to use a prefix for path filter (as long as glob expressions cannot be passed to restic).

Guess the best that can be done is reviewing what restic can support exactly for path filters and log a warning whenever a specified filter depends on the local environment instead of paths from the repository.

[creativeprojects]

It is a bit of a difficult one indeed. Although I'm not sure if restic is actually doing much about it:
Most of it should be replaced by the shell, all environment variables and glob should be.
The only thing restic does (I think?) is make the path absolute. I'll have to double check though.

[jkellerer]

Wanted to check it as well, though I assume for the moment path has to be what snapshots command shows as path list for a snapshot. I’m not sure whether path can be a prefix or is matched literally.

If that is true, the most reasonable fix would be to resolve the path but warn when resolving changes the user input.

Resolving in this case means, resolve copied backup sources like backup sources and make them absolute. For custom path values make them absolute only.

Depending on what restic really supports the warning log can be designed to contain more info how to prevent the warning by adjusting the path filter.

Will update the PR when I know more.

[creativeprojects]

Sorry I made a change on the master branch to fix an issue with the profiles command on inherited profiles, it looks like there's a conflict now

[jkellerer]

Sorry I made a change on the master branch to fix an issue with the profiles ...

No problem, rebase and merge was no issue.

Btw. I've had time to check --path and it is what we collected earlier. Matching is literally (no wildcards, no prefix is allowed) and the --path filters are matching against the absolute path of the sources which means this is relative to pwd when running resticprofile. Multiple --path filters combine with and (as I had expected).

So I'll go ahead and resolve the paths like described earlier and whenever this changes the path, I'll log a warning that the path match might not be stable and tags or custom paths should be used.

[jkellerer]

Update is almost ready but needs more tests.

[jkellerer]

.. I know it became quite a change but it makes future adjustments easier as new things have to be added at less places.

The latest update prints the following when "path" is not absolute:

2022/03/11 22:02:53 the configuration contains relative 'path' items which may lead to unstable results in restic commands that select snapshots. Consider using absolute paths in 'path' (and 'source') or use 'tag' instead of 'path' (path = false) to select snapshots for restic commands. Affected paths:
> path "term" changes to "/Projects/github/resticprofile/term"
> path (from source) "contrib" changes to "/Projects/github/resticprofile/contrib"
> path (from source) "profiles.yaml" changes to "/Projects/github/resticprofile/profiles.yaml"

[creativeprojects]

Cool, I tried it and it works perfectly with my test profiles.
I also noticed your DefinedCommands() improvement with reflection, thanks (speaking of which, go 1.18 is out, exciting!).
I think it's ready to go whenever you're happy with it

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
6 Code Smells

No Coverage information
0.0% Duplication

[creativeprojects]

Looks good to me 👍🏻
Thank you very much on this PR 😄
Happy to merge?

[jkellerer]

Merged 😀