-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Removed all parts related to runtime guard - https://github.com/envoy… #34199
Conversation
c642081
to
deda219
Compare
/wait |
a56e630
to
866cd3d
Compare
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.
/wait-any
@@ -330,10 +329,6 @@ absl::string_view ExtractorImpl::extractJWT(absl::string_view value_str, | |||
} | |||
|
|||
// There should be two dots (periods; 0x2e) inside the string, but we don't verify that here | |||
if (Runtime::runtimeFeatureEnabled("envoy.reloadable_features.token_passed_entirely")) { | |||
return value_str.substr(starting); |
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.
Sorry, I don't get something fundamental in this PR.
In #28678 you introduced a feature, and added a runtime-guard around it, so that if one wants to temporarily disable it, they can.
This PR, IIUC, is removing the implementation that was added (reverting the previous PR). Why is that?
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.
Yes! you're right I missed it that runtime guard is by default enabled.
Now I fixed it and now it will always run my change.
Till now code is cutting the valid token and from now always pass it as is.
Thanks for your important comment!
b68a517
to
7692a42
Compare
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.
Signed-off-by: Daniel Danan <danield@radware.com>
…work with my changes (without runtime guard) + returned a comment to extractor.cc Signed-off-by: Daniel Danan <danield@radware.com>
Signed-off-by: Daniel Danan <danield@radware.com>
Signed-off-by: Daniel Danan <danield@radware.com>
48bd678
to
6e88fcd
Compare
Added to release notes |
Co-authored-by: Adi (Suissa) Peleg <adip@google.com> Signed-off-by: danieldradware <117576776+danieldradware@users.noreply.github.com>
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.
LGTM, thanks!
@zuercher for senior stamp |
…proxy/envoy/issues/33627
Commit Message:
Removed all parts related to runtime guard
Additional Description:
As Envoy team asked from me to remove all relevant parts of runtime guard (#33627)
In this PR I removed it.
Risk Level:
Testing:
Removed tests
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]