feat(acme): add short-name aliases for ACME resources#2721
Conversation
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 enhances the Proxmox Terraform provider by introducing a set of user-friendly short-name aliases for existing ACME-related resources and data sources. This change streamlines the configuration experience for users, while ensuring backward compatibility through deprecation warnings and robust state migration capabilities. The update is a key step in the ongoing resource type name migration strategy. 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 correctly implements the short-name aliasing for ACME resources and data sources as outlined in ADR-007. The changes are well-structured, using embedding for code reuse and implementing MoveState for a smooth user migration path. The old resource names are correctly marked as deprecated. I have a few suggestions to improve the clarity of the documentation for the newly added files.
6252fcf to
b9a7053
Compare
Add proxmox_acme_account, proxmox_acme_dns_plugin resources and proxmox_acme_account, proxmox_acme_accounts, proxmox_acme_plugin, proxmox_acme_plugins datasources as short-name aliases. Old names emit a deprecation warning. Resources implement MoveState for Terraform moved block support. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Pavel Boldyrev <pavel@bpg.sh>
b9a7053 to
dd7afa2
Compare
Signed-off-by: Pavel Boldyrev <pavel@bpg.sh>
Add proxmox_acme_account, proxmox_acme_dns_plugin resources and proxmox_acme_account, proxmox_acme_accounts, proxmox_acme_plugin, proxmox_acme_plugins datasources as short-name aliases. Old names emit a deprecation warning. Resources implement MoveState for Terraform moved block support. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Pavel Boldyrev <pavel@bpg.sh>
What does this PR do?
Adds short-name
proxmox_*aliases for all ACME resources and datasources as part of ADR-007 Phase 2:Resources:
proxmox_acme_account,proxmox_acme_dns_pluginData sources:
proxmox_acme_account,proxmox_acme_accounts,proxmox_acme_plugin,proxmox_acme_pluginsOld names emit a deprecation warning. Resources implement
MoveStatefor Terraformmovedblock support. Old docs show a deprecation banner. Existing acceptance tests updated to use short names.Review fixes
resource.Testwithresource.ParallelTestin all ACME test files (ADR-006); remove explicitt.Parallel()from datasource tests to avoid Go 1.26 double-call panicImportState/ImportStateVerifyround-trip test steps for both resources (ADR-006)// proxmox_*comments on all short-name registrations inprovider.go(consistency with access resources)resource_acme_account.go(pre-existing copy-paste artifact)Contributor's Note
make lintand fixed any issues.make docs; SDK: manual/docs/edits).!in PR title).Proof of Work
Community Note
Relates #2133