Enhance cloudstack_disk_offering resource and datasource

[bddvlpr]

Adds a bunch of missing parameters to the disk offering resource. Initialized the data source for it while I was at it.

Tested on simulator.

[Copilot]

Pull request overview

This PR enhances the cloudstack_disk_offering Terraform resource to support additional CloudStack disk-offering parameters and completes the CRUD implementation, while also introducing a new cloudstack_disk_offering data source (plus docs and acceptance tests) so disk offerings can be discovered and referenced by other resources.

Changes:

• Expanded cloudstack_disk_offering resource schema (custom sizing, storage/provisioning type, tags, display flag) and implemented Read/Update/Delete + Importer.
• Added cloudstack_disk_offering data source with filter-based selection and acceptance tests.
• Updated/added website documentation for the resource and data source.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
website/docs/r/disk_offering.html.markdown	Documents the expanded disk offering resource arguments and attributes.
website/docs/d/disk_offering.html.markdown	Adds documentation for the new disk offering data source.
cloudstack/resource_cloudstack_disk_offering.go	Implements full disk offering resource CRUD/import and adds missing schema parameters.
cloudstack/resource_cloudstack_disk_offering_test.go	Adds acceptance tests for disk offering resource scenarios (basic/customized/update).
cloudstack/provider.go	Registers the new disk offering data source with the provider.
cloudstack/data_source_cloudstack_disk_offering.go	Implements the disk offering data source and filter logic.
cloudstack/data_source_cloudstack_disk_offering_test.go	Adds an acceptance test validating the disk offering data source against a created resource.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (3)

cloudstack/resource_cloudstack_disk_offering.go:64

• customized has Default: false but Create() forces it to true whenever disk_size is omitted. This means a config that omits both fields will plan with customized=false and then read back customized=true, causing perpetual diffs / forced recreation. The service offering resource avoids this by making customized Computed: true instead of defaulting it.

			"customized": {
				Description:   "Whether the disk offering allows a custom disk size at deployment time",
				Type:          schema.TypeBool,
				Optional:      true,
				ForceNew:      true,
				Default:       false,
				ConflictsWith: []string{"disk_size"},
			},

website/docs/r/disk_offering.html.markdown:36

• The customized argument docs currently say it "Defaults to false" but also say it's "implied when disk_size is omitted" (and the implementation sets it to true when disk_size is omitted). Please update the wording so the default/implicit behavior is not contradictory and matches provider behavior.

* `customized` - (Optional) Whether the disk offering allows a custom disk size
    to be specified at deployment time. Conflicts with `disk_size`, and is
    implied when `disk_size` is omitted. Defaults to `false`. Changing this
    forces a new resource to be created.

cloudstack/resource_cloudstack_disk_offering_test.go:95

• The acceptance tests cover customized = true explicitly, but not the documented/implemented behavior where omitting both disk_size and customized should create a customized offering. Adding coverage for that case would prevent regressions (and would have caught the schema default drift issue).

const testAccCloudStackDiskOffering_customized = `
resource "cloudstack_disk_offering" "custom" {
  name              = "custom_disk_offering"
  display_text      = "Custom Test"
  customized        = true
  storage_type      = "local"
  provisioning_type = "thin"
  tags              = "ssd"
}

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (2)

cloudstack/resource_cloudstack_disk_offering.go:61

• customized is declared with Default: false, but the Create logic forces customized=true whenever disk_size is omitted (even when the user didn’t set customized). With a default, Terraform will treat an omitted customized as false in config, so after Read sets customized=true from the API this will cause a perpetual diff / forced recreation. Other offering resources avoid this by making customized Computed: true with no default (e.g. service offering).

			"customized": {
				Description:   "Whether the disk offering allows a custom disk size at deployment time",
				Type:          schema.TypeBool,
				Optional:      true,
				ForceNew:      true,

website/docs/r/disk_offering.html.markdown:36

• Docs say customized defaults to false, but the resource Create path implicitly sets customized=true whenever disk_size is omitted. With the schema fix to make customized computed/derived, the docs should describe it as derived from whether disk_size is set rather than a fixed default, otherwise users will hit confusing diffs.

* `customized` - (Optional) Whether the disk offering allows a custom disk size
    to be specified at deployment time. Conflicts with `disk_size`, and is
    implied when `disk_size` is omitted. Defaults to `false`. Changing this
    forces a new resource to be created.

[poddm]

This is missing a number of fields. I added all API fields in this PR. #313

note, I didn't add the data sources in my PR.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (4)

cloudstack/resource_cloudstack_disk_offering.go:63

• customized has Default: false, but Create forces customized=true whenever disk_size is omitted. This makes it impossible to represent the “unset” state and can lead to a permanent diff when users omit both arguments (docs say it should become customized automatically). Consider aligning this with cloudstack_service_offering (Optional+Computed) by removing the default and making customized computed.

			"customized": {
				Description:   "Whether the disk offering allows a custom disk size at deployment time",
				Type:          schema.TypeBool,
				Optional:      true,
				ForceNew:      true,
				Default:       false,
				ConflictsWith: []string{"disk_size"},

cloudstack/resource_cloudstack_disk_offering.go:129

• The customized calculation can’t distinguish between customized = false set explicitly vs not set at all because GetOk("customized") is false for boolean false, and then the code unconditionally overrides it to true when disk_size is omitted. If customized is intended to be implied only when unset, use GetOkExists and/or return a validation error when customized=false and disk_size is not set.

	if v, ok := d.GetOk("disk_size"); ok {
		p.SetDisksize(int64(v.(int)))
	}

	customized := false
	if v, ok := d.GetOk("customized"); ok {
		customized = v.(bool)
	}
	if _, ok := d.GetOk("disk_size"); !ok {
		customized = true
	}
	p.SetCustomized(customized)

website/docs/r/disk_offering.html.markdown:36

• The docs say customized "Defaults to false", but the resource logic treats customized as implied/true when disk_size is omitted. This is a conditional default, so documenting an unconditional default of false is misleading.

* `customized` - (Optional) Whether the disk offering allows a custom disk size
    to be specified at deployment time. Conflicts with `disk_size`, and is
    implied when `disk_size` is omitted. Defaults to `false`. Changing this
    forces a new resource to be created.

cloudstack/resource_cloudstack_disk_offering_test.go:84

• There’s no acceptance test that re-applies a config with neither disk_size nor customized set to verify the implied-customized behavior and catch a potential permadiff. Adding a second PlanOnly: true step (like other tests in the repo) would ensure the resource converges when customized is implied.

func TestAccCloudStackDiskOffering_customized(t *testing.T) {
	var do cloudstack.DiskOffering
	resource.Test(t, resource.TestCase{
		PreCheck:     func() { testAccPreCheck(t) },
		Providers:    testAccProviders,
		CheckDestroy: testAccCheckCloudStackDiskOfferingDestroy,
		Steps: []resource.TestStep{
			{
				Config: testAccCloudStackDiskOffering_customized,
				Check: resource.ComposeTestCheckFunc(
					testAccCheckCloudStackDiskOfferingExists("cloudstack_disk_offering.custom", &do),
					resource.TestCheckResourceAttr("cloudstack_disk_offering.custom", "customized", "true"),
					resource.TestCheckResourceAttr("cloudstack_disk_offering.custom", "storage_type", "local"),
					resource.TestCheckResourceAttr("cloudstack_disk_offering.custom", "provisioning_type", "thin"),
					resource.TestCheckResourceAttr("cloudstack_disk_offering.custom", "tags", "ssd"),
				),
			},
		},
	})
}

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

🟡 Changes recommended

It introduces a breaking removal of autoscale data sources and has idempotency issues around customized defaults/inference that can cause perpetual diffs.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (3)

cloudstack/resource_cloudstack_disk_offering.go:62

• customized is declared with Default: false, but Create forces customized=true whenever disk_size is omitted. That combination can cause perpetual diffs/recreates when users omit both fields (or import an offering) because Terraform will treat customized as explicitly false in config while state/API returns true. Align this with the existing pattern used by cloudstack_service_offering by making customized Optional+Computed and removing the unconditional default.

				Description:   "Whether the disk offering allows a custom disk size at deployment time",
				Type:          schema.TypeBool,
				Optional:      true,
				ForceNew:      true,
				Default:       false,

cloudstack/resource_cloudstack_disk_offering.go:129

• Create currently overrides an explicitly configured customized=false whenever disk_size is omitted (customized gets forced to true). That makes the provider ignore user intent and also hides a configuration error case (fixed-size offering without disk_size). Compute customized from inputs in a single place: if disk_size is set => customized=false; else if customized is explicitly set => use it; else default to customized=true. If customized is explicitly false while disk_size is unset, return a helpful error.

	customized := false
	if v, ok := d.GetOk("customized"); ok {
		customized = v.(bool)
	}
	if _, ok := d.GetOk("disk_size"); !ok {
		customized = true
	}
	p.SetCustomized(customized)

cloudstack/provider.go:84

• This change removes the autoscale data sources from DataSourcesMap (while the autoscale resources still exist). That is a breaking provider surface-area change and doesn’t match the PR’s stated scope (disk offering resource/datasource). If unintentional, re-add the autoscale data sources here.

		DataSourcesMap: map[string]*schema.Resource{
			"cloudstack_condition":                 dataSourceCloudstackCondition(),
			"cloudstack_counter":                   dataSourceCloudstackCounter(),
			"cloudstack_template":                  dataSourceCloudstackTemplate(),
			"cloudstack_ssh_keypair":               dataSourceCloudstackSSHKeyPair(),

• Files reviewed: 7/7 changed files
• Comments generated: 1
• Review effort level: Lite

[sudo87]

@bddvlpr could you please address following comments:

• provider.go removes DataSourcesMap entries for cloudstack_autoscale_policy, cloudstack_autoscale_vm_group, cloudstack_autoscale_vm_profile. Unrelated to this PR, breaks those data sources.
• data_source_cloudstack_disk_offering.go — filtering by customized always fails with "Unknown filter field customized" since the API's JSON key is iscustomized, not customized.
• resource_cloudstack_disk_offering.go — customized is Optional+Default:false+ForceNew but not Computed. When both disk_size and customized are omitted, Create forces customized=true server-side, but state then conflicts with the schema default on every subsequent plan, causing perpetual destroy/recreate. Mark it (and likely disk_size) Computed: true to fix.

[bddvlpr]

Unsure how the deletion of the entries got in, was an issue with a rebase.
I've addressed the other comments and squashed the commits down.
Renaming iscustomized to customized to align with its resource, too.