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

Add HeaderKind to enable setting request headers #165

Merged
merged 3 commits into from
Feb 6, 2025

Conversation

adam-cattermole
Copy link
Member

@adam-cattermole adam-cattermole commented Feb 5, 2025

In the wasm-shim refactor the request vs response headers that was fixed prior (#143) was missed. I've introduced an enum called HeaderKind which explicitly differentiates the Request vs Response types for the specific actions, and have enforced that these can only be added at the correct phase.

I've updated the authconfig to add request headers, so this is verifiable as follows:

make local-setup
kubectl port-forward --namespace kuadrant-system deployment/envoy 8000:8000
curl -H "Host: test.a.auth.com" -H "Authorization: APIKEY IAMALICE" http://127.0.0.1:8000/get -i

See My-Header in the echoed request:

HTTP/1.1 200 OK
content-type: application/json
server: envoy
date: Wed, 05 Feb 2025 16:10:52 GMT
content-length: 545
x-envoy-upstream-service-time: 2

{
  "method": "GET",
  "path": "/get",
  "query_string": null,
  "body": "",
  "headers": {
    "Host": "test.a.auth.com",
    "User-Agent": "curl/8.7.1",
    "Accept": "*/*",
    "Authorization": "APIKEY IAMALICE",
    "X-Forwarded-For": "10.244.0.10",
    "X-Forwarded-Proto": "http",
    "X-Envoy-Internal": "true",
    "X-Request-Id": "9641a263-bfdb-49b7-b1c2-a7662ec1e74e",
    "My-Header": "{\"my-key\":true}",
    "X-Envoy-Expected-Rq-Timeout-Ms": "15000",
    "Version": "HTTP/1.1"
  },
  "uuid": "fbbaad8b-2d15-42b1-9c3d-c21721d37b59"
}

Resolves #164

Signed-off-by: Adam Cattermole <[email protected]>
@adam-cattermole adam-cattermole added the bug Something isn't working label Feb 5, 2025
@adam-cattermole adam-cattermole self-assigned this Feb 5, 2025
@adam-cattermole adam-cattermole requested a review from a team as a code owner February 5, 2025 16:04
Comment on lines +19 to +20
response_headers_to_add: Option<Headers>,
request_headers_to_add: Option<Headers>,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These have been renamed and are Option that is initialised once and extended until it is taken. Crucially if this is None it can no longer be initialised, we have added the headers and we are no longer in the correct phase

@adam-cattermole adam-cattermole changed the title Add HeaderKind to add request headers Add HeaderKind to enable settting request headers Feb 5, 2025
@adam-cattermole adam-cattermole changed the title Add HeaderKind to enable settting request headers Add HeaderKind to enable setting request headers Feb 5, 2025
Copy link
Member

@didierofrivia didierofrivia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🍸

@adam-cattermole adam-cattermole added this pull request to the merge queue Feb 6, 2025
Merged via the queue into main with commit 6b3fb69 Feb 6, 2025
13 checks passed
@adam-cattermole adam-cattermole deleted the fix-request-headers branch February 6, 2025 11:54
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
Status: Done
Development

Successfully merging this pull request may close these issues.

Success response headers are not added to the response
2 participants