[Part of #177] Feature: refector nginx deployment #195
Conversation
mwallschlaeger
added
enhancement
| location /geoserver { | ||
| client_max_body_size {{ .Values.nginx.maxClientBodySize }}; | ||
|
|
||
| {{- if .Values.nginx.external_cors.enabled }} |
There was a problem hiding this comment.
You may consider removing CORS config at nginx completely, just in favor of configuring CORS via django-cors-headers.
There was a problem hiding this comment.
@AlexGacon any opinion on that from your side?
There was a problem hiding this comment.
It could be a solution, even if it implies having custom code on the geonode side if required (but since we now support GeoNode projects, it sounds possible)
|
@AlexGacon I would like to add this PR to the next release. Any opinions on this? If you have an alternate nginx conf this might breaks your system right? |
|
@mwallschlaeger yes indeed it would break the current setup of one of my customer. We will try the solution suggested by @ridoo : my only fear is that we replace a mechanism relying on a maintained and proven solution by a mechanism relying on a solution whose maintenance is less proven. |
|
@AlexGacon yes thats why I"m afraid of merging this into the main branch. But potentially this changes will improve the performance of geonode-k8s and make the chart more the k8s way. Which gives us more flexibility with the configuration of the endpoints without manipulating nginx configs anymore. I will try to benchmark perfomance impact of this change and will comback with results on this, later this week. |
mwallschlaeger
deleted the
issue_#177_Feature_refector_nginx_deployment_pycsw_geoserver
branch
Description
if ingress is enabled, this change will let
/geoserverand/catalogue/csw(.Values.pycsw.endpoint) be an individual path inside thekubernetes.ingressobject in geonode-k8s. This change will improve performance of traffic towardsgeoserverandpycswas this traffic does not traverse geonode-nginx container anymore,@AlexGacon external cors (.Values.nginx.external_cors.enabled) must now be set as
ingress.annotationsthis PR is not a full fix of issue #177 more like a partial fix so far.
Type of Change
Please select the relevant option:
Related Issue
If there is an existing issue related to this pull request, please reference it here.
ref #177
Checklist
Please ensure that your pull request meets the following requirements:
Additional Notes
Any additional information or context regarding the pull request can be provided here.
Thank you for creating this pull request