Skip to content

Conversation

@christhemorse
Copy link
Contributor

Description

This PR adds some new API fields for a few different resources:

  • databases - added ca_certificate field
  • instances - added snapshot_id field
  • bare-metals - added snapshot_id field

Also added a missing image_id field from the instance read response, which is consistently part of the API response and already in the relevant data source.

Checklist:

  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • Have you linted your code locally prior to submission?
  • Have you successfully ran tests with your changes locally?

@github-actions
Copy link

github-actions bot commented Sep 3, 2025

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Schema Change

The snapshot_id field was moved from an optional/ForceNew input to a computed+optional+ForceNew attribute. Verify that this change does not break existing Terraform configurations that previously supplied snapshot_id on create.

"snapshot_id": {
	Type:     schema.TypeString,
	Computed: true,
	ForceNew: true,
	Optional: true,
},
Missing Schema

image_id and snapshot_id are set in the READ function but are not declared in the resource schema. Add them to the schema to prevent runtime panics.

if err := d.Set("image_id", instance.ImageID); err != nil {
	return diag.Errorf("unable to set resource instance `image_id` read value: %v", err)
}
if err := d.Set("snapshot_id", instance.SnapshotID); err != nil {
	return diag.Errorf("unable to set resource instance `snapshot_id` read value: %v", err)
}

@github-actions
Copy link

github-actions bot commented Sep 3, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Prevent accidental server recreation

Remove ForceNew: true from the snapshot_id field. Marking a computed field as
ForceNew will force resource recreation on every refresh when the API returns a
non-empty value, causing perpetual diffs and unwanted destruction of bare-metal
servers.

vultr/resource_vultr_bare_metal_server.go [118-123]

 "snapshot_id": {
     Type:     schema.TypeString,
     Computed: true,
-    ForceNew: true,
     Optional: true,
 },
Suggestion importance[1-10]: 9

__

Why: This is a critical issue - having ForceNew: true on a computed field like snapshot_id will cause the bare metal server to be destroyed and recreated whenever the API returns a snapshot ID, leading to data loss and service disruption.

High

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.

1 participant