autorestic check creates an empty string forgetoption that is invalid

[joshuataylor]

Describe the bug Running autorestic check against a yaml adds this config option to a location:

    forgetoption: ""

However this is invalid if doing another check or doing the first backup.

Expected behavior It works

Environment

• OS: ArchLinux 5.17.5
• Version: 1.7.1

Additional context I had to remove the string for it to work, other than that it worked absolutely perfectly magically fantastically :rocket:

[em429]

I ran into the same, plus I ran into autorestic check transforming my copy key into copyoptions: {} which a consequential check detected as a problem, and had to manually remove.

[ghost]

I'm also affected by this behaviour. Actually I'd like to prevent autorestic from automatically changing the configuration, as I provide that via Ansible.

[Chostakovitch]

I ran into the same issue.

It happens there, when checking a backend with no secret key: https://github.com/cupcakearmy/autorestic/blob/3bc091f826704d0a95d4d500d60083bddd3dee74/internal/backend.go#L113 The key generated and must be added to the configuration file, hence the file modification.

When doing so viper writes all possible default values for backends and locations, using the structure field names: https://github.com/cupcakearmy/autorestic/blob/3bc091f826704d0a95d4d500d60083bddd3dee74/internal/location.go#L53-L54

The problem is that it writes forgetoption and copyoption, which are invalid keys.

It thought about simply modifying forgetoption to forget but it would conflict with a function with the same name. Maybe the simplest way would be to modify the function's name to something like doForget so current configuration and documentation is not affected.

I'll send a PR soon.

[ghost]

I see. In our case the key is provided using an environment variable AUTORESTIC_CLOUD_RESTIC_PASSWORD ("CLOUD" being our backend name). So is this key not considered?

[Chostakovitch]

@godvoigt I cannot reproduce your behaviour. With a backend named cloud, running

AUTORESTIC_CLOUD_RESTIC_PASSWORD=test autorestic -c <config_path> check

will recognize the environment variable, internally setting RESTIC_PASSWORD to its value, and will not rewrite the configuration file according to the code.

[ghost]

@Chostakovitch I think we messed up then calling autorestic so that the environment variable was not set.

[cupcakearmy]

I think we should simply remove the key generation feature and never write to the config at all. It makes not so much sense and introduces confusion with no real benefit.

[Chostakovitch]

@cupcakearmy Yeah, totally agree. I'm also using Ansible and configuration rewriting is quite unexpected. Maybe #197 is still slightly relevant in case we need to write a configuration file in the future.

[ovizii]

It also seems to reformat and rearrange the config. i.e. version: 2 ended up as the last line after autorestic was done with my config file.

[cupcakearmy]

Next bigger update I def. want to remove the generation feature. Also because as @ovizii the default go writer sorts everything alphabetically and mixes up everything and is kind of unexpected behaviour

[perry-mitchell]

@cupcakearmy I'm currently being affected by this in a new project. I need to use check to init, as backup fails if run by itself first. How can I bypass this issue to initialise a new backend, without having to wait for the PR addressing this to be merged?

The backends are created dynamically using terraform and are therefore empty when the first scheduled backup is to run. I want to use check to initialise them but then this issue is a blocker for that. Thanks.