quic: Remove an extra "#comment: " in the quic transport socket proto #42471
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Ryan Hamilton <[email protected]>
|
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
ravenblackx
approved these changes
Dec 8, 2025
adisuissa
approved these changes
Dec 9, 2025
Contributor
adisuissa
left a comment
adisuissa
left a comment
There was a problem hiding this comment.
LGTM, thanks!
/lgtm api
Member
|
@RyanTheOptimist this needs to be enabled properly Did you forget to add 'envoy.transport_sockets.quic' to extensions_build_config.bzl, extensions_metadata.yaml, contrib_build_config.bzl, or contrib/extensions_metadata.yaml?which is odd as im strugggling to understand how it works at all without being registed in extensions_build_config |
Contributor
I think it exists without extensions_build_config because build-wise it's part of a different 'module', and the yaml ones only impact docs. Hopefully, for that reason, only extensions_metadata.yaml is actually required. (I think that's probably the case since the failing thing is a docs test, and extensions_build_config is not related to that.) |
Member
|
/retest |
Member
|
i dont think that would work - the docs test is checking against both envoy_build_extensions and the metadata the point is you cant publish docs for something that is not itself published (and conversely everything published must have docs) |
Member
|
@phlax what would you suggest as a next step to move this forward? |
Member
|
im not sure if totally understand the context, or how its already in use if its not registered - but these docs need to move to whatever is really using it, or it needs to be enabled properly |
Contributor
|
Closing this for #42781 to take over. (I think that version will pass, but waiting for the bazel folks to fix their cert so CI can run before I undraft it.) |
auto-merge was automatically disabled
December 26, 2025 19:14
Pull request was closed
kyessenov
pushed a commit
that referenced
this pull request
Jan 6, 2026
…ion in the docs (#42781) Commit Message: quic: Make the quic transport socket proto correctly appear as an option in the docs Additional Description: This appears to have been added 5 years ago to hide the extension but we can make it visible now. This PR is based on #42471 but fixed so `do_ci.sh docs` passes. A question was raised about whether it needs to be in build extensions; there is precedent for extensions that are not, e.g. [envoy.matching.inputs.request_headers](https://github.com/search?q=repo%3Aenvoyproxy%2Fenvoy%20envoy.matching.inputs.request_headers&type=code) appears in `extensions_metadata.yaml` and does not appear in `extensions_build_config.bzl`. I think the difference is things that aren't from extension sources must instead appear as a `builtin` in `extensions_schema.yaml`, which appears to resolve the other failing test. It also needed a line of doc-visible comment added under the `#extension` line to avoid interpreting it some other way that resulted in a duplicate labels error. Previews of generated docs changes: [All the new header stuff](https://storage.googleapis.com/envoy-cncf-pr/fdb6735/docs/api-v3/extensions/transport_sockets/quic/v3/quic_transport.proto.html#extension-envoy-transport-sockets-quic) vs. [before](https://www.envoyproxy.io/docs/envoy/v1.36.4/api-v3/extensions/transport_sockets/quic/v3/quic_transport.proto.html) <details> <summary>screenshot for posterity</summary> <img width="927" height="831" alt="image" src="https://github.com/user-attachments/assets/3fa1feff-ab5e-4762-bbd7-a06e1a1333ff" /> </details> [quic appears in FilterChain transport_socket options](https://storage.googleapis.com/envoy-cncf-pr/fdb6735/docs/api-v3/config/listener/v3/listener_components.proto.html#envoy-v3-api-field-config-listener-v3-filterchain-transport-socket) vs. [before](https://www.envoyproxy.io/docs/envoy/v1.36.4/api-v3/config/listener/v3/listener_components.proto#extension-category-envoy-transport-sockets-downstream#envoy-v3-api-field-config-listener-v3-filterchain-transport-socket) <details> <summary>screenshot for posterity</summary> <img width="915" height="458" alt="image" src="https://github.com/user-attachments/assets/a4dadb34-bc9b-4bef-b41e-694f705e7cbc" /> </details> [quic appears in Cluster.TransportSocketMatch transport_socket options](https://storage.googleapis.com/envoy-cncf-pr/fdb6735/docs/api-v3/config/cluster/v3/cluster.proto.html#extension-category-envoy-transport-sockets-upstream) vs. [before](https://www.envoyproxy.io/docs/envoy/v1.36.4/api-v3/config/cluster/v3/cluster.proto.html#envoy-v3-api-field-config-cluster-v3-cluster-transportsocketmatch-transport-socket) <details> <summary>screenshot for posterity</summary> <img width="924" height="485" alt="image" src="https://github.com/user-attachments/assets/f1e029ea-965f-4ea6-99ed-88ed5b8dd552" /> </details> Risk Level: changes docs only Testing: CI Docs Changes: yes it is Release Notes: n/a Platform Specific Features: n/a --------- Signed-off-by: Raven Black <[email protected]>
botengyao
pushed a commit
to botengyao/envoy
that referenced
this pull request
Jan 6, 2026
…ion in the docs (envoyproxy#42781) Commit Message: quic: Make the quic transport socket proto correctly appear as an option in the docs Additional Description: This appears to have been added 5 years ago to hide the extension but we can make it visible now. This PR is based on envoyproxy#42471 but fixed so `do_ci.sh docs` passes. A question was raised about whether it needs to be in build extensions; there is precedent for extensions that are not, e.g. [envoy.matching.inputs.request_headers](https://github.com/search?q=repo%3Aenvoyproxy%2Fenvoy%20envoy.matching.inputs.request_headers&type=code) appears in `extensions_metadata.yaml` and does not appear in `extensions_build_config.bzl`. I think the difference is things that aren't from extension sources must instead appear as a `builtin` in `extensions_schema.yaml`, which appears to resolve the other failing test. It also needed a line of doc-visible comment added under the `#extension` line to avoid interpreting it some other way that resulted in a duplicate labels error. Previews of generated docs changes: [All the new header stuff](https://storage.googleapis.com/envoy-cncf-pr/fdb6735/docs/api-v3/extensions/transport_sockets/quic/v3/quic_transport.proto.html#extension-envoy-transport-sockets-quic) vs. [before](https://www.envoyproxy.io/docs/envoy/v1.36.4/api-v3/extensions/transport_sockets/quic/v3/quic_transport.proto.html) <details> <summary>screenshot for posterity</summary> <img width="927" height="831" alt="image" src="https://github.com/user-attachments/assets/3fa1feff-ab5e-4762-bbd7-a06e1a1333ff" /> </details> [quic appears in FilterChain transport_socket options](https://storage.googleapis.com/envoy-cncf-pr/fdb6735/docs/api-v3/config/listener/v3/listener_components.proto.html#envoy-v3-api-field-config-listener-v3-filterchain-transport-socket) vs. [before](https://www.envoyproxy.io/docs/envoy/v1.36.4/api-v3/config/listener/v3/listener_components.proto#extension-category-envoy-transport-sockets-downstream#envoy-v3-api-field-config-listener-v3-filterchain-transport-socket) <details> <summary>screenshot for posterity</summary> <img width="915" height="458" alt="image" src="https://github.com/user-attachments/assets/a4dadb34-bc9b-4bef-b41e-694f705e7cbc" /> </details> [quic appears in Cluster.TransportSocketMatch transport_socket options](https://storage.googleapis.com/envoy-cncf-pr/fdb6735/docs/api-v3/config/cluster/v3/cluster.proto.html#extension-category-envoy-transport-sockets-upstream) vs. [before](https://www.envoyproxy.io/docs/envoy/v1.36.4/api-v3/config/cluster/v3/cluster.proto.html#envoy-v3-api-field-config-cluster-v3-cluster-transportsocketmatch-transport-socket) <details> <summary>screenshot for posterity</summary> <img width="924" height="485" alt="image" src="https://github.com/user-attachments/assets/f1e029ea-965f-4ea6-99ed-88ed5b8dd552" /> </details> Risk Level: changes docs only Testing: CI Docs Changes: yes it is Release Notes: n/a Platform Specific Features: n/a --------- Signed-off-by: Raven Black <[email protected]> Signed-off-by: Boteng Yao <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
quic: Remove an extra "#comment: " in the quic transport socket proto
This appears to have been added 5 years ago to hide the extension but we can make it visible now.