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

Add Proxmox Backup Server storage type #169

Merged
merged 2 commits into from
Sep 1, 2023
Merged

Add Proxmox Backup Server storage type #169

merged 2 commits into from
Sep 1, 2023

Conversation

lae
Copy link
Owner

@lae lae commented Oct 19, 2021

Closes #163

Possibly some stuff missing, actually. I have some changes for prune-backups stashed currently because I need to write more parsing logic around it so I'm putting that off because it seems like it's optional (it's not mentioned in https://pve.proxmox.com/wiki/Storage:_Proxmox_Backup_Server) and applies to storages in general (so I'll work on it separately later). The encryption-key JSON might require further validation, but I have no idea what it's supposed to look like.

This isn't being added to the test environment since PVE apparently does try to contact PBS during creation and we obviously can't set up a test PBS instance just for this. However, I think that might only be if it's set to autogen? So maybe it'll skip contacting PBS if valid JSON is provided (and thus we could try to implement a test that way).

@lae lae force-pushed the feature/pbs-storage branch 2 times, most recently from fce9ae6 to 9014790 Compare October 19, 2021 16:18
@alexschomb
Copy link

Hi,

I did some thorough testing of your commits and can confirm that it partly works.

I had to remove anything related to master_pubkey from the code because I always received the following error:

An exception occurred during task execution. To see the full traceback, use -vvv. The error was: AttributeError: 'ProxmoxStorage' object has no attribute 'master_pubkey'

As a result, I expect encryption_key to be broken, and it doesn't seem to work at all. In #163 you asked for the format of such encryption_key and I am happy to provide you with an example of an automatically generated key:

{"kdf":null,"created":"2022-01-17T16:55:00+01:00","modified":"2022-01-17T16:55:00+01:00","data":"a59lqYGkPJO2gRq3buHscIfAoLJsztG+ZBn7CaUQ0yM=","fingerprint":"6c:c2:1a:1e:f7:65:cf:ce:2a:ed:4e:b8:7a:5e:3d:b3:36:9c:fa:6b:da:2e:4b:02:dd:16:2e:b1:1f:ec:8b:e3"}

With master_pubkey traces removed, no encryption_key variable set and fingerprint provided, the task works just fine and successfully adds the pbs type storage to the PVE host.

But with PVE/PBS it is also possible to add a new pbs storage without providing any fingerprint. This requires the PBS server to have a signed certificate, which can effortlessly be provided with Let's Encrypt. Unfortunately the Ansible role task fails with the following error message if no fingerprint is given:

An exception occurred during task execution. To see the full traceback, use -vvv. The error was: TypeError: expected string or bytes-like object failed

@user1007017
Copy link

user1007017 commented Mar 7, 2023

Unfortunately the Ansible role task fails with the following error message if no fingerprint is given:

An exception occurred during task execution. To see the full traceback, use -vvv. The error was: TypeError: expected string or bytes-like object failed

Seems there is an error in the fingerprint validation which causes the creation of other stores to fail -
after commenting out the 3 lines in the following snippet in proxmox_storage.py it runs until the end

# Validate the parameters given to us
       fingerprint_re = re.compile('^([A-Fa-f0-9]{2}:){31}[A-Fa-f0-9]{2}$')
       # if not fingerprint_re.match(self.fingerprint):
           # self.module.fail_json(msg=(f"fingerprint must be of the format, "
                                   #    f"{fingerprint_re.pattern}."))

I had to comment the master_pubkey parts as well

@lae lae force-pushed the feature/pbs-storage branch from 9014790 to bdadfdb Compare July 28, 2023 11:43
@lae
Copy link
Owner Author

lae commented Jul 28, 2023

Rebased against develop and fixed the logic issue with the fingerprint param. Can someone test this for me? Will merge afterwards.

@lae lae merged commit 35bcd8b into develop Sep 1, 2023
@lae lae deleted the feature/pbs-storage branch September 1, 2023 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature Request - "proxmox backup server" as storage option
4 participants