Updated documentation and example configs to v3 API#10644
Updated documentation and example configs to v3 API#10644mattklein123 merged 35 commits intoenvoyproxy:masterfrom dmitri-d:update-docs-with-api-v3
Conversation
Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>
Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>
Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>
Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>
Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>
Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>
Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>
Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>
Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>
Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>
Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>
Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>
Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>
Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>
Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>
Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>
Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>
Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>
Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>
Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>
Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>
Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>
Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>
Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>
|
htuch
left a comment
There was a problem hiding this comment.
This is totally epic, thanks so much for taking this on, generated docs look good. I notice that version history still has v2 refs, which is what we want (no need to rewrite history); are there other files that were deliberately skipped?
Re: correctness of example fragments. I don't think it's reasonable to expect a human to manually verify the entire YAML parse of every example, but we should do a few representative ones to smoke test.
Would also like a review from @mattklein123 on the generated docs, since he generally looks at these major doc changes.
| - name: local_service | ||
| domains: ["*"] | ||
| per_filter_config: | ||
| typed_per_filter_config: |
There was a problem hiding this comment.
Did you build a list of all deprecated fields to grep for? Just curious how your script works. Can you share it in a branch or gist? It's fine if it's super-hacky, just want to get a better idea of what I should be trying to validate.
There was a problem hiding this comment.
Deprecated fields in examples were updated by hand (I did build a list of deprecated fields via grep first). I can share the script I used for updating v2 messages/fields/types/enums to v3 ones though.
There was a problem hiding this comment.
Gist with the script (pretty hacky!): https://gist.github.com/dmitri-d/e5957de963d7e51004f4fce2ba82a383
There was a problem hiding this comment.
See my other comment about how this can be less manual in the future, but if there is anything we should commit for future use let's definitely do that.
There was a problem hiding this comment.
@dmitri-d is there some place you grepped out a list of all the deprecated field names and then checked to see if any still exist in docs? I realize there could be many false positives, but this would give me confidence that we haven't missed any (dot-the-Is, cross-the-Ts).
| - socket_address: | ||
| address: localhost | ||
| port_value: 4630 | ||
| load_assignment: |
There was a problem hiding this comment.
For the new fragments that you wrote, did you verify they worked in some kind of test harness, e.g. that this load assignment would parse? It looks reasonable, but, would be reassuring to have some kind of machine check.
Long term we want #8837, but that's outside the scope of this PR; I'd be reassured if at least they were known to parse as valid at least once.
There was a problem hiding this comment.
I did not, agree it would be a good idea.
There was a problem hiding this comment.
Ping on this one. I think it would be good to at least verify one example of each kind of change you've made to ensure there isn't typo/subtle failure. What you could do is take https://github.com/envoyproxy/envoy/blob/master/configs/google_com_proxy.v2.yaml and copy+paste in fragments with some surrounding context, verify it can be loaded with envoy -c. I don't think we need anything beyond that.
Is this a tractable amount of work or is it going to be too crazy?
docs/root/configuration/listeners/listener_filters/original_src_filter.rst
Show resolved
Hide resolved
|
https://338053-65214191-gh.circle-artifacts.com/0/generated/docs/intro/arch_overview/advanced/data_sharing_between_filters.html?highlight=per_filter_config#http-per-route-filter-configuration is an example of where |
|
Example configs at https://338053-65214191-gh.circle-artifacts.com/0/generated/docs/intro/arch_overview/security/ssl.html?highlight=verify_subject_alt_name still have |
Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>
Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>
Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>
Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>
Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>
Turns out I managed to miss a bunch of directories during the first pass. |
mattklein123
left a comment
There was a problem hiding this comment.
Thanks I skimmed through and agreed this is epic. I mainly have some comments on some issues we might want to open to figure out how to make this less manual from your experience. I'm pretty fearful about doc bugs from manual editing, but without doing some of the already opened issues around config checking I think it's going to be pretty difficult. :(
Thank you so much for helping with this!
| - name: local_service | ||
| domains: ["*"] | ||
| per_filter_config: | ||
| typed_per_filter_config: |
There was a problem hiding this comment.
See my other comment about how this can be less manual in the future, but if there is anything we should commit for future use let's definitely do that.
docs/root/configuration/listeners/listener_filters/original_src_filter.rst
Show resolved
Hide resolved
Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>
… docs Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>
Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>
|
htuch
left a comment
There was a problem hiding this comment.
Thanks again for this epic PR. Modulo the typo comment and the question on how we can validate some representative sample in a bounded and small amount of time, this LGTM and is ready to go. Odds are we've missed one or two things, but we can launch-and-iterate.
/wait
…api-v3 Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>
|
Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>
|
htuch
left a comment
There was a problem hiding this comment.
LGTM, thanks! Needs master merge.
Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>
|
htuch
left a comment
There was a problem hiding this comment.
LGTM, will leave this for @mattklein123 to merge in case he has any more comments.
mattklein123
left a comment
There was a problem hiding this comment.
Thank you! Let's ship and iterate.
Resolves #9735