Skip to content
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

TestIngressRecoverFromInvalidPath is failing when changing router flavor to expressions #5127

Open
1 task done
pmalek opened this issue Nov 8, 2023 · 3 comments
Open
1 task done
Labels
bug Something isn't working

Comments

@pmalek
Copy link
Member

pmalek commented Nov 8, 2023

Is there an existing issue for this?

  • 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

Path: `/~^^/*$`, // invalid regex
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 pmalek added the bug Something isn't working label Nov 8, 2023
@randmonkey
Copy link
Contributor

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

@pmalek
Copy link
Member Author

pmalek commented Nov 14, 2023

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 pmalek changed the title TestIngressRecoverFromInvalidPath is failing when changing router flavor to `expressions TestIngressRecoverFromInvalidPath is failing when changing router flavor to expressions Nov 16, 2023
@randmonkey
Copy link
Contributor

randmonkey commented Nov 17, 2023

Sounds good, I will add this case (while we should be able to validate this after #5190).

@randmonkey randmonkey assigned randmonkey and unassigned randmonkey Nov 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants