Add support for VPC offering resource - #306
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds Terraform support for managing and querying CloudStack VPC offerings by introducing a new cloudstack_vpc_offering resource and cloudstack_vpc_offering data source, along with acceptance tests and website documentation.
Changes:
- Added
cloudstack_vpc_offeringresource implementation (CRUD + import). - Added
cloudstack_vpc_offeringdata source with filter support and acceptance tests. - Added provider registration and website docs for both the resource and data source.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| website/docs/r/vpc_offering.html.markdown | Adds resource documentation and usage example for cloudstack_vpc_offering. |
| website/docs/d/vpc_offering.html.markdown | Adds data source documentation and example for data.cloudstack_vpc_offering. |
| cloudstack/resource_cloudstack_vpc_offering.go | Implements the VPC offering Terraform resource (schema + CRUD). |
| cloudstack/resource_cloudstack_vpc_offering_test.go | Adds acceptance tests for the new VPC offering resource (basic, capabilities, import). |
| cloudstack/provider.go | Registers the new resource and data source in the provider. |
| cloudstack/data_source_cloudstack_vpc_offering.go | Implements the VPC offering data source, including filtering and attribute population. |
| cloudstack/data_source_cloudstack_vpc_offering_test.go | Adds acceptance tests for the new VPC offering data source. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…-provider into vpc-offering-suppot
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Suppressed comments (8)
cloudstack/resource_cloudstack_vpc_offering.go:75
- These boolean attributes are documented as defaulting to
falsebut the schema doesn’t setDefault: false. With the SDK, this can lead to “unset vs false” drift (especially after import/refresh) and also makesGetOkbehave unintuitively for booleans. SetDefault: falsefor these optional booleans (and consider usingGetOkExistswhere you need to distinguish “unset” vs “explicitly set to false”).
"enable": {
Type: schema.TypeBool,
Optional: true,
Description: "set to true if the offering is to be enabled during creation. Default is false",
},
"for_nsx": {
Type: schema.TypeBool,
Optional: true,
Description: "true if the VPC offering is meant to be used for NSX, false otherwise",
ForceNew: true,
},
"nsx_support_lb": {
Type: schema.TypeBool,
Optional: true,
Description: "true if the NSX supports Lb service, false otherwise",
ForceNew: true,
},
cloudstack/resource_cloudstack_vpc_offering.go:105
- These boolean attributes are documented as defaulting to
falsebut the schema doesn’t setDefault: false. With the SDK, this can lead to “unset vs false” drift (especially after import/refresh) and also makesGetOkbehave unintuitively for booleans. SetDefault: falsefor these optional booleans (and consider usingGetOkExistswhere you need to distinguish “unset” vs “explicitly set to false”).
"specify_as_number": {
Type: schema.TypeBool,
Optional: true,
Description: "true if the VPC offering supports choosing AS number",
ForceNew: true,
},
cloudstack/resource_cloudstack_vpc_offering.go:288
- The schema includes
nsx_support_lb,network_provider, andservice_offering_id, but the Read function never sets them. This makes imported state incomplete/incorrect and can cause confusing plans after import or after out-of-band changes (even if it often “works” right after Create due to prior state). Populate these fields in Read when present in the API response (e.g.,o.Nsxsupportlb, provider field, service offering id field).
d.Set("enable", o.State == "Enabled")
d.Set("is_default", o.Isdefault)
d.Set("for_nsx", o.Fornsx)
d.Set("specify_as_number", o.Specifyasnumber)
cloudstack/data_source_cloudstack_vpc_offering.go:134
- The error message is hard to act on and slightly ungrammatical. It also assumes “regex” even though the user thinks in terms of “filters”. Consider something like: “no VPC offering matched the supplied filters” and include the filter name/value (or at least the regex) to aid debugging.
if len(vpcOfferings) == 0 {
return fmt.Errorf("No VPC offering is matching with the specified regex")
}
website/docs/d/vpc_offering.html.markdown:6
- The data source implementation treats
filter.valueas a regular expression and, when multiple matches are found, selects the latest offering by creation date. None of that behavior is documented here, which can surprise users. Also, “Cloudstack/cloudstack” casing should be consistent with “CloudStack” used elsewhere. Update the doc to explicitly state that filter values are regexes and that the data source resolves multiple matches by selecting the newest offering.
page_title: "Cloudstack: cloudstack_vpc_offering"
sidebar_current: "docs-cloudstack-cloudstack_vpc_offering"
description: |-
Gets information about cloudstack VPC offering.
website/docs/d/vpc_offering.html.markdown:11
- The data source implementation treats
filter.valueas a regular expression and, when multiple matches are found, selects the latest offering by creation date. None of that behavior is documented here, which can surprise users. Also, “Cloudstack/cloudstack” casing should be consistent with “CloudStack” used elsewhere. Update the doc to explicitly state that filter values are regexes and that the data source resolves multiple matches by selecting the newest offering.
Use this datasource to get information about a VPC offering for use in other resources.
website/docs/d/vpc_offering.html.markdown:26
- The data source implementation treats
filter.valueas a regular expression and, when multiple matches are found, selects the latest offering by creation date. None of that behavior is documented here, which can surprise users. Also, “Cloudstack/cloudstack” casing should be consistent with “CloudStack” used elsewhere. Update the doc to explicitly state that filter values are regexes and that the data source resolves multiple matches by selecting the newest offering.
* `filter` - (Required) One or more name/value pairs to filter off of. You can apply filters on any exported attributes.
website/docs/r/vpc_offering.html.markdown:45
- The resource schema description uses lowercase
ipv4/dualstack, while the docs list"IPv4"/"dualstack". Pick one canonical set of values (matching what the CloudStack API actually expects/returns) and make the docs and schema consistent to avoid user confusion and failed applies due to case sensitivity.
* `internet_protocol` - (Optional) The internet protocol. Possible values are "IPv4" or "dualstack". Defaults to "IPv4".
| if v, ok := d.GetOk("service_offering_id"); ok { | ||
| serviceOfferingID, e := retrieveID(cs, "service_offering", v.(string)) | ||
| if e != nil { | ||
| return e.Error() |
kiranchavala
left a comment
There was a problem hiding this comment.
LGTM , Tested manually
resource "cloudstack_vpc_offering" "example" {
name = "example-vpc-offering"
display_text = "Example VPC Offering"
enable = true
supported_services = ["Dhcp", "Dns", "SourceNat", "PortForwarding", "Lb", "UserData", "StaticNat", "NetworkACL"]
service_provider_list = {
Dhcp = "VpcVirtualRouter"
Dns = "VpcVirtualRouter"
SourceNat = "VpcVirtualRouter"
PortForwarding = "VpcVirtualRouter"
Lb = "VpcVirtualRouter"
UserData = "VpcVirtualRouter"
StaticNat = "VpcVirtualRouter"
NetworkACL = "VpcVirtualRouter"
}
}
terraform 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_vpc_offering.example will be created
+ resource "cloudstack_vpc_offering" "example" {
+ display_text = "Example VPC Offering"
+ enable = true
+ id = (known after apply)
+ internet_protocol = (known after apply)
+ is_default = (known after apply)
+ name = "example-vpc-offering"
+ network_mode = (known after apply)
+ routing_mode = (known after apply)
+ service_provider_list = {
+ "Dhcp" = "VpcVirtualRouter"
+ "Dns" = "VpcVirtualRouter"
+ "Lb" = "VpcVirtualRouter"
+ "NetworkACL" = "VpcVirtualRouter"
+ "PortForwarding" = "VpcVirtualRouter"
+ "SourceNat" = "VpcVirtualRouter"
+ "StaticNat" = "VpcVirtualRouter"
+ "UserData" = "VpcVirtualRouter"
}
+ supported_services = [
+ "Dhcp",
+ "Dns",
+ "Lb",
+ "NetworkACL",
+ "PortForwarding",
+ "SourceNat",
+ "StaticNat",
+ "UserData",
]
+ service_capability_list (known after apply)
}
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_vpc_offering.example: Creating...
cloudstack_vpc_offering.example: Creation complete after 0s [id=ebddd01c-3d32-4a35-8ac3-d9d47485865c]
Apply complete! Resources: 1 added, 0 changed, 0 destroyed.
terraform destroy
cloudstack_vpc_offering.example: Refreshing state... [id=ebddd01c-3d32-4a35-8ac3-d9d47485865c]
Terraform used the selected providers to generate the following execution plan. Resource actions are indicated
with the following symbols:
- destroy
Terraform will perform the following actions:
# cloudstack_vpc_offering.example will be destroyed
- resource "cloudstack_vpc_offering" "example" {
- display_text = "Example VPC Offering" -> null
- enable = true -> null
- for_nsx = false -> null
- id = "ebddd01c-3d32-4a35-8ac3-d9d47485865c" -> null
- internet_protocol = "IPv4" -> null
- is_default = false -> null
- name = "example-vpc-offering" -> null
- service_provider_list = {
- "Dhcp" = "VpcVirtualRouter"
- "Dns" = "VpcVirtualRouter"
- "Lb" = "VpcVirtualRouter"
- "NetworkACL" = "VpcVirtualRouter"
- "PortForwarding" = "VpcVirtualRouter"
- "SourceNat" = "VpcVirtualRouter"
- "StaticNat" = "VpcVirtualRouter"
- "UserData" = "VpcVirtualRouter"
} -> null
- specify_as_number = false -> null
- supported_services = [
- "Dhcp",
- "Dns",
- "Lb",
- "NetworkACL",
- "PortForwarding",
- "SourceNat",
- "StaticNat",
- "UserData",
] -> null
}
Plan: 0 to add, 0 to change, 1 to destroy.
Do you really want to destroy all resources?
Terraform will destroy all your managed infrastructure, as shown above.
There is no undo. Only 'yes' will be accepted to confirm.
Enter a value: yes
cloudstack_vpc_offering.example: Destroying... [id=ebddd01c-3d32-4a35-8ac3-d9d47485865c]
cloudstack_vpc_offering.example: Destruction complete after 1s
Destroy complete! Resources: 1 destroyed.
Depends on: apache/cloudstack-go#158
Can be tested by locally pointing to the local go repo by adding the following like to go.mod