metrics: Add nil check for metricsHandler in AdminMetrics.serveHTTP

admin.go

@@ -119,9 +119,6 @@ type AdminConfig struct {
 	// EXPERIMENTAL: This feature is subject to change.
 	Remote *RemoteAdmin `json:"remote,omitempty"`
 
-	// Holds onto the routers so that we can later provision them
-	// if they require provisioning.
-	routers []AdminRouter
 }
 
 // ConfigSettings configures the management of configuration.
@@ -220,7 +217,7 @@ type AdminPermissions struct {
 
 // newAdminHandler reads admin's config and returns an http.Handler suitable
 // for use in an admin endpoint server, which will be listening on listenAddr.
-func (admin *AdminConfig) newAdminHandler(addr NetworkAddress, remote bool, _ Context) adminHandler {
+func (admin *AdminConfig) newAdminHandler(addr NetworkAddress, remote bool, ctx Context) (adminHandler, error) {
 	muxWrap := adminHandler{mux: http.NewServeMux()}
 
 	// secure the local or remote endpoint respectively
@@ -277,34 +274,21 @@ func (admin *AdminConfig) newAdminHandler(addr NetworkAddress, remote bool, _ Co
 	// register third-party module endpoints
 	for _, m := range GetModules("admin.api") {
 		router := m.New().(AdminRouter)
-		for _, route := range router.Routes() {
-			addRoute(route.Pattern, handlerLabel, route.Handler)
-		}
-		admin.routers = append(admin.routers, router)
-	}
 
-	return muxWrap
-}
-
-// provisionAdminRouters provisions all the router modules
-// in the admin.api namespace that need provisioning.
-func (admin *AdminConfig) provisionAdminRouters(ctx Context) error {
-	for _, router := range admin.routers {
-		provisioner, ok := router.(Provisioner)
-		if !ok {
-			continue
+		// provision the router before registering its routes, so
+		// handlers have access to all provisioned state
+		if provisioner, ok := router.(Provisioner); ok {
+			if err := provisioner.Provision(ctx); err != nil {
+				return adminHandler{}, fmt.Errorf("provisioning admin router module %s: %v", m.ID, err)
+			}
 		}
 
-		err := provisioner.Provision(ctx)
-		if err != nil {
-			return err
+		for _, route := range router.Routes() {
+			addRoute(route.Pattern, handlerLabel, route.Handler)
 		}
 	}
 
-	// We no longer need the routers once provisioned, allow for GC
-	admin.routers = nil
-
-	return nil
+	return muxWrap, nil
 }
 
 // allowedOrigins returns a list of origins that are allowed.
@@ -428,11 +412,7 @@ func replaceLocalAdminServer(cfg *Config, ctx Context) error {
 		return err
 	}
 
-	handler := cfg.Admin.newAdminHandler(addr, false, ctx)
-
-	// run the provisioners for loaded modules to make sure local
-	// state is properly re-initialized in the new admin server
-	err = cfg.Admin.provisionAdminRouters(ctx)
+	handler, err := cfg.Admin.newAdminHandler(addr, false, ctx)
 	if err != nil {
 		return err
 	}
@@ -556,11 +536,7 @@ func replaceRemoteAdminServer(ctx Context, cfg *Config) error {
 
 	// make the HTTP handler but disable Host/Origin enforcement
 	// because we are using TLS authentication instead
-	handler := cfg.Admin.newAdminHandler(addr, true, ctx)
-
-	// run the provisioners for loaded modules to make sure local
-	// state is properly re-initialized in the new admin server
-	err = cfg.Admin.provisionAdminRouters(ctx)
+	handler, err := cfg.Admin.newAdminHandler(addr, true, ctx)
 	if err != nil {
 		return err
 	}

admin_test.go

@@ -282,7 +282,10 @@ func TestAdminHandlerBuiltinRouteErrors(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Failed to parse address: %v", err)
 	}
-	handler := cfg.Admin.newAdminHandler(addr, false, Context{})
+	handler, err := cfg.Admin.newAdminHandler(addr, false, Context{})
+	if err != nil {
+		t.Fatalf("Failed to create admin handler: %v", err)
+	}
 
 	tests := []struct {
 		name           string
@@ -403,7 +406,10 @@ func TestNewAdminHandlerRouterRegistration(t *testing.T) {
 	admin := &AdminConfig{
 		EnforceOrigin: false,
 	}
-	handler := admin.newAdminHandler(addr, false, Context{})
+	handler, err := admin.newAdminHandler(addr, false, Context{})
+	if err != nil {
+		t.Fatalf("Failed to create admin handler: %v", err)
+	}
 
 	req := httptest.NewRequest("GET", "/mock", nil)
 	req.Host = "localhost:2019"
@@ -415,10 +421,6 @@ func TestNewAdminHandlerRouterRegistration(t *testing.T) {
 		t.Errorf("Expected status code %d but got %d", http.StatusOK, rr.Code)
 		t.Logf("Response body: %s", rr.Body.String())
 	}
-
-	if len(admin.routers) != 1 {
-		t.Errorf("Expected 1 router to be stored, got %d", len(admin.routers))
-	}
 }
 
 type mockProvisionableRouter struct {
@@ -456,19 +458,16 @@ func TestAdminRouterProvisioning(t *testing.T) {
 		name         string
 		provisionErr error
 		wantErr      bool
-		routersAfter int // expected number of routers after provisioning
 	}{
 		{
 			name:         "successful provisioning",
 			provisionErr: nil,
 			wantErr:      false,
-			routersAfter: 0,
 		},
 		{
 			name:         "provisioning error",
 			provisionErr: fmt.Errorf("provision failed"),
 			wantErr:      true,
-			routersAfter: 1,
 		},
 	}
 
@@ -504,8 +503,7 @@ func TestAdminRouterProvisioning(t *testing.T) {
 				t.Fatalf("Failed to parse address: %v", err)
 			}
 
-			_ = admin.newAdminHandler(addr, false, Context{})
-			err = admin.provisionAdminRouters(Context{})
+			_, err = admin.newAdminHandler(addr, false, Context{})
 
 			if test.wantErr {
 				if err == nil {
@@ -516,10 +514,6 @@ func TestAdminRouterProvisioning(t *testing.T) {
 					t.Errorf("Expected no error but got: %v", err)
 				}
 			}
-
-			if len(admin.routers) != test.routersAfter {
-				t.Errorf("Expected %d routers after provisioning, got %d", test.routersAfter, len(admin.routers))
-			}
 		})
 	}
 }

caddy.go

@@ -441,13 +441,6 @@ func run(newCfg *Config, start bool) (Context, error) {
 		}
 	}()
 
-	// Provision any admin routers which may need to access
-	// some of the other apps at runtime
-	err = ctx.cfg.Admin.provisionAdminRouters(ctx)
-	if err != nil {
-		return ctx, err
-	}
-
 	// Start
 	err = func() error {
 		started := make([]string, 0, len(ctx.cfg.apps))