Alvearie / alvearie-helm

repository for the helm chart source and package for Alvearie projects
https://artifacthub.io/packages/helm/linuxforhealth
Apache License 2.0
3 stars 5 forks source link

[FHIR] Expose HTTP port 9080 #47

Closed chgl closed 3 years ago

chgl commented 3 years ago

Currently, only port 9443 is exposed as a service. Since this endpoint uses a self-signed TLS certificate, it's often challenging to interact with the server from within the cluster without running into TLS errors.

It would be helpful to expose port 9080 as well. - either by default or maybe as an option.

I tried just adding

          ports:
            - name: https
              containerPort: 9443
              protocol: TCP
            - name: http
              containerPort: 9080
              protocol: TCP

to the deployment, but it looks like the server.xml has to be modified as well:

    <httpEndpoint id="defaultHttpEndpoint" host="*" httpPort="-1" httpsPort="9443" onError="FAIL"/>

Does it make sense to add a new env var to configure the httpPort first?

    <!-- not sure the default `-1` does even work here. -->
    <httpEndpoint id="defaultHttpEndpoint" host="*" httpPort="${HTTP_PORT:-1}" httpsPort="9443" onError="FAIL"/>
lmsurpre commented 3 years ago

Our security team makes us run with TLS, even for in-cluster stuff. I actually do think thats the best answer for FHIR due to the sensitive nature of the data.

However, I agree that we should make it easier to enable HTTP support for folks that want to opt in to that.

Relates to https://github.com/Alvearie/alvearie-helm/issues/35 in that you can also accomplish the server.xml change via a configDropin like the following:

<server>
    <!-- Serve non-TLS traffic on port 9080 -->
    <httpEndpoint id="defaultHttpEndpoint" httpPort="9080"/>
</server>

However, I agree that this one might warrant just a single helm value where changing it from its default (-1 ?) would both

  1. add the http port to the service; and
  2. configure liberty to expose the same http port
chgl commented 3 years ago

I actually do think thats the best answer for FHIR due to the sensitive nature of the data.

I definitely agree, although I think transparent encryption using something like mTLS via a service mesh is a convenient option as well.

Would the solution be to add a config value like enableInsecureHttpEndpoint (to drive home that it's really insecure by default) which would add a config drop-in to theconfigDropins/overrides` which simply contains your XML snippet and expose the service?

Or should we wait for #35 and add it as a templated XML server config? Or add an env var to the upstream default server.xml?

lmsurpre commented 3 years ago

Great questions. If you're interested in contributing it, I'd say just go with whatever is easiest. I have some other changes locally that I need to get in a PR soon and we can always evolve it later when we get to #35.

Would the solution be to add a config value like enableInsecureHttpEndpoint (to drive home that it's really insecure by default) which would add a config drop-in to the configDropins/overrides` which simply contains your XML snippet and expose the service?

Yeah, something like that. I was trying to decide if there is any benefit to allowing them to configure the port or if it should be just true/false... I think just true/false would be ok since we don't support customizing the HTTPS port and it doesn't seem like that would have much value.

lmsurpre commented 3 years ago

@chgl FYI I invited you to the project as a contributor so that I can add you as a reviewer on PRs...hope that is ok :-)

chgl commented 3 years ago

oh exciting! Thanks :)

chgl commented 3 years ago

Closed via #50