docker icon indicating copy to clipboard operation
docker copied to clipboard

Added basic HTTPS support

Open mbosecke opened this issue 1 year ago • 5 comments

Added basic HTTPS support.

The user would configure this by mounting a JKS file to the container and providing some basic environment variables to specify the location of the file, the password for the file, and the alias of the key within the file. I updated the README to document this.

It's basic in the sense that it does require a fair bit of onus on the end-user to assemble a JKS file. The container itself will not create one using a self-signed certificate or anything like that. That could be a future enhancement.

mbosecke avatar Nov 15 '23 22:11 mbosecke

Is this addition a good fit for a geoserver docker? My guess is that normally, you'd add an nginx container next to it, which can handle the https part. Or you'd put the geoserver docker in a kubernetes cluster which has basic support for https ingress, including handling it via letsencrypt.

The "jks" part would be a better fit for the generic upstream tomcat docker, it is not geoserver-specific. (Though, if I remember correctly, the tomcat docker folks only want to do the basic docker packaging of the basic tomcat, so I'm guessing they'll also say "just use an nginx docker next to it".)

reinout avatar Nov 16 '23 08:11 reinout

To be totally honest, my only reason for implementing this is to try and work around a bug with the CAS extension that occurs when geoserver is served over non-HTTPS; the extension isn't obeying the "proxy base URL" setting in a way I would hope it to. I still haven't fully worked around that bug, but I mention this to highlight that this pull request doesn't come from a "legitimate" use-case on my end. I personally wouldn't use HTTPS in the docker container if I didn't have to.

However, to counter that argument, it looks like the most popular "docker-geoserver" project that I can find has implemented such a feature so I assume that some people have use cases for this. If I was to speculate, I could imagine that it would be useful for very simple architectures where there is a lack of moving pieces (i.e. no kubernetes, nor an external reverse proxy, etc). Simple but potentially valid use cases. Perhaps dev environments, or with "testcontainers", or just one-off geoserver deployments for misc purposes.

mbosecke avatar Nov 16 '23 15:11 mbosecke

I've submitted my concerns with the CAS extension to this ticket.

The CAS extension has some flaws, and I've confirmed that in combination with the inability to run geoserver on HTTPS within docker, it amplifies those flaws making it effectively unusable. IMO I think this project should support HTTPS and the CAS extension needs to be improved in regards to how it's building dynamic URLs.

mbosecke avatar Nov 29 '23 20:11 mbosecke

Thank you. I generally agree with @reinout that https should better be handled on another level (docker-compose with nginx or kubernetes with ingress). On the other side I don't see a problem to merge this as long this is a fully optional feature that is turned off by default.

I would leave this open some more days for discussion, but if there are no objections, then i'm happy to merge it

buehner avatar Dec 20 '23 11:12 buehner

Due to the merge of https://github.com/geoserver/docker/pull/39 we have conflicts now.

Would be great if you could resolve them, if you still want this feature to be merged @mbosecke

buehner avatar Feb 09 '24 08:02 buehner

Seems we can merge again, thx. As this is a fully optional feature and I could test this locally, I will merge now.

buehner avatar May 29 '24 12:05 buehner