ref(config): deprecate old config variables

Ayman Bagabas created

Split config environment variables

Change summary

.gitignore                       |  5 -
cmd/soft/serve.go                |  3 -
config/config.go                 |  6 +-
config/config_test.go            |  6 -
examples/setuid/main.go          |  6 +-
server/cmd/middleware_test.go    |  9 ++-
server/config/config.go          | 76 ++++++++++++++++++++++++++-------
server/git/daemon/daemon.go      | 18 ++++---
server/git/daemon/daemon_test.go | 25 +++++++----
server/server.go                 | 10 ++--
server/server_test.go            | 14 +++---
server/session_test.go           |  7 +-
12 files changed, 115 insertions(+), 70 deletions(-)

Detailed changes

.gitignore 🔗

@@ -1,6 +1,5 @@
-cmd/soft/soft
-.ssh
-.repos
+soft
+soft-serve
 dist
 testdata
 completions/

cmd/soft/serve.go 🔗

@@ -2,7 +2,6 @@ package main
 
 import (
 	"context"
-	"log"
 	"os"
 	"os/signal"
 	"syscall"
@@ -23,8 +22,6 @@ var (
 			cfg := config.DefaultConfig()
 			s := server.NewServer(cfg)
 
-			log.Printf("Starting SSH server on %s:%d", cfg.BindAddr, cfg.Port)
-
 			done := make(chan os.Signal, 1)
 			lch := make(chan error, 1)
 			go func() {

config/config.go 🔗

@@ -75,7 +75,7 @@ func NewConfig(cfg *config.Config) (*Config, error) {
 	var yamlUsers string
 	var displayHost string
 	host := cfg.Host
-	port := cfg.Port
+	port := cfg.SSH.Port
 
 	pks := make([]string, 0)
 	for _, k := range cfg.InitialAdminKeys {
@@ -94,7 +94,7 @@ func NewConfig(cfg *config.Config) (*Config, error) {
 		pks = append(pks, pk)
 	}
 
-	rs := NewRepoSource(cfg.RepoPath)
+	rs := NewRepoSource(cfg.RepoPath())
 	c := &Config{
 		Cfg: cfg,
 	}
@@ -259,7 +259,7 @@ func createFile(path string, content string) error {
 
 func (cfg *Config) createDefaultConfigRepo(yaml string) error {
 	cn := defaultConfigRepo
-	rp := filepath.Join(cfg.Cfg.RepoPath, cn) + ".git"
+	rp := filepath.Join(cfg.Cfg.RepoPath(), cn) + ".git"
 	rs := cfg.Source
 	err := rs.LoadRepo(cn)
 	if errors.Is(err, fs.ErrNotExist) {

config/config_test.go 🔗

@@ -9,8 +9,7 @@ import (
 
 func TestMultipleInitialKeys(t *testing.T) {
 	cfg, err := NewConfig(&config.Config{
-		RepoPath: t.TempDir(),
-		KeyPath:  t.TempDir(),
+		DataPath: t.TempDir(),
 		InitialAdminKeys: []string{
 			"testdata/k1.pub",
 			"ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIFxIobhwtfdwN7m1TFt9wx3PsfvcAkISGPxmbmbauST8 a@b",
@@ -28,8 +27,7 @@ func TestMultipleInitialKeys(t *testing.T) {
 
 func TestEmptyInitialKeys(t *testing.T) {
 	cfg, err := NewConfig(&config.Config{
-		RepoPath: t.TempDir(),
-		KeyPath:  t.TempDir(),
+		DataPath: t.TempDir(),
 	})
 	is := is.New(t)
 	is.NoErr(err)

examples/setuid/main.go 🔗

@@ -44,13 +44,13 @@ func main() {
 		log.Fatalf("Setuid error: %s", err)
 	}
 	cfg := config.DefaultConfig()
-	cfg.Port = *port
+	cfg.SSH.Port = *port
 	s := server.NewServer(cfg)
 
 	done := make(chan os.Signal, 1)
 	signal.Notify(done, os.Interrupt, syscall.SIGINT, syscall.SIGTERM)
 
-	log.Printf("Starting SSH server on %s:%d", cfg.BindAddr, cfg.Port)
+	log.Printf("Starting SSH server on %s:%d", cfg.Host, cfg.SSH.Port)
 	go func() {
 		if err := s.SSHServer.Serve(ls); err != nil {
 			log.Fatalln(err)
@@ -59,7 +59,7 @@ func main() {
 
 	<-done
 
-	log.Printf("Stopping SSH server on %s:%d", cfg.BindAddr, cfg.Port)
+	log.Printf("Stopping SSH server on %s:%d", cfg.Host, cfg.SSH.Port)
 	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
 	defer func() { cancel() }()
 	if err := s.Shutdown(ctx); err != nil {

server/cmd/middleware_test.go 🔗

@@ -19,10 +19,11 @@ func TestMiddleware(t *testing.T) {
 	})
 	is := is.New(t)
 	appCfg, err := config.NewConfig(&sconfig.Config{
-		Host:     "localhost",
-		Port:     22223,
-		RepoPath: "testmiddleware/repos",
-		KeyPath:  "testmiddleware/key",
+		Host: "localhost",
+		SSH: sconfig.SSHConfig{
+			Port: 22223,
+		},
+		DataPath: "testmiddleware",
 	})
 	is.NoErr(err)
 	_ = testsession.New(t, &ssh.Server{

server/config/config.go 🔗

@@ -14,33 +14,75 @@ type Callbacks interface {
 	Fetch(repo string)
 }
 
+// SSHConfig is the SSH configuration for the server.
+type SSHConfig struct {
+	Port int `env:"PORT" envDefault:"23231"`
+}
+
+// GitConfig is the Git protocol configuration for the server.
+type GitConfig struct {
+	Port       int `env:"PORT" envDefault:"9418"`
+	MaxTimeout int `env:"MAX_TIMEOUT" envDefault:"300"`
+	// MaxReadTimeout is the maximum time a client can take to send a request.
+	MaxReadTimeout int `env:"MAX_READ_TIMEOUT" envDefault:"3"`
+	MaxConnections int `env:"SOFT_SERVE_GIT_MAX_CONNECTIONS" envDefault:"32"`
+}
+
 // Config is the configuration for Soft Serve.
 type Config struct {
-	BindAddr      string `env:"SOFT_SERVE_BIND_ADDRESS" envDefault:""`
-	Host          string `env:"SOFT_SERVE_HOST" envDefault:"localhost"`
-	Port          int    `env:"SOFT_SERVE_PORT" envDefault:"23231"`
-	GitPort       int    `env:"SOFT_SERVE_GIT_PORT" envDefault:"9418"`
-	GitMaxTimeout int    `env:"SOFT_SERVE_GIT_MAX_TIMEOUT" envDefault:"300"`
-	// MaxReadTimeout is the maximum time a client can take to send a request.
-	GitMaxReadTimeout int      `env:"SOFT_SERVE_GIT_MAX_READ_TIMEOUT" envDefault:"3"`
-	GitMaxConnections int      `env:"SOFT_SERVE_GIT_MAX_CONNECTIONS" envDefault:"32"`
-	KeyPath           string   `env:"SOFT_SERVE_KEY_PATH"`
-	RepoPath          string   `env:"SOFT_SERVE_REPO_PATH" envDefault:".repos"`
-	InitialAdminKeys  []string `env:"SOFT_SERVE_INITIAL_ADMIN_KEY" envSeparator:"\n"`
-	Callbacks         Callbacks
-	ErrorLog          *log.Logger
+	Host string    `env:"HOST" envDefault:"localhost"`
+	SSH  SSHConfig `env:"SSH" envPrefix:"SSH_"`
+	Git  GitConfig `env:"GIT" envPrefix:"GIT_"`
+
+	DataPath string `env:"DATA_PATH" envDefault:"soft-serve"`
+
+	// Deprecated: use SOFT_SERVE_SSH_PORT instead.
+	Port int `env:"PORT"`
+	// Deprecated: use DataPath instead.
+	KeyPath string `env:"KEY_PATH"`
+	// Deprecated: use DataPath instead.
+	ReposPath string `env:"REPO_PATH"`
+
+	InitialAdminKeys []string `env:"INITIAL_ADMIN_KEY" envSeparator:"\n"`
+	Callbacks        Callbacks
+	ErrorLog         *log.Logger
+}
+
+// RepoPath returns the path to the repositories.
+func (c *Config) RepoPath() string {
+	if c.ReposPath != "" {
+		log.Printf("warning: SOFT_SERVE_REPO_PATH is deprecated, use SOFT_SERVE_DATA_PATH instead")
+		return c.ReposPath
+	}
+	return filepath.Join(c.DataPath, "repos")
+}
+
+// SSHPath returns the path to the SSH directory.
+func (c *Config) SSHPath() string {
+	return filepath.Join(c.DataPath, "ssh")
+}
+
+// PrivateKeyPath returns the path to the SSH key.
+func (c *Config) PrivateKeyPath() string {
+	if c.KeyPath != "" {
+		log.Printf("warning: SOFT_SERVE_KEY_PATH is deprecated, use SOFT_SERVE_DATA_PATH instead")
+		return c.KeyPath
+	}
+	return filepath.Join(c.DataPath, "ssh", "soft_serve")
 }
 
 // DefaultConfig returns a Config with the values populated with the defaults
 // or specified environment variables.
 func DefaultConfig() *Config {
 	cfg := &Config{ErrorLog: log.Default()}
-	if err := env.Parse(cfg); err != nil {
+	if err := env.Parse(cfg, env.Options{
+		Prefix: "SOFT_SERVE_",
+	}); err != nil {
 		log.Fatalln(err)
 	}
-	if cfg.KeyPath == "" {
-		// NB: cross-platform-compatible path
-		cfg.KeyPath = filepath.Join(".ssh", "soft_serve_server_ed25519")
+	if cfg.Port != 0 {
+		log.Printf("warning: SOFT_SERVE_PORT is deprecated, use SOFT_SERVE_SSH_PORT instead")
+		cfg.SSH.Port = cfg.Port
 	}
 	return cfg.WithCallbacks(nil)
 }

server/git/daemon/daemon.go 🔗

@@ -29,11 +29,12 @@ type Daemon struct {
 	conns    map[net.Conn]struct{}
 	cfg      *config.Config
 	wg       sync.WaitGroup
+	once     sync.Once
 }
 
 // NewDaemon returns a new Git daemon.
 func NewDaemon(cfg *config.Config, auth git.Hooks) (*Daemon, error) {
-	addr := fmt.Sprintf("%s:%d", cfg.BindAddr, cfg.GitPort)
+	addr := fmt.Sprintf("%s:%d", cfg.Host, cfg.Git.Port)
 	d := &Daemon{
 		addr:  addr,
 		auth:  auth,
@@ -53,7 +54,7 @@ func NewDaemon(cfg *config.Config, auth git.Hooks) (*Daemon, error) {
 // Start starts the Git TCP daemon.
 func (d *Daemon) Start() error {
 	// set up channel on which to send accepted connections
-	listen := make(chan net.Conn, d.cfg.GitMaxConnections)
+	listen := make(chan net.Conn, d.cfg.Git.MaxConnections)
 	go d.acceptConnection(d.listener, listen)
 
 	// loop work cycle with accept connections or interrupt
@@ -107,14 +108,14 @@ func (d *Daemon) handleClient(c net.Conn) {
 	defer delete(d.conns, c)
 
 	// Close connection if there are too many open connections.
-	if len(d.conns) >= d.cfg.GitMaxConnections {
+	if len(d.conns) >= d.cfg.Git.MaxConnections {
 		log.Printf("git: max connections reached, closing %s", c.RemoteAddr())
 		fatal(c, git.ErrMaxConns)
 		return
 	}
 
 	// Set connection timeout.
-	if err := c.SetDeadline(time.Now().Add(time.Duration(d.cfg.GitMaxTimeout) * time.Second)); err != nil {
+	if err := c.SetDeadline(time.Now().Add(time.Duration(d.cfg.Git.MaxTimeout) * time.Second)); err != nil {
 		log.Printf("git: error setting deadline: %v", err)
 		fatal(c, git.ErrSystemMalfunction)
 		return
@@ -123,7 +124,7 @@ func (d *Daemon) handleClient(c net.Conn) {
 	readc := make(chan struct{}, 1)
 	go func() {
 		select {
-		case <-time.After(time.Duration(d.cfg.GitMaxReadTimeout) * time.Second):
+		case <-time.After(time.Duration(d.cfg.Git.MaxReadTimeout) * time.Second):
 			log.Printf("git: read timeout from %s", c.RemoteAddr())
 			fatal(c, git.ErrMaxTimeout)
 		case <-readc:
@@ -168,11 +169,11 @@ func (d *Daemon) handleClient(c net.Conn) {
 		repo += ".git"
 	}
 
-	err := git.GitPack(c, c, c, cmd, d.cfg.RepoPath, repo)
+	err := git.GitPack(c, c, c, cmd, d.cfg.RepoPath(), repo)
 	if err == git.ErrInvalidRepo {
 		trimmed := strings.TrimSuffix(repo, ".git")
 		log.Printf("git: invalid repo %q trying again %q", repo, trimmed)
-		err = git.GitPack(c, c, c, cmd, d.cfg.RepoPath, trimmed)
+		err = git.GitPack(c, c, c, cmd, d.cfg.RepoPath(), trimmed)
 	}
 	if err != nil {
 		fatal(c, err)
@@ -182,12 +183,13 @@ func (d *Daemon) handleClient(c net.Conn) {
 
 // Close closes the underlying listener.
 func (d *Daemon) Close() error {
+	d.once.Do(func() { close(d.exit) })
 	return d.listener.Close()
 }
 
 // Shutdown gracefully shuts down the daemon.
 func (d *Daemon) Shutdown(_ context.Context) error {
-	close(d.exit)
+	d.once.Do(func() { close(d.exit) })
 	d.wg.Wait()
 	return nil
 }

server/git/daemon/daemon_test.go 🔗

@@ -2,7 +2,6 @@ package daemon
 
 import (
 	"bytes"
-	"context"
 	"io"
 	"log"
 	"net"
@@ -18,13 +17,21 @@ import (
 var testDaemon *Daemon
 
 func TestMain(m *testing.M) {
-	cfg := config.DefaultConfig()
-	// Reduce the max connections to 3 so we can test the timeout.
-	cfg.GitMaxConnections = 3
-	// Reduce the max timeout to 100 second so we can test the timeout.
-	cfg.GitMaxTimeout = 100
-	// Reduce the max read timeout to 1 second so we can test the timeout.
-	cfg.GitMaxReadTimeout = 1
+	testdata := "testdata"
+	defer os.RemoveAll(testdata)
+	cfg := &config.Config{
+		Host:     "",
+		DataPath: testdata,
+		Git: config.GitConfig{
+			// Reduce the max timeout to 100 second so we can test the timeout.
+			MaxTimeout: 100,
+			// Reduce the max read timeout to 1 second so we can test the timeout.
+			MaxReadTimeout: 1,
+			// Reduce the max connections to 3 so we can test the timeout.
+			MaxConnections: 3,
+			Port:           9418,
+		},
+	}
 	ac, err := appCfg.NewConfig(cfg)
 	if err != nil {
 		log.Fatal(err)
@@ -39,7 +46,7 @@ func TestMain(m *testing.M) {
 			log.Fatal(err)
 		}
 	}()
-	defer d.Shutdown(context.Background())
+	defer d.Close()
 	os.Exit(m.Run())
 }
 

server/server.go 🔗

@@ -45,7 +45,7 @@ func NewServer(cfg *config.Config) *Server {
 			// Command middleware must come after the git middleware.
 			cm.Middleware(ac),
 			// Git middleware.
-			gm.Middleware(cfg.RepoPath, ac),
+			gm.Middleware(cfg.RepoPath(), ac),
 			// Logging middleware must be last to be executed first.
 			lm.Middleware(),
 		),
@@ -53,8 +53,8 @@ func NewServer(cfg *config.Config) *Server {
 	s, err := wish.NewServer(
 		ssh.PublicKeyAuth(ac.PublicKeyHandler),
 		ssh.KeyboardInteractiveAuth(ac.KeyboardInteractiveHandler),
-		wish.WithAddress(fmt.Sprintf("%s:%d", cfg.BindAddr, cfg.Port)),
-		wish.WithHostKeyPath(cfg.KeyPath),
+		wish.WithAddress(fmt.Sprintf("%s:%d", cfg.Host, cfg.SSH.Port)),
+		wish.WithHostKeyPath(cfg.PrivateKeyPath()),
 		wish.WithMiddleware(mw...),
 	)
 	if err != nil {
@@ -81,14 +81,14 @@ func (s *Server) Reload() error {
 func (s *Server) Start() error {
 	var errg errgroup.Group
 	errg.Go(func() error {
-		log.Printf("Starting Git server on %s:%d", s.Config.BindAddr, s.Config.GitPort)
+		log.Printf("Starting Git server on %s:%d", s.Config.Host, s.Config.Git.Port)
 		if err := s.GitServer.Start(); err != daemon.ErrServerClosed {
 			return err
 		}
 		return nil
 	})
 	errg.Go(func() error {
-		log.Printf("Starting SSH server on %s:%d", s.Config.BindAddr, s.Config.Port)
+		log.Printf("Starting SSH server on %s:%d", s.Config.Host, s.Config.SSH.Port)
 		if err := s.SSHServer.ListenAndServe(); err != ssh.ErrServerClosed {
 			return err
 		}

server/server_test.go 🔗

@@ -19,9 +19,10 @@ import (
 
 var (
 	cfg = &config.Config{
-		BindAddr: "",
-		Host:     "localhost",
-		Port:     22222,
+		Host: "",
+		SSH: config.SSHConfig{
+			Port: 22222,
+		},
 	}
 )
 
@@ -54,7 +55,7 @@ func TestPushRepo(t *testing.T) {
 	is.NoErr(err)
 	_, err = r.CreateRemote(&gconfig.RemoteConfig{
 		Name: "origin",
-		URLs: []string{fmt.Sprintf("ssh://%s:%d/%s", cfg.Host, cfg.Port, "testrepo")},
+		URLs: []string{fmt.Sprintf("ssh://%s:%d/%s", cfg.Host, cfg.SSH.Port, "testrepo")},
 	})
 	auth, err := gssh.NewPublicKeysFromFile("git", pkPath, "")
 	is.NoErr(err)
@@ -77,7 +78,7 @@ func TestCloneRepo(t *testing.T) {
 	is.NoErr(err)
 
 	dst := t.TempDir()
-	url := fmt.Sprintf("ssh://%s:%d/config", cfg.Host, cfg.Port)
+	url := fmt.Sprintf("ssh://%s:%d/config", cfg.Host, cfg.SSH.Port)
 	err = ggit.Clone(url, dst, ggit.CloneOptions{
 		CommandOptions: ggit.CommandOptions{
 			Envs: []string{
@@ -91,8 +92,7 @@ func TestCloneRepo(t *testing.T) {
 func setupServer(t *testing.T) *Server {
 	t.Helper()
 	tmpdir := t.TempDir()
-	cfg.RepoPath = filepath.Join(tmpdir, "repos")
-	cfg.KeyPath = filepath.Join(tmpdir, "key")
+	cfg.DataPath = tmpdir
 	s := NewServer(cfg)
 	go func() {
 		s.Start()

server/session_test.go 🔗

@@ -55,11 +55,10 @@ func TestSession(t *testing.T) {
 func setup(tb testing.TB) *gossh.Session {
 	is := is.New(tb)
 	tb.Helper()
-	cfg.RepoPath = tb.TempDir()
+	cfg.DataPath = tb.TempDir()
 	ac, err := appCfg.NewConfig(&config.Config{
-		Port:     22226,
-		KeyPath:  tb.TempDir(),
-		RepoPath: tb.TempDir(),
+		SSH:      config.SSHConfig{Port: 22226},
+		DataPath: tb.TempDir(),
 		InitialAdminKeys: []string{
 			"ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIMJlb/qf2B2kMNdBxfpCQqI2ctPcsOkdZGVh5zTRhKtH",
 		},