Skip to content
New issue

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

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

Already on GitHub? # to your account

Disable auto key generation for config managment #202

Open
varac opened this issue May 22, 2022 · 4 comments
Open

Disable auto key generation for config managment #202

varac opened this issue May 22, 2022 · 4 comments
Labels
bug Something isn't working

Comments

@varac
Copy link
Contributor

varac commented May 22, 2022

Is your feature request related to a problem? Please describe.

I'm using config management to deploy autorestic on multiple servers. My ansible role creates autorestic config files in a declarative manner.
Even though I like the idea of auto-generating keys, and adding them to the config file for interactive sessions, this functionality stands against the principle of declarative config management.
Usually it works well when I provide a key with ansible, so autorestic doesn't need to modify the config file. However, there are situations where you try to setup a new backend and doen't supply the key yet, and a auto-modified config is pretty confusing.

Describe the solution you'd like

  • Add a flag that disables auo key generation of the config file (i.e. --no-key-generation)

First I thought it would be good to ask the user for permission to modify the config file but in automated backups using cron jobs i.e. this is not possible, so autorestic should just fail in case of a missing key, if this option is set.

What do you think ?

@cupcakearmy
Copy link
Owner

#194 (comment)

I think I'll remove writing to config. Makes a lot of thinks more complicated that they need to be and creates confusion

@cupcakearmy cupcakearmy added the bug Something isn't working label May 24, 2022
@FarisZR
Copy link
Contributor

FarisZR commented Jun 3, 2022

Adding a new command to generate the key rather than editing the config would be great

@dotboris
Copy link
Contributor

dotboris commented Jul 2, 2024

I've done some digging and it seems like writing the config file is only done if:

  1. The backend has no key option defined
  2. The AUTORESTIC_{BACKEND}_RESTIC_PASSWORD variable is not set

See:

if b.Key == "" {
// Check if key is set in environment
env, _ := b.getEnv()
if _, found := env["RESTIC_PASSWORD"]; !found {
// No key set in config file or env => generate random key and save file
key := generateRandomKey()
b.Key = key
c := GetConfig()
tmp := c.Backends[b.name]
tmp.Key = key
c.Backends[b.name] = tmp
if err := c.SaveConfig(); err != nil {
return err
}
}
}

If your autorestic.yml file is getting rewritten then it's likely that the you're specifying the key incorrectly somehow.

In my case, I defined a backend named local-test and I tried passing in the key through the AUTORESTIC_LOCAL_TEST_RESTIC_SECRET={my secret} environment variable. Turns out that autorestic converts backend names to environment variables by simply upper casing the name. It doesn't convert - to _. See:

// From Envfile and passed as env
var prefix = "AUTORESTIC_" + strings.ToUpper(b.name) + "_"
for _, variable := range os.Environ() {
var splitted = strings.SplitN(variable, "=", 2)
if strings.HasPrefix(splitted[0], prefix) {
env[strings.TrimPrefix(splitted[0], prefix)] = splitted[1]
}
}

To fix my issue, I changed the backend name name not to include -.

To fix this in general I think that removing the behavior that write to the config file is probably a good idea tho I think that we can do more to avoid confusion. Specifically, I'm suggesting:

  1. Handle special characters in backend names when reading from environment variables. They should probably be all converted to _. (When reading backend config values from environment variables treat all special characters in backend name as _ #382)
  2. Add an option or flag to disable key generation. That way, if you accidentally mess up passing the key like I did, it would stop / crash and give me an error message. (Add option to crash autorestic when key is missing instead of generating a new key #383)

@FedericoStra
Copy link

I've done some digging and it seems like writing the config file is only done if:

1. The backend has no `key` option defined

2. The `AUTORESTIC_{BACKEND}_RESTIC_PASSWORD` variable is not set

Would it be possible to allow also AUTORESTIC_{BACKEND}_RESTIC_PASSWORD_COMMAND? I don't like having the password written in clear text in .config/autorestic/.autorestic.env as

AUTORESTIC_{BACKEND}_RESTIC_PASSWORD=...

I would prefer

AUTORESTIC_{BACKEND}_RESTIC_PASSWORD_COMMAND='pass show backup/autorestic'

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants