fix(k8s): tolerate unknown appProtocol, give kuma.io/protocol annotat…

…ion priority Signed-off-by: Mike Beaumont <mjboamail@gmail.com>
kumahq · Jun 28, 2023 · dc15103 · dc15103
1 parent e977dae
commit dc15103
Show file tree

Hide file tree

Showing 5 changed files with 15 additions and 9 deletions.
diff --git a/pkg/plugins/runtime/k8s/controllers/inbound_converter.go b/pkg/plugins/runtime/k8s/controllers/inbound_converter.go
@@ -187,20 +187,26 @@ func ProtocolTagFor(svc *kube_core.Service, svcPort *kube_core.ServicePort) stri
 	var protocolValue string
 	protocolAnnotation := fmt.Sprintf("%d.service.kuma.io/protocol", svcPort.Port)
 
-	if svcPort.AppProtocol != nil {
+	if explicitKumaProtocol, ok := svc.Annotations[protocolAnnotation]; ok {
+		protocolValue = explicitKumaProtocol
+	} else if svcPort.AppProtocol != nil {
 		protocolValue = *svcPort.AppProtocol
-	} else {
-		protocolValue = svc.Annotations[protocolAnnotation]
+		// `appProtocol` can be any protocol and if we don't explicitly support
+		// it, let the default below take effect
+		if core_mesh.ParseProtocol(protocolValue) == core_mesh.ProtocolUnknown {
+			protocolValue = ""
+		}
 	}
 
 	if protocolValue == "" {
 		// if `appProtocol` or `<port>.service.kuma.io/protocol` is missing or has an empty value
 		// we want Dataplane to have a `protocol: tcp` tag in order to get user's attention
 		protocolValue = core_mesh.ProtocolTCP
 	}
-	// if `appProtocol` or `<port>.service.kuma.io/protocol` field is present but has an invalid value
+
+	// if `<port>.service.kuma.io/protocol` field is present but has an invalid value
 	// we still want Dataplane to have a `protocol: <lowercase value>` tag in order to make it clear
-	// to a user that at least `appProtocol` or `<port>.service.kuma.io/protocol` has an effect
+	// to a user that at least `<port>.service.kuma.io/protocol` has an effect
 	return strings.ToLower(protocolValue)
 }
 

diff --git a/pkg/plugins/runtime/k8s/controllers/pod_converter_test.go b/pkg/plugins/runtime/k8s/controllers/pod_converter_test.go
@@ -844,7 +844,7 @@ var _ = Describe("ProtocolTagFor(..)", func() {
 		}),
 		Entry("appProtocol has an unknown value", testCase{
 			appProtocol: utilpointer.String("not-yet-supported-protocol"),
-			expected:    "not-yet-supported-protocol", // we want Kuma's behavior to be straightforward to a user (just copy appProtocol lowercase value)
+			expected:    "tcp", // we want Kuma's behavior to be straightforward to a user (appProtocol is not Kuma specific)
 		}),
 		Entry("appProtocol has a lowercase value", testCase{
 			appProtocol: utilpointer.String("HtTp"),

diff --git a/pkg/plugins/runtime/k8s/controllers/testdata/01.dataplane.yaml b/pkg/plugins/runtime/k8s/controllers/testdata/01.dataplane.yaml
@@ -33,7 +33,7 @@ spec:
         k8s.kuma.io/namespace: demo
         k8s.kuma.io/service-name: sample
         k8s.kuma.io/service-port: "7071"
-        kuma.io/protocol: mongo
+        kuma.io/protocol: tcp
         kuma.io/service: sample_playground_svc_7071
         kuma.io/zone: zone-1
         version: "0.1"

diff --git a/pkg/plugins/runtime/k8s/controllers/testdata/10.dataplane.yaml b/pkg/plugins/runtime/k8s/controllers/testdata/10.dataplane.yaml
@@ -31,7 +31,7 @@ spec:
         k8s.kuma.io/namespace: demo
         k8s.kuma.io/service-name: sample
         k8s.kuma.io/service-port: "7071"
-        kuma.io/protocol: mongo
+        kuma.io/protocol: tcp
         kuma.io/service: sample_playground_svc_7071
         kuma.io/zone: zone-1
         version: "0.1"

diff --git a/pkg/plugins/runtime/k8s/controllers/testdata/11.dataplane.yaml b/pkg/plugins/runtime/k8s/controllers/testdata/11.dataplane.yaml
@@ -34,7 +34,7 @@ spec:
         k8s.kuma.io/namespace: demo
         k8s.kuma.io/service-name: sample
         k8s.kuma.io/service-port: "7071"
-        kuma.io/protocol: mongo
+        kuma.io/protocol: tcp
         kuma.io/service: sample_playground_svc_7071
         kuma.io/zone: zone-1
         version: "0.1"