Adds documentation for the configuration parser and tests

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 {
+// V_0_1_RegistryConfiguration is the unmarshalable Registry configuration struct specific to
-	LogLevel string      `yaml:"loglevel"`
+// Version{0, 1}
-	Storage  interface{} `yaml:"storage"`
+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_<storage driver type>_<parameter name>, 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_<storage driver type>_<parameter name>
 	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 ""

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)