Repository: distribution/distribution (go)

Issue:
panic: unexpected end of JSON input
It looks like if `scheduler-state.json` is empty (crash or node issue during writing the file? or running out of disk space?), the registry will then crash on startup, until this file is removed.

Perhaps if the state file is corrupted it could recover automatically with a blank state?

Logs:
```
time="2017-08-18T15:17:05Z" level=info msg="redis not configured" go.version=go1.7.3 instance.id=9a8f36a4-5003-4b89-82ba-68f2a2346646 version=v2.6.0 
time="2017-08-18T15:17:05Z" level=info msg="Starting upload purge in 25m0s" go.version=go1.7.3 instance.id=9a8f36a4-5003-4b89-82ba-68f2a2346646 version=v2.6.0 
panic: unexpected end of JSON input

goroutine 1 [running]:
panic(0xb4cc80, 0xc420396490)
	/usr/local/go/src/runtime/panic.go:500 +0x1a1
github.com/docker/distribution/registry/handlers.NewApp(0x10667c0, 0xc4203b8750, 0xc42036aa80, 0x10667c0)
	/go/src/github.com/docker/distribution/registry/handlers/app.go:312 +0x17d4
github.com/docker/distribution/registry.NewRegistry(0x7fad44e9c040, 0xc4203b8750, 0xc42036aa80, 0xe, 0x0, 0x0)
	/go/src/github.com/docker/distribution/registry/registry.go:86 +0x213
github.com/docker/distribution/registry.glob..func1(0x108e140, 0xc4203963d0, 0x1, 0x1)
	/go/src/github.com/docker/distribution/registry/registry.go:55 +0x106
github.com/docker/distribution/vendor/github.com/spf13/cobra.(*Command).execute(0x108e140, 0xc420396380, 0x1, 0x1, 0x108e140, 0xc420396380)
	/go/src/github.com/docker/distribution/vendor/github.com/spf13/cobra/command.go:495 +0x190
github.com/docker/distribution/vendor/github.com/spf13/cobra.(*Command).Execute(0x108e2e0, 0xc4201b7f40, 0xc4200001a0)
	/go/src/github.com/docker/distribution/vendor/github.com/spf13/cobra/command.go:560 +0x3c3
main.main()
	/go/src/github.com/docker/distribution/cmd/registry/main.go:24 +0x2d
time="2017-08-18T15:17:05Z" level=info msg="using inmemory blob descriptor cache" go.version=go1.7.3 instance.id=9a8f36a4-5003-4b89-82ba-68f2a2346646 version=v2.6.0 
```

Pull request:
Stop proxy scheduler on system exit
For pull through cache registry with configured TTL, a scheduler responsible for blobs garbage collection is started here: [registry/proxy/proxyregistry.go#L109](https://github.com/distribution/distribution/blob/51a72c2aef976bd55de3a7b8b0120f97b4169476/registry/proxy/proxyregistry.go#L109), but its [Stop](https://github.com/distribution/distribution/blob/51a72c2aef976bd55de3a7b8b0120f97b4169476/registry/proxy/scheduler/scheduler.go#L208-L209) method is never invoked.

Scheduler state is written to `scheduler-state.json` if a state change is detected every 5 seconds.
When terminating the registry process, this can result in an inconsistent/corrupted `scheduler-state.json`:
- recent state changes are not saved at all
- `scheduler-state.json` has been truncated, but no new content has been written

This PR explicitly invokes scheduler Stop method on `SIGINT` & `SIGTERM` to ensure that scheduler state is preserved.

Fixes #2374 

Patch:
diff --git a/docs/content/about/configuration.md b/docs/content/about/configuration.md
index e173388a..18f0afab 100644
--- a/docs/content/about/configuration.md
+++ b/docs/content/about/configuration.md
@@ -613,8 +613,7 @@ security.
 | `service` | yes      | The service being authenticated.                      |
 | `issuer`  | yes      | The name of the token issuer. The issuer inserts this into the token so it must match the value configured for the issuer. |
 | `rootcertbundle` | yes | The absolute path to the root certificate bundle. This bundle contains the public part of the certificates used to sign authentication tokens. |
-| `autoredirect`   | no      | When set to `true`, `realm` will automatically be set using the Host header of the request as the domain and a path of `/auth/token/`(or specified by `autoredirectpath`), the `realm` URL Scheme will use `X-Forwarded-Proto` header if set, otherwise it will be set to `https`. |
-| `autoredirectpath`   | no      | The path to redirect to if `autoredirect` is set to `true`, default: `/auth/token/`. |
+| `autoredirect`   | no      | When set to `true`, `realm` will automatically be set using the Host header of the request as the domain and a path of `/auth/token/`|
 
 
 For more information about Token based authentication configuration, see the
diff --git a/registry/auth/token/accesscontroller.go b/registry/auth/token/accesscontroller.go
index a525fbb7..bf7d222f 100644
--- a/registry/auth/token/accesscontroller.go
+++ b/registry/auth/token/accesscontroller.go
@@ -9,7 +9,6 @@ import (
 	"fmt"
 	"io"
 	"net/http"
-	"net/url"
 	"os"
 	"strings"
 
@@ -84,12 +83,11 @@ var (
 
 // authChallenge implements the auth.Challenge interface.
 type authChallenge struct {
-	err              error
-	realm            string
-	autoRedirect     bool
-	autoRedirectPath string
-	service          string
-	accessSet        accessSet
+	err          error
+	realm        string
+	autoRedirect bool
+	service      string
+	accessSet    accessSet
 }
 
 var _ auth.Challenge = authChallenge{}
@@ -104,28 +102,13 @@ func (ac authChallenge) Status() int {
 	return http.StatusUnauthorized
 }
 
-func buildAutoRedirectURL(r *http.Request, autoRedirectPath string) string {
-	scheme := "https"
-
-	if forwardedProto := r.Header.Get("X-Forwarded-Proto"); len(forwardedProto) > 0 {
-		scheme = forwardedProto
-	}
-
-	u := &url.URL{
-		Scheme: scheme,
-		Host:   r.Host,
-		Path:   autoRedirectPath,
-	}
-	return u.String()
-}
-
 // challengeParams constructs the value to be used in
 // the WWW-Authenticate response challenge header.
 // See https://tools.ietf.org/html/rfc6750#section-3
 func (ac authChallenge) challengeParams(r *http.Request) string {
 	var realm string
 	if ac.autoRedirect {
-		realm = buildAutoRedirectURL(r, ac.autoRedirectPath)
+		realm = fmt.Sprintf("https://%s/auth/token", r.Host)
 	} else {
 		realm = ac.realm
 	}
@@ -151,29 +134,23 @@ func (ac authChallenge) SetHeaders(r *http.Request, w http.ResponseWriter) {
 
 // accessController implements the auth.AccessController interface.
 type accessController struct {
-	realm            string
-	autoRedirect     bool
-	autoRedirectPath string
-	issuer           string
-	service          string
-	rootCerts        *x509.CertPool
-	trustedKeys      map[string]crypto.PublicKey
+	realm        string
+	autoRedirect bool
+	issuer       string
+	service      string
+	rootCerts    *x509.CertPool
+	trustedKeys  map[string]crypto.PublicKey
 }
 
-const (
-	defaultAutoRedirectPath = "/auth/token"
-)
-
 // tokenAccessOptions is a convenience type for handling
 // options to the constructor of an accessController.
 type tokenAccessOptions struct {
-	realm            string
-	autoRedirect     bool
-	autoRedirectPath string
-	issuer           string
-	service          string
-	rootCertBundle   string
-	jwks             string
+	realm          string
+	autoRedirect   bool
+	issuer         string
+	service        string
+	rootCertBundle string
+	jwks           string
 }
 
 // checkOptions gathers the necessary options
@@ -210,19 +187,6 @@ func checkOptions(options map[string]interface{}) (tokenAccessOptions, error) {
 		}
 		opts.autoRedirect = autoRedirect
 	}
-	if opts.autoRedirect {
-		autoRedirectPathVal, ok := options["autoredirectpath"]
-		if ok {
-			autoRedirectPath, ok := autoRedirectPathVal.(string)
-			if !ok {
-				return opts, fmt.Errorf("token auth requires a valid option string: autoredirectpath")
-			}
-			opts.autoRedirectPath = autoRedirectPath
-		}
-		if opts.autoRedirectPath == "" {
-			opts.autoRedirectPath = defaultAutoRedirectPath
-		}
-	}
 
 	return opts, nil
 }
@@ -323,13 +287,12 @@ func newAccessController(options map[string]interface{}) (auth.AccessController,
 	}
 
 	return &accessController{
-		realm:            config.realm,
-		autoRedirect:     config.autoRedirect,
-		autoRedirectPath: config.autoRedirectPath,
-		issuer:           config.issuer,
-		service:          config.service,
-		rootCerts:        rootPool,
-		trustedKeys:      trustedKeys,
+		realm:        config.realm,
+		autoRedirect: config.autoRedirect,
+		issuer:       config.issuer,
+		service:      config.service,
+		rootCerts:    rootPool,
+		trustedKeys:  trustedKeys,
 	}, nil
 }
 
@@ -337,11 +300,10 @@ func newAccessController(options map[string]interface{}) (auth.AccessController,
 // for actions on resources described by the given access items.
 func (ac *accessController) Authorized(req *http.Request, accessItems ...auth.Access) (*auth.Grant, error) {
 	challenge := &authChallenge{
-		realm:            ac.realm,
-		autoRedirect:     ac.autoRedirect,
-		autoRedirectPath: ac.autoRedirectPath,
-		service:          ac.service,
-		accessSet:        newAccessSet(accessItems...),
+		realm:        ac.realm,
+		autoRedirect: ac.autoRedirect,
+		service:      ac.service,
+		accessSet:    newAccessSet(accessItems...),
 	}
 
 	prefix, rawToken, ok := strings.Cut(req.Header.Get("Authorization"), " ")
diff --git a/registry/handlers/app.go b/registry/handlers/app.go
index 83e987d8..91c56e76 100644
--- a/registry/handlers/app.go
+++ b/registry/handlers/app.go
@@ -430,6 +430,14 @@ func (app *App) RegisterHealthChecks(healthRegistries ...*health.Registry) {
 	}
 }
 
+// Shutdown close the underlying registry
+func (app *App) Shutdown() error {
+	if r, ok := app.registry.(proxy.Closer); ok {
+		return r.Close()
+	}
+	return nil
+}
+
 // register a handler with the application, by route name. The handler will be
 // passed through the application filters and context will be constructed at
 // request time.
diff --git a/registry/proxy/proxyregistry.go b/registry/proxy/proxyregistry.go
index 33dcc4af..55c8f4be 100644
--- a/registry/proxy/proxyregistry.go
+++ b/registry/proxy/proxyregistry.go
@@ -211,6 +211,15 @@ func (pr *proxyingRegistry) BlobStatter() distribution.BlobStatter {
 	return pr.embedded.BlobStatter()
 }
 
+type Closer interface {
+	// Close release all resources used by this object
+	Close() error
+}
+
+func (pr *proxyingRegistry) Close() error {
+	return pr.scheduler.Stop()
+}
+
 // authChallenger encapsulates a request to the upstream to establish credential challenges
 type authChallenger interface {
 	tryEstablishChallenges(context.Context) error
diff --git a/registry/proxy/scheduler/scheduler.go b/registry/proxy/scheduler/scheduler.go
index ed1d9d41..78366c2f 100644
--- a/registry/proxy/scheduler/scheduler.go
+++ b/registry/proxy/scheduler/scheduler.go
@@ -206,12 +206,13 @@ func (ttles *TTLExpirationScheduler) startTimer(entry *schedulerEntry, ttl time.
 }
 
 // Stop stops the scheduler.
-func (ttles *TTLExpirationScheduler) Stop() {
+func (ttles *TTLExpirationScheduler) Stop() error {
 	ttles.Lock()
 	defer ttles.Unlock()
 
-	if err := ttles.writeState(); err != nil {
-		dcontext.GetLogger(ttles.ctx).Errorf("Error writing scheduler state: %s", err)
+	err := ttles.writeState()
+	if err != nil {
+		err = fmt.Errorf("error writing scheduler state: %w", err)
 	}
 
 	for _, entry := range ttles.entries {
@@ -221,6 +222,7 @@ func (ttles *TTLExpirationScheduler) Stop() {
 	close(ttles.doneChan)
 	ttles.saveTimer.Stop()
 	ttles.stopped = true
+	return err
 }
 
 func (ttles *TTLExpirationScheduler) writeState() error {
diff --git a/registry/registry.go b/registry/registry.go
index b7989f02..3d3bf1eb 100644
--- a/registry/registry.go
+++ b/registry/registry.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"crypto/tls"
 	"crypto/x509"
+	"errors"
 	"fmt"
 	"net/http"
 	"os"
@@ -312,7 +313,7 @@ func (registry *Registry) ListenAndServe() error {
 	}
 
 	// setup channel to get notified on SIGTERM signal
-	signal.Notify(registry.quit, syscall.SIGTERM)
+	signal.Notify(registry.quit, os.Interrupt, syscall.SIGTERM)
 	serveErr := make(chan error)
 
 	// Start serving in goroutine and listen for stop signal in main thread
@@ -332,9 +333,13 @@ func (registry *Registry) ListenAndServe() error {
 	}
 }
 
-// Shutdown gracefully shuts down the registry's HTTP server.
+// Shutdown gracefully shuts down the registry's HTTP server and application object.
 func (registry *Registry) Shutdown(ctx context.Context) error {
-	return registry.server.Shutdown(ctx)
+	err := registry.server.Shutdown(ctx)
+	if appErr := registry.app.Shutdown(); appErr != nil {
+		err = errors.Join(err, appErr)
+	}
+	return err
 }
 
 func configureDebugServer(config *configuration.Configuration) {
diff --git a/registry/storage/purgeuploads.go b/registry/storage/purgeuploads.go
index 9e9407a1..e1a876e7 100644
--- a/registry/storage/purgeuploads.go
+++ b/registry/storage/purgeuploads.go
@@ -93,7 +93,7 @@ func getOutstandingUploads(ctx context.Context, driver storageDriver.StorageDriv
 			ud.containingDir = filePath
 		}
 		if file == "startedat" {
-			if t, err := readStartedAtFile(ctx, driver, filePath); err == nil {
+			if t, err := readStartedAtFile(driver, filePath); err == nil {
 				ud.startedAt = t
 			} else {
 				errors = pushError(errors, filePath, err)
@@ -124,8 +124,9 @@ func uuidFromPath(path string) (string, bool) {
 }
 
 // readStartedAtFile reads the date from an upload's startedAtFile
-func readStartedAtFile(ctx context.Context, driver storageDriver.StorageDriver, path string) (time.Time, error) {
-	startedAtBytes, err := driver.GetContent(ctx, path)
+func readStartedAtFile(driver storageDriver.StorageDriver, path string) (time.Time, error) {
+	// todo:(richardscothern) - pass in a context
+	startedAtBytes, err := driver.GetContent(context.Background(), path)
 	if err != nil {
 		return time.Now(), err
 	}


Test patch:
diff --git a/registry/auth/token/accesscontroller_test.go b/registry/auth/token/accesscontroller_test.go
deleted file mode 100644
index fde20311..00000000
--- a/registry/auth/token/accesscontroller_test.go
+++ /dev/null
@@ -1,89 +0,0 @@
-package token
-
-import (
-	"net/http"
-	"net/http/httptest"
-	"testing"
-)
-
-func TestBuildAutoRedirectURL(t *testing.T) {
-	cases := []struct {
-		name             string
-		reqGetter        func() *http.Request
-		autoRedirectPath string
-		expectedURL      string
-	}{{
-		name: "http",
-		reqGetter: func() *http.Request {
-			req := httptest.NewRequest("GET", "http://example.com/", nil)
-			return req
-		},
-		autoRedirectPath: "/auth",
-		expectedURL:      "https://example.com/auth",
-	}, {
-		name: "x-forwarded",
-		reqGetter: func() *http.Request {
-			req := httptest.NewRequest("GET", "http://example.com/", nil)
-			req.Header.Set("X-Forwarded-Proto", "http")
-			return req
-		},
-		autoRedirectPath: "/auth/token",
-		expectedURL:      "http://example.com/auth/token",
-	}}
-
-	for _, tc := range cases {
-		t.Run(tc.name, func(t *testing.T) {
-			req := tc.reqGetter()
-			result := buildAutoRedirectURL(req, tc.autoRedirectPath)
-			if result != tc.expectedURL {
-				t.Errorf("expected %s, got %s", tc.expectedURL, result)
-			}
-		})
-	}
-}
-
-func TestCheckOptions(t *testing.T) {
-	realm := "https://auth.example.com/token/"
-	issuer := "test-issuer.example.com"
-	service := "test-service.example.com"
-
-	options := map[string]interface{}{
-		"realm":            realm,
-		"issuer":           issuer,
-		"service":          service,
-		"rootcertbundle":   "",
-		"autoredirect":     true,
-		"autoredirectpath": "/auth",
-	}
-
-	ta, err := checkOptions(options)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if ta.autoRedirect != true {
-		t.Fatal("autoredirect should be true")
-	}
-	if ta.autoRedirectPath != "/auth" {
-		t.Fatal("autoredirectpath should be /auth")
-	}
-
-	options = map[string]interface{}{
-		"realm":                        realm,
-		"issuer":                       issuer,
-		"service":                      service,
-		"rootcertbundle":               "",
-		"autoredirect":                 true,
-		"autoredirectforcetlsdisabled": true,
-	}
-
-	ta, err = checkOptions(options)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if ta.autoRedirect != true {
-		t.Fatal("autoredirect should be true")
-	}
-	if ta.autoRedirectPath != "/auth/token" {
-		t.Fatal("autoredirectpath should be /auth/token")
-	}
-}
diff --git a/registry/proxy/scheduler/scheduler_test.go b/registry/proxy/scheduler/scheduler_test.go
index 38fa0f58..fb0869c2 100644
--- a/registry/proxy/scheduler/scheduler_test.go
+++ b/registry/proxy/scheduler/scheduler_test.go
@@ -136,7 +136,12 @@ func TestRestoreOld(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Error starting ttlExpirationScheduler: %s", err)
 	}
-	defer s.Stop()
+	defer func(s *TTLExpirationScheduler) {
+		err := s.Stop()
+		if err != nil {
+			t.Fatalf("Error stopping ttlExpirationScheduler: %s", err)
+		}
+	}(s)
 
 	wg.Wait()
 	mu.Lock()
@@ -177,7 +182,10 @@ func TestStopRestore(t *testing.T) {
 
 	// Start and stop before all operations complete
 	// state will be written to fs
-	s.Stop()
+	err = s.Stop()
+	if err != nil {
+		t.Fatalf(err.Error())
+	}
 	time.Sleep(10 * time.Millisecond)
 
 	// v2 will restore state from fs