kubernetes-ingress-controller icon indicating copy to clipboard operation
kubernetes-ingress-controller copied to clipboard

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

Open pmalek opened this issue 1 year ago • 3 comments

Is there an existing issue for this?

  • [X] I have searched the existing issues

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

pmalek avatar Nov 08 '23 16:11 pmalek

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:

  • Create some k8s resources that will generate invalid config for both router flavors
  • Skip this case when expression router enabled

randmonkey avatar Nov 09 '23 10:11 randmonkey

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.

pmalek avatar Nov 14 '23 13:11 pmalek

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).

randmonkey avatar Nov 17 '23 03:11 randmonkey