search

Kong / charts

Helm chart for Kong
Apache License 2.0
239 stars 473 forks source link

fix(proxy): Add appProtocol as optional field to kong service TLS port #1018

Closed gyenwahangelCA closed 4 months ago

gyenwahangelCA commented 4 months ago

Raised case no. 00042199

When configuring a GCP internal application load balancer for Kong via the networking.gke.io/v1 API, the appProtocol field for the kong service needs to set for GCP to correctly set the backend protocol to HTTPS. I have verified this by editing the chart and can confirm that this fixes the issue. Unfortunately, appProtocol was removed from the chart on 19/01/23 to fix AKS loadbalancer: https://github.com/Kong/charts/pull/705/files.

This change would allow the appProtocol field to be set optionally, so that both cases would work.

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

CLAassistant commented 4 months ago

CLA assistant check
All committers have signed the CLA.

czeslavo commented 4 months ago

@gyenwahangelCA, thank you for proposing the change! It makes sense to me to make it optional and satisfy both described use cases. I've updated README docs with the new SVC.tls.appProtocol value description, added a golden test case, and prepared a release so it's published straight away after we merge this PR.

Can you please sign the CLA to unblock merging this?

gyenwahangelCA commented 4 months ago

Hi @czeslavo, I have now signed the CLA. Thank you for your quick response on this one.

gyenwahangelCA commented 4 months ago

I have added another commit to allow setting of appProtocol for the http port as well. Please update any necessary files to accommodate this. Thanks