You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by ro...@apache.org on 2020/09/24 06:53:01 UTC
[cloudstack-kubernetes-provider] branch master updated: Support
loadBalancerSourceRanges (#9)
This is an automated email from the ASF dual-hosted git repository.
rohit pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cloudstack-kubernetes-provider.git
The following commit(s) were added to refs/heads/master by this push:
new 2f17d26 Support loadBalancerSourceRanges (#9)
2f17d26 is described below
commit 2f17d26e671f0b598dff5c71f1b1c9fbd27dbe76
Author: Gregor Riepl <Gr...@swisstxt.ch>
AuthorDate: Thu Sep 24 08:52:52 2020 +0200
Support loadBalancerSourceRanges (#9)
* Add support for loadBalancerSourceRanges
* Added basic existing firewall rule comparison and creation boilerplate
* Implemented LB firewall rule creation+update and deletion
* WIP proper rule apply/cleanup
* Split protocol type into enum and fix variable names
* Split CIDR list
* Fixed a minor error handling logic error
* Modified update logic to always refresh firewall rules
* Make log less verbose
* Better error logging for firewall rules
* Fixed firewall rule protocol names
* Log firewall rule deletion steps
* Need listall=true for list commands...
* Log public IP ID
* Send project with list firewall rules call
* Drop debug logging corrections, they're unrelated
* Improve error handling on firewall rule creation
* Fixed string format error
* Better debugging for rules
* Delete matching fw rules first to prevent conflict
---
cloudstack_loadbalancer.go | 342 +++++++++++++++++++++++++++++++++++----------
protocol.go | 105 ++++++++++++++
2 files changed, 377 insertions(+), 70 deletions(-)
diff --git a/cloudstack_loadbalancer.go b/cloudstack_loadbalancer.go
index 91d4eff..2b30483 100644
--- a/cloudstack_loadbalancer.go
+++ b/cloudstack_loadbalancer.go
@@ -23,6 +23,7 @@ import (
"context"
"fmt"
"strconv"
+ "strings"
"github.com/xanzy/go-cloudstack/v2/cloudstack"
"k8s.io/klog"
@@ -31,13 +32,9 @@ import (
cloudprovider "k8s.io/cloud-provider"
)
-// ServiceAnnotationLoadBalancerProxyProtocol is the annotation used on the
-// service to enable the proxy protocol on a CloudStack load balancer.
-// The value of this annotation is ignored, even if it is seemingly boolean.
-// Simple presence of the annotation will enable it.
-// Note that this protocol only applies to TCP service ports and
-// CloudStack 4.6 is required for it to work.
-const ServiceAnnotationLoadBalancerProxyProtocol = "service.beta.kubernetes.io/cloudstack-load-balancer-proxy-protocol"
+// defaultAllowedCIDR is the network range that is allowed on the firewall
+// by default when no explicit CIDR list is given on a LoadBalancer.
+const defaultAllowedCIDR = "0.0.0.0/0"
type loadBalancer struct {
*cloudstack.CloudStackClient
@@ -129,51 +126,70 @@ func (cs *CSCloud) EnsureLoadBalancer(ctx context.Context, clusterName string, s
for _, port := range service.Spec.Ports {
// Construct the protocol name first, we need it a few times
- protocol, err := constructProtocolName(port, service.Annotations)
- if err != nil {
- return nil, err
+ protocol := ProtocolFromServicePort(port, service.Annotations)
+ 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)
// If the load balancer rule exists and is up-to-date, we move on to the next rule.
- exists, needsUpdate, err := lb.checkLoadBalancerRule(lbRuleName, port, protocol)
+ lbRule, needsUpdate, err := lb.checkLoadBalancerRule(lbRuleName, port, protocol)
if err != nil {
return nil, err
}
- if exists && !needsUpdate {
- klog.V(4).Infof("Load balancer rule %v is up-to-date", lbRuleName)
- // Delete the rule from the map, to prevent it being deleted.
- delete(lb.rules, lbRuleName)
- continue
+
+ if lbRule != nil {
+ if needsUpdate {
+ klog.V(4).Infof("Updating load balancer rule: %v", lbRuleName)
+ if err := lb.updateLoadBalancerRule(lbRuleName, protocol); err != nil {
+ return nil, err
+ }
+ // Delete the rule from the map, to prevent it being deleted.
+ delete(lb.rules, lbRuleName)
+ } else {
+ klog.V(4).Infof("Load balancer rule %v is up-to-date", lbRuleName)
+ // Delete the rule from the map, to prevent it being deleted.
+ delete(lb.rules, lbRuleName)
+ }
+ } else {
+ klog.V(4).Infof("Creating load balancer rule: %v", lbRuleName)
+ lbRule, err = lb.createLoadBalancerRule(lbRuleName, port, protocol)
+ if err != nil {
+ return nil, err
+ }
+
+ klog.V(4).Infof("Assigning hosts (%v) to load balancer rule: %v", lb.hostIDs, lbRuleName)
+ if err = lb.assignHostsToRule(lbRule, lb.hostIDs); err != nil {
+ return nil, err
+ }
}
- if needsUpdate {
- klog.V(4).Infof("Updating load balancer rule: %v", lbRuleName)
- if err := lb.updateLoadBalancerRule(lbRuleName, protocol); err != nil {
+ if lbRule != nil {
+ klog.V(4).Infof("Creating firewall rules for load balancer rule: %v (%v:%v:%v)", lbRuleName, protocol, lbRule.Publicip, port.Port)
+ if _ , err := lb.updateFirewallRule(lbRule.Publicipid, int(port.Port), protocol, service.Spec.LoadBalancerSourceRanges); err != nil {
return nil, err
}
- // Delete the rule from the map, to prevent it being deleted.
- delete(lb.rules, lbRuleName)
- continue
}
+ }
- klog.V(4).Infof("Creating load balancer rule: %v", lbRuleName)
- lbRule, err := lb.createLoadBalancerRule(lbRuleName, port, protocol)
+ // Cleanup any rules that are now still in the rules map, as they are no longer needed.
+ for _, lbRule := range lb.rules {
+ protocol := ProtocolFromLoadBalancer(lbRule.Protocol)
+ if protocol == LoadBalancerProtocolInvalid {
+ return nil, fmt.Errorf("Error parsing protocol %v: %v", lbRule.Protocol, err)
+ }
+ port, err := strconv.ParseInt(lbRule.Publicport, 10, 32)
if err != nil {
- return nil, err
+ return nil, fmt.Errorf("Error parsing port %s: %v", lbRule.Publicport, err)
}
- klog.V(4).Infof("Assigning hosts (%v) to load balancer rule: %v", lb.hostIDs, lbRuleName)
- if err = lb.assignHostsToRule(lbRule, lb.hostIDs); err != nil {
+ klog.V(4).Infof("Deleting firewall rules associated with load balancer rule: %v (%v:%v:%v)", lbRule.Name, protocol, lbRule.Publicip, port)
+ if _, err := lb.deleteFirewallRule(lbRule.Publicipid, int(port), protocol); err != nil {
return nil, err
}
- }
-
- // Cleanup any rules that are now still in the rules map, as they are no longer needed.
- for _, lbRule := range lb.rules {
klog.V(4).Infof("Deleting obsolete load balancer rule: %v", lbRule.Name)
if err := lb.deleteLoadBalancerRule(lbRule); err != nil {
return nil, err
@@ -243,9 +259,22 @@ func (cs *CSCloud) EnsureLoadBalancerDeleted(ctx context.Context, clusterName st
}
for _, lbRule := range lb.rules {
- klog.V(4).Infof("Deleting load balancer rule: %v", lbRule.Name)
- if err := lb.deleteLoadBalancerRule(lbRule); err != nil {
- return err
+ klog.V(4).Infof("Deleting firewall rules for load balancer: %v", lbRule.Name)
+ protocol := ProtocolFromLoadBalancer(lbRule.Protocol)
+ if protocol == LoadBalancerProtocolInvalid {
+ klog.Errorf("Error parsing protocol: %v", lbRule.Protocol)
+ } else {
+ port, err := strconv.ParseInt(lbRule.Publicport, 10, 32)
+ if err != nil {
+ klog.Errorf("Error parsing port: %v", err)
+ } else {
+ lb.deleteFirewallRule(lbRule.Publicipid, int(port), protocol)
+ }
+
+ klog.V(4).Infof("Deleting load balancer rule: %v", lbRule.Name)
+ if err := lb.deleteLoadBalancerRule(lbRule); err != nil {
+ return err
+ }
}
}
@@ -428,67 +457,43 @@ func (lb *loadBalancer) releaseLoadBalancerIP() error {
return nil
}
-// constructProtocolName builds a CS API compatible protocol name that incorporates
-// data from a ServicePort and (optionally) annotations on the service.
-// Currently supported are: "tcp", "udp" and "tcp-proxy".
-// The latter two require CloudStack 4.6 or later.
-func constructProtocolName(port v1.ServicePort, annotations map[string]string) (string, error) {
- proxy := false
- // FIXME this accepts any value as true, even "false", 0 or other falsey stuff
- if _, ok := annotations[ServiceAnnotationLoadBalancerProxyProtocol]; ok {
- proxy = true
- }
- switch port.Protocol {
- case v1.ProtocolTCP:
- if proxy {
- return "tcp-proxy", nil
- } else {
- return "tcp", nil
- }
- case v1.ProtocolUDP:
- return "udp", nil
- default:
- return "", fmt.Errorf("unsupported load balancer protocol: %v", port.Protocol)
- }
-}
-
// 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 v1.ServicePort, protocol string) (bool, bool, error) {
+func (lb *loadBalancer) checkLoadBalancerRule(lbRuleName string, port v1.ServicePort, protocol LoadBalancerProtocol) (*cloudstack.LoadBalancerRule, bool, error) {
lbRule, ok := lb.rules[lbRuleName]
if !ok {
- return false, false, nil
+ return nil, false, nil
}
// Check if any of the values we cannot update (those that require a new load balancer rule) are changed.
if lbRule.Publicip == lb.ipAddr && lbRule.Privateport == strconv.Itoa(int(port.NodePort)) && lbRule.Publicport == strconv.Itoa(int(port.Port)) {
updateAlgo := lbRule.Algorithm != lb.algorithm
- updateProto := lbRule.Protocol != protocol
- return true, updateAlgo || updateProto, nil
+ updateProto := lbRule.Protocol != protocol.CSProtocol()
+ return lbRule, updateAlgo || updateProto, nil
}
// Delete the load balancer rule so we can create a new one using the new values.
if err := lb.deleteLoadBalancerRule(lbRule); err != nil {
- return false, false, err
+ return nil, false, err
}
- return false, false, nil
+ return nil, false, nil
}
// updateLoadBalancerRule updates a load balancer rule.
-func (lb *loadBalancer) updateLoadBalancerRule(lbRuleName string, protocol string) error {
+func (lb *loadBalancer) updateLoadBalancerRule(lbRuleName string, protocol LoadBalancerProtocol) error {
lbRule := lb.rules[lbRuleName]
p := lb.LoadBalancer.NewUpdateLoadBalancerRuleParams(lbRule.Id)
p.SetAlgorithm(lb.algorithm)
- p.SetProtocol(protocol)
+ p.SetProtocol(protocol.CSProtocol())
_, err := lb.LoadBalancer.UpdateLoadBalancerRule(p)
return err
}
// createLoadBalancerRule creates a new load balancer rule and returns it's ID.
-func (lb *loadBalancer) createLoadBalancerRule(lbRuleName string, port v1.ServicePort, protocol string) (*cloudstack.LoadBalancerRule, error) {
+func (lb *loadBalancer) createLoadBalancerRule(lbRuleName string, port v1.ServicePort, protocol LoadBalancerProtocol) (*cloudstack.LoadBalancerRule, error) {
p := lb.LoadBalancer.NewCreateLoadBalancerRuleParams(
lb.algorithm,
lbRuleName,
@@ -499,10 +504,10 @@ func (lb *loadBalancer) createLoadBalancerRule(lbRuleName string, port v1.Servic
p.SetNetworkid(lb.networkID)
p.SetPublicipid(lb.ipAddrID)
- p.SetProtocol(protocol)
+ p.SetProtocol(protocol.CSProtocol())
- // Do not create corresponding firewall rule.
- p.SetOpenfirewall(true)
+ // Do not open the firewall implicitly, we always create explicit firewall rules
+ p.SetOpenfirewall(false)
// Create a new load balancer rule.
r, err := lb.LoadBalancer.CreateLoadBalancerRule(p)
@@ -588,3 +593,200 @@ func symmetricDifference(hostIDs []string, lbInstances []*cloudstack.VirtualMach
return assign, remove
}
+
+// compareStringSlice compares two unsorted slices of strings without sorting them first.
+//
+// The slices are equal if and only if both contain the same number of every unique element.
+//
+// Thanks to: https://stackoverflow.com/a/36000696
+func compareStringSlice(x, y []string) bool {
+ if len(x) != len(y) {
+ return false
+ }
+ // create a map of string -> int
+ diff := make(map[string]int, len(x))
+ for _, _x := range x {
+ // 0 value for int is 0, so just increment a counter for the string
+ diff[_x]++
+ }
+ for _, _y := range y {
+ // If the string _y is not in diff bail out early
+ if _, ok := diff[_y]; !ok {
+ return false
+ }
+ diff[_y] -= 1
+ if diff[_y] == 0 {
+ delete(diff, _y)
+ }
+ }
+ if len(diff) == 0 {
+ return true
+ }
+ return false
+}
+
+func ruleToString(rule *cloudstack.FirewallRule) string {
+ ls := &strings.Builder{}
+ if rule == nil {
+ ls.WriteString("nil")
+ } else {
+ switch rule.Protocol {
+ case "tcp":
+ fallthrough
+ case "udp":
+ fmt.Fprintf(ls, "{[%s] -> %s:[%d-%d] (%s)}", rule.Cidrlist, rule.Ipaddress, rule.Startport, rule.Endport, rule.Protocol)
+ case "icmp":
+ fmt.Fprintf(ls, "{[%s] -> %s [%d,%d] (%s)}", rule.Cidrlist, rule.Ipaddress, rule.Icmptype, rule.Icmpcode, rule.Protocol)
+ default:
+ fmt.Fprintf(ls, "{[%s] -> %s (%s)}", rule.Cidrlist, rule.Ipaddress, rule.Protocol)
+ }
+ }
+ return ls.String()
+}
+
+func rulesToString(rules []*cloudstack.FirewallRule) string {
+ ls := &strings.Builder{}
+ first := true
+ for _, rule := range rules {
+ if first {
+ first = false
+ } else {
+ ls.WriteString(", ")
+ }
+ ls.WriteString(ruleToString(rule))
+ }
+ return ls.String()
+}
+
+func rulesMapToString(rules map[*cloudstack.FirewallRule]bool) string {
+ ls := &strings.Builder{}
+ first := true
+ for rule, _ := range rules {
+ if first {
+ first = false
+ } else {
+ ls.WriteString(", ")
+ }
+ ls.WriteString(ruleToString(rule))
+ }
+ return ls.String()
+}
+
+// updateFirewallRule creates a firewall rule for a load balancer rule
+//
+// If the rule list is empty, all internet (IPv4: 0.0.0.0/0) is opened for the
+// load balancer's port+protocol implicitly.
+//
+// Returns true if the firewall rule was created or updated
+func (lb *loadBalancer) updateFirewallRule(publicIpId string, publicPort int, protocol LoadBalancerProtocol, allowedIPs []string) (bool, error) {
+ if len(allowedIPs) == 0 {
+ allowedIPs = []string{defaultAllowedCIDR}
+ }
+
+ p := lb.Firewall.NewListFirewallRulesParams()
+ p.SetIpaddressid(publicIpId)
+ p.SetListall(true)
+ if lb.projectID != "" {
+ p.SetProjectid(lb.projectID)
+ }
+ klog.V(4).Infof("Listing firewall rules for %v", p)
+ r, err := lb.Firewall.ListFirewallRules(p)
+ if err != nil {
+ return false, fmt.Errorf("error fetching firewall rules for public IP %v: %v", publicIpId, err)
+ }
+ klog.V(4).Infof("All firewall rules for %v: %v", lb.ipAddr, rulesToString(r.FirewallRules))
+
+ // find all rules that have a matching proto+port
+ // a map may or may not be faster, but is a bit easier to understand
+ filtered := make(map[*cloudstack.FirewallRule]bool)
+ for _, rule := range r.FirewallRules {
+ if rule.Protocol == protocol.IPProtocol() && rule.Startport == publicPort && rule.Endport == publicPort {
+ filtered[rule] = true
+ } else {
+ }
+ }
+ klog.V(4).Infof("Matching rules for %v: %v", lb.ipAddr, rulesMapToString(filtered))
+
+ // determine if we already have a rule with matching cidrs
+ var match *cloudstack.FirewallRule
+ for rule := range filtered {
+ cidrlist := strings.Split(rule.Cidrlist, ",")
+ if compareStringSlice(cidrlist, allowedIPs) {
+ klog.V(4).Infof("Found identical rule: %v", rule)
+ match = rule
+ break
+ }
+ }
+
+ if match != nil {
+ // no need to create a new rule - but prevent deletion of the matching rule
+ delete(filtered, match)
+ }
+
+ // delete all other rules that didn't match the CIDR list
+ // do this first to prevent CS rule conflict errors
+ klog.V(4).Infof("Firewall rules to be deleted for %v: %v", lb.ipAddr, rulesMapToString(filtered))
+ for rule := range filtered {
+ p := lb.Firewall.NewDeleteFirewallRuleParams(rule.Id)
+ _, err = lb.Firewall.DeleteFirewallRule(p)
+ if err != nil {
+ // report the error, but keep on deleting the other rules
+ klog.Errorf("Error deleting old firewall rule %v: %v", rule.Id, err)
+ }
+ }
+
+ // create new rule if necessary
+ if match == nil {
+ // no rule found, create a new one
+ p := lb.Firewall.NewCreateFirewallRuleParams(publicIpId, protocol.IPProtocol())
+ p.SetCidrlist(allowedIPs)
+ p.SetStartport(publicPort)
+ p.SetEndport(publicPort)
+ _, err = lb.Firewall.CreateFirewallRule(p)
+ if err != nil {
+ // return immediately if we can't create the new rule
+ return false, fmt.Errorf("error creating new firewall rule for public IP %v, proto %v, port %v, allowed %v: %v", publicIpId, protocol, publicPort, allowedIPs, err)
+ }
+ }
+
+ // return true (because we changed something), but also the last error if deleting one old rule failed
+ return true, err
+}
+
+// deleteFirewallRule deletes the firewall rule associated with the ip:port:protocol combo
+//
+// returns true when corresponding rules were deleted
+func (lb *loadBalancer) deleteFirewallRule(publicIpId string, publicPort int, protocol LoadBalancerProtocol) (bool, error) {
+ p := lb.Firewall.NewListFirewallRulesParams()
+ p.SetIpaddressid(publicIpId)
+ p.SetListall(true)
+ if lb.projectID != "" {
+ p.SetProjectid(lb.projectID)
+ }
+ r, err := lb.Firewall.ListFirewallRules(p)
+ if err != nil {
+ return false, fmt.Errorf("error fetching firewall rules for public IP %v: %v", publicIpId, err)
+ }
+
+ // filter by proto:port
+ filtered := make([]*cloudstack.FirewallRule, 0, 1)
+ for _, rule := range r.FirewallRules {
+ if rule.Protocol == protocol.IPProtocol() && rule.Startport == publicPort && rule.Endport == publicPort {
+ filtered = append(filtered, rule)
+ }
+ }
+
+ // delete all rules
+ deleted := false
+ for _, rule := range filtered {
+ p := lb.Firewall.NewDeleteFirewallRuleParams(rule.Id)
+ _, err = lb.Firewall.DeleteFirewallRule(p)
+ if err != nil {
+ klog.Errorf("Error deleting old firewall rule %v: %v", rule.Id, err)
+ } else {
+ deleted = true
+ }
+ }
+
+ return deleted, err
+}
diff --git a/protocol.go b/protocol.go
new file mode 100644
index 0000000..0fd6afe
--- /dev/null
+++ b/protocol.go
@@ -0,0 +1,105 @@
+package cloudstack
+
+import (
+ "k8s.io/api/core/v1"
+)
+
+// LoadBalancerProtocol represents a specific network protocol supported by the CloudStack load balancer.
+//
+// It also allows easy mapping to standard protocol names.
+type LoadBalancerProtocol int
+
+const (
+ LoadBalancerProtocolTCP LoadBalancerProtocol = iota
+ LoadBalancerProtocolUDP
+ LoadBalancerProtocolTCPProxy
+ LoadBalancerProtocolInvalid
+)
+
+// ServiceAnnotationLoadBalancerProxyProtocol is the annotation used on the
+// service to enable the proxy protocol on a CloudStack load balancer.
+// The value of this annotation is ignored, even if it is seemingly boolean.
+// Simple presence of the annotation will enable it.
+// Note that this protocol only applies to TCP service ports and
+// CloudStack 4.6 is required for it to work.
+const ServiceAnnotationLoadBalancerProxyProtocol = "service.beta.kubernetes.io/cloudstack-load-balancer-proxy-protocol"
+
+// String returns the same value as CSProtocol.
+func (p LoadBalancerProtocol) String() string {
+ return p.CSProtocol()
+}
+
+// CSProtocol returns the full CloudStack protocol name.
+// Returns "" if the value is unknown.
+func (p LoadBalancerProtocol) CSProtocol() string {
+ switch p {
+ case LoadBalancerProtocolTCP:
+ return "tcp"
+ case LoadBalancerProtocolUDP:
+ return "udp"
+ case LoadBalancerProtocolTCPProxy:
+ return "tcp-proxy"
+ default:
+ return ""
+ }
+}
+
+// IPProtocol returns the standard IP protocol name.
+// Returns "" if the value is unknown.
+func (p LoadBalancerProtocol) IPProtocol() string {
+ switch p {
+ case LoadBalancerProtocolTCP:
+ fallthrough
+ case LoadBalancerProtocolTCPProxy:
+ return "tcp"
+ case LoadBalancerProtocolUDP:
+ return "udp"
+ default:
+ return ""
+ }
+}
+
+// ProtocolFromServicePort selects a suitable CloudStack protocol type
+// based on a ServicePort object and annotations from a LoadBalancer definition.
+//
+// Supported combinations include:
+// v1.ProtocolTCP="tcp" -> "tcp"
+// v1.ProtocolTCP="udp" -> "udp" (CloudStack 4.6 and later)
+// v1.ProtocolTCP="tcp" + annotation "service.beta.kubernetes.io/cloudstack-load-balancer-proxy-protocol"
+// -> "tcp-proxy" (CloudStack 4.6 and later)
+//
+// Other values return LoadBalancerProtocolInvalid.
+func ProtocolFromServicePort(port v1.ServicePort, annotations map[string]string) LoadBalancerProtocol {
+ proxy := false
+ // FIXME this accepts any value as true, even "false", 0 or other falsey stuff
+ if _, ok := annotations[ServiceAnnotationLoadBalancerProxyProtocol]; ok {
+ proxy = true
+ }
+ switch port.Protocol {
+ case v1.ProtocolTCP:
+ if proxy {
+ return LoadBalancerProtocolTCPProxy
+ } else {
+ return LoadBalancerProtocolTCP
+ }
+ case v1.ProtocolUDP:
+ return LoadBalancerProtocolUDP
+ default:
+ return LoadBalancerProtocolInvalid
+ }
+}
+
+// ProtocolFromLoadBalancer returns the protocol corresponding to the
+// CloudStack load balancer protocol name.
+func ProtocolFromLoadBalancer(protocol string) LoadBalancerProtocol {
+ switch protocol {
+ case "tcp":
+ return LoadBalancerProtocolTCP
+ case "udp":
+ return LoadBalancerProtocolUDP
+ case "tcp-proxy":
+ return LoadBalancerProtocolTCPProxy
+ default:
+ return LoadBalancerProtocolInvalid
+ }
+}