All committers have signed the CLA.

Thanks for this PR. Unfortunately, I don't think this really belongs here. To borrow from OO philosophy, the Law of Demeter or principle of least knowledge applies here. Nginx reverse proxy concerns for THREDDS are not germane to this project. If you really need to make customizations that is what docker volume mounts are for. Something like

docker run -it --rm -p 8888:8080 -v  /home/joe-user/server.xml:/usr/local/tomcat/conf/server.xml unidata/thredds-docker:latest

with your customizations in /home/joe-user/server.xml.

Alternatively, you can build your own container that inherits from unidata/thredds-docker that has your own server.xml.

As an aside, I've done this reverse proxy type setup before. I recall it not being totally easy to get right. You can find what I did at this now defunct project. It uses a technology called traefik in conjunction with nginx.

To borrow from OO philosophy, the Law of Demeter or principle of least knowledge applies here. Nginx reverse proxy concerns for THREDDS are not germane to this project.

Totally agreed with the philosophy. I probably should have explained my changes better, my bad.

The changes is not specific to Nginx, it was tested with Nginx because that is what I have.

The change is to enable Tomcat to handle the 2 http headers "X-Forwarded-Proto" and "X-forwarded-For" out of the box. Out of the box means much better onboarding experience for new users deploying Thredds in their organization.

Being able to simply handle http headers opens up Thredds to behave nicely behind multiple different proxies, not just Apache with mod_jk or mod_proxy as documented here https://www.unidata.ucar.edu/software/tds/current/reference/TomcatBehindProxyServer.html. This docs feels like only Apache is supported as front proxy to Thredds, which is not true.

If the 2 http headers are not set by the front proxy, then it's just like now where there is no handling for them. In fact, setting these 2 headers is not default behavior for Nginx, I had to set them in the Nginx config. Those 2 http headers together with the "Host" header (this "Host" header already works out of the box with Thredds) should still be documented in how to configure front proxy for Thredds.

It's a best practice to set those http headers for front proxies and when best practices are followed, Thredds docker image will "just work".

Alternatively, you can build your own container that inherits from unidata/thredds-docker that has your own server.xml.

Yes, this is another avenue but generally, imho for a good user experience, things should just work.

As an aside, I've done this reverse proxy type setup before. I recall it not being totally easy to get right. You can find what I did at this now defunct project. It uses a technology called traefik in conjunction with nginx.

No experience with traefik but solusions using http headers is generic so should work with any proxies.

Another reason for "any proxies" solution is that Thredds might not be directly behind the proxy serving as SSL termination. In large scale deployment, it might possibly be this set up instead: customer client/browser -> Apache/Nginx for SSL termination -> HAProxy for load balancing -> some customer built custom proxy for authentication and access control -> then finally Thredds. There might be even more proxies in the middle, now that containers and micro-services are popular.

So this PR will improve the user experience when users are using the Thredds docker image and their proxy is properly configured. If user install and configure Thredds by hand, this RemoveIpValve should still be documented and be added manually by the user.

But any bits to improve generic out of the box user experience is good. So I would like to request reconsideration to accept my PR.

Your comments presume unidata/tomcat-docker is solely for THREDDS which it is not. If it were uniquely for THREDDS, then we would not have created this project and would have put all the work into the unidata/thredds-docker Dockerfile. unidata/tomcat-docker should not have to know anything about THREDDS -- back to the separation of concerns point I was making earlier.

In addition, where does the server.xml herein come from anyway? It is essentially the stock server.xml from tomcat:8.5 with some minor OWASP recommended security improvements and also re-establishing relaxed chars which used to be allowed in earlier versions of Tomcat. Apart from that, it is just the generic server.xml from tomcat:8.5

What I would suggest as your next step is to make a PR to https://github.com/docker-library/tomcat/pulls since every argument you have made thus far would also apply to docker-library/tomcat.