refactor: make cli modular#390
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 1 minutes and 20 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)
WalkthroughReworks CLI orchestration to a struct-based rootCmd with a Run entrypoint, adds top-level create/verify/generate commands, replaces the version command with a struct wrapper, and removes legacy nested Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant OS as OS
participant Main as main.go
participant Root as rootCmd.Run()
participant Viper as Viper
participant Cobra as Cobra
participant Sub as Subcommand.Register()
OS->>Main: start
Main->>Root: Run()
activate Root
Root->>Viper: instantiate/load config, bind env
Root->>Cobra: build root cobra.Command
Root->>Sub: Register() each subcommand (create/verify/generate/version)
Sub-->>Root: attach subcommands
Root->>Cobra: Execute()
deactivate Root
sequenceDiagram
autonumber
participant User as Operator
participant CLI as create command
participant Form as huh form (optional)
participant Bcrypt as bcrypt
participant Log as zerolog
User->>CLI: invoke create (flags or -i)
alt interactive
CLI->>Form: prompt username/password/docker
Form-->>CLI: inputs
end
CLI->>CLI: validate username/password
CLI->>Bcrypt: GenerateFromPassword(password)
Bcrypt-->>CLI: hash / error
alt docker output
CLI->>CLI: escape `$` in hash
end
CLI->>Log: emit created user info
sequenceDiagram
autonumber
participant User as Operator
participant CLI as verify command
participant Parse as utils.ParseUser
participant Bcrypt as bcrypt
participant TOTP as totp validation
participant Log as zerolog
User->>CLI: invoke verify (flags or -i)
alt interactive
CLI->>CLI: collect inputs
end
CLI->>Parse: ParseUser(userString)
Parse-->>CLI: {username, hash, totpSecret}
CLI->>CLI: compare usernames
CLI->>Bcrypt: CompareHashAndPassword(hash, password)
Bcrypt-->>CLI: ok / fail
alt totpSecret present
CLI->>TOTP: validate(code, secret)
TOTP-->>CLI: ok / fail
end
CLI->>Log: emit verification result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #390 +/- ##
==========================================
- Coverage 25.43% 24.66% -0.78%
==========================================
Files 36 34 -2
Lines 2587 2668 +81
==========================================
Hits 658 658
- Misses 1893 1974 +81
Partials 36 36 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![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 (1)
cmd/root.go (1)
128-137: Consider consistent command creation pattern.The
userCmdandtotpCmdare created inline with plain&cobra.Command{}literals, while the root command uses a structured type withnewRootCmd(). This is functionally correct (these are simple parent commands without run logic), but for consistency you might consider:- userCmd := &cobra.Command{ - Use: "user", - Short: "User utilities", - Long: `Utilities for creating and verifying tinyauth compatible users.`, - } - totpCmd := &cobra.Command{ - Use: "totp", - Short: "Totp utilities", - Long: `Utilities for creating and verifying totp codes.`, - } + userCmd := newUserCmd() + totpCmd := newTotpCmd()This would require adding simple wrapper functions, but it unifies the pattern and makes future extensions (e.g., adding a run function to these commands) easier.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
cmd/create.go(1 hunks)cmd/generate.go(1 hunks)cmd/root.go(2 hunks)cmd/totp/generate/generate.go(0 hunks)cmd/totp/totp.go(0 hunks)cmd/user/create/create.go(0 hunks)cmd/user/user.go(0 hunks)cmd/user/verify/verify.go(0 hunks)cmd/verify.go(1 hunks)cmd/version.go(1 hunks)main.go(1 hunks)
💤 Files with no reviewable changes (5)
- cmd/user/verify/verify.go
- cmd/totp/totp.go
- cmd/user/user.go
- cmd/user/create/create.go
- cmd/totp/generate/generate.go
🧰 Additional context used
🧬 Code graph analysis (6)
cmd/version.go (2)
cmd/root.go (1)
Run(123-152)internal/config/config.go (3)
Version(5-5)CommitHash(6-6)BuildTimestamp(7-7)
cmd/verify.go (2)
cmd/root.go (1)
Run(123-152)internal/utils/user_utils.go (1)
ParseUser(63-92)
cmd/root.go (3)
internal/config/config.go (2)
Config(17-44)Version(5-5)internal/utils/app_utils.go (1)
GetLogLevel(116-135)internal/bootstrap/app_bootstrap.go (1)
NewBootstrapApp(40-44)
cmd/create.go (1)
cmd/root.go (1)
Run(123-152)
cmd/generate.go (2)
cmd/root.go (1)
Run(123-152)internal/utils/user_utils.go (1)
ParseUser(63-92)
main.go (1)
cmd/root.go (1)
Run(123-152)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (go)
🔇 Additional comments (1)
cmd/root.go (1)
98-121: LGTM!The config loading, validation, and bootstrap logic is well-structured. Error handling with
log.Fatal()is appropriate for a CLI application.
Co-authored-by: Ryc O'Chet <Rycochet@users.noreply.github.com>
| huh.NewInput().Title("User (username:hash:totp)").Value(&c.user).Validate((func(s string) error { | ||
| if s == "" { | ||
| return errors.New("user cannot be empty") | ||
| } | ||
| return nil | ||
| })), | ||
| huh.NewInput().Title("Username").Value(&c.username).Validate((func(s string) error { | ||
| if s == "" { | ||
| return errors.New("username cannot be empty") | ||
| } | ||
| return nil | ||
| })), | ||
| huh.NewInput().Title("Password").Value(&c.password).Validate((func(s string) error { | ||
| if s == "" { | ||
| return errors.New("password cannot be empty") | ||
| } | ||
| return nil | ||
| })), |
There was a problem hiding this comment.
It's the user as in user1:foo, the username is the user1 and the password is your password in plain text.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
cmd/root.go (1)
16-21: Remove unusedrootfield and dead conditional.
root *cobra.Commandis never initialized; Lines 89-91 never run. Drop both for clarity.type rootCmd struct { - root *cobra.Command cmd *cobra.Command viper *viper.Viper } @@ - if c.root != nil { - c.root.AddCommand(c.cmd) - }Also applies to: 89-91
🧹 Nitpick comments (5)
cmd/root.go (5)
84-85: Handle Viper binding errors.Binding env and flags can fail; fail fast with context.
- c.viper.BindEnv(opt.name, envVar) + if err := c.viper.BindEnv(opt.name, envVar); err != nil { + log.Fatal().Err(err).Str("key", opt.name).Str("env", envVar).Msg("Failed to bind env var") + } @@ - c.viper.BindPFlags(c.cmd.Flags()) + if err := c.viper.BindPFlags(c.cmd.Flags()); err != nil { + log.Fatal().Err(err).Msg("Failed to bind flags") + }Also applies to: 87-87
30-35: Silence Cobra’s usage/errors on runtime failures.Prevents duplicate help/error output on failures; keeps logs clean.
c.cmd = &cobra.Command{ Use: "tinyauth", Short: "The simplest way to protect your apps with a login screen", Long: `Tinyauth is a simple authentication middleware that adds a simple login screen or OAuth with Google, Github or any other provider to all of your docker apps.`, Run: c.run, + SilenceUsage: true, + SilenceErrors: true, }
37-38: Avoid redundant env strategies or add a key replacer.You currently use both AutomaticEnv and per‑key BindEnv. Consider one:
- Keep BindEnv per key and drop AutomaticEnv, or
- Drop per‑key BindEnv and add a key replacer so AutomaticEnv maps dashes to underscores:
c.viper.SetEnvKeyReplacer(strings.NewReplacer("-", "_")) c.viper.AutomaticEnv()Either choice reduces duplication and surprise.
Also applies to: 82-85
112-114: Set zerolog global level and avoid redundant cast.Ensure all loggers respect the level, not only
log.Logger.- log.Logger = log.Level(zerolog.Level(utils.GetLogLevel(conf.LogLevel))) + level := utils.GetLogLevel(conf.LogLevel) + zerolog.SetGlobalLevel(level) + log.Logger = log.Level(level)
134-136: Capitalize TOTP in help strings.Minor UX polish.
totpCmd := &cobra.Command{ Use: "totp", - Short: "Totp utilities", - Long: `Utilities for creating and verifying totp codes.`, + Short: "TOTP utilities", + Long: `Utilities for creating and verifying TOTP codes.`, }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cmd/root.go(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
cmd/root.go (3)
internal/config/config.go (2)
Config(17-44)Version(5-5)internal/utils/app_utils.go (1)
GetLogLevel(116-135)internal/bootstrap/app_bootstrap.go (1)
NewBootstrapApp(40-44)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (go)
🔇 Additional comments (1)
cmd/root.go (1)
106-110:ip4_addrtag is supported by validator/v10
go-playground/validator v10 includes bothipv4(simple format) andip4_addr(resolvable endpoint) validators, so your use ofip4_addris valid.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
cmd/create.go (1)
85-85: Password logging issue has been resolved.The previous concern about logging the plaintext password has been addressed. The log line now only includes the username, which is appropriate.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
cmd/create.go(1 hunks)cmd/verify.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
cmd/verify.go (2)
cmd/root.go (1)
Run(123-152)internal/utils/user_utils.go (1)
ParseUser(63-92)
cmd/create.go (1)
cmd/root.go (1)
Run(123-152)
2 participants
Summary by CodeRabbit