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 have a similar PR here with all of the fields. #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.

[sudo87]

thank you @bddvlpr for addressing the comments. overall changes are good, only minor nit:

customized docs still say "Defaults to false", but the schema now uses Computed: true, not a static default.

[bddvlpr]

My bad, didn't see this remark. Should be resolved now.

[sudo87]

clgtm

[kiranchavala]

LGTM tested manually

1. Basic fixed-size disk offering (regression check for the swapped-args bug)

resource "cloudstack_disk_offering" "basic" {
  name         = "tf-pr301-basic"
  display_text = "PR301 basic disk offering"
  disk_size    = 10
}

erraform apply 

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated
with the following symbols:
  + create

Terraform will perform the following actions:

  # cloudstack_disk_offering.basic will be created
  + resource "cloudstack_disk_offering" "basic" {
      + customized        = (known after apply)
      + disk_size         = 10
      + display_offering  = true
      + display_text      = "PR301 basic disk offering"
      + id                = (known after apply)
      + name              = "tf-pr301-basic"
      + provisioning_type = "thin"
      + storage_type      = "shared"
    }

Plan: 1 to add, 0 to change, 0 to destroy.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

cloudstack_disk_offering.basic: Creating...
cloudstack_disk_offering.basic: Creation complete after 0s [id=bb1fc167-7273-479c-bdd4-73a11abecb67]

2. Update in place

resource "cloudstack_disk_offering" "basic" {
  name         = "tf-pr301-basic"
  display_text = "PR301 basic disk offering updated"
  disk_size    = 10
  tags         = "gold"
}

terraform apply
cloudstack_disk_offering.basic: Refreshing state... [id=bb1fc167-7273-479c-bdd4-73a11abecb67]

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated
with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # cloudstack_disk_offering.basic will be updated in-place
  ~ resource "cloudstack_disk_offering" "basic" {
      ~ display_text      = "PR301 basic disk offering" -> "PR301 basic disk offering updated"
        id                = "bb1fc167-7273-479c-bdd4-73a11abecb67"
        name              = "tf-pr301-basic"
      + tags              = "gold"
        # (5 unchanged attributes hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

cloudstack_disk_offering.basic: Modifying... [id=bb1fc167-7273-479c-bdd4-73a11abecb67]
cloudstack_disk_offering.basic: Modifications complete after 1s [id=bb1fc167-7273-479c-bdd4-73a11abecb67]

Apply complete! Resources: 0 added, 1 changed, 0 destroyed.

3. Customized (no fixed size) offering + ConflictsWith

resource "cloudstack_disk_offering" "custom" {
  name              = "tf-pr301-custom"
  display_text      = "PR301 customized disk offering"
  customized        = true
  storage_type      = "local"
  provisioning_type = "sparse"
  tags              = "ssd"
}

 terraform apply
cloudstack_disk_offering.basic: Refreshing state... [id=bb1fc167-7273-479c-bdd4-73a11abecb67]

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated
with the following symbols:
  + create

Terraform will perform the following actions:

  # cloudstack_disk_offering.custom will be created
  + resource "cloudstack_disk_offering" "custom" {
      + customized        = true
      + disk_size         = (known after apply)
      + display_offering  = true
      + display_text      = "PR301 customized disk offering"
      + id                = (known after apply)
      + name              = "tf-pr301-custom"
      + provisioning_type = "sparse"
      + storage_type      = "local"
      + tags              = "ssd"
    }

Plan: 1 to add, 0 to change, 0 to destroy.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

cloudstack_disk_offering.custom: Creating...
cloudstack_disk_offering.custom: Creation complete after 1s [id=28e4e048-2093-4de9-abe4-3fb95d01aa2e]

Apply complete! Resources: 1 added, 0 changed, 0 destroyed.

resource "cloudstack_disk_offering" "bad" {
  name         = "tf-pr301-bad"
  display_text = "should fail"
  disk_size    = 10
  customized   = true
}

terraform apply
╷
│ Error: Conflicting configuration arguments
│ 
│   with cloudstack_disk_offering.bad,
│   on main.tf line 27, in resource "cloudstack_disk_offering" "bad":
│   27:   disk_size    = 10
│ 
│ "disk_size": conflicts with customized
╵
╷
│ Error: Conflicting configuration arguments
│ 
│   with cloudstack_disk_offering.bad,
│   on main.tf line 28, in resource "cloudstack_disk_offering" "bad":
│   28:   customized   = true
│ 
│ "customized": conflicts with disk_size

4. Change disk_size from 10 to 20 on the Step 2 resource

resource "cloudstack_disk_offering" "basic" {
  name         = "tf-pr301-basic"
  display_text = "PR301 basic disk offering updated"
  disk_size    = 20
  tags         = "gold"
}

terraform apply
cloudstack_disk_offering.custom: Refreshing state... [id=28e4e048-2093-4de9-abe4-3fb95d01aa2e]
cloudstack_disk_offering.basic: Refreshing state... [id=bb1fc167-7273-479c-bdd4-73a11abecb67]

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated
with the following symbols:
-/+ destroy and then create replacement

Terraform will perform the following actions:

  # cloudstack_disk_offering.basic must be replaced
-/+ resource "cloudstack_disk_offering" "basic" {
      ~ customized        = false -> (known after apply)
      ~ disk_size         = 10 -> 20 # forces replacement
      ~ id                = "bb1fc167-7273-479c-bdd4-73a11abecb67" -> (known after apply)
        name              = "tf-pr301-basic"
        tags              = "gold"
        # (4 unchanged attributes hidden)
    }

Plan: 1 to add, 0 to change, 1 to destroy.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

cloudstack_disk_offering.basic: Destroying... [id=bb1fc167-7273-479c-bdd4-73a11abecb67]
cloudstack_disk_offering.basic: Destruction complete after 0s
cloudstack_disk_offering.basic: Creating...
cloudstack_disk_offering.basic: Creation complete after 1s [id=3d6d05d5-ddc4-426f-955a-9e93df7ce1b6]

Apply complete! Resources: 1 added, 0 changed, 1 destroyed.

5. Data source

data "cloudstack_disk_offering" "lookup" {
  filter {
    name  = "name"
    value = "^tf-pr301-basic$"
  }
  depends_on = [cloudstack_disk_offering.basic]
}

output "found_disk_size" {
  value = data.cloudstack_disk_offering.lookup.disk_size
}

terraform apply
cloudstack_disk_offering.basic: Refreshing state... [id=3d6d05d5-ddc4-426f-955a-9e93df7ce1b6]
cloudstack_disk_offering.custom: Refreshing state... [id=28e4e048-2093-4de9-abe4-3fb95d01aa2e]
data.cloudstack_disk_offering.lookup: Reading...
data.cloudstack_disk_offering.lookup: Read complete after 0s [id=3d6d05d5-ddc4-426f-955a-9e93df7ce1b6]

Changes to Outputs:
  + found_disk_size = 20

You can apply this plan to save these new output values to the Terraform state, without changing any real
infrastructure.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes


Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

Outputs:

found_disk_size = 20

6. Terraform import

resource "cloudstack_disk_offering" "imported" {
  # leave minimal for now — terraform plan after import will show
  # everything that doesn't match yet, then fill values in below
  name         = "tf-pr301-imported"
  display_text = "Imported"
}

terraform import cloudstack_disk_offering.imported 624de2ab-9aae-418f-aad4-5660984ebec6
cloudstack_disk_offering.imported: Importing from ID "624de2ab-9aae-418f-aad4-5660984ebec6"...
cloudstack_disk_offering.imported: Import prepared!
  Prepared cloudstack_disk_offering for import
cloudstack_disk_offering.imported: Refreshing state... [id=624de2ab-9aae-418f-aad4-5660984ebec6]
data.cloudstack_disk_offering.lookup: Reading...
data.cloudstack_disk_offering.lookup: Read complete after 0s [id=3d6d05d5-ddc4-426f-955a-9e93df7ce1b6]

Import successful!

The resources that were imported are shown above. These resources are now in
your Terraform state and will henceforth be managed by Terraform.