Kong / kubernetes-ingress-controller

:gorilla: Kong for Kubernetes: The official Ingress Controller for Kubernetes.
https://docs.konghq.com/kubernetes-ingress-controller/
Apache License 2.0
2.2k stars 590 forks source link

`TestIngressRecoverFromInvalidPath` is failing when changing router flavor to `expressions` #5127

Open pmalek opened 11 months ago

pmalek commented 11 months ago

Is there an existing issue for this?

Current Behavior

After fixing the router flavor setting in integration tests #5112 (which we didn't set properly when we originally changed the setting in #4934) TestIngressRecoverFromInvalidPath is failing by accepting the supposedly invalid regex https://github.com/Kong/kubernetes-ingress-controller/blob/513db87cbf94ce66207f74365b502e8cde841357/test/integration/ingress_test.go#L868 which shouldn't be accepted.

Expected Behavior

No response

Steps To Reproduce

No response

Kong Ingress Controller version

No response

Kubernetes version

No response

Anything else?

No response

randmonkey commented 11 months ago

This ingress will generate a valid expression when expression router enabled:

((http.path == "/bar") || (http.path ^= "/bar/")) || (http.path ~ "^^/*$")

expression router will NOT validate regexes on the RHS of predicates and reject invalid regexes. Instead, NO strings could satisfy the predicate if regex is invalid. So the method to test recovery from invalid configurations could be:

pmalek commented 10 months ago

I was trying to come up with some examples of configurations which would fail to get applied and I came up with a plugin that references a non existing Secret

apiVersion: v1
kind: Service
metadata:
  labels:
    app: httpbin
  name: httpbin-deployment
spec:
  ports:
  - port: 80
    protocol: TCP
    targetPort: 80
  selector:
    app: httpbin
  type: ClusterIP
---
apiVersion: configuration.konghq.com/v1
kind: KongPlugin
metadata:
  name: key-auth-1
plugin: key-auth
configFrom:
  secretKeyRef:
    name: secret1
    key: key1
---
apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  name: httpbin-ingress
  annotations:
    konghq.com/strip-path: "true"
    konghq.com/plugins: key-auth-1
spec:
  ingressClassName: kong
  rules:
  - http:
      paths:
      - path: /
        pathType: Prefix
        backend:
          service:
            name: httpbin-deployment
            port:
              number: 80

Would that make sense? It's not blocked by CEL expressions or the admission webhook so this should fit this use case. We can then check in the test that the plugin wasn't applied.

OTOH this does allow the configuration to be applied, without only the broken plugin.

randmonkey commented 10 months ago

Sounds good, I will add this case (while we should be able to validate this after https://github.com/Kong/kubernetes-ingress-controller/issues/5190).