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

matrix-backup-borg: add ability to backup to unencrypted repositories #1754

Merged
merged 12 commits into from Apr 18, 2022
Merged

matrix-backup-borg: add ability to backup to unencrypted repositories #1754

merged 12 commits into from Apr 18, 2022

Conversation

thebiblelover7
Copy link
Contributor

No description provided.

@thebiblelover7
Copy link
Contributor Author

This PR could be resolved by #1755 , so I don't know what should be done.

@etkecc
Copy link
Contributor

etkecc commented Apr 14, 2022

Yes, you can do that with #1755 with configuration extension

@spantaleev
Copy link
Owner

How common are unencrypted Borg repositories? Perhaps backing up to such repositories is much quicker on low-power hardware, because it skips having to encrypt everything.

If it's a common use case, regardless of #1755, we can keep your PR to have a dedicated option for easy usage. Is setting this "ok" flag to true enough to do use an unencrypted repository? Or is something else necessary? It seems like if you leave matrix_backup_borg_storage_encryption_passphrase empty, roles/matrix-backup-borg/tasks/validate_config.yml will choke. We should probably fix this, and potentially update the docs too.


You've also named the variable matrix_backup_borg_unknown_unencrypted_access, which is inconsistent with how we name variables: matrix_(SERVICE_NAME)_(ORIGINAL_NAME_OF_CONFIGURATION_SETTING).

It should be called matrix_backup_borg_unknown_unencrypted_repo_access_is_ok instead.


Another good practice is to use |to_json when spewing variables into JSON/YAML configuration files.

-unknown_unencrypted_repo_access_is_ok: {{ matrix_backup_borg_unknown_unencrypted_access }}
+unknown_unencrypted_repo_access_is_ok: {{ matrix_backup_borg_unknown_unencrypted_repo_access_is_ok|to_json }}

@thebiblelover7
Copy link
Contributor Author

@spantaleev I am currently backing up to my raspberry pi, and I don't need encryption which is why I disabled it on my repo.

I will try to fix those things soon.

@etkecc
Copy link
Contributor

etkecc commented Apr 16, 2022

The #1755 has been merged. Could you try to use configuration extension for your use case and post feedback here, please?

It works the same as in other roles (eg synapse)

@thebiblelover7
Copy link
Contributor Author

The #1755 has been merged. Could you try to use configuration extension for your use case and post feedback here, please?

It works the same as in other roles (eg synapse)

What I've seen is that it fails because you don't pass a password to matrix_backup_borg_storage_encryption_passphrase. As long as you pass it anything, it doesn't fail. I'm currently trying to find a way to have a conditional with_items so that it can only be verified when matrix_backup_borg_unknown_unencrypted_repo_access_is_ok is false.

I'm not very good with ansible, so any help is appreciated!

@spantaleev
Copy link
Owner

I'm currently trying to find a way to have a conditional with_items so that it can only be verified when matrix_backup_borg_unknown_unencrypted_repo_access_is_ok is false.

You can see some conditional with_items usage here:

- name: Ensure Element paths exists
file:
path: "{{ item.path }}"
state: directory
mode: 0750
owner: "{{ matrix_user_username }}"
group: "{{ matrix_user_groupname }}"
with_items:
- {path: "{{ matrix_client_element_data_path }}", when: true}
- {path: "{{ matrix_client_element_docker_src_files_path }}", when: "{{ matrix_client_element_container_image_self_build }}"}
when: "item.when|bool"

Alternatively, you can use another fail task for matrix_backup_borg_storage_encryption_passphrase which is "hidden" behind when: not matrix_backup_borg_unknown_unencrypted_repo_access_is_ok. Example:

- name: Fail if encryption enabled and encryption passphrase empty
  fail:
    msg: >-
      You need to define a required configuration setting (`matrix_backup_borg_storage_encryption_passphrase`).
  when: "matrix_backup_borg_storage_encryption_passphrase == '' and not matrix_backup_borg_unknown_unencrypted_repo_access_is_ok"

@etkecc
Copy link
Contributor

etkecc commented Apr 18, 2022

I suppose you need to update validate_config as well, because it checks for encryption passphrase and some other things.
As @spantaleev suggested - add a new fail when no passphrase is given and unencrypted backups disabled in the validate_config file

Btw, don't forget to pull new changes first

@thebiblelover7
Copy link
Contributor Author

@spantaleev Seems to me like this is ready to be merged. Let me know if there is anything else I should change!

I've tested it on my system and it seems to work, too.

@spantaleev spantaleev merged commit 949fdd0 into spantaleev:master Apr 18, 2022
@spantaleev
Copy link
Owner

Thank for contributing this and for the patience! 👍

spantaleev added a commit that referenced this pull request Apr 19, 2022
ksnieck pushed a commit to ksnieck/matrix-docker-ansible-deploy that referenced this pull request Jul 4, 2022
…spantaleev#1754)

* matrix-backup-borg: added option for unencrypted repo access

* matrix-backup-borg: fixed requiring password for unencrypted repos; changed variable name

* matrix-backup-borg: add unknown_unencrypted_repo_access_is_ok to config.yaml.j2

* matrix-backup-borg: cleanup comments

* matrix-backup-borg: add documentation regarding unencrypted repos

* matrix-backup-borg: add readability and ease of use to code

* matrix-backup-borg: fix wording in defaults/main.yml comment

* matrix-backup-borg: add quotes to docs

* Indicate the variable to use

Co-authored-by: Slavi Pantaleev <slavi@devture.com>
ksnieck pushed a commit to ksnieck/matrix-docker-ansible-deploy that referenced this pull request Jul 4, 2022
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.

None yet

3 participants