From 3853e66f4bf7ee04fc3e7c65fc672d77174aad3a Mon Sep 17 00:00:00 2001 From: David Lawrence Date: Mon, 9 Mar 2015 10:55:52 -0700 Subject: [PATCH] don't panic during a request when configuring repository middleware. Return a 500 with an appropriate error Signed-off-by: David Lawrence (github: endophage) --- configuration/configuration.go | 2 +- registry/handlers/app.go | 71 +++++++++++++++++++++++---------- registry/storage/layerreader.go | 6 +-- 3 files changed, 53 insertions(+), 26 deletions(-) diff --git a/configuration/configuration.go b/configuration/configuration.go index eae9070d0..6c03b27fa 100644 --- a/configuration/configuration.go +++ b/configuration/configuration.go @@ -300,7 +300,7 @@ type Middleware struct { // Name the middleware registers itself as Name string `yaml:"name"` // Flag to disable middleware easily - Disabled bool `yaml:"Disabled,omitempty"` + Disabled bool `yaml:"disabled,omitempty"` // Map of parameters that will be passed to the middleware's initialization function Options Parameters `yaml:"options"` } diff --git a/registry/handlers/app.go b/registry/handlers/app.go index 8cd7c7390..1b5effbc8 100644 --- a/registry/handlers/app.go +++ b/registry/handlers/app.go @@ -89,16 +89,17 @@ func NewApp(ctx context.Context, configuration configuration.Configuration) *App // a health check. panic(err) } + app.driver, err = applyStorageMiddleware(app.driver, configuration.Middleware["storage"]) + if err != nil { + panic(err) + } app.configureEvents(&configuration) app.registry = storage.NewRegistryWithDriver(app.driver) - for _, mw := range configuration.Middleware["registry"] { - rmw, err := registrymiddleware.Get(mw.Name, mw.Options, app.registry) - if err != nil { - panic(fmt.Sprintf("unable to configure registry middleware (%s): %s", mw.Name, err)) - } - app.registry = rmw + app.registry, err = applyRegistryMiddleware(app.registry, configuration.Middleware["registry"]) + if err != nil { + panic(err) } authType := configuration.Auth.Type() @@ -111,14 +112,6 @@ func NewApp(ctx context.Context, configuration configuration.Configuration) *App app.accessController = accessController } - for _, mw := range configuration.Middleware["storage"] { - smw, err := storagemiddleware.Get(mw.Name, mw.Options, app.driver) - if err != nil { - panic(fmt.Sprintf("unable to configure storage middleware (%s): %v", mw.Name, err)) - } - app.driver = smw - } - return app } @@ -258,12 +251,13 @@ func (app *App) dispatcher(dispatch dispatchFunc) http.Handler { repository, app.eventBridge(context, r)) - for _, mw := range app.Config.Middleware["repository"] { - rmw, err := repositorymiddleware.Get(mw.Name, mw.Options, context.Repository) - if err != nil { - panic(fmt.Sprintf("unable to configure repository middleware (%s): %s", mw.Name, err)) - } - context.Repository = rmw + context.Repository, err = applyRepoMiddleware(context.Repository, app.Config.Middleware["repository"]) + if err != nil { + ctxu.GetLogger(context).Errorf("error initializing repository middleware: %v", err) + context.Errors.Push(v2.ErrorCodeUnknown, err) + w.WriteHeader(http.StatusInternalServerError) + serveJSON(w, context.Errors) + return } } @@ -433,3 +427,40 @@ func appendAccessRecords(records []auth.Access, method string, repo string) []au } return records } + +// applyRegistryMiddleware wraps a registry instance with the configured middlewares +func applyRegistryMiddleware(registry distribution.Registry, middlewares []configuration.Middleware) (distribution.Registry, error) { + for _, mw := range middlewares { + rmw, err := registrymiddleware.Get(mw.Name, mw.Options, registry) + if err != nil { + return nil, fmt.Errorf("unable to configure registry middleware (%s): %s", mw.Name, err) + } + registry = rmw + } + return registry, nil + +} + +// applyRepoMiddleware wraps a repository with the configured middlewares +func applyRepoMiddleware(repository distribution.Repository, middlewares []configuration.Middleware) (distribution.Repository, error) { + for _, mw := range middlewares { + rmw, err := repositorymiddleware.Get(mw.Name, mw.Options, repository) + if err != nil { + return nil, err + } + repository = rmw + } + return repository, nil +} + +// applyStorageMiddleware wraps a storage driver with the configured middlewares +func applyStorageMiddleware(driver storagedriver.StorageDriver, middlewares []configuration.Middleware) (storagedriver.StorageDriver, error) { + for _, mw := range middlewares { + smw, err := storagemiddleware.Get(mw.Name, mw.Options, driver) + if err != nil { + return nil, fmt.Errorf("unable to configure storage middleware (%s): %v", mw.Name, err) + } + driver = smw + } + return driver, nil +} diff --git a/registry/storage/layerreader.go b/registry/storage/layerreader.go index b9b05c5c3..1129eb9e6 100644 --- a/registry/storage/layerreader.go +++ b/registry/storage/layerreader.go @@ -18,10 +18,6 @@ type layerReader struct { var _ distribution.Layer = &layerReader{} -func (lr *layerReader) Path() string { - return lr.path -} - func (lr *layerReader) Digest() digest.Digest { return lr.digest } @@ -42,7 +38,7 @@ func (lr *layerReader) Close() error { func (lr *layerReader) ServeHTTP(w http.ResponseWriter, r *http.Request) { w.Header().Set("Docker-Content-Digest", lr.digest.String()) - if url, err := lr.fileReader.driver.URLFor(lr.Path(), map[string]interface{}{}); err == nil { + if url, err := lr.fileReader.driver.URLFor(lr.path, map[string]interface{}{}); err == nil { http.Redirect(w, r, url, http.StatusTemporaryRedirect) } http.ServeContent(w, r, lr.digest.String(), lr.CreatedAt(), lr)