Fixes #10815: Warning is logged when v2 xDS is used by dmitri-d · Pull Request #10964 · envoyproxy/envoy

dmitri-d · 2020-04-27T18:13:50Z

Signed-off-by: Dmitri Dolguikh ddolguik@redhat.com

Commit Message: Warning is logged when v2 xDS is used
Additional Description: Fixes #10815
Risk Level: low
Testing:
Docs Changes:
Release Notes:
[Optional Fixes #Issue]
[Optional Deprecated:]

Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>

dmitri-d · 2020-05-01T22:11:51Z

merged in latest master

Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>

dmitri-d · 2020-05-04T18:21:56Z

ping @yanavlasov

Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>

dmitri-d · 2020-05-12T21:41:01Z

merged in latest master

dmitri-d · 2020-05-12T21:42:39Z

ping @yanavlasov

mattklein123

Thanks one small comment.

/wait

source/common/config/subscription_factory_impl.cc

Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>

mattklein123

LGTM with small comment, thank you!

/wait

source/common/config/subscription_factory_impl.cc

include/envoy/runtime/runtime.h

Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>

dmitri-d · 2020-05-14T23:17:05Z

responded to feedback

Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>

mattklein123

Sorry 2 more things. Thank you!

/wait

mattklein123 · 2020-05-15T16:04:07Z

source/common/runtime/runtime_features.cc

    "envoy.reloadable_features.strict_authority_validation",
    "envoy.reloadable_features.reject_unsupported_transfer_encodings",
    // Begin alphabetically sorted section.
+    "envoy.api.enable_deprecated_warning",


Sorry I should have commented on this before but 2 things:

I think the name of this should be a bit more specific to what it is guarding.

This needs a release note.

I think the name of this should be a bit more specific to what it is guarding.

Any suggestions? The current name was suggested by @htuch...

envoy.api.enable_deprecated_v2_api_warning ?

Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>

dmitri-d · 2020-05-20T19:04:19Z

fixed formatting issues

mattklein123

/wait

docs/root/version_history/current.rst

Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>

dmitri-d · 2020-05-20T21:35:52Z

fixed merge issue

docs/root/version_history/current.rst

Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>

dmitri-d · 2020-05-21T17:48:53Z

Moved the release note into the deprecation section

dmitri-d · 2020-05-21T20:29:08Z

Build failure on MacOS is due to a test (//test/integration:http2_integration_test) timeout. Ping @mattklein123.

mattklein123 · 2020-05-21T20:58:59Z

Build failure on MacOS is due to a test (//test/integration:http2_integration_test) timeout. Ping

#11290 cc @alyssawilk

mattklein123

LGTM without errant change.

/wait

source/extensions/extensions_build_config.bzl

Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>

dmitri-d · 2020-05-21T21:21:45Z

reverted changes to source/extensions/extensions_build_config.bzl
merged in master

mattklein123

Thanks!

The earlier PR envoyproxy#10964 added in validation that we're not using v2 xDS transport, but we didn't have any warning on v2 bootstraps, v2 extension configs inside a v3 config and also for v2 resource types. This PR adds support for these validations by warning and incrementing the deprecated feature use stat. The following notifications are added: * Rate limited warning log * Trace-level log * The runtime.deprecated_features is bumped, but only after server initialization, so this doesn't apply to bootstrap. This is consistent with how other deprecated features are treated today. Risk level: Medium (it's possible that this will cause monitoring to alert, the boosting logic is also quite complex). Testing: Unit tests and server bootstrap tests added. Fixes envoyproxy#10815 Signed-off-by: Harvey Tuch <htuch@google.com>

The earlier PR #10964 added in validation that we're not using v2 xDS transport, but we didn't have any warning on v2 bootstraps, v2 extension configs inside a v3 config and also for v2 resource types. This PR adds support for these validations by warning and incrementing the deprecated feature use stat. The following notifications are added: - Rate limited warning log - Trace-level log - The runtime.deprecated_features is bumped, but only after server initialization, so this doesn't apply to bootstrap. This is consistent with how other deprecated features are treated today. Risk level: Medium (it's possible that this will cause monitoring to alert, the boosting logic is also quite complex). Testing: Unit tests and server bootstrap tests added. Fixes #10815 Signed-off-by: Harvey Tuch <htuch@google.com>

Fixes #10815: Warning is logged when v2 xDS is used

584c8d4

Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>

zuercher assigned yanavlasov Apr 27, 2020

Dmitri Dolguikh added 4 commits April 27, 2020 12:21

Merge remote-tracking branch 'upstream' into v2-deprecation-warn

7104958

Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>

Forgot to commit test changes

4bab636

Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>

Added a test

28fcf2b

Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>

Merge remote-tracking branch 'upstream' into v2-deprecation-warn

5aa74f4

Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>

Fixed formatting

0de191c

Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>

Merge remote-tracking branch 'upstream/master' into v2-deprecation-warn

2a472d4

Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>

yanavlasov previously approved these changes May 14, 2020

View reviewed changes

mattklein123 requested changes May 14, 2020

View reviewed changes

source/common/config/subscription_factory_impl.cc Show resolved Hide resolved

repokitteh-read-only bot added the waiting label May 14, 2020

mattklein123 self-assigned this May 14, 2020

Increment deprecated_feature_use when deprecated api is used

7dd0dc0

Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>

dmitri-d dismissed yanavlasov’s stale review via 7dd0dc0 May 14, 2020 22:45

repokitteh-read-only bot removed the waiting label May 14, 2020

Merge remote-tracking branch 'upstream/master' into v2-deprecation-warn

cadb6bb

Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>

mattklein123 requested changes May 14, 2020

View reviewed changes

source/common/config/subscription_factory_impl.cc Show resolved Hide resolved

include/envoy/runtime/runtime.h Outdated Show resolved Hide resolved

repokitteh-read-only bot added the waiting label May 14, 2020

Responded to feedback

d6ca192

Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>

repokitteh-read-only bot removed the waiting label May 14, 2020

Fixed member function definition

ca7115b

Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>

mattklein123 requested changes May 15, 2020

View reviewed changes

repokitteh-read-only bot added the waiting label May 15, 2020

Made runtime feature name more specific, added a release note

da134cd

Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>

repokitteh-read-only bot removed the waiting label May 15, 2020

repokitteh-read-only bot added the waiting label May 20, 2020

Fixed formatting issues

cd10e67

Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>

repokitteh-read-only bot removed the waiting label May 20, 2020

mattklein123 requested changes May 20, 2020

View reviewed changes

docs/root/version_history/current.rst Outdated Show resolved Hide resolved

repokitteh-read-only bot added the waiting label May 20, 2020

Fixed merging issue

615d18e

Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>

repokitteh-read-only bot removed the waiting label May 20, 2020

mattklein123 reviewed May 20, 2020

View reviewed changes

docs/root/version_history/current.rst Outdated Show resolved Hide resolved

repokitteh-read-only bot added the waiting label May 20, 2020

Moved the release note into the deprecation section

0f5a1d5

Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>

repokitteh-read-only bot removed the waiting label May 21, 2020

mattklein123 requested changes May 21, 2020

View reviewed changes

source/extensions/extensions_build_config.bzl Outdated Show resolved Hide resolved

repokitteh-read-only bot added the waiting label May 21, 2020

Dmitri Dolguikh added 2 commits May 21, 2020 14:20

reverted changes to source/extensions/extensions_build_config.bzl

1521ad4

Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>

Merge remote-tracking branch 'upstream' into v2-deprecation-warn

bc0c0b0

Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>

repokitteh-read-only bot removed the waiting label May 21, 2020

mattklein123 approved these changes May 21, 2020

View reviewed changes

mattklein123 merged commit 4408997 into envoyproxy:master May 21, 2020

dmitri-d deleted the v2-deprecation-warn branch May 21, 2020 23:37

htuch mentioned this pull request Sep 23, 2020

v2 -> v3 API postmortem #10943

Closed

htuch mentioned this pull request Sep 23, 2020

config: extended warnings on v2 API usage. #13238

Merged

Conversation

dmitri-d commented Apr 27, 2020

Uh oh!

dmitri-d commented May 1, 2020

Uh oh!

dmitri-d commented May 4, 2020

Uh oh!

dmitri-d commented May 12, 2020

Uh oh!

dmitri-d commented May 12, 2020

Uh oh!

mattklein123 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mattklein123 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

dmitri-d commented May 14, 2020

Uh oh!

mattklein123 left a comment

Choose a reason for hiding this comment

Uh oh!

mattklein123 May 15, 2020

Choose a reason for hiding this comment

Uh oh!

dmitri-d May 15, 2020

Choose a reason for hiding this comment

Uh oh!

mattklein123 May 15, 2020

Choose a reason for hiding this comment

Uh oh!

dmitri-d commented May 20, 2020

Uh oh!

mattklein123 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dmitri-d commented May 20, 2020

Uh oh!

Uh oh!

dmitri-d commented May 21, 2020

Uh oh!

dmitri-d commented May 21, 2020

Uh oh!

mattklein123 commented May 21, 2020

Uh oh!

mattklein123 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dmitri-d commented May 21, 2020

Uh oh!

mattklein123 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants