refactor: convert rootCmd from global to local variable

- Add appVersion variable to store curVersion() result during init
- Change initCLI() to return *cobra.Command
- Move rootCmd creation inside initCLI() as local variable
- Replace all rootCmd.Version usage with appVersion variable
- Update Main() function to capture returned rootCmd from initCLI()
- Remove sync.Once guard from tests and use initCLI() directly
- Remove sync import from test file as it's no longer needed

This refactoring improves encapsulation by eliminating global state,
reduces version computation overhead, and simplifies test setup by
removing the need for sync.Once guards. All tests pass and the
application builds successfully.
This commit is contained in:
Cuong Manh Le
2025-08-05 14:37:21 +07:00
committed by Cuong Manh Le
parent 4d8e10ca0d
commit b187ec98a3
6 changed files with 32 additions and 39 deletions
+18 -14
View File
@@ -61,6 +61,8 @@ var (
defaultConfigFile = "ctrld.toml" defaultConfigFile = "ctrld.toml"
rootCertPool *x509.CertPool rootCertPool *x509.CertPool
errSelfCheckNoAnswer = errors.New("no response from ctrld listener. You can try to re-launch with flag --skip_self_checks") errSelfCheckNoAnswer = errors.New("no response from ctrld listener. You can try to re-launch with flag --skip_self_checks")
// Store version once during init to avoid repeated calls to curVersion()
appVersion = curVersion()
) )
var basicModeFlags = []string{"listen", "primary_upstream", "secondary_upstream", "domains"} var basicModeFlags = []string{"listen", "primary_upstream", "secondary_upstream", "domains"}
@@ -83,15 +85,6 @@ _/ ___\ __\_ __ \ | / __ |
\/ dns forwarding proxy \/ \/ dns forwarding proxy \/
` `
var rootCmd = &cobra.Command{
Use: "ctrld",
Short: strings.TrimLeft(rootShortDesc, "\n"),
Version: curVersion(),
PersistentPreRun: func(cmd *cobra.Command, args []string) {
initConsoleLogging()
},
}
func curVersion() string { func curVersion() string {
if version != "dev" && !strings.HasPrefix(version, "v") { if version != "dev" && !strings.HasPrefix(version, "v") {
version = "v" + version version = "v" + version
@@ -105,12 +98,21 @@ func curVersion() string {
return fmt.Sprintf("%s-%s", version, commit) return fmt.Sprintf("%s-%s", version, commit)
} }
func initCLI() { func initCLI() *cobra.Command {
// Enable opening via explorer.exe on Windows. // Enable opening via explorer.exe on Windows.
// See: https://github.com/spf13/cobra/issues/844. // See: https://github.com/spf13/cobra/issues/844.
cobra.MousetrapHelpText = "" cobra.MousetrapHelpText = ""
cobra.EnableCommandSorting = false cobra.EnableCommandSorting = false
rootCmd := &cobra.Command{
Use: "ctrld",
Short: strings.TrimLeft(rootShortDesc, "\n"),
Version: appVersion,
PersistentPreRun: func(cmd *cobra.Command, args []string) {
initConsoleLogging()
},
}
rootCmd.PersistentFlags().CountVarP( rootCmd.PersistentFlags().CountVarP(
&verbose, &verbose,
"verbose", "verbose",
@@ -132,6 +134,8 @@ func initCLI() {
InitClientsCmd(rootCmd) InitClientsCmd(rootCmd)
InitUpgradeCmd(rootCmd) InitUpgradeCmd(rootCmd)
InitLogCmd(rootCmd) InitLogCmd(rootCmd)
return rootCmd
} }
// isMobile reports whether the current OS is a mobile platform. // isMobile reports whether the current OS is a mobile platform.
@@ -603,12 +607,12 @@ func processCDFlags(cfg *ctrld.Config) (*controld.ResolverConfig, error) {
bo := backoff.NewBackoff("processCDFlags", logf, 30*time.Second) bo := backoff.NewBackoff("processCDFlags", logf, 30*time.Second)
bo.LogLongerThan = 30 * time.Second bo.LogLongerThan = 30 * time.Second
ctx := ctrld.LoggerCtx(context.Background(), logger) ctx := ctrld.LoggerCtx(context.Background(), logger)
resolverConfig, err := controld.FetchResolverConfig(ctx, cdUID, rootCmd.Version, cdDev) resolverConfig, err := controld.FetchResolverConfig(ctx, cdUID, appVersion, cdDev)
for { for {
if errUrlNetworkError(err) { if errUrlNetworkError(err) {
bo.BackOff(ctx, err) bo.BackOff(ctx, err)
logger.Warn().Msg("could not fetch resolver using bootstrap DNS, retrying...") logger.Warn().Msg("could not fetch resolver using bootstrap DNS, retrying...")
resolverConfig, err = controld.FetchResolverConfig(ctx, cdUID, rootCmd.Version, cdDev) resolverConfig, err = controld.FetchResolverConfig(ctx, cdUID, appVersion, cdDev)
continue continue
} }
break break
@@ -1391,7 +1395,7 @@ func cdUIDFromProvToken() string {
req := &controld.UtilityOrgRequest{ProvToken: cdOrg, Hostname: customHostname} req := &controld.UtilityOrgRequest{ProvToken: cdOrg, Hostname: customHostname}
// Process provision token if provided. // Process provision token if provided.
loggerCtx := ctrld.LoggerCtx(context.Background(), mainLog.Load()) loggerCtx := ctrld.LoggerCtx(context.Background(), mainLog.Load())
resolverConfig, err := controld.FetchResolverUID(loggerCtx, req, rootCmd.Version, cdDev) resolverConfig, err := controld.FetchResolverUID(loggerCtx, req, appVersion, cdDev)
if err != nil { if err != nil {
mainLog.Load().Fatal().Err(err).Msgf("failed to fetch resolver uid with provision token: %s", cdOrg) mainLog.Load().Fatal().Err(err).Msgf("failed to fetch resolver uid with provision token: %s", cdOrg)
} }
@@ -1715,7 +1719,7 @@ func runningIface(s service.Service) *ifaceResponse {
// doValidateCdRemoteConfig fetches and validates custom config for cdUID. // doValidateCdRemoteConfig fetches and validates custom config for cdUID.
func doValidateCdRemoteConfig(cdUID string, fatal bool) error { func doValidateCdRemoteConfig(cdUID string, fatal bool) error {
loggerCtx := ctrld.LoggerCtx(context.Background(), mainLog.Load()) loggerCtx := ctrld.LoggerCtx(context.Background(), mainLog.Load())
rc, err := controld.FetchResolverConfig(loggerCtx, cdUID, rootCmd.Version, cdDev) rc, err := controld.FetchResolverConfig(loggerCtx, cdUID, appVersion, cdDev)
if err != nil { if err != nil {
logger := mainLog.Load().Fatal() logger := mainLog.Load().Fatal()
if !fatal { if !fatal {
+9 -20
View File
@@ -2,7 +2,6 @@ package cli
import ( import (
"bytes" "bytes"
"sync"
"testing" "testing"
"github.com/spf13/cobra" "github.com/spf13/cobra"
@@ -10,20 +9,10 @@ import (
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
) )
// setupTestCLI initializes the CLI for testing, ensuring it's only done once
var cliInitOnce sync.Once
func setupTestCLI() {
cliInitOnce.Do(func() {
initCLI()
})
}
// TestBasicCommandStructure tests the actual root command structure // TestBasicCommandStructure tests the actual root command structure
func TestBasicCommandStructure(t *testing.T) { func TestBasicCommandStructure(t *testing.T) {
// Test the actual global rootCmd that's used in the application // Test the actual root command that's returned from initCLI()
// Initialize the CLI to set up the root command rootCmd := initCLI()
setupTestCLI()
// Test that root command has basic properties // Test that root command has basic properties
assert.Equal(t, "ctrld", rootCmd.Use) assert.Equal(t, "ctrld", rootCmd.Use)
@@ -93,7 +82,7 @@ func TestServiceCommandSubCommands(t *testing.T) {
// TestCommandHelp tests basic help functionality // TestCommandHelp tests basic help functionality
func TestCommandHelp(t *testing.T) { func TestCommandHelp(t *testing.T) {
// Initialize the CLI to set up the root command // Initialize the CLI to set up the root command
setupTestCLI() rootCmd := initCLI()
// Test help command execution // Test help command execution
var buf bytes.Buffer var buf bytes.Buffer
@@ -109,7 +98,7 @@ func TestCommandHelp(t *testing.T) {
// TestCommandVersion tests version command // TestCommandVersion tests version command
func TestCommandVersion(t *testing.T) { func TestCommandVersion(t *testing.T) {
// Initialize the CLI to set up the root command // Initialize the CLI to set up the root command
setupTestCLI() rootCmd := initCLI()
var buf bytes.Buffer var buf bytes.Buffer
rootCmd.SetOut(&buf) rootCmd.SetOut(&buf)
@@ -125,7 +114,7 @@ func TestCommandVersion(t *testing.T) {
// TestCommandErrorHandling tests error handling // TestCommandErrorHandling tests error handling
func TestCommandErrorHandling(t *testing.T) { func TestCommandErrorHandling(t *testing.T) {
// Initialize the CLI to set up the root command // Initialize the CLI to set up the root command
setupTestCLI() rootCmd := initCLI()
// Test invalid flag instead of invalid command // Test invalid flag instead of invalid command
rootCmd.SetArgs([]string{"--invalid-flag"}) rootCmd.SetArgs([]string{"--invalid-flag"})
@@ -136,7 +125,7 @@ func TestCommandErrorHandling(t *testing.T) {
// TestCommandFlags tests flag functionality // TestCommandFlags tests flag functionality
func TestCommandFlags(t *testing.T) { func TestCommandFlags(t *testing.T) {
// Initialize the CLI to set up the root command // Initialize the CLI to set up the root command
setupTestCLI() rootCmd := initCLI()
// Test that root command has expected flags // Test that root command has expected flags
verboseFlag := rootCmd.PersistentFlags().Lookup("verbose") verboseFlag := rootCmd.PersistentFlags().Lookup("verbose")
@@ -151,7 +140,7 @@ func TestCommandFlags(t *testing.T) {
// TestCommandExecution tests basic command execution // TestCommandExecution tests basic command execution
func TestCommandExecution(t *testing.T) { func TestCommandExecution(t *testing.T) {
// Initialize the CLI to set up the root command // Initialize the CLI to set up the root command
setupTestCLI() rootCmd := initCLI()
// Test that root command can be executed (help command) // Test that root command can be executed (help command)
var buf bytes.Buffer var buf bytes.Buffer
@@ -167,7 +156,7 @@ func TestCommandExecution(t *testing.T) {
// TestCommandArgs tests argument handling // TestCommandArgs tests argument handling
func TestCommandArgs(t *testing.T) { func TestCommandArgs(t *testing.T) {
// Initialize the CLI to set up the root command // Initialize the CLI to set up the root command
setupTestCLI() rootCmd := initCLI()
// Test that root command can handle arguments properly // Test that root command can handle arguments properly
// Test with no args (should succeed) // Test with no args (should succeed)
@@ -183,7 +172,7 @@ func TestCommandArgs(t *testing.T) {
// TestCommandSubcommands tests subcommand functionality // TestCommandSubcommands tests subcommand functionality
func TestCommandSubcommands(t *testing.T) { func TestCommandSubcommands(t *testing.T) {
// Initialize the CLI to set up the root command // Initialize the CLI to set up the root command
setupTestCLI() rootCmd := initCLI()
// Test that root command has subcommands // Test that root command has subcommands
commands := rootCmd.Commands() commands := rootCmd.Commands()
+1 -1
View File
@@ -218,7 +218,7 @@ func (p *prog) registerControlServerHandler() {
loggerCtx := ctrld.LoggerCtx(context.Background(), p.logger.Load()) loggerCtx := ctrld.LoggerCtx(context.Background(), p.logger.Load())
// Re-fetch pin code from API. // Re-fetch pin code from API.
if rc, err := controld.FetchResolverConfig(loggerCtx, cdUID, rootCmd.Version, cdDev); rc != nil { if rc, err := controld.FetchResolverConfig(loggerCtx, cdUID, appVersion, cdDev); rc != nil {
if rc.DeactivationPin != nil { if rc.DeactivationPin != nil {
cdDeactivationPin.Store(*rc.DeactivationPin) cdDeactivationPin.Store(*rc.DeactivationPin)
} else { } else {
+1 -1
View File
@@ -1097,7 +1097,7 @@ func (p *prog) doSelfUninstall(pr *proxyResponse) {
if p.refusedQueryCount > selfUninstallMaxQueries { if p.refusedQueryCount > selfUninstallMaxQueries {
p.checkingSelfUninstall = true p.checkingSelfUninstall = true
loggerCtx := ctrld.LoggerCtx(context.Background(), p.logger.Load()) loggerCtx := ctrld.LoggerCtx(context.Background(), p.logger.Load())
_, err := controld.FetchResolverConfig(loggerCtx, cdUID, rootCmd.Version, cdDev) _, err := controld.FetchResolverConfig(loggerCtx, cdUID, appVersion, cdDev)
logger.Debug().Msg("maximum number of refused queries reached, checking device status") logger.Debug().Msg("maximum number of refused queries reached, checking device status")
selfUninstallCheck(err, p, logger) selfUninstallCheck(err, p, logger)
+1 -1
View File
@@ -60,7 +60,7 @@ func init() {
func Main() { func Main() {
ctrld.InitConfig(v, "ctrld") ctrld.InitConfig(v, "ctrld")
initCLI() rootCmd := initCLI()
if err := rootCmd.Execute(); err != nil { if err := rootCmd.Execute(); err != nil {
mainLog.Load().Error().Msg(err.Error()) mainLog.Load().Error().Msg(err.Error())
os.Exit(1) os.Exit(1)
+2 -2
View File
@@ -304,7 +304,7 @@ func (p *prog) apiConfigReload() {
doReloadApiConfig := func(forced bool, logger *ctrld.Logger) { doReloadApiConfig := func(forced bool, logger *ctrld.Logger) {
loggerCtx := ctrld.LoggerCtx(context.Background(), p.logger.Load()) loggerCtx := ctrld.LoggerCtx(context.Background(), p.logger.Load())
resolverConfig, err := controld.FetchResolverConfig(loggerCtx, cdUID, rootCmd.Version, cdDev) resolverConfig, err := controld.FetchResolverConfig(loggerCtx, cdUID, appVersion, cdDev)
selfUninstallCheck(err, p, logger) selfUninstallCheck(err, p, logger)
if err != nil { if err != nil {
logger.Warn().Err(err).Msg("could not fetch resolver config") logger.Warn().Err(err).Msg("could not fetch resolver config")
@@ -362,7 +362,7 @@ func (p *prog) apiConfigReload() {
} }
if cfgErr != nil { if cfgErr != nil {
logger.Warn().Err(err).Msg("skipping invalid custom config") logger.Warn().Err(err).Msg("skipping invalid custom config")
if _, err := controld.UpdateCustomLastFailed(loggerCtx, cdUID, rootCmd.Version, cdDev, true); err != nil { if _, err := controld.UpdateCustomLastFailed(loggerCtx, cdUID, appVersion, cdDev, true); err != nil {
logger.Error().Err(err).Msg("could not mark custom last update failed") logger.Error().Err(err).Msg("could not mark custom last update failed")
} }
return return