Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: Reference pages for service-router and service-resolver config entries #17145

Merged
merged 27 commits into from
May 17, 2023

Conversation

boruszak
Copy link
Contributor

Description

This PR updates the service resolver and service router configuration entry reference pages as part of the Consul Education team's IA efforts. These changes restructure the page to make information easier to find, and to promote a consistent layout between reference pages.

PR Checklist

  • updated test coverage
  • external facing docs updated
  • appropriate backport labels added
  • not a security concern

@github-actions github-actions bot added the type/docs Documentation needs to be created/updated/clarified label Apr 26, 2023
@boruszak boruszak added the pr/no-changelog PR does not need a corresponding .changelog entry label Apr 26, 2023
Copy link
Contributor Author

@boruszak boruszak left a comment

Choose a reason for hiding this comment

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

Leaving questions on some specific items for verification from engineering SMEs.

Copy link
Contributor

@eddie-rowe eddie-rowe left a comment

Choose a reason for hiding this comment

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

Looks good overall to me.
Grateful for the directed comments - made it very accessible to contribute.

Copy link
Contributor

@trujillo-adam trujillo-adam left a comment

Choose a reason for hiding this comment

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

I didn't get 100% through everything but most of this looks really good. We should discuss it next week, but I think the decision to use tables for large sections of configuration parameters is, to some extent, conflicting with the format we are trying to establish. We discussed it earlier, but I was under the impression that the tables would be more the exception than the rule.

- Default: None
- Data type: Map containing one or more of the following parameters:

| Parameter | Description | Data type | Default |
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
| Parameter | Description | Data type | Default |
| Parameter | Description | Data type | Default |

I think these params are better served as their own entries in the specification. Including the reason I mentioned in another comment about the control+f workflow, it also breaks the configuration model. We tell readers to click on a parameter to view it's details and the links to these parameters don't work. We could just link them all to #routes-destination, but I think it's a little counter to the spirit of the format we're trying to establish. For lower-nested configurations, it totally makes sense to place them into a table and link to the parent param, as in the Add, Set, Remove maps in the RequestHeaders and ResponseHeaders fields. But I think this is too much of a deviation from the format.

Copy link
Contributor

@trujillo-adam trujillo-adam left a comment

Choose a reason for hiding this comment

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

Looks great! I just saw one typo and had one suggestion about linking to an example from one of the params in the resolver spec. Also, we are only using the curly braces in the HCL headings because there isn't a shorthand for maps in YAML, correct?

Co-authored-by: trujillo-adam <47586768+trujillo-adam@users.noreply.github.com>
@boruszak boruszak added pr/no-backport backport/1.15 This release series is no longer active on CE. Use backport/ent/1.15. and removed pr/no-backport labels May 17, 2023
@boruszak boruszak merged commit bd5a3c1 into main May 17, 2023
@boruszak boruszak deleted the docs/l7-config-entry-reference branch May 17, 2023 20:50
boruszak added a commit that referenced this pull request May 19, 2023
…entries (#17145)

* service-resolve configuration entry reference

* Updates

* missing backtick

* service router configuration entry reference

* link fixes + tab fixes

* link and tab fixes

* link fixes

* service resolver improvements

* hierarchy fixes

* spacing

* links + formatting

* proofing fixes

* mmore fixes

* Apply suggestions from code review

suggestions from code review for service resolver

Co-authored-by: trujillo-adam <47586768+trujillo-adam@users.noreply.github.com>

* policy sections edits

* service router code review

* Tables to sections - service router HCL

* YAML tables to sections

* formatting fixes

* converting tables to sections - service resolver

* final tables to sections

* Adjustments/alignments

* nanosecond fix

* Update website/content/docs/connect/config-entries/service-router.mdx

Co-authored-by: trujillo-adam <47586768+trujillo-adam@users.noreply.github.com>

* link to filter example config

---------

Co-authored-by: trujillo-adam <47586768+trujillo-adam@users.noreply.github.com>
boruszak added a commit that referenced this pull request May 19, 2023
…lver config entries into release/1.15.x (#17397)

* no-op commit due to failed cherry-picking

* docs: Reference pages for service-router and service-resolver config entries (#17145)

* service-resolve configuration entry reference

* Updates

* missing backtick

* service router configuration entry reference

* link fixes + tab fixes

* link and tab fixes

* link fixes

* service resolver improvements

* hierarchy fixes

* spacing

* links + formatting

* proofing fixes

* mmore fixes

* Apply suggestions from code review

suggestions from code review for service resolver

Co-authored-by: trujillo-adam <47586768+trujillo-adam@users.noreply.github.com>

* policy sections edits

* service router code review

* Tables to sections - service router HCL

* YAML tables to sections

* formatting fixes

* converting tables to sections - service resolver

* final tables to sections

* Adjustments/alignments

* nanosecond fix

* Update website/content/docs/connect/config-entries/service-router.mdx

Co-authored-by: trujillo-adam <47586768+trujillo-adam@users.noreply.github.com>

* link to filter example config

---------

Co-authored-by: trujillo-adam <47586768+trujillo-adam@users.noreply.github.com>

* Update website/content/docs/connect/config-entries/service-resolver.mdx

* merge fix

---------

Co-authored-by: temp <temp@hashicorp.com>
Co-authored-by: Jeff Boruszak <104028618+boruszak@users.noreply.github.com>
Co-authored-by: trujillo-adam <47586768+trujillo-adam@users.noreply.github.com>
Co-authored-by: boruszak <jeffrey.boruszak@hashicorp.com>
nickethier pushed a commit that referenced this pull request May 26, 2023
…entries (#17145)

* service-resolve configuration entry reference

* Updates

* missing backtick

* service router configuration entry reference

* link fixes + tab fixes

* link and tab fixes

* link fixes

* service resolver improvements

* hierarchy fixes

* spacing

* links + formatting

* proofing fixes

* mmore fixes

* Apply suggestions from code review

suggestions from code review for service resolver

Co-authored-by: trujillo-adam <47586768+trujillo-adam@users.noreply.github.com>

* policy sections edits

* service router code review

* Tables to sections - service router HCL

* YAML tables to sections

* formatting fixes

* converting tables to sections - service resolver

* final tables to sections

* Adjustments/alignments

* nanosecond fix

* Update website/content/docs/connect/config-entries/service-router.mdx

Co-authored-by: trujillo-adam <47586768+trujillo-adam@users.noreply.github.com>

* link to filter example config

---------

Co-authored-by: trujillo-adam <47586768+trujillo-adam@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.15 This release series is no longer active on CE. Use backport/ent/1.15. pr/no-changelog PR does not need a corresponding .changelog entry type/docs Documentation needs to be created/updated/clarified
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants