-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
[api] Fix header validations #10335
[api] Fix header validations #10335
Conversation
Signed-off-by: Asra Ali <asraa@google.com>
I wanted to get this patch ready ASAP as a fix, but TODO to find the appropriate way to flip the strict validations. Thoughts? Deprecation and new wrapped message? Add hook (with flag to flip) to make strict in a newer version/log message about it |
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. I don't think we can feature flip strict validation, as that is basically feature flipping API features. This violates the "PGV is stable like the proto" property.
@htuch, are we comfortable with this from a stable API perspective? |
The goal was to align them to stable API policy :) the checks here mimic those done in debug asserts, which provides the implied contract we don't expect to have |
@alyssawilk yeah, this basically undoes the API breaking change in #10093 by replacing it with something that was strongly implied in the API. |
This enables "non-strict" header validations, that match the ones in place with Envoy's
ASSERT(valid())
code. The defaultstrict: true
checks checked for RFC-compliance, which may break previously valid configs.Part of #10318
Signed-off-by: Asra Ali asraa@google.com