Skip to content

feat: add httproute from gateway-api to create chart template#12912

Merged
gjenkins8 merged 7 commits intohelm:mainfrom
hegerdes:feat/chart-httproute-template
Mar 11, 2025
Merged

feat: add httproute from gateway-api to create chart template#12912
gjenkins8 merged 7 commits intohelm:mainfrom
hegerdes:feat/chart-httproute-template

Conversation

@hegerdes
Copy link
Copy Markdown
Contributor

@hegerdes hegerdes commented Mar 26, 2024

This closes #12603

What this PR does / why we need it:

This PR adds the HTTPRoute from gateway-api to the example getting started chart.
It is disabled by default, just like ingress.
Adding the resource will make helm charts future-proof for the gateway-api and may also increase its adoption rate.

Special notes for your reviewer:

If enabled, the via httpRoute.enabled: true, it will produce a valid HTTPRoute but it will not route real traffic since a Gateway must be installed and configured which is in the responsibility of the cluster admin, just like ingress-controller.

The gateway api is still quite young and the gateway-api CRDs are not yet part of a default k8s installation. They currently have to be installed separably with kubectl apply -f https://github.com/kubernetes-sigs/gateway-api/releases/download/v1.0.0/standard-install.yaml.
The template checks if the v1 or the v1alpha2 version is installed and sets the api version accordingly.
If the CRDs are missing the installation fails.

If applicable:

  • this PR contains documentation
  • this PR contains unit tests
  • this PR has been tested for backwards compatibility

@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Mar 26, 2024
@hegerdes hegerdes force-pushed the feat/chart-httproute-template branch from 57dc6e0 to 4ee174f Compare March 26, 2024 18:33
@hegerdes hegerdes mentioned this pull request Mar 26, 2024
Copy link
Copy Markdown
Member

@gjenkins8 gjenkins8 left a comment

Choose a reason for hiding this comment

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

nice, thanks for the PR.

A suggestion: perhaps add an example on how to access the installed deployment into NOTES.txt (https://github.com/helm/helm/blob/main/pkg/chartutil/create.go#L438) ?

(I'm not familiar enough with gateway API to know if this is entirely reasonable; but if so, might be helpful to chart consumers/authors?)

@pull-request-size pull-request-size bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Mar 28, 2024
@hegerdes
Copy link
Copy Markdown
Contributor Author

nice, thanks for the PR.

A suggestion: perhaps add an example on how to access the installed deployment into NOTES.txt (https://github.com/helm/helm/blob/main/pkg/chartutil/create.go#L438) ?

(I'm not familiar enough with gateway API to know if this is entirely reasonable; but if so, might be helpful to chart consumers/authors?)

Thanks, and great suggestion. I added some infos to the NOTES.txt, but since configuration in the gateway-api are set in Gateway and some in HTTPRoute it is not super easy to give a perfect example for accessing the app since the chart deployment cant look into the Gateway configuration. But I added the infos for accessing it via the most common way (Host and Path based routing) as well on how to get the relevant information from the Gateway

With HTTPRoute enabled and hosts set in the values the output would look like this:

1. Get the application URL by running these commands:
    export APP_HOSTNAME=example.com
    echo "Visit http://$APP_HOSTNAME/headers to use your application"

    NOTE: Your HTTPRoute depends on the listener configuration of your gateway and your HTTPRoute rules.
    These can be set for path, method, header and query parameters.
    You can check the gateway configuration with 'kubectl get --namespace default gateway/gateway -o yaml'

With HTTPRoute enabled and no hosts set in the values the output would look like this:

1. Get the application URL by running these commands:
    export APP_HOSTNAME=$(kubectl get --namespace default gateway/gateway -o jsonpath="{.spec.listeners[0].hostname}")
    echo "Visit http://$APP_HOSTNAME/headers to use your application"

    NOTE: Your HTTPRoute depends on the listener configuration of your gateway and your HTTPRoute rules.
    These can be set for path, method, header and query parameters.
    You can check the gateway configuration with 'kubectl get --namespace default gateway/gateway -o yaml'

Namespaces and Paths are all tempalted form the values/release namespace

@kfox1111
Copy link
Copy Markdown

Looking good. Thanks for working on this :)

@gjenkins8 gjenkins8 added this to the 3.15.0 milestone Apr 18, 2024
Copy link
Copy Markdown
Contributor

@sabre1041 sabre1041 left a comment

Choose a reason for hiding this comment

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

Reviewed the content of the PR and here are some comments:

  1. Let's align the default hostname among Ingress and HTTPRoute so that there is pariety. Yes, you could potentially install both, but in most cases, you would pick one or the other. So set the default hostname of the HTTPRoute to be chart-example.local
  2. Support for v1beta1. While the Gateway API is now GA, most who are currently using it is making use of the beta version. Similar to Ingress, there should be a version check to determine which API Group/Version to apply
  3. Overall thoughts: I am hesitant about adding this to the default scaffolded chart. This is mainly from a user experience perspective as we would want the getting started experience to be as smooth as possible. Most users will not have the Gateway API installed and there is a chance that they would face errors when attempting to leverage some of the features that are available in the default scaffolded chart.

@hegerdes
Copy link
Copy Markdown
Contributor Author

Thanks for the feedback @sabre1041

  1. Let's align the default hostname among Ingress and HTTPRoute so that there is pariety. Yes, you could potentially install both, but in most cases, you would pick one or the other. So set the default hostname of the HTTPRoute to be chart-example.local

Sure will do!

  1. Support for v1beta1. While the Gateway API is now GA, most who are currently using it is making use of the beta version. Similar to Ingress, there should be a version check to determine which API Group/Version to apply

I thought so too at first but then the question was raised if it is used that much by people, so I removed it. I can revert that without a problem but you also mentioned user experience. I will make the assumption that the people using v1beta1 are technical enough and on the cutting edge side so they will know to change the api version to v1beta1. And if not and they use v1beta1 functions the deployment will just fail. Let the community/maintainers discuss and decide. I have no real preference here.

  1. Overall thoughts: I am hesitant about adding this to the default scaffolded chart. This is mainly from a user experience perspective as we would want the getting started experience to be as smooth as possible. Most users will not have the Gateway API installed and there is a chance that they would face errors when attempting to leverage some of the features that are available in the default scaffolded chart.

I understand that concern. My goal was to make using the GatewayAPI easy because at fist I struggled transforming something usable into helm. To not disturb the user experience the gateway stuff is disabled by default, you have to enable it explicitly and then it should provide a good starting point for your first routes. I see it similar to Ingress and HorizontalPodAutoscaler. A lot of people don't use these and just delete the template but if you need them it is nice to have a good starting point.

@sabre1041
Copy link
Copy Markdown
Contributor

Thanks for the feedback @sabre1041

  1. Let's align the default hostname among Ingress and HTTPRoute so that there is pariety. Yes, you could potentially install both, but in most cases, you would pick one or the other. So set the default hostname of the HTTPRoute to be chart-example.local

Sure will do!

  1. Support for v1beta1. While the Gateway API is now GA, most who are currently using it is making use of the beta version. Similar to Ingress, there should be a version check to determine which API Group/Version to apply

I thought so too at first but then the question was raised if it is used that much by people, so I removed it. I can revert that without a problem but you also mentioned user experience. I will make the assumption that the people using v1beta1 are technical enough and on the cutting edge side so they will know to change the api version to v1beta1. And if not and they use v1beta1 functions the deployment will just fail. Let the community/maintainers discuss and decide. I have no real preference here.

  1. Overall thoughts: I am hesitant about adding this to the default scaffolded chart. This is mainly from a user experience perspective as we would want the getting started experience to be as smooth as possible. Most users will not have the Gateway API installed and there is a chance that they would face errors when attempting to leverage some of the features that are available in the default scaffolded chart.

I understand that concern. My goal was to make using the GatewayAPI easy because at fist I struggled transforming something usable into helm. To not disturb the user experience the gateway stuff is disabled by default, you have to enable it explicitly and then it should provide a good starting point for your first routes. I see it similar to Ingress and HorizontalPodAutoscaler. A lot of people don't use these and just delete the template but if you need them it is nice to have a good starting point.

The biggest concern with including an HTTPRoute with the default set of scaffolded resources during Helm chart creation is that unlike Ingress or HorizontalPodAutoscaler, an HTTPRoute is not an included API within the cluster. So, if they attempt to enable the feature as part of the installation of a chart to the cluster, an error will be returned, not as a result of an error within the contents of the chart, but due to component(s) not being available within the cluster. Its all about the user experience.

@kfox1111
Copy link
Copy Markdown

The Kubernetes folks intend for the gateway api to replace the ingress api though. There is a chicken and the egg problem though. Enough software needs to adopt the ability to use the gateway api before enough clusters deploy it. Clusters are reluctant to deploy it unless software uses it. Software developers are less likely to support it if they don't have a good example of how to do it. Hence this pr's goal to help break the deadlock.

@hegerdes hegerdes force-pushed the feat/chart-httproute-template branch from 416d8d6 to d549cf6 Compare June 8, 2024 17:27
@hegerdes
Copy link
Copy Markdown
Contributor Author

hegerdes commented Jun 8, 2024

Changed the default HTTPRoute hostname in values cording to chart-example.local as suggested by @sabre1041

Rebased from main

@sabre1041
Copy link
Copy Markdown
Contributor

Changed the default HTTPRoute hostname in values cording to chart-example.local as suggested by @sabre1041

Rebased from main

Any thoughts about supporting older versions of the Gateway API pre GA?

@mattfarina mattfarina modified the milestones: 3.15.0, 3.16.0 Jun 12, 2024
@hegerdes
Copy link
Copy Markdown
Contributor Author

Seems like there a no real opinions on that. Also asked in the k8s gateway-api slack channel.

Given that v1.1 is already released I don`t see the real need to support beta versions as I dont think they will be needed once it hits a higher adoption rate.

@gjenkins8
Copy link
Copy Markdown
Member

There was a concern raised (on the Helm weekly developer call), that as the gateway API is not part of the core/standard API. Rather, that is an extension installed via optional CRDs. Helm shouldn't include a "non-standard" (non-core) API as part of a standard chart definition.

(I didn't realize when first seeing this PR, that the API would remain non-core for the foreseeable future, iiuc)

Personally, the Gateway API seems to much closer to being a part of core Kubernetes, than many other CRDs e.g. cert-manager's. But strictly, while it is optionally installed via CRDs. It is never going to be part of a "default" Kubernetes API set.

I specifically asked what Helm maintainers see the function of the helm create scaffolding. And the consensus was the scaffolded chart should be a best practices for a vanilla Kubernetes setup. So in that sense. I agree the Gateway API doesn't meet this standard.

@youngnick
Copy link
Copy Markdown

So, a few comments from the Gateway API maintainer POV (that's me!):

  • Gateway API is a set of extension CRDs that are an official part of Kubernetes. They are subject to the same API review and promotion controls that included APIs are in the core Kubernetes binaries. To be honest, Gateway API is the first of many here - this pattern is already adopted by the AdminNetworkPolicy folks, and more will follow. It makes sense for Helm to have a way to handle this "definitely a Kubernetes API but not included by default" issue. (There's a larger issue about ways to ensure that the required version of Gateway API is installed as well, but that's a way larger discussion). As @kfox1111 says, Gateway API and these other APIs are most likely never going to be installed by default though.
  • For Helm charts, I think the way to go is to end up with three settings - hostname, gatewayName, gatewayNamespace, which can be then populated into a HTTPRoute resource and link it up with already-existing infrastructure. Setting one of these should require all three, and the HTTPRoute files should only be included if the values are not empty. (Presumably, this goes for helm create as well).

I'm happy to come to a Helm community call or something at some point to discuss this in a higher-bandwidth way, or to meet up at Kubecon NA and talk there too.

There are two Helm-related problems that have been on my mind for some time:

  • How to help chart authors include HTTPRoutes more easily (that's this one)
  • How to help chart authors say "Gateway API being installed is a dependency" and "Gateway API must be at least this version", both of which are surprisingly hard problems that will take a while to work out. But having more communication between Gateway API folks and Helm folks here will help a lot, I think.

@scottrigby scottrigby modified the milestones: 3.16.0, 3.17.0 Sep 11, 2024
@gjenkins8
Copy link
Copy Markdown
Member

Apologies for the belated reply. I agree with the above. And I would like to work out how to move forward.

Overall, I'm not exactly sure what the answers are. I think the question is less "how does Helm handle HTTPRoute and gateway API?", and more: "How does Helm (and other tooling) handle 'out-of-tree' types?"

I do think worthy to try and sync at KubeCon. I will be there, as well as other Helm maintainers. And if possible, jumping on the Helm developer call (9:30am Pacific, Thursdays; see #helm-dev on Kubernetes slack for zoom link and password) to try and prompt some attention might help too.

gjenkins8
gjenkins8 previously approved these changes Nov 15, 2024
Copy link
Copy Markdown
Member

@gjenkins8 gjenkins8 left a comment

Choose a reason for hiding this comment

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

I approve, but other maintainers may still have differing opinions

@gjenkins8 gjenkins8 added the Needs v3 backport Label PRs for v4/main, which are still applicable to v3 so need a separate backport PR label Nov 15, 2024
@hegerdes
Copy link
Copy Markdown
Contributor Author

Thanks for approving and your feedback @gjenkins8. I will update the comment tomorrow and will also rebase.

Just out of interest: Did the topic of any none core kubernetes-apis come up at kubecon? I was not able to attend, but am quite interested if there where opinions exchanged. Regarding the role of helm and gateway-api

@kfox1111
Copy link
Copy Markdown

kfox1111 commented Feb 6, 2025

Whats left to get this in?

Copy link
Copy Markdown
Contributor

@sabre1041 sabre1041 left a comment

Choose a reason for hiding this comment

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

Logic wide, it looks good. Just a couple cosmetic changes since they are applied by default, to all newly created charts

@hegerdes
Copy link
Copy Markdown
Contributor Author

Implemented the requested changes to values.yaml template

sabre1041
sabre1041 previously approved these changes Feb 11, 2025
Copy link
Copy Markdown
Contributor

@sabre1041 sabre1041 left a comment

Choose a reason for hiding this comment

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

LGTM

@gjenkins8
Copy link
Copy Markdown
Member

@hegerdes -- could you fix the conflicts please. I didn't see Andy's prior approval (otherwise I would have merged). Once conflicts are fixed, we should be able to do a quick round of re-approvals and get this merged.

hegerdes and others added 7 commits March 11, 2025 16:47
Adds the HTTPRoute from https://gateway-api.sigs.k8s.io/reference/spec/ to the example getting started chart.

This closes helm#12603

Signed-off-by: Henrik Gerdes <hegerdes@outlook.de>
Signed-off-by: Henrik Gerdes <hegerdes@outlook.de>
Signed-off-by: Henrik Gerdes <hegerdes@outlook.de>
Signed-off-by: Henrik Gerdes <hegerdes@outlook.de>
Signed-off-by: Henrik Gerdes <hegerdes@outlook.de>
Co-authored-by: George Jenkins <gvjenkins@gmail.com>
Signed-off-by: Henrik Gerdes <hegerdes@outlook.de>
Signed-off-by: Henrik Gerdes <hegerdes@outlook.de>
@hegerdes
Copy link
Copy Markdown
Contributor Author

@gjenkins8 No problem!

No code changes required, git handled the modified repo structure just fine in a simple rebase

Copy link
Copy Markdown
Contributor

@sabre1041 sabre1041 left a comment

Choose a reason for hiding this comment

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

LGTM

@gjenkins8 gjenkins8 merged commit 4ac69fc into helm:main Mar 11, 2025
5 checks passed
@scottrigby scottrigby removed the Has One Approval This PR has one approval. It still needs a second approval to be merged. label Mar 12, 2025
@gjenkins8
Copy link
Copy Markdown
Member

gjenkins8 commented Mar 12, 2025

Thanks for the patience @hegerdes

Since main is now targeting Helm v4, I've created a backport PR to dev-v3 branch: #30658

@scottrigby scottrigby added v3 port complete For completed v2->v3 ports and removed Needs v3 backport Label PRs for v4/main, which are still applicable to v3 so need a separate backport PR labels Nov 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature size/L Denotes a PR that changes 100-499 lines, ignoring generated files. v3 port complete For completed v2->v3 ports

Projects

None yet

Development

Successfully merging this pull request may close these issues.

gateway api template

7 participants