feat(backup): add proxmox_backup_job resource and proxmox_backup_jobs data source#2711
feat(backup): add proxmox_backup_job resource and proxmox_backup_jobs data source#2711
proxmox_backup_job resource and proxmox_backup_jobs data source#2711Conversation
…_jobs` data source Signed-off-by: Pavel Boldyrev <pavel@bpg.sh>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly expands the Proxmox Terraform provider's capabilities by introducing a new resource and data source for managing cluster-wide backup jobs. It enables users to define and control their Proxmox backup strategies directly through Terraform, covering a wide array of configuration options from basic scheduling to advanced performance and retention settings. The changes also include internal improvements for more robust attribute handling and adherence to project architectural decision records. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces the proxmox_backup_job resource and proxmox_backup_jobs data source, providing comprehensive management for Proxmox VE cluster backup jobs. The implementation is robust, effectively handling complex API interactions such as the delete parameter for updates, conversion of list attributes to/from comma-separated strings, and the inconsistent data types for prune-backups through a custom PruneBackupsString type. The new helper functions in fwprovider/attribute/attribute.go correctly manage nil values for optional and computed fields, preventing unwanted API calls and perpetual diffs. Documentation and acceptance tests are well-integrated, ensuring the new features are clear and functional. Overall, this is a high-quality addition that significantly enhances the provider's capabilities.
Signed-off-by: Pavel Boldyrev <pavel@bpg.sh>
|
/gemini review again |
There was a problem hiding this comment.
Code Review
This pull request introduces a well-implemented proxmox_backup_job resource and proxmox_backup_jobs data source. The code is robust, handling API inconsistencies gracefully, and is accompanied by thorough acceptance tests. I've only found a few minor opportunities to improve the documentation for clarity and consistency.
proxmox_backup_job resource and proxmox_backup_jobs data sourceproxmox_backup_job resource and proxmox_backup_jobs data source
Signed-off-by: Pavel Boldyrev <pavel@bpg.sh>
…der-proxmox into feat/2540-backup-job
What does this PR do?
Adds a new
proxmox_backup_jobresource andproxmox_backup_jobsdata source for managing Proxmox VE cluster backup jobs via the/cluster/backupAPI. This implements the backup job management requested in #1815 by porting and rewriting the abandoned PR #2540 to comply with all current project ADRs.The resource supports full CRUD lifecycle including VM targeting (by ID list, pool, or all), scheduling, storage, compression, retention policies (
prune_backups), fleecing, performance tuning, and field deletion via the Proxmoxdeleteparameter. Uses theproxmox_prefix per ADR-007.Also adds shared
attribute.StringPtrFromValue/CustomBoolPtrFromValue/Int64PtrFromValuehelpers for safely convertingOptional+Computedfields (whereValueStringPointer()returns&""for Unknown values).Contributor's Note
make lintand fixed any issues.make docs; SDK: manual/docs/edits).!in PR title).make exampleto verify the change works (mainly for SDK / provider config changes).Proof of Work
Acceptance Tests (6/6 pass)
Test scenarios:
mailto, then removes it, verifiesdeleteparameter sent to APIvmid = ["100", "101", "102"], updates to["100", "200"], verifies import round-trip with listprune_backups, verifies key sorting round-tripAPI Verification (mitmproxy)
Verified all API calls with
mitmdump --flow-detail 4. 29 calls, zero errors.Create (POST /cluster/backup/) — sends only set fields, no empty strings for omitted Optional+Computed fields:
Update with field deletion (PUT /cluster/backup/{id}) —
deleteparameter correctly sent:Minimal Create — only required + explicitly set fields, no leaking empty strings:
Community Note
Closes #1815 | Relates #2540