API: add runtime guards for risky behavior changes to EnvoyGateway#6556
API: add runtime guards for risky behavior changes to EnvoyGateway#6556zhaohuabing merged 4 commits intoenvoyproxy:mainfrom
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6556 +/- ##
==========================================
+ Coverage 71.11% 71.17% +0.05%
==========================================
Files 220 220
Lines 37970 37985 +15
==========================================
+ Hits 27003 27036 +33
+ Misses 9393 9378 -15
+ Partials 1574 1571 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
7c2d87d to
d321b33
Compare
api/v1alpha1/envoygateway_types.go
Outdated
| // The names of these flags will be included in the release notes alongside an explanation of the change. | ||
| // Please note that these flags are temporary and will be removed in future releases once the features are stable. | ||
| type FeatureFlags struct { | ||
| Enabled []string `json:"enabled,omitempty"` |
There was a problem hiding this comment.
featureFlags:
- name: UseAddrPortAsListenerName
enabled: true
or
featureFlags:
enabled:
- UseAddrPortAsListenerName
cc @envoyproxy/gateway-maintainers @envoyproxy/gateway-reviewers
There was a problem hiding this comment.
Tough choice: option 2 is a bit easier to understand at first glance, and option 1 is more future-proof.
There was a problem hiding this comment.
options 2 is a clear approach, featureGates is about disable/enable.
Option1 is to make each feature is a struct, but I don't think we need to expand more fields besides enable/disable.
So I am +1 for option2
There was a problem hiding this comment.
option 1 feels more natural to me, prefer to have opt-in feature flags
we might do it similarly to the standard k8s feature flags, but we might not have so many feature gates in the end 👀
https://github.com/kubernetes/component-base/blob/master/featuregate/feature_gate.go
There was a problem hiding this comment.
In Kubernetes, most new or experimental features are introduced behind feature gates—which makes a lot of sense. Kubernetes has been around for over 10 years and has a massive user base, so they need to be extremely careful with any changes that could impact stability or compatibility.
EG is still evolving rapidly. For us, a “feature gate” is less about enabling experimental APIs and more about introducing runtime guards to control risky or breaking behavior changes.
There was a problem hiding this comment.
should we then rename it to runtime flags or guard to highlight this is not around feature enablement but around modifying runtime behavior thats not exposed via an API
There was a problem hiding this comment.
+1 to option 2, even better if we can just have the flags directly as everything is supposed to be a boolean gate if named and used correctly, example:
runtimeFlags:
- enableUseAddrPortAsListenerName
Anything else that takes in a dynamic value as input should ideally be part of API.
Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
|
|
||
| // RuntimeFlags defines the runtime flags for Envoy Gateway. | ||
| // Unlike ExtensionAPIs, these flags are temporary and will be removed in future releases once the related features are stable. | ||
| RuntimeFlags *RuntimeFlags `json:"runtimeFlags,omitempty"` |
api/v1alpha1/envoygateway_types.go
Outdated
| RuntimeFlags *RuntimeFlags `json:"runtimeFlags,omitempty"` | ||
| } | ||
|
|
||
| // RuntimeFlag defines a runtime flag for Envoy Gateway. |
There was a problem hiding this comment.
Repharse:
// RuntimeFlag defines a runtime flag used to guard breaking changes or risky experimental features in new Envoy Gateway releases.
// A runtime flag may be enabled or disabled by default and can be toggled through the EnvoyGateway resource.
Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
|
|
||
| // RuntimeFlag defines a runtime flag used to guard breaking changes or risky experimental features in new Envoy Gateway releases. | ||
| // A runtime flag may be enabled or disabled by default and can be toggled through the EnvoyGateway resource. | ||
| type RuntimeFlag string |
There was a problem hiding this comment.
can we add a validation tag here
There was a problem hiding this comment.
Oh sorry I misunderstood your comment - will add it in the implementation PR.
This PR introduces a mechanism to gate breaking changes or experimental features in new Envoy Gateway releases. e.g. changing the naming of the xDS listener, which can cause incompatible changes to existing EnvoyPatchPolicies and ExtensionManager implementations.
Each flag may be enabled or disabled by default and can be toggled through the EnvoyGateway resource.
The names of these flags will be included in the release notes alongside an explanation of the change.
These flags are temporary and will be removed in future releases once the features are stable.
Release Notes: No. The release note for the first runtime flag will be included in #6544.
Inspired by Envoy runtime flag: https://github.com/envoyproxy/envoy/blob/main/CONTRIBUTING.md#runtime-guarding
The implementation will be included in #6544.