Soft delete port forwarding, load balancing and firewall rules#13015
Soft delete port forwarding, load balancing and firewall rules#13015bernardodemarco wants to merge 2 commits into
Conversation
|
@blueorangutan package |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #13015 +/- ##
============================================
- Coverage 18.09% 18.08% -0.01%
+ Complexity 16723 16717 -6
============================================
Files 6037 6037
Lines 542580 542632 +52
Branches 66427 66433 +6
============================================
- Hits 98155 98137 -18
- Misses 433399 433468 +69
- Partials 11026 11027 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@blueorangutan package |
|
@bernardodemarco a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 17475 |
| CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.nics','enabled', 'TINYINT(1) NOT NULL DEFAULT 1 COMMENT ''Indicates whether the NIC is enabled or not'' '); | ||
|
|
||
| -- Soft delete port forwarding, load balancing and firewall rules | ||
| CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.firewall_rules', 'removed', 'datetime DEFAULT NULL'); |
There was a problem hiding this comment.
it is a very good idea to add removed column to firewall rules/lbs/pf rules.
for other tables, I suspect if the changes are necessary
There was a problem hiding this comment.
@weizhouapache, thanks for your review!
Regarding the tables firewall_rules, load_balancer_vm_map, load_balancer_cert_map, load_balancer_healthcheck_policies, and load_balancer_stickiness_policies, we should keep the removed column to preserve a complete overview of the LB, FW and PF metadata.
However, for the tables global_load_balancer_lb_rule_map, elastic_lb_vm_map, tungsten_lb_health_monitor, and global_load_balancing_rules, I believe they are related to legacy ACS features. Personally, I have never seen them being used. I added the removed column to maintain consistency with the existing standard/pattern, but I can remove these changes if you think that would be preferable. Let me know your thoughts on this.
bernardodemarco
force-pushed
the
soft-delete-pf-lb-and-fw-rules
branch
from
cfe94ff to
5fc9ad7
Compare
|
maybe I am missing something @bernardodemarco . If we “only” mark as removed, should we add some garbage collection? |
|
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
|
Hello, @DaanHoogland and @weizhouapache Sorry for the delay in answering your reviews here. I'll try to answer them and to troubleshoot these simulator tests errors during this week |
bernardodemarco
force-pushed
the
soft-delete-pf-lb-and-fw-rules
branch
from
5fc9ad7 to
29e86a4
Compare
@DaanHoogland, yes, we could. Currently, almost every delete/remove API in ACS only marks resources as removed instead of expunging them from the DB. I personally prefer this approach because, in recovery scenarios, it makes it much easier to retrieve the desired metadata. Additionally, the core idea of the current PR is to keep PF, LB and FW metadata in the DB after their removal, enabling cloud providers to properly investigate and analyze users' past network usage. Therefore, specially for these resources, it is even more important to not expunge them. If desired, one viable approach would be to extend the |
|
@blueorangutan package |
|
@bernardodemarco a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress. |
4 participants
Description
Currently, when port forwarding, load balancing and firewall rules are removed, their records are permanently deleted from the database. This makes it impossible to track and audit previously applied rules after their removal.
Therefore, this PR proposes to, instead of deleting the records associated with such network rules, only marking them as removed. To achieve that, a
removedcolumn has been added to the tables that store the rules metadata, allowing to track the exact date and time when a rule was removed.Maintaining these records is essential for auditing and traceability purposes. It ensures that historical network configurations remain accessible for cloud administrators/operators, enabling proper investigation, compliance verification and analysis of past behavior, even after rules are no longer active.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Screenshots (if appropriate):
How Has This Been Tested?
Egress rules (ER)
Records persisted in the database after the tests
Firewall (FW)
Records persisted in the database after the tests
Port forwarding (PF)
Records persisted in the database after the tests
Public Load balancing (LB)
Records persisted in the database after the tests
Internal Load balancing (LB)
Records persisted in the database after the tests