From a6a45ff6c56d2d0df1ac86f22d38997da3ba3b39 Mon Sep 17 00:00:00 2001
From: Francis Lavoie <lavofr@gmail.com>
Date: Mon, 20 May 2024 13:14:58 -0400
Subject: [PATCH] context: AppIfConfigured returns error; consider
 not-yet-provisioned modules (#6292)

* context: Add new `AppStrict()` method to avoid instantiating empty apps

* Rename AppStrict -> AppIfConfigured

---------

Co-authored-by: Matthew Holt <mholt@users.noreply.github.com>
---
 context.go                   | 29 +++++++++++++----------------
 modules/caddypki/adminapi.go |  7 +++++--
 modules/caddytls/capools.go  | 12 ++++++------
 modules/caddytls/tls.go      |  2 +-
 4 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/context.go b/context.go
index 4307dda88..53d94a524 100644
--- a/context.go
+++ b/context.go
@@ -453,23 +453,20 @@ func (ctx Context) App(name string) (any, error) {
 	return modVal, nil
 }
 
-// AppIfConfigured returns an app by its name if it has been
-// configured. Can be called instead of App() to avoid
-// instantiating an empty app when that's not desirable. If
-// the app has not been loaded, nil is returned.
-//
-// We return any type instead of the App type because it is not
-// intended for the caller of this method to be the one to start
-// or stop App modules. The caller is expected to assert to the
-// concrete type.
-func (ctx Context) AppIfConfigured(name string) any {
-	if ctx.cfg == nil {
-		// this can happen if the currently-active context
-		// is being accessed, but no config has successfully
-		// been loaded yet
-		return nil
+// AppIfConfigured is like App, but it returns an error if the
+// app has not been configured. This is useful when the app is
+// required and its absence is a configuration error, or when
+// the app is optional and you don't want to instantiate a
+// new one that hasn't been explicitly configured.
+func (ctx Context) AppIfConfigured(name string) (any, error) {
+	if app, ok := ctx.cfg.apps[name]; ok {
+		return app, nil
 	}
-	return ctx.cfg.apps[name]
+	appRaw := ctx.cfg.AppsRaw[name]
+	if appRaw == nil {
+		return nil, fmt.Errorf("app module %s is not configured", name)
+	}
+	return ctx.App(name)
 }
 
 // Storage returns the configured Caddy storage implementation.
diff --git a/modules/caddypki/adminapi.go b/modules/caddypki/adminapi.go
index 435af349a..c454f6458 100644
--- a/modules/caddypki/adminapi.go
+++ b/modules/caddypki/adminapi.go
@@ -50,8 +50,11 @@ func (a *adminAPI) Provision(ctx caddy.Context) error {
 	a.ctx = ctx
 	a.log = ctx.Logger(a) // TODO: passing in 'a' is a hack until the admin API is officially extensible (see #5032)
 
-	// Avoid initializing PKI if it wasn't configured
-	if pkiApp := a.ctx.AppIfConfigured("pki"); pkiApp != nil {
+	// Avoid initializing PKI if it wasn't configured.
+	// We intentionally ignore the error since it's not
+	// fatal if the PKI app is not explicitly configured.
+	pkiApp, err := ctx.AppIfConfigured("pki")
+	if err == nil {
 		a.pkiApp = pkiApp.(*PKI)
 	}
 
diff --git a/modules/caddytls/capools.go b/modules/caddytls/capools.go
index dc5e60087..c73bc4832 100644
--- a/modules/caddytls/capools.go
+++ b/modules/caddytls/capools.go
@@ -187,9 +187,9 @@ func (PKIRootCAPool) CaddyModule() caddy.ModuleInfo {
 
 // Loads the PKI app and load the root certificates into the certificate pool
 func (p *PKIRootCAPool) Provision(ctx caddy.Context) error {
-	pkiApp := ctx.AppIfConfigured("pki")
-	if pkiApp == nil {
-		return fmt.Errorf("PKI app not configured")
+	pkiApp, err := ctx.AppIfConfigured("pki")
+	if err != nil {
+		return fmt.Errorf("pki_root CA pool requires that a PKI app is configured: %v", err)
 	}
 	pki := pkiApp.(*caddypki.PKI)
 	for _, caID := range p.Authority {
@@ -259,9 +259,9 @@ func (PKIIntermediateCAPool) CaddyModule() caddy.ModuleInfo {
 
 // Loads the PKI app and load the intermediate certificates into the certificate pool
 func (p *PKIIntermediateCAPool) Provision(ctx caddy.Context) error {
-	pkiApp := ctx.AppIfConfigured("pki")
-	if pkiApp == nil {
-		return fmt.Errorf("PKI app not configured")
+	pkiApp, err := ctx.AppIfConfigured("pki")
+	if err != nil {
+		return fmt.Errorf("pki_intermediate CA pool requires that a PKI app is configured: %v", err)
 	}
 	pki := pkiApp.(*caddypki.PKI)
 	for _, caID := range p.Authority {
diff --git a/modules/caddytls/tls.go b/modules/caddytls/tls.go
index 14965533e..c233977e1 100644
--- a/modules/caddytls/tls.go
+++ b/modules/caddytls/tls.go
@@ -353,7 +353,7 @@ func (t *TLS) Cleanup() error {
 	// if a new TLS app was loaded, remove certificates from the cache that are no longer
 	// being managed or loaded by the new config; if there is no more TLS app running,
 	// then stop cert maintenance and let the cert cache be GC'ed
-	if nextTLS := caddy.ActiveContext().AppIfConfigured("tls"); nextTLS != nil {
+	if nextTLS, err := caddy.ActiveContext().AppIfConfigured("tls"); err == nil && nextTLS != nil {
 		nextTLSApp := nextTLS.(*TLS)
 
 		// compute which certificates were managed or loaded into the cert cache by this