[release-4.13] OCPBUGS-15225: Add support for protocolStrategy API field to enable force_tcp configuration

go.mod

@@ -6,7 +6,7 @@ require (
 	github.com/apparentlymart/go-cidr v1.0.0
 	github.com/google/go-cmp v0.5.9
 	github.com/kevinburke/go-bindata v3.11.0+incompatible
-	github.com/openshift/api v0.0.0-20230120195050-6ba31fa438f2
+	github.com/openshift/api v0.0.0-20230711143145-b5a47f95ba70
 	github.com/openshift/build-machinery-go v0.0.0-20220913142420-e25cf57ea46d
 	github.com/sirupsen/logrus v1.8.1
 	k8s.io/api v0.26.1

go.sum

@@ -212,8 +212,8 @@ github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e h1:fD57ERR4JtEqsWb
 github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e/go.mod h1:zD1mROLANZcx1PVRCS0qkT7pwLkGfwJo4zjcN/Tysno=
 github.com/onsi/ginkgo/v2 v2.6.0 h1:9t9b9vRUbFq3C4qKFCGkVuq/fIHji802N1nrtkh1mNc=
 github.com/onsi/gomega v1.24.1 h1:KORJXNNTzJXzu4ScJWssJfJMnJ+2QJqhoQSRwNlze9E=
-github.com/openshift/api v0.0.0-20230120195050-6ba31fa438f2 h1:+nw0/d4spq880W7S74Twi5YU2ulsl3/a9o4OEZptYp0=
-github.com/openshift/api v0.0.0-20230120195050-6ba31fa438f2/go.mod h1:ctXNyWanKEjGj8sss1KjjHQ3ENKFm33FFnS5BKaIPh4=
+github.com/openshift/api v0.0.0-20230711143145-b5a47f95ba70 h1:fFU38cQYzz+gZAMfQ0wbxPpbWq5IFYPAa5nPAAaXX7w=
+github.com/openshift/api v0.0.0-20230711143145-b5a47f95ba70/go.mod h1:ctXNyWanKEjGj8sss1KjjHQ3ENKFm33FFnS5BKaIPh4=
 github.com/openshift/build-machinery-go v0.0.0-20220913142420-e25cf57ea46d h1:RR4ah7FfaPR1WePizm0jlrsbmPu91xQZnAsVVreQV1k=
 github.com/openshift/build-machinery-go v0.0.0-20220913142420-e25cf57ea46d/go.mod h1:b1BuldmJlbA/xYtdZvKi+7j5YGB44qJUJDZ9zwiNCfE=
 github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=

manifests/0000_70_dns-operator_00.crd.yaml

@@ -112,6 +112,12 @@ spec:
                               - RoundRobin
                               - Sequential
                             type: string
+                          protocolStrategy:
+                            description: protocolStrategy specifies the protocol to use for upstream DNS requests. Valid values for protocolStrategy are "TCP" and omitted. When omitted, this means no opinion and the platform is left to choose a reasonable default, which is subject to change over time. The current default is to use the protocol of the original client request. "TCP" specifies that the platform should use TCP for all upstream DNS requests, even if the client request uses UDP. "TCP" is useful for UDP-specific issues such as those created by non-compliant upstream resolvers, but may consume more bandwidth or increase DNS response time. Note that protocolStrategy only affects the protocol of DNS requests that CoreDNS makes to upstream resolvers. It does not affect the protocol of DNS requests between clients and CoreDNS.
+                            enum:
+                              - TCP
+                              - ""
+                            type: string
                           transportConfig:
                             description: "transportConfig is used to configure the transport type, server name, and optional custom CA or CA bundle to use when forwarding DNS requests to an upstream resolver. \n The default value is \"\" (empty) which results in a standard cleartext connection being used when forwarding DNS requests to an upstream resolver."
                             properties:
@@ -172,6 +178,12 @@ spec:
                         - RoundRobin
                         - Sequential
                       type: string
+                    protocolStrategy:
+                      description: protocolStrategy specifies the protocol to use for upstream DNS requests. Valid values for protocolStrategy are "TCP" and omitted. When omitted, this means no opinion and the platform is left to choose a reasonable default, which is subject to change over time. The current default is to use the protocol of the original client request. "TCP" specifies that the platform should use TCP for all upstream DNS requests, even if the client request uses UDP. "TCP" is useful for UDP-specific issues such as those created by non-compliant upstream resolvers, but may consume more bandwidth or increase DNS response time. Note that protocolStrategy only affects the protocol of DNS requests that CoreDNS makes to upstream resolvers. It does not affect the protocol of DNS requests between clients and CoreDNS.
+                      enum:
+                        - TCP
+                        - ""
+                      type: string
                     transportConfig:
                       description: "transportConfig is used to configure the transport type, server name, and optional custom CA or CA bundle to use when forwarding DNS requests to an upstream resolver. \n The default value is \"\" (empty) which results in a standard cleartext connection being used when forwarding DNS requests to an upstream resolver."
                       properties:

pkg/operator/controller/controller_dns_configmap.go

@@ -55,6 +55,9 @@ var corefileTemplate = template.Must(template.New("Corefile").Funcs(template.Fun
         {{- end}}
         {{- end}}
         policy {{ CoreDNSForwardingPolicy .Policy }}
+        {{- if eq "TCP" $fp.ProtocolStrategy }}
+        force_tcp
+        {{- end}}
     }
     {{- end}}
     errors
@@ -91,6 +94,9 @@ var corefileTemplate = template.Must(template.New("Corefile").Funcs(template.Fun
         {{- end}}
         {{- end}}
         policy {{ CoreDNSForwardingPolicy .Policy }}
+        {{- if eq "TCP" $.UpstreamResolvers.ProtocolStrategy }}
+        force_tcp
+        {{- end}}
     }
     {{- end}}
     cache {{ .PositiveTTL }} {
@@ -159,8 +165,9 @@ func desiredDNSConfigMap(dns *operatorv1.DNS, clusterDomain string, caBundleRevi
 				Type: operatorv1.SystemResolveConfType,
 			},
 		},
-		Policy:          operatorv1.SequentialForwardingPolicy,
-		TransportConfig: dns.Spec.UpstreamResolvers.TransportConfig,
+		Policy:           operatorv1.SequentialForwardingPolicy,
+		TransportConfig:  dns.Spec.UpstreamResolvers.TransportConfig,
+		ProtocolStrategy: dns.Spec.UpstreamResolvers.ProtocolStrategy,
 	}
 
 	if len(dns.Spec.UpstreamResolvers.Upstreams) > 0 {

pkg/operator/controller/controller_dns_configmap_test.go

@@ -908,6 +908,88 @@ func TestDesiredDNSConfigmapUpstreamResolvers(t *testing.T) {
 			},
 			expectedCoreFile: mustLoadTestFile(t, "tls_with_non_existing_cabundle"),
 		},
+		{
+			name: "CR of protocolStrategy of TCP on ForwardPlugin",
+			dns: &operatorv1.DNS{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: DefaultDNSController,
+				},
+				Spec: operatorv1.DNSSpec{
+					Servers: []operatorv1.Server{
+						{
+							Name:  "foo",
+							Zones: []string{"foo.com"},
+							ForwardPlugin: operatorv1.ForwardPlugin{
+								Upstreams:        []string{"1.1.1.1", "2.2.2.2:5353"},
+								Policy:           operatorv1.RoundRobinForwardingPolicy,
+								ProtocolStrategy: operatorv1.ProtocolStrategyTCP,
+							},
+						},
+					},
+				},
+			},
+			expectedCoreFile: mustLoadTestFile(t, "forwardplugin_protocolstrategy_tcp"),
+		},
+		{
+			name: "CR of protocolStrategy of None or Default on ForwardPlugin",
+			dns: &operatorv1.DNS{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: DefaultDNSController,
+				},
+				Spec: operatorv1.DNSSpec{
+					Servers: []operatorv1.Server{
+						{
+							Name:  "foo",
+							Zones: []string{"foo.com"},
+							ForwardPlugin: operatorv1.ForwardPlugin{
+								Upstreams:        []string{"1.1.1.1", "2.2.2.2:5353"},
+								Policy:           operatorv1.RoundRobinForwardingPolicy,
+								ProtocolStrategy: operatorv1.ProtocolStrategyDefault,
+							},
+						},
+					},
+				},
+			},
+			expectedCoreFile: mustLoadTestFile(t, "forwardplugin_protocolstrategy_none"),
+		},
+		{
+			name: "CR with upstreamResolver with protocolStrategy of TCP",
+			dns: &operatorv1.DNS{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: DefaultDNSController,
+				},
+				Spec: operatorv1.DNSSpec{
+					UpstreamResolvers: operatorv1.UpstreamResolvers{
+						Upstreams: []operatorv1.Upstream{
+							{
+								Type: operatorv1.SystemResolveConfType,
+							},
+						},
+						ProtocolStrategy: operatorv1.ProtocolStrategyTCP,
+					},
+				},
+			},
+			expectedCoreFile: mustLoadTestFile(t, "upstreamresolver_protocolstrategy_tcp"),
+		},
+		{
+			name: "CR with upstreamResolver with protocolStrategy of Default or None",
+			dns: &operatorv1.DNS{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: DefaultDNSController,
+				},
+				Spec: operatorv1.DNSSpec{
+					UpstreamResolvers: operatorv1.UpstreamResolvers{
+						Upstreams: []operatorv1.Upstream{
+							{
+								Type: operatorv1.SystemResolveConfType,
+							},
+						},
+						ProtocolStrategy: operatorv1.ProtocolStrategyDefault,
+					},
+				},
+			},
+			expectedCoreFile: mustLoadTestFile(t, "upstreamresolver_protocolstrategy_none"),
+		},
 	}
 
 	clusterDomain := "cluster.local"

pkg/operator/controller/testdata/forwardplugin_protocolstrategy_none

@@ -0,0 +1,41 @@
+# foo
+foo.com:5353 {
+    prometheus 127.0.0.1:9153
+    forward . 1.1.1.1 2.2.2.2:5353 {
+        policy round_robin
+    }
+    errors
+    log . {
+        class error
+    }
+    bufsize 512
+    cache 900 {
+        denial 9984 30
+    }
+}
+.:5353 {
+    bufsize 512
+    errors
+    log . {
+        class error
+    }
+    health {
+        lameduck 20s
+    }
+    ready
+    kubernetes cluster.local in-addr.arpa ip6.arpa {
+        pods insecure
+        fallthrough in-addr.arpa ip6.arpa
+    }
+    prometheus 127.0.0.1:9153
+    forward . /etc/resolv.conf {
+        policy sequential
+    }
+    cache 900 {
+        denial 9984 30
+    }
+    reload
+}
+hostname.bind:5353 {
+    chaos
+}

pkg/operator/controller/testdata/forwardplugin_protocolstrategy_tcp

@@ -0,0 +1,42 @@
+# foo
+foo.com:5353 {
+    prometheus 127.0.0.1:9153
+    forward . 1.1.1.1 2.2.2.2:5353 {
+        policy round_robin
+        force_tcp
+    }
+    errors
+    log . {
+        class error
+    }
+    bufsize 512
+    cache 900 {
+        denial 9984 30
+    }
+}
+.:5353 {
+    bufsize 512
+    errors
+    log . {
+        class error
+    }
+    health {
+        lameduck 20s
+    }
+    ready
+    kubernetes cluster.local in-addr.arpa ip6.arpa {
+        pods insecure
+        fallthrough in-addr.arpa ip6.arpa
+    }
+    prometheus 127.0.0.1:9153
+    forward . /etc/resolv.conf {
+        policy sequential
+    }
+    cache 900 {
+        denial 9984 30
+    }
+    reload
+}
+hostname.bind:5353 {
+    chaos
+}

pkg/operator/controller/testdata/upstreamresolver_protocolstrategy_none

@@ -0,0 +1,26 @@
+.:5353 {
+    bufsize 512
+    errors
+    log . {
+        class error
+    }
+    health {
+        lameduck 20s
+    }
+    ready
+    kubernetes cluster.local in-addr.arpa ip6.arpa {
+        pods insecure
+        fallthrough in-addr.arpa ip6.arpa
+    }
+    prometheus 127.0.0.1:9153
+    forward . /etc/resolv.conf {
+        policy sequential
+    }
+    cache 900 {
+        denial 9984 30
+    }
+    reload
+}
+hostname.bind:5353 {
+    chaos
+}

pkg/operator/controller/testdata/upstreamresolver_protocolstrategy_tcp

@@ -0,0 +1,27 @@
+.:5353 {
+    bufsize 512
+    errors
+    log . {
+        class error
+    }
+    health {
+        lameduck 20s
+    }
+    ready
+    kubernetes cluster.local in-addr.arpa ip6.arpa {
+        pods insecure
+        fallthrough in-addr.arpa ip6.arpa
+    }
+    prometheus 127.0.0.1:9153
+    forward . /etc/resolv.conf {
+        policy sequential
+        force_tcp
+    }
+    cache 900 {
+        denial 9984 30
+    }
+    reload
+}
+hostname.bind:5353 {
+    chaos
+}