Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[feature request] Allow profiles and sections to be disabled #282

Open
snstamml opened this issue Nov 1, 2023 · 10 comments
Open

[feature request] Allow profiles and sections to be disabled #282

snstamml opened this issue Nov 1, 2023 · 10 comments
Labels
enhancement New feature or request

Comments

@snstamml
Copy link

snstamml commented Nov 1, 2023

As resticprofile execute a restic command "blindly", accidently running an unconfigured command like "forget" could be fatal. As a safety measure, it would be helpful if a section could be disabled deliberately, e.g.:

myprofile:
  forget:
    enabled: false

Similarly, as a convenience, a complete profile could be disabled in the same way:

myprofile:
  enabled: false
@jkellerer
Copy link
Collaborator

Was also thinking about something like this as there are use cases where a dedicated profile may be created just for a subset of commands.

Just not yet sure about how to configure it most efficiently. Disabling individual sections is one option but if you want to enforce that only a handful of commands is used it wouldn’t scale very well.

Maybe an include / exclude list of available commands might be better e.g. that could also be used to prevent sections of a base (e.g. default) profile from getting used accidentally.

@creativeprojects
Copy link
Owner

Funny enough this week I was considering introducing dot profiles but then I remembered it might not work with TOML.

When you need a base profile that you can't really run because it's incomplete, you prefix it with a dot:

.base_profile:
  source: /

real_profile:
  inherit:
    - .base_profile
  • you cannot run any command on a dot profile
  • you don't see them in show command

Need to find a nice way to do that in TOML though 🤔

@jkellerer
Copy link
Collaborator

Maybe use a prefix like “__” (e.g. __default) and allow to configure the base profile prefix in global for the case that anyone needs to use this name. (Would have also been the case for dot prefixes)

A limit of available commands would also be helpful in addition but the base profile filter by prefix would simply disable all commands (using the name is a nice solution as it doesn’t inherit and this can even be used within the inheritance chain)

@creativeprojects
Copy link
Owner

creativeprojects commented Nov 1, 2023

Making it configurable with a default of __ would certainly work 👍🏻 I like that

@jkellerer
Copy link
Collaborator

jkellerer commented Nov 5, 2023

Ok in this case we have 2 different ways of configuring it:

The original proposal:

profiles:
  default:
    enabled: false

  check-only:
    inherit: default
    enabled: true    # must be set as default is disabled
    backup:
      enabled: false

  backup-only:
    inherit: default

    prune:
      enabled: false
    restore:
      enabled: false
    mount:
      enabled: false

   my-backup:
     inherit: backup-only
     enabled: true    # must be set as default is disabled

Is straight forward but very verbose.

Was thinking about something like this as an alternative, when integrating the dot profile idea (@creativeprojects, @snstamml what do you think?):

version: "2"
global:
  base-profiles: "__*" # default, doesn't need to be set

profiles:
  __base:
    # disabled by default

  check-only:
    inherit: __base
    commands:
      enabled: [check]
      #disabled: [backup]  # -- would disable just backup.. 

  __backup-only:
    inherit: __base
    commands:
      enabled: [backup]

   my-backup:
     inherit: __backup-only   

With "commands" section only being available in version 2 because of the quirks we have with list inheritance (or maybe we address it in a 1.1 format).

@creativeprojects
Copy link
Owner

The second version seems more readable in general.

Clarifications

  • So the concept of command enabled/disabled on base profiles is only for inheritance.
    We still don't allow to run a base/hidden profile, right?
  • By default, all commands are available unless specified anywhere in the inheritance path

Maybe it's easier with some examples:

profiles:
  __base_all: # default values

  profile_all: # all commands enabled, as there's nothing specified in the inheritance path
    inherit: __base_all
  
  __base_backup: # only backup command available
    commands:
      enabled: [backup]
  
  profile_backup: # only backup command available, from inheritance
    inherit: __base_backup

  profile_check_only: # only check command available, inheritance path only has defaults
    inherit: __base_all
    commands:
      enabled: [check]

  profile_backup_check: # check command available, also backup from inheritance
    inherit: __base_backup
    commands:
      enabled: [check]
  
  __base_no_forget: # forget command disabled
    commands:
      disabled: [forget]
  
  __profile_forget: # forget command enabled
    inherit: __base_no_forget
    commands:
      enabled: [forget]

Am I missing some corner cases?

or maybe we address it in a 1.1 format

It's not a bad idea actually.
But the thing is, when people take the time reading the documentation to see the benefits of moving to v1.1 they might as well convert their profile to v2 while they're at it, so I'm not entirely sure about the benefits 😐.
By that I mean my idea of keeping v1 alive is not to force users to reconfigure their system if they don't need to (if they're happy the way it works).

@jkellerer
Copy link
Collaborator

jkellerer commented Nov 5, 2023

So the concept of command enabled/disabled on base profiles is only for inheritance.

Exactly

By default, all commands are available

Exactly

Am I missing some corner cases?

No, that's also how I'm seeing it, ...with one exception: Inheritance still works as usual (list inheritance with append, as otherwise it replaces the parent)

  profile_backup_check: # check command available, also backup from inheritance
    inherit: __base_backup
    commands:
      enabled...: [check]

profile_forget: # forget command enabled
inherit: __base_no_forget

That is the major corner case. Whether enabled or disabled has the higher priority on a conflict. Likely it makes sense to say what is allowed is allowed and cannot be denied. This rule then applies in the final enabled/disabled filter lists after applying all inheritance / composition rules.

... my idea of keeping v1 alive is not to force users ...

Agree. Was thinking to make 1.1 the default if not specified. This means we could remove any issues that we discovered so far that were not intended in the first place and only for the corner case that someone relies on it, we introduce the breaking change that a version must be set.

With this change we could even detect version 2 by default when "profiles" is set. So that a version is required only when a specific behaviour must be enforced.

Nevertheless that is all optional and out of the scope of this issue.

@creativeprojects
Copy link
Owner

There's another thing that is bothering me: the introduction of a commands subsection makes me want to move the commands definition in it 🤔

@jkellerer
Copy link
Collaborator

Is doesn't need to be called commands. On the other hand we have resticprofile [flags] command [command flags] and the command's name is what we allow or deny.

Maybe it helps if we flatten it enabled-commands and disabled-commands (or commands-enabled and commands-disabled)?

@jkellerer
Copy link
Collaborator

2 more thoughts:

The filter should really only apply to commands specified in a request (CLI). It should not affect command chains like check-after or retention.after-backup (or if we implement it later: aliases).

schedule and unschedule are valid commands and for the variant --all, we need to make sure not to select profiles for scheduling that have them disabled (e.g. all base profiles). That is by the way the only option to inherit schedule settings with a default schedule properly. The new scheduling format may simplify this of course but I guess both options remain available.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants