Copilot commented on code in PR #104:
URL:
https://github.com/apache/cloudstack-kubernetes-provider/pull/104#discussion_r4044664392
##########
cloudstack_loadbalancer.go:
##########
@@ -681,11 +639,372 @@ func (lb *loadBalancer) getCIDRList(service
*corev1.Service) ([]string, error) {
return cidrList, nil
}
-// checkLoadBalancerRule checks if the rule already exists and if it does, if
it can be updated. If
-// it does exist but cannot be updated, it will delete the existing rule so it
can be created again.
-func (lb *loadBalancer) checkLoadBalancerRule(lbRuleName string, port
corev1.ServicePort, protocol LoadBalancerProtocol, service *corev1.Service,
version semver.Version) (*cloudstack.LoadBalancerRule, bool, error) {
- lbRule, ok := lb.rules[lbRuleName]
- if !ok {
+// splitCIDRList splits the CIDR list of an existing CloudStack rule into its
entries.
+// CloudStack has reported these both comma and space separated, and a CIDR
can contain
+// neither character, so treat both as separators.
+func splitCIDRList(cidrList string) []string {
+ return strings.FieldsFunc(cidrList, func(r rune) bool {
+ return r == ',' || r == ' '
+ })
+}
+
+// resolveLoadBalancerRules maps every service port to the load balancer rule
that should
+// represent it, claiming each match as it goes so that what remains in
lb.rules is exactly
+// the obsolete set and no rule can be claimed twice.
+func (lb *loadBalancer) resolveLoadBalancerRules(service *corev1.Service,
version semver.Version) ([]desiredLBRule, error) {
+ desired := make([]desiredLBRule, 0, len(service.Spec.Ports))
+
+ for _, port := range service.Spec.Ports {
+ // Construct the protocol name first, we need it a few times
+ protocol := ProtocolFromServicePort(port, service)
+ if protocol == LoadBalancerProtocolInvalid {
+ return nil, fmt.Errorf("unsupported load balancer
protocol: %v", port.Protocol)
+ }
+
+ // All ports have their own load balancer rule, so add the port
to lbName to keep the names unique.
+ lbRuleName := fmt.Sprintf("%s-%s-%d", lb.name, protocol,
port.Port)
+
+ lbRule, needsUpdate, err :=
lb.checkLoadBalancerRule(lb.findLoadBalancerRule(lbRuleName, port, protocol),
lbRuleName, port, protocol, service, version)
+ if err != nil {
+ return nil, err
+ }
+
+ if lbRule != nil {
+ // Claim by the rule's actual name: after a protocol
change it still carries the old one.
+ delete(lb.rules, lbRule.Name)
+ }
+
+ desired = append(desired, desiredLBRule{
+ name: lbRuleName,
+ port: port,
+ protocol: protocol,
+ existing: lbRule,
+ update: needsUpdate,
+ })
+ }
+
+ return desired, nil
+}
+
+// findLoadBalancerRule locates the existing CloudStack rule for a desired
service port. It
+// prefers an exact name match, then falls back to matching on the tuple. That
fallback is what
+// lets a protocol change (tcp <-> tcp-proxy) update the existing rule instead
of creating a
+// conflicting one.
+//
+// Only rules on the IP being reconciled towards are eligible; a rule on any
other IP is left
+// for the prune pass, which also cleans up the firewall rules it leaves
behind.
+func (lb *loadBalancer) findLoadBalancerRule(lbRuleName string, port
corev1.ServicePort, protocol LoadBalancerProtocol) *cloudstack.LoadBalancerRule
{
+ if lbRule, ok := lb.rules[lbRuleName]; ok && lbRule.Publicipid ==
lb.ipAddrID {
+ return lbRule
+ }
+
+ publicPort := strconv.Itoa(int(port.Port))
+ var names []string
+ for name, lbRule := range lb.rules {
+ if lbRule.Publicipid == lb.ipAddrID &&
+ ProtocolFromLoadBalancer(lbRule.Protocol).IPProtocol()
== protocol.IPProtocol() &&
+ lbRule.Publicport == publicPort {
+ names = append(names, name)
+ }
+ }
+ if len(names) == 0 {
+ return nil
+ }
+
+ // Map iteration order is randomized; sort so the pick is deterministic.
+ sort.Strings(names)
+ if len(names) > 1 {
+ klog.Warningf("Multiple load balancer rules match %s port %s:
%v; using %v", protocol.IPProtocol(), publicPort, names, names[0])
+ }
+ return lb.rules[names[0]]
+}
+
+// portProtocol is the tuple CloudStack refuses to place two load balancer
rules on, and that
+// firewall and network ACL rules are keyed on. IPProtocol maps both tcp and
tcp-proxy to
+// "tcp", so a tcp and a tcp-proxy rule on one port share a tuple, and one
firewall/ACL rule.
+type portProtocol struct {
+ ipProtocol string
+ publicPort int32
+}
+
+// obsoleteRule is a rule no desired service port claimed, with its tuple
already parsed.
+type obsoleteRule struct {
+ rule *cloudstack.LoadBalancerRule
+ protocol LoadBalancerProtocol
+ tuple portProtocol
+}
+
+// partitionObsoleteRules splits the rules left in lb.rules — those no desired
port claimed —
+// into the ones holding a tuple that a rule still to be created needs, and
the rest.
+func (lb *loadBalancer) partitionObsoleteRules(desired []desiredLBRule)
(blocking, rest []obsoleteRule) {
+ // CloudStack refuses two load balancer rules with overlapping public
port ranges on one
+ // IP whatever their protocols, so the port alone decides what blocks a
create. Note this
+ // is deliberately coarser than the firewall/ACL claim, which is per
protocol because
+ // firewall rules are.
+ neededPorts := make(map[int32]bool)
+ for _, d := range desired {
+ if d.existing == nil {
+ neededPorts[d.port.Port] = true
+ }
+ }
+
+ // Iterate in name order so the prune sequence is reproducible.
+ names := make([]string, 0, len(lb.rules))
+ for name := range lb.rules {
+ names = append(names, name)
+ }
+ sort.Strings(names)
+
+ for _, name := range names {
+ lbRule := lb.rules[name]
+
+ port, err := strconv.ParseInt(lbRule.Publicport, 10, 32)
+ if err != nil {
+ klog.Errorf("Skipping obsolete load balancer rule %v
with invalid public port %v: %v", lbRule.Name, lbRule.Publicport, err)
+ continue
+ }
+
+ // Conflicts are per public IP, so only a rule on the IP being
reconciled towards can
+ // block a create.
+ blocksACreate := lbRule.Publicipid == lb.ipAddrID &&
neededPorts[int32(port)]
+
+ // A protocol the provider cannot interpret leaves its firewall
or ACL rule
+ // unresolvable, so such a rule is normally left alone. One
holding a port a create
+ // needs still has to go, or CloudStack rejects that create as
a port conflict.
+ protocol := ProtocolFromLoadBalancer(lbRule.Protocol)
+ if protocol == LoadBalancerProtocolInvalid && !blocksACreate {
+ klog.Errorf("Skipping obsolete load balancer rule %v
with unknown protocol %v", lbRule.Name, lbRule.Protocol)
+ continue
+ }
+
+ obsolete := obsoleteRule{
+ rule: lbRule,
+ protocol: protocol,
+ tuple: portProtocol{protocol.IPProtocol(),
int32(port)},
+ }
+
+ if blocksACreate {
+ blocking = append(blocking, obsolete)
+ } else {
+ rest = append(rest, obsolete)
+ }
+ }
+
+ return blocking, rest
+}
+
+// ruleNetworkID is the network whose ACL rules an existing load balancer rule
was opened in, or
+// "" when that network cannot be established. CloudStack omits the network on
rules of some
+// network types; such a rule is known to belong to the network being
reconciled towards only
+// when it sits on the public IP being reconciled towards.
+func (lb *loadBalancer) ruleNetworkID(lbRule *cloudstack.LoadBalancerRule)
string {
+ if lbRule.Networkid != "" {
+ return lbRule.Networkid
+ }
+ if lbRule.Publicipid == lb.ipAddrID {
+ return lb.networkID
+ }
+ return ""
+}
+
+// claimedTuples are the tuples the desired service ports still need, and
whose firewall or
+// network ACL rules therefore have to survive a prune.
+func claimedTuples(desired []desiredLBRule) map[portProtocol]bool {
+ claimed := make(map[portProtocol]bool, len(desired))
+ for _, d := range desired {
+ claimed[portProtocol{d.protocol.IPProtocol(), d.port.Port}] =
true
+ }
+ return claimed
+}
+
+// pruneFirewallRule deletes the firewall rule admitting traffic to an
obsolete load balancer
+// rule. Firewall rules belong to a single public IP, so a claim only covers a
rule on the IP
+// the service is being reconciled towards.
+func (lb *loadBalancer) pruneFirewallRule(o obsoleteRule, claimed
map[portProtocol]bool) error {
+ lbRule, port := o.rule, int(o.tuple.publicPort)
+
+ if claimed[o.tuple] && lbRule.Publicipid == lb.ipAddrID {
+ klog.V(4).Infof("Keeping firewall rules of obsolete load
balancer rule %v (%v:%v:%v): still claimed by a service port", lbRule.Name,
o.protocol, lbRule.Publicip, port)
+ return nil
+ }
+
+ klog.V(4).Infof("Deleting firewall rules associated with load balancer
rule: %v (%v:%v:%v)", lbRule.Name, o.protocol, lbRule.Publicip, port)
+ _, err := lb.deleteFirewallRule(lbRule.Publicipid, port, o.protocol)
+ return err
+}
+
+// pruneNetworkACLRule deletes the network ACL rule admitting traffic to an
obsolete load
+// balancer rule, in the network that rule belongs to. ACL rules belong to a
network rather than
+// an IP, so a claim only covers a rule in the network the service is being
reconciled towards.
+func (lb *loadBalancer) pruneNetworkACLRule(o obsoleteRule, claimed
map[portProtocol]bool, networkID string) error {
+ lbRule, port := o.rule, int(o.tuple.publicPort)
+
+ if claimed[o.tuple] && networkID == lb.networkID {
+ klog.V(4).Infof("Keeping Network ACL rules of obsolete load
balancer rule %v (%v:%v:%v): still claimed by a service port", lbRule.Name,
o.protocol, networkID, port)
+ return nil
+ }
+
+ klog.V(4).Infof("Deleting Network ACL rules associated with load
balancer rule: %v (%v:%v:%v)", lbRule.Name, o.protocol, networkID, port)
+ _, err := lb.deleteNetworkACLRule(port, o.protocol, networkID)
+ return err
+}
+
+// rememberNetwork records a network already fetched, so resolving the network
of a rule in it
+// costs no further call.
+func (lb *loadBalancer) rememberNetwork(networkID string, network
*cloudstack.Network) {
+ if lb.networks == nil {
+ lb.networks = make(map[string]*cloudstack.Network)
+ }
+ lb.networks[networkID] = network
+}
+
+// networkByID is the network with the given ID, or nil when CloudStack no
longer has it. An
+// empty ID is nil rather than a lookup, which GetNetworkByID would answer
with an arbitrary
+// network from an unfiltered list.
+func (lb *loadBalancer) networkByID(networkID string) (*cloudstack.Network,
error) {
+ if networkID == "" {
+ return nil, nil
+ }
+ if network, ok := lb.networks[networkID]; ok {
+ return network, nil
+ }
+
+ network, count, err := lb.Network.GetNetworkByID(networkID,
cloudstack.WithProject(lb.projectID))
+ if err != nil {
+ return nil, fmt.Errorf("error fetching network %v: %v",
networkID, err)
+ }
+ if count == 0 {
+ network = nil
+ }
+ lb.rememberNetwork(networkID, network)
+
+ return network, nil
+}
+
+// ruleNetwork is the network an existing load balancer rule was created in,
or nil when that
+// network cannot be established, either because CloudStack reported no
network for the rule or
+// because the network has since been deleted.
+func (lb *loadBalancer) ruleNetwork(lbRule *cloudstack.LoadBalancerRule)
(*cloudstack.Network, error) {
+ return lb.networkByID(lb.ruleNetworkID(lbRule))
+}
+
+// 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.
+//
+// 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
+// being scoped to the rule's own public IP, while any ACL rule is left in
place.
+func (lb *loadBalancer) pruneRuleOpening(o obsoleteRule, claimed
map[portProtocol]bool) error {
+ if o.protocol == LoadBalancerProtocolInvalid {
+ klog.Warningf("Leaving the firewall or Network ACL rule of
obsolete load balancer rule %v in place: unknown protocol %v", o.rule.Name,
o.rule.Protocol)
+ return nil
Review Comment:
Returning success for an unrecognized protocol lets `pruneRules` delete the
load-balancer rule while no firewall/ACL cleanup is possible (the next lines
proceed to `deleteLoadBalancerRule`). On a VPC tier this can leave a broad
ingress ACL for a port no longer backed by the service, and on firewall
networks it leaves a stale opening. Preserve the rule until its opening can be
identified, or perform a safe ownership check before deleting it.
##########
test/e2e/vpc_test.go:
##########
@@ -259,3 +259,68 @@ func TestVPC_ExplicitLoadBalancerIPReleased(t *testing.T) {
return ip.Allocated == "", nil
})
}
+
+// TestVPC_ProxyProtocolACL covers the proxy protocol on a VPC tier, where
+// ingress is opened with a Network ACL rule rather than a firewall rule.
+// updateNetworkACL used to create the rule with the CloudStack protocol name
+// tcp-proxy, which the API rejects, so a proxy protocol service on a tier
never
+// reconciled at all. The ACL rule is keyed on the IP protocol, so it must be
+// created as tcp and be the same single rule before and after the toggle.
+func TestVPC_ProxyProtocolACL(t *testing.T) {
+ f, aclID, _ := vpcFramework(t)
+
+ // An ACL rule belongs to the tier, so this test uses a port of its
own. Note
+ // that 8081 is the virtual router's HAProxy stats port, which
CloudStack
+ // refuses to load balance.
+ const port = "8085"
+ svc := f.CreateLBService(func(s *corev1.Service) {
+ s.Annotations = map[string]string{annotationProxyProtocol:
"true"}
+ s.Spec.Ports = []corev1.ServicePort{
+ {Name: "http", Port: 8085, Protocol:
corev1.ProtocolTCP},
+ }
+ })
+ lbName := defaultLoadBalancerName(svc)
+
+ f.WaitForIngressIP(svc)
+ rules := f.WaitForLBRules(lbName, 1)
+ if rules[0].Protocol != "tcp-proxy" {
+ t.Errorf("rule protocol = %q, want tcp-proxy",
rules[0].Protocol)
+ }
+
+ f.Eventually(lbSyncTimeout, lbSyncInterval, "the tcp network ACL rule
for port "+port,
+ func() (bool, error) {
+ n, err := countACLRules(f, aclID, port)
+ return n >= 1, err
+ })
+
+ aclRules, err := f.ACLRules(aclID)
+ if err != nil {
+ t.Fatalf("listing ACL rules: %v", err)
+ }
+ for _, r := range aclRules {
+ if r.Startport == port && !strings.EqualFold(r.Protocol, "tcp")
{
+ t.Errorf("ACL rule for port %s has protocol %q, want
tcp", port, r.Protocol)
+ }
+ }
+
+ // Turning the annotation off keeps the one ACL rule: both protocols
share it.
+ f.UpdateService(svc, func(s *corev1.Service) {
+ delete(s.Annotations, annotationProxyProtocol)
+ })
+ f.Eventually(lbSyncTimeout, lbSyncInterval, "the rule to settle back on
tcp",
+ func() (bool, error) {
+ current, err := f.LBRules(lbName)
+ if err != nil || len(current) != 1 {
+ return false, err
+ }
+ return current[0].Protocol == "tcp", nil
+ })
+
+ n, err := countACLRules(f, aclID, port)
+ if err != nil {
+ t.Fatalf("counting ACL rules: %v", err)
+ }
+ if n != 1 {
+ t.Errorf("ACL rules for port %s = %d, want exactly 1 across the
toggle", port, n)
+ }
Review Comment:
After the toggle, this only waits for the load-balancer rule update.
EnsureLoadBalancer updates the LB rule before reconciling its Network ACL, so
the immediate count can observe the transient state before the ACL call
completes and make this E2E test flaky. Poll for exactly one matching ACL rule
here as well.
##########
cloudstack_loadbalancer.go:
##########
@@ -681,11 +639,372 @@ func (lb *loadBalancer) getCIDRList(service
*corev1.Service) ([]string, error) {
return cidrList, nil
}
-// checkLoadBalancerRule checks if the rule already exists and if it does, if
it can be updated. If
-// it does exist but cannot be updated, it will delete the existing rule so it
can be created again.
-func (lb *loadBalancer) checkLoadBalancerRule(lbRuleName string, port
corev1.ServicePort, protocol LoadBalancerProtocol, service *corev1.Service,
version semver.Version) (*cloudstack.LoadBalancerRule, bool, error) {
- lbRule, ok := lb.rules[lbRuleName]
- if !ok {
+// splitCIDRList splits the CIDR list of an existing CloudStack rule into its
entries.
+// CloudStack has reported these both comma and space separated, and a CIDR
can contain
+// neither character, so treat both as separators.
+func splitCIDRList(cidrList string) []string {
+ return strings.FieldsFunc(cidrList, func(r rune) bool {
+ return r == ',' || r == ' '
+ })
+}
+
+// resolveLoadBalancerRules maps every service port to the load balancer rule
that should
+// represent it, claiming each match as it goes so that what remains in
lb.rules is exactly
+// the obsolete set and no rule can be claimed twice.
+func (lb *loadBalancer) resolveLoadBalancerRules(service *corev1.Service,
version semver.Version) ([]desiredLBRule, error) {
+ desired := make([]desiredLBRule, 0, len(service.Spec.Ports))
+
+ for _, port := range service.Spec.Ports {
+ // Construct the protocol name first, we need it a few times
+ protocol := ProtocolFromServicePort(port, service)
+ if protocol == LoadBalancerProtocolInvalid {
+ return nil, fmt.Errorf("unsupported load balancer
protocol: %v", port.Protocol)
+ }
+
+ // All ports have their own load balancer rule, so add the port
to lbName to keep the names unique.
+ lbRuleName := fmt.Sprintf("%s-%s-%d", lb.name, protocol,
port.Port)
+
+ lbRule, needsUpdate, err :=
lb.checkLoadBalancerRule(lb.findLoadBalancerRule(lbRuleName, port, protocol),
lbRuleName, port, protocol, service, version)
+ if err != nil {
+ return nil, err
+ }
+
+ if lbRule != nil {
+ // Claim by the rule's actual name: after a protocol
change it still carries the old one.
+ delete(lb.rules, lbRule.Name)
+ }
+
+ desired = append(desired, desiredLBRule{
+ name: lbRuleName,
+ port: port,
+ protocol: protocol,
+ existing: lbRule,
+ update: needsUpdate,
+ })
+ }
+
+ return desired, nil
+}
+
+// findLoadBalancerRule locates the existing CloudStack rule for a desired
service port. It
+// prefers an exact name match, then falls back to matching on the tuple. That
fallback is what
+// lets a protocol change (tcp <-> tcp-proxy) update the existing rule instead
of creating a
+// conflicting one.
+//
+// Only rules on the IP being reconciled towards are eligible; a rule on any
other IP is left
+// for the prune pass, which also cleans up the firewall rules it leaves
behind.
+func (lb *loadBalancer) findLoadBalancerRule(lbRuleName string, port
corev1.ServicePort, protocol LoadBalancerProtocol) *cloudstack.LoadBalancerRule
{
+ if lbRule, ok := lb.rules[lbRuleName]; ok && lbRule.Publicipid ==
lb.ipAddrID {
+ return lbRule
+ }
+
+ publicPort := strconv.Itoa(int(port.Port))
+ var names []string
+ for name, lbRule := range lb.rules {
+ if lbRule.Publicipid == lb.ipAddrID &&
+ ProtocolFromLoadBalancer(lbRule.Protocol).IPProtocol()
== protocol.IPProtocol() &&
+ lbRule.Publicport == publicPort {
+ names = append(names, name)
+ }
+ }
+ if len(names) == 0 {
+ return nil
+ }
+
+ // Map iteration order is randomized; sort so the pick is deterministic.
+ sort.Strings(names)
+ if len(names) > 1 {
+ klog.Warningf("Multiple load balancer rules match %s port %s:
%v; using %v", protocol.IPProtocol(), publicPort, names, names[0])
+ }
+ return lb.rules[names[0]]
+}
+
+// portProtocol is the tuple CloudStack refuses to place two load balancer
rules on, and that
+// firewall and network ACL rules are keyed on. IPProtocol maps both tcp and
tcp-proxy to
+// "tcp", so a tcp and a tcp-proxy rule on one port share a tuple, and one
firewall/ACL rule.
+type portProtocol struct {
+ ipProtocol string
+ publicPort int32
+}
+
+// obsoleteRule is a rule no desired service port claimed, with its tuple
already parsed.
+type obsoleteRule struct {
+ rule *cloudstack.LoadBalancerRule
+ protocol LoadBalancerProtocol
+ tuple portProtocol
+}
+
+// partitionObsoleteRules splits the rules left in lb.rules — those no desired
port claimed —
+// into the ones holding a tuple that a rule still to be created needs, and
the rest.
+func (lb *loadBalancer) partitionObsoleteRules(desired []desiredLBRule)
(blocking, rest []obsoleteRule) {
+ // CloudStack refuses two load balancer rules with overlapping public
port ranges on one
+ // IP whatever their protocols, so the port alone decides what blocks a
create. Note this
+ // is deliberately coarser than the firewall/ACL claim, which is per
protocol because
+ // firewall rules are.
+ neededPorts := make(map[int32]bool)
+ for _, d := range desired {
+ if d.existing == nil {
+ neededPorts[d.port.Port] = true
+ }
+ }
+
+ // Iterate in name order so the prune sequence is reproducible.
+ names := make([]string, 0, len(lb.rules))
+ for name := range lb.rules {
+ names = append(names, name)
+ }
+ sort.Strings(names)
+
+ for _, name := range names {
+ lbRule := lb.rules[name]
+
+ port, err := strconv.ParseInt(lbRule.Publicport, 10, 32)
+ if err != nil {
+ klog.Errorf("Skipping obsolete load balancer rule %v
with invalid public port %v: %v", lbRule.Name, lbRule.Publicport, err)
+ continue
+ }
+
+ // Conflicts are per public IP, so only a rule on the IP being
reconciled towards can
+ // block a create.
+ blocksACreate := lbRule.Publicipid == lb.ipAddrID &&
neededPorts[int32(port)]
+
+ // A protocol the provider cannot interpret leaves its firewall
or ACL rule
+ // unresolvable, so such a rule is normally left alone. One
holding a port a create
+ // needs still has to go, or CloudStack rejects that create as
a port conflict.
+ protocol := ProtocolFromLoadBalancer(lbRule.Protocol)
+ if protocol == LoadBalancerProtocolInvalid && !blocksACreate {
+ klog.Errorf("Skipping obsolete load balancer rule %v
with unknown protocol %v", lbRule.Name, lbRule.Protocol)
+ continue
+ }
+
+ obsolete := obsoleteRule{
+ rule: lbRule,
+ protocol: protocol,
+ tuple: portProtocol{protocol.IPProtocol(),
int32(port)},
+ }
+
+ if blocksACreate {
+ blocking = append(blocking, obsolete)
+ } else {
+ rest = append(rest, obsolete)
+ }
+ }
+
+ return blocking, rest
+}
+
+// ruleNetworkID is the network whose ACL rules an existing load balancer rule
was opened in, or
+// "" when that network cannot be established. CloudStack omits the network on
rules of some
+// network types; such a rule is known to belong to the network being
reconciled towards only
+// when it sits on the public IP being reconciled towards.
+func (lb *loadBalancer) ruleNetworkID(lbRule *cloudstack.LoadBalancerRule)
string {
+ if lbRule.Networkid != "" {
+ return lbRule.Networkid
+ }
+ if lbRule.Publicipid == lb.ipAddrID {
+ return lb.networkID
+ }
+ return ""
+}
+
+// claimedTuples are the tuples the desired service ports still need, and
whose firewall or
+// network ACL rules therefore have to survive a prune.
+func claimedTuples(desired []desiredLBRule) map[portProtocol]bool {
+ claimed := make(map[portProtocol]bool, len(desired))
+ for _, d := range desired {
+ claimed[portProtocol{d.protocol.IPProtocol(), d.port.Port}] =
true
+ }
+ return claimed
+}
+
+// pruneFirewallRule deletes the firewall rule admitting traffic to an
obsolete load balancer
+// rule. Firewall rules belong to a single public IP, so a claim only covers a
rule on the IP
+// the service is being reconciled towards.
+func (lb *loadBalancer) pruneFirewallRule(o obsoleteRule, claimed
map[portProtocol]bool) error {
+ lbRule, port := o.rule, int(o.tuple.publicPort)
+
+ if claimed[o.tuple] && lbRule.Publicipid == lb.ipAddrID {
+ klog.V(4).Infof("Keeping firewall rules of obsolete load
balancer rule %v (%v:%v:%v): still claimed by a service port", lbRule.Name,
o.protocol, lbRule.Publicip, port)
+ return nil
+ }
+
+ klog.V(4).Infof("Deleting firewall rules associated with load balancer
rule: %v (%v:%v:%v)", lbRule.Name, o.protocol, lbRule.Publicip, port)
+ _, err := lb.deleteFirewallRule(lbRule.Publicipid, port, o.protocol)
+ return err
+}
+
+// pruneNetworkACLRule deletes the network ACL rule admitting traffic to an
obsolete load
+// balancer rule, in the network that rule belongs to. ACL rules belong to a
network rather than
+// an IP, so a claim only covers a rule in the network the service is being
reconciled towards.
+func (lb *loadBalancer) pruneNetworkACLRule(o obsoleteRule, claimed
map[portProtocol]bool, networkID string) error {
+ lbRule, port := o.rule, int(o.tuple.publicPort)
+
+ if claimed[o.tuple] && networkID == lb.networkID {
+ klog.V(4).Infof("Keeping Network ACL rules of obsolete load
balancer rule %v (%v:%v:%v): still claimed by a service port", lbRule.Name,
o.protocol, networkID, port)
+ return nil
Review Comment:
This deletion is scoped only to the current Service's desired tuples, but a
Network ACL rule is shared by the whole tier rather than by a load-balancer
rule or public IP. If another Service on the same network still uses (for
example) TCP/80 on a different public IP, pruning this stale rule will delete
the ACL entry and break that other Service. Before deleting, check for other
load-balancer rules/services using the same network/protocol/port or otherwise
reference-count the shared opening.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
