Skip to content

route.proto: add is_optional field to ClusterSpecifierPlugin#20301

Merged
lizan merged 3 commits intoenvoyproxy:mainfrom
dfawley:optcsp
Mar 24, 2022
Merged

route.proto: add is_optional field to ClusterSpecifierPlugin#20301
lizan merged 3 commits intoenvoyproxy:mainfrom
dfawley:optcsp

Conversation

@dfawley
Copy link
Copy Markdown
Member

@dfawley dfawley commented Mar 11, 2022

Signed-off-by: Doug Fawley dfawley@google.com

Commit Message: route.proto: add is_optional field to ClusterSpecifierPlugin
Additional Description:

When deploying a new cluster specifier plugin, it is often necessary to add it to the configuration before all clients can be updated to support it, with routing rules configured to prevent clients without support from selecting any routes referencing the plugin. This field will allow those clients to suppress the default behavior of NACKing any resource containing the unknown plugin.

Risk Level: None
Testing: None
Docs Changes: None
Release Notes: None
Platform Specific Features: None

@markdroth @ejona86

Signed-off-by: Doug Fawley <dfawley@google.com>
@repokitteh-read-only
Copy link
Copy Markdown

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @markdroth
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).

🐱

Caused by: #20301 was opened by dfawley.

see: more, trace.

@markdroth
Copy link
Copy Markdown
Contributor

This looks great to me.

@lizan, can you please provide a non-Google API review here? Thanks!

@lizan
Copy link
Copy Markdown
Member

lizan commented Mar 11, 2022

/lgtm api

@ggreenway
Copy link
Copy Markdown
Member

@dfawley please fix CI.

.
Signed-off-by: Doug Fawley <dfawley@google.com>
Signed-off-by: Doug Fawley <dfawley@google.com>
@dfawley
Copy link
Copy Markdown
Member Author

dfawley commented Mar 18, 2022

@ggreenway apologies; I had not noticed the CI failures. I fixed the formatting, so it should be good to go now.

@markdroth
Copy link
Copy Markdown
Contributor

/lgtm api (based on Lizan's earlier approval, since it's just a formatting change since then)

@repokitteh-read-only
Copy link
Copy Markdown

please specify a single label can be specified

🐱

Caused by: a #20301 (comment) was created by @markdroth.

see: more, trace.

@markdroth
Copy link
Copy Markdown
Contributor

/lgtm api

@lizan
Copy link
Copy Markdown
Member

lizan commented Mar 22, 2022

@dfawley do you plan to implement this in this PR? If not it should have not-implemented-hide tag.

@lizan lizan added the waiting label Mar 22, 2022
@markdroth
Copy link
Copy Markdown
Contributor

@lizan The entire ClusterSpecifierPlugin message is already not implemented in Envoy, so this just adds another field to a message that's already not implemented. There is a not-implemented tag here. I don't think we need another one for this individual field.

@markdroth
Copy link
Copy Markdown
Contributor

@lizan Any reason not to merge this? Thanks!

@lizan lizan merged commit 55539d3 into envoyproxy:main Mar 24, 2022
@dfawley dfawley deleted the optcsp branch March 25, 2022 16:16
ravenblackx pushed a commit to ravenblackx/envoy that referenced this pull request Jun 8, 2022
…oxy#20301)

When deploying a new cluster specifier plugin, it is often necessary to add it to the configuration before all clients can be updated to support it, with routing rules configured to prevent clients without support from selecting any routes referencing the plugin.  This field will allow those clients to suppress the default behavior of NACKing any resource containing the unknown plugin.

Risk Level: None
Testing: None
Docs Changes: None
Release Notes: None
Platform Specific Features: None

Signed-off-by: Doug Fawley <dfawley@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants