kubernetes / ingress-nginx

Ingress NGINX Controller for Kubernetes
https://kubernetes.github.io/ingress-nginx/
Apache License 2.0
17.5k stars 8.26k forks source link

Fcgi validation error for values specified with if_not_empty. #11051

Open gogojlb opened 8 months ago

gogojlb commented 8 months ago

What happened: Fcgi parameter validation error:

E0301 16:59:25.011393       7 main.go:149] "fcgi annotation error" err="fcgi contains invalid key or value" configmap="test-fcgi" namespace="test-fcgi" key="HTTPS" value="$https if_not_empty"
E0301 16:59:25.011419       7 annotations.go:190] "ingress contains invalid annotation value" err="annotation fastcgi-params-configmap contains invalid value"
E0301 16:59:25.011428       7 store.go:938] annotation fastcgi-params-configmap contains invalid value

Validation regex for fcgi values does't pass variables specified with if_not_empty

What you expected to happen:

To be able to set fcgi variables specified with if_not_empty. Nginx doc reference

NGINX Ingress controller version (exec into the pod and run nginx-ingress-controller --version.):

-------------------------------------------------------------------------------
NGINX Ingress controller
  Release:       v1.10.0
  Build:         71f78d49f0a496c31d4c19f095469f3f23900f8a
  Repository:    https://github.com/kubernetes/ingress-nginx
  nginx version: nginx/1.25.3

-------------------------------------------------------------------------------

Kubernetes version (use kubectl version):

Client Version: version.Info{Major:"1", Minor:"27", GitVersion:"v1.27.1", GitCommit:"4c9411232e10168d7b050c49a1b59f6df9d7ea4b", GitTreeState:"clean", BuildDate:"2023-04-14T13:14:41Z", GoVersion:"go1.20.3", Compiler:"gc", Platform:"darwin/arm64"}
Kustomize Version: v5.0.1
Server Version: version.Info{Major:"1", Minor:"26+", GitVersion:"v1.26.12-eks-5e0fdde", GitCommit:"95c835ee1111774fe5e8b327187034d8136720a0", GitTreeState:"clean", BuildDate:"2024-01-02T20:34:50Z", GoVersion:"go1.20.12", Compiler:"gc", Platform:"linux/amd64"}

Environment:

How to reproduce this issue:

``` apiVersion: v1 kind: Namespace metadata: labels: name: test-fcgi name: test-fcgi --- apiVersion: apps/v1 kind: Deployment metadata: name: nginx-deployment namespace: test-fcgi spec: selector: matchLabels: app: nginx replicas: 1 template: metadata: labels: app: nginx spec: containers: - name: nginx image: docker.io/library/nginx:1.19-alpine ports: - containerPort: 80 --- apiVersion: v1 kind: Service metadata: name: test-fcgi namespace: test-fcgi spec: selector: app: nginx ports: - protocol: TCP port: 80 targetPort: 80 --- apiVersion: v1 data: HTTPS: $https if_not_empty # <-- validation fails here kind: ConfigMap metadata: name: test-fcgi namespace: test-fcgi --- apiVersion: networking.k8s.io/v1 kind: Ingress metadata: annotations: kubernetes.io/ingress.class: test-fcgi nginx.ingress.kubernetes.io/backend-protocol: FCGI nginx.ingress.kubernetes.io/fastcgi-params-configmap: test-fcgi name: test-fcgi namespace: test-fcgi spec: rules: - host: test-fcgi.test.local http: paths: - backend: service: name: test-fcgi port: number: 80 path: / pathType: Prefix ```
k8s-ci-robot commented 8 months ago

This issue is currently awaiting triage.

If Ingress contributors determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
longwuyuan commented 8 months ago

/remove-kind bug

@gogojlb its possible that your issue-description is too cryptic and succint

I am not a developer so I do not understand the use case of a HTTPS variable in the configMap but I was hoping to reproduce and actually understand the reproduced problem in the context of knowing ;

Reason being first we deprecated fpm support, after a survey that supported deprecation decision. Then someone said they use it & want it so we reverted and got confirmation that fcgi for fpm feature was working. And this issue is a report that something is wrong with that feature. Now there are not too many resources available to look into this so the simplification of the issue description actually goes a long way in triaging

/triage needs-information

longwuyuan commented 8 months ago

/assign

gogojlb commented 8 months ago

Hi @longwuyuan , thanks for your attention to this issue and sorry for the confusion. Let me clarify it a bit.

it is not the fcgi functionality itself that is at issue here, but just ingress/configmap objects validation. I'm assuming that ingress-nginx supports the same set of fcgi parameters as plain nginx. In that case, while parameter values specified with if_not_empty are valid from nginx/ngx_http_fastcgi_module perspective, they are not valid from nginx-ingress perspective due strict validation regex that doesn't allow a space.

err="fcgi contains invalid key or value" configmap="test-fcgi" namespace="test-fcgi" key="HTTPS" value="$https if_not_empty"

If my assumptions are correct, I can also hypothesize that the problem can be solved by modifying the regexp to e.g. ^[A-Za-z0-9\-\_\$\{\}/.]*( if_not_empty){0,1}$.

using a nginx:blah image which is a plain old nginx

It is just a dummy nginx deployment to provide target for the ingress/service.

longwuyuan commented 8 months ago

Ah thanks for the clarification. Helps a lot.

I am not a developer so cant say for sure but I think the fcgi parameters will have been implemented in go or lua and thus less likely that all the parameters would be honored in a confgMap (specially one with a space). I will check to what limit I can but please don't wait for me.

Hoping someone else will have more helpful comments.

github-actions[bot] commented 7 months ago

This is stale, but we won't close it automatically, just bare in mind the maintainers may be busy with other tasks and will reach your issue ASAP. If you have any question or request to prioritize this, please reach #ingress-nginx-dev on Kubernetes Slack.