Skip to content

config: officially deprecating deprecated_v1 fields#4780

Closed
alyssawilk wants to merge 2 commits intoenvoyproxy:masterfrom
alyssawilk:v1_deprecation
Closed

config: officially deprecating deprecated_v1 fields#4780
alyssawilk wants to merge 2 commits intoenvoyproxy:masterfrom
alyssawilk:v1_deprecation

Conversation

@alyssawilk
Copy link
Copy Markdown
Contributor

For removal in the next release ~3 months out

Risk Level: Low (deprecating one lingering proto field)
Testing: n/a
Docs Changes: updated DEPRECATED.md
Release Notes: n/a
Deprecated: deprecated deprecated config

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
@alyssawilk
Copy link
Copy Markdown
Contributor Author

@envoyproxy/senior-maintainers still good with this? If so I'll land and envoy-announce.

Copy link
Copy Markdown
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

Comment thread DEPRECATED.md Outdated

## Version 1.9.0 (pending)

* Use of the v2 API "deprecated_v1" fields is deprecated.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit: *deprecated_v1*

…ve deprecated_v1 in this release. alternately I could remove it?)

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Comment thread DEPRECATED.md
## Version 1.8.0 (Oct 4, 2018)

* Use of the v1 API (including `*.deprecated_v1` fields in the v2 API) is deprecated.
* Use of the v1 API is deprecated.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No rewriting history! :)

@rshriram
Copy link
Copy Markdown
Member

does this include tcp_proxy? coz we couldn't remove the tcp proxy deprecated_v1 config (@venilnoronha had a Pr). I think it boiled down to lack of source IP match in filter chain match.

@venilnoronha
Copy link
Copy Markdown
Member

PR #4625 is still open.


// [#not-implemented-hide:]
DeprecatedV1 deprecated_v1 = 7;
DeprecatedV1 deprecated_v1 = 7 [deprecated = true];
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think there are more than this as @rshriram points out? I'm pretty sure we have a bunch of these...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I believe the rest were already deprecated :-)
I'm going to close this out regardless though - I'd thought updating the deprecation notice from last release to this release would make deprecation schedule more clear but if we'd prefer to not rewrite history I don't think deprecating it every version until we remove is helpful

@mattklein123 mattklein123 self-assigned this Oct 20, 2018
@alyssawilk alyssawilk closed this Oct 22, 2018
@alyssawilk alyssawilk deleted the v1_deprecation branch November 28, 2018 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants