Move configuration parsing to Config package

Changelog: changed
Signed-off-by: Balasankar "Balu" C <balasankarc@autistici.org>

5 files changed, 638 insertions, 3 deletions

diff --git a/internal/config/config.go b/internal/config/config.go
index e2956bb6..34d3c9a6 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -1,15 +1,133 @@
 package config
 
 import (
+	"encoding/base64"
+	"fmt"
+	"io/ioutil"
+	"net/url"
+	"strings"
 	"time"
+
+	"github.com/namsral/flag"
+	log "github.com/sirupsen/logrus"
+
+	"gitlab.com/gitlab-org/gitlab-pages/internal/tlsconfig"
 )
 
+// Config stores all the config options relevant to GitLab Pages.
 type Config struct {
-	Zip *ZipServing
+	General         *General
+	ArtifactsServer *ArtifactsServer
+	Authentication  *Auth
+	Daemon          *Daemon
+	GitLab          *GitLab
+	Listeners       *Listeners
+	Log             *Log
+	Sentry          *Sentry
+	TLS             *TLS
+	Zip             *ZipServing
+
+	// Fields used to share information between files. These are not directly
+	// set by command line flags, but rather populated based on info from them.
+	// ListenMetrics points to a file descriptor of a socket, whose address is
+	// specified by `Config.General.MetricsAddress`.
+	ListenMetrics uintptr
+
+	// These fields contain the raw strings passed for listen-http,
+	// listen-https, listen-proxy and listen-https-proxyv2 settings. It is used
+	// by appmain() to create listeners, and the pointers to these listeners
+	// gets assigned to Config.Listeners.* fields
+	ListenHTTPStrings         MultiStringFlag
+	ListenHTTPSStrings        MultiStringFlag
+	ListenProxyStrings        MultiStringFlag
+	ListenHTTPSProxyv2Strings MultiStringFlag
+}
+
+// General groups settings that are general to GitLab Pages and can not
+// be categorized under other head.
+type General struct {
+	Domain                    string
+	DomainConfigurationSource string
+	HTTP2                     bool
+	MaxConns                  int
+	MetricsAddress            string
+	RedirectHTTP              bool
+	RootCertificate           []byte
+	RootDir                   string
+	RootKey                   []byte
+	StatusPath                string
+
+	DisableCrossOriginRequests bool
+	InsecureCiphers            bool
+	PropagateCorrelationID     bool
+
+	ShowVersion bool
+
+	CustomHeaders []string
+}
+
+// ArtifactsServer groups settings related to configuring Artifacts
+// server
+type ArtifactsServer struct {
+	URL            string
+	TimeoutSeconds int
+}
+
+// Auth groups settings related to configuring Authentication with
+// GitLab
+type Auth struct {
+	Secret       string
+	ClientID     string
+	ClientSecret string
+	RedirectURI  string
+	Scope        string
+}
+
+// Daemon groups settings related to configuring GitLab Pages daemon
+type Daemon struct {
+	UID           int
+	GID           int
+	InplaceChroot bool
+}
+
+// GitLab groups settings related to configuring GitLab client used to
+// interact with GitLab API
+type GitLab struct {
+	Server             string
+	InternalServer     string
+	APISecretKey       []byte
+	ClientHTTPTimeout  time.Duration
+	JWTTokenExpiration time.Duration
+}
+
+// Listeners groups settings related to configuring various listeners
+// (HTTP, HTTPS, Proxy, HTTPSProxyv2)
+type Listeners struct {
+	HTTP         []uintptr
+	HTTPS        []uintptr
+	Proxy        []uintptr
+	HTTPSProxyv2 []uintptr
 }
 
-// ZipServing stores all configuration values to be used by the zip VFS opening and
-// caching
+// Log groups settings related to configuring logging
+type Log struct {
+	Format  string
+	Verbose bool
+}
+
+// Sentry groups settings related to configuring Sentry
+type Sentry struct {
+	DSN         string
+	Environment string
+}
+
+// TLS groups settings related to configuring TLS
+type TLS struct {
+	MinVersion uint16
+	MaxVersion uint16
+}
+
+// ZipServing groups settings to be used by the zip VFS opening and caching
 type ZipServing struct {
 	ExpirationInterval time.Duration
 	CleanupInterval    time.Duration
@@ -17,3 +135,273 @@ type ZipServing struct {
 	OpenTimeout        time.Duration
 	AllowedPaths       []string
 }
+
+func gitlabServerFromFlags() string {
+	if *gitLabServer != "" {
+		return *gitLabServer
+	}
+
+	if *gitLabAuthServer != "" {
+		log.Warn("auth-server parameter is deprecated, use gitlab-server instead")
+		return *gitLabAuthServer
+	}
+
+	u, err := url.Parse(*artifactsServer)
+	if err != nil {
+		return ""
+	}
+
+	u.Path = ""
+	return u.String()
+}
+
+func internalGitlabServerFromFlags() string {
+	if *internalGitLabServer != "" {
+		return *internalGitLabServer
+	}
+
+	return gitlabServerFromFlags()
+}
+
+func setGitLabAPISecretKey(secretFile string, config *Config) {
+	encoded := readFile(secretFile)
+
+	decoded := make([]byte, base64.StdEncoding.DecodedLen(len(encoded)))
+	secretLength, err := base64.StdEncoding.Decode(decoded, encoded)
+	if err != nil {
+		log.WithError(err).Fatal("Failed to decode GitLab API secret")
+	}
+
+	if secretLength != 32 {
+		log.WithError(fmt.Errorf("expected 32 bytes GitLab API secret but got %d bytes", secretLength)).Fatal("Failed to decode GitLab API secret")
+	}
+
+	config.GitLab.APISecretKey = decoded
+}
+
+func checkAuthenticationConfig(config *Config) {
+	if config.Authentication.Secret == "" && config.Authentication.ClientID == "" &&
+		config.Authentication.ClientSecret == "" && config.Authentication.RedirectURI == "" {
+		return
+	}
+	assertAuthConfig(config)
+}
+
+func assertAuthConfig(config *Config) {
+	if config.Authentication.Secret == "" {
+		log.Fatal("auth-secret must be defined if authentication is supported")
+	}
+	if config.Authentication.ClientID == "" {
+		log.Fatal("auth-client-id must be defined if authentication is supported")
+	}
+	if config.Authentication.ClientSecret == "" {
+		log.Fatal("auth-client-secret must be defined if authentication is supported")
+	}
+	if config.GitLab.Server == "" {
+		log.Fatal("gitlab-server must be defined if authentication is supported")
+	}
+	if config.Authentication.RedirectURI == "" {
+		log.Fatal("auth-redirect-uri must be defined if authentication is supported")
+	}
+}
+
+func validateArtifactsServer(artifactsServer string, artifactsServerTimeoutSeconds int) {
+	u, err := url.Parse(artifactsServer)
+	if err != nil {
+		log.Fatal(err)
+	}
+	// url.Parse ensures that the Scheme attribute is always lower case.
+	if u.Scheme != "http" && u.Scheme != "https" {
+		log.Fatal("artifacts-server scheme must be either http:// or https://")
+	}
+
+	if artifactsServerTimeoutSeconds < 1 {
+		log.Fatal("artifacts-server-timeout must be greater than or equal to 1")
+	}
+}
+
+// fatal will log a fatal error and exit.
+func fatal(err error, message string) {
+	log.WithError(err).Fatal(message)
+}
+
+func readFile(file string) (result []byte) {
+	result, err := ioutil.ReadFile(file)
+	if err != nil {
+		fatal(err, "could not read file")
+	}
+	return
+}
+
+// InternalGitLabServerURL returns URL to a GitLab instance.
+func (config Config) InternalGitLabServerURL() string {
+	return config.GitLab.InternalServer
+}
+
+// GitlabClientSecret returns GitLab server access token.
+func (config Config) GitlabAPISecret() []byte {
+	return config.GitLab.APISecretKey
+}
+
+func (config Config) GitlabClientConnectionTimeout() time.Duration {
+	return config.GitLab.ClientHTTPTimeout
+}
+
+func (config Config) GitlabJWTTokenExpiry() time.Duration {
+	return config.GitLab.JWTTokenExpiration
+}
+
+func (config Config) DomainConfigSource() string {
+	return config.General.DomainConfigurationSource
+}
+
+func loadConfig() *Config {
+	config := &Config{
+		General: &General{
+			Domain:                     strings.ToLower(*pagesDomain),
+			DomainConfigurationSource:  *domainConfigSource,
+			HTTP2:                      *useHTTP2,
+			MaxConns:                   int(*maxConns),
+			MetricsAddress:             *metricsAddress,
+			RedirectHTTP:               *redirectHTTP,
+			RootDir:                    *pagesRoot,
+			StatusPath:                 *pagesStatus,
+			DisableCrossOriginRequests: *disableCrossOriginRequests,
+			InsecureCiphers:            *insecureCiphers,
+			PropagateCorrelationID:     *propagateCorrelationID,
+			CustomHeaders:              header.Split(),
+			ShowVersion:                *showVersion,
+		},
+		GitLab: &GitLab{
+			ClientHTTPTimeout:  *gitlabClientHTTPTimeout,
+			JWTTokenExpiration: *gitlabClientJWTExpiry,
+		},
+		ArtifactsServer: &ArtifactsServer{
+			TimeoutSeconds: *artifactsServerTimeout,
+			URL:            *artifactsServer,
+		},
+		Authentication: &Auth{
+			Secret:       *secret,
+			ClientID:     *clientID,
+			ClientSecret: *clientSecret,
+			RedirectURI:  *redirectURI,
+			Scope:        *authScope,
+		},
+		Daemon: &Daemon{
+			UID:           int(*daemonUID),
+			GID:           int(*daemonGID),
+			InplaceChroot: *daemonInplaceChroot,
+		},
+		Log: &Log{
+			Format:  *logFormat,
+			Verbose: *logVerbose,
+		},
+		Sentry: &Sentry{
+			DSN:         *sentryDSN,
+			Environment: *sentryEnvironment,
+		},
+		TLS: &TLS{
+			MinVersion: tlsconfig.AllTLSVersions[*tlsMinVersion],
+			MaxVersion: tlsconfig.AllTLSVersions[*tlsMaxVersion],
+		},
+		Zip: &ZipServing{
+			ExpirationInterval: *zipCacheExpiration,
+			CleanupInterval:    *zipCacheCleanup,
+			RefreshInterval:    *zipCacheRefresh,
+			OpenTimeout:        *zipOpenTimeout,
+			AllowedPaths:       []string{*pagesRoot},
+		},
+
+		// Actual listener pointers will be populated in appMain. We populate the
+		// raw strings here so that they are available in appMain
+		ListenHTTPStrings:         listenHTTP,
+		ListenHTTPSStrings:        listenHTTPS,
+		ListenProxyStrings:        listenProxy,
+		ListenHTTPSProxyv2Strings: listenHTTPSProxyv2,
+		Listeners:                 &Listeners{},
+	}
+
+	// Populating remaining General settings
+	for _, file := range []struct {
+		contents *[]byte
+		path     string
+	}{
+		{&config.General.RootCertificate, *pagesRootCert},
+		{&config.General.RootKey, *pagesRootKey},
+	} {
+		if file.path != "" {
+			*file.contents = readFile(file.path)
+		}
+	}
+
+	// Populating remaining GitLab settings
+	config.GitLab.Server = gitlabServerFromFlags()
+	config.GitLab.InternalServer = internalGitlabServerFromFlags()
+	if *gitLabAPISecretKey != "" {
+		setGitLabAPISecretKey(*gitLabAPISecretKey, config)
+	}
+
+	// Validating Artifacts server settings
+	if *artifactsServer != "" {
+		validateArtifactsServer(*artifactsServer, *artifactsServerTimeout)
+	}
+
+	// Validating Authentication settings
+	checkAuthenticationConfig(config)
+
+	// Validating TLS settings
+	if err := tlsconfig.ValidateTLSVersions(*tlsMinVersion, *tlsMaxVersion); err != nil {
+		fatal(err, "invalid TLS version")
+	}
+
+	return config
+}
+
+func LogConfig(config *Config) {
+	log.WithFields(log.Fields{
+		"artifacts-server":              *artifactsServer,
+		"artifacts-server-timeout":      *artifactsServerTimeout,
+		"daemon-gid":                    *daemonGID,
+		"daemon-uid":                    *daemonUID,
+		"daemon-inplace-chroot":         *daemonInplaceChroot,
+		"default-config-filename":       flag.DefaultConfigFlagname,
+		"disable-cross-origin-requests": *disableCrossOriginRequests,
+		"domain":                        config.General.Domain,
+		"insecure-ciphers":              config.General.InsecureCiphers,
+		"listen-http":                   listenHTTP,
+		"listen-https":                  listenHTTPS,
+		"listen-proxy":                  listenProxy,
+		"listen-https-proxyv2":          listenHTTPSProxyv2,
+		"log-format":                    *logFormat,
+		"metrics-address":               *metricsAddress,
+		"pages-domain":                  *pagesDomain,
+		"pages-root":                    *pagesRoot,
+		"pages-status":                  *pagesStatus,
+		"propagate-correlation-id":      *propagateCorrelationID,
+		"redirect-http":                 config.General.RedirectHTTP,
+		"root-cert":                     *pagesRootKey,
+		"root-key":                      *pagesRootCert,
+		"status_path":                   config.General.StatusPath,
+		"tls-min-version":               *tlsMinVersion,
+		"tls-max-version":               *tlsMaxVersion,
+		"use-http-2":                    config.General.HTTP2,
+		"gitlab-server":                 config.GitLab.Server,
+		"internal-gitlab-server":        config.GitLab.InternalServer,
+		"api-secret-key":                *gitLabAPISecretKey,
+		"domain-config-source":          config.General.DomainConfigurationSource,
+		"auth-redirect-uri":             config.Authentication.RedirectURI,
+		"auth-scope":                    config.Authentication.Scope,
+		"zip-cache-expiration":          config.Zip.ExpirationInterval,
+		"zip-cache-cleanup":             config.Zip.CleanupInterval,
+		"zip-cache-refresh":             config.Zip.RefreshInterval,
+		"zip-open-timeout":              config.Zip.OpenTimeout,
+	}).Debug("Start daemon with configuration")
+}
+
+// LoadConfig parses configuration settings passed as command line arguments or
+// via config file, and populates a Config object with those values
+func LoadConfig() *Config {
+	initFlags()
+
+	return loadConfig()
+}
diff --git a/internal/config/config_test.go b/internal/config/config_test.go
new file mode 100644
index 00000000..7fa2b253
--- /dev/null
+++ b/internal/config/config_test.go
@@ -0,0 +1,54 @@
+package config
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestGitLabServerFromFlags(t *testing.T) {
+	tests := []struct {
+		name             string
+		gitLabServer     string
+		gitLabAuthServer string
+		artifactsServer  string
+		expected         string
+	}{
+		{
+			name:             "When gitLabServer is set",
+			gitLabServer:     "https://gitlabserver.com",
+			gitLabAuthServer: "https://authserver.com",
+			artifactsServer:  "https://artifactsserver.com",
+			expected:         "https://gitlabserver.com",
+		},
+		{
+			name:             "When auth server is set",
+			gitLabServer:     "",
+			gitLabAuthServer: "https://authserver.com",
+			artifactsServer:  "https://artifactsserver.com",
+			expected:         "https://authserver.com",
+		},
+		{
+			name:             "When only artifacts server is set",
+			gitLabServer:     "",
+			gitLabAuthServer: "",
+			artifactsServer:  "https://artifactsserver.com",
+			expected:         "https://artifactsserver.com",
+		},
+		{
+			name:             "When only artifacts server includes path",
+			gitLabServer:     "",
+			gitLabAuthServer: "",
+			artifactsServer:  "https://artifactsserver.com:8080/api/path",
+			expected:         "https://artifactsserver.com:8080",
+		}}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			gitLabServer = &test.gitLabServer
+			gitLabAuthServer = &test.gitLabAuthServer
+			artifactsServer = &test.artifactsServer
+			require.Equal(t, test.expected, gitlabServerFromFlags())
+		})
+	}
+}
diff --git a/internal/config/flags.go b/internal/config/flags.go
new file mode 100644
index 00000000..11bd335a
--- /dev/null
+++ b/internal/config/flags.go
@@ -0,0 +1,81 @@
+package config
+
+import (
+	"time"
+
+	"github.com/namsral/flag"
+
+	"gitlab.com/gitlab-org/gitlab-pages/internal/tlsconfig"
+)
+
+var (
+	pagesRootCert           = flag.String("root-cert", "", "The default path to file certificate to serve static pages")
+	pagesRootKey            = flag.String("root-key", "", "The default path to file certificate to serve static pages")
+	redirectHTTP            = flag.Bool("redirect-http", false, "Redirect pages from HTTP to HTTPS")
+	useHTTP2                = flag.Bool("use-http2", true, "Enable HTTP2 support")
+	pagesRoot               = flag.String("pages-root", "shared/pages", "The directory where pages are stored")
+	pagesDomain             = flag.String("pages-domain", "gitlab-example.com", "The domain to serve static pages")
+	artifactsServer         = flag.String("artifacts-server", "", "API URL to proxy artifact requests to, e.g.: 'https://gitlab.com/api/v4'")
+	artifactsServerTimeout  = flag.Int("artifacts-server-timeout", 10, "Timeout (in seconds) for a proxied request to the artifacts server")
+	pagesStatus             = flag.String("pages-status", "", "The url path for a status page, e.g., /@status")
+	metricsAddress          = flag.String("metrics-address", "", "The address to listen on for metrics requests")
+	sentryDSN               = flag.String("sentry-dsn", "", "The address for sending sentry crash reporting to")
+	sentryEnvironment       = flag.String("sentry-environment", "", "The environment for sentry crash reporting")
+	daemonUID               = flag.Uint("daemon-uid", 0, "Drop privileges to this user")
+	daemonGID               = flag.Uint("daemon-gid", 0, "Drop privileges to this group")
+	daemonInplaceChroot     = flag.Bool("daemon-inplace-chroot", false, "Fall back to a non-bind-mount chroot of -pages-root when daemonizing")
+	propagateCorrelationID  = flag.Bool("propagate-correlation-id", false, "Reuse existing Correlation-ID from the incoming request header `X-Request-ID` if present")
+	logFormat               = flag.String("log-format", "text", "The log output format: 'text' or 'json'")
+	logVerbose              = flag.Bool("log-verbose", false, "Verbose logging")
+	_                       = flag.String("admin-secret-path", "", "DEPRECATED")
+	_                       = flag.String("admin-unix-listener", "", "DEPRECATED")
+	_                       = flag.String("admin-https-listener", "", "DEPRECATED")
+	_                       = flag.String("admin-https-cert", "", "DEPRECATED")
+	_                       = flag.String("admin-https-key", "", "DEPRECATED")
+	secret                  = flag.String("auth-secret", "", "Cookie store hash key, should be at least 32 bytes long")
+	gitLabAuthServer        = flag.String("auth-server", "", "DEPRECATED, use gitlab-server instead. GitLab server, for example https://www.gitlab.com")
+	gitLabServer            = flag.String("gitlab-server", "", "GitLab server, for example https://www.gitlab.com")
+	internalGitLabServer    = flag.String("internal-gitlab-server", "", "Internal GitLab server used for API requests, useful if you want to send that traffic over an internal load balancer, example value https://www.gitlab.com (defaults to value of gitlab-server)")
+	gitLabAPISecretKey      = flag.String("api-secret-key", "", "File with secret key used to authenticate with the GitLab API")
+	gitlabClientHTTPTimeout = flag.Duration("gitlab-client-http-timeout", 10*time.Second, "GitLab API HTTP client connection timeout in seconds (default: 10s)")
+	gitlabClientJWTExpiry   = flag.Duration("gitlab-client-jwt-expiry", 30*time.Second, "JWT Token expiry time in seconds (default: 30s)")
+	domainConfigSource      = flag.String("domain-config-source", "auto", "Domain configuration source 'disk', 'auto' or 'gitlab' (default: 'auto'). DEPRECATED: gitlab-pages will use the API-based configuration starting from 14.0 see https://gitlab.com/gitlab-org/gitlab-pages/-/issues/382")
+	clientID                = flag.String("auth-client-id", "", "GitLab application Client ID")
+	clientSecret            = flag.String("auth-client-secret", "", "GitLab application Client Secret")
+	redirectURI             = flag.String("auth-redirect-uri", "", "GitLab application redirect URI")
+	authScope               = flag.String("auth-scope", "api", "Scope to be used for authentication (must match GitLab Pages OAuth application settings)")
+	maxConns                = flag.Uint("max-conns", 5000, "Limit on the number of concurrent connections to the HTTP, HTTPS or proxy listeners")
+	insecureCiphers         = flag.Bool("insecure-ciphers", false, "Use default list of cipher suites, may contain insecure ones like 3DES and RC4")
+	tlsMinVersion           = flag.String("tls-min-version", "tls1.2", tlsconfig.FlagUsage("min"))
+	tlsMaxVersion           = flag.String("tls-max-version", "", tlsconfig.FlagUsage("max"))
+	zipCacheExpiration      = flag.Duration("zip-cache-expiration", 60*time.Second, "Zip serving archive cache expiration interval")
+	zipCacheCleanup         = flag.Duration("zip-cache-cleanup", 30*time.Second, "Zip serving archive cache cleanup interval")
+	zipCacheRefresh         = flag.Duration("zip-cache-refresh", 30*time.Second, "Zip serving archive cache refresh interval")
+	zipOpenTimeout          = flag.Duration("zip-open-timeout", 30*time.Second, "Zip archive open timeout")
+
+	disableCrossOriginRequests = flag.Bool("disable-cross-origin-requests", false, "Disable cross-origin requests")
+
+	showVersion = flag.Bool("version", false, "Show version")
+
+	// See initFlags()
+	listenHTTP         = MultiStringFlag{separator: ","}
+	listenHTTPS        = MultiStringFlag{separator: ","}
+	listenProxy        = MultiStringFlag{separator: ","}
+	listenHTTPSProxyv2 = MultiStringFlag{separator: ","}
+
+	header = MultiStringFlag{separator: ";;"}
+)
+
+// initFlags will be called from LoadConfig
+func initFlags() {
+	flag.Var(&listenHTTP, "listen-http", "The address(es) to listen on for HTTP requests")
+	flag.Var(&listenHTTPS, "listen-https", "The address(es) to listen on for HTTPS requests")
+	flag.Var(&listenProxy, "listen-proxy", "The address(es) to listen on for proxy requests")
+	flag.Var(&listenHTTPSProxyv2, "listen-https-proxyv2", "The address(es) to listen on for HTTPS PROXYv2 requests (https://www.haproxy.org/download/1.8/doc/proxy-protocol.txt)")
+	flag.Var(&header, "header", "The additional http header(s) that should be send to the client")
+
+	// read from -config=/path/to/gitlab-pages-config
+	flag.String(flag.DefaultConfigFlagname, "", "path to config file")
+
+	flag.Parse()
+}
diff --git a/internal/config/multi_string_flag.go b/internal/config/multi_string_flag.go
new file mode 100644
index 00000000..fc63299b
--- /dev/null
+++ b/internal/config/multi_string_flag.go
@@ -0,0 +1,51 @@
+package config
+
+import (
+	"errors"
+	"strings"
+)
+
+var errMultiStringSetEmptyValue = errors.New("value cannot be empty")
+
+const defaultSeparator = ","
+
+// MultiStringFlag implements the flag.Value interface and allows a string flag
+// to be specified multiple times on the command line.
+//
+// e.g.: -listen-http 127.0.0.1:80 -listen-http [::1]:80
+type MultiStringFlag struct {
+	value     []string
+	separator string
+}
+
+// String returns the list of parameters joined with a commas (",")
+func (s *MultiStringFlag) String() string {
+	return strings.Join(s.value, s.sep())
+}
+
+// Set appends the value to the list of parameters
+func (s *MultiStringFlag) Set(value string) error {
+	if value == "" {
+		return errMultiStringSetEmptyValue
+	}
+
+	s.value = append(s.value, value)
+	return nil
+}
+
+// Split each flag
+func (s *MultiStringFlag) Split() (result []string) {
+	for _, str := range s.value {
+		result = append(result, strings.Split(str, s.sep())...)
+	}
+
+	return
+}
+
+func (s *MultiStringFlag) sep() string {
+	if s.separator == "" {
+		return defaultSeparator
+	}
+
+	return s.separator
+}
diff --git a/internal/config/multi_string_flag_test.go b/internal/config/multi_string_flag_test.go
new file mode 100644
index 00000000..a79247a9
--- /dev/null
+++ b/internal/config/multi_string_flag_test.go
@@ -0,0 +1,61 @@
+package config
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestMultiStringFlagAppendsOnSet(t *testing.T) {
+	var concrete MultiStringFlag
+	iface := &concrete
+
+	require.NoError(t, iface.Set("foo"))
+	require.NoError(t, iface.Set("bar"))
+
+	require.EqualError(t, iface.Set(""), "value cannot be empty")
+
+	require.Equal(t, MultiStringFlag{value: []string{"foo", "bar"}}, concrete)
+}
+
+func TestMultiStringFlag_Split(t *testing.T) {
+	tests := []struct {
+		name       string
+		s          *MultiStringFlag
+		wantResult []string
+	}{
+		{
+			name:       "empty_string",
+			s:          &MultiStringFlag{}, // -flag ""
+			wantResult: []string{},
+		},
+		{
+			name:       "one_value",
+			s:          &MultiStringFlag{value: []string{"value1"}}, // -flag "value1"
+			wantResult: []string{"value1"},
+		},
+		{
+			name:       "multiple_values",
+			s:          &MultiStringFlag{value: []string{"value1", "", "value3"}}, // -flag "value1,,value3"
+			wantResult: []string{"value1", "", "value3"},
+		},
+		{
+			name:       "multiple_values_in_one_string",
+			s:          &MultiStringFlag{value: []string{"value1,value2"}}, // -flag "value1,value2"
+			wantResult: []string{"value1", "value2"},
+		},
+		{
+			name:       "different_separator",
+			s:          &MultiStringFlag{value: []string{"value1", "value2"}, separator: ";"}, // -flag "value1;value2"
+			wantResult: []string{"value1", "value2"},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			gotResult := tt.s.Split()
+			require.ElementsMatch(t, tt.wantResult, gotResult)
+			require.Equal(t, strings.Join(gotResult, tt.s.separator), strings.Join(tt.wantResult, tt.s.separator))
+		})
+	}
+}