Add configuration validation

- Verify that required fields have values
- Verify that MaxConcurrentSessions > 0
- Add comments describing config fields
- Add tests for invalid configuration

internal/config/config.go

@@ -1,7 +1,9 @@
 package config
 
 import (
+	"errors"
 	"fmt"
+	"os"
 	"strings"
 
 	"github.com/spf13/viper"
@@ -12,25 +14,73 @@ type ConfigurationValidator interface {
 }
 
 type Configuration struct {
-	Verbosity  int
-	Debug      bool
+	// Debug toggles human readable logs or JSON logs (default).
+	Debug bool
+
+	// Verbosity sets the verbosity level of log messages, with 0 (default)
+	// logging only critical messages and each higher number increasing the
+	// number of messages logged.
+	Verbosity int
+
+	// SharedPath is a file path that both Preprocessing and Enduro can access
+	// (required).
+	//
+	// Enduro will deposit transfers in SharedPath for preprocessing.
+	// Preprocessing must write transfer updates to SharedPath for retrieval by
+	// Enduro and preservation processing.
 	SharedPath string
-	Temporal   Temporal
-	Worker     WorkerConfig
+
+	Temporal Temporal
+	Worker   WorkerConfig
 }
 
 type Temporal struct {
-	Address      string
-	Namespace    string
-	TaskQueue    string
+	// Address is the Temporal server host and port (default: "localhost:7233").
+	Address string
+
+	// Namespace is the Temporal namespace the preprocessing worker should run
+	// in (default: "default").
+	Namespace string
+
+	// TaskQueue is the Temporal task queue from which the preprocessing worker
+	// will pull tasks (required).
+	TaskQueue string
+
+	// WorkflowName is the name of the preprocessing Temporal workflow
+	// (required).
 	WorkflowName string
 }
 
 type WorkerConfig struct {
+	// MaxConcurrentSessions limits the number of workflow sessions the
+	// preprocessing worker can handle simultaneously (default: 1).
 	MaxConcurrentSessions int
 }
 
-func (c Configuration) Validate() error { return nil }
+func (c Configuration) Validate() error {
+	var errs error
+
+	// Verify that the required fields have values.
+	if c.SharedPath == "" {
+		errs = errors.Join(errs, errRequired("SharedPath"))
+	}
+	if c.Temporal.TaskQueue == "" {
+		errs = errors.Join(errs, errRequired("TaskQueue"))
+	}
+	if c.Temporal.WorkflowName == "" {
+		errs = errors.Join(errs, errRequired("WorkflowName"))
+	}
+
+	// Verify that MaxConcurrentSessions is >= 1.
+	if c.Worker.MaxConcurrentSessions < 1 {
+		errs = errors.Join(errs, fmt.Errorf(
+			"Worker.MaxConcurrentSessions: %d is less than the minimum value (1)",
+			c.Worker.MaxConcurrentSessions,
+		))
+	}
+
+	return errs
+}
 
 func Read(config *Configuration, configFile string) (found bool, configFileUsed string, err error) {
 	v := viper.New()
@@ -47,28 +97,40 @@ func Read(config *Configuration, configFile string) (found bool, configFileUsed
 	v.SetDefault("Worker.MaxConcurrentSessions", 1)
 
 	if configFile != "" {
+		// Viper will not return a viper.ConfigFileNotFoundError error when
+		// SetConfigFile() is passed a path to a file that doesn't exist, so we
+		// need to check ourselves.
+		if _, err := os.Stat(configFile); errors.Is(err, os.ErrNotExist) {
+			return false, "", fmt.Errorf("configuration file not found: %s", configFile)
+		}
+
 		v.SetConfigFile(configFile)
 	}
 
-	err = v.ReadInConfig()
-	_, ok := err.(viper.ConfigFileNotFoundError)
-	if !ok {
-		found = true
-	}
-	if found && err != nil {
-		return found, configFileUsed, fmt.Errorf("failed to read configuration file: %w", err)
+	if err = v.ReadInConfig(); err != nil {
+		switch err.(type) {
+		case viper.ConfigFileNotFoundError:
+			return false, "", err
+		default:
+			return true, "", fmt.Errorf("failed to read configuration file: %w", err)
+		}
 	}
 
 	err = v.Unmarshal(config)
 	if err != nil {
-		return found, configFileUsed, fmt.Errorf("failed to unmarshal configuration: %w", err)
+		return true, "", fmt.Errorf("failed to unmarshal configuration: %w", err)
 	}
 
 	if err := config.Validate(); err != nil {
-		return found, configFileUsed, fmt.Errorf("failed to validate the provided config: %w", err)
+		return true, "", errors.Join(
+			fmt.Errorf("invalid configuration:"),
+			err,
+		)
 	}
 
-	configFileUsed = v.ConfigFileUsed()
+	return true, v.ConfigFileUsed(), nil
+}
 
-	return found, configFileUsed, nil
+func errRequired(name string) error {
+	return fmt.Errorf("%s: missing required value", name)
 }

internal/config/config_test.go

@@ -23,30 +23,103 @@ maxConcurrentSessions = 1
 `
 
 func TestConfig(t *testing.T) {
-	tmpDir := fs.NewDir(
-		t, "",
-		fs.WithFile(
-			"preprocessing.toml",
-			testConfig,
-		),
-	)
-	configFile := tmpDir.Join("preprocessing.toml")
-
-	var c config.Configuration
-	found, configFileUsed, err := config.Read(&c, configFile)
-
-	assert.NilError(t, err)
-	assert.Equal(t, found, true)
-	assert.Equal(t, configFileUsed, configFile)
-
-	assert.Equal(t, c.Debug, true)
-	assert.Equal(t, c.Verbosity, 2)
-	assert.Equal(t, c.SharedPath, "/home/preprocessing/shared")
-
-	assert.Equal(t, c.Temporal.Address, "host:port")
-	assert.Equal(t, c.Temporal.Namespace, "default")
-	assert.Equal(t, c.Temporal.TaskQueue, "preprocessing")
-	assert.Equal(t, c.Temporal.WorkflowName, "preprocessing")
-
-	assert.Equal(t, c.Worker.MaxConcurrentSessions, 1)
+	t.Parallel()
+
+	type test struct {
+		name       string
+		configFile string
+		toml       string
+		wantFound  bool
+		wantCfg    config.Configuration
+		wantErr    string
+	}
+
+	for _, tc := range []test{
+		{
+			name:       "Loads configuration from a TOML file",
+			configFile: "preprocessing.toml",
+			toml:       testConfig,
+			wantFound:  true,
+			wantCfg: config.Configuration{
+				Debug:      true,
+				Verbosity:  2,
+				SharedPath: "/home/preprocessing/shared",
+				Temporal: config.Temporal{
+					Address:      "host:port",
+					Namespace:    "default",
+					TaskQueue:    "preprocessing",
+					WorkflowName: "preprocessing",
+				},
+				Worker: config.WorkerConfig{
+					MaxConcurrentSessions: 1,
+				},
+			},
+		},
+		{
+			name:       "Errors when configuration values are not valid",
+			configFile: "preprocessing.toml",
+			wantFound:  true,
+			wantErr: `invalid configuration:
+SharedPath: missing required value
+TaskQueue: missing required value
+WorkflowName: missing required value`,
+		},
+		{
+			name:       "Errors when MaxConcurrentSessions is less than 1",
+			configFile: "preprocessing.toml",
+			toml: `# Config
+sharedPath = "/home/preprocessing/shared"
+[temporal]
+taskQueue = "preprocessing"
+workflowName = "preprocessing"
+[worker]
+maxConcurrentSessions = -1
+`,
+			wantFound: true,
+			wantErr:   `Worker.MaxConcurrentSessions: -1 is less than the minimum value (1)`,
+		},
+		{
+			name:       "Errors when TOML is invalid",
+			configFile: "preprocessing.toml",
+			toml:       "bad TOML",
+			wantFound:  true,
+			wantErr:    "failed to read configuration file: While parsing config: toml: expected character =",
+		},
+		{
+			name:      "Errors when no config file is found in the default paths",
+			wantFound: false,
+			wantErr:   "Config File \"preprocessing\" Not Found in",
+		},
+		{
+			name:       "Errors when the given configFile is not found",
+			configFile: "missing.toml",
+			wantFound:  false,
+			wantErr:    "configuration file not found: ",
+		},
+	} {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			tmpDir := fs.NewDir(t, "preprocessing-test", fs.WithFile("preprocessing.toml", tc.toml))
+
+			configFile := ""
+			if tc.configFile != "" {
+				configFile = tmpDir.Join(tc.configFile)
+			}
+
+			var c config.Configuration
+			found, configFileUsed, err := config.Read(&c, configFile)
+			if tc.wantErr != "" {
+				assert.Equal(t, found, tc.wantFound)
+				assert.ErrorContains(t, err, tc.wantErr)
+				return
+			}
+
+			assert.NilError(t, err)
+			assert.Equal(t, found, true)
+			assert.Equal(t, configFileUsed, configFile)
+			assert.DeepEqual(t, c, tc.wantCfg)
+		})
+	}
 }