[OCCM] Set Hostname in load balancer status when using PROXY protocol

.zuul.yaml

@@ -121,7 +121,7 @@
         - cloud-provider-openstack-acceptance-test-e2e-conformance:
             files:
               - cmd/openstack-cloud-controller-manager/.*
-              - pkg/cloudprovider/.*
+              - pkg/openstack/.*
               - pkg/util/.*
               - tests/e2e/cloudprovider/.*
               - go.mod$
@@ -138,7 +138,7 @@
         - cloud-provider-openstack-acceptance-test-e2e-conformance-stable-branch-v1.20:
             files:
               - cmd/openstack-cloud-controller-manager/.*
-              - pkg/cloudprovider/.*
+              - pkg/openstack/.*
               - pkg/util/.*
               - tests/e2e/cloudprovider/.*
               - go.mod$
@@ -155,7 +155,7 @@
         - cloud-provider-openstack-acceptance-test-e2e-conformance-stable-branch-v1.19:
             files:
               - cmd/openstack-cloud-controller-manager/.*
-              - pkg/cloudprovider/.*
+              - pkg/openstack/.*
               - pkg/util/.*
               - tests/e2e/cloudprovider/.*
               - go.mod$
@@ -172,7 +172,7 @@
         - cloud-provider-openstack-acceptance-test-e2e-conformance-stable-branch-v1.18:
             files:
               - cmd/openstack-cloud-controller-manager/.*
-              - pkg/cloudprovider/.*
+              - pkg/openstack/.*
               - pkg/util/.*
               - tests/e2e/cloudprovider/.*
               - go.mod$

docs/openstack-cloud-controller-manager/expose-applications-using-loadbalancer-type-service.md

@@ -9,7 +9,7 @@
     - [Switching between Floating Subnets by using preconfigured Classes](#switching-between-floating-subnets-by-using-preconfigured-classes)
     - [Creating Service by specifying a floating IP](#creating-service-by-specifying-a-floating-ip)
     - [Restrict Access For LoadBalancer Service](#restrict-access-for-loadbalancer-service)
-  - [Issues](#issues)
+    - [Use PROXY protocol to preserve client IP](#use-proxy-protocol-to-preserve-client-ip)
 
 <!-- END doctoc generated TOC please keep comment here to allow auto update -->
 
@@ -278,6 +278,148 @@ spec:
 
 `loadBalancerSourceRanges` field supports to be updated.
 
-## Issues
-
-- `spec.externalTrafficPolicy` is not supported.
+### Use PROXY protocol to preserve client IP
+
+When exposing services like nginx-ingress-controller, it's a common requirement that the client connection information could pass through proxy servers and load balancers, therefore visible to the backend services. Knowing the originating IP address of a client may be useful for setting a particular language for a website, keeping a denylist of IP addresses, or simply for logging and statistics purposes.
+
+This requires that not only the proxy server(e.g. NGINX) should support PROXY protocol, but also the external load balancer (created by openstack-cloud-controller-manager in our case) should be able to send the correct data traffic to the proxy server.
+
+This guide uses nginx-ingress-controller as an example.
+
+To enable PROXY protocol support, the openstack-cloud-controller-manager config option [enable-ingress-hostname](./using-openstack-cloud-controller-manager.md#load-balancer) should set to `true`.
+
+1. Set up the nginx-ingress-controller
+
+   Refer to https://docs.nginx.com/nginx-ingress-controller/installation for how to install nginx-ingress-controller deployment or daemonset. Before creating load balancer service, make sure to enable PROXY protocol in the nginx config.
+
+   ```yaml
+   proxy-protocol: "True"
+   real-ip-header: "proxy_protocol"
+   set-real-ip-from: "0.0.0.0/0"
+   ```
+
+2. Create load balancer service
+
+   Use the following manifest to create nginx-ingress Service of LoadBalancer type.
+
+   ```yaml
+   apiVersion: v1
+   kind: Service
+   metadata:
+     name: nginx-ingress
+     namespace: nginx-ingress
+     annotations:
+       loadbalancer.openstack.org/proxy-protocol: "true"
+   spec:
+     externalTrafficPolicy: Cluster
+     type: LoadBalancer
+     ports:
+     - port: 80
+       targetPort: 80
+       protocol: TCP
+       name: http
+     - port: 443
+       targetPort: 443
+       protocol: TCP
+       name: https
+     selector:
+       app: nginx-ingress
+   ```
+
+   Wait until the service gets an external IP.
+
+   ```bash
+   $ kubectl -n nginx-ingress get svc
+   NAME            TYPE           CLUSTER-IP       EXTERNAL-IP             PORT(S)                      AGE
+   nginx-ingress   LoadBalancer   10.104.112.154   103.250.240.24.nip.io   80:32418/TCP,443:30009/TCP   107s
+   ```
+
+3. To validate the PROXY protocol is working, create a service that can print the request header and an ingress backed by nginx-ingress-controller.
+
+   ```bash
+   $ cat <<EOF | kubectl apply -f -
+   apiVersion: apps/v1
+   kind: Deployment
+   metadata:
+     name: echoserver
+     namespace: default
+     labels:
+       app: echoserver
+   spec:
+     replicas: 1
+     selector:
+       matchLabels:
+         app: echoserver
+     template:
+       metadata:
+         labels:
+           app: echoserver
+       spec:
+         containers:
+         - name: echoserver
+           image: gcr.io/google-containers/echoserver:1.10
+           imagePullPolicy: IfNotPresent
+           ports:
+             - containerPort: 8080
+   EOF
+
+   $ kubectl expose deployment echoserver --type=ClusterIP --target-port=8080
+
+   $ cat <<EOF | kubectl apply -f -
+   apiVersion: networking.k8s.io/v1
+   kind: Ingress
+   metadata:
+     name: test-proxy-protocol
+     namespace: default
+     annotations:
+       kubernetes.io/ingress.class: "nginx"
+   spec:
+     rules:
+       - host: test.com
+         http:
+           paths:
+           - path: /ping
+             pathType: Exact
+             backend:
+               service:
+                 name: echoserver
+                 port:
+                   number: 80
+   EOF
+
+   $ kubectl get ing
+   NAME                   CLASS    HOSTS      ADDRESS                 PORTS   AGE
+   test-proxy-protocol    <none>   test.com   103.250.240.24.nip.io   80      58m
+   ```
+
+   Now, send request to the ingress URL defined above, you should see your public IP address is shown in the Request Headers (`x-forwarded-for` or `x-real-ip`).
+
+   ```bash
+   $ ip=103.250.240.24.nip.io
+   $ curl -sH "Host: test.com" http://$ip/ping | sed '/^\s*$/d'
+   Hostname: echoserver-5c79dc5747-m4spf
+   Pod Information:
+           -no pod information available-
+   Server values:
+           server_version=nginx: 1.13.3 - lua: 10008
+   Request Information:
+           client_address=10.244.215.132
+           method=GET
+           real path=/ping
+           query=
+           request_version=1.1
+           request_scheme=http
+           request_uri=http://test.com:8080/ping
+   Request Headers:
+           accept=*/*
+           connection=close
+           host=test.com
+           user-agent=curl/7.58.0
+           x-forwarded-for=103.197.63.236
+           x-forwarded-host=test.com
+           x-forwarded-port=80
+           x-forwarded-proto=http
+           x-real-ip=103.197.63.236
+   Request Body:
+           -no body in request-
+   ```

docs/openstack-cloud-controller-manager/using-openstack-cloud-controller-manager.md

@@ -16,6 +16,7 @@
   - [Metrics](#metrics)
   - [Limitation](#limitation)
     - [OpenStack availability zone must not contain blank](#openstack-availability-zone-must-not-contain-blank)
+    - [externalTrafficPolicy support](#externaltrafficpolicy-support)
 
 <!-- END doctoc generated TOC please keep comment here to allow auto update -->
 
@@ -236,6 +237,12 @@ Although the openstack-cloud-controller-manager was initially implemented with N
   * floating-subnet-tags. The same with `floating-subnet-tags` option above.
   * network-id. The same with `network-id` option above.
   * subnet-id. The same with `subnet-id` option above.
+
+* `enable-ingress-hostname`
+
+  Used with proxy protocol (set by annotation `loadbalancer.openstack.org/proxy-protocol: "true"`) by adding a dns suffix (nip.io) to the load balancer IP address. Default false.
+
+  This option is currently a workaround for the issue https://github.com/kubernetes/ingress-nginx/issues/3996, should be removed or refactored after the Kubernetes [KEP-1860](https://github.com/kubernetes/enhancements/tree/master/keps/sig-network/1860-kube-proxy-IP-node-binding) is implemented.
 
 ### Metadata
 
@@ -260,3 +267,9 @@ Refer to [Metrics for openstack-cloud-controller-manager](../metrics.md)
 ### OpenStack availability zone must not contain blank
 
 `topology.kubernetes.io/zone` is used to label node and its value comes from availability zone of the node, according to [label spec](https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#syntax-and-character-set) it does not support blank (' ') but OpenStack availability zone supports blank. So your OpenStack availability zone must not contain blank otherwise it will lead to node that belongs to this availability zone register failure, see [#1379](https://github.com/kubernetes/cloud-provider-openstack/issues/1379) for further information.
+
+### externalTrafficPolicy support
+
+`externalTrafficPolicy` denotes if this Service desires to route external traffic to node-local or cluster-wide endpoints. "Local" preserves the client source IP and avoids a second hop for LoadBalancer and Nodeport type services, but risks potentially imbalanced traffic spreading. "Cluster" obscures the client source IP and may cause a second hop to another node, but should have good overall load-spreading.
+
+openstack-cloud-controller-manager only supports `externalTrafficPolicy: Cluster` for now.

pkg/openstack/loadbalancer.go

@@ -107,6 +107,9 @@ const (
 	// ServiceAnnotationLoadBalancerEnableHealthMonitor defines whether or not to create health monitor for the load balancer
 	// pool, if not specified, use 'create-monitor' config. The health monitor can be created or deleted dynamically.
 	ServiceAnnotationLoadBalancerEnableHealthMonitor = "loadbalancer.openstack.org/enable-health-monitor"
+
+	// See https://nip.io
+	defaultProxyHostnameSuffix = "nip.io"
 )
 
 // LbaasV2 is a LoadBalancer implementation for Neutron LBaaS v2 API
@@ -1809,6 +1812,16 @@ func (lbaas *LbaasV2) ensureOctaviaLoadBalancer(ctx context.Context, clusterName
 		Ingress: []corev1.LoadBalancerIngress{{IP: addr}},
 	}
 
+	// If the load balancer is using the PROXY protocol, expose its IP address via
+	// the Hostname field to prevent kube-proxy from injecting an iptables bypass.
+	// This is a workaround until
+	// https://github.com/kubernetes/enhancements/tree/master/keps/sig-network/1860-kube-proxy-IP-node-binding
+	// is implemented (maybe in v1.22).
+	if svcConf.enableProxyProtocol && lbaas.opts.EnableIngressHostname {
+		fakeHostname := fmt.Sprintf("%s.%s", status.Ingress[0].IP, defaultProxyHostnameSuffix)
+		status.Ingress = []corev1.LoadBalancerIngress{{Hostname: fakeHostname}}
+	}
+
 	return status, nil
 }
 

pkg/openstack/openstack.go

@@ -92,27 +92,28 @@ type LoadBalancer struct {
 
 // LoadBalancerOpts have the options to talk to Neutron LBaaSV2 or Octavia
 type LoadBalancerOpts struct {
-	LBVersion            string              `gcfg:"lb-version"`           // overrides autodetection. Only support v2.
-	UseOctavia           bool                `gcfg:"use-octavia"`          // uses Octavia V2 service catalog endpoint
-	SubnetID             string              `gcfg:"subnet-id"`            // overrides autodetection.
-	NetworkID            string              `gcfg:"network-id"`           // If specified, will create virtual ip from a subnet in network which has available IP addresses
-	FloatingNetworkID    string              `gcfg:"floating-network-id"`  // If specified, will create floating ip for loadbalancer, or do not create floating ip.
-	FloatingSubnetID     string              `gcfg:"floating-subnet-id"`   // If specified, will create floating ip for loadbalancer in this particular floating pool subnetwork.
-	FloatingSubnet       string              `gcfg:"floating-subnet"`      // If specified, will create floating ip for loadbalancer in one of the matching floating pool subnetworks.
-	FloatingSubnetTags   string              `gcfg:"floating-subnet-tags"` // If specified, will create floating ip for loadbalancer in one of the matching floating pool subnetworks.
-	LBClasses            map[string]*LBClass // Predefined named Floating networks and subnets
-	LBMethod             string              `gcfg:"lb-method"` // default to ROUND_ROBIN.
-	LBProvider           string              `gcfg:"lb-provider"`
-	CreateMonitor        bool                `gcfg:"create-monitor"`
-	MonitorDelay         util.MyDuration     `gcfg:"monitor-delay"`
-	MonitorTimeout       util.MyDuration     `gcfg:"monitor-timeout"`
-	MonitorMaxRetries    uint                `gcfg:"monitor-max-retries"`
-	ManageSecurityGroups bool                `gcfg:"manage-security-groups"`
-	NodeSecurityGroupIDs []string            // Do not specify, get it automatically when enable manage-security-groups. TODO(FengyunPan): move it into cache
-	InternalLB           bool                `gcfg:"internal-lb"`    // default false
-	CascadeDelete        bool                `gcfg:"cascade-delete"` // applicable only if use-octavia is set to True
-	FlavorID             string              `gcfg:"flavor-id"`
-	AvailabilityZone     string              `gcfg:"availability-zone"`
+	LBVersion             string              `gcfg:"lb-version"`           // overrides autodetection. Only support v2.
+	UseOctavia            bool                `gcfg:"use-octavia"`          // uses Octavia V2 service catalog endpoint
+	SubnetID              string              `gcfg:"subnet-id"`            // overrides autodetection.
+	NetworkID             string              `gcfg:"network-id"`           // If specified, will create virtual ip from a subnet in network which has available IP addresses
+	FloatingNetworkID     string              `gcfg:"floating-network-id"`  // If specified, will create floating ip for loadbalancer, or do not create floating ip.
+	FloatingSubnetID      string              `gcfg:"floating-subnet-id"`   // If specified, will create floating ip for loadbalancer in this particular floating pool subnetwork.
+	FloatingSubnet        string              `gcfg:"floating-subnet"`      // If specified, will create floating ip for loadbalancer in one of the matching floating pool subnetworks.
+	FloatingSubnetTags    string              `gcfg:"floating-subnet-tags"` // If specified, will create floating ip for loadbalancer in one of the matching floating pool subnetworks.
+	LBClasses             map[string]*LBClass // Predefined named Floating networks and subnets
+	LBMethod              string              `gcfg:"lb-method"` // default to ROUND_ROBIN.
+	LBProvider            string              `gcfg:"lb-provider"`
+	CreateMonitor         bool                `gcfg:"create-monitor"`
+	MonitorDelay          util.MyDuration     `gcfg:"monitor-delay"`
+	MonitorTimeout        util.MyDuration     `gcfg:"monitor-timeout"`
+	MonitorMaxRetries     uint                `gcfg:"monitor-max-retries"`
+	ManageSecurityGroups  bool                `gcfg:"manage-security-groups"`
+	NodeSecurityGroupIDs  []string            // Do not specify, get it automatically when enable manage-security-groups. TODO(FengyunPan): move it into cache
+	InternalLB            bool                `gcfg:"internal-lb"`    // default false
+	CascadeDelete         bool                `gcfg:"cascade-delete"` // applicable only if use-octavia is set to True
+	FlavorID              string              `gcfg:"flavor-id"`
+	AvailabilityZone      string              `gcfg:"availability-zone"`
+	EnableIngressHostname bool                `gcfg:"enable-ingress-hostname"` // Used with proxy protocol by adding a dns suffix to the load balancer IP address. Default false.
 }
 
 // LBClass defines the corresponding floating network, floating subnet or internal subnet ID
@@ -208,6 +209,7 @@ func ReadConfig(config io.Reader) (Config, error) {
 	cfg.LoadBalancer.MonitorTimeout = util.MyDuration{Duration: 3 * time.Second}
 	cfg.LoadBalancer.MonitorMaxRetries = 1
 	cfg.LoadBalancer.CascadeDelete = true
+	cfg.LoadBalancer.EnableIngressHostname = false
 
 	err := gcfg.FatalOnly(gcfg.ReadInto(&cfg, config))
 	if err != nil {

tests/e2e/cloudprovider/test-lb-service.sh

@@ -9,6 +9,7 @@
 #   - It's recommended to run the script on a host with as less proxies to the public as possible, otherwise the
 #     x-forwarded-for test will probably fail.
 #   - This script is not responsible for resource clean up if there is test case fails.
+set -x
 
 TIMEOUT=${TIMEOUT:-300}
 FLOATING_IP=${FLOATING_IP:-""}