Conversation
✅ Deploy Preview for cerulean-figolla-1f9435 canceled.
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #8161 +/- ##
==========================================
+ Coverage 73.88% 73.93% +0.05%
==========================================
Files 241 239 -2
Lines 36609 36523 -86
==========================================
- Hits 27047 27003 -44
+ Misses 7656 7609 -47
- Partials 1906 1911 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
5c438ec to
71d124b
Compare
b71c336 to
a36af69
Compare
Signed-off-by: zirain <zirain2009@gmail.com>
Signed-off-by: zirain <zirain2009@gmail.com>
Signed-off-by: zirain <zirain2009@gmail.com>
Signed-off-by: zirain <zirain2009@gmail.com>
Signed-off-by: zirain <zirain2009@gmail.com>
Signed-off-by: zirain <zirain2009@gmail.com>
Signed-off-by: zirain <zirain2009@gmail.com>
Signed-off-by: zirain <zirain2009@gmail.com>
Signed-off-by: zirain <zirain2009@gmail.com>
cc02c98 to
bd0bbfe
Compare
Signed-off-by: zirain <zirain2009@gmail.com>
0c478ed to
7ced098
Compare
|
|
||
| // GatewayAPI defines an experimental Gateway API resource that can be enabled. | ||
| // +enum | ||
| // +kubebuilder:validation:Enum=XListenerSet;XBackendTrafficPolicy |
There was a problem hiding this comment.
XListenerSet promote to v1 in Gateway API v1.5
There was a problem hiding this comment.
guessing this is the breaking change because even if a user wants to stay on XListenerSet, then cant ? and must migrate to ListenerSet when upgrading to v1.8 ?
There was a problem hiding this comment.
yes, If we need rethink about this in the future, do we need to support X API.
There was a problem hiding this comment.
for what it's worth, we (cert-manager) have decided to do a clean cut upgrade to ListenerSet as opposed to keeping both resources. There was a discussion on whether to keep the X resources around in 1.5: https://kubernetes.slack.com/archives/CR0H13KGA/p1770328007249819
There was a problem hiding this comment.
+1 to removing XListenerSet support from 1.8 since we've already set expectations in the GatewayAPISettings that These APIs are experimental today and are subject to change or removal as they mature.
Maintaining both XListenerSet and ListenerSet would add quite a bit of ongoing overhead, and it might not be worth keeping both around.
| - attachedRoutes: 0 | ||
| conditions: | ||
| - lastTransitionTime: null | ||
| message: Listener must have TLS set when protocol is TLS. |
0629b69 to
883882a
Compare
f862d52 to
0e211a9
Compare
Signed-off-by: zirain <zirain2009@gmail.com>
| // TODO: fix following conformance tests | ||
| tests.ListenerSetHostnameConflict, | ||
| tests.ListenerSetProtocolConflict, | ||
| tests.TLSRouteHostnameIntersection, | ||
| tests.TLSRouteInvalidNoMatchingListener, | ||
| tests.TLSRouteInvalidNoMatchingListenerHostname, | ||
| tests.TLSRouteInvalidReferenceGrant, | ||
| tests.TLSRouteListenerTerminateSupportedKinds, | ||
| tests.TLSRouteSimpleSameNamespace, | ||
| tests.TLSRouteTerminateSimpleSameNamespace, | ||
| tests.TLSRouteMixedTerminationSameNamespace, | ||
| tests.GatewayInvalidTLSBackendConfiguration, | ||
| tests.GatewayWithAttachedRoutes, | ||
| tests.GatewayTLSBackendClientCertificate, | ||
| tests.GatewayFrontendClientCertificateValidation, | ||
| tests.GatewayInvalidFrontendClientCertificateValidation, | ||
| tests.GatewayFrontendInvalidDefaultClientCertificateValidation, | ||
| tests.HTTPRouteHTTPSListenerDetectMisdirectedRequests, | ||
| tests.HTTPRoute303Redirect, | ||
| tests.HTTPRoute307Redirect, | ||
| tests.HTTPRoute308Redirect, | ||
| tests.HTTPRouteHostnameIntersection, |
There was a problem hiding this comment.
TODO: Tracked in seperated issue.
| - {{ include "eg.rbac.namespaced.gateway.envoyproxy.status" . | nindent 2 | trim }} | ||
| - {{ include "eg.rbac.namespaced.gateway.networking" . | nindent 2 | trim }} | ||
| - {{ include "eg.rbac.namespaced.gateway.networking.status" . | nindent 2 | trim }} | ||
| - {{ include "eg.rbac.namespaced.gateway.networking.experimental" . | nindent 2 | trim }} |
There was a problem hiding this comment.
curious why these are removed
| resourceType: string(EndpointEnvoyConfigType), | ||
| expect: true, | ||
| }, | ||
| { |
|
|
||
| var ( | ||
| //go:embed charts/gateway-helm/crds/gatewayapi-crds.yaml | ||
| //go:embed gatewayapi-crds.yaml |
There was a problem hiding this comment.
cuirous why are we moving this out of chart
There was a problem hiding this comment.
The gatewayapi-crds is large than 1MB, which make helm install never success.
Because helm store the release info in secret(the default driver), which have 1MB limitation.
There was a problem hiding this comment.
I think keeping the CRDs in gateway-helm is still useful — at least for development and testing workflows.
Does #8283 help?
|
hey thanks for driving this work ! is the plan to rm all CRDs from |
It's hard, most of them are related to bump Gateway API v1.5. |
Signed-off-by: zirain <zirain2009@gmail.com>
|
/retest |
1 similar comment
|
/retest |
Signed-off-by: zirain <zirain2009@gmail.com>
There was a problem hiding this comment.
Shouldn't this file be renamed to listenerset.go?
There was a problem hiding this comment.
yes, that is right.
let's do that in seperated PR.
I think keeping the CRDs in gateway-helm is still useful — at least for development and testing workflows. Does #8283 help? |
zhaohuabing
left a comment
There was a problem hiding this comment.
Should we watch TLSRoute v1 instead of v1alpha3?
Continuing watching v1alpha3 would break standard channel users as it's not in standard channel.
sigs.k8s.io/gateway-apiandsigs.k8s.io/gateway-api/conformanceinternal/gatewayapi/conformaceundertest, it's not used, and production shouldn't rely on conformance.