fix: Fix some issues with existing code

cmd/oidc-broker/daemon/config.go

@@ -37,7 +37,7 @@ func initViperConfig(name string, cmd *cobra.Command, vip *viper.Viper) (err err
 		vip.AddConfigPath("./")
 		vip.AddConfigPath("$HOME/")
 		vip.AddConfigPath("$SNAP_DATA/")
-		vip.AddConfigPath(filepath.Join("/etc", cmdName))
+		vip.AddConfigPath(filepath.Join("/etc", name))
 		// Add the executable path to the config search path.
 		if binPath, err := os.Executable(); err != nil {
 			slog.Warn(fmt.Sprintf("Failed to get current executable path, not adding it as a config dir: %v", err))

cmd/oidc-broker/daemon/daemon.go

@@ -17,24 +17,22 @@ import (
 	"github.com/ubuntu/oidc-broker/internal/daemon"
 )
 
-// cmdName is the binary name for the agent.
-var cmdName = filepath.Base(os.Args[0])
-
 // App encapsulate commands and options of the daemon, which can be controlled by env variables and config files.
 type App struct {
 	rootCmd cobra.Command
 	viper   *viper.Viper
 	config  daemonConfig
 
 	daemon *daemon.Daemon
+	name   string
 
 	ready chan struct{}
 }
 
 // only overriable for tests.
 type systemPaths struct {
-	BrokersConf string
-	Cache       string
+	BrokerConf string
+	Cache      string
 }
 
 // daemonConfig defines configuration parameters of the daemon.
@@ -44,31 +42,31 @@ type daemonConfig struct {
 }
 
 // New registers commands and return a new App.
-func New() *App {
-	a := App{ready: make(chan struct{})}
+func New(name string) *App {
+	a := App{ready: make(chan struct{}), name: name}
 	a.rootCmd = cobra.Command{
-		Use:   fmt.Sprintf("%s COMMAND", cmdName),
-		Short: fmt.Sprintf("%s authentication broker", cmdName),
-		Long:  fmt.Sprintf("Authentication daemon %s to communicate with our authentication daemon.", cmdName),
+		Use:   fmt.Sprintf("%s COMMAND", name),
+		Short: fmt.Sprintf("%s authentication broker", name),
+		Long:  fmt.Sprintf("Authentication daemon %s to communicate with our authentication daemon.", name),
 		Args:  cobra.NoArgs,
 		PersistentPreRunE: func(cmd *cobra.Command, args []string) error {
 			// Command parsing has been successful. Returns to not print usage anymore.
 			a.rootCmd.SilenceUsage = true
 
 			// Set config defaults
-			systemCache := filepath.Join("/var", "lib", cmdName)
+			systemCache := filepath.Join("/var", "lib", name)
 			if snapData := os.Getenv("SNAP_DATA"); snapData != "" {
 				systemCache = filepath.Join(snapData, "cache")
 			}
 			a.config = daemonConfig{
 				Paths: systemPaths{
-					BrokersConf: filepath.Join(consts.DefaultBrokersConfPath, cmdName),
-					Cache:       systemCache,
+					BrokerConf: filepath.Join(consts.DefaultBrokersConfPath, name),
+					Cache:      systemCache,
 				},
 			}
 
 			// Install and unmarshall configuration
-			if err := initViperConfig(cmdName, &a.rootCmd, a.viper); err != nil {
+			if err := initViperConfig(name, &a.rootCmd, a.viper); err != nil {
 				return err
 			}
 			if err := a.viper.Unmarshal(&a.config); err != nil {
@@ -108,7 +106,7 @@ func (a *App) serve(config daemonConfig) error {
 		return fmt.Errorf("error initializing users cache directory at %q: %v", config.Paths.Cache, err)
 	}
 
-	s, err := brokerservice.New(ctx, cmdName)
+	s, err := brokerservice.New(ctx, a.name)
 	if err != nil {
 		close(a.ready)
 		return err

cmd/oidc-broker/daemon/daemon_test.go

@@ -48,9 +48,7 @@ func TestVersion(t *testing.T) {
 	fields := strings.Fields(out)
 	require.Len(t, fields, 2, "wrong number of fields in version: %s", out)
 
-	want := "authd"
-
-	require.Equal(t, want, fields[0], "Wrong executable name")
+	require.Equal(t, t.Name(), fields[0], "Wrong executable name")
 	require.Equal(t, consts.Version, fields[1], "Wrong version")
 }
 
@@ -224,8 +222,7 @@ func TestAutoDetectConfig(t *testing.T) {
 	// Remove configuration next binary for other tests to not pick it up.
 	defer os.Remove(configNextToBinaryPath)
 
-	a := daemon.New()
-
+	a := daemon.New(t.Name())
 	wg := sync.WaitGroup{}
 	wg.Add(1)
 	go func() {
@@ -248,21 +245,19 @@ func TestNoConfigSetDefaults(t *testing.T) {
 	// TODO
 	t.Fail()
 
-	a := daemon.New()
-	// Use version to still run preExec to load no config but without running server
+	a := daemon.New(t.Name()) // Use version to still run preExec to load no config but without running server
 	a.SetArgs("version")
 
 	err := a.Run()
 	require.NoError(t, err, "Run should not return an error")
 
 	require.Equal(t, 0, a.Config().Verbosity, "Default Verbosity")
-	require.Equal(t, consts.DefaultBrokersConfPath, a.Config().Paths.BrokersConf, "Default brokers configuration path")
+	require.Equal(t, filepath.Join(consts.DefaultBrokersConfPath, t.Name()), a.Config().Paths.BrokerConf, "Default broker configuration path")
 	//require.Equal(t, consts.DefaultCacheDir, a.Config().Paths.Cache, "Default cache directory")
 }
 
 func TestBadConfigReturnsError(t *testing.T) {
-	a := daemon.New()
-	// Use version to still run preExec to load no config but without running server
+	a := daemon.New(t.Name()) // Use version to still run preExec to load no config but without running server
 	a.SetArgs("version", "--config", "/does/not/exist.yaml")
 
 	err := a.Run()

cmd/oidc-broker/daemon/export_test.go

@@ -21,7 +21,7 @@ func NewForTests(t *testing.T, conf *DaemonConfig, args ...string) *App {
 	argsWithConf := []string{"--config", p}
 	argsWithConf = append(argsWithConf, args...)
 
-	a := New()
+	a := New(t.Name())
 	a.rootCmd.SetArgs(argsWithConf)
 	return a
 }

cmd/oidc-broker/daemon/version.go

@@ -12,13 +12,13 @@ func (a *App) installVersion() {
 		Use:                                                     "version",
 		Short:/*i18n.G(*/ "Returns version of daemon and exits", /*)*/
 		Args:                                                    cobra.NoArgs,
-		RunE:                                                    func(cmd *cobra.Command, args []string) error { return getVersion() },
+		RunE:                                                    func(cmd *cobra.Command, args []string) error { return a.getVersion() },
 	}
 	a.rootCmd.AddCommand(cmd)
 }
 
 // getVersion returns the current service version.
-func getVersion() (err error) {
-	fmt.Printf( /*i18n.G(*/ "%s\t%s" /*)*/ +"\n", cmdName, consts.Version)
+func (a *App) getVersion() (err error) {
+	fmt.Printf( /*i18n.G(*/ "%s\t%s" /*)*/ +"\n", a.name, consts.Version)
 	return nil
 }

cmd/oidc-broker/main.go

@@ -5,6 +5,7 @@ import (
 	"log/slog"
 	"os"
 	"os/signal"
+	"path/filepath"
 	"sync"
 	"syscall"
 
@@ -20,7 +21,7 @@ import (
 
 func main() {
 	i18n.InitI18nDomain(consts.TEXTDOMAIN, po.Files)
-	a := daemon.New()
+	a := daemon.New(filepath.Base(os.Args[0]))
 	os.Exit(run(a))
 }
 

internal/brokerservice/brokerservice.go

@@ -107,7 +107,12 @@ func (s *Service) Serve() error {
 
 // Stop stop the service and do all the necessary cleanup operation.
 func (s *Service) Stop() error {
-	close(s.serve)
-	s.disconnect()
+	// Check if already stopped.
+	select {
+	case <-s.serve:
+	default:
+		close(s.serve)
+		s.disconnect()
+	}
 	return nil
 }