From 375d115285cc4894d03d26e1a0adccbaa4e05940 Mon Sep 17 00:00:00 2001 From: SarthakB11 Date: Fri, 1 May 2026 11:02:43 +0000 Subject: [PATCH 1/2] fix(controller): return error instead of panicking on unknown auth mode getAuthenticator panicked when authCfg.Mode was anything other than trusted-proxy or unsecure. a panic at startup bypasses defer-driven cleanup (e.g. the tracing shutdown registered earlier in app.Start), produces a stack trace where a typed error message would be clearer, and surfaces to kubernetes only as a CrashLoopBackOff with no reason set on the pod. return a wrapped fmt.Errorf instead and propagate it up through the ExtensionConfig closure so app.Start can log and exit cleanly. swap the panic-recover test for one that asserts the typed error path. closes the panic-on-startup behavior at go/core/cmd/controller/main.go:51. Signed-off-by: SarthakB11 --- go/core/cmd/controller/auth_mode_test.go | 21 ++++++++++++--------- go/core/cmd/controller/main.go | 15 ++++++++++----- 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/go/core/cmd/controller/auth_mode_test.go b/go/core/cmd/controller/auth_mode_test.go index 2963f8ea4..b6618607f 100644 --- a/go/core/cmd/controller/auth_mode_test.go +++ b/go/core/cmd/controller/auth_mode_test.go @@ -32,7 +32,10 @@ func TestGetAuthenticator(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - authenticator := getAuthenticator(tt.authCfg) + authenticator, err := getAuthenticator(tt.authCfg) + if err != nil { + t.Fatalf("getAuthenticator() unexpected error: %v", err) + } gotType := getTypeName(authenticator) if gotType != tt.wantType { t.Errorf("getAuthenticator() = %s, want %s", gotType, tt.wantType) @@ -41,14 +44,14 @@ func TestGetAuthenticator(t *testing.T) { } } -func TestGetAuthenticatorPanicsOnUnknownMode(t *testing.T) { - defer func() { - r := recover() - if r == nil { - t.Fatal("expected panic for unknown auth mode, got none") - } - }() - getAuthenticator(struct{ Mode, UserIDClaim string }{"proxy", ""}) +func TestGetAuthenticatorErrorsOnUnknownMode(t *testing.T) { + authenticator, err := getAuthenticator(struct{ Mode, UserIDClaim string }{"proxy", ""}) + if err == nil { + t.Fatal("expected error for unknown auth mode, got nil") + } + if authenticator != nil { + t.Errorf("expected nil authenticator on error, got %T", authenticator) + } } func getTypeName(v auth.AuthProvider) string { diff --git a/go/core/cmd/controller/main.go b/go/core/cmd/controller/main.go index 576dee94a..b2d741ab2 100644 --- a/go/core/cmd/controller/main.go +++ b/go/core/cmd/controller/main.go @@ -17,6 +17,8 @@ limitations under the License. package main import ( + "fmt" + "github.com/kagent-dev/kagent/go/core/internal/httpserver/auth" "github.com/kagent-dev/kagent/go/core/pkg/app" pkgauth "github.com/kagent-dev/kagent/go/core/pkg/auth" @@ -31,7 +33,10 @@ import ( func main() { authorizer := &auth.NoopAuthorizer{} app.Start(func(bootstrap app.BootstrapConfig) (*app.ExtensionConfig, error) { - authenticator := getAuthenticator(bootstrap.Config.Auth) + authenticator, err := getAuthenticator(bootstrap.Config.Auth) + if err != nil { + return nil, err + } return &app.ExtensionConfig{ Authenticator: authenticator, Authorizer: authorizer, @@ -41,13 +46,13 @@ func main() { }, nil) } -func getAuthenticator(authCfg struct{ Mode, UserIDClaim string }) pkgauth.AuthProvider { +func getAuthenticator(authCfg struct{ Mode, UserIDClaim string }) (pkgauth.AuthProvider, error) { switch authCfg.Mode { case "trusted-proxy": - return auth.NewProxyAuthenticator(authCfg.UserIDClaim) + return auth.NewProxyAuthenticator(authCfg.UserIDClaim), nil case "unsecure": - return &auth.UnsecureAuthenticator{} + return &auth.UnsecureAuthenticator{}, nil default: - panic("unknown auth mode: " + authCfg.Mode + " (valid modes: unsecure, trusted-proxy)") + return nil, fmt.Errorf("unknown auth mode %q (valid modes: unsecure, trusted-proxy)", authCfg.Mode) } } From 4431347f72f1a7a219de8203302398b55e1bd996 Mon Sep 17 00:00:00 2001 From: SarthakB11 Date: Sun, 3 May 2026 10:04:54 +0000 Subject: [PATCH 2/2] test(controller): assert auth-mode error message names invalid mode and valid options per copilot review feedback: the prior assertion only checked that some error was returned, so a regression that dropped the invalid mode name or the supported-values hint would still pass. assert both pieces of the user-facing message explicitly. Signed-off-by: SarthakB11 --- go/core/cmd/controller/auth_mode_test.go | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/go/core/cmd/controller/auth_mode_test.go b/go/core/cmd/controller/auth_mode_test.go index b6618607f..8acbffa4f 100644 --- a/go/core/cmd/controller/auth_mode_test.go +++ b/go/core/cmd/controller/auth_mode_test.go @@ -1,6 +1,7 @@ package main import ( + "strings" "testing" authimpl "github.com/kagent-dev/kagent/go/core/internal/httpserver/auth" @@ -45,13 +46,26 @@ func TestGetAuthenticator(t *testing.T) { } func TestGetAuthenticatorErrorsOnUnknownMode(t *testing.T) { - authenticator, err := getAuthenticator(struct{ Mode, UserIDClaim string }{"proxy", ""}) + const invalidMode = "proxy" + authenticator, err := getAuthenticator(struct{ Mode, UserIDClaim string }{invalidMode, ""}) if err == nil { t.Fatal("expected error for unknown auth mode, got nil") } if authenticator != nil { t.Errorf("expected nil authenticator on error, got %T", authenticator) } + // The error message must surface the invalid mode and the supported values + // so misconfigured deployments get an actionable message rather than just a + // generic failure. + msg := err.Error() + if !strings.Contains(msg, invalidMode) { + t.Errorf("error message %q does not include the invalid mode %q", msg, invalidMode) + } + for _, valid := range []string{"unsecure", "trusted-proxy"} { + if !strings.Contains(msg, valid) { + t.Errorf("error message %q does not list supported mode %q", msg, valid) + } + } } func getTypeName(v auth.AuthProvider) string {