From 2013ef5a5b7f779fd6a5a88f29ee2ed8f10aec62 Mon Sep 17 00:00:00 2001 From: Brian Bland Date: Wed, 29 Oct 2014 17:42:23 -0700 Subject: [PATCH] Adds documentation for the configuration parser and tests --- configuration/configuration.go | 69 ++++++++++++++++++++++++++--- configuration/configuration_test.go | 21 +++++++++ 2 files changed, 83 insertions(+), 7 deletions(-) diff --git a/configuration/configuration.go b/configuration/configuration.go index 04135bbc..15481559 100644 --- a/configuration/configuration.go +++ b/configuration/configuration.go @@ -10,13 +10,19 @@ import ( "gopkg.in/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 type Configuration struct { Version Version `yaml:"version"` Registry Registry `yaml:"registry"` } +// Version is a major/minor version pair +// Minor version upgrades should be strictly additive +// Major version upgrades indicate structure or type changes type Version struct { Major uint Minor uint @@ -26,17 +32,26 @@ func (version Version) String() string { return fmt.Sprintf("%d.%d", version.Major, version.Minor) } +// MarshalYAML is implemented to serialize the Version into a string format func (version Version) MarshalYAML() (interface{}, error) { return version.String(), nil } +// Registry defines the configuration for a registry type Registry struct { + // LogLevel specifies the level at which the registry will be logged LogLevel string - Storage Storage + + // Storage specifies the configuration of the registry's object storage + Storage Storage } +// Storage defines the configuration for registry object storage type Storage struct { - Type string + // Type specifies the storage driver type (examples: inmemory, filesystem, s3, ...) + Type string + + // Parameters specifies the key/value parameters map passed to the storage driver constructor Parameters map[string]string } @@ -44,16 +59,44 @@ func (storage Storage) MarshalYAML() (interface{}, error) { return yaml.MapSlice{yaml.MapItem{storage.Type, storage.Parameters}}, 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 string `yaml:"version"` + // 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"` } -type v_0_1_RegistryConfiguration struct { - LogLevel string `yaml:"loglevel"` - Storage interface{} `yaml:"storage"` +// 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"` } +// Parse parses an input configuration yaml document into a Configuration struct +// This should 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 func Parse(in []byte) (*Configuration, error) { var untypedConfig untypedConfiguration var config Configuration @@ -65,6 +108,8 @@ func Parse(in []byte) (*Configuration, error) { if untypedConfig.Version == "" { 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) @@ -79,6 +124,7 @@ func Parse(in []byte) (*Configuration, error) { } 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) @@ -100,6 +146,7 @@ func Parse(in []byte) (*Configuration, error) { return &config, nil } +// parseV_0_1_Registry parses a Registry configuration for Version{0, 1} func parseV_0_1_Registry(registry interface{}) (*Registry, error) { envMap := getEnvMap() @@ -107,7 +154,7 @@ func parseV_0_1_Registry(registry interface{}) (*Registry, error) { if err != nil { return nil, err } - var v_0_1 v_0_1_RegistryConfiguration + var v_0_1 V_0_1_RegistryConfiguration err = yaml.Unmarshal(registryBytes, &v_0_1) if err != nil { return nil, err @@ -123,8 +170,10 @@ func parseV_0_1_Registry(registry interface{}) (*Registry, error) { 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)) @@ -136,6 +185,8 @@ func parseV_0_1_Registry(registry interface{}) (*Registry, error) { 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 { @@ -164,6 +215,8 @@ func parseV_0_1_Registry(registry interface{}) (*Registry, error) { return nil, fmt.Errorf("Must provide exactly one storage type, optionally with parameters. Provided: %v", v_0_1.Storage) } + // Find all environment variables of the format: + // REGISTRY_STORAGE__ storageParamsRegexp, err := regexp.Compile(fmt.Sprintf("^REGISTRY_STORAGE_%s_([A-Z0-9]+)$", strings.ToUpper(storage.Type))) if err != nil { return nil, err @@ -177,6 +230,7 @@ func parseV_0_1_Registry(registry interface{}) (*Registry, error) { return &Registry{LogLevel: v_0_1.LogLevel, Storage: storage}, nil } +// getEnvMap reads the current environment variables and converts these into a key/value map func getEnvMap() map[string]string { envMap := make(map[string]string) for _, env := range os.Environ() { @@ -186,6 +240,7 @@ 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 "" diff --git a/configuration/configuration_test.go b/configuration/configuration_test.go index 53dc43a7..8be767fb 100644 --- a/configuration/configuration_test.go +++ b/configuration/configuration_test.go @@ -12,6 +12,7 @@ import ( // Hook up gocheck into the "go test" runner 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, @@ -36,6 +37,7 @@ var configStruct = Configuration{ }, } +// configYamlV_0_1 is a Version{0, 1} yaml document representing configStruct var configYamlV_0_1 = ` version: 0.1 @@ -65,6 +67,8 @@ func (suite *ConfigSuite) SetUpTest(c *C) { suite.expectedConfig = copyConfig(configStruct) } +// TestMarshalRoundtrip validates that configStruct can be marshaled and unmarshaled without +// changing any parameters func (suite *ConfigSuite) TestMarshalRoundtrip(c *C) { configBytes, err := yaml.Marshal(suite.expectedConfig) c.Assert(err, IsNil) @@ -73,12 +77,15 @@ func (suite *ConfigSuite) TestMarshalRoundtrip(c *C) { c.Assert(config, DeepEquals, suite.expectedConfig) } +// TestParseSimple validates that configYamlV_0_1 can be parsed into a struct matching configStruct func (suite *ConfigSuite) TestParseSimple(c *C) { config, err := Parse([]byte(configYamlV_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) { os.Setenv("REGISTRY_STORAGE", "s3") os.Setenv("REGISTRY_STORAGE_S3_REGION", "us-east-1") @@ -88,6 +95,9 @@ func (suite *ConfigSuite) TestParseWithSameEnvStorage(c *C) { c.Assert(config, DeepEquals, suite.expectedConfig) } +// TestParseWithDifferentEnvStorageParams validates that providing environment variables that change +// 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" @@ -102,6 +112,8 @@ func (suite *ConfigSuite) TestParseWithDifferentEnvStorageParams(c *C) { c.Assert(config, DeepEquals, suite.expectedConfig) } +// 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{}} @@ -112,6 +124,9 @@ func (suite *ConfigSuite) TestParseWithDifferentEnvStorageType(c *C) { c.Assert(config, DeepEquals, suite.expectedConfig) } +// TestParseWithDifferentEnvStorageTypeAndParams validates that providing an environment variable +// 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" @@ -124,6 +139,8 @@ func (suite *ConfigSuite) TestParseWithDifferentEnvStorageTypeAndParams(c *C) { c.Assert(config, DeepEquals, suite.expectedConfig) } +// TestParseWithSameEnvLoglevel validates that providing an environment variable defining the log +// level to the same as the one provided in the yaml will not change the parsed Configuration struct func (suite *ConfigSuite) TestParseWithSameEnvLoglevel(c *C) { os.Setenv("REGISTRY_LOGLEVEL", "info") @@ -132,6 +149,8 @@ func (suite *ConfigSuite) TestParseWithSameEnvLoglevel(c *C) { c.Assert(config, DeepEquals, suite.expectedConfig) } +// 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" @@ -142,6 +161,8 @@ func (suite *ConfigSuite) TestParseWithDifferentEnvLoglevel(c *C) { c.Assert(config, DeepEquals, suite.expectedConfig) } +// 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} configBytes, err := yaml.Marshal(suite.expectedConfig)