Copilot commented on code in PR #280:
URL:
https://github.com/apache/cloudstack-terraform-provider/pull/280#discussion_r2910254578
##########
cloudstack/resource_cloudstack_instance.go:
##########
@@ -516,6 +541,11 @@ func resourceCloudStackInstanceRead(d
*schema.ResourceData, meta interface{}) er
d.Set("display_name", vm.Displayname)
d.Set("group", vm.Group)
+ // Set the project if the instance belongs to one
+ if vm.Project != "" {
+ d.Set("project", vm.Project)
+ }
Review Comment:
In resourceCloudStackInstanceRead, setting `project` before calling
`setValueOrID(d, "project", ...)` breaks the "preserve ID vs name" behavior: if
the user configured `project` as an ID, this `d.Set` overwrites it with the
project name, causing `setValueOrID` to treat it as a name and persist the name
instead of the ID. Remove this early `d.Set("project", vm.Project)` (or gate it
on `d.Get("project") == ""`) and rely on the existing `setValueOrID(d,
"project", vm.Project, vm.Projectid)` later in the function.
##########
website/docs/r/network.html.markdown:
##########
@@ -61,7 +82,9 @@ The following arguments are supported:
`none`, this will force a new resource to be created. (defaults `none`)
* `project` - (Optional) The name or ID of the project to deploy this
- instance to. Changing this forces a new resource to be created.
+ instance to. Changing this forces a new resource to be created. If not
+ specified and `vpc_id` is provided, the project will be automatically
+ inherited from the VPC.
Review Comment:
The argument reference for `project` says "deploy this instance to", but
this is the Network resource. Please adjust the wording to refer to the
network/resource rather than an instance.
##########
website/docs/r/network_acl.html.markdown:
##########
@@ -30,7 +53,8 @@ The following arguments are supported:
new resource to be created.
* `project` - (Optional) The name or ID of the project to deploy this
- instance to. Changing this forces a new resource to be created.
+ instance to. Changing this forces a new resource to be created. If not
+ specified, the project will be automatically inherited from the VPC.
Review Comment:
The argument reference describes `project` as deploying an "instance", but
this is the Network ACL resource. Please update the wording to refer to the
ACL/resource (e.g., "deploy this resource to") to avoid confusing users.
##########
website/docs/r/port_forward.html.markdown:
##########
@@ -25,6 +27,38 @@ resource "cloudstack_port_forward" "default" {
}
```
+### Port Forward with Automatic Project Inheritance
+
+```hcl
+# Create a VPC in a project
+resource "cloudstack_vpc" "project_vpc" {
+ name = "project-vpc"
+ cidr = "10.0.0.0/16"
+ vpc_offering = "Default VPC offering"
+ project = "my-project"
+ zone = "zone-1"
+}
+
+# IP address automatically inherits project from VPC
+resource "cloudstack_ipaddress" "project_ip" {
+ vpc_id = cloudstack_vpc.project_vpc.id
+ zone = "zone-1"
+}
+
+# Port forward automatically inherits project from IP address
+resource "cloudstack_port_forward" "project_forward" {
+ ip_address_id = cloudstack_ipaddress.project_ip.id
+ # project is automatically inherited from the IP address
+
+ forward {
+ protocol = "tcp"
+ private_port = 80
+ public_port = 8080
+ virtual_machine_id = cloudstack_instance.web.id
+ }
Review Comment:
This new example references `cloudstack_instance.web.id` but does not define
a `cloudstack_instance.web` resource (or otherwise explain it), so it cannot be
applied as-is. Consider either adding a minimal instance definition in the
example or using a placeholder/literal ID (consistent with the basic example)
to keep the docs copy/pasteable.
##########
website/docs/r/ipaddress.html.markdown:
##########
@@ -35,7 +86,10 @@ The following arguments are supported:
acquired and associated. Changing this forces a new resource to be created.
* `project` - (Optional) The name or ID of the project to deploy this
- instance to. Changing this forces a new resource to be created.
+ instance to. Changing this forces a new resource to be created. If not
+ specified and `vpc_id` is provided, the project will be automatically
+ inherited from the VPC. If not specified and `network_id` is provided,
+ the project will be automatically inherited from the network.
Review Comment:
The argument reference for `project` says "deploy this instance to", but
this is the IP address resource. Please adjust the wording to refer to the IP
address/resource rather than an instance.
##########
cloudstack/resource_cloudstack_network_acl_test.go:
##########
@@ -110,6 +133,19 @@ func testAccCheckCloudStackNetworkACLBasicAttributes(
}
}
+func testAccCheckCloudStackNetworkACLProjectInherited(
+ acl *cloudstack.NetworkACLList) resource.TestCheckFunc {
+ return func(s *terraform.State) error {
+ // The ACL itself doesn't have project info, but we verify it
was created
+ // successfully which means it inherited the project from the
VPC
+ if acl.Name != "terraform-acl" {
+ return fmt.Errorf("Expected ACL name to be
'terraform-acl', got: %s", acl.Name)
+ }
+
+ return nil
+ }
Review Comment:
`testAccCheckCloudStackNetworkACLProjectInherited` doesn't actually validate
project inheritance (it only checks the ACL name). Since the state assertion
already checks `project`, this helper is redundant/misleading; either drop it
or update it to validate the inherited project via a reliable API signal (e.g.,
look up the VPC and assert its project matches the state value).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
