Some custom response return codes in AuthPolicy do not work
When I set custom response return codes in AuthPolicy, I'm not sure why, but it can't send the response back now. Curl shows Unsupported HTTP/1 subversion in response as there is a wrong HTTP protocol version used. httpcore.RemoteProtocolError: illegal status line: bytearray(b'HTTP/1.1 0 Unknown') this is from the python library.
Seems like the issue remains only when specifying code that doesn't have any purpose by the standard (e.g. code 431 works, but 430 does not)
AuthPolicy:
spec:
rules:
authentication:
default:
credentials:
authorizationHeader:
prefix: Bearer
jwt:
issuerUrl: 'http://1.2.3.4:8080/realms/realm-averevki--hbxe'
ttl: 0
metrics: false
priority: 0
authorization:
Whitelist:
metrics: false
patternMatching:
patterns:
- operator: eq
selector: context.request.http.path
value: /allow
priority: 0
response:
unauthenticated:
code: 333
unauthorized:
code: 444
targetRef:
group: gateway.networking.k8s.io
kind: HTTPRoute
name: route-averevki--lhhk
Gateway
spec:
gatewayClassName: istio
listeners:
- allowedRoutes:
namespaces:
from: Same
hostname: '*.apps.kua.redhat.com'
name: api
port: 80
protocol: HTTP
HTTPRoute
spec:
hostnames:
- hostname-averevki--zwt-kuadrant.apps.kua.redhat.com
parentRefs:
- group: gateway.networking.k8s.io
kind: Gateway
name: gw-averevki--st7
rules:
- backendRefs:
- group: ''
kind: Service
name: httpbin-averevki--v8n
namespace: kuadrant
port: 8080
weight: 1
matches:
- path:
type: PathPrefix
value: /
What is your expected behavior? Should the user be allowed to send custom status codes that out of spec as a response? Or is the issue we allow a user to define status codes that are out of spec?
What is your expected behavior? Should the user be allowed to send custom status codes that out of spec as a response? Or is the issue we allow a user to define status codes that are out of spec?
I expect it to return 333 and 444 instead of the error. I'm not sure what range should be there. I'd limit the values you can put there so it won't provoke unexpected behavior if you leave it as it is implemented now. But I like the option to define any code for the response better (from 100 to 600)
I know it worked before, and now it is an unexpected change. So I guess it became a bug
I was re-reading the spec and 2 things come to mind:
- Envoy specify a closed set of supported HTTP codes and behaviour is not clear for codes described as "Unasigned". Authorino narrows the list even further by accepting only code within the 300-599 range.
- There's now another layer in between Authorino and the setting of the response code, the wasm-shim. Somebody should look at that end too.
- There's now another layer in between Authorino and the setting of the response code, the wasm-shim. Somebody should look at that end too.
I'll take a look at the wasm-shim side
From what I see, Kuadrant is working as expected. You told it to return 333 and it is returning, indeed, 333. It is your clients, either curl or python clients that complain not understanding that non standard return code.
So the right question here is: Should kuadrant allow configuration that specifies non standard return codes?
From the wasm-shim side - when a request is denied we try to create the response using the status/headers/body from authorino. If authorino is configured to use status codes that are not from the valid list from envoy we end up getting back a StatusCode::Empty = 0 which we try to send over causing the error that appears in the response.
I'd say we either restrict the set of status codes to those valid ones.. or alternatively we could "silently" override the code to a different one if we receive an empty status code, but I prefer the first option.
@adam-cattermole I want to confirm what your saying in your comment. This issue is related to changes that was done in the wasm-shim?, meaning it is not an issue directly with the kuadrant operator?
@adam-cattermole I want to confirm what your saying in your comment. This issue is related to changes that was done in the wasm-shim?, meaning it is not an issue directly with the kuadrant operator?
The issue is not so much a change that needs to be made in the wasm-shim, more on making a decision as to what valid status codes are. Envoy will not return us a status code in the wasm filter outside of those in the supported list, even if this is supported by Authorino. This means if a user configures to respond with one that is not on that list and authorino returns this in a response, we will not know what it is (as we get back StatusCode::Empty = 0 instead).
Ok, Thanks @adam-cattermole, so if the user was to use a different Gateway and not Envoy, the wasm filter would return the supported status code for the different Gateway. If this is the case I think a document change to say the supported status codes is dependent on the Gateway provider that is used. Would you agree?
if the user was to use a different Gateway and not Envoy, the wasm filter would return the supported status code for the different Gateway
Sorry for confusion, when I mention envoy here I mean the proxy (that our wasm filter interacts with) - this is unrelated to the gateway provider itself, and the issue is present on all of our current supported installation methods
What is the proxy in this case? What I am trying to work out is can we fix this issue within our repos? or is it an external change?
Yep it is handled by us, referring to my comment above we have to decide whether:
- We document that we only support the same set of response codes that we can retrieve from the proxy
- or we actually restrict these via the CRD to prevent non-valid codes (and I guess document as well)
- (or we silently ignore invalid response codes and provide the default valid one)
This took me to long to get my head around. It is the envoy proxy crate that is used with in the wasm shim that is causing the limitation. I would be in favor of limiting the values a user can have within the CRDs, linking to the envoy proxy crate maybe enough for documentation.
I would like to generate the list of values from the crate. But because the kube builder and how it generates the CRDs I think this could be problematic.
or we actually restrict these via the CRD to prevent non-valid codes (and I guess document as well)
This + documentation 👍