Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add configuration validation #10

Merged
merged 1 commit into from
May 28, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Add configuration validation
- Verify that required fields have values
- Verify that MaxConcurrentSessions > 0
- Add comments describing config fields
- Add tests for invalid configuration
djjuhasz authored and jraddaoui committed May 28, 2024

Verified

This commit was signed with the committer’s verified signature.
jraddaoui José Raddaoui Marín
commit 0225937cadd78393ad6d5daac90e6b1d0cedcee1
100 changes: 81 additions & 19 deletions internal/config/config.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package config

import (
"errors"
"fmt"
"os"
"strings"

"github.com/spf13/viper"
@@ -12,25 +14,73 @@
}

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 @@
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)

Check warning on line 121 in internal/config/config.go

Codecov / codecov/patch

internal/config/config.go#L121

Added line #L121 was not covered by tests
}

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)
}
125 changes: 99 additions & 26 deletions internal/config/config_test.go
Original file line number Diff line number Diff line change
@@ -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)
})
}
}