Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
btzq opened this issue Mar 26, 2024 · 8 comments
Open

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

btzq opened this issue Mar 26, 2024 · 8 comments
Labels
enhancement New feature or request
Milestone

Comments

@btzq
Copy link

btzq commented Mar 26, 2024

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
@kiranchavala kiranchavala added this to the v0.6.0 milestone Mar 27, 2024
@CodeBleu
Copy link
Collaborator

@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
Copy link
Member

@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
Copy link
Author

btzq commented Apr 12, 2024

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
Copy link
Collaborator

@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

@kiranchavala kiranchavala added the enhancement New feature or request label Apr 29, 2024
@CodeBleu
Copy link
Collaborator

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
Copy link
Collaborator

@btzq Is this a duplicate? #104

@btzq btzq changed the title Support 'Descriptions' in ACL Rule Support 'Descriptions' & 'Rule #' in ACL Rule May 31, 2024
@btzq
Copy link
Author

btzq commented May 31, 2024

@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
Copy link
Collaborator

CodeBleu commented Jun 4, 2024

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 😄

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())

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants