feat: allow user to specify domain in container labels in order to identify it#198
Conversation
|
Warning Rate limit exceeded@steveiliop56 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 3 minutes and 59 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThe changes update the label retrieval logic in the Docker integration by modifying the Changes
Sequence Diagram(s)sequenceDiagram
participant Handler
participant Docker
participant Utils
Handler->>Docker: GetLabels(id, domain)
Docker->>Docker: Log start, check connection
Docker->>Docker: Iterate containers
Docker->>Docker: Inspect container, log warnings if needed
Docker->>Utils: utils.GetLabels(container.Labels)
Utils-->>Docker: Labels (including Domain)
Docker->>Docker: Match on id or domain
Docker-->>Handler: Return labels if match, else empty
Possibly related PRs
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
internal/types/config.go (1)
103-109: Consider adding a struct-tag for the newDomainfield
Domainwill be populated exclusively throughpaerser/parser.Decode, so a tag is not strictly required.
However, for consistency with the rest of the codebase (which widely relies onmapstructure/jsontags) and to avoid surprises if the struct is re-used elsewhere, add an explicit tag, e.g.:- Domain string + Domain string `label:"domain"`This makes the intent self-documented and prevents accidental mismatches if different decoding mechanisms are introduced later.
internal/utils/utils.go (1)
204-205: Redundant path segment inparser.DecodecallAdding
"tinyauth.domain"keeps the pattern used for the other fields, so it works.
But note that the first argument"tinyauth"already covers all first-level keys below it.
Passing both"tinyauth"and"tinyauth.domain"is therefore redundant and slightly slows down the decoder.- err := parser.Decode(labels, &labelsParsed, "tinyauth", "tinyauth.users", "tinyauth.allowed", "tinyauth.headers", "tinyauth.domain", "tinyauth.oauth") + err := parser.Decode(labels, &labelsParsed, "tinyauth", "tinyauth.users", "tinyauth.allowed", "tinyauth.headers", "tinyauth.domain", "tinyauth.oauth") // keep for now, but "tinyauth.domain" could be droppedNo functional bug, but worth keeping in mind.
internal/docker/docker.go (1)
77-85: RepeatedPingon every request is expensive
DockerConnected()performs a network round-trip (/ping).
Executing it on every auth request adds latency and unnecessary load; connection state rarely changes mid-process.Consider caching the result for a short TTL or performing the connectivity test once during startup.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
internal/docker/docker.go(3 hunks)internal/handlers/handlers.go(1 hunks)internal/types/config.go(1 hunks)internal/utils/utils.go(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
internal/handlers/handlers.go (2)
internal/docker/docker.go (1)
Docker(18-21)internal/utils/utils.go (1)
GetLabels(199-214)
internal/docker/docker.go (2)
internal/utils/utils.go (1)
GetLabels(199-214)internal/types/config.go (1)
Labels(103-109)
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Refactor