static backend app protocol #11384
static backend app protocol #11384
Conversation
Signed-off-by: Jenny Shu <[email protected]>
github-actions
bot
added
kind/feature
| } | ||
|
|
||
| // AppProtocol defines the application protocol to use when communicating with the backend. | ||
| // +kubebuilder:validation:Enum=http2;grpc;grpc-web;kubernetes.io/h2c;kubernetes.io/ws |
There was a problem hiding this comment.
not sure if we want to be restrictive here or just allow any string
There was a problem hiding this comment.
makes sense to restrict. is there a reason a user would have an arbitrary string?
There was a problem hiding this comment.
is there a reason a user would have an arbitrary string?
reasons for allowing any string:
- k8s Service allows any string
- they might be using protocols that we don't explicitly handle
reasons for using enum:
- it spells out the protocols that we explicitly support
- anything else would map to the same 'default' protocol which is effectively the same as leaving it blank
Signed-off-by: Jenny Shu <[email protected]>
| // +optional | ||
| // +kubebuilder:validation:Optional |
There was a problem hiding this comment.
do you need both lines?
There was a problem hiding this comment.
hmm not sure, in some other places we are using both as well. i'll try removing one and see if it makes a difference
There was a problem hiding this comment.
so it seems they are redundant, but i think i want to do a pass in a separate PR to clean up all the kubebuilder markers (i noticed typos in some of them)
| --- | ||
| apiVersion: v1 | ||
| kind: Service | ||
| apiVersion: gateway.kgateway.dev/v1alpha1 |
There was a problem hiding this comment.
why was the service replace with backend? should we have both?
There was a problem hiding this comment.
good question. i previously had 6 translator tests (Service with each appProtocol+default). now we support the same thing with Backends, and i thought having a new translator test for each app protocol was overkill, so i've changed it to have 3 tests each for Service and Backend (http2, ws, default), and i added unit tests in pkg/pluginsdk/ir/backend_test.go to test the parsing of all the various protocols
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
3 participants
Description
Allow configuring app protocol on Static Backends.
Fixes #10622
Change Type
/kind new_feature
Changelog
Additional Notes