From 96d26842f8e251743c496e8ff8db22cae9752391 Mon Sep 17 00:00:00 2001 From: Brian Bland Date: Tue, 4 Nov 2014 09:41:32 -0800 Subject: [PATCH] Refactors configuration parser, removes Registry level from config file Most conditional parsing code has been moved into UnmarshalYAML functions for simplicity Uses the BrianBland fork of goyaml in configuration.go temporarily until fix https://github.com/go-yaml/yaml/pull/52 is merged in --- configuration/README.md | 43 ++-- configuration/configuration.go | 335 +++++++++++++++------------- configuration/configuration_test.go | 109 ++++----- 3 files changed, 250 insertions(+), 237 deletions(-) diff --git a/configuration/README.md b/configuration/README.md index edc8eccff..03ac8ab3f 100644 --- a/configuration/README.md +++ b/configuration/README.md @@ -13,41 +13,34 @@ The version is specified as a string of the form `MajorVersion.MinorVersion`, wh File Structure (as of Version 0.1) ------------------------------------ -The configuration structure is defined in `configuration.go`, and is best described by the following two examples: +The configuration structure is defined by the `Configuration` struct in `configuration.go`, and is best described by the following two examples: ```yaml version: 0.1 - -registry: - loglevel: info - storage: - s3: - region: us-east-1 - bucket: my-bucket - rootpath: /registry - encrypt: true - secure: false - accesskey: SAMPLEACCESSKEY - secretkey: SUPERSECRET - host: ~ - port: ~ +loglevel: info +storage: + s3: + region: us-east-1 + bucket: my-bucket + rootpath: /registry + encrypt: true + secure: false + accesskey: SAMPLEACCESSKEY + secretkey: SUPERSECRET + host: ~ + port: ~ ``` ```yaml version: 0.1 - -registry: - loglevel: debug - storage: inmemory +loglevel: debug +storage: inmemory ``` ### version The version is expected to remain a top-level field, as to allow for a consistent version check before parsing the remainder of the configuration file. -### registry -The registry configuration consists of two fields: `loglevel` and `storage` - -#### loglevel +### loglevel This specifies the log level of the registry. Supported values: @@ -56,7 +49,7 @@ Supported values: * `info` * `debug` -#### storage +### storage This specifies the storage driver, and may be provided either as a string (only the driver type) or as a driver name with a parameters map, as seen in the first example above. The parameters map will be passed into the factory constructor of the given storage driver type. @@ -70,7 +63,7 @@ Environment Variables To support the workflow of running a docker registry from a standard container without having to modify configuration files, the registry configuration also supports environment variables for overriding fields. -Any field that is a descendent of `registry` can be replaced by providing an environment variable of the following form: `REGISTRY_[_]...`. +Any configuration field other than version can be replaced by providing an environment variable of the following form: `REGISTRY_[_]...`. For example, to change the loglevel to `error`, one can provide `REGISTRY_LOGLEVEL=error`, and to change the s3 storage driver's region parameter to `us-west-1`, one can provide `REGISTRY_STORAGE_S3_LOGLEVEL=us-west-1`. diff --git a/configuration/configuration.go b/configuration/configuration.go index 154815595..901a25716 100644 --- a/configuration/configuration.go +++ b/configuration/configuration.go @@ -7,99 +7,182 @@ import ( "strconv" "strings" - "gopkg.in/yaml.v2" + "gopkg.in/BrianBland/yaml.v2" ) -// CurrentVersion is the most recent Version that can be parsed -var CurrentVersion = Version{Major: 0, Minor: 1} - -// Configuration is a versioned system configuration -// When marshaled into yaml, this produces a document matching the current version's format +// Configuration is a versioned registry configuration, intended to be provided by a yaml file, and +// optionally modified by environment variables type Configuration struct { + // Version is the version which defines the format of the rest of the configuration Version Version `yaml:"version"` - Registry Registry `yaml:"registry"` + + // Loglevel is the level at which registry operations are logged + Loglevel Loglevel `yaml:"loglevel"` + + // Storage is the configuration for the registry's storage driver + Storage Storage `yaml:"storage"` } -// Version is a major/minor version pair -// Minor version upgrades should be strictly additive +// v_0_1_Configuration is a Version 0.1 Configuration struct +// This is currently aliased to Configuration, as it is the current version +type v_0_1_Configuration Configuration + +// Version is a major/minor version pair of the form Major.Minor // Major version upgrades indicate structure or type changes -type Version struct { - Major uint - Minor uint +// Minor version upgrades should be strictly additive +type Version string + +// MajorMinorVersion constructs a Version from its Major and Minor components +func MajorMinorVersion(major, minor uint) Version { + return Version(fmt.Sprintf("%d.%d", major, minor)) } -func (version Version) String() string { - return fmt.Sprintf("%d.%d", version.Major, version.Minor) +func (version Version) major() (uint, error) { + majorPart := strings.Split(string(version), ".")[0] + major, err := strconv.ParseUint(majorPart, 10, 0) + return uint(major), err } -// MarshalYAML is implemented to serialize the Version into a string format -func (version Version) MarshalYAML() (interface{}, error) { - return version.String(), nil +// Major returns the major version portion of a Version +func (version Version) Major() uint { + major, _ := version.major() + return major } -// Registry defines the configuration for a registry -type Registry struct { - // LogLevel specifies the level at which the registry will be logged - LogLevel string +func (version Version) minor() (uint, error) { + minorPart := strings.Split(string(version), ".")[1] + minor, err := strconv.ParseUint(minorPart, 10, 0) + return uint(minor), err +} - // Storage specifies the configuration of the registry's object storage - Storage Storage +// Minor returns the minor version portion of a Version +func (version Version) Minor() uint { + minor, _ := version.minor() + return minor +} + +// UnmarshalYAML implements the yaml.Unmarshaler interface +// Unmarshals a string of the form X.Y into a Version, validating that X and Y can represent uints +func (version *Version) UnmarshalYAML(unmarshal func(interface{}) error) error { + var versionString string + err := unmarshal(&versionString) + if err != nil { + return err + } + + newVersion := Version(versionString) + if _, err := newVersion.major(); err != nil { + return err + } + + if _, err := newVersion.minor(); err != nil { + return err + } + + *version = newVersion + return nil +} + +// CurrentVersion is the most recent Version that can be parsed +var CurrentVersion = MajorMinorVersion(0, 1) + +// Loglevel is the level at which operations are logged +// This can be error, warn, info, or debug +type Loglevel string + +// UnmarshalYAML implements the yaml.Umarshaler interface +// Unmarshals a string into a Loglevel, lowercasing the string and validating that it represents a +// valid loglevel +func (loglevel *Loglevel) UnmarshalYAML(unmarshal func(interface{}) error) error { + var loglevelString string + err := unmarshal(&loglevelString) + if err != nil { + return err + } + + loglevelString = strings.ToLower(loglevelString) + switch loglevelString { + case "error", "warn", "info", "debug": + default: + return fmt.Errorf("Invalid loglevel %s Must be one of [error, warn, info, debug]", loglevelString) + } + + *loglevel = Loglevel(loglevelString) + return nil } // Storage defines the configuration for registry object storage -type Storage struct { - // Type specifies the storage driver type (examples: inmemory, filesystem, s3, ...) - Type string +type Storage map[string]Parameters - // Parameters specifies the key/value parameters map passed to the storage driver constructor - Parameters map[string]string +// Type returns the storage driver type, such as filesystem or s3 +func (storage Storage) Type() string { + // Return only key in this map + for k := range storage { + return k + } + return "" } +// Parameters returns the Parameters map for a Storage configuration +func (storage Storage) Parameters() Parameters { + return storage[storage.Type()] +} + +// setParameter changes the parameter at the provided key to the new value +func (storage Storage) setParameter(key, value string) { + storage[storage.Type()][key] = value +} + +// UnmarshalYAML implements the yaml.Unmarshaler interface +// Unmarshals a single item map into a Storage or a string into a Storage type with no parameters +func (storage *Storage) UnmarshalYAML(unmarshal func(interface{}) error) error { + var storageMap map[string]Parameters + err := unmarshal(&storageMap) + if err == nil { + if len(storageMap) > 1 { + types := make([]string, 0, len(storageMap)) + for k := range storageMap { + types = append(types, k) + } + return fmt.Errorf("Must provide exactly one storage type. Provided: %v", types) + } + *storage = storageMap + return nil + } + + var storageType string + err = unmarshal(&storageType) + if err == nil { + *storage = Storage{storageType: Parameters{}} + return nil + } + + return err +} + +// MarshalYAML implements the yaml.Marshaler interface func (storage Storage) MarshalYAML() (interface{}, error) { - return yaml.MapSlice{yaml.MapItem{storage.Type, storage.Parameters}}, nil + if storage.Parameters == nil { + return storage.Type, nil + } + return map[string]Parameters(storage), nil } -// untypedConfiguration is the unmarshalable configuration struct that only assumes the existence of -// a version string parameter -// This is done to parse the configuration version, then parse the remainder with a version-specific -// parser -type untypedConfiguration struct { - // Version is the version string defined in a configuration yaml - // This can safely parse versions defined as float types in yaml - Version string `yaml:"version"` - - // Registry is an untyped placeholder for the Registry configuration, which can later be parsed - // into a current Registry struct - Registry interface{} `yaml:"registry"` -} - -// V_0_1_RegistryConfiguration is the unmarshalable Registry configuration struct specific to -// Version{0, 1} -type V_0_1_RegistryConfiguration struct { - // LogLevel is the level at which the registry will log - // The loglevel can be overridden with the environment variable REGISTRY_LOGLEVEL, for example: - // REGISTRY_LOGLEVEL=info - LogLevel string `yaml:"loglevel"` - - // Storage is an untyped placeholder for the Storage configuration, which can later be parsed as - // a Storage struct - // The storage type can be overridden with the environment variable REGISTRY_STORAGE, for - // example: REGISTRY_STORAGE=s3 - // Note: If REGISTRY_STORAGE changes the storage type, all included parameters will be ignored - // The storage parameters can be overridden with any environment variable of the format: - // REGISTRY_STORAGE__, for example: - // REGISTRY_STORAGE_S3_BUCKET=my-bucket - Storage interface{} `yaml:"storage"` -} +// Parameters defines a key-value parameters mapping +type Parameters map[string]string // Parse parses an input configuration yaml document into a Configuration struct -// This should be capable of handling old configuration format versions +// This should generally be capable of handling old configuration format versions // -// Environment variables may be used to override configuration parameters other than version, which -// may be defined on a per-version basis. See V_0_1_RegistryConfiguration for more details +// Environment variables may be used to override configuration parameters other than version, +// following the scheme below: +// Configuration.Abc may be replaced by the value of REGISTRY_ABC, +// Configuration.Abc.Xyz may be replaced by the value of REGISTRY_ABC_XYZ, and so forth func Parse(in []byte) (*Configuration, error) { - var untypedConfig untypedConfiguration - var config Configuration + var untypedConfig struct { + Version Version + } + var config *Configuration err := yaml.Unmarshal(in, &untypedConfig) if err != nil { @@ -109,128 +192,70 @@ func Parse(in []byte) (*Configuration, error) { return nil, fmt.Errorf("Please specify a configuration version. Current version is %s", CurrentVersion) } - // Convert the version string from X.Y to Version{X, Y} - versionParts := strings.Split(untypedConfig.Version, ".") - if len(versionParts) != 2 { - return nil, fmt.Errorf("Invalid version: %s Expected format: X.Y", untypedConfig.Version) - } - majorVersion, err := strconv.ParseUint(versionParts[0], 10, 0) - if err != nil { - return nil, fmt.Errorf("Major version must be of type uint, received %v", versionParts[0]) - } - minorVersion, err := strconv.ParseUint(versionParts[1], 10, 0) - if err != nil { - return nil, fmt.Errorf("Minor version must be of type uint, received %v", versionParts[1]) - } - config.Version = Version{Major: uint(majorVersion), Minor: uint(minorVersion)} - // Parse the remainder of the configuration depending on the provided version - switch config.Version { - case Version{0, 1}: - registry, err := parseV_0_1_Registry(untypedConfig.Registry) + switch untypedConfig.Version { + case "0.1": + config, err = parseV_0_1_Registry(in) if err != nil { return nil, err } - - config.Registry = *registry default: - return nil, fmt.Errorf("Unsupported configuration version %s Current version is %s", config.Version, CurrentVersion) + return nil, fmt.Errorf("Unsupported configuration version %s Current version is %s", untypedConfig.Version, CurrentVersion) } - switch config.Registry.LogLevel { - case "error", "warn", "info", "debug": - default: - return nil, fmt.Errorf("Invalid loglevel %s Must be one of [error, warn, info, debug]", config.Registry.LogLevel) - } - - return &config, nil + return config, nil } -// parseV_0_1_Registry parses a Registry configuration for Version{0, 1} -func parseV_0_1_Registry(registry interface{}) (*Registry, error) { +// parseV_0_1_Registry parses a registry Configuration for Version 0.1 +func parseV_0_1_Registry(in []byte) (*Configuration, error) { envMap := getEnvMap() - registryBytes, err := yaml.Marshal(registry) - if err != nil { - return nil, err - } - var v_0_1 V_0_1_RegistryConfiguration - err = yaml.Unmarshal(registryBytes, &v_0_1) + var config v_0_1_Configuration + err := yaml.Unmarshal(in, &config) if err != nil { return nil, err } - if logLevel, ok := envMap["REGISTRY_LOGLEVEL"]; ok { - v_0_1.LogLevel = logLevel - } - v_0_1.LogLevel = strings.ToLower(v_0_1.LogLevel) - - var storage Storage - storage.Parameters = make(map[string]string) - - switch v_0_1.Storage.(type) { - case string: - // Storage is provided only by type - storage.Type = v_0_1.Storage.(string) - case map[interface{}]interface{}: - // Storage is provided as a {type: parameters} map - storageMap := v_0_1.Storage.(map[interface{}]interface{}) - if len(storageMap) > 1 { - keys := make([]string, 0, len(storageMap)) - for key := range storageMap { - keys = append(keys, toString(key)) - } - return nil, fmt.Errorf("Must provide exactly one storage type. Provided: %v", keys) + // Override config.Loglevel if environment variable is provided + if loglevel, ok := envMap["REGISTRY_LOGLEVEL"]; ok { + var newLoglevel Loglevel + err := yaml.Unmarshal([]byte(loglevel), &newLoglevel) + if err != nil { + return nil, err } - var params map[interface{}]interface{} - // There will only be one key-value pair at this point - for k, v := range storageMap { - // Parameters may be parsed as numerical or boolean values, so just convert these to - // strings - storage.Type = toString(k) - paramsMap, ok := v.(map[interface{}]interface{}) - if !ok { - return nil, fmt.Errorf("Must provide parameters as a map[string]string. Provided: %#v", v) - } - params = paramsMap - } - for k, v := range params { - storage.Parameters[toString(k)] = toString(v) - } - - case interface{}: - // Bad type for storage - return nil, fmt.Errorf("Registry storage must be provided by name, optionally with parameters. Provided: %v", v_0_1.Storage) + config.Loglevel = newLoglevel } + // Override config.Storage if environment variable is provided if storageType, ok := envMap["REGISTRY_STORAGE"]; ok { - if storageType != storage.Type { - storage.Type = storageType + if storageType != config.Storage.Type() { // Reset the storage parameters because we're using a different storage type - storage.Parameters = make(map[string]string) + config.Storage = Storage{storageType: Parameters{}} } } - if storage.Type == "" { - return nil, fmt.Errorf("Must provide exactly one storage type, optionally with parameters. Provided: %v", v_0_1.Storage) + if config.Storage.Type() == "" { + return nil, fmt.Errorf("Must provide exactly one storage type, optionally with parameters. Provided: %v", config.Storage) } - // Find all environment variables of the format: + // Override storage parameters with all environment variables of the format: // REGISTRY_STORAGE__ - storageParamsRegexp, err := regexp.Compile(fmt.Sprintf("^REGISTRY_STORAGE_%s_([A-Z0-9]+)$", strings.ToUpper(storage.Type))) + storageParamsRegexp, err := regexp.Compile(fmt.Sprintf("^REGISTRY_STORAGE_%s_([A-Z0-9]+)$", strings.ToUpper(config.Storage.Type()))) if err != nil { return nil, err } for k, v := range envMap { if submatches := storageParamsRegexp.FindStringSubmatch(k); submatches != nil { - storage.Parameters[strings.ToLower(submatches[1])] = v + config.Storage.setParameter(strings.ToLower(submatches[1]), v) } } - return &Registry{LogLevel: v_0_1.LogLevel, Storage: storage}, nil + return (*Configuration)(&config), nil } // getEnvMap reads the current environment variables and converts these into a key/value map +// This is used to distinguish between empty strings returned by os.GetEnv(key) because of undefined +// environment variables and explicitly empty ones func getEnvMap() map[string]string { envMap := make(map[string]string) for _, env := range os.Environ() { @@ -239,11 +264,3 @@ func getEnvMap() map[string]string { } return envMap } - -// toString converts reasonable objects into strings that may be used for configuration parameters -func toString(v interface{}) string { - if v == nil { - return "" - } - return fmt.Sprint(v) -} diff --git a/configuration/configuration_test.go b/configuration/configuration_test.go index 8be767fb9..cde679e24 100644 --- a/configuration/configuration_test.go +++ b/configuration/configuration_test.go @@ -14,46 +14,46 @@ func Test(t *testing.T) { TestingT(t) } // configStruct is a canonical example configuration, which should map to configYamlV_0_1 var configStruct = Configuration{ - Version: Version{ - Major: 0, - Minor: 1, - }, - Registry: Registry{ - LogLevel: "info", - Storage: Storage{ - Type: "s3", - Parameters: map[string]string{ - "region": "us-east-1", - "bucket": "my-bucket", - "rootpath": "/registry", - "encrypt": "true", - "secure": "false", - "accesskey": "SAMPLEACCESSKEY", - "secretkey": "SUPERSECRET", - "host": "", - "port": "", - }, + Version: "0.1", + Loglevel: "info", + Storage: Storage{ + "s3": Parameters{ + "region": "us-east-1", + "bucket": "my-bucket", + "rootpath": "/registry", + "encrypt": "true", + "secure": "false", + "accesskey": "SAMPLEACCESSKEY", + "secretkey": "SUPERSECRET", + "host": "", + "port": "", }, }, } -// configYamlV_0_1 is a Version{0, 1} yaml document representing configStruct +// configYamlV_0_1 is a Version 0.1 yaml document representing configStruct var configYamlV_0_1 = ` version: 0.1 +loglevel: info +storage: + s3: + region: us-east-1 + bucket: my-bucket + rootpath: /registry + encrypt: true + secure: false + accesskey: SAMPLEACCESSKEY + secretkey: SUPERSECRET + host: ~ + port: ~ +` -registry: - loglevel: info - storage: - s3: - region: us-east-1 - bucket: my-bucket - rootpath: /registry - encrypt: true - secure: false - accesskey: SAMPLEACCESSKEY - secretkey: SUPERSECRET - host: ~ - port: ~ +// inmemoryConfigYamlV_0_1 is a Version 0.1 yaml document specifying an inmemory storage driver with +// no parameters +var inmemoryConfigYamlV_0_1 = ` +version: 0.1 +loglevel: info +storage: inmemory ` type ConfigSuite struct { @@ -84,6 +84,16 @@ func (suite *ConfigSuite) TestParseSimple(c *C) { c.Assert(config, DeepEquals, suite.expectedConfig) } +// TestParseInmemory validates that configuration yaml with storage provided as a string can be +// parsed into a Configuration struct with no storage parameters +func (suite *ConfigSuite) TestParseInmemory(c *C) { + suite.expectedConfig.Storage = Storage{"inmemory": Parameters{}} + + config, err := Parse([]byte(inmemoryConfigYamlV_0_1)) + c.Assert(err, IsNil) + c.Assert(config, DeepEquals, suite.expectedConfig) +} + // TestParseWithSameEnvStorage validates that providing environment variables that match the given // storage type and parameters will not alter the parsed Configuration struct func (suite *ConfigSuite) TestParseWithSameEnvStorage(c *C) { @@ -99,9 +109,9 @@ func (suite *ConfigSuite) TestParseWithSameEnvStorage(c *C) { // and add to the given storage parameters will change and add parameters to the parsed // Configuration struct func (suite *ConfigSuite) TestParseWithDifferentEnvStorageParams(c *C) { - suite.expectedConfig.Registry.Storage.Parameters["region"] = "us-west-1" - suite.expectedConfig.Registry.Storage.Parameters["secure"] = "true" - suite.expectedConfig.Registry.Storage.Parameters["newparam"] = "some Value" + suite.expectedConfig.Storage.setParameter("region", "us-west-1") + suite.expectedConfig.Storage.setParameter("secure", "true") + suite.expectedConfig.Storage.setParameter("newparam", "some Value") os.Setenv("REGISTRY_STORAGE_S3_REGION", "us-west-1") os.Setenv("REGISTRY_STORAGE_S3_SECURE", "true") @@ -115,7 +125,7 @@ func (suite *ConfigSuite) TestParseWithDifferentEnvStorageParams(c *C) { // TestParseWithDifferentEnvStorageType validates that providing an environment variable that // changes the storage type will be reflected in the parsed Configuration struct func (suite *ConfigSuite) TestParseWithDifferentEnvStorageType(c *C) { - suite.expectedConfig.Registry.Storage = Storage{Type: "inmemory", Parameters: map[string]string{}} + suite.expectedConfig.Storage = Storage{"inmemory": Parameters{}} os.Setenv("REGISTRY_STORAGE", "inmemory") @@ -128,8 +138,8 @@ func (suite *ConfigSuite) TestParseWithDifferentEnvStorageType(c *C) { // that changes the storage type will be reflected in the parsed Configuration struct and that // environment storage parameters will also be included func (suite *ConfigSuite) TestParseWithDifferentEnvStorageTypeAndParams(c *C) { - suite.expectedConfig.Registry.Storage = Storage{Type: "filesystem", Parameters: map[string]string{}} - suite.expectedConfig.Registry.Storage.Parameters["rootdirectory"] = "/tmp/testroot" + suite.expectedConfig.Storage = Storage{"filesystem": Parameters{}} + suite.expectedConfig.Storage.setParameter("rootdirectory", "/tmp/testroot") os.Setenv("REGISTRY_STORAGE", "filesystem") os.Setenv("REGISTRY_STORAGE_FILESYSTEM_ROOTDIRECTORY", "/tmp/testroot") @@ -152,7 +162,7 @@ func (suite *ConfigSuite) TestParseWithSameEnvLoglevel(c *C) { // TestParseWithDifferentEnvLoglevel validates that providing an environment variable defining the // log level will override the value provided in the yaml document func (suite *ConfigSuite) TestParseWithDifferentEnvLoglevel(c *C) { - suite.expectedConfig.Registry.LogLevel = "error" + suite.expectedConfig.Loglevel = "error" os.Setenv("REGISTRY_LOGLEVEL", "error") @@ -164,7 +174,7 @@ func (suite *ConfigSuite) TestParseWithDifferentEnvLoglevel(c *C) { // TestParseInvalidVersion validates that the parser will fail to parse a newer configuration // version than the CurrentVersion func (suite *ConfigSuite) TestParseInvalidVersion(c *C) { - suite.expectedConfig.Version = Version{Major: CurrentVersion.Major, Minor: CurrentVersion.Minor + 1} + suite.expectedConfig.Version = MajorMinorVersion(CurrentVersion.Major(), CurrentVersion.Minor()+1) configBytes, err := yaml.Marshal(suite.expectedConfig) c.Assert(err, IsNil) _, err = Parse(configBytes) @@ -174,18 +184,11 @@ func (suite *ConfigSuite) TestParseInvalidVersion(c *C) { func copyConfig(config Configuration) *Configuration { configCopy := new(Configuration) - configCopy.Version = *new(Version) - configCopy.Version.Major = config.Version.Major - configCopy.Version.Minor = config.Version.Minor - - configCopy.Registry = *new(Registry) - configCopy.Registry.LogLevel = config.Registry.LogLevel - - configCopy.Registry.Storage = *new(Storage) - configCopy.Registry.Storage.Type = config.Registry.Storage.Type - configCopy.Registry.Storage.Parameters = make(map[string]string) - for k, v := range config.Registry.Storage.Parameters { - configCopy.Registry.Storage.Parameters[k] = v + configCopy.Version = MajorMinorVersion(config.Version.Major(), config.Version.Minor()) + configCopy.Loglevel = config.Loglevel + configCopy.Storage = Storage{config.Storage.Type(): Parameters{}} + for k, v := range config.Storage.Parameters() { + configCopy.Storage.setParameter(k, v) } return configCopy