Skip to content

Add firewall rules for Services on tiers of a VPC with the Firewall service #115

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.

Sign up for GitHub

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

Open
adestis-mf wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add firewall rules for Services on tiers of a VPC with the Firewall service #115

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2a25232
Add firewall rules for Services on tiers of a VPC with the Firewall s…
adestis-mf Nov 12, 2026
466eeab
Keep the refused VPC lookups in a sync.Map value of CSCloud
adestis-mf Nov 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 65 additions & 10 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,14 @@ On VPC tiers the controller also calls `updateNetworkACLItem`, to take over Netw
earlier releases created (see [Per-Service Network ACL rules](#per-service-network-acl-rules)).
Without it the controller creates its own rules and leaves the old ones for you to delete.

From CloudStack 4.23 on, the controller also calls `listVPCs` on VPC tiers, to find out whether the
VPC has the Firewall service (see [VPC Networks](#vpc-networks)). If the account may not call it,
the controller manages Network ACL rules only, as earlier releases did, and logs a warning once per
VPC. As of CloudStack 4.23, the role that CloudStack's Kubernetes service creates for its accounts
in projects (`Project Kubernetes Service Role`) includes neither `listVPCs` nor
`listNetworkACLLists`, which the controller needs to manage Network ACL rules. Add them to that
role, before the rule that denies everything else.

To create the secret, use the following command:
```bash
kubectl -n kube-system create secret generic cloudstack-secret --from-file=cloud-config
Expand Down Expand Up @@ -320,18 +328,18 @@ annotation by hand — doing so can leak a public IP or release one that you all
### Restricting Source Traffic

Traffic is filtered at two independent layers, which are configured separately. The second layer is
either a firewall rule or a Network ACL rule, depending on what the network offers:
a firewall rule, a Network ACL rule, or both, depending on the network:

| Layer | Configured by | Default |
| --- | --- | --- |
| CloudStack load balancer rule | `service.beta.kubernetes.io/cloudstack-load-balancer-source-cidrs` annotation | `0.0.0.0/0` |
| Firewall rule — isolated networks, and VPC networks that offer the Firewall service | `spec.loadBalancerSourceRanges` | `0.0.0.0/0` |
| Network ACL rule — VPC networks without the Firewall service | Not configurable | `0.0.0.0/0` |
| Firewall rule — isolated networks, and VPC tiers where the tier or its VPC offers the Firewall service | `spec.loadBalancerSourceRanges` | `0.0.0.0/0` |
| Network ACL rule — VPC tiers whose own offering does not include the Firewall service | Not configurable | `0.0.0.0/0` |

Traffic has to be allowed by both layers, so where firewall rules are used, either setting alone is
Traffic has to be allowed by every layer, so where firewall rules are used, either setting alone is
enough to block unwanted sources. Setting both keeps the two rules consistent in CloudStack.
Where Network ACL rules are used, `spec.loadBalancerSourceRanges` has no effect and the annotation
is the only way to restrict sources.
Where only Network ACL rules are used, `spec.loadBalancerSourceRanges` has no effect and the
annotation is the only way to restrict sources.

The two layers turn traffic away differently. The firewall discards the packets on the virtual
router, so a blocked client simply times out. The load balancer rule lets the connection be
Expand Down Expand Up @@ -405,10 +413,13 @@ controller creates **Network ACL** rules instead of firewall rules for the manag
associates public IPs with the VPC rather than with the network.

CloudStack 4.23 adds support for firewall rules on public IPs in VPC networks. This requires a
network offering that includes the Firewall service and is not enabled by default. The controller
chooses the mechanism based on the services the network offers, so on such networks it manages
firewall rules instead of Network ACL rules, and `spec.loadBalancerSourceRanges` is applied to
them.
VPC offering that includes the Firewall service and is not enabled by default. A tier lists the
service only if its own network offering includes it, and the VPC's virtual router drops traffic to
a public IP with a load balancer rule unless a firewall rule allows it. On a tier the controller
therefore reads the VPC with `listVPCs`, and if the VPC has the Firewall service, it manages a
firewall rule on the public IP in addition to the tier's Network ACL rule, with
`spec.loadBalancerSourceRanges` applied to the firewall rule. A network whose own offering includes
the Firewall service gets firewall rules instead of Network ACL rules.

When the controller manages Network ACL rules:
* Each Service gets its own ACL rule for each port, even when another Service on the tier uses the
Expand Down Expand Up @@ -561,8 +572,52 @@ The controller keeps the Service's load balancer rule until its ACL rule is dele
Kubernetes only retries the cleanup while a load balancer rule is left. Fix the cause shown in the
log, for example the account's permissions, and the cleanup finishes on its own.

### A deleted Service stays `Terminating`, log shows `Error deleting firewall rule, keeping load balancer rule`

On a tier of a VPC with the Firewall service the Service also has a firewall rule, and the
controller keeps the load balancer rule until that is deleted too, for the same reason. The same
applies when the log shows `Error reading the VPC`: without the VPC the controller cannot tell
whether there is a firewall rule. Fix the cause shown in the log and the cleanup finishes on its
own.

### A port in a VPC with the Firewall service stays closed, log shows `Cannot read VPC`

The controller could not read the VPC to find out whether it has the Firewall service, so it added
the Network ACL rule but no firewall rule, and the VPC's virtual router drops the traffic. Allow the
controller's account to call `listVPCs` (see [Kubernetes](#kubernetes)). The warning is logged once
per VPC until the controller restarts. The controller adds the firewall rule the next time it syncs
the Service, which happens when the Service changes or the controller restarts.

## Upgrading

### Firewall rules on tiers of a VPC with the Firewall service

This affects CloudStack 4.23 and later, on VPC tiers whose VPC offering includes the Firewall
service but whose own offering does not. Earlier releases added only a Network ACL rule there, so
the VPC's firewall kept the Service's ports closed. A tier whose own offering includes the Firewall
service still gets a firewall rule only, which is now also deleted with the Service. Other networks
behave as before.

Before upgrading:

* As on isolated networks, the controller treats the firewall rules on a Service's IP for its ports
and protocols as the Service's: it replaces a rule with another CIDR list and deletes the rules
with the Service. If you added such rules by hand, for example to work around the missing rule,
and they allow fewer sources than `0.0.0.0/0`, put those CIDRs into the Service's
`spec.loadBalancerSourceRanges`. Otherwise the first sync replaces them with a rule for
`0.0.0.0/0`.
* Allow the controller's account to call `listVPCs`; see [Kubernetes](#kubernetes). Without it the
controller keeps managing only the ACL rule.
* If the cloud config sets `version`, keep it at the CloudStack version in use. The controller reads
the VPC only when that version is 4.23 or later.

After upgrading, the first sync of each Service adds a firewall rule on its public IP for each port,
allowing the CIDRs in `spec.loadBalancerSourceRanges`, or `0.0.0.0/0` if it is empty.

An earlier release only deletes the ACL rule. After a downgrade, the firewall rule of a Service or
port that is removed later stays on its IP if the IP is kept, for example the cluster's API IP used
as `loadBalancerIP`. Delete such rules by hand.

### Per-Service Network ACL rules

This affects VPC tiers whose ACL rules the controller manages, that is tiers with a custom ACL list.
Expand Down
5 changes: 5 additions & 0 deletions cloudstack.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"io"
"os"
"strings"
"sync"
"time"

"github.com/apache/cloudstack-go/v2/cloudstack"
Expand Down Expand Up @@ -69,6 +70,10 @@ type CSCloud struct {
region string
version semver.Version
clientBuilder cloudprovider.ControllerClientBuilder

// vpcReadRefused holds the VPCs the account may not read, so that is logged once per VPC.
// Its zero value is ready to use, also in a CSCloud built without newCSCloud.
vpcReadRefused sync.Map
}

func init() {
Expand Down
146 changes: 136 additions & 10 deletions cloudstack_loadbalancer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
"sort"
"strconv"
"strings"
"sync"
"unicode"

"github.com/apache/cloudstack-go/v2/cloudstack"
Expand Down Expand Up @@ -77,6 +78,10 @@ const (
// the rule and creating it again.
var cidrListUpdateVersion = semver.Version{Major: 4, Minor: 22, Patch: 0}

// vpcFirewallVersion is the first CloudStack release in which a VPC offering can include the
// Firewall service. Below it no VPC has a firewall, so the VPC is not read.
var vpcFirewallVersion = semver.Version{Major: 4, Minor: 23, Patch: 0}

type loadBalancer struct {
*cloudstack.CloudStackClient

Expand All @@ -90,6 +95,9 @@ type loadBalancer struct {
rules map[string]*cloudstack.LoadBalancerRule
duplicateRules []*cloudstack.LoadBalancerRule
networks map[string]*cloudstack.Network
vpcFirewalls map[string]bool
vpcReadRefused *sync.Map
version semver.Version
ipAssociatedByController bool
}

Expand Down Expand Up @@ -190,6 +198,13 @@ func (cs *CSCloud) EnsureLoadBalancer(ctx context.Context, clusterName string, s
if err := stickiness.normalise(supportedStickinessMethods(network)); err != nil {
return nil, err
}
// Likewise read the VPC of a tier now, so a failed lookup fails before an IP is associated.
// The answer is remembered for applyLoadBalancerRules.
if usesNetworkACL(network) {
if _, err := lb.vpcHasFirewall(network); err != nil {
return nil, err
}
}

rulesApplied := false
if !lb.hasLoadBalancerIP() {
Expand Down Expand Up @@ -333,6 +348,64 @@ func isNetworkACLSupported(services []cloudstack.NetworkServiceInternal) bool {
return false
}

// vpcHasFirewall reports whether the VPC a network belongs to has the Firewall service. Since
// CloudStack 4.23 a VPC offering can include it. A tier lists the service only if its own network
// offering includes it, and the VPC virtual router drops traffic to a public IP with a load
// balancer rule unless a firewall rule admits it, on top of the tier's Network ACL. A network
// outside a VPC, or on an earlier release, has no VPC firewall.
//
// The answer is remembered per VPC. An account that may not call listVPCs is treated as if the
// VPC had no Firewall service, as earlier releases did. The account CloudStack's Kubernetes
// service creates for clusters in projects is one such account, and most VPCs have no Firewall
// service, so the warning that names the missing API is logged once per VPC.
func (lb *loadBalancer) vpcHasFirewall(network *cloudstack.Network) (bool, error) {
if network.Vpcid == "" || lb.version.LT(vpcFirewallVersion) {
return false, nil
}
if hasFirewall, ok := lb.vpcFirewalls[network.Vpcid]; ok {
return hasFirewall, nil
}

hasFirewall := false
vpc, _, err := lb.VPC.GetVPCByID(network.Vpcid, cloudstack.WithProject(lb.projectID))
switch {
case err != nil && isNotAllowed(err):
if lb.firstVPCReadRefusal(network.Vpcid) {
klog.Warningf("Cannot read VPC %v to check for the Firewall service, so no firewall rules are managed there; allow the account to call listVPCs: %v", network.Vpcid, err)
} else {
klog.V(4).Infof("Cannot read VPC %v to check for the Firewall service for load balancer %v: %v", network.Vpcid, lb.name, err)
}
case err != nil:
return false, fmt.Errorf("error fetching VPC %v of network %v: %v", network.Vpcid, network.Id, err)
default:
hasFirewall = slices.ContainsFunc(vpc.Service, func(svc cloudstack.VPCServiceInternal) bool {
return svc.Name == "Firewall"
})
}

if lb.vpcFirewalls == nil {
lb.vpcFirewalls = make(map[string]bool)
}
lb.vpcFirewalls[network.Vpcid] = hasFirewall

return hasFirewall, nil
}

// firstVPCReadRefusal reports whether the account was refused reading the VPC for the first time.
func (lb *loadBalancer) firstVPCReadRefusal(vpcID string) bool {
if lb.vpcReadRefused == nil {
return true
}
_, seen := lb.vpcReadRefused.LoadOrStore(vpcID, struct{}{})
return !seen
}

// usesNetworkACL reports whether the controller opens ports on the network with Network ACL rules
// rather than firewall rules.
func usesNetworkACL(network *cloudstack.Network) bool {
return !isFirewallSupported(network.Service) && isNetworkACLSupported(network.Service)
}

// EnsureLoadBalancerDeleted deletes the specified load balancer if it exists, returning
// nil if the load balancer specified either didn't exist or was successfully deleted.
func (cs *CSCloud) EnsureLoadBalancerDeleted(ctx context.Context, clusterName string, service *corev1.Service) error {
Expand All @@ -351,7 +424,7 @@ func (cs *CSCloud) EnsureLoadBalancerDeleted(ctx context.Context, clusterName st
klog.Errorf("Error removing duplicate load balancer rules for %v/%v: %v", service.Namespace, service.Name, sweepErr)
}

var aclErr error
var openingErr error
for _, lbRule := range lb.rules {
klog.V(4).Infof("Deleting firewall rules / Network ACLs for load balancer: %v", lbRule.Name)
protocol := ProtocolFromLoadBalancer(lbRule.Protocol)
Expand Down Expand Up @@ -383,14 +456,36 @@ func (cs *CSCloud) EnsureLoadBalancerDeleted(ctx context.Context, clusterName st
klog.Errorf("Error deleting firewall rule: %v", err)
}
} else {
// In a VPC with the Firewall service the port also has a firewall rule on the
// IP. Like the ACL rule below, it is reached only through this rule, which is
// kept for the retry if the VPC cannot be read or the rule cannot be deleted.
vpcFirewall, err := lb.vpcHasFirewall(network)
if err != nil {
klog.Errorf("Error reading the VPC of network %v, keeping load balancer rule %v for the retry: %v", networkId, lbRule.Name, err)
if openingErr == nil {
openingErr = err
}
continue
}
if vpcFirewall {
klog.V(4).Infof("Deleting firewall rules for %v - %v", int(port), protocol)
if _, err := lb.deleteFirewallRule(lbRule.Publicipid, int(port), protocol); err != nil {
klog.Errorf("Error deleting firewall rule, keeping load balancer rule %v for the retry: %v", lbRule.Name, err)
if openingErr == nil {
openingErr = err
}
continue
}
}

klog.V(4).Infof("Deleting network ACLs for %v - %v", int(port), protocol)
_, err = lb.deleteNetworkACLRule(int(port), protocol, networkId)
if err != nil {
// Kubernetes stops retrying once no load balancer rule is left, and only
// this rule leads back to the tier and port of its ACL rule, so it is kept.
klog.Errorf("Error deleting Network ACL rule, keeping load balancer rule %v for the retry: %v", lbRule.Name, err)
if aclErr == nil {
aclErr = err
if openingErr == nil {
openingErr = err
}
continue
}
Expand All @@ -405,8 +500,8 @@ func (cs *CSCloud) EnsureLoadBalancerDeleted(ctx context.Context, clusterName st
}

// Releasing the IP would make CloudStack revoke the rules kept for the retry.
if aclErr != nil {
return errors.Join(aclErr, sweepErr)
if openingErr != nil {
return errors.Join(openingErr, sweepErr)
}

if lb.ipAddr != "" {
Expand Down Expand Up @@ -473,6 +568,8 @@ func (cs *CSCloud) getLoadBalancer(service *corev1.Service) (*loadBalancer, erro
name: cs.GetLoadBalancerName(context.TODO(), "", service),
projectID: cs.projectID,
rules: make(map[string]*cloudstack.LoadBalancerRule),
vpcReadRefused: &cs.vpcReadRefused,
version: cs.version,
}
Comment on lines 568 to 573

@adestis-mf adestis-mf Nov 12, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Good point. vpcReadRefused is now a sync.Map value in CSCloud, whose zero value is ready to use, and each sync gets a pointer to it, so the warning is logged once per VPC however CSCloud is built (466eeab, with a test that builds it as a literal). Initializing it lazily in getLoadBalancer would be an unsynchronized write, as syncs can run concurrently.


p := cs.client.LoadBalancer.NewListLoadBalancerRulesParams()
Expand Down Expand Up @@ -986,7 +1083,8 @@ func (lb *loadBalancer) ruleNetwork(lbRule *cloudstack.LoadBalancerRule) (*cloud
// pruneRuleOpening deletes the firewall or network ACL rule admitting traffic to an obsolete
// load balancer rule, unless a desired service port still claims that same opening. Which of
// the two a rule has follows the network that rule belongs to, not the one being reconciled
// towards, so a rule left behind in a network of the other kind does not keep its opening.
// towards, so a rule left behind in a network of the other kind does not keep its opening. A
// tier of a VPC with the Firewall service has both.
//
// A rule with an uninterpretable protocol keeps its opening, which cannot be identified without
// one. A rule whose network cannot be established still has its firewall rule deleted, that
Expand All @@ -1009,6 +1107,15 @@ func (lb *loadBalancer) pruneRuleOpening(o obsoleteRule, claimed map[portProtoco
case isFirewallSupported(network.Service):
return lb.pruneFirewallRule(o, claimed)
case isNetworkACLSupported(network.Service):
vpcFirewall, err := lb.vpcHasFirewall(network)
if err != nil {
return err
}
if vpcFirewall {
if err := lb.pruneFirewallRule(o, claimed); err != nil {
return err
}
}
return lb.pruneNetworkACLRule(o, claimed, network.Id)
}

Expand Down Expand Up @@ -1109,6 +1216,16 @@ func (lb *loadBalancer) repairEmptyRule(lbRule *cloudstack.LoadBalancerRule) err
// port and reconciles the firewall or network ACL rules it needs. It returns the rule of each
// desired port, in the same order.
func (lb *loadBalancer) applyLoadBalancerRules(desired []desiredLBRule, service *corev1.Service, network *cloudstack.Network, version semver.Version) ([]*cloudstack.LoadBalancerRule, error) {
// A tier of a VPC with the Firewall service needs a firewall rule as well as its ACL rule.
// This is checked before any rule is created.
vpcFirewall := false
if usesNetworkACL(network) {
var err error
if vpcFirewall, err = lb.vpcHasFirewall(network); err != nil {
return nil, err
}
}

rules := make([]*cloudstack.LoadBalancerRule, 0, len(desired))
for _, d := range desired {
lbRule, err := lb.ensureLoadBalancerRule(d, service, version)
Expand All @@ -1127,6 +1244,12 @@ func (lb *loadBalancer) applyLoadBalancerRules(desired []desiredLBRule, service
if _, err := lb.updateNetworkACL(int(d.port.Port), d.protocol, network.Id); err != nil {
return nil, err
}
if vpcFirewall {
klog.V(4).Infof("Creating firewall rules for load balancer rule: %v (%v:%v:%v)", d.name, d.protocol, lbRule.Publicip, d.port.Port)
if _, err := lb.updateFirewallRule(lbRule.Publicipid, int(d.port.Port), d.protocol, service.Spec.LoadBalancerSourceRanges); err != nil {
return nil, err
}
}
}
}

Expand Down Expand Up @@ -2183,19 +2306,22 @@ func (lb *loadBalancer) deleteFirewallRule(publicIpId string, publicPort int, pr
}
}

// delete all rules
// delete all rules; the first failure is returned, so a later success does not hide it
deleted := false
var firstErr error
for _, rule := range filtered {
p := lb.Firewall.NewDeleteFirewallRuleParams(rule.Id)
_, err = lb.Firewall.DeleteFirewallRule(p)
if err != nil {
if _, err := lb.Firewall.DeleteFirewallRule(p); err != nil {
klog.Errorf("Error deleting old firewall rule %v: %v", rule.Id, err)
if firstErr == nil {
firstErr = err
}
} else {
deleted = true
}
}

return deleted, err
return deleted, firstErr
}

// deleteNetworkACLRule deletes this load balancer's Network ACL rules for a port on a tier and
Expand Down
Loading