[release-4.11] OCPBUGS-3560: Allow PROXY protocol for "Private" strategy

go.mod

@@ -16,7 +16,7 @@ require (
 	github.com/go-logr/zapr v1.2.0
 	github.com/google/go-cmp v0.5.6
 	github.com/kevinburke/go-bindata v3.11.0+incompatible
-	github.com/openshift/api v0.0.0-20220712151050-2647eb31dee7
+	github.com/openshift/api v0.0.0-20230426102702-398424d53f74
 	github.com/openshift/library-go v0.0.0-20220525173854-9b950a41acdc
 	github.com/pkg/errors v0.9.1
 	github.com/prometheus/client_golang v1.12.1

go.sum

@@ -594,8 +594,8 @@ github.com/onsi/gomega v1.7.1/go.mod h1:XdKZgCCFLUoM/7CFJVPcG8C1xQ1AJ0vpAezJrB7J
 github.com/onsi/gomega v1.10.1/go.mod h1:iN09h71vgCQne3DLsj+A5owkum+a2tYe+TOCB1ybHNo=
 github.com/onsi/gomega v1.10.3/go.mod h1:V9xEwhxec5O8UDM77eCW8vLymOMltsqPVYWrpDsH8xc=
 github.com/onsi/gomega v1.18.1 h1:M1GfJqGRrBrrGGsbxzV5dqM2U2ApXefZCQpkukxYRLE=
-github.com/openshift/api v0.0.0-20220712151050-2647eb31dee7 h1:zjlaMHqzNrrm8bnltBnrKLwxALoLAH/8UAkBEESrEOg=
-github.com/openshift/api v0.0.0-20220712151050-2647eb31dee7/go.mod h1:LEnw1IVscIxyDnltE3Wi7bQb/QzIM8BfPNKoGA1Qlxw=
+github.com/openshift/api v0.0.0-20230426102702-398424d53f74 h1:2M+jyeT5ItCoeYqytYfkXQZo3m5amf8JWCpYSAKO7fU=
+github.com/openshift/api v0.0.0-20230426102702-398424d53f74/go.mod h1:LEnw1IVscIxyDnltE3Wi7bQb/QzIM8BfPNKoGA1Qlxw=
 github.com/openshift/build-machinery-go v0.0.0-20211213093930-7e33a7eb4ce3/go.mod h1:b1BuldmJlbA/xYtdZvKi+7j5YGB44qJUJDZ9zwiNCfE=
 github.com/openshift/library-go v0.0.0-20220525173854-9b950a41acdc h1:j+upvKc1uLzuL+q/JXie8+IMohOooTCaEC9w+4d1Ztk=
 github.com/openshift/library-go v0.0.0-20220525173854-9b950a41acdc/go.mod h1:AMZwYwSdbvALDl3QobEzcJ2IeDO7DYLsr42izKzh524=

manifests/00-custom-resource-definition.yaml

@@ -350,6 +350,30 @@ spec:
                   private:
                     description: private holds parameters for the Private endpoint
                       publishing strategy. Present only if type is Private.
+                    properties:
+                      protocol:
+                        description: "protocol specifies whether the IngressController
+                          expects incoming connections to use plain TCP or whether
+                          the IngressController expects PROXY protocol. \n PROXY protocol
+                          can be used with load balancers that support it to communicate
+                          the source addresses of client connections when forwarding
+                          those connections to the IngressController.  Using PROXY
+                          protocol enables the IngressController to report those source
+                          addresses instead of reporting the load balancer's address
+                          in HTTP headers and logs.  Note that enabling PROXY protocol
+                          on the IngressController will cause connections to fail
+                          if you are not using a load balancer that uses PROXY protocol
+                          to forward connections to the IngressController.  See http://www.haproxy.org/download/2.2/doc/proxy-protocol.txt
+                          for information about PROXY protocol. \n The following values
+                          are valid for this field: \n * The empty string. * \"TCP\".
+                          * \"PROXY\". \n The empty string specifies the default,
+                          which is TCP without PROXY protocol.  Note that the default
+                          is subject to change."
+                        enum:
+                        - ""
+                        - TCP
+                        - PROXY
+                        type: string
                     type: object
                   type:
                     description: "type is the publishing strategy to use. Valid values
@@ -1573,6 +1597,30 @@ spec:
                   private:
                     description: private holds parameters for the Private endpoint
                       publishing strategy. Present only if type is Private.
+                    properties:
+                      protocol:
+                        description: "protocol specifies whether the IngressController
+                          expects incoming connections to use plain TCP or whether
+                          the IngressController expects PROXY protocol. \n PROXY protocol
+                          can be used with load balancers that support it to communicate
+                          the source addresses of client connections when forwarding
+                          those connections to the IngressController.  Using PROXY
+                          protocol enables the IngressController to report those source
+                          addresses instead of reporting the load balancer's address
+                          in HTTP headers and logs.  Note that enabling PROXY protocol
+                          on the IngressController will cause connections to fail
+                          if you are not using a load balancer that uses PROXY protocol
+                          to forward connections to the IngressController.  See http://www.haproxy.org/download/2.2/doc/proxy-protocol.txt
+                          for information about PROXY protocol. \n The following values
+                          are valid for this field: \n * The empty string. * \"TCP\".
+                          * \"PROXY\". \n The empty string specifies the default,
+                          which is TCP without PROXY protocol.  Note that the default
+                          is subject to change."
+                        enum:
+                        - ""
+                        - TCP
+                        - PROXY
+                        type: string
                     type: object
                   type:
                     description: "type is the publishing strategy to use. Valid values

pkg/operator/controller/ingress/controller.go

@@ -433,7 +433,12 @@ func setDefaultPublishingStrategy(ic *operatorv1.IngressController, platformStat
 			effectiveStrategy.HostNetwork.Protocol = operatorv1.TCPProtocol
 		}
 	case operatorv1.PrivateStrategyType:
-		// No parameters.
+		if effectiveStrategy.Private == nil {
+			effectiveStrategy.Private = &operatorv1.PrivateStrategy{}
+		}
+		if effectiveStrategy.Private.Protocol == operatorv1.DefaultProtocol {
+			effectiveStrategy.Private.Protocol = operatorv1.TCPProtocol
+		}
 	}
 	if ic.Status.EndpointPublishingStrategy == nil {
 		ic.Status.EndpointPublishingStrategy = effectiveStrategy
@@ -566,6 +571,17 @@ func setDefaultPublishingStrategy(ic *operatorv1.IngressController, platformStat
 			}
 		}
 		return changed
+	case operatorv1.PrivateStrategyType:
+		// Update if PROXY protocol is turned on or off.
+		if ic.Status.EndpointPublishingStrategy.Private == nil {
+			ic.Status.EndpointPublishingStrategy.Private = &operatorv1.PrivateStrategy{}
+		}
+		statusPrivate := ic.Status.EndpointPublishingStrategy.Private
+		specPrivate := effectiveStrategy.Private
+		if specPrivate != nil && specPrivate.Protocol != statusPrivate.Protocol {
+			statusPrivate.Protocol = specPrivate.Protocol
+			return true
+		}
 	}
 
 	return false
@@ -1009,6 +1025,10 @@ func IsProxyProtocolNeeded(ic *operatorv1.IngressController, platform *configv1.
 		if ic.Status.EndpointPublishingStrategy.NodePort != nil {
 			return ic.Status.EndpointPublishingStrategy.NodePort.Protocol == operatorv1.ProxyProtocol, nil
 		}
+	case operatorv1.PrivateStrategyType:
+		if ic.Status.EndpointPublishingStrategy.Private != nil {
+			return ic.Status.EndpointPublishingStrategy.Private.Protocol == operatorv1.ProxyProtocol, nil
+		}
 	}
 	return false, nil
 }

pkg/operator/controller/ingress/controller_test.go

@@ -352,6 +352,19 @@ func TestSetDefaultPublishingStrategyHandlesUpdates(t *testing.T) {
 				Type: operatorv1.HostNetworkStrategyType,
 			}
 		}
+		private = func(proto operatorv1.IngressControllerProtocol) *operatorv1.EndpointPublishingStrategy {
+			return &operatorv1.EndpointPublishingStrategy{
+				Type: operatorv1.PrivateStrategyType,
+				Private: &operatorv1.PrivateStrategy{
+					Protocol: proto,
+				},
+			}
+		}
+		privateWithNull = func() *operatorv1.EndpointPublishingStrategy {
+			return &operatorv1.EndpointPublishingStrategy{
+				Type: operatorv1.PrivateStrategyType,
+			}
+		}
 	)
 
 	testCases := []struct {
@@ -500,6 +513,36 @@ func TestSetDefaultPublishingStrategyHandlesUpdates(t *testing.T) {
 			expectedResult: true,
 			expectedIC:     makeIC(spec(hostNetworkWithNull()), status(hostNetwork(operatorv1.TCPProtocol))),
 		},
+		{
+			name:           "private protocol changed to PROXY with null status.endpointPublishingStrategy.private",
+			ic:             makeIC(spec(private(operatorv1.ProxyProtocol)), status(privateWithNull())),
+			expectedResult: true,
+			expectedIC:     makeIC(spec(private(operatorv1.ProxyProtocol)), status(private(operatorv1.ProxyProtocol))),
+		},
+		{
+			name:           "private spec.endpointPublishingStrategy.private set to null",
+			ic:             makeIC(spec(privateWithNull()), status(private(operatorv1.TCPProtocol))),
+			expectedResult: false,
+			expectedIC:     makeIC(spec(privateWithNull()), status(private(operatorv1.TCPProtocol))),
+		},
+		{
+			name:           "private protocol changed from empty to PROXY",
+			ic:             makeIC(spec(private(operatorv1.ProxyProtocol)), status(private(""))),
+			expectedResult: true,
+			expectedIC:     makeIC(spec(private(operatorv1.ProxyProtocol)), status(private(operatorv1.ProxyProtocol))),
+		},
+		{
+			name:           "private protocol changed from TCP to PROXY",
+			ic:             makeIC(spec(private(operatorv1.ProxyProtocol)), status(private(operatorv1.TCPProtocol))),
+			expectedResult: true,
+			expectedIC:     makeIC(spec(private(operatorv1.ProxyProtocol)), status(private(operatorv1.ProxyProtocol))),
+		},
+		{
+			name:           "private protocol changed from PROXY to TCP",
+			ic:             makeIC(spec(private(operatorv1.TCPProtocol)), status(private(operatorv1.ProxyProtocol))),
+			expectedResult: true,
+			expectedIC:     makeIC(spec(private(operatorv1.TCPProtocol)), status(private(operatorv1.TCPProtocol))),
+		},
 	}
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
@@ -1005,6 +1048,24 @@ func TestIsProxyProtocolNeeded(t *testing.T) {
 		privateStrategy = operatorv1.EndpointPublishingStrategy{
 			Type: operatorv1.PrivateStrategyType,
 		}
+		privateStrategyWithDefault = operatorv1.EndpointPublishingStrategy{
+			Type: operatorv1.PrivateStrategyType,
+			Private: &operatorv1.PrivateStrategy{
+				Protocol: operatorv1.DefaultProtocol,
+			},
+		}
+		privateStrategyWithTCP = operatorv1.EndpointPublishingStrategy{
+			Type: operatorv1.PrivateStrategyType,
+			Private: &operatorv1.PrivateStrategy{
+				Protocol: operatorv1.TCPProtocol,
+			},
+		}
+		privateStrategyWithPROXY = operatorv1.EndpointPublishingStrategy{
+			Type: operatorv1.PrivateStrategyType,
+			Private: &operatorv1.PrivateStrategy{
+				Protocol: operatorv1.ProxyProtocol,
+			},
+		}
 	)
 	testCases := []struct {
 		description string
@@ -1093,11 +1154,29 @@ func TestIsProxyProtocolNeeded(t *testing.T) {
 			expect:      true,
 		},
 		{
-			description: "private strategy shouldn't use PROXY",
+			description: "empty private strategy shouldn't use PROXY",
 			strategy:    &privateStrategy,
 			platform:    &awsPlatform,
 			expect:      false,
 		},
+		{
+			description: "private strategy specifying default shouldn't use PROXY",
+			strategy:    &privateStrategyWithDefault,
+			platform:    &awsPlatform,
+			expect:      false,
+		},
+		{
+			description: "private strategy specifying TCP shouldn't use PROXY",
+			strategy:    &privateStrategyWithTCP,
+			platform:    &awsPlatform,
+			expect:      false,
+		},
+		{
+			description: "private strategy specifying PROXY should use PROXY",
+			strategy:    &privateStrategyWithPROXY,
+			platform:    &awsPlatform,
+			expect:      true,
+		},
 	}
 
 	for _, tc := range testCases {