Public IP quarantine feature by BryanMLima · Pull Request #7378 · apache/cloudstack

BryanMLima · 2023-03-29T12:27:08Z

Description

This PR aims to add the concept of quarantine for public IPs. The feature is fully described in issue #7369. The main goal of this functionality is to block the allocation of a disassociated public IP address by a configurable duration, public.ip.address.quarantine.duration. This can prevent the allocation of this recently disassociated IP to another user. More critically, this feature can prevent the allocation of an IP that has been blacklisted for inappropriate use by the previous owner.

Fixes: #7369

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)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

How Has This Been Tested?

I added some unit tests to validate the core logic of this feature. Moreover, I tested in a local lab the whole process of disassociation of a public IP address and trying to allocate different accounts with different roles. As designed, only the previous owner was able to allocate the quarantined IP. Furthermore, I tested if the previous behavior is maintained, as the default value for the global configuration public.ip.address.quarantine.duration is 0, no public IP address is added to quarantined, as expected.

github-actions · 2023-04-12T11:41:13Z

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

sonarqubecloud · 2023-04-20T14:27:53Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
15 Code Smells

16.6% Coverage
0.2% Duplication

DaanHoogland · 2023-04-28T08:14:02Z

SonarCloud Quality Gate failed.
0 Bugs 0 Vulnerabilities 0 Security Hotspots 15 Code Smells
16.6% Coverage 0.2% Duplication

@BryanMLima can you have a look at those code smells? most of those are not your doing but there are some easy wins there.

codecov · 2023-04-28T09:30:28Z

Codecov Report

Attention: 163 lines in your changes are missing coverage. Please review.

Comparison is base (bd52fa8) 29.20% compared to head (3fb9586) 29.13%.
Report is 50 commits behind head on main.

Files	Patch %	Lines
...n/java/com/cloud/network/IpAddressManagerImpl.java	48.48%	31 Missing and 3 partials ⚠️
.../cloudstack/api/response/IpQuarantineResponse.java	0.00%	26 Missing ⚠️
...ava/com/cloud/network/vo/PublicIpQuarantineVO.java	21.42%	22 Missing ⚠️
...src/main/java/com/cloud/api/ApiResponseHelper.java	0.00%	13 Missing ⚠️
...i/command/user/address/UpdateQuarantinedIpCmd.java	0.00%	12 Missing ⚠️
...i/command/user/address/RemoveQuarantinedIpCmd.java	0.00%	11 Missing ⚠️
...ain/java/com/cloud/api/query/QueryManagerImpl.java	63.33%	9 Missing and 2 partials ⚠️
...ain/java/com/cloud/network/NetworkServiceImpl.java	67.64%	11 Missing ⚠️
...pi/command/user/address/ListQuarantinedIpsCmd.java	0.00%	9 Missing ⚠️
...c/main/java/com/cloud/utils/db/GenericDaoBase.java	27.27%	7 Missing and 1 partial ⚠️
... and 3 more

Additional details and impacted files

@@ Coverage Diff @@## main #7378 +/- ##
============================================
- Coverage 29.20% 29.13% -0.07% - Complexity 30891 31034 +143 
============================================
Files 5150 5192 +42 Lines 363420 366141 +2721 Branches 53230 53529 +299 ============================================
+ Hits 106144 106690 +546 - Misses 242730 244844 +2114 - Partials 14546 14607 +61

Flag	Coverage Δ
simulator-marvin-tests	`25.06% <28.21%> (-0.19%)`	⬇️
uitests	`4.49% <0.00%> (-0.06%)`	⬇️
unit-tests	`14.81% <28.70%> (+0.12%)`	⬆️

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.

github-actions · 2023-05-01T18:54:04Z

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

blueorangutan · 2023-10-26T07:29:51Z

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

Test	Result	Time (s)	Test File
test_another_user_can_allocate_ip_after_quarantined_has_ended_network	`Error`	4.78	test_quarantined_ips.py
test_another_user_can_allocate_ip_after_quarantined_has_ended_vpc	`Error`	0.32	test_quarantined_ips.py
test_only_owner_can_allocate_ip_in_quarantine_network	`Error`	1.48	test_quarantined_ips.py
test_only_owner_can_allocate_ip_in_quarantine_vpc	`Error`	0.33	test_quarantined_ips.py
test_DeleteDomain	`Error`	23.21	test_accounts.py
test_forceDeleteDomain	`Failure`	20.25	test_accounts.py
ContextSuite context=TestPublicIP>:setup	`Error`	11.14	test_network.py
test_reboot_router	`Error`	9.51	test_network.py
test_releaseIP	`Error`	9.36	test_network.py
ContextSuite context=TestRouterRules>:setup	`Error`	18.73	test_network.py
test_DeployVmAffinityGroup	`Error`	7.43	test_affinity_groups.py
test_network_acl	`Error`	1.15	test_network_acl.py
ContextSuite context=TestNatRuleUsage>:setup	`Error`	32.97	test_usage.py
ContextSuite context=TestPublicIPUsage>:setup	`Error`	42.05	test_usage.py
ContextSuite context=TestSnapshotUsage>:setup	`Error`	51.13	test_usage.py
ContextSuite context=TestVmUsage>:setup	`Error`	80.07	test_usage.py
ContextSuite context=TestVpnUsage>:setup	`Error`	231.55	test_usage.py
test_02_upgrade_kubernetes_cluster	`Failure`	568.20	test_kubernetes_clusters.py
test_08_upgrade_kubernetes_ha_cluster	`Failure`	662.43	test_kubernetes_clusters.py
ContextSuite context=TestSnapshotStandaloneBackup>:setup	`Error`	99.11	test_snapshots.py
ContextSuite context=TestSetSourceNatIp>:setup	`Error`	0.00	test_set_sourcenat.py
test_01_native_to_native_network_migration	`Error`	22.01	test_migration.py
test_02_native_to_native_vpc_migration	`Error`	4.15	test_migration.py
test_04_deploy_vm_for_other_user_and_test_vm_operations	`Failure`	46.32	test_network_permissions.py
ContextSuite context=TestNetworkPermissions>:teardown	`Error`	1.37	test_network_permissions.py
test_02_vpc_privategw_static_routes	`Failure`	3.05	test_privategw_acl.py
test_03_vpc_privategw_restart_vpc_cleanup	`Failure`	3.03	test_privategw_acl.py
test_04_rvpc_privategw_static_routes	`Failure`	4.16	test_privategw_acl.py
ContextSuite context=TestRouterDHCPOpts>:setup	`Error`	113.83	test_router_dhcphosts.py
test_02_routervm_iptables_policies	`Error`	4.34	test_routers_iptables_default_policy.py
test_01_single_VPC_iptables_policies	`Error`	1.21	test_routers_iptables_default_policy.py
test_01_isolate_network_FW_PF_default_routes_egress_true	`Error`	5.48	test_routers_network_ops.py
test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true	`Error`	0.26	test_routers_network_ops.py
test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false	`Error`	0.28	test_routers_network_ops.py
test_03_RVR_Network_check_router_state	`Error`	0.24	test_routers_network_ops.py
ContextSuite context=TestServiceOfferings>:setup	`Error`	9.86	test_service_offerings.py
ContextSuite context=TestTemplates>:setup	`Error`	45.35	test_templates.py
ContextSuite context=TestVmAutoScaling>:setup	`Error`	0.00	test_vm_autoscaling.py
test_01_secure_vm_migration	`Error`	69.27	test_vm_life_cycle.py
ContextSuite context=TestVMLifeCycle>:setup	`Error`	9.37	test_vm_life_cycle.py
ContextSuite context=TestVolumes>:setup	`Error`	7.40	test_volumes.py
test_02_redundant_VPC_default_routes	`Error`	4.14	test_vpc_redundant.py
test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers	`Error`	3.19	test_vpc_redundant.py
test_04_rvpc_network_garbage_collector_nics	`Error`	4.08	test_vpc_redundant.py
test_05_rvpc_multi_tiers	`Error`	3.08	test_vpc_redundant.py
test_01_VPC_nics_after_destroy	`Error`	3.17	test_vpc_router_nics.py
test_01_vpc_site2site_vpn_multiple_options	`Failure`	1.31	test_vpc_vpn.py
test_01_vpc_site2site_vpn_multiple_options	`Error`	1.32	test_vpc_vpn.py
test_01_vpc_remote_access_vpn	`Failure`	0.09	test_vpc_vpn.py
test_01_vpc_site2site_vpn	`Failure`	3.34	test_vpc_vpn.py
test_01_vpc_site2site_vpn	`Error`	3.35	test_vpc_vpn.py
test_02_cancel_host_maintenace_with_migration_jobs	`Error`	0.45	test_host_maintenance.py
test_03_cancel_host_maintenace_with_migration_jobs_failure	`Error`	0.49	test_host_maintenance.py
test_01_cancel_host_maintenance_ssh_enabled_agent_connected	`Failure`	1.16	test_host_maintenance.py
test_03_cancel_host_maintenance_ssh_disabled_agent_connected	`Failure`	0.15	test_host_maintenance.py
test_04_cancel_host_maintenance_ssh_disabled_agent_disconnected	`Failure`	0.11	test_host_maintenance.py

DaanHoogland · 2023-10-27T08:01:18Z

@BryanMLima , can you resolve the conflicts?

shwstppr · 2023-10-27T20:59:16Z

@blueorangutan package

blueorangutan · 2023-10-27T21:00:03Z

@shwstppr 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 · 2023-10-27T21:58:12Z

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

shwstppr · 2023-10-28T12:20:16Z

@blueorangutan test

blueorangutan · 2023-10-28T12:22:03Z

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

blueorangutan · 2023-10-29T00:58:28Z

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

Test	Result	Time (s)	Test File
test_another_user_can_allocate_ip_after_quarantined_has_ended_vpc	`Error`	5.11	test_quarantined_ips.py
test_only_owner_can_allocate_ip_in_quarantine_vpc	`Error`	5.05	test_quarantined_ips.py

shwstppr · 2023-10-30T06:02:33Z

@BryanMLima can you please check the failing smoke test?

shwstppr · 2023-11-03T10:10:34Z

@BryanMLima any update on this?

BryanMLima · 2023-11-07T17:50:58Z

@shwstppr sorry for the delay, I will have time to work on this later this week.

BryanMLima · 2023-11-10T18:29:48Z

@blueorangutan package

blueorangutan · 2023-11-10T18:30:03Z

@BryanMLima 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 · 2023-11-10T19:35:06Z

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

DaanHoogland · 2023-11-13T08:18:42Z

@blueorangutan test

blueorangutan · 2023-11-13T08:20:03Z

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

…s ID

BryanMLima · 2023-11-14T18:56:23Z

Hi @BryanMLima
The feature is nice
Can the following api's have an additional parameter of "ipaddress"
So that the user can easily provide the ip address instead of providing the id
1. update quarantinedip
2. remove quarantinedip
Similar to The associateIPaddress api call which has ipaddress parameter
https://cloudstack.apache.org/api/apidocs-4.18/apis/associateIpAddress.html

@kiranchavala, I addressed this enhancement, now it is possible to inform the ID of the quarantine or the address of the public IP in quarantine. It will be useful for the UI implementation of this functionality in the future.

DaanHoogland · 2023-11-15T08:24:15Z

test results:

Smoke tests completed. 116 look OK, 2 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_01_invalid_upgrade_kubernetes_cluster	`Failure`	3605.01	test_kubernetes_clusters.py
test_02_upgrade_kubernetes_cluster	`Failure`	3617.44	test_kubernetes_clusters.py
test_03_deploy_and_scale_kubernetes_cluster	`Failure`	0.06	test_kubernetes_clusters.py
test_04_autoscale_kubernetes_cluster	`Failure`	0.04	test_kubernetes_clusters.py
test_05_basic_lifecycle_kubernetes_cluster	`Failure`	0.03	test_kubernetes_clusters.py
test_06_delete_kubernetes_cluster	`Failure`	0.05	test_kubernetes_clusters.py
test_07_deploy_kubernetes_ha_cluster	`Failure`	0.04	test_kubernetes_clusters.py
test_08_upgrade_kubernetes_ha_cluster	`Failure`	0.04	test_kubernetes_clusters.py
test_09_delete_kubernetes_ha_cluster	`Failure`	0.04	test_kubernetes_clusters.py
test_10_vpc_tier_kubernetes_cluster	`Failure`	44.61	test_kubernetes_clusters.py
test_11_test_unmanaged_cluster_lifecycle	`Error`	1.22	test_kubernetes_clusters.py
ContextSuite context=TestKubernetesCluster>:teardown	`Error`	50.34	test_kubernetes_clusters.py
test_hostha_enable_ha_when_host_in_maintenance	`Error`	303.87	test_hostha_kvm.py

I will manually run the hostha test to make sure it is not related to the changes here.

DaanHoogland · 2023-11-15T09:29:09Z

in a clean env:

Tests Enable HA when host is in Maintenance mode, should be Ineligible ... === TestName: test_hostha_enable_ha_when_host_in_maintenance | Status : SUCCESS ===

BryanMLima added 5 commits March 24, 2023 15:42

Add IP quarantine feature
308247a

Small fixes
7f1ee45

Fix unit tests
6774f94

Merge branch 'main' of github.com:apache/cloudstack into ip-quarantine
1a08248

Only add IP to quarantine if config is bigger than 0
366d5f7

boring-cyborgBot added component:api component:database component:management-server component:networking component:UI component:vpc Python Warning... Python code Ahead! labels Mar 29, 2023

Change since version to 4.18
3ebaa1f

github-actionsBot added the status:has-conflicts label Apr 12, 2023

Merge branch 'main' of github.com:apache/cloudstack into ip-quarantine
1bd60dc

github-actionsBot removed the status:has-conflicts label Apr 18, 2023

BryanMLima added 3 commits April 20, 2023 10:00

Remove unecessary API_NAMES vars
2e2052b

Address code smells considering the PR scope
c867384

Fix checkstyles
4dcbd90

DaanHoogland reviewed Apr 28, 2023
View reviewed changes

github-actionsBot added the status:has-conflicts label May 1, 2023

BryanMLima added 3 commits May 19, 2023 13:34

Merge branch 'main' of github.com:apache/cloudstack into ip-quarantine
6ae190d

Fix duplicate quarantine IP DB entry and address Daan review
287dace

Fix duplicate quarantine IP DB entry and address Daan review
2be1f71

github-actionsBot removed the status:has-conflicts label May 19, 2023

BryanMLima added 2 commits October 27, 2023 17:08

Reset quarantine config on smoke test tear down
ebb75c1

Merge branch 'main' of github.com:apache/cloudstack into ip-quarantine
998e3e0

github-actionsBot removed the status:has-conflicts label Oct 27, 2023

Allocate specified source NAT IP only if user informed
5325f56

BryanMLima added 3 commits November 13, 2023 18:09

Allow removing IP from quarantine informing its address as well as it…
00b28ca
…s ID

Address NPE
301b20f

Allow updating IP quarantine informing its address as well as its ID
3fb9586

DaanHoogland merged commit 1f29f6f into apache:mainNov 15, 2023

dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Nov 16, 2023
Public IP quarantine feature (apache#7378)
ece08ff

DaanHoogland removed the status:needs-testing label Jan 28, 2026

kiranchavala mentioned this pull request Mar 12, 2026
UI : Show list of quarantined ips #12797
Open

Uh oh!

Conversation

BryanMLima commented Mar 29, 2023

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

How Has This Been Tested?

Uh oh!

github-actionsBot commented Apr 12, 2023

Uh oh!

sonarqubecloudBot commented Apr 20, 2023

Uh oh!

DaanHoogland commented Apr 28, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecovBot commented Apr 28, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actionsBot commented May 1, 2023

Uh oh!

blueorangutan commented Oct 26, 2023

Uh oh!

DaanHoogland commented Oct 27, 2023

Uh oh!

shwstppr commented Oct 27, 2023

Uh oh!

blueorangutan commented Oct 27, 2023

Uh oh!

blueorangutan commented Oct 27, 2023

Uh oh!

shwstppr commented Oct 28, 2023

Uh oh!

blueorangutan commented Oct 28, 2023

Uh oh!

blueorangutan commented Oct 29, 2023

Uh oh!

shwstppr commented Oct 30, 2023

Uh oh!

shwstppr commented Nov 3, 2023

Uh oh!

BryanMLima commented Nov 7, 2023

Uh oh!

BryanMLima commented Nov 10, 2023

Uh oh!

blueorangutan commented Nov 10, 2023

Uh oh!

blueorangutan commented Nov 10, 2023

Uh oh!

DaanHoogland commented Nov 13, 2023

Uh oh!

blueorangutan commented Nov 13, 2023

Uh oh!

BryanMLima commented Nov 14, 2023

Uh oh!

DaanHoogland commented Nov 15, 2023

Uh oh!

DaanHoogland commented Nov 15, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

codecovBot commented Apr 28, 2023•
edited
Loading