fix: fixed domain mapping to return to treafik#112
Conversation
|
@Pieterv24 is attempting to deploy a commit to the HHF Technologies' projects Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📜 Recent review details
|
| Layer / File(s) | Summary |
|---|---|
Ordered TLS domain normalization services/config_proxy.go |
OrderedTLSConfig.Domains accepts []interface{}, and TLS normalization retains string and object domain entries without filtering. |
Estimated code review effort: 2 (Simple) | ~10 minutes
Poem
I’m a rabbit with domains in a row,
Strings and objects now safely flow.
No entry gets lost in the TLS night,
Ordered and preserved just right.
Hop, hop—config takes flight!
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title is related to the main change: preserving and mapping TLS domains for Traefik. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
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.
Comment @coderabbitai help to get the list of available commands.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request changes the Domains field in OrderedTLSConfig from []string to []interface{} to allow preserving the original format of domains. However, removing the []string case in the type switch will cause any []string values to be silently ignored. It is recommended to handle the []string case by explicitly converting it to []interface{} to avoid potential configuration loss.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| switch v := domains.(type) { | ||
| case []interface{}: | ||
| for _, d := range v { | ||
| if s, ok := d.(string); ok { | ||
| ordered.Domains = append(ordered.Domains, s) | ||
| } | ||
| } | ||
| case []string: | ||
| // Preserve original format as-is (strings or objects) | ||
| ordered.Domains = v | ||
| } |
There was a problem hiding this comment.
By removing the case []string: block, any domains slice of type []string (which can occur in unit tests or when constructed programmatically) will be silently ignored because it does not match case []interface{}:. In Go, []string cannot be directly assigned to []interface{}, so we must explicitly convert it to preserve the original behavior and prevent potential configuration loss.
| switch v := domains.(type) { | |
| case []interface{}: | |
| for _, d := range v { | |
| if s, ok := d.(string); ok { | |
| ordered.Domains = append(ordered.Domains, s) | |
| } | |
| } | |
| case []string: | |
| // Preserve original format as-is (strings or objects) | |
| ordered.Domains = v | |
| } | |
| switch v := domains.(type) { | |
| case []interface{}: | |
| // Preserve original format as-is (strings or objects) | |
| ordered.Domains = v | |
| case []string: | |
| ordered.Domains = make([]interface{}, len(v)) | |
| for i, s := range v { | |
| ordered.Domains[i] = s | |
| } | |
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@services/config_proxy.go`:
- Around line 1261-1266: Update mapToOrderedTLS so the tls["domains"] handling
preserves []map[string]interface{} and []string in addition to []interface{},
converting each typed slice into the expected ordered domain representation
without altering its entries. Add a regression test covering generated
object-shaped domains from config_generator.go.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 48871765-16e1-4adc-826e-da62d0c99168
📒 Files selected for processing (1)
services/config_proxy.go
📜 Review details
⚠️ CI failures not shown inline (1)
Commit Status: Vercel: Vercel
Conclusion: failure
Authorization required to deploy.
🔇 Additional comments (1)
services/config_proxy.go (1)
67-69: LGTM!
1 participant
The domains object in the TLS field of routers is not being passed though to traefik correctly.
This messes with the ability of Traefik to create wildcard domains.
Response from pangolin (expected from MM too):
{ "http": { "middlewares": { ... }, "routers": { "8-example-router": { "entryPoints": [ "websecure" ], "middlewares": [ "badger" ], "service": "8-example-service", "rule": "Host(`target.example.com`)", "priority": 100, "tls": { "certResolver": "letsencrypt", "domains": [ { "main": "*.example.com" } ] } }, }, "services": { ... } }Response from Middleware Manager:
{ "http": { "middlewares": { ... }, "routers": { "8-example-router": { "entryPoints": [ "websecure" ], "middlewares": [ "badger" ], "service": "8-example-service", "rule": "Host(`target.example.com`)", "priority": 100, "tls": { "certResolver": "letsencrypt" } }, }, "services": { ... } }This PR should address the issue.
Summary by CodeRabbit