Skip to content

docs/security-and-lb: illustrate topologies with images#1474

Open
hagen1778 wants to merge 4 commits into
masterfrom
docs-auth-lb-images
Open

docs/security-and-lb: illustrate topologies with images#1474
hagen1778 wants to merge 4 commits into
masterfrom
docs-auth-lb-images

Conversation

@hagen1778

@hagen1778 hagen1778 commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

This change illustrates common vmauth topologies with simple excalidraw diagrams. Some of them could have been mermaid diagrams, but more complicated topologies looked not good in mermaid. So I decided to keep it consistent within the same excalidraw style.

Follow-up after #1098 (comment)


@TomFern I request your review on this PR for the following reasons:

  1. My illustrations could be far from ideal and I believe in your skills on making them more clear for users
  2. Other examples in this doc may require similar illustrations. I only addressed required in docs: replace some mermaid charts with excalidraw #1098 (comment)
  3. I'd like you to be involved and start maintaining diagram style across the docs in VM-related projects. These excalidraw diagrams were made based on the common VM library. You can find the full reference here (requires Exc+).

This change illustrates common vmauth topologies with
simple excalidraw diagrams. Some of them could have been mermaid
diagrams, but more complicated topologies looked not good in mermaid.
So I decided to keep it consistent within the same excalidraw style.

Follow-up after #1098 (comment)

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues found across 10 files

Re-trigger cubic

@func25 func25 requested a review from vadimalekseev June 4, 2026 13:45
@TomFern

TomFern commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

@hagen1778 I'm working on the diagrams, but it will take some time. I think we should only work tidying up the images modified in this PR and open another PR if additional images need converting to excalidraw.

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.

Are we sure that every user will understand curl flags like -u and -H? 😁
Probably it is better to just use username, without password and route details, like:

frontend-logs-viewer -> vmauth --{service=frontend-logs}--> victorialogs
mobile-logs-viewer -> vmauth --{service=mobile-logs}--> victorialogs
audit-logs-viewer -> vmauth --(no filters enforced)--> victorialogs

I think the config above this diagram makes the details clear enough. The diagram itself should just show high-level routing without the extra details

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'll change the text as suggested and we can see how it looks. If we're not convinced we can change it further.

Comment thread docs/victorialogs/security-and-lb.md Outdated
Below is a diagram of this configuration:

![security-and-lb-tenants.webp](security-and-lb-tenants.webp)
![security-and-lb-tenant-assignment.webp](security-and-lb-tenant-assignment.webp)

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.

This diagram shows a tenant assignment based on request path. But the config above assigns a tenant based on Basic auth credentials.

@TomFern

TomFern commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

@hagen1778 @vadimalekseev I've updated the diagrams. The last one is the one I modified most, to keep with the general layout of the previous ones.

@hagen1778

Copy link
Copy Markdown
Contributor Author

Thanks for making images nicer @TomFern !

@valyala valyala left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@hagen1778 , @TomFern , why the *.excalidraw sources for the added images are missing in this commit? This may complicate maintenance of these images in the future when these images will require some changes.

P.S. If you use AI tools for preparing your work, please verify carefully all the output generated by AI before submitting a pull request, since AI tools still generate garbage in ~20% of cases.

Comment thread docs/victorialogs/security-and-lb-search-auth.webp Outdated
- enforce width limit for smaller pictures, to keep the scale
- remove unused images
- rename images for consistency
- update source .excalidraw to reflect changes
@hagen1778

hagen1778 commented Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

@hagen1778 , @TomFern , why the *.excalidraw sources for the added images are missing in this commit? This may complicate maintenance of these images in the future when these images will require some changes.

The source is provided as single file that contains all diagrams - see https://github.com/VictoriaMetrics/VictoriaLogs/blob/535d523c0575c900827ba757631a2d496f2c429c/docs/victorialogs/security-and-lb.excalidraw. I did this to simplify management of too many files. The source file is named after the document name, so it should be easy to locate and update if needed.

If you use AI tools for preparing your work, please verify carefully all the output generated by AI before submitting a pull request, since AI tools still generate garbage in ~20% of cases.

Sure! Thanks for bringing it! I think if I'd use AI those images would look better :) But it is just me for now.

@hagen1778

Copy link
Copy Markdown
Contributor Author

Comments were addressed. @vadimalekseev @valyala I'd appreciate one more review.

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.

4 participants