-
Notifications
You must be signed in to change notification settings - Fork 271
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Superfluous nginx deployment? #367
Comments
Sounds interesting! I think nobody thought about it. Would be interested to test this out if you make a PR. |
I guess this depends in the ingress you are using, but we should definitely have an option for enabling it. |
I think the major ingress controllers like |
I'm interresing into this enhancement. What do you think about remove the nginx part and modify the image:
repository: nextcloud
# version: "24.0.3"
type: "apache"
# tag: 24.0.3-apache
pullPolicy: IfNotPresent
# pullSecrets:
# - myRegistrKeySecretName The user can set the image tag by At the ingress section, add a |
The probes parts must be updated too. When Currently, the probes are disabled :
|
@tcoupin we currently have I experimented with this a little bit after #386 was merged, and I could set the Here's the config map I used: apiVersion: v1
kind: ConfigMap
metadata:
name: nextcloud-fast-cm
namespace: nextcloud
data:
SCRIPT_FILENAME: "/var/www/html/index.php"
PATH_INFO: "$path_info"
HTTPS: "on"
modHeadersAvailable: true
# Enable pretty urls
front_controller_active: true I can't figure out what parameters to pass into the ConfigMap to make this work 🤔 Here's my image:
repository: nextcloud
# image will be nextcloud:$APP_VERSION-fpm e.g. nextcloud:26.0.1-fpm
flavor: fpm
ingress:
enabled: true
# className: ingress-nginx
annotations:
kubernetes.io/ingress.class : "ingress-nginx"
cert-manager.io/cluster-issuer: "letsencrypt"
kubernetes.io/tls-acme: "true"
nginx.ingress.kubernetes.io/backend-protocol: "FCGI"
nginx.ingress.kubernetes.io/fastcgi-index: "index.php"
nginx.ingress.kubernetes.io/fastcgi-params-configmap: "nextcloud-fast-cm"
nginx.ingress.kubernetes.io/proxy-body-size: 4G
nginx.ingress.kubernetes.io/enable-cors: "true"
nginx.ingress.kubernetes.io/cors-allow-headers: "X-Forwarded-For"
nginx.ingress.kubernetes.io/server-snippet: |-
server_tokens off;
proxy_hide_header X-Powered-By;
rewrite ^/.well-known/webfinger /public.php?service=webfinger last;
rewrite ^/.well-known/host-meta /public.php?service=host-meta last;
rewrite ^/.well-known/host-meta.json /public.php?service=host-meta-json;
location = /.well-known/carddav {
return 301 $scheme://$host/remote.php/dav;
}
location = /.well-known/caldav {
return 301 $scheme://$host/remote.php/dav;
}
location = /robots.txt {
allow all;
log_not_found off;
access_log off;
}
location ~ ^/(?:build|tests|config|lib|3rdparty|templates|data)/ {
deny all;
}
location ~ ^/(?:autotest|occ|issue|indie|db_|console) {
deny all;
}
tls:
- secretName: nextcloud-tls
hosts:
- nextcloud.dev.coolwebsitefordogs.com
labels: {}
path: / |
@JosefWN were you ever able to experiment with this? Also open to anyone else in the community that may have had experience with this, as I'm unsure how to make this work. |
I still haven't had success in making this work, but I got a lot further than last time. Here's where I got to the other day while testing this again: I used the the branch in this PR to test everything, because I needed to be able to set securityContext without having an nginx container: #379 Here's my FastCGI configMap for the ingress-nginx object to use that I deployed to the nextcloud namespace on my cluster: apiVersion: v1
kind: ConfigMap
metadata:
annotations:
argocd.argoproj.io/hook: PreSync
name: nextcloud-fastcgi-cm
data:
modHeadersAvailable: "true"
front_controller_active: "true"
DOCUMENT_ROOT: "/var/www/html"
SCRIPT_FILENAME: "$document_root$fastcgi_script_name"
PATH_INFO: "$fastcgi_path_info"
HTTPS: "1" Image and ingress section of my values.yaml: image:
flavor: fpm-alpine
ingress:
enabled: true
className: nginx
annotations:
cert-manager.io/cluster-issuer: letsencrypt-prod
nginx.ingress.kubernetes.io/backend-protocol: "FCGI"
nginx.ingress.kubernetes.io/fastcgi-index: "index.php"
nginx.ingress.kubernetes.io/fastcgi-params-configmap: "nextcloud-fastcgi-cm"
nginx.ingress.kubernetes.io/proxy-body-size: 10G
nginx.ingress.kubernetes.io/cors-allow-headers: "X-Forwarded-For"
nginx.ingress.kubernetes.io/server-snippet: |-
location ~ \.php(?:$|/) {
fastcgi_split_path_info ^(.+?\.php)(/.*)$;
set $path_info $fastcgi_path_info;
}
proxy_hide_header X-Powered-By;
rewrite ^/.well-known/webfinger /index.php/.well-known/webfinger last;
rewrite ^/.well-known/nodeinfo /index.php/.well-known/nodeinfo last;
rewrite ^/.well-known/host-meta /public.php?service=host-meta last;
rewrite ^/.well-known/host-meta.json /public.php?service=host-meta-json;
location = /.well-known/carddav {
return 301 $scheme://$host/remote.php/dav/;
}
location = /.well-known/caldav {
return 301 $scheme://$host/remote.php/dav/;
}
tls:
- secretName: nextcloud-tls
hosts:
- "cloud.example.com" click me for the full
|
To make this work, you need to define all of the "location" elements from the nginx config provided by Nextcloud in the ingress definition https://github.com/nextcloud/helm/blob/main/charts/nextcloud/templates/nginx-config.yaml In an ingress the equivalent for each of those statements is a path, and currently in this chart those are not overridable in a way that allows the addition of additonal paths: https://github.com/nextcloud/helm/blob/main/charts/nextcloud/templates/ingress.yaml In addition to this, this block https://github.com/nextcloud/helm/blob/main/charts/nextcloud/templates/nginx-config.yaml#L118 and the fonts one below it a problematic. FPM is a php execution server and not a file server, much like it's pythonic "WSGI" server counterparts - they are not for serving static files like images or fonts. This is always handled by the web proxy, whether it's PHP in Apache, or FPM behind an HTTP proxy. With the included nginx, the static files in the html directory are available to it because the volume mounts containing the static files are mounted in a location such that the path can be tried first by NGINX before forwarding the request to FPM I've managed Kube a while but honestly have never looked into mounts being presented to an ingress controller for such a purpose - My search based on the ideas in this issue lead me to the official response, which is what I expected: nginxinc/kubernetes-ingress#323 (comment)
And from the original comment in 2018:
Honestly, this feels like the right response. Allowing an ingress controller to mount volumes would require it to have access to mount volume resources in all namespaces, versus today where it just has rights to mount TLS secrets and route to services in those namespaces. This issue is a dead end in my eyes and that's a good thing. If you are interested in not having NGINX behind NGINX, you could look at alternative ingress controllers that are not built on webservers and are purely proxies - for example the haproxy ingress controller, or better yet - Cilium. ======== As an additional note, if Nextcloud (or any service you have in PHP-FPM) does not need to serve static files, then you can certainly just use ingress-nginx to proxy it. If you are adamant about wanting to reduce service-specific web proxies for static files in your cluster, then going with a non-http ingress controller is a first step, then you can deploy your own proxy-pod and configure it to mount all required volumes containing static files and understand all services it needs to proxy. Effectively, build a proxy tier for all your applications that need static file serving and have any ingress for apps that have static files route to it instead of the application. I feel like maybe that type of web proxy service may have a solution in K8S land that is not part of an Ingress service, but I'm not aware of one. I tend to prefer idempotent separation of concerns and would not want to have to roll proxies that front many services in order to mount another volume. You're also going to have to use a RWM volume type for that which presents other challenges. |
@Routhinator thanks a lot for your insights! I agree with you, the ingress controller should not mount the volumes and have that level of acces. In my eyes we can close this issue then |
Agree with @Routhinator and @provokateurin so closing this :) If someone has any further insights to provide, feel free to open a GitHub Discussion or continue commenting on this issue. If things change in the future with regards to the new gateway api or something, we can talk about reopening this. |
Looking to potentially switch to the FPM image. If I understood things correctly, using the fpm image, you would have a request/response flow like this:
To my understanding, this sort of defeats the purpose of using FastCGI to reduce latencies/bandwidth in the first place?
Is there any reason this cannot be simplified to:
FastCGI support in
ingress-nginx
: https://kubernetes.github.io/ingress-nginx/user-guide/fcgi-services/Am I missing something?
The text was updated successfully, but these errors were encountered: