Skip to content

Updated documentation and example configs to v3 API#10644

Merged
mattklein123 merged 35 commits intoenvoyproxy:masterfrom
dmitri-d:update-docs-with-api-v3
May 4, 2020
Merged

Updated documentation and example configs to v3 API#10644
mattklein123 merged 35 commits intoenvoyproxy:masterfrom
dmitri-d:update-docs-with-api-v3

Conversation

@dmitri-d
Copy link
Copy Markdown
Contributor

@dmitri-d dmitri-d commented Apr 3, 2020

Resolves #9735

Dmitri Dolguikh added 18 commits April 3, 2020 11:27
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>
@dmitri-d dmitri-d changed the title WIP: Update documentation and example configs to v3 API Updated documentation and example configs to v3 API Apr 8, 2020
Dmitri Dolguikh added 2 commits April 8, 2020 16:17
Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>
Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>
@htuch htuch self-assigned this Apr 10, 2020
Dmitri Dolguikh added 2 commits April 15, 2020 16:06
Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>
Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>
@htuch htuch self-requested a review April 16, 2020 21:29
Dmitri Dolguikh added 2 commits April 16, 2020 14:59
Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>
Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>
@dmitri-d
Copy link
Copy Markdown
Contributor Author

  • merged in latest changes from master

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.

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:
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.

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.

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.

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.

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.

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.

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.

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.

@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:
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.

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.

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 did not, agree it would be a good idea.

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.

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?

@htuch
Copy link
Copy Markdown
Member

htuch commented Apr 20, 2020

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 per_filter_config still has untyped references. These are in text rather than code, but I think they can be spotted by grep.

@htuch
Copy link
Copy Markdown
Member

htuch commented Apr 20, 2020

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 verify_subject_alt_name and v2 references in type URLs.

Dmitri Dolguikh added 5 commits April 20, 2020 13:54
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>
@dmitri-d
Copy link
Copy Markdown
Contributor Author

  • fixed issues in the two files that @htuch found, and a bunch more.

Turns out I managed to miss a bunch of directories during the first pass.

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

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:
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.

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.

Dmitri Dolguikh added 3 commits April 23, 2020 10:39
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>
@dmitri-d
Copy link
Copy Markdown
Contributor Author

  • merged in latest master

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.

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>
@dmitri-d
Copy link
Copy Markdown
Contributor Author

  • updated with the latest master

Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>
@dmitri-d
Copy link
Copy Markdown
Contributor Author

  • verified example configurations, fixed several of them

htuch
htuch previously approved these changes May 1, 2020
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, thanks! Needs master merge.

Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>
@dmitri-d
Copy link
Copy Markdown
Contributor Author

dmitri-d commented May 2, 2020

  • merged in latest master

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, will leave this for @mattklein123 to merge in case he has any more comments.

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

Thank you! Let's ship and iterate.

@mattklein123 mattklein123 merged commit f64ae9c into envoyproxy:master May 4, 2020
@dmitri-d dmitri-d deleted the update-docs-with-api-v3 branch May 4, 2020 20:09
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.

Update documentation and example configs to v3 API

3 participants