Skip to content
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.

Sign up for GitHub

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

Jump to bottom

config: flipping deprecated configs fatal by default #9636

Merged
merged 1 commit into from
Jan 15, 2020
Merged

config: flipping deprecated configs fatal by default #9636

merged 1 commit into from
Jan 15, 2020

Conversation

alyssawilk
Copy link
Contributor

@alyssawilk alyssawilk commented Jan 9, 2020 •
edited
Loading

This PR makes the following fatal by default:
from cluster.proto: ORIGINAL_DST_LB, tls_context, extension_protocol_options
from health_check.proto: use_http2
from route_components.proto: allow_origin regex, pattern, method, regex_match, value
from http_connection_manager.proto: operation_name
from trace.proto: HTTP_JSON_V1
from string.proto: regex

Risk Level: Medium (who knows who is using them)
Testing: test framework updates
Docs Changes: n/a
Release Notes: n/a

Originally #8847

@repokitteh-read-only
Copy link

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/.

🐱

Caused by: #9636 was opened by alyssawilk.

see: more, trace.

htuch
htuch reviewed Jan 9, 2020
View reviewed changes
Copy link
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM on the flag flips.

@repokitteh-read-only
Copy link

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/.

🐱

Caused by: #9636 was synchronize by alyssawilk.

see: more, trace.

@mattklein123 mattklein123 self-assigned this Jan 12, 2020
@alyssawilk
Tagging the rest of deprecated features unsupported.
e4edba3 
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
@repokitteh-read-only
Copy link

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/.

🐱

Caused by: #9636 was synchronize by alyssawilk.

see: more, trace.

@alyssawilk alyssawilk marked this pull request as ready for review January 15, 2020 01:26
@alyssawilk
Copy link
Contributor Author

Ok, cleaned up, and should finally be good to go!
thanks so much the proto clean-up to unblock these, @htuch

htuch
htuch approved these changes Jan 15, 2020
View reviewed changes
Copy link
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! @mattklein123 any further comments? This is probably the last part of the deprecation work beyond what I need to do with protoxform.

mattklein123
mattklein123 approved these changes Jan 15, 2020
View reviewed changes
Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work!

@alyssawilk alyssawilk merged commit 33100e8 into envoyproxy:master Jan 15, 2020
@alyssawilk alyssawilk deleted the dirty_flag_flips branch May 18, 2020 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattklein123 mattklein123 mattklein123 approved these changes

@htuch htuch htuch approved these changes

Assignees

@mattklein123 mattklein123

@htuch htuch

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@alyssawilk @mattklein123 @htuch