-
Notifications
You must be signed in to change notification settings - Fork 28
Add support for Cilium-based Gateway API #216
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
base: main
Are you sure you want to change the base?
Conversation
5930d13 to
6fd1cd1
Compare
| spec = { | ||
| gatewayClassName = "cilium" | ||
| infrastructure = { | ||
| annotations = local.ingress_load_balancer_annotations | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks more like Gateway spec. I think the GatewayClass is created automatically when GatewayAPI CRDs are detected: https://github.com/cilium/cilium/blob/v1.18.2/install/kubernetes/cilium/values.yaml#L1015-L1019
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has been not finalized yet, nor even tested. Please don't review drafts if possible, thank you.
| validation { | ||
| condition = ( | ||
| var.gateway_api_provider != "cilium" || | ||
| (var.gateway_api_provider == "cilium" && | ||
| var.cilium_helm_version == "v1.18.2" && | ||
| var.gateway_api_version == "v1.3.0") | ||
| ) | ||
| error_message = "When gateway_api_provider is 'cilium', cilium_helm_version must be 'v1.18.2' and gateway_api_version must be 'v1.3.0'." | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let’s avoid such explicit version tracking in this module. We should only ships config compatible with the component versions in use.
| variable "gateway_api_provider" { | ||
| type = string | ||
| default = "cilium" | ||
| description = "Specifies the Gateway API provider. Options are 'cilium' (Cilium Controller), or 'ingate' (InGate Ingress & Gateway API Controller)." | ||
|
|
||
| validation { | ||
| condition = contains(["cilium", "ingate"], var.gateway_api_provider) | ||
| error_message = "Invalid Gateway API provider. Allowed values are 'cilium', or 'ingate'." | ||
| } | ||
|
|
||
| validation { | ||
| condition = var.gateway_api_provider != "cilium" || var.cilium_enabled | ||
| error_message = "Gateway API provider cannot be set to 'cilium' unless Cilium is also enabled." | ||
| } | ||
|
|
||
| validation { | ||
| condition = var.gateway_api_provider != "ingate" | ||
| error_message = "Gateway API provider 'ingate' is not yet supported." | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’d remove that, since we should aim to support multiple providers. Our main focus should be on Cilium, as it’s already shipped with this module. At this point, I’m not sure if or when we’ll add other GatewayAPIs, especially since deep integration with this module isn’t necessary anymore. Let’s finish Cilium GatewayAPI first, then decide if adding more providers is really needed.
| validation { | ||
| condition = ( | ||
| var.gateway_api_provider != "ingate" || | ||
| (var.gateway_api_provider == "ingate" && var.gateway_api_version == "v1.2.0") | ||
| ) | ||
| error_message = "When gateway_api_provider is 'ingate', gateway_api_version must be 'v1.2.0'." | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let’s avoid such explicit version tracking in this module. We should only ships config compatible with the component versions in use.
| type = bool | ||
| default = false | ||
| description = "Enables the experimental Gateway API features. These features are not yet part of the official Gateway API specification and may change in future releases." | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing newline at the end
|
I don't see a way to provide the LB annotations via |
Signed-off-by: Mateusz Paluszkiewicz <theaifam5@gmail.com>
6fd1cd1 to
4214d6a
Compare
|
I've just noticed there's also a limit of 8 annotations for the |
Closes: #215
Depends On: #214