From 88795e0a142703ec861a1c416898551e8f0bd4d8 Mon Sep 17 00:00:00 2001 From: Brian Bland Date: Mon, 17 Nov 2014 15:44:07 -0800 Subject: [PATCH] Lots of various golint fixes Changes some names to match go conventions Comments all exported methods Removes dot imports --- client/client.go | 32 ++-- common/names.go | 41 +++- common/tarsum.go | 2 + configuration/configuration.go | 12 +- configuration/configuration_test.go | 28 +-- errors.go | 54 ++++-- images.go | 1 + storage/doc.go | 4 +- .../filesystem/{filesystem.go => driver.go} | 57 +++--- .../{filesystem_test.go => driver_test.go} | 2 +- .../inmemory/{inmemory.go => driver.go} | 51 +++-- .../{inmemory_test.go => driver_test.go} | 7 +- storagedriver/ipc/client.go | 13 ++ storagedriver/ipc/ipc.go | 30 +-- storagedriver/ipc/server.go | 24 ++- storagedriver/s3/s3.go | 68 ++++--- storagedriver/s3/s3_test.go | 7 +- storagedriver/storagedriver.go | 63 ++++--- storagedriver/testsuites/testsuites.go | 178 ++++++++++-------- 19 files changed, 417 insertions(+), 257 deletions(-) rename storagedriver/filesystem/{filesystem.go => driver.go} (64%) rename storagedriver/filesystem/{filesystem_test.go => driver_test.go} (90%) rename storagedriver/inmemory/{inmemory.go => driver.go} (62%) rename storagedriver/inmemory/{inmemory_test.go => driver_test.go} (75%) diff --git a/client/client.go b/client/client.go index dcfa8b2dd..c41586951 100644 --- a/client/client.go +++ b/client/client.go @@ -79,7 +79,7 @@ type clientImpl struct { // TODO(bbland): use consistent route generation between server and client func (r *clientImpl) GetImageManifest(name, tag string) (*registry.ImageManifest, error) { - response, err := http.Get(r.imageManifestUrl(name, tag)) + response, err := http.Get(r.imageManifestURL(name, tag)) if err != nil { return nil, err } @@ -100,7 +100,7 @@ func (r *clientImpl) GetImageManifest(name, tag string) (*registry.ImageManifest } return nil, errors default: - return nil, ®istry.UnexpectedHttpStatusError{Status: response.Status} + return nil, ®istry.UnexpectedHTTPStatusError{Status: response.Status} } decoder := json.NewDecoder(response.Body) @@ -120,7 +120,7 @@ func (r *clientImpl) PutImageManifest(name, tag string, manifest *registry.Image } putRequest, err := http.NewRequest("PUT", - r.imageManifestUrl(name, tag), bytes.NewReader(manifestBytes)) + r.imageManifestURL(name, tag), bytes.NewReader(manifestBytes)) if err != nil { return err } @@ -144,13 +144,13 @@ func (r *clientImpl) PutImageManifest(name, tag string, manifest *registry.Image } return errors default: - return ®istry.UnexpectedHttpStatusError{Status: response.Status} + return ®istry.UnexpectedHTTPStatusError{Status: response.Status} } } func (r *clientImpl) DeleteImage(name, tag string) error { deleteRequest, err := http.NewRequest("DELETE", - r.imageManifestUrl(name, tag), nil) + r.imageManifestURL(name, tag), nil) if err != nil { return err } @@ -176,7 +176,7 @@ func (r *clientImpl) DeleteImage(name, tag string) error { } return errors default: - return ®istry.UnexpectedHttpStatusError{Status: response.Status} + return ®istry.UnexpectedHTTPStatusError{Status: response.Status} } return nil @@ -204,7 +204,7 @@ func (r *clientImpl) ListImageTags(name string) ([]string, error) { } return nil, errors default: - return nil, ®istry.UnexpectedHttpStatusError{Status: response.Status} + return nil, ®istry.UnexpectedHTTPStatusError{Status: response.Status} } tags := struct { @@ -258,7 +258,7 @@ func (r *clientImpl) GetImageLayer(name, tarsum string, byteOffset int) (io.Read return nil, 0, errors default: response.Body.Close() - return nil, 0, ®istry.UnexpectedHttpStatusError{Status: response.Status} + return nil, 0, ®istry.UnexpectedHTTPStatusError{Status: response.Status} } } @@ -290,7 +290,7 @@ func (r *clientImpl) InitiateLayerUpload(name, tarsum string) (string, error) { } return "", errors default: - return "", ®istry.UnexpectedHttpStatusError{Status: response.Status} + return "", ®istry.UnexpectedHTTPStatusError{Status: response.Status} } } @@ -316,7 +316,7 @@ func (r *clientImpl) GetLayerUploadStatus(location string) (int, int, error) { } return 0, 0, errors default: - return 0, 0, ®istry.UnexpectedHttpStatusError{Status: response.Status} + return 0, 0, ®istry.UnexpectedHTTPStatusError{Status: response.Status} } } @@ -358,7 +358,7 @@ func (r *clientImpl) UploadLayer(location string, layer io.ReadCloser, length in } return errors default: - return ®istry.UnexpectedHttpStatusError{Status: response.Status} + return ®istry.UnexpectedHTTPStatusError{Status: response.Status} } } @@ -409,7 +409,7 @@ func (r *clientImpl) UploadLayerChunk(location string, layerChunk io.ReadCloser, } return errors default: - return ®istry.UnexpectedHttpStatusError{Status: response.Status} + return ®istry.UnexpectedHTTPStatusError{Status: response.Status} } } @@ -451,7 +451,7 @@ func (r *clientImpl) FinishChunkedLayerUpload(location string, length int, check } return errors default: - return ®istry.UnexpectedHttpStatusError{Status: response.Status} + return ®istry.UnexpectedHTTPStatusError{Status: response.Status} } } @@ -483,13 +483,13 @@ func (r *clientImpl) CancelLayerUpload(location string) error { } return errors default: - return ®istry.UnexpectedHttpStatusError{Status: response.Status} + return ®istry.UnexpectedHTTPStatusError{Status: response.Status} } } -// imageManifestUrl is a helper method for returning the full url to an image +// imageManifestURL is a helper method for returning the full url to an image // manifest -func (r *clientImpl) imageManifestUrl(name, tag string) string { +func (r *clientImpl) imageManifestURL(name, tag string) string { return fmt.Sprintf("%s/v2/%s/image/%s", r.Endpoint, name, tag) } diff --git a/common/names.go b/common/names.go index 43f7247aa..0172c4806 100644 --- a/common/names.go +++ b/common/names.go @@ -7,11 +7,24 @@ import ( ) const ( + // RepositoryNameComponentMinLength is the minimum number of characters in a + // single repository name slash-delimited component RepositoryNameComponentMinLength = 2 + + // RepositoryNameComponentMaxLength is the maximum number of characters in a + // single repository name slash-delimited component RepositoryNameComponentMaxLength = 30 - RepositoryNameMinComponents = 2 - RepositoryNameMaxComponents = 5 + // RepositoryNameMinComponents is the minimum number of slash-delimited + // components that a repository name must have + RepositoryNameMinComponents = 2 + + // RepositoryNameMaxComponents is the maximum number of slash-delimited + // components that a repository name must have + RepositoryNameMaxComponents = 5 + + // RepositoryNameTotalLengthMax is the maximum total number of characters in + // a repository name RepositoryNameTotalLengthMax = 255 ) @@ -19,6 +32,9 @@ const ( // start with at least two letters or numbers, with following parts able to // separated by one period, dash or underscore. var RepositoryNameComponentRegexp = regexp.MustCompile(`[a-z0-9]+(?:[._-][a-z0-9]+)*`) + +// RepositoryNameComponentAnchoredRegexp is the version of +// RepositoryNameComponentRegexp which must completely match the content var RepositoryNameComponentAnchoredRegexp = regexp.MustCompile(`^` + RepositoryNameComponentRegexp.String() + `$`) // TODO(stevvooe): RepositoryName needs to be limited to some fixed length. @@ -35,13 +51,30 @@ var TagNameRegexp = regexp.MustCompile(`[\w][\w.-]{0,127}`) // TODO(stevvooe): Contribute these exports back to core, so they are shared. var ( + // ErrRepositoryNameComponentShort is returned when a repository name + // contains a component which is shorter than + // RepositoryNameComponentMinLength ErrRepositoryNameComponentShort = fmt.Errorf("respository name component must be %v or more characters", RepositoryNameComponentMinLength) - ErrRepositoryNameComponentLong = fmt.Errorf("respository name component must be %v characters or less", RepositoryNameComponentMaxLength) + // ErrRepositoryNameComponentLong is returned when a repository name + // contains a component which is longer than + // RepositoryNameComponentMaxLength + ErrRepositoryNameComponentLong = fmt.Errorf("respository name component must be %v characters or less", RepositoryNameComponentMaxLength) + + // ErrRepositoryNameMissingComponents is returned when a repository name + // contains fewer than RepositoryNameMinComponents components ErrRepositoryNameMissingComponents = fmt.Errorf("repository name must have at least %v components", RepositoryNameMinComponents) + + // ErrRepositoryNameTooManyComponents is returned when a repository name + // contains more than RepositoryNameMaxComponents components ErrRepositoryNameTooManyComponents = fmt.Errorf("repository name %v or less components", RepositoryNameMaxComponents) - ErrRepositoryNameLong = fmt.Errorf("repository name must not be more than %v characters", RepositoryNameTotalLengthMax) + // ErrRepositoryNameLong is returned when a repository name is longer than + // RepositoryNameTotalLengthMax + ErrRepositoryNameLong = fmt.Errorf("repository name must not be more than %v characters", RepositoryNameTotalLengthMax) + + // ErrRepositoryNameComponentInvalid is returned when a repository name does + // not match RepositoryNameComponentRegexp ErrRepositoryNameComponentInvalid = fmt.Errorf("repository name component must match %q", RepositoryNameComponentRegexp.String()) ) diff --git a/common/tarsum.go b/common/tarsum.go index 5a6e7d215..a1a56d6d8 100644 --- a/common/tarsum.go +++ b/common/tarsum.go @@ -25,6 +25,8 @@ type TarSumInfo struct { Digest string } +// InvalidTarSumError provides informations about a TarSum that cannot be parsed +// by ParseTarSum. type InvalidTarSumError struct { TarSum string } diff --git a/configuration/configuration.go b/configuration/configuration.go index c9bb72e3c..85d74d95d 100644 --- a/configuration/configuration.go +++ b/configuration/configuration.go @@ -23,9 +23,9 @@ type Configuration struct { Storage Storage `yaml:"storage"` } -// v_0_1_Configuration is a Version 0.1 Configuration struct +// v0_1Configuration 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 +type v0_1Configuration Configuration // Version is a major/minor version pair of the form Major.Minor // Major version upgrades indicate structure or type changes @@ -195,7 +195,7 @@ func Parse(in []byte) (*Configuration, error) { // Parse the remainder of the configuration depending on the provided version switch untypedConfig.Version { case "0.1": - config, err = parseV_0_1_Registry(in) + config, err = parseV0_1Registry(in) if err != nil { return nil, err } @@ -206,11 +206,11 @@ 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(in []byte) (*Configuration, error) { +// parseV0_1Registry parses a registry Configuration for Version 0.1 +func parseV0_1Registry(in []byte) (*Configuration, error) { envMap := getEnvMap() - var config v_0_1_Configuration + var config v0_1Configuration err := yaml.Unmarshal(in, &config) if err != nil { return nil, err diff --git a/configuration/configuration_test.go b/configuration/configuration_test.go index cde679e24..31d15b7a0 100644 --- a/configuration/configuration_test.go +++ b/configuration/configuration_test.go @@ -12,7 +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 +// configStruct is a canonical example configuration, which should map to configYamlV0_1 var configStruct = Configuration{ Version: "0.1", Loglevel: "info", @@ -31,8 +31,8 @@ var configStruct = Configuration{ }, } -// configYamlV_0_1 is a Version 0.1 yaml document representing configStruct -var configYamlV_0_1 = ` +// configYamlV0_1 is a Version 0.1 yaml document representing configStruct +var configYamlV0_1 = ` version: 0.1 loglevel: info storage: @@ -48,9 +48,9 @@ storage: port: ~ ` -// inmemoryConfigYamlV_0_1 is a Version 0.1 yaml document specifying an inmemory storage driver with +// inmemoryConfigYamlV0_1 is a Version 0.1 yaml document specifying an inmemory storage driver with // no parameters -var inmemoryConfigYamlV_0_1 = ` +var inmemoryConfigYamlV0_1 = ` version: 0.1 loglevel: info storage: inmemory @@ -77,9 +77,9 @@ 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 +// TestParseSimple validates that configYamlV0_1 can be parsed into a struct matching configStruct func (suite *ConfigSuite) TestParseSimple(c *C) { - config, err := Parse([]byte(configYamlV_0_1)) + config, err := Parse([]byte(configYamlV0_1)) c.Assert(err, IsNil) c.Assert(config, DeepEquals, suite.expectedConfig) } @@ -89,7 +89,7 @@ func (suite *ConfigSuite) TestParseSimple(c *C) { func (suite *ConfigSuite) TestParseInmemory(c *C) { suite.expectedConfig.Storage = Storage{"inmemory": Parameters{}} - config, err := Parse([]byte(inmemoryConfigYamlV_0_1)) + config, err := Parse([]byte(inmemoryConfigYamlV0_1)) c.Assert(err, IsNil) c.Assert(config, DeepEquals, suite.expectedConfig) } @@ -100,7 +100,7 @@ func (suite *ConfigSuite) TestParseWithSameEnvStorage(c *C) { os.Setenv("REGISTRY_STORAGE", "s3") os.Setenv("REGISTRY_STORAGE_S3_REGION", "us-east-1") - config, err := Parse([]byte(configYamlV_0_1)) + config, err := Parse([]byte(configYamlV0_1)) c.Assert(err, IsNil) c.Assert(config, DeepEquals, suite.expectedConfig) } @@ -117,7 +117,7 @@ func (suite *ConfigSuite) TestParseWithDifferentEnvStorageParams(c *C) { os.Setenv("REGISTRY_STORAGE_S3_SECURE", "true") os.Setenv("REGISTRY_STORAGE_S3_NEWPARAM", "some Value") - config, err := Parse([]byte(configYamlV_0_1)) + config, err := Parse([]byte(configYamlV0_1)) c.Assert(err, IsNil) c.Assert(config, DeepEquals, suite.expectedConfig) } @@ -129,7 +129,7 @@ func (suite *ConfigSuite) TestParseWithDifferentEnvStorageType(c *C) { os.Setenv("REGISTRY_STORAGE", "inmemory") - config, err := Parse([]byte(configYamlV_0_1)) + config, err := Parse([]byte(configYamlV0_1)) c.Assert(err, IsNil) c.Assert(config, DeepEquals, suite.expectedConfig) } @@ -144,7 +144,7 @@ func (suite *ConfigSuite) TestParseWithDifferentEnvStorageTypeAndParams(c *C) { os.Setenv("REGISTRY_STORAGE", "filesystem") os.Setenv("REGISTRY_STORAGE_FILESYSTEM_ROOTDIRECTORY", "/tmp/testroot") - config, err := Parse([]byte(configYamlV_0_1)) + config, err := Parse([]byte(configYamlV0_1)) c.Assert(err, IsNil) c.Assert(config, DeepEquals, suite.expectedConfig) } @@ -154,7 +154,7 @@ func (suite *ConfigSuite) TestParseWithDifferentEnvStorageTypeAndParams(c *C) { func (suite *ConfigSuite) TestParseWithSameEnvLoglevel(c *C) { os.Setenv("REGISTRY_LOGLEVEL", "info") - config, err := Parse([]byte(configYamlV_0_1)) + config, err := Parse([]byte(configYamlV0_1)) c.Assert(err, IsNil) c.Assert(config, DeepEquals, suite.expectedConfig) } @@ -166,7 +166,7 @@ func (suite *ConfigSuite) TestParseWithDifferentEnvLoglevel(c *C) { os.Setenv("REGISTRY_LOGLEVEL", "error") - config, err := Parse([]byte(configYamlV_0_1)) + config, err := Parse([]byte(configYamlV0_1)) c.Assert(err, IsNil) c.Assert(config, DeepEquals, suite.expectedConfig) } diff --git a/errors.go b/errors.go index 29cfdd705..abec49655 100644 --- a/errors.go +++ b/errors.go @@ -10,18 +10,43 @@ import ( type ErrorCode int const ( + // ErrorCodeUnknown is a catch-all for errors not defined below. ErrorCodeUnknown ErrorCode = iota // The following errors can happen during a layer upload. + + // ErrorCodeInvalidChecksum is returned when uploading a layer if the + // provided checksum does not match the layer contents. ErrorCodeInvalidChecksum + + // ErrorCodeInvalidLength is returned when uploading a layer if the provided + // length does not match the content length. ErrorCodeInvalidLength + + // ErrorCodeInvalidTarsum is returned when the provided tarsum does not + // match the computed tarsum of the contents. ErrorCodeInvalidTarsum // The following errors can happen during manifest upload. + + // ErrorCodeInvalidName is returned when the name in the manifest does not + // match the provided name. ErrorCodeInvalidName + + // ErrorCodeInvalidTag is returned when the tag in the manifest does not + // match the provided tag. ErrorCodeInvalidTag + + // ErrorCodeUnverifiedManifest is returned when the manifest fails signature + // validation. ErrorCodeUnverifiedManifest + + // ErrorCodeUnknownLayer is returned when the manifest references a + // nonexistent layer. ErrorCodeUnknownLayer + + // ErrorCodeUntrustedSignature is returned when the manifest is signed by an + // untrusted source. ErrorCodeUntrustedSignature ) @@ -83,6 +108,7 @@ func (ec ErrorCode) String() string { return s } +// Message returned the human-readable error message for this error code. func (ec ErrorCode) Message() string { m, ok := errorCodesMessages[ec] @@ -93,16 +119,20 @@ func (ec ErrorCode) Message() string { return m } +// MarshalText encodes the receiver into UTF-8-encoded text and returns the +// result. func (ec ErrorCode) MarshalText() (text []byte, err error) { return []byte(ec.String()), nil } +// UnmarshalText decodes the form generated by MarshalText. func (ec *ErrorCode) UnmarshalText(text []byte) error { *ec = stringToErrorCode[string(text)] return nil } +// Error provides a wrapper around ErrorCode with extra Details provided. type Error struct { Code ErrorCode `json:"code,omitempty"` Message string `json:"message,omitempty"` @@ -173,7 +203,7 @@ type DetailUnknownLayer struct { } // RepositoryNotFoundError is returned when making an operation against a -// repository that does not exist in the registry +// repository that does not exist in the registry. type RepositoryNotFoundError struct { Name string } @@ -183,7 +213,7 @@ func (e *RepositoryNotFoundError) Error() string { } // ImageManifestNotFoundError is returned when making an operation against a -// given image manifest that does not exist in the registry +// given image manifest that does not exist in the registry. type ImageManifestNotFoundError struct { Name string Tag string @@ -195,7 +225,7 @@ func (e *ImageManifestNotFoundError) Error() string { } // LayerAlreadyExistsError is returned when attempting to create a new layer -// that already exists in the registry +// that already exists in the registry. type LayerAlreadyExistsError struct { Name string TarSum string @@ -207,7 +237,7 @@ func (e *LayerAlreadyExistsError) Error() string { } // LayerNotFoundError is returned when making an operation against a given image -// layer that does not exist in the registry +// layer that does not exist in the registry. type LayerNotFoundError struct { Name string TarSum string @@ -221,7 +251,7 @@ func (e *LayerNotFoundError) Error() string { // LayerUploadNotFoundError is returned when making a layer upload operation // against an invalid layer upload location url // This may be the result of using a cancelled, completed, or stale upload -// locationn +// location. type LayerUploadNotFoundError struct { Location string } @@ -232,9 +262,9 @@ func (e *LayerUploadNotFoundError) Error() string { } // LayerUploadInvalidRangeError is returned when attempting to upload an image -// layer chunk that is out of order +// layer chunk that is out of order. // This provides the known LayerSize and LastValidRange which can be used to -// resume the upload +// resume the upload. type LayerUploadInvalidRangeError struct { Location string LastValidRange int @@ -247,12 +277,12 @@ func (e *LayerUploadInvalidRangeError) Error() string { e.Location, e.LastValidRange, e.LayerSize) } -// UnexpectedHttpStatusError is returned when an unexpected http status is -// returned when making a registry api call -type UnexpectedHttpStatusError struct { +// UnexpectedHTTPStatusError is returned when an unexpected HTTP status is +// returned when making a registry api call. +type UnexpectedHTTPStatusError struct { Status string } -func (e *UnexpectedHttpStatusError) Error() string { - return fmt.Sprintf("Received unexpected http status: %s", e.Status) +func (e *UnexpectedHTTPStatusError) Error() string { + return fmt.Sprintf("Received unexpected HTTP status: %s", e.Status) } diff --git a/images.go b/images.go index 84fe217df..e30c6a5fd 100644 --- a/images.go +++ b/images.go @@ -36,6 +36,7 @@ type ImageManifest struct { // imageManifest is used to avoid recursion in unmarshaling type imageManifest ImageManifest +// UnmarshalJSON populates a new ImageManifest struct from JSON data. func (m *ImageManifest) UnmarshalJSON(b []byte) error { var manifest imageManifest err := json.Unmarshal(b, &manifest) diff --git a/storage/doc.go b/storage/doc.go index ecc3fc0b8..387d92348 100644 --- a/storage/doc.go +++ b/storage/doc.go @@ -1,3 +1,3 @@ -// This package contains storage services for use in the registry application. -// It should be considered an internal package, as of Go 1.4. +// Package storage contains storage services for use in the registry +// application. It should be considered an internal package, as of Go 1.4. package storage diff --git a/storagedriver/filesystem/filesystem.go b/storagedriver/filesystem/driver.go similarity index 64% rename from storagedriver/filesystem/filesystem.go rename to storagedriver/filesystem/driver.go index 96c781601..461342593 100644 --- a/storagedriver/filesystem/filesystem.go +++ b/storagedriver/filesystem/driver.go @@ -11,11 +11,11 @@ import ( "github.com/docker/docker-registry/storagedriver/factory" ) -const DriverName = "filesystem" -const DefaultRootDirectory = "/tmp/registry/storage" +const driverName = "filesystem" +const defaultRootDirectory = "/tmp/registry/storage" func init() { - factory.Register(DriverName, &filesystemDriverFactory{}) + factory.Register(driverName, &filesystemDriverFactory{}) } // filesystemDriverFactory implements the factory.StorageDriverFactory interface @@ -25,17 +25,17 @@ func (factory *filesystemDriverFactory) Create(parameters map[string]string) (st return FromParameters(parameters), nil } -// FilesystemDriver is a storagedriver.StorageDriver implementation backed by a local filesystem -// All provided paths will be subpaths of the RootDirectory -type FilesystemDriver struct { +// Driver is a storagedriver.StorageDriver implementation backed by a local +// filesystem. All provided paths will be subpaths of the RootDirectory +type Driver struct { rootDirectory string } -// FromParameters constructs a new FilesystemDriver with a given parameters map +// FromParameters constructs a new Driver with a given parameters map // Optional Parameters: // - rootdirectory -func FromParameters(parameters map[string]string) *FilesystemDriver { - var rootDirectory = DefaultRootDirectory +func FromParameters(parameters map[string]string) *Driver { + var rootDirectory = defaultRootDirectory if parameters != nil { rootDir, ok := parameters["rootdirectory"] if ok { @@ -45,19 +45,20 @@ func FromParameters(parameters map[string]string) *FilesystemDriver { return New(rootDirectory) } -// New constructs a new FilesystemDriver with a given rootDirectory -func New(rootDirectory string) *FilesystemDriver { - return &FilesystemDriver{rootDirectory} +// New constructs a new Driver with a given rootDirectory +func New(rootDirectory string) *Driver { + return &Driver{rootDirectory} } -// subPath returns the absolute path of a key within the FilesystemDriver's storage -func (d *FilesystemDriver) subPath(subPath string) string { +// subPath returns the absolute path of a key within the Driver's storage +func (d *Driver) subPath(subPath string) string { return path.Join(d.rootDirectory, subPath) } // Implement the storagedriver.StorageDriver interface -func (d *FilesystemDriver) GetContent(path string) ([]byte, error) { +// GetContent retrieves the content stored at "path" as a []byte. +func (d *Driver) GetContent(path string) ([]byte, error) { contents, err := ioutil.ReadFile(d.subPath(path)) if err != nil { return nil, storagedriver.PathNotFoundError{Path: path} @@ -65,7 +66,8 @@ func (d *FilesystemDriver) GetContent(path string) ([]byte, error) { return contents, nil } -func (d *FilesystemDriver) PutContent(subPath string, contents []byte) error { +// PutContent stores the []byte content at a location designated by "path". +func (d *Driver) PutContent(subPath string, contents []byte) error { fullPath := d.subPath(subPath) parentDir := path.Dir(fullPath) err := os.MkdirAll(parentDir, 0755) @@ -77,7 +79,9 @@ func (d *FilesystemDriver) PutContent(subPath string, contents []byte) error { return err } -func (d *FilesystemDriver) ReadStream(path string, offset uint64) (io.ReadCloser, error) { +// ReadStream retrieves an io.ReadCloser for the content stored at "path" with a +// given byte offset. +func (d *Driver) ReadStream(path string, offset uint64) (io.ReadCloser, error) { file, err := os.OpenFile(d.subPath(path), os.O_RDONLY, 0644) if err != nil { return nil, err @@ -95,7 +99,9 @@ func (d *FilesystemDriver) ReadStream(path string, offset uint64) (io.ReadCloser return file, nil } -func (d *FilesystemDriver) WriteStream(subPath string, offset, size uint64, reader io.ReadCloser) error { +// WriteStream stores the contents of the provided io.ReadCloser at a location +// designated by the given path. +func (d *Driver) WriteStream(subPath string, offset, size uint64, reader io.ReadCloser) error { defer reader.Close() resumableOffset, err := d.CurrentSize(subPath) @@ -154,7 +160,9 @@ func (d *FilesystemDriver) WriteStream(subPath string, offset, size uint64, read return err } -func (d *FilesystemDriver) CurrentSize(subPath string) (uint64, error) { +// CurrentSize retrieves the curernt size in bytes of the object at the given +// path. +func (d *Driver) CurrentSize(subPath string) (uint64, error) { fullPath := d.subPath(subPath) fileInfo, err := os.Stat(fullPath) @@ -166,7 +174,9 @@ func (d *FilesystemDriver) CurrentSize(subPath string) (uint64, error) { return uint64(fileInfo.Size()), nil } -func (d *FilesystemDriver) List(subPath string) ([]string, error) { +// List returns a list of the objects that are direct descendants of the given +// path. +func (d *Driver) List(subPath string) ([]string, error) { subPath = strings.TrimRight(subPath, "/") fullPath := d.subPath(subPath) @@ -188,12 +198,15 @@ func (d *FilesystemDriver) List(subPath string) ([]string, error) { return keys, nil } -func (d *FilesystemDriver) Move(sourcePath string, destPath string) error { +// Move moves an object stored at sourcePath to destPath, removing the original +// object. +func (d *Driver) Move(sourcePath string, destPath string) error { err := os.Rename(d.subPath(sourcePath), d.subPath(destPath)) return err } -func (d *FilesystemDriver) Delete(subPath string) error { +// Delete recursively deletes all objects stored at "path" and its subpaths. +func (d *Driver) Delete(subPath string) error { fullPath := d.subPath(subPath) _, err := os.Stat(fullPath) diff --git a/storagedriver/filesystem/filesystem_test.go b/storagedriver/filesystem/driver_test.go similarity index 90% rename from storagedriver/filesystem/filesystem_test.go rename to storagedriver/filesystem/driver_test.go index 7eb4024c5..1d9bac54d 100644 --- a/storagedriver/filesystem/filesystem_test.go +++ b/storagedriver/filesystem/driver_test.go @@ -20,5 +20,5 @@ func init() { return New(rootDirectory), nil } testsuites.RegisterInProcessSuite(filesystemDriverConstructor, testsuites.NeverSkip) - testsuites.RegisterIPCSuite(DriverName, map[string]string{"rootdirectory": rootDirectory}, testsuites.NeverSkip) + testsuites.RegisterIPCSuite(driverName, map[string]string{"rootdirectory": rootDirectory}, testsuites.NeverSkip) } diff --git a/storagedriver/inmemory/inmemory.go b/storagedriver/inmemory/driver.go similarity index 62% rename from storagedriver/inmemory/inmemory.go rename to storagedriver/inmemory/driver.go index 14590a3a4..8685eb254 100644 --- a/storagedriver/inmemory/inmemory.go +++ b/storagedriver/inmemory/driver.go @@ -13,34 +13,35 @@ import ( "github.com/docker/docker-registry/storagedriver/factory" ) -const DriverName = "inmemory" +const driverName = "inmemory" func init() { - factory.Register(DriverName, &inMemoryDriverFactory{}) + factory.Register(driverName, &inMemoryDriverFactory{}) } -// inMemoryDriverFacotry implements the factory.StorageDriverFactory interface +// inMemoryDriverFacotry implements the factory.StorageDriverFactory interface. type inMemoryDriverFactory struct{} func (factory *inMemoryDriverFactory) Create(parameters map[string]string) (storagedriver.StorageDriver, error) { return New(), nil } -// InMemoryDriver is a storagedriver.StorageDriver implementation backed by a local map -// Intended solely for example and testing purposes -type InMemoryDriver struct { +// Driver is a storagedriver.StorageDriver implementation backed by a local map. +// Intended solely for example and testing purposes. +type Driver struct { storage map[string][]byte mutex sync.RWMutex } -// New constructs a new InMemoryDriver -func New() *InMemoryDriver { - return &InMemoryDriver{storage: make(map[string][]byte)} +// New constructs a new Driver. +func New() *Driver { + return &Driver{storage: make(map[string][]byte)} } -// Implement the storagedriver.StorageDriver interface +// Implement the storagedriver.StorageDriver interface. -func (d *InMemoryDriver) GetContent(path string) ([]byte, error) { +// GetContent retrieves the content stored at "path" as a []byte. +func (d *Driver) GetContent(path string) ([]byte, error) { d.mutex.RLock() defer d.mutex.RUnlock() contents, ok := d.storage[path] @@ -50,14 +51,17 @@ func (d *InMemoryDriver) GetContent(path string) ([]byte, error) { return contents, nil } -func (d *InMemoryDriver) PutContent(path string, contents []byte) error { +// PutContent stores the []byte content at a location designated by "path". +func (d *Driver) PutContent(path string, contents []byte) error { d.mutex.Lock() defer d.mutex.Unlock() d.storage[path] = contents return nil } -func (d *InMemoryDriver) ReadStream(path string, offset uint64) (io.ReadCloser, error) { +// ReadStream retrieves an io.ReadCloser for the content stored at "path" with a +// given byte offset. +func (d *Driver) ReadStream(path string, offset uint64) (io.ReadCloser, error) { d.mutex.RLock() defer d.mutex.RUnlock() contents, err := d.GetContent(path) @@ -73,7 +77,9 @@ func (d *InMemoryDriver) ReadStream(path string, offset uint64) (io.ReadCloser, return ioutil.NopCloser(bytes.NewReader(buf)), nil } -func (d *InMemoryDriver) WriteStream(path string, offset, size uint64, reader io.ReadCloser) error { +// WriteStream stores the contents of the provided io.ReadCloser at a location +// designated by the given path. +func (d *Driver) WriteStream(path string, offset, size uint64, reader io.ReadCloser) error { defer reader.Close() d.mutex.RLock() defer d.mutex.RUnlock() @@ -100,7 +106,9 @@ func (d *InMemoryDriver) WriteStream(path string, offset, size uint64, reader io return nil } -func (d *InMemoryDriver) CurrentSize(path string) (uint64, error) { +// CurrentSize retrieves the curernt size in bytes of the object at the given +// path. +func (d *Driver) CurrentSize(path string) (uint64, error) { d.mutex.RLock() defer d.mutex.RUnlock() contents, ok := d.storage[path] @@ -110,7 +118,9 @@ func (d *InMemoryDriver) CurrentSize(path string) (uint64, error) { return uint64(len(contents)), nil } -func (d *InMemoryDriver) List(path string) ([]string, error) { +// List returns a list of the objects that are direct descendants of the given +// path. +func (d *Driver) List(path string) ([]string, error) { subPathMatcher, err := regexp.Compile(fmt.Sprintf("^%s/[^/]+", path)) if err != nil { return nil, err @@ -133,7 +143,9 @@ func (d *InMemoryDriver) List(path string) ([]string, error) { return keys, nil } -func (d *InMemoryDriver) Move(sourcePath string, destPath string) error { +// Move moves an object stored at sourcePath to destPath, removing the original +// object. +func (d *Driver) Move(sourcePath string, destPath string) error { d.mutex.Lock() defer d.mutex.Unlock() contents, ok := d.storage[sourcePath] @@ -145,10 +157,11 @@ func (d *InMemoryDriver) Move(sourcePath string, destPath string) error { return nil } -func (d *InMemoryDriver) Delete(path string) error { +// Delete recursively deletes all objects stored at "path" and its subpaths. +func (d *Driver) Delete(path string) error { d.mutex.Lock() defer d.mutex.Unlock() - subPaths := make([]string, 0) + var subPaths []string for k := range d.storage { if strings.HasPrefix(k, path) { subPaths = append(subPaths, k) diff --git a/storagedriver/inmemory/inmemory_test.go b/storagedriver/inmemory/driver_test.go similarity index 75% rename from storagedriver/inmemory/inmemory_test.go rename to storagedriver/inmemory/driver_test.go index feea5eab1..87549542b 100644 --- a/storagedriver/inmemory/inmemory_test.go +++ b/storagedriver/inmemory/driver_test.go @@ -5,16 +5,17 @@ import ( "github.com/docker/docker-registry/storagedriver" "github.com/docker/docker-registry/storagedriver/testsuites" - . "gopkg.in/check.v1" + + "gopkg.in/check.v1" ) // Hook up gocheck into the "go test" runner. -func Test(t *testing.T) { TestingT(t) } +func Test(t *testing.T) { check.TestingT(t) } func init() { inmemoryDriverConstructor := func() (storagedriver.StorageDriver, error) { return New(), nil } testsuites.RegisterInProcessSuite(inmemoryDriverConstructor, testsuites.NeverSkip) - testsuites.RegisterIPCSuite(DriverName, nil, testsuites.NeverSkip) + testsuites.RegisterIPCSuite(driverName, nil, testsuites.NeverSkip) } diff --git a/storagedriver/ipc/client.go b/storagedriver/ipc/client.go index 7f41081a3..332afe1e5 100644 --- a/storagedriver/ipc/client.go +++ b/storagedriver/ipc/client.go @@ -173,6 +173,7 @@ func (driver *StorageDriverClient) Stop() error { // Implement the storagedriver.StorageDriver interface over IPC +// GetContent retrieves the content stored at "path" as a []byte. func (driver *StorageDriverClient) GetContent(path string) ([]byte, error) { if err := driver.exited(); err != nil { return nil, err @@ -204,6 +205,7 @@ func (driver *StorageDriverClient) GetContent(path string) ([]byte, error) { return contents, nil } +// PutContent stores the []byte content at a location designated by "path". func (driver *StorageDriverClient) PutContent(path string, contents []byte) error { if err := driver.exited(); err != nil { return err @@ -230,6 +232,8 @@ func (driver *StorageDriverClient) PutContent(path string, contents []byte) erro return nil } +// ReadStream retrieves an io.ReadCloser for the content stored at "path" with a +// given byte offset. func (driver *StorageDriverClient) ReadStream(path string, offset uint64) (io.ReadCloser, error) { if err := driver.exited(); err != nil { return nil, err @@ -255,6 +259,8 @@ func (driver *StorageDriverClient) ReadStream(path string, offset uint64) (io.Re return response.Reader, nil } +// WriteStream stores the contents of the provided io.ReadCloser at a location +// designated by the given path. func (driver *StorageDriverClient) WriteStream(path string, offset, size uint64, reader io.ReadCloser) error { if err := driver.exited(); err != nil { return err @@ -280,6 +286,8 @@ func (driver *StorageDriverClient) WriteStream(path string, offset, size uint64, return nil } +// CurrentSize retrieves the curernt size in bytes of the object at the given +// path. func (driver *StorageDriverClient) CurrentSize(path string) (uint64, error) { if err := driver.exited(); err != nil { return 0, err @@ -305,6 +313,8 @@ func (driver *StorageDriverClient) CurrentSize(path string) (uint64, error) { return response.Position, nil } +// List returns a list of the objects that are direct descendants of the given +// path. func (driver *StorageDriverClient) List(path string) ([]string, error) { if err := driver.exited(); err != nil { return nil, err @@ -330,6 +340,8 @@ func (driver *StorageDriverClient) List(path string) ([]string, error) { return response.Keys, nil } +// Move moves an object stored at sourcePath to destPath, removing the original +// object. func (driver *StorageDriverClient) Move(sourcePath string, destPath string) error { if err := driver.exited(); err != nil { return err @@ -355,6 +367,7 @@ func (driver *StorageDriverClient) Move(sourcePath string, destPath string) erro return nil } +// Delete recursively deletes all objects stored at "path" and its subpaths. func (driver *StorageDriverClient) Delete(path string) error { if err := driver.exited(); err != nil { return err diff --git a/storagedriver/ipc/ipc.go b/storagedriver/ipc/ipc.go index 00baee1c6..898d10bf9 100644 --- a/storagedriver/ipc/ipc.go +++ b/storagedriver/ipc/ipc.go @@ -9,10 +9,10 @@ import ( "github.com/docker/libchan" ) -// IPCStorageDriver is the interface which IPC storage drivers must implement. As external storage +// StorageDriver is the interface which IPC storage drivers must implement. As external storage // drivers may be defined to use a different version of the storagedriver.StorageDriver interface, // we use an additional version check to determine compatiblity. -type IPCStorageDriver interface { +type StorageDriver interface { // Version returns the storagedriver.StorageDriver interface version which this storage driver // implements, which is used to determine driver compatibility Version() (storagedriver.Version, error) @@ -36,23 +36,25 @@ type Request struct { ResponseChannel libchan.Sender } -type responseError struct { +// ResponseError is a serializable error type. +type ResponseError struct { Type string Message string } -// ResponseError wraps an error in a serializable struct containing the error's type and message -func ResponseError(err error) *responseError { +// WrapError wraps an error in a serializable struct containing the error's type +// and message. +func WrapError(err error) *ResponseError { if err == nil { return nil } - return &responseError{ + return &ResponseError{ Type: reflect.TypeOf(err).String(), Message: err.Error(), } } -func (err *responseError) Error() string { +func (err *ResponseError) Error() string { return fmt.Sprintf("%s: %s", err.Type, err.Message) } @@ -61,38 +63,38 @@ func (err *responseError) Error() string { // VersionResponse is a response for a Version request type VersionResponse struct { Version storagedriver.Version - Error *responseError + Error *ResponseError } // ReadStreamResponse is a response for a ReadStream request type ReadStreamResponse struct { Reader io.ReadCloser - Error *responseError + Error *ResponseError } // WriteStreamResponse is a response for a WriteStream request type WriteStreamResponse struct { - Error *responseError + Error *ResponseError } // CurrentSizeResponse is a response for a CurrentSize request type CurrentSizeResponse struct { Position uint64 - Error *responseError + Error *ResponseError } // ListResponse is a response for a List request type ListResponse struct { Keys []string - Error *responseError + Error *ResponseError } // MoveResponse is a response for a Move request type MoveResponse struct { - Error *responseError + Error *ResponseError } // DeleteResponse is a response for a Delete request type DeleteResponse struct { - Error *responseError + Error *ResponseError } diff --git a/storagedriver/ipc/server.go b/storagedriver/ipc/server.go index f374bf062..71422f931 100644 --- a/storagedriver/ipc/server.go +++ b/storagedriver/ipc/server.go @@ -33,7 +33,9 @@ func StorageDriverServer(driver storagedriver.StorageDriver) error { } else { for { receiver, err := transport.WaitReceiveChannel() - if err != nil { + if err == io.EOF { + return nil + } else if err != nil { panic(err) } go receive(driver, receiver) @@ -49,7 +51,9 @@ func receive(driver storagedriver.StorageDriver, receiver libchan.Receiver) { for { var request Request err := receiver.Receive(&request) - if err != nil { + if err == io.EOF { + return + } else if err != nil { panic(err) } go handleRequest(driver, request) @@ -70,7 +74,7 @@ func handleRequest(driver storagedriver.StorageDriver, request Request) { content, err := driver.GetContent(path) var response ReadStreamResponse if err != nil { - response = ReadStreamResponse{Error: ResponseError(err)} + response = ReadStreamResponse{Error: WrapError(err)} } else { response = ReadStreamResponse{Reader: ioutil.NopCloser(bytes.NewReader(content))} } @@ -87,7 +91,7 @@ func handleRequest(driver storagedriver.StorageDriver, request Request) { err = driver.PutContent(path, contents) } response := WriteStreamResponse{ - Error: ResponseError(err), + Error: WrapError(err), } err = request.ResponseChannel.Send(&response) if err != nil { @@ -100,7 +104,7 @@ func handleRequest(driver storagedriver.StorageDriver, request Request) { reader, err := driver.ReadStream(path, offset) var response ReadStreamResponse if err != nil { - response = ReadStreamResponse{Error: ResponseError(err)} + response = ReadStreamResponse{Error: WrapError(err)} } else { response = ReadStreamResponse{Reader: ioutil.NopCloser(reader)} } @@ -117,7 +121,7 @@ func handleRequest(driver storagedriver.StorageDriver, request Request) { reader, _ := request.Parameters["Reader"].(io.ReadCloser) err := driver.WriteStream(path, offset, size, reader) response := WriteStreamResponse{ - Error: ResponseError(err), + Error: WrapError(err), } err = request.ResponseChannel.Send(&response) if err != nil { @@ -128,7 +132,7 @@ func handleRequest(driver storagedriver.StorageDriver, request Request) { position, err := driver.CurrentSize(path) response := CurrentSizeResponse{ Position: position, - Error: ResponseError(err), + Error: WrapError(err), } err = request.ResponseChannel.Send(&response) if err != nil { @@ -139,7 +143,7 @@ func handleRequest(driver storagedriver.StorageDriver, request Request) { keys, err := driver.List(path) response := ListResponse{ Keys: keys, - Error: ResponseError(err), + Error: WrapError(err), } err = request.ResponseChannel.Send(&response) if err != nil { @@ -150,7 +154,7 @@ func handleRequest(driver storagedriver.StorageDriver, request Request) { destPath, _ := request.Parameters["DestPath"].(string) err := driver.Move(sourcePath, destPath) response := MoveResponse{ - Error: ResponseError(err), + Error: WrapError(err), } err = request.ResponseChannel.Send(&response) if err != nil { @@ -160,7 +164,7 @@ func handleRequest(driver storagedriver.StorageDriver, request Request) { path, _ := request.Parameters["Path"].(string) err := driver.Delete(path) response := DeleteResponse{ - Error: ResponseError(err), + Error: WrapError(err), } err = request.ResponseChannel.Send(&response) if err != nil { diff --git a/storagedriver/s3/s3.go b/storagedriver/s3/s3.go index 7ef7df6b7..82071b2e8 100644 --- a/storagedriver/s3/s3.go +++ b/storagedriver/s3/s3.go @@ -13,7 +13,7 @@ import ( "github.com/docker/docker-registry/storagedriver/factory" ) -const DriverName = "s3" +const driverName = "s3" // minChunkSize defines the minimum multipart upload chunk size // S3 API requires multipart upload chunks to be at least 5MB @@ -23,7 +23,7 @@ const minChunkSize = uint64(5 * 1024 * 1024) const listPartsMax = 1000 func init() { - factory.Register(DriverName, &s3DriverFactory{}) + factory.Register(driverName, &s3DriverFactory{}) } // s3DriverFactory implements the factory.StorageDriverFactory interface @@ -33,22 +33,22 @@ func (factory *s3DriverFactory) Create(parameters map[string]string) (storagedri return FromParameters(parameters) } -// S3Driver is a storagedriver.StorageDriver implementation backed by Amazon S3 +// Driver is a storagedriver.StorageDriver implementation backed by Amazon S3 // Objects are stored at absolute keys in the provided bucket -type S3Driver struct { +type Driver struct { S3 *s3.S3 Bucket *s3.Bucket Encrypt bool } -// FromParameters constructs a new S3Driver with a given parameters map +// FromParameters constructs a new Driver with a given parameters map // Required parameters: // - accesskey // - secretkey // - region // - bucket // - encrypt -func FromParameters(parameters map[string]string) (*S3Driver, error) { +func FromParameters(parameters map[string]string) (*Driver, error) { accessKey, ok := parameters["accesskey"] if !ok || accessKey == "" { return nil, fmt.Errorf("No accesskey parameter provided") @@ -85,9 +85,9 @@ func FromParameters(parameters map[string]string) (*S3Driver, error) { return New(accessKey, secretKey, region, encryptBool, bucket) } -// New constructs a new S3Driver with the given AWS credentials, region, encryption flag, and +// New constructs a new Driver with the given AWS credentials, region, encryption flag, and // bucketName -func New(accessKey string, secretKey string, region aws.Region, encrypt bool, bucketName string) (*S3Driver, error) { +func New(accessKey string, secretKey string, region aws.Region, encrypt bool, bucketName string) (*Driver, error) { auth := aws.Auth{AccessKey: accessKey, SecretKey: secretKey} s3obj := s3.New(auth, region) bucket := s3obj.Bucket(bucketName) @@ -99,20 +99,24 @@ func New(accessKey string, secretKey string, region aws.Region, encrypt bool, bu } } - return &S3Driver{s3obj, bucket, encrypt}, nil + return &Driver{s3obj, bucket, encrypt}, nil } // Implement the storagedriver.StorageDriver interface -func (d *S3Driver) GetContent(path string) ([]byte, error) { +// GetContent retrieves the content stored at "path" as a []byte. +func (d *Driver) GetContent(path string) ([]byte, error) { return d.Bucket.Get(path) } -func (d *S3Driver) PutContent(path string, contents []byte) error { +// PutContent stores the []byte content at a location designated by "path". +func (d *Driver) PutContent(path string, contents []byte) error { return d.Bucket.Put(path, contents, d.getContentType(), getPermissions(), d.getOptions()) } -func (d *S3Driver) ReadStream(path string, offset uint64) (io.ReadCloser, error) { +// ReadStream retrieves an io.ReadCloser for the content stored at "path" with a +// given byte offset. +func (d *Driver) ReadStream(path string, offset uint64) (io.ReadCloser, error) { headers := make(http.Header) headers.Add("Range", "bytes="+strconv.FormatUint(offset, 10)+"-") @@ -124,7 +128,9 @@ func (d *S3Driver) ReadStream(path string, offset uint64) (io.ReadCloser, error) return nil, err } -func (d *S3Driver) WriteStream(path string, offset, size uint64, reader io.ReadCloser) error { +// WriteStream stores the contents of the provided io.ReadCloser at a location +// designated by the given path. +func (d *Driver) WriteStream(path string, offset, size uint64, reader io.ReadCloser) error { defer reader.Close() chunkSize := minChunkSize @@ -177,7 +183,9 @@ func (d *S3Driver) WriteStream(path string, offset, size uint64, reader io.ReadC return nil } -func (d *S3Driver) CurrentSize(path string) (uint64, error) { +// CurrentSize retrieves the curernt size in bytes of the object at the given +// path. +func (d *Driver) CurrentSize(path string) (uint64, error) { _, parts, err := d.getAllParts(path) if err != nil { return 0, err @@ -190,7 +198,9 @@ func (d *S3Driver) CurrentSize(path string) (uint64, error) { return (((uint64(len(parts)) - 1) * uint64(parts[0].Size)) + uint64(parts[len(parts)-1].Size)), nil } -func (d *S3Driver) List(path string) ([]string, error) { +// List returns a list of the objects that are direct descendants of the given +// path. +func (d *Driver) List(path string) ([]string, error) { if path[len(path)-1] != '/' { path = path + "/" } @@ -224,7 +234,9 @@ func (d *S3Driver) List(path string) ([]string, error) { return append(files, directories...), nil } -func (d *S3Driver) Move(sourcePath string, destPath string) error { +// Move moves an object stored at sourcePath to destPath, removing the original +// object. +func (d *Driver) Move(sourcePath string, destPath string) error { /* This is terrible, but aws doesn't have an actual move. */ _, err := d.Bucket.PutCopy(destPath, getPermissions(), s3.CopyOptions{Options: d.getOptions(), MetadataDirective: "", ContentType: d.getContentType()}, @@ -236,7 +248,8 @@ func (d *S3Driver) Move(sourcePath string, destPath string) error { return d.Delete(sourcePath) } -func (d *S3Driver) Delete(path string) error { +// Delete recursively deletes all objects stored at "path" and its subpaths. +func (d *Driver) Delete(path string) error { listResponse, err := d.Bucket.List(path, "", "", listPartsMax) if err != nil || len(listResponse.Contents) == 0 { return storagedriver.PathNotFoundError{Path: path} @@ -263,30 +276,29 @@ func (d *S3Driver) Delete(path string) error { return nil } -func (d *S3Driver) getHighestIdMulti(path string) (multi *s3.Multi, err error) { +func (d *Driver) getHighestIDMulti(path string) (multi *s3.Multi, err error) { multis, _, err := d.Bucket.ListMulti(path, "") if err != nil && !hasCode(err, "NoSuchUpload") { return nil, err } - uploadId := "" + uploadID := "" if len(multis) > 0 { for _, m := range multis { - if m.Key == path && m.UploadId >= uploadId { - uploadId = m.UploadId + if m.Key == path && m.UploadId >= uploadID { + uploadID = m.UploadId multi = m } } return multi, nil - } else { - multi, err := d.Bucket.InitMulti(path, d.getContentType(), getPermissions(), d.getOptions()) - return multi, err } + multi, err = d.Bucket.InitMulti(path, d.getContentType(), getPermissions(), d.getOptions()) + return multi, err } -func (d *S3Driver) getAllParts(path string) (*s3.Multi, []s3.Part, error) { - multi, err := d.getHighestIdMulti(path) +func (d *Driver) getAllParts(path string) (*s3.Multi, []s3.Part, error) { + multi, err := d.getHighestIDMulti(path) if err != nil { return nil, nil, err } @@ -300,7 +312,7 @@ func hasCode(err error, code string) bool { return ok && s3err.Code == code } -func (d *S3Driver) getOptions() s3.Options { +func (d *Driver) getOptions() s3.Options { return s3.Options{SSE: d.Encrypt} } @@ -308,6 +320,6 @@ func getPermissions() s3.ACL { return s3.Private } -func (d *S3Driver) getContentType() string { +func (d *Driver) getContentType() string { return "application/octet-stream" } diff --git a/storagedriver/s3/s3_test.go b/storagedriver/s3/s3_test.go index 576c36231..6d7b3ff74 100644 --- a/storagedriver/s3/s3_test.go +++ b/storagedriver/s3/s3_test.go @@ -8,11 +8,12 @@ import ( "github.com/crowdmob/goamz/aws" "github.com/docker/docker-registry/storagedriver" "github.com/docker/docker-registry/storagedriver/testsuites" - . "gopkg.in/check.v1" + + "gopkg.in/check.v1" ) // Hook up gocheck into the "go test" runner. -func Test(t *testing.T) { TestingT(t) } +func Test(t *testing.T) { check.TestingT(t) } func init() { accessKey := os.Getenv("AWS_ACCESS_KEY") @@ -38,7 +39,7 @@ func init() { } testsuites.RegisterInProcessSuite(s3DriverConstructor, skipCheck) - testsuites.RegisterIPCSuite(DriverName, map[string]string{ + testsuites.RegisterIPCSuite(driverName, map[string]string{ "accesskey": accessKey, "secretkey": secretKey, "region": region, diff --git a/storagedriver/storagedriver.go b/storagedriver/storagedriver.go index fcf691c66..1b6c5c00d 100644 --- a/storagedriver/storagedriver.go +++ b/storagedriver/storagedriver.go @@ -7,67 +7,73 @@ import ( "strings" ) -// Version is a string representing the storage driver version, of the form Major.Minor. -// The registry must accept storage drivers with equal major version and greater minor version, -// but may not be compatible with older storage driver versions. +// Version is a string representing the storage driver version, of the form +// Major.Minor. +// The registry must accept storage drivers with equal major version and greater +// minor version, but may not be compatible with older storage driver versions. type Version string -// Major returns the major (primary) component of a version +// Major returns the major (primary) component of a version. func (version Version) Major() uint { majorPart := strings.Split(string(version), ".")[0] major, _ := strconv.ParseUint(majorPart, 10, 0) return uint(major) } -// Minor returns the minor (secondary) component of a version +// Minor returns the minor (secondary) component of a version. func (version Version) Minor() uint { minorPart := strings.Split(string(version), ".")[1] minor, _ := strconv.ParseUint(minorPart, 10, 0) return uint(minor) } -// CurrentVersion is the current storage driver Version +// CurrentVersion is the current storage driver Version. const CurrentVersion Version = "0.1" -// StorageDriver defines methods that a Storage Driver must implement for a filesystem-like -// key/value object storage +// StorageDriver defines methods that a Storage Driver must implement for a +// filesystem-like key/value object storage. type StorageDriver interface { - // GetContent retrieves the content stored at "path" as a []byte - // Should primarily be used for small objects + // GetContent retrieves the content stored at "path" as a []byte. + // This should primarily be used for small objects. GetContent(path string) ([]byte, error) - // PutContent stores the []byte content at a location designated by "path" - // Should primarily be used for small objects + // PutContent stores the []byte content at a location designated by "path". + // This should primarily be used for small objects. PutContent(path string, content []byte) error - // ReadStream retrieves an io.ReadCloser for the content stored at "path" with a given byte - // offset - // May be used to resume reading a stream by providing a nonzero offset + // ReadStream retrieves an io.ReadCloser for the content stored at "path" + // with a given byte offset. + // May be used to resume reading a stream by providing a nonzero offset. ReadStream(path string, offset uint64) (io.ReadCloser, error) - // WriteStream stores the contents of the provided io.ReadCloser at a location designated by - // the given path - // The driver will know it has received the full contents when it has read "size" bytes - // May be used to resume writing a stream by providing a nonzero offset - // The offset must be no larger than the CurrentSize for this path + // WriteStream stores the contents of the provided io.ReadCloser at a + // location designated by the given path. + // The driver will know it has received the full contents when it has read + // "size" bytes. + // May be used to resume writing a stream by providing a nonzero offset. + // The offset must be no larger than the CurrentSize for this path. WriteStream(path string, offset, size uint64, readCloser io.ReadCloser) error - // CurrentSize retrieves the curernt size in bytes of the object at the given path - // It should be safe to read or write anywhere up to this point + // CurrentSize retrieves the curernt size in bytes of the object at the + // given path. + // It should be safe to read or write anywhere up to this point. CurrentSize(path string) (uint64, error) - // List returns a list of the objects that are direct descendants of the given path + // List returns a list of the objects that are direct descendants of the + //given path. List(path string) ([]string, error) - // Move moves an object stored at sourcePath to destPath, removing the original object - // Note: This may be no more efficient than a copy followed by a delete for many implementations + // Move moves an object stored at sourcePath to destPath, removing the + // original object. + // Note: This may be no more efficient than a copy followed by a delete for + // many implementations. Move(sourcePath string, destPath string) error - // Delete recursively deletes all objects stored at "path" and its subpaths + // Delete recursively deletes all objects stored at "path" and its subpaths. Delete(path string) error } -// PathNotFoundError is returned when operating on a nonexistent path +// PathNotFoundError is returned when operating on a nonexistent path. type PathNotFoundError struct { Path string } @@ -76,7 +82,8 @@ func (err PathNotFoundError) Error() string { return fmt.Sprintf("Path not found: %s", err.Path) } -// InvalidOffsetError is returned when attempting to read or write from an invalid offset +// InvalidOffsetError is returned when attempting to read or write from an +// invalid offset. type InvalidOffsetError struct { Path string Offset uint64 diff --git a/storagedriver/testsuites/testsuites.go b/storagedriver/testsuites/testsuites.go index d2859913d..217237f7b 100644 --- a/storagedriver/testsuites/testsuites.go +++ b/storagedriver/testsuites/testsuites.go @@ -11,15 +11,15 @@ import ( "github.com/docker/docker-registry/storagedriver" "github.com/docker/docker-registry/storagedriver/ipc" - . "gopkg.in/check.v1" + "gopkg.in/check.v1" ) -// Hook up gocheck into the "go test" runner -func Test(t *testing.T) { TestingT(t) } +// Test hooks up gocheck into the "go test" runner. +func Test(t *testing.T) { check.TestingT(t) } // RegisterInProcessSuite registers an in-process storage driver test suite with the go test runner func RegisterInProcessSuite(driverConstructor DriverConstructor, skipCheck SkipCheck) { - Suite(&DriverSuite{ + check.Suite(&DriverSuite{ Constructor: driverConstructor, SkipCheck: skipCheck, }) @@ -50,7 +50,7 @@ func RegisterIPCSuite(driverName string, ipcParams map[string]string, skipCheck driverClient := suite.StorageDriver.(*ipc.StorageDriverClient) return driverClient.Stop() } - Suite(suite) + check.Suite(suite) } // SkipCheck is a function used to determine if a test suite should be skipped @@ -75,77 +75,93 @@ type DriverSuite struct { storagedriver.StorageDriver } -func (suite *DriverSuite) SetUpSuite(c *C) { +// SetUpSuite sets up the gocheck test suite +func (suite *DriverSuite) SetUpSuite(c *check.C) { if reason := suite.SkipCheck(); reason != "" { c.Skip(reason) } d, err := suite.Constructor() - c.Assert(err, IsNil) + c.Assert(err, check.IsNil) suite.StorageDriver = d } -func (suite *DriverSuite) TearDownSuite(c *C) { +// TearDownSuite tears down the gocheck test suite +func (suite *DriverSuite) TearDownSuite(c *check.C) { if suite.Teardown != nil { err := suite.Teardown() - c.Assert(err, IsNil) + c.Assert(err, check.IsNil) } } -func (suite *DriverSuite) TestWriteRead1(c *C) { +// TestWriteRead1 tests a simple write-read workflow +func (suite *DriverSuite) TestWriteRead1(c *check.C) { filename := randomString(32) contents := []byte("a") suite.writeReadCompare(c, filename, contents, contents) } -func (suite *DriverSuite) TestWriteRead2(c *C) { +// TestWriteRead2 tests a simple write-read workflow with unicode data +func (suite *DriverSuite) TestWriteRead2(c *check.C) { filename := randomString(32) contents := []byte("\xc3\x9f") suite.writeReadCompare(c, filename, contents, contents) } -func (suite *DriverSuite) TestWriteRead3(c *C) { +// TestWriteRead3 tests a simple write-read workflow with a small string +func (suite *DriverSuite) TestWriteRead3(c *check.C) { filename := randomString(32) contents := []byte(randomString(32)) suite.writeReadCompare(c, filename, contents, contents) } -func (suite *DriverSuite) TestWriteRead4(c *C) { +// TestWriteRead4 tests a simple write-read workflow with 1MB of data +func (suite *DriverSuite) TestWriteRead4(c *check.C) { filename := randomString(32) contents := []byte(randomString(1024 * 1024)) suite.writeReadCompare(c, filename, contents, contents) } -func (suite *DriverSuite) TestReadNonexistent(c *C) { +// TestReadNonexistent tests reading content from an empty path +func (suite *DriverSuite) TestReadNonexistent(c *check.C) { filename := randomString(32) _, err := suite.StorageDriver.GetContent(filename) - c.Assert(err, NotNil) + c.Assert(err, check.NotNil) } -func (suite *DriverSuite) TestWriteReadStreams1(c *C) { +// TestWriteReadStreams1 tests a simple write-read streaming workflow +func (suite *DriverSuite) TestWriteReadStreams1(c *check.C) { filename := randomString(32) contents := []byte("a") suite.writeReadCompareStreams(c, filename, contents, contents) } -func (suite *DriverSuite) TestWriteReadStreams2(c *C) { +// TestWriteReadStreams2 tests a simple write-read streaming workflow with +// unicode data +func (suite *DriverSuite) TestWriteReadStreams2(c *check.C) { filename := randomString(32) contents := []byte("\xc3\x9f") suite.writeReadCompareStreams(c, filename, contents, contents) } -func (suite *DriverSuite) TestWriteReadStreams3(c *C) { +// TestWriteReadStreams3 tests a simple write-read streaming workflow with a +// small amount of data +func (suite *DriverSuite) TestWriteReadStreams3(c *check.C) { filename := randomString(32) contents := []byte(randomString(32)) suite.writeReadCompareStreams(c, filename, contents, contents) } -func (suite *DriverSuite) TestWriteReadStreams4(c *C) { +// TestWriteReadStreams4 tests a simple write-read streaming workflow with 1MB +// of data +func (suite *DriverSuite) TestWriteReadStreams4(c *check.C) { filename := randomString(32) contents := []byte(randomString(1024 * 1024)) suite.writeReadCompareStreams(c, filename, contents, contents) } -func (suite *DriverSuite) TestContinueStreamAppend(c *C) { +// TestContinueStreamAppend tests that a stream write can be appended to without +// corrupting the data +func (suite *DriverSuite) TestContinueStreamAppend(c *check.C) { filename := randomString(32) defer suite.StorageDriver.Delete(filename) @@ -158,31 +174,33 @@ func (suite *DriverSuite) TestContinueStreamAppend(c *C) { fullContents := append(append(contentsChunk1, contentsChunk2...), contentsChunk3...) err := suite.StorageDriver.WriteStream(filename, 0, 3*chunkSize, ioutil.NopCloser(bytes.NewReader(contentsChunk1))) - c.Assert(err, IsNil) + c.Assert(err, check.IsNil) offset, err := suite.StorageDriver.CurrentSize(filename) - c.Assert(err, IsNil) + c.Assert(err, check.IsNil) if offset > chunkSize { c.Fatalf("Offset too large, %d > %d", offset, chunkSize) } err = suite.StorageDriver.WriteStream(filename, offset, 3*chunkSize, ioutil.NopCloser(bytes.NewReader(fullContents[offset:2*chunkSize]))) - c.Assert(err, IsNil) + c.Assert(err, check.IsNil) offset, err = suite.StorageDriver.CurrentSize(filename) - c.Assert(err, IsNil) + c.Assert(err, check.IsNil) if offset > 2*chunkSize { c.Fatalf("Offset too large, %d > %d", offset, 2*chunkSize) } err = suite.StorageDriver.WriteStream(filename, offset, 3*chunkSize, ioutil.NopCloser(bytes.NewReader(fullContents[offset:]))) - c.Assert(err, IsNil) + c.Assert(err, check.IsNil) received, err := suite.StorageDriver.GetContent(filename) - c.Assert(err, IsNil) - c.Assert(received, DeepEquals, fullContents) + c.Assert(err, check.IsNil) + c.Assert(received, check.DeepEquals, fullContents) } -func (suite *DriverSuite) TestReadStreamWithOffset(c *C) { +// TestReadStreamWithOffset tests that the appropriate data is streamed when +// reading with a given offset +func (suite *DriverSuite) TestReadStreamWithOffset(c *check.C) { filename := randomString(32) defer suite.StorageDriver.Delete(filename) @@ -193,43 +211,46 @@ func (suite *DriverSuite) TestReadStreamWithOffset(c *C) { contentsChunk3 := []byte(randomString(chunkSize)) err := suite.StorageDriver.PutContent(filename, append(append(contentsChunk1, contentsChunk2...), contentsChunk3...)) - c.Assert(err, IsNil) + c.Assert(err, check.IsNil) reader, err := suite.StorageDriver.ReadStream(filename, 0) - c.Assert(err, IsNil) + c.Assert(err, check.IsNil) defer reader.Close() readContents, err := ioutil.ReadAll(reader) - c.Assert(err, IsNil) + c.Assert(err, check.IsNil) - c.Assert(readContents, DeepEquals, append(append(contentsChunk1, contentsChunk2...), contentsChunk3...)) + c.Assert(readContents, check.DeepEquals, append(append(contentsChunk1, contentsChunk2...), contentsChunk3...)) reader, err = suite.StorageDriver.ReadStream(filename, chunkSize) - c.Assert(err, IsNil) + c.Assert(err, check.IsNil) defer reader.Close() readContents, err = ioutil.ReadAll(reader) - c.Assert(err, IsNil) + c.Assert(err, check.IsNil) - c.Assert(readContents, DeepEquals, append(contentsChunk2, contentsChunk3...)) + c.Assert(readContents, check.DeepEquals, append(contentsChunk2, contentsChunk3...)) reader, err = suite.StorageDriver.ReadStream(filename, chunkSize*2) - c.Assert(err, IsNil) + c.Assert(err, check.IsNil) defer reader.Close() readContents, err = ioutil.ReadAll(reader) - c.Assert(err, IsNil) + c.Assert(err, check.IsNil) - c.Assert(readContents, DeepEquals, contentsChunk3) + c.Assert(readContents, check.DeepEquals, contentsChunk3) } -func (suite *DriverSuite) TestReadNonexistentStream(c *C) { +// TestReadNonexistentStream tests that reading a stream for a nonexistent path +// fails +func (suite *DriverSuite) TestReadNonexistentStream(c *check.C) { filename := randomString(32) _, err := suite.StorageDriver.ReadStream(filename, 0) - c.Assert(err, NotNil) + c.Assert(err, check.NotNil) } -func (suite *DriverSuite) TestList(c *C) { +// TestList checks the returned list of keys after populating a directory tree +func (suite *DriverSuite) TestList(c *check.C) { rootDirectory := randomString(uint64(8 + rand.Intn(8))) defer suite.StorageDriver.Delete(rootDirectory) @@ -239,22 +260,24 @@ func (suite *DriverSuite) TestList(c *C) { childFile := parentDirectory + "/" + randomString(uint64(8+rand.Intn(8))) childFiles[i] = childFile err := suite.StorageDriver.PutContent(childFile, []byte(randomString(32))) - c.Assert(err, IsNil) + c.Assert(err, check.IsNil) } sort.Strings(childFiles) keys, err := suite.StorageDriver.List(rootDirectory) - c.Assert(err, IsNil) - c.Assert(keys, DeepEquals, []string{parentDirectory}) + c.Assert(err, check.IsNil) + c.Assert(keys, check.DeepEquals, []string{parentDirectory}) keys, err = suite.StorageDriver.List(parentDirectory) - c.Assert(err, IsNil) + c.Assert(err, check.IsNil) sort.Strings(keys) - c.Assert(keys, DeepEquals, childFiles) + c.Assert(keys, check.DeepEquals, childFiles) } -func (suite *DriverSuite) TestMove(c *C) { +// TestMove checks that a moved object no longer exists at the source path and +// does exist at the destination +func (suite *DriverSuite) TestMove(c *check.C) { contents := []byte(randomString(32)) sourcePath := randomString(32) destPath := randomString(32) @@ -263,50 +286,55 @@ func (suite *DriverSuite) TestMove(c *C) { defer suite.StorageDriver.Delete(destPath) err := suite.StorageDriver.PutContent(sourcePath, contents) - c.Assert(err, IsNil) + c.Assert(err, check.IsNil) err = suite.StorageDriver.Move(sourcePath, destPath) - c.Assert(err, IsNil) + c.Assert(err, check.IsNil) received, err := suite.StorageDriver.GetContent(destPath) - c.Assert(err, IsNil) - c.Assert(received, DeepEquals, contents) + c.Assert(err, check.IsNil) + c.Assert(received, check.DeepEquals, contents) _, err = suite.StorageDriver.GetContent(sourcePath) - c.Assert(err, NotNil) + c.Assert(err, check.NotNil) } -func (suite *DriverSuite) TestMoveNonexistent(c *C) { +// TestMoveNonexistent checks that moving a nonexistent key fails +func (suite *DriverSuite) TestMoveNonexistent(c *check.C) { sourcePath := randomString(32) destPath := randomString(32) err := suite.StorageDriver.Move(sourcePath, destPath) - c.Assert(err, NotNil) + c.Assert(err, check.NotNil) } -func (suite *DriverSuite) TestRemove(c *C) { +// TestDelete checks that the delete operation removes data from the storage +// driver +func (suite *DriverSuite) TestDelete(c *check.C) { filename := randomString(32) contents := []byte(randomString(32)) defer suite.StorageDriver.Delete(filename) err := suite.StorageDriver.PutContent(filename, contents) - c.Assert(err, IsNil) + c.Assert(err, check.IsNil) err = suite.StorageDriver.Delete(filename) - c.Assert(err, IsNil) + c.Assert(err, check.IsNil) _, err = suite.StorageDriver.GetContent(filename) - c.Assert(err, NotNil) + c.Assert(err, check.NotNil) } -func (suite *DriverSuite) TestRemoveNonexistent(c *C) { +// TestDeleteNonexistent checks that removing a nonexistent key fails +func (suite *DriverSuite) TestDeleteNonexistent(c *check.C) { filename := randomString(32) err := suite.StorageDriver.Delete(filename) - c.Assert(err, NotNil) + c.Assert(err, check.NotNil) } -func (suite *DriverSuite) TestRemoveFolder(c *C) { +// TestDeleteFolder checks that deleting a folder removes all child elements +func (suite *DriverSuite) TestDeleteFolder(c *check.C) { dirname := randomString(32) filename1 := randomString(32) filename2 := randomString(32) @@ -316,47 +344,47 @@ func (suite *DriverSuite) TestRemoveFolder(c *C) { defer suite.StorageDriver.Delete(path.Join(dirname, filename2)) err := suite.StorageDriver.PutContent(path.Join(dirname, filename1), contents) - c.Assert(err, IsNil) + c.Assert(err, check.IsNil) err = suite.StorageDriver.PutContent(path.Join(dirname, filename2), contents) - c.Assert(err, IsNil) + c.Assert(err, check.IsNil) err = suite.StorageDriver.Delete(dirname) - c.Assert(err, IsNil) + c.Assert(err, check.IsNil) _, err = suite.StorageDriver.GetContent(path.Join(dirname, filename1)) - c.Assert(err, NotNil) + c.Assert(err, check.NotNil) _, err = suite.StorageDriver.GetContent(path.Join(dirname, filename2)) - c.Assert(err, NotNil) + c.Assert(err, check.NotNil) } -func (suite *DriverSuite) writeReadCompare(c *C, filename string, contents, expected []byte) { +func (suite *DriverSuite) writeReadCompare(c *check.C, filename string, contents, expected []byte) { defer suite.StorageDriver.Delete(filename) err := suite.StorageDriver.PutContent(filename, contents) - c.Assert(err, IsNil) + c.Assert(err, check.IsNil) readContents, err := suite.StorageDriver.GetContent(filename) - c.Assert(err, IsNil) + c.Assert(err, check.IsNil) - c.Assert(readContents, DeepEquals, contents) + c.Assert(readContents, check.DeepEquals, contents) } -func (suite *DriverSuite) writeReadCompareStreams(c *C, filename string, contents, expected []byte) { +func (suite *DriverSuite) writeReadCompareStreams(c *check.C, filename string, contents, expected []byte) { defer suite.StorageDriver.Delete(filename) err := suite.StorageDriver.WriteStream(filename, 0, uint64(len(contents)), ioutil.NopCloser(bytes.NewReader(contents))) - c.Assert(err, IsNil) + c.Assert(err, check.IsNil) reader, err := suite.StorageDriver.ReadStream(filename, 0) - c.Assert(err, IsNil) + c.Assert(err, check.IsNil) defer reader.Close() readContents, err := ioutil.ReadAll(reader) - c.Assert(err, IsNil) + c.Assert(err, check.IsNil) - c.Assert(readContents, DeepEquals, contents) + c.Assert(readContents, check.DeepEquals, contents) } var pathChars = []byte("abcdefghijklmnopqrstuvwxyz")