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

request response route buffering #1011

Closed
wants to merge 3 commits into from

Conversation

shaneutt
Copy link
Member

@shaneutt shaneutt commented Jan 22, 2021

What this PR does / why we need it:
The request_buffering and response_buffering options were added in Kong/kong#6057 and enable users to disable buffering for use cases which call for it (namely, large payloads using HTTP 1.1 chunked encoding).

This PR adds request_buffering and response_buffering options to KongIngress and Ingress annotations.

Which issue this PR fixes
fixes #924

Special notes for your reviewer:

Copy link
Contributor

@rainest rainest left a comment

Choose a reason for hiding this comment

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

internal/ingress/controller/parser/kongstate/route.go Outdated Show resolved Hide resolved
internal/ingress/controller/parser/kongstate/route.go Outdated Show resolved Hide resolved
@shaneutt shaneutt force-pushed the request_response_route_buffering branch from fd7781a to f5b000d Compare January 22, 2021 18:49
@codecov
Copy link

codecov bot commented Jan 22, 2021

Codecov Report

Merging #1011 (f5b000d) into main (daa9afa) will decrease coverage by 0.15%.
The diff coverage is 30.76%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1011      +/-   ##
==========================================
- Coverage   49.56%   49.41%   -0.16%     
==========================================
  Files          32       32              
  Lines        3198     3224      +26     
==========================================
+ Hits         1585     1593       +8     
- Misses       1483     1499      +16     
- Partials      130      132       +2     
Impacted Files Coverage Δ
internal/ingress/annotations/annotations.go 82.75% <0.00%> (-6.14%) ⬇️
...ernal/ingress/controller/parser/kongstate/route.go 83.61% <36.36%> (-6.71%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update daa9afa...f5b000d. Read the comment docs.

@shaneutt
Copy link
Member Author

My overview omitted details on the tests usually added for these. This should have them also, for annotation extraction and overriding. Examples of those can be found at:

https://github.com/Kong/kubernetes-ingress-controller/blob/1.1.0/internal/ingress/annotations/annotations_test.go#L310-L340 (for extraction)

https://github.com/Kong/kubernetes-ingress-controller/blob/1.1.0/internal/ingress/controller/parser/kongstate/route_test.go#L408-L494 (for annotation overrides) and https://github.com/Kong/kubernetes-ingress-controller/blob/1.1.0/internal/ingress/controller/parser/kongstate/route_test.go#L35-L52 (for KongIngress overrides).

Sorry, I'm still work in progress so I was reviewing my own work first, and as per the description I'm still adding tests so those will come soon I had not forgotten them 👍

@shaneutt
Copy link
Member Author

I'm going to close this for now because I realize that I have reviewers requested, but this draft is still a work in progress and I don't need review just yet so I don't want anyone to spend time unnecessarily.

@shaneutt shaneutt closed this Jan 22, 2021
@shaneutt shaneutt deleted the request_response_route_buffering branch January 22, 2021 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/feature New feature or request priority/high work in progress Work In Progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support Kong 2.2 request/response route buffering
2 participants