search

curityio / idsvr-helm

This repository contains the Curity Identity Server helm chart source code.
Apache License 2.0
10 stars 21 forks source link

Support specifying runtime deployment port as a parameter #13

Closed shuaibiyy closed 3 years ago

shuaibiyy commented 3 years ago

By introducing a curity.runtime.deployment.port parameter, the deployment container's port can remain on the default port (i.e. 8443) while allowing the runtime service (via curity.runtime.service.port) to be on a different port.

An example use case is one where kube dns is relied upon to reach kubernetes services by name only, i.e., being able to use http://curity-idsvr-runtime-svc instead of http://curity-idsvr-runtime-svc:8443.

anestos commented 3 years ago

Isn't it a good practice to still expose the Pod's port even if it is used only by the service? I would leave the default curity.runtime.deployment.port to be empty and expose an extra port from the Pod if that is set. i.e 

{{- if .Values.curity.runtime.deployment.port }}
- name: deployment-port
              containerPort: {{ .Values.curity.runtime.deployment.port }}
              protocol: TCP
{{- end }}
shuaibiyy commented 3 years ago

@anestos not sure I completely understand. With curity.runtime.deployment.port, the pod's http-port will still be exposed, and based on the curity docker image – it'd probably always be 8443. In the service spec, we then get to map the http-port to whatever port we want to expose on the service (e.g. port 80):

- port: {{ .Values.curity.runtime.service.port }}
  targetPort: http-port # always refers to whatever the value of `curity.runtime.deployment.port` is
  protocol: TCP
  name: http-port
anestos commented 3 years ago

Sorry i missanderstood the change. Looks good then

shuaibiyy commented 3 years ago

@anestos @travisspencer can this be merged?