Support Kong 2.2 request/response route buffering

[rainest]

Kong 2.2 introduces new boolean route configuration options, request_buffering and response_buffering. Both default to true.

KIC will need to support these via new annotations and KongIngress route configuration options.

[mflendrich]

Kong 2.2 does not introduce any breaking change here - these options make it possible to tell Kong to flush the buffers immediately so that Kong does not introduce latency.

[mflendrich]

Discussion to be had as to whether we want to implement this both as an annotation and in the KongIngress resource, or only in one of those places.

[rainest]

More detailed knowledge share on what this will look like in practice. These are both boolean flags, so the code for them will be similar to the boolean annotation/override on strip-path: https://docs.konghq.com/kubernetes-ingress-controller/1.1.x/references/annotations/#konghqcomstrip-path

Annotations and KongIngress resources provide two means to set Kong settings that don't fit well into the standard K8S resources. We originally had KongIngress only, but found that having to maintain a separate resource that could apply to Ingresses and Services both with links that were sometimes automatic (based on KongIngress and the other resource having the same name) and sometimes manual (via an annotation) wasn't great UX, and started moving as many settings as possible into annotations.

IMO the annotation code is cleaner and the overall UX is better, so if we could move to creating those only, I'm all for it. However, I'd asked @hbagdi if deprecating KongIngress entirely was an option previously and was told it wasn't, so while I think that's something we should consider revisiting, for now I still add both methods of setting these features when adding a new one.

In the implementation, this happens via an override system in the parser. IIRC these are the parts needed to add a new override:

1. Define an annotation key.
2. Define a function to extract that key
3. Add an override by annotation function that uses the extract key function and call it from the annotation overrider.
4. Add a new key to the KongIngress CRD. Note that this is the base manifest that provides the source of truth, and you'll need to generate the other manifests using hack/build-single-manifests.sh. CI will alert you if you forget to do this, but the existing system has a known limitation: kustomize sometimes changes its output order in ways that don't make any functional difference in the resulting manifest, but will trip up the diff checker. At present, I still work around this by bumping the kustomize version used by CI to match the version I have locally, but we'll need to find a better solution for that problem eventually.
5. Add logic to the KongIngress overrider to extract the new KongIngress key.

I've used strip_path here as it's a boolean, as are these new parameters. #863 is a more recent PR that shows another example override addition, but it's a bit more complex, as it also has validation and sanitization, similar to the methods annotation. The above steps also don't go over the Go-based tests; please see the SNI and methods PRs for examples of those. At a high level, our unit tests

1. Check that the annotation extracter function can extract those values from a manifest in the annotations test.
2. Create a fake Ingress or Service resource with annotations and a KongIngress resource with the new field, feed those through the parser, and confirm that the go-kong structs that come out have the expected Kong setting.

We're not perfectly consistent with those, and they have changed a bit due to the recent parser restructuring. Although the SNI PR is still in progress, it uses the modern code, whereas the methods PR predates the parser restructure.

[erikvanbrakel]

Does the onboarding label mean that this is probably going to go into the next release? And if so, any clue on when that will be?

If it helps I could take a stab at implementing this, let me know :-)

[mflendrich]

@erikvanbrakel

Does the onboarding label mean that this is probably going to go into the next release?

The onboarding label has a similar meaning to good first issue in that it's a good learning item to get more familiar with the codebase :). It has nothing to do with release planning. But, coincidentally, this issue is work in progress by us already and is likely to be included in the next minor release.

If it helps I could take a stab at implementing this, let me know :-)

Like I said above, this is already being worked on. We'd very happily welcome your contributions on other items in our issue tracker. 🙂

[shaneutt]

thanks for the awesome writeup @rainest 😃

[shaneutt]

resolved by:

• request response route buffering go-kong#15
• request response buffering options #1016