Display the interpolated domain for DNS setup from service info#2139
Conversation
📝 WalkthroughWalkthroughCertificate generation now prefers deployed service domain names over compose file definitions. When a deployed service lacks a domain, cert generation is skipped with a warning. Subsequent domain expansion and certificate issuance use this corrected domain base. ChangesDomain Selection in Cert Generation
Estimated Code Review Effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=warning msg="The linter 'gomodguard' is deprecated (since v2.12.0) due to: new major version. Replaced by gomodguard_v2." Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/pkg/cli/cert.go (1)
116-119: 💤 Low valueConsider clarifying the warning message wording.
The message "domainname is deployed without a domainname" is slightly redundant. Consider simplifying to make the intent clearer.
📝 Suggested rewording
- term.Warnf("service %q: `domainname` is deployed without a domainname, skipping cert generation", service.Name) + term.Warnf("service %q: deployed without a domainname, skipping cert generation", service.Name)or alternatively:
- term.Warnf("service %q: `domainname` is deployed without a domainname, skipping cert generation", service.Name) + term.Warnf("service %q: no domainname in deployed service, skipping cert generation", service.Name)🤖 Prompt for 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. In `@src/pkg/cli/cert.go` around lines 116 - 119, The warning message in the loop that checks serviceInfo.Domainname (where serviceInfo.Domainname == "" and service.Name is used) is redundant and should be reworded; update the term.Warnf call to a clearer message such as "service %q: no domainname set, skipping certificate generation" or "service %q: domain not configured, skipping cert generation" so it references service.Name and clearly explains why certificate generation is skipped.
🤖 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.
Nitpick comments:
In `@src/pkg/cli/cert.go`:
- Around line 116-119: The warning message in the loop that checks
serviceInfo.Domainname (where serviceInfo.Domainname == "" and service.Name is
used) is redundant and should be reworded; update the term.Warnf call to a
clearer message such as "service %q: no domainname set, skipping certificate
generation" or "service %q: domain not configured, skipping cert generation" so
it references service.Name and clearly explains why certificate generation is
skipped.
4 participants
Description
Right now the DNS instruction displays the raw value of domainname from compose file, which main contain uninterpreted variables, making the instruction invalid, we should display the actual domain name returned from serviceinfo.
Checklist
Summary by CodeRabbit