-
Notifications
You must be signed in to change notification settings - Fork 5.3k
tooling: restoring runtime fatal-by-defaults and changing to fully qualified #9591
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
Changes from all commits
d5a261f
be47f9f
6d35c10
aea3988
257e930
3b28ac1
97c7304
23b37e5
ed0dd5e
0875430
b748791
d52433d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| syntax = "proto3"; | ||
|
|
||
| package envoy.annotations; | ||
|
|
||
| import "google/protobuf/descriptor.proto"; | ||
|
|
||
| // Allows tagging proto fields as fatal by default. One Envoy release after | ||
| // deprecation, deprecated fields will be disallowed by default, a state which | ||
| // is reversible with :ref:`runtime overrides <config_runtime_deprecation>`. | ||
|
|
||
| // Magic number in this file derived from top 28bit of SHA256 digest of | ||
| // "envoy.annotation.disallowed_by_default" | ||
| extend google.protobuf.FieldOptions { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One other interesting thing here is we're getting awfully close to being able to either encode the runtime key here as an annotation, or via reflection to being able to derive it. Not sure where the status quo there is today. Something worth thinking about, even if it's beyond the scope of this PR. |
||
| bool disallowed_by_default = 189503207; | ||
| } | ||
|
|
||
| // Magic number in this file derived from top 28bit of SHA256 digest of | ||
| // "envoy.annotation.disallowed_by_default_enum" | ||
| extend google.protobuf.EnumValueOptions { | ||
| bool disallowed_by_default_enum = 70100853; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,7 @@ import "envoy/api/v2/core/grpc_service.proto"; | |
| import "google/protobuf/duration.proto"; | ||
| import "google/protobuf/wrappers.proto"; | ||
|
|
||
| import "envoy/annotations/deprecation.proto"; | ||
| import "udpa/annotations/migrate.proto"; | ||
| import "validate/validate.proto"; | ||
|
|
||
|
|
@@ -40,7 +41,8 @@ message ApiConfigSource { | |
| enum ApiType { | ||
| // Ideally this would be 'reserved 0' but one can't reserve the default | ||
| // value. Instead we throw an exception if this is ever used. | ||
| UNSUPPORTED_REST_LEGACY = 0 [deprecated = true]; | ||
| UNSUPPORTED_REST_LEGACY = 0 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm kind of surprised this wasn't disabled earlier, we have zero support for this AFAIK.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It was disabled, just the old way via the runtime strings :-) |
||
| [deprecated = true, (envoy.annotations.disallowed_by_default_enum) = true]; | ||
|
|
||
| // REST-JSON v2 API. The `canonical JSON encoding | ||
| // <https://developers.google.com/protocol-buffers/docs/proto3#json>`_ for | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -225,6 +225,8 @@ Lines starting with ``#`` as the first character are treated as comments. | |
| Comments can be used to provide context on an existing value. Comments are also useful in an | ||
| otherwise empty file to keep a placeholder for deployment in a time of need. | ||
|
|
||
| .. _config_runtime_deprecation: | ||
|
|
||
| Using runtime overrides for deprecated features | ||
| ----------------------------------------------- | ||
|
|
||
|
|
@@ -238,13 +240,12 @@ increments the :ref:`deprecated_feature_use <runtime_stats>` runtime stat. | |
| Users are encouraged to go to :ref:`deprecated <deprecated>` to see how to | ||
| migrate to the new code path and make sure it is suitable for their use case. | ||
|
|
||
| In the second phase the message and filename will be added to | ||
| :repo:`runtime_features.cc <source/common/runtime/runtime_features.cc>` | ||
| and use of that configuration field will cause the config to be rejected by default. | ||
| This fail-by-default mode can be overridden in runtime configuration by setting | ||
| envoy.deprecated_features.filename.proto:fieldname or envoy.deprecated_features.filename.proto:enum_value | ||
| In the second phase the field will be tagged as disallowed_by_default | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: |
||
| and use of that configuration field will cause the config to be rejected by default. | ||
| This disallowed mode can be overridden in runtime configuration by setting | ||
| envoy.deprecated_features:full_fieldname or envoy.deprecated_features:full_enum_value | ||
| to true. For example, for a deprecated field | ||
| ``Foo.Bar.Eep`` in ``baz.proto`` set ``envoy.deprecated_features.baz.proto:Eep`` to | ||
| ``Foo.Bar.Eep`` set ``envoy.deprecated_features:Foo.bar.Eep`` to | ||
| ``true``. Use of this override is **strongly discouraged**. | ||
| Fatal-by-default configuration indicates that the removal of the old code paths is imminent. It is | ||
| far better for both Envoy users and for Envoy contributors if any bugs or feature gaps with the new | ||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
Do you mind adding some comments about what these annotations do? I'm a little confused what the proposed flow is?