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

Fix forgetoption and configoption when saving the config file #402

Merged
merged 1 commit into from
Nov 4, 2024

Conversation

dotboris
Copy link
Contributor

There's a number of open issues about crashes coming from forgetoption and configoption:

This happens when autorestic generates a secret key for a backend that doesn't already have one. Under the hood autorestic will generate the key and then write the config to disk. During this write operation, invalid config options were introduced which leads autorestic to crash on following runs.

This bug happens because of a quirk in viper (the config lib used by autorestic). When viper reads a config to a struct it does so using the mapstructure tags but when it write the config from a struct it doesn't use those. It instead just passes it to https://pkg.go.dev/gopkg.in/yaml.v2 which uses the yaml tags.

The fix here is a bit of a hack. I simply duplicated all the mapstructure tags into new yaml tags with the exact same values. The mapstructure tags are used for reading and the yaml tags are used for writing. This is obviously ugly but it does fix the issue.

I know that @cupcakearmy had some plans of removing the config writing feature from autorestic. While I think that this is a good idea in the long run, I'm hoping that this fix here is a good way to fix the bug without introducing a potentially breaking change for users.

Example result

Here's an example of the resulting config from this PR:

example-config.yml:

version: 2
locations:
  test:
    type: local
    from: test
    to: [example]
backends:
  example:
    type: local
    path: foo

Generating the key:

$ go run . check -c example-config.yml 
Using config: 	 /home/dotboris/code/autorestic/example-config.yml
Using lock:	 .autorestic.lock.yml
Saved a backup copy of your file next to the original.
Initializing backend "example"...
Everything is fine.

Updated config:

backends:
  example:
    type: local
    path: foo
    key: 6qC9thANvNfz3M9VekpjM4QYPNtVYbBkD40hnIjd03x0ekEqwyFevLRApfqYZi1OfWlMvZC4rNntliODPzYw
locations:
  test:
    from:
    - test
    type: local
    to:
    - example
version: 2

Notice that autorestic has written a key for the example backend but the config is not polluted with noisy fields. The only issue with this is that the fields are re-ordered. This is not great but I believe that it's tolerable.

There are two practical changes when the config gets updated:
- The `forgetoption` and `configoption` bug is now gone
- Superfluous config keys no longer get written out
Copy link

vercel bot commented Oct 16, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
autorestic ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 16, 2024 0:30am

@cupcakearmy cupcakearmy merged commit 6895df1 into cupcakearmy:master Nov 4, 2024
4 checks passed
@dotboris dotboris deleted the fix-forgetoption branch November 4, 2024 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants