Skip to content

Network ACL entries are shared per tier but deleted per Service, taking other Services' ports down #107

Description

@vishesh92

Summary

On a VPC tier the controller opens a port with a Network ACL entry matched only on protocol and
port. The entry is shared by every LoadBalancer Service on that tier, and by anything an operator
added by hand, but cleanup deletes it whenever any one Service stops using that port. The other
Services lose ingress on that port until their next reconcile recreates the entry.

Impact

  • Two Services on one tier both exposing TCP/443 on their own public IPs share one ACL entry.
    Deleting either Service, or moving it off port 443, removes the entry. Traffic to the other
    Service on 443 is dropped until its next reconcile.
  • An entry created by an operator for the same protocol and port is adopted rather than
    duplicated, then deleted on the same trigger. If the operator's entry had a restricted CIDR, the
    controller recreates it as 0.0.0.0/0 on the next reconcile, silently widening exposure.

Firewall rules on isolated networks have the same ownership model but are scoped to the
Service's own public IP, so they only affect that Service.

Reproduction

  1. VPC tier with a custom ACL list attached (not default_allow / default_deny).
  2. Create Service A, type: LoadBalancer, port 443.
  3. Create Service B, type: LoadBalancer, port 443. listNetworkACLs shows a single TCP/443
    entry on the tier.
  4. Delete Service A.
  5. The TCP/443 entry is gone. Service B does not answer on 443 until its next reconcile.

Cause

updateNetworkACL lists the tier's entries and skips creation if any entry matches the IP
protocol and port, so entries are shared and carry no marker of who created them.
deleteNetworkACLRule lists the same way and deletes the first match. It is called from:

  • EnsureLoadBalancerDeleted, for every rule of the Service being deleted;
  • the prune path in EnsureLoadBalancer (pruneNetworkACLRule), for a rule the Service no
    longer needs. Since Fix protocol changes on existing load balancer rules #104 an entry the same Service still claims is kept and the delete is
    scoped to the rule's own network, but nothing considers other Services.

This is the design the ACL path shipped with in #69 (July 2024). The cleanup loop it replaced in
#104 deleted the same way, so #104 narrowed the problem without fixing it. Raised by review on
#104: .

Proposed fix

Tag what the controller creates and delete only what it tagged. CloudStack resource tags support
NetworkACL, FirewallRule and PublicIpAddress.

  • On create, tag the ACL entry (and firewall rule) with the owning Service, for example
    cloudstack-ccm/service=<service UID>.
  • Stop sharing entries: create one entry per Service even when a matching one exists, so each
    entry has exactly one owner. Verify CloudStack accepts same-protocol/port items in one ACL list;
    if it does not, fall back to one tag key per Service on the shared entry and delete only when the
    last key is removed.
  • On cleanup, delete only entries tagged with this Service. Untagged entries are left alone, which
    also stops the controller from deleting or widening operator-created entries.
  • Existing untagged entries created by older controller versions need a one-time adoption rule,
    for example tag on first match during reconcile.

A cheaper mitigation without tags is to list the tier's load balancer rules before deleting and
skip the delete if any other rule uses the same protocol and port. It closes the cross-Service
case but not the operator-created case, and it races with concurrent reconciles.

Related

The same tagging covers the public IP ownership gaps left over from #105: an IP whose rule was
deleted but whose release failed is never retried, and the pre-release check counts only load
balancer rules so an IP carrying port forwarding or static NAT can be released. Those can be
folded into this issue or filed separately.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions