apache / cloudstack-terraform-provider

CloudStack Terraform Provider
https://cloudstack.apache.org
Apache License 2.0
28 stars 34 forks source link

Support 'Descriptions' & 'Rule #' in ACL Rule #106

Open btzq opened 5 months ago

btzq commented 5 months ago

According the Cloudstack GUI, ACL Rule has a description field.

But it is not available in the Terraform Provider.

Would like to request that this value be made available in the provider so that we can version control our ACL Rules, and also make it easy to read via the GUI.

Screenshot 2024-03-26 at 10 40 09 pm
CodeBleu commented 5 months ago

@btzq @kiranchavala Any issue with updating this Issue description to be:

Support 'Descriptions' & 'Rule #' in ACL Rule

I can open another issue if needed, but to me it makes more sense to keep this together

Being able to specify the Rule # is currently not avail in TF either, but is in the UI

image

weizhouapache commented 5 months ago

@btzq @CodeBleu I just want to point out that the UI and api are inconsistent.

'Rule #' on UI = 'number' in Api 'Description' on UI = 'reason' in Api

https://cloudstack.apache.org/api/apidocs-4.19/apis/createNetworkACL.html

btzq commented 5 months ago

HI @CodeBleu , renaming it to your proposed title sounds okay to me. It makes more sense too.

Noted @weizhouapache. But this is not a showstopper is it? Although behind the scenes these 2 are different APIs, the user experience can be made transparent?

kiranchavala commented 4 months ago

@btzq

Thanks for reporting the issue

Will, mark it as an improvement request to support description parameter to acl rule

https://registry.terraform.io/providers/cloudstack/cloudstack/latest/docs/resources/network_acl_rule

CodeBleu commented 3 months ago

HI @CodeBleu , renaming it to your proposed title sounds okay to me. It makes more sense too.

Noted @weizhouapache. But this is not a showstopper is it? Although behind the scenes these 2 are different APIs, the user experience can be made transparent?

@btzq Do you mind going ahead and updating the description to :Support 'Descriptions' & 'Rule #' in ACL Rule

CodeBleu commented 3 months ago

@btzq Is this a duplicate? https://github.com/apache/cloudstack-terraform-provider/issues/104

btzq commented 3 months ago

@CodeBleu

They are similar. But #104 is more for supporting Terraform Import for 'ACL Rules'.

106 is for support Descriptions in ACL List.

I could remove the 'Support Addint Descriptions for ACL Rules' from #104 to avoid confusion?

Also updated ticket title as requested.

CodeBleu commented 3 months ago

I wish I new more Go so I could write a proper go test for this. I was able to do a basic test of adding descriptions to Rules and also adding Rules with a Rule ID # ( as along as the existing Rule ID # doesn't exist). Updating existing rule with a Rule ID # that already exists produces the following:

  • CloudStack API error 431 (CSExceptionErrorCode: 4350): ACL item with number 1 already exists in ACL: 8590d921-9972-4a71-9d28-e4b5616faed2

I would have submitted a PR if I had some better logic to handle existing Rule ID #'s and proper testing, but since I'm a n00b at this Go stuff I figured I'd at least post the git diff patch :smile:

diff --git a/cloudstack/resource_cloudstack_network_acl_rule.go b/cloudstack/resource_cloudstack_network_acl_rule.go
index f2daac7..12590ac 100644
--- a/cloudstack/resource_cloudstack_network_acl_rule.go
+++ b/cloudstack/resource_cloudstack_network_acl_rule.go
@@ -63,6 +63,16 @@ func resourceCloudStackNetworkACLRule() *schema.Resource {
                            Default:  "allow",
                        },

+                       "description": {
+                           Type:     schema.TypeString,
+                           Optional: true,
+                       },
+
+                       "rule_id": {
+                           Type:     schema.TypeInt,
+                           Optional: true,
+                       },
+
                        "cidr_list": {
                            Type:     schema.TypeSet,
                            Required: true,
@@ -198,6 +208,14 @@ func createNetworkACLRule(d *schema.ResourceData, meta interface{}, rule map[str
    // Create a new parameter struct
    p := cs.NetworkACL.NewCreateNetworkACLParams(rule["protocol"].(string))

+   if desc, ok := rule["description"]; ok {
+       p.SetReason(desc.(string))
+   }
+
+   if ruleId, ok := rule["rule_id"]; ok {
+       p.SetNumber(ruleId.(int))
+   }
+
    // Set the acl ID
    p.SetAclid(d.Id())