Add support for setting appProtocol to service ports

[matrix-root]

Fix problem with Istio mentioned in #23

Adds service.setAppProtocol option in values which allows render service ports with additional field.
Allows Istio to correctly identify service protocols as described in docs.

[FZambia]

@LvLs9 thanks! What do you think if we make this in a bit different way? Like this:

diff --git a/charts/centrifugo/templates/service-grpc-api.yaml b/charts/centrifugo/templates/service-grpc-api.yaml
index 75bcf63..1709e94 100644
--- a/charts/centrifugo/templates/service-grpc-api.yaml
+++ b/charts/centrifugo/templates/service-grpc-api.yaml
@@ -16,6 +16,9 @@ spec:
     - port: {{ .Values.grpcService.port }}
       targetPort: grpc
       protocol: TCP
+      {{- if .Values.grpcService.appProtocol }}
+      appProtocol: {{ .Values.grpcService.appProtocol }}
+      {{- end }}
       name: grpc
       {{- if (and (eq .Values.grpcService.type "NodePort") (not (empty .Values.grpcService.nodePort))) }}
       nodePort: {{ .Values.grpcService.nodePort }}
diff --git a/charts/centrifugo/templates/service-internal.yaml b/charts/centrifugo/templates/service-internal.yaml
index e469597..22a0a87 100644
--- a/charts/centrifugo/templates/service-internal.yaml
+++ b/charts/centrifugo/templates/service-internal.yaml
@@ -16,6 +16,9 @@ spec:
     - port: {{ .Values.internalService.port }}
       targetPort: internal
       protocol: TCP
+      {{- if .Values.internalService.appProtocol }}
+      appProtocol: {{ .Values.internalService.appProtocol }}
+      {{- end }}
       name: internal
       {{- if (and (eq .Values.internalService.type "NodePort") (not (empty .Values.internalService.nodePort))) }}
       nodePort: {{ .Values.internalService.nodePort }}
diff --git a/charts/centrifugo/templates/service-uni-grpc.yaml b/charts/centrifugo/templates/service-uni-grpc.yaml
index 56716f9..ffc5e15 100644
--- a/charts/centrifugo/templates/service-uni-grpc.yaml
+++ b/charts/centrifugo/templates/service-uni-grpc.yaml
@@ -16,6 +16,9 @@ spec:
     - port: {{ .Values.uniGrpcService.port }}
       targetPort: uni-grpc
       protocol: TCP
+      {{- if .Values.uniGrpcService.appProtocol }}
+      appProtocol: {{ .Values.uniGrpcService.appProtocol }}
+      {{- end }}
       name: uni-grpc
       {{- if (and (eq .Values.uniGrpcService.type "NodePort") (not (empty .Values.uniGrpcService.nodePort))) }}
       nodePort: {{ .Values.uniGrpcService.nodePort }}
diff --git a/charts/centrifugo/templates/service.yaml b/charts/centrifugo/templates/service.yaml
index 5b1d9cb..d6b4a8d 100644
--- a/charts/centrifugo/templates/service.yaml
+++ b/charts/centrifugo/templates/service.yaml
@@ -32,6 +32,9 @@ spec:
     - port: {{ .Values.grpcService.port }}
       targetPort: grpc
       protocol: TCP
+      {{- if .Values.grpcService.appProtocol }}
+      appProtocol: {{ .Values.grpcService.appProtocol }}
+      {{- end }}
       name: grpc
       {{- if (and (eq .Values.grpcService.type "NodePort") (not (empty .Values.grpcService.nodePort))) }}
       nodePort: {{ .Values.grpcService.nodePort }}
diff --git a/charts/centrifugo/values.yaml b/charts/centrifugo/values.yaml
index c0203cd..cd14143 100644
--- a/charts/centrifugo/values.yaml
+++ b/charts/centrifugo/values.yaml
@@ -36,6 +36,9 @@ service:
   ##
   annotations: {}
   ##
+  ## Specify custom appProtocol for a service port.
+  appProtocol: ""
+  ##
   ## Use separate service for internal endpoints. It could be useful for configuring same port number for all services.
   useSeparateInternalService: false
   ## Use separate service for GRPC API endpoints. It could be useful for configuring same port number for all services.
@@ -53,6 +56,8 @@ internalService:
     # prometheus.io/scrape: "true"
     # prometheus.io/path: "/metrics"
     # prometheus.io/port: "9000"
+  ## Specify custom appProtocol for a service port.
+  appProtocol: ""
 
 grpcService:
   port: 10000
@@ -61,6 +66,8 @@ grpcService:
   # Static NodePort, if set.
   # nodePort: 30102
   annotations: {}
+  ## Specify custom appProtocol for a service port.
+  appProtocol: ""
 
 uniGrpcService:
   port: 11000
@@ -69,6 +76,8 @@ uniGrpcService:
   # Static NodePort, if set.
   # nodePort: 30103
   annotations: {}
+  ## Specify custom appProtocol for a service port.
+  appProtocol: ""
 
 ingress:
   enabled: false

This requires more things to configure – but seems to fit better existing service separations in the chart.

Another reason is that I am not fully sure TCP is the best choice for appProtocol everywhere - since Istio users can benefit from defining appProtocol as grpc for grpc services, and I think external port should work fine with http appProtocol (maybe it does not at the moment - but can be an Istio bug?).

So I think having an explicit settings here is the safest thing to do without more deep investigation of Istio behavior.

[matrix-root]

Yes, agreed 👍

[FZambia]

You also need to bump version https://github.com/centrifugal/helm-charts/blob/master/charts/centrifugo/Chart.yaml#L4 (I think to 7.2.1 is fine for these changes) - otherwise CI will complain.

[FZambia]

Many thanks, Lev!