Hard-fail on config parsing errors

Signed-off-by: David Son <davbson@amazon.com>

Author: sondavidb

config/config.go

@@ -39,6 +39,7 @@
 package config
 
 import (
+	"errors"
 	"fmt"
 	"os"
 
@@ -84,8 +85,8 @@ func NewConfig() *Config {
 
 	// Set any defaults which do not align with Go zero values.
 	var initParsers = []configParser{defaultPullModes, defaultDirectoryCacheConfig}
-	for _, p := range append(initParsers, parsers...) {
-		p(cfg)
+	if err := parseConfig(cfg, append(initParsers, parsers...)); err != nil {
+		return nil
 	}
 
 	return cfg
@@ -102,18 +103,26 @@ func NewConfigFromToml(cfgPath string) (*Config, error) {
 	defer f.Close()
 
 	cfg := NewConfig()
+	if cfg == nil {
+		return nil, errors.New("error creating default config")
+	}
 	// Get configuration from specified file
 	if err = toml.NewDecoder(f).Decode(cfg); err != nil {
 		return nil, fmt.Errorf("failed to load config file %q: %w", cfgPath, err)
 	}
-	parseConfig(cfg)
+	if err := parseConfig(cfg, parsers); err != nil {
+		return nil, fmt.Errorf("config file at %q: %w", cfgPath, err)
+	}
 	return cfg, nil
 }
 
-func parseConfig(cfg *Config) {
-	for _, p := range parsers {
-		p(cfg)
+func parseConfig(cfg *Config, cfgParsers []configParser) error {
+	for _, p := range cfgParsers {
+		if err := p(cfg); err != nil {
+			return fmt.Errorf("failed to parse config: %v", err)
+		}
 	}
+	return nil
 }
 
 func parseRootConfig(cfg *Config) error {

config/config_test.go

@@ -265,6 +265,18 @@ concurrent_download_chunk_size = "1MB"
 				}
 			},
 		},
+		{
+			name: "IncorrectChunkConfig",
+			config: []byte(`
+[pull_modes.parallel_pull_unpack]
+concurrent_download_chunk_size = "badchunksize"
+`),
+			assert: func(t *testing.T, actual *Config, err error) {
+				if err == nil {
+					t.Error("Expected error, got none")
+				}
+			},
+		},
 	}
 
 	for _, test := range tests {

integration/startup_test.go

@@ -21,9 +21,11 @@ import (
 	"regexp"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/awslabs/soci-snapshotter/config"
 	shell "github.com/awslabs/soci-snapshotter/util/dockershell"
+	"github.com/awslabs/soci-snapshotter/util/testutil"
 	"github.com/rs/xid"
 )
 
@@ -137,3 +139,47 @@ func TestSnapshotterSystemdStartup(t *testing.T) {
 		})
 	}
 }
+
+// TestSnapshotterStartupWithBadConfig ensures snapshotter does not start if snapshotter values are incorrect.
+// Note that incorrect fields are ignored by TOML and thus are expected to work
+func TestSnapshotterStartupWithBadConfig(t *testing.T) {
+	tests := []struct {
+		name           string
+		opts           []snapshotterConfigOpt
+		expectedErrStr string
+	}{
+		{
+			name:           "Bad parallel pull size",
+			opts:           []snapshotterConfigOpt{withParallelPullMode(), withConcurrentDownloadChunkSizeStr("badstring")},
+			expectedErrStr: "invalid size format",
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(tt *testing.T) {
+			sh, cleanup := newSnapshotterBaseShell(t)
+			defer cleanup()
+			// rebootContainerd would be ideal here, but that always uses a config,
+			// so we just manually start SOCI with/without a config file.
+			err := testutil.KillMatchingProcess(sh, "soci-snapshotter-grpc")
+			if err != nil {
+				sh.Fatal("failed to kill soci: %v", err)
+			}
+			configToml := getSnapshotterConfigToml(t, tc.opts...)
+			snRunCmd := []string{"/usr/local/bin/soci-snapshotter-grpc", "--log-level", sociLogLevel}
+			snRunCmd = addConfig(t, sh, configToml, snRunCmd...)
+
+			errCh := make(chan error, 1)
+			go func() {
+				_, err := sh.OLog(snRunCmd...)
+				errCh <- err
+			}()
+
+			select {
+			case <-errCh:
+			case <-time.After(2 * time.Second):
+				t.Fatalf("expected err %s but snapshotter did not fail within 2 seconds", tc.expectedErrStr)
+			}
+		})
+	}
+}

integration/util_test.go

@@ -383,6 +383,12 @@ func withConcurrentDownloadChunkSize(chunkSize int64) snapshotterConfigOpt {
 	}
 }
 
+func withConcurrentDownloadChunkSizeStr(chunkSizeStr string) snapshotterConfigOpt {
+	return func(c *config.Config) {
+		c.PullModes.Parallel.ConcurrentDownloadChunkSizeStr = chunkSizeStr
+	}
+}
+
 func withConcurrentUnpacks(n int64) snapshotterConfigOpt {
 	return func(c *config.Config) {
 		c.PullModes.Parallel.MaxConcurrentUnpacks = n