Skip to content

Allow more generic searches of ACLs#9566

Merged
DaanHoogland merged 5 commits into
apache:mainfrom
shapeblue:ghi8859-allowSearchACLs
Sep 10, 2024
Merged

Allow more generic searches of ACLs#9566
DaanHoogland merged 5 commits into
apache:mainfrom
shapeblue:ghi8859-allowSearchACLs

Conversation

@DaanHoogland

Copy link
Copy Markdown
Contributor

Description

This PR...

Fixes: #8859

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)
  • build/CI
  • test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

@codecov

codecov Bot commented Aug 22, 2024

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 0% with 9 lines in your changes missing coverage. Please review.

Project coverage is 15.77%. Comparing base (f8d8a9c) to head (21f9cbc).
Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
...a/com/cloud/network/vpc/NetworkACLServiceImpl.java 0.00% 9 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #9566      +/-   ##
============================================
- Coverage     15.77%   15.77%   -0.01%     
  Complexity    12542    12542              
============================================
  Files          5623     5623              
  Lines        491611   491619       +8     
  Branches      60012    63123    +3111     
============================================
- Hits          77568    77565       -3     
- Misses       405584   405594      +10     
- Partials       8459     8460       +1     
Flag Coverage Δ
uitests 4.05% <ø> (-0.01%) ⬇️
unittests 16.59% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@DaanHoogland DaanHoogland changed the title Ghi8859 allow search ac ls Allow more generic searches of ACLs Aug 22, 2024
@DaanHoogland DaanHoogland marked this pull request as draft August 22, 2024 13:00
@DaanHoogland DaanHoogland force-pushed the ghi8859-allowSearchACLs branch from 4b33e4a to fae65ad Compare August 23, 2024 09:46
@DaanHoogland

Copy link
Copy Markdown
Contributor Author

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan

Copy link
Copy Markdown

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10778

@DaanHoogland

Copy link
Copy Markdown
Contributor Author

@blueorangutan test

@DaanHoogland DaanHoogland marked this pull request as ready for review August 23, 2024 14:05
@blueorangutan

Copy link
Copy Markdown

@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

@blueorangutan

Copy link
Copy Markdown

[SF] Trillian test result (tid-11162)
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 51662 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9566-t11162-kvm-ol8.zip
Smoke tests completed. 139 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File

@kiranchavala kiranchavala left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DaanHoogland currently the keyword search is working only for protocol field and description

aclsearch

can the keyword search improved to other parameters like

(cmk) > list networkacls -h
listNetworkACLs: Lists all network ACL items
API Params               Type     Description
==========               ====     ===========
account                  string   list resources by account. Must be used w
                                  ith the domainId parameter.
aclid                    uuid     list network ACL items by ACL ID
action                   string   list network ACL items by action
domainid                 uuid     list only resources belonging to the doma
                                  in specified
fordisplay               boolean  list resources by display flag; only ROOT
                                   admin is eligible to pass this paramete
                                  r
id                       uuid     Lists network ACL Item with the specified
                                   ID
isrecursive              boolean  defaults to false, but if true, lists all
                                   resources from the parent specified by
                                  the domainId till leaves.
keyword                  string   List by keyword
listall                  boolean  If set to false, list only resources belo
                                  nging to the command's caller; if set to
                                   true - list resources that the caller i
                                  s authorized to see. Default value is fa
                                  lse. Resources dedicated to a project ar
                                  e listed only if using the projectid par
                                  ameter.
networkid                uuid     list network ACL items by network ID
page                     integer
pagesize                 integer
projectid                uuid     list objects by project; if projectid=-1
                                  lists All VMs
protocol                 string   list network ACL items by protocol
tags                     map      List resources by tags (key/value pairs)
traffictype              string   list network ACL items by traffic type -
                                  ingress or egress

@DaanHoogland

Copy link
Copy Markdown
Contributor Author

@kiranchavala that would be a bigger effort, the search is no only for acl item fields. As for account, aclid, domain etc, those are implied (at least in the GUI). things like traffic_type and action, these are enumerals and require a different kind of selection, i.e. they are not free text searchable and hence are left out of scope.

@bernardodemarco bernardodemarco left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I manually tested in a local environment.

Regarding the listNetworkACLs API, the filtering is working as expected. I created four ACL rules and filtered them by protocol and description.

All ACL rules:
image

Filtering by description:
image

Filtering by protocol:
image

@DaanHoogland, what do you think about using one of the Ant Design components for the input field?


Regarding the listNetworkACLLists, when executing the API with the keyword parameter, apparently a NPE is not being handled.

(localcloud) 🐱 > list networkacllists vpcid="f9510528-eb2c-405f-b690-21a540c00661" keyword="acl"
🙈 Error: (HTTP 530, error code 4250) Caught error: null

Management server logs:

2024-09-01T16:09:42,439 ERROR [c.c.n.v.d.NetworkACLDaoImpl] (qtp1438988851-385:[ctx-49b0b1cf, ctx-54f34d08]) (logid:d0c03e10) Caught: com.mysql.cj.jdbc.ClientPreparedStatement: SELECT network_acl.id, network_acl.vpc_id, network_acl.uuid, network_acl.name, network_acl.description, network_acl.display FROM network_acl WHERE network_acl.vpc_id IN (1,0)  AND network_acl.display = 1  AND  (network_acl.name LIKE x'2561636c25'  OR network_acl.description LIKE x'2561636c25'  OR )  ORDER BY network_acl.id DESC  LIMIT 0, 500 java.lang.NullPointerException

2024-09-01T16:09:42,439 ERROR [c.c.a.ApiServer] (qtp1438988851-385:[ctx-49b0b1cf, ctx-54f34d08]) (logid:d0c03e10) unhandled exception executing api command: [Ljava.lang.String;@33af317 com.cloud.utils.exception.CloudRuntimeException: Caught error: null

@DaanHoogland

Copy link
Copy Markdown
Contributor Author

I manually tested in a local environment.

thanks @bernardodemarco

@DaanHoogland, what do you think about using one of the Ant Design components for the input field?

which one would you like to see implemented?

Regarding the listNetworkACLLists, when executing the API with the keyword parameter, apparently a NPE is not being handled.

(localcloud) 🐱 > list networkacllists vpcid="f9510528-eb2c-405f-b690-21a540c00661" keyword="acl"
🙈 Error: (HTTP 530, error code 4250) Caught error: null

Management server logs:

2024-09-01T16:09:42,439 ERROR [c.c.n.v.d.NetworkACLDaoImpl] (qtp1438988851-385:[ctx-49b0b1cf, ctx-54f34d08]) (logid:d0c03e10) Caught: com.mysql.cj.jdbc.ClientPreparedStatement: SELECT network_acl.id, network_acl.vpc_id, network_acl.uuid, network_acl.name, network_acl.description, network_acl.display FROM network_acl WHERE network_acl.vpc_id IN (1,0)  AND network_acl.display = 1  AND  (network_acl.name LIKE x'2561636c25'  OR network_acl.description LIKE x'2561636c25'  OR )  ORDER BY network_acl.id DESC  LIMIT 0, 500 java.lang.NullPointerException
2024-09-01T16:09:42,439 ERROR [c.c.a.ApiServer] (qtp1438988851-385:[ctx-49b0b1cf, ctx-54f34d08]) (logid:d0c03e10) unhandled exception executing api command: [Ljava.lang.String;@33af317 com.cloud.utils.exception.CloudRuntimeException: Caught error: null

thanks, will have a look

@bernardodemarco

Copy link
Copy Markdown
Member

which one would you like to see implemented?

I believe that the a-input or a-input-search components could be used, what do you think?

@DaanHoogland

Copy link
Copy Markdown
Contributor Author

@bernardodemarco took some time (priorities) but updated the PR now.

@DaanHoogland

Copy link
Copy Markdown
Contributor Author

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan

Copy link
Copy Markdown

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 11013

@DaanHoogland

Copy link
Copy Markdown
Contributor Author

@blueorangutan test

@blueorangutan

Copy link
Copy Markdown

@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

@blueorangutan

Copy link
Copy Markdown

[SF] Trillian test result (tid-11388)
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 54779 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9566-t11388-kvm-ol8.zip
Smoke tests completed. 138 look OK, 2 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File
test_mount_shared_fs Failure 86.12 test_sharedfs_lifecycle.py
test_resize_shared_fs Error 0.23 test_sharedfs_lifecycle.py
test_01_redundant_vpc_site2site_vpn Failure 442.09 test_vpc_vpn.py

@DaanHoogland

Copy link
Copy Markdown
Contributor Author

@kiranchavala @bernardodemarco is this ok by you? (cc @JoaoJandre )

@bernardodemarco bernardodemarco left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DaanHoogland, I've tested it again, and everything seems to be working well.

All ACL rules:
image


Filtering by protocol:
image


Filtering by description:
image


Execution of the listNetworkACLLists API with the keyword parameter:

(localcloud) 🐱 > list networkacllists keyword="vpc"
{
  "count": 1,
  "networkacllist": [
    {
      "description": "ACL 01",
      "fordisplay": true,
      "id": "e85bc061-1ffe-475a-92f0-a7dfbcfbd878",
      "name": "vpc-01-acl-01"
    }
  ]
}

@kiranchavala

Copy link
Copy Markdown
Member

@kiranchavala that would be a bigger effort, the search is no only for acl item fields. As for account, aclid, domain etc, those are implied (at least in the GUI). things like traffic_type and action, these are enumerals and require a different kind of selection, i.e. they are not free text searchable and hence are left out of scope.

Thanks @DaanHoogland we can revisit them in future if required

Comment thread server/src/main/java/com/cloud/network/vpc/NetworkACLServiceImpl.java Outdated
@vishesh92

Copy link
Copy Markdown
Member

@blueorangutan ui

@blueorangutan

Copy link
Copy Markdown

@vishesh92 a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.

@blueorangutan

Copy link
Copy Markdown

UI build: ✔️
Live QA URL: https://qa.cloudstack.cloud/simulator/pr/9566 (QA-JID-442)

Comment thread ui/src/views/network/AclListRulesTab.vue Outdated
@DaanHoogland DaanHoogland force-pushed the ghi8859-allowSearchACLs branch from 37b089f to 21f9cbc Compare September 9, 2024 12:22
@DaanHoogland

Copy link
Copy Markdown
Contributor Author

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan

Copy link
Copy Markdown

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 11057

@DaanHoogland

Copy link
Copy Markdown
Contributor Author

@blueorangutan test

@blueorangutan

Copy link
Copy Markdown

@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

@vishesh92 vishesh92 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. Tested on my local env with simulator. Search with keyword is working as expected.

@DaanHoogland DaanHoogland added this to the 4.20.0.0 milestone Sep 10, 2024
@DaanHoogland DaanHoogland merged commit b1f683d into apache:main Sep 10, 2024
@DaanHoogland DaanHoogland deleted the ghi8859-allowSearchACLs branch September 10, 2024 08:37
@blueorangutan

Copy link
Copy Markdown

[SF] Trillian test result (tid-11432)
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 64949 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9566-t11432-kvm-ol8.zip
Smoke tests completed. 138 look OK, 2 have errors, 1 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File
ContextSuite context=TestVMSchedule>:setup Error 0.00 test_vm_schedule.py
test_01_redundant_vpc_site2site_vpn Failure 742.59 test_vpc_vpn.py
test_01_redundant_vpc_site2site_vpn Error 742.60 test_vpc_vpn.py
ContextSuite context=TestRVPCSite2SiteVpn>:teardown Error 786.18 test_vpc_vpn.py
all_test_vm_life_cycle Skipped --- test_vm_life_cycle.py

dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Sep 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow users to Search ACL Rule

5 participants