From 17c570757827500fb2a2ca4ffc0d9546f1eddf94 Mon Sep 17 00:00:00 2001 From: Ziad Date: Wed, 4 Jun 2025 12:20:48 +0000 Subject: [PATCH 1/2] OIDC Authentication Support and Tests --- pkg/kubernetes/configuration.go | 21 +- pkg/kubernetes/configuration_test.go | 152 +++++++++ pkg/kubernetes/kubernetes.go | 23 +- pkg/kubernetes/kubernetes_test.go | 442 +++++++++++++++++++++++++++ 4 files changed, 629 insertions(+), 9 deletions(-) create mode 100644 pkg/kubernetes/kubernetes_test.go diff --git a/pkg/kubernetes/configuration.go b/pkg/kubernetes/configuration.go index 97ccf22..0d934aa 100644 --- a/pkg/kubernetes/configuration.go +++ b/pkg/kubernetes/configuration.go @@ -85,9 +85,26 @@ func (k *Kubernetes) ConfigurationView(minify bool) (string, error) { Server: k.cfg.Host, InsecureSkipTLSVerify: k.cfg.Insecure, } - cfg.AuthInfos["user"] = &clientcmdapi.AuthInfo{ - Token: k.cfg.BearerToken, + + // Create auth info with appropriate authentication method + authInfo := &clientcmdapi.AuthInfo{} + + // If using bearer token + if k.cfg.BearerToken != "" { + authInfo.Token = k.cfg.BearerToken + } + + // If using OIDC auth provider + if k.cfg.AuthProvider != nil { + authInfo.AuthProvider = k.cfg.AuthProvider } + + // If using exec provider (for OIDC or other auth methods) + if k.cfg.ExecProvider != nil { + authInfo.Exec = k.cfg.ExecProvider + } + + cfg.AuthInfos["user"] = authInfo cfg.Contexts["context"] = &clientcmdapi.Context{ Cluster: "cluster", AuthInfo: "user", diff --git a/pkg/kubernetes/configuration_test.go b/pkg/kubernetes/configuration_test.go index 1e43433..86bcccf 100644 --- a/pkg/kubernetes/configuration_test.go +++ b/pkg/kubernetes/configuration_test.go @@ -3,9 +3,12 @@ package kubernetes import ( "errors" "k8s.io/client-go/rest" + "k8s.io/client-go/tools/clientcmd/api" + v1 "k8s.io/client-go/tools/clientcmd/api/v1" "os" "path" "runtime" + "sigs.k8s.io/yaml" "strings" "testing" ) @@ -136,3 +139,152 @@ users: } }) } + +func TestConfigurationViewWithOIDC(t *testing.T) { + // Save the original InClusterConfig function + originalInClusterConfig := InClusterConfig + + // Mock InClusterConfig to return a config with OIDC auth provider + InClusterConfig = func() (*rest.Config, error) { + return &rest.Config{ + Host: "https://kubernetes.default.svc", + AuthProvider: &api.AuthProviderConfig{ + Name: "oidc", + Config: map[string]string{ + "client-id": "test-client", + "client-secret": "test-secret", + "id-token": "test-id-token", + "refresh-token": "test-refresh-token", + "idp-issuer-url": "https://example.org", + }, + }, + }, nil + } + + // Restore the original function when the test completes + defer func() { + InClusterConfig = originalInClusterConfig + }() + + // Create a Kubernetes instance with empty kubeconfig to force in-cluster mode + k := &Kubernetes{ + Kubeconfig: "", + } + + // Call ConfigurationView + configYaml, err := k.ConfigurationView(true) + if err != nil { + t.Fatalf("ConfigurationView failed: %v", err) + } + + // Parse the YAML + var config v1.Config + if err := yaml.Unmarshal([]byte(configYaml), &config); err != nil { + t.Fatalf("Failed to parse config YAML: %v", err) + } + + // Verify the auth provider is included + if len(config.AuthInfos) != 1 { + t.Fatalf("Expected 1 auth info, got %d", len(config.AuthInfos)) + } + + authInfo := config.AuthInfos[0] + if authInfo.Name != "user" { + t.Errorf("Expected auth info name to be 'user', got '%s'", authInfo.Name) + } + + if authInfo.AuthInfo.AuthProvider == nil { + t.Fatalf("Expected auth provider to be present, got nil") + } + + if authInfo.AuthInfo.AuthProvider.Name != "oidc" { + t.Errorf("Expected auth provider name to be 'oidc', got '%s'", authInfo.AuthInfo.AuthProvider.Name) + } + + // Verify the auth provider config + authProviderConfig := authInfo.AuthInfo.AuthProvider.Config + expectedKeys := []string{"client-id", "client-secret", "id-token", "refresh-token", "idp-issuer-url"} + for _, key := range expectedKeys { + if value, exists := authProviderConfig[key]; !exists { + t.Errorf("Expected auth provider config to have key '%s', but it was missing", key) + } else if key == "client-id" && value != "test-client" { + t.Errorf("Expected auth provider config key '%s' to have value 'test-client', got '%s'", key, value) + } + } +} + +func TestConfigurationViewWithExecProvider(t *testing.T) { + // Save the original InClusterConfig function + originalInClusterConfig := InClusterConfig + + // Mock InClusterConfig to return a config with ExecProvider + InClusterConfig = func() (*rest.Config, error) { + return &rest.Config{ + Host: "https://kubernetes.default.svc", + ExecProvider: &api.ExecConfig{ + Command: "aws", + Args: []string{"eks", "get-token", "--cluster-name", "test-cluster"}, + Env: []api.ExecEnvVar{ + { + Name: "AWS_PROFILE", + Value: "test-profile", + }, + }, + APIVersion: "client.authentication.k8s.io/v1beta1", + }, + }, nil + } + + // Restore the original function when the test completes + defer func() { + InClusterConfig = originalInClusterConfig + }() + + // Create a Kubernetes instance with empty kubeconfig to force in-cluster mode + k := &Kubernetes{ + Kubeconfig: "", + } + + // Call ConfigurationView + configYaml, err := k.ConfigurationView(true) + if err != nil { + t.Fatalf("ConfigurationView failed: %v", err) + } + + // Parse the YAML + var config v1.Config + if err := yaml.Unmarshal([]byte(configYaml), &config); err != nil { + t.Fatalf("Failed to parse config YAML: %v", err) + } + + // Verify the exec provider is included + if len(config.AuthInfos) != 1 { + t.Fatalf("Expected 1 auth info, got %d", len(config.AuthInfos)) + } + + authInfo := config.AuthInfos[0] + if authInfo.Name != "user" { + t.Errorf("Expected auth info name to be 'user', got '%s'", authInfo.Name) + } + + if authInfo.AuthInfo.Exec == nil { + t.Fatalf("Expected exec provider to be present, got nil") + } + + execConfig := authInfo.AuthInfo.Exec + if execConfig.Command != "aws" { + t.Errorf("Expected exec command to be 'aws', got '%s'", execConfig.Command) + } + + if len(execConfig.Args) != 4 || execConfig.Args[0] != "eks" || execConfig.Args[1] != "get-token" { + t.Errorf("Expected exec args to be ['eks', 'get-token', '--cluster-name', 'test-cluster'], got %v", execConfig.Args) + } + + if len(execConfig.Env) != 1 || execConfig.Env[0].Name != "AWS_PROFILE" || execConfig.Env[0].Value != "test-profile" { + t.Errorf("Expected exec env to have AWS_PROFILE=test-profile, got %v", execConfig.Env) + } + + if execConfig.APIVersion != "client.authentication.k8s.io/v1beta1" { + t.Errorf("Expected exec APIVersion to be 'client.authentication.k8s.io/v1beta1', got '%s'", execConfig.APIVersion) + } +} diff --git a/pkg/kubernetes/kubernetes.go b/pkg/kubernetes/kubernetes.go index 8fedf38..3a4f748 100644 --- a/pkg/kubernetes/kubernetes.go +++ b/pkg/kubernetes/kubernetes.go @@ -2,6 +2,7 @@ package kubernetes import ( "context" + "github.com/fsnotify/fsnotify" "github.com/manusa/kubernetes-mcp-server/pkg/helm" v1 "k8s.io/api/core/v1" @@ -131,13 +132,21 @@ func (k *Kubernetes) Derived(ctx context.Context) *Kubernetes { } klog.V(5).Infof("%s header found, using provided bearer token", AuthorizationBearerTokenHeader) derivedCfg := rest.CopyConfig(k.cfg) - derivedCfg.BearerToken = bearerToken - derivedCfg.BearerTokenFile = "" - derivedCfg.Username = "" - derivedCfg.Password = "" - derivedCfg.AuthProvider = nil - derivedCfg.AuthConfigPersister = nil - derivedCfg.ExecProvider = nil + + // If we have a bearer token, use it for authentication + if bearerToken != "" { + derivedCfg.BearerToken = bearerToken + derivedCfg.BearerTokenFile = "" + derivedCfg.Username = "" + derivedCfg.Password = "" + + // Only clear auth providers if we're using a bearer token + // This preserves OIDC configuration when no token is provided + derivedCfg.AuthProvider = nil + derivedCfg.AuthConfigPersister = nil + derivedCfg.ExecProvider = nil + } + derivedCfg.Impersonate = rest.ImpersonationConfig{} clientCmdApiConfig, err := k.clientCmdConfig.RawConfig() if err != nil { diff --git a/pkg/kubernetes/kubernetes_test.go b/pkg/kubernetes/kubernetes_test.go new file mode 100644 index 0000000..689154e --- /dev/null +++ b/pkg/kubernetes/kubernetes_test.go @@ -0,0 +1,442 @@ +package kubernetes + +import ( + "context" + "testing" + + "k8s.io/client-go/rest" + "k8s.io/client-go/tools/clientcmd" + "k8s.io/client-go/tools/clientcmd/api" +) + +func TestDerived(t *testing.T) { + t.Run("with bearer token", func(t *testing.T) { + // Setup + mockConfig := &api.Config{ + Clusters: map[string]*api.Cluster{ + "test-cluster": { + Server: "https://example.com", + }, + }, + AuthInfos: map[string]*api.AuthInfo{ + "test-user": { + AuthProvider: &api.AuthProviderConfig{ + Name: "oidc", + Config: map[string]string{ + "client-id": "test-client", + "client-secret": "test-secret", + "id-token": "test-id-token", + "refresh-token": "test-refresh-token", + "idp-issuer-url": "https://example.org", + }, + }, + }, + }, + Contexts: map[string]*api.Context{ + "test-context": { + Cluster: "test-cluster", + AuthInfo: "test-user", + }, + }, + CurrentContext: "test-context", + } + + k := &Kubernetes{ + cfg: &rest.Config{ + Host: "https://example.com", + BearerToken: "original-token", + AuthProvider: &api.AuthProviderConfig{ + Name: "oidc", + Config: map[string]string{ + "client-id": "test-client", + "client-secret": "test-secret", + "id-token": "test-id-token", + "refresh-token": "test-refresh-token", + "idp-issuer-url": "https://example.org", + }, + }, + ExecProvider: &api.ExecConfig{ + Command: "test-command", + }, + }, + clientCmdConfig: clientcmd.NewDefaultClientConfig(*mockConfig, &clientcmd.ConfigOverrides{}), + } + + // Create context with bearer token + ctx := context.WithValue(context.Background(), AuthorizationBearerTokenHeader, "new-token") + + // Execute + derived := k.Derived(ctx) + + // Verify + if derived == k { + t.Error("expected a new derived instance, got the same instance") + } + if derived.cfg.BearerToken != "new-token" { + t.Errorf("expected bearer token to be 'new-token', got '%s'", derived.cfg.BearerToken) + } + if derived.cfg.AuthProvider != nil { + t.Errorf("expected AuthProvider to be nil, got %v", derived.cfg.AuthProvider) + } + if derived.cfg.ExecProvider != nil { + t.Errorf("expected ExecProvider to be nil, got %v", derived.cfg.ExecProvider) + } + if derived.cfg.BearerTokenFile != "" { + t.Errorf("expected BearerTokenFile to be empty, got '%s'", derived.cfg.BearerTokenFile) + } + if derived.cfg.Username != "" { + t.Errorf("expected Username to be empty, got '%s'", derived.cfg.Username) + } + if derived.cfg.Password != "" { + t.Errorf("expected Password to be empty, got '%s'", derived.cfg.Password) + } + }) + + t.Run("without bearer token", func(t *testing.T) { + // Setup + mockConfig := &api.Config{ + Clusters: map[string]*api.Cluster{ + "test-cluster": { + Server: "https://example.com", + }, + }, + AuthInfos: map[string]*api.AuthInfo{ + "test-user": { + AuthProvider: &api.AuthProviderConfig{ + Name: "oidc", + Config: map[string]string{ + "client-id": "test-client", + "client-secret": "test-secret", + "id-token": "test-id-token", + "refresh-token": "test-refresh-token", + "idp-issuer-url": "https://example.org", + }, + }, + }, + }, + Contexts: map[string]*api.Context{ + "test-context": { + Cluster: "test-cluster", + AuthInfo: "test-user", + }, + }, + CurrentContext: "test-context", + } + + k := &Kubernetes{ + cfg: &rest.Config{ + Host: "https://example.com", + BearerToken: "original-token", + AuthProvider: &api.AuthProviderConfig{ + Name: "oidc", + Config: map[string]string{ + "client-id": "test-client", + "client-secret": "test-secret", + "id-token": "test-id-token", + "refresh-token": "test-refresh-token", + "idp-issuer-url": "https://example.org", + }, + }, + ExecProvider: &api.ExecConfig{ + Command: "test-command", + }, + }, + clientCmdConfig: clientcmd.NewDefaultClientConfig(*mockConfig, &clientcmd.ConfigOverrides{}), + } + + // Create context without bearer token + ctx := context.Background() + + // Execute + derived := k.Derived(ctx) + + // Verify - should return the same instance when no bearer token + if derived != k { + t.Error("expected the same instance when no bearer token is provided") + } + }) + + t.Run("with empty bearer token", func(t *testing.T) { + // Setup + mockConfig := &api.Config{ + Clusters: map[string]*api.Cluster{ + "test-cluster": { + Server: "https://example.com", + }, + }, + AuthInfos: map[string]*api.AuthInfo{ + "test-user": { + AuthProvider: &api.AuthProviderConfig{ + Name: "oidc", + Config: map[string]string{ + "client-id": "test-client", + "client-secret": "test-secret", + "id-token": "test-id-token", + "refresh-token": "test-refresh-token", + "idp-issuer-url": "https://example.org", + }, + }, + }, + }, + Contexts: map[string]*api.Context{ + "test-context": { + Cluster: "test-cluster", + AuthInfo: "test-user", + }, + }, + CurrentContext: "test-context", + } + + k := &Kubernetes{ + cfg: &rest.Config{ + Host: "https://example.com", + BearerToken: "original-token", + AuthProvider: &api.AuthProviderConfig{ + Name: "oidc", + Config: map[string]string{ + "client-id": "test-client", + "client-secret": "test-secret", + "id-token": "test-id-token", + "refresh-token": "test-refresh-token", + "idp-issuer-url": "https://example.org", + }, + }, + ExecProvider: &api.ExecConfig{ + Command: "test-command", + }, + }, + clientCmdConfig: clientcmd.NewDefaultClientConfig(*mockConfig, &clientcmd.ConfigOverrides{}), + } + + // Create context with empty bearer token + ctx := context.WithValue(context.Background(), AuthorizationBearerTokenHeader, "") + + // Execute + derived := k.Derived(ctx) + + // Verify - should return the same instance when empty bearer token is provided + if derived != k { + t.Error("expected the same instance when empty bearer token is provided") + } + }) + + t.Run("OIDC configuration preservation", func(t *testing.T) { + // Setup + mockConfig := &api.Config{ + Clusters: map[string]*api.Cluster{ + "test-cluster": { + Server: "https://example.com", + }, + }, + AuthInfos: map[string]*api.AuthInfo{ + "test-user": { + AuthProvider: &api.AuthProviderConfig{ + Name: "oidc", + Config: map[string]string{ + "client-id": "test-client", + "client-secret": "test-secret", + "id-token": "test-id-token", + "refresh-token": "test-refresh-token", + "idp-issuer-url": "https://oidc.example.org", + }, + }, + }, + }, + Contexts: map[string]*api.Context{ + "test-context": { + Cluster: "test-cluster", + AuthInfo: "test-user", + }, + }, + CurrentContext: "test-context", + } + + k := &Kubernetes{ + cfg: &rest.Config{ + Host: "https://example.com", + AuthProvider: &api.AuthProviderConfig{ + Name: "oidc", + Config: map[string]string{ + "client-id": "test-client", + "client-secret": "test-secret", + "id-token": "test-id-token", + "refresh-token": "test-refresh-token", + "idp-issuer-url": "https://oidc.example.org", + }, + }, + }, + clientCmdConfig: clientcmd.NewDefaultClientConfig(*mockConfig, &clientcmd.ConfigOverrides{}), + } + + // Create context without bearer token + ctx := context.Background() + + // Execute + derived := k.Derived(ctx) + + // Verify - should return same instance and preserve OIDC config + if derived != k { + t.Error("expected the same instance when no bearer token is provided") + } + if k.cfg.AuthProvider == nil { + t.Error("expected original OIDC AuthProvider to be preserved") + } + if k.cfg.AuthProvider.Name != "oidc" { + t.Errorf("expected AuthProvider.Name to be 'oidc', got '%s'", k.cfg.AuthProvider.Name) + } + if k.cfg.AuthProvider.Config["idp-issuer-url"] != "https://oidc.example.org" { + t.Errorf("expected idp-issuer-url to be preserved, got '%s'", k.cfg.AuthProvider.Config["idp-issuer-url"]) + } + }) +} + +func TestConfigurationViewOIDC(t *testing.T) { + t.Run("in-cluster with OIDC auth provider", func(t *testing.T) { + // Mock InClusterConfig to return a valid config + originalInClusterConfig := InClusterConfig + defer func() { InClusterConfig = originalInClusterConfig }() + + InClusterConfig = func() (*rest.Config, error) { + return &rest.Config{ + Host: "https://kubernetes.default.svc", + AuthProvider: &api.AuthProviderConfig{ + Name: "oidc", + Config: map[string]string{ + "client-id": "test-client", + "idp-issuer-url": "https://oidc.example.org", + }, + }, + }, nil + } + + k := &Kubernetes{ + cfg: &rest.Config{ + Host: "https://kubernetes.default.svc", + AuthProvider: &api.AuthProviderConfig{ + Name: "oidc", + Config: map[string]string{ + "client-id": "test-client", + "idp-issuer-url": "https://oidc.example.org", + }, + }, + }, + } + + // Execute + configYaml, err := k.ConfigurationView(false) + + // Verify + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if configYaml == "" { + t.Error("expected non-empty configuration YAML") + } + + // Check that OIDC configuration is included + if !contains(configYaml, "oidc") { + t.Error("expected OIDC configuration to be present in the output") + } + if !contains(configYaml, "test-client") { + t.Error("expected client-id to be present in the output") + } + if !contains(configYaml, "https://oidc.example.org") { + t.Error("expected idp-issuer-url to be present in the output") + } + }) + + t.Run("in-cluster with exec provider", func(t *testing.T) { + // Mock InClusterConfig to return a valid config + originalInClusterConfig := InClusterConfig + defer func() { InClusterConfig = originalInClusterConfig }() + + InClusterConfig = func() (*rest.Config, error) { + return &rest.Config{ + Host: "https://kubernetes.default.svc", + ExecProvider: &api.ExecConfig{ + Command: "oidc-login", + Args: []string{"get-token"}, + }, + }, nil + } + + k := &Kubernetes{ + cfg: &rest.Config{ + Host: "https://kubernetes.default.svc", + ExecProvider: &api.ExecConfig{ + Command: "oidc-login", + Args: []string{"get-token"}, + }, + }, + } + + // Execute + configYaml, err := k.ConfigurationView(false) + + // Verify + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if configYaml == "" { + t.Error("expected non-empty configuration YAML") + } + + // Check that exec configuration is included + if !contains(configYaml, "oidc-login") { + t.Error("expected exec command to be present in the output") + } + if !contains(configYaml, "get-token") { + t.Error("expected exec args to be present in the output") + } + }) + + t.Run("in-cluster with bearer token", func(t *testing.T) { + // Mock InClusterConfig to return a valid config + originalInClusterConfig := InClusterConfig + defer func() { InClusterConfig = originalInClusterConfig }() + + InClusterConfig = func() (*rest.Config, error) { + return &rest.Config{ + Host: "https://kubernetes.default.svc", + BearerToken: "test-bearer-token", + }, nil + } + + k := &Kubernetes{ + cfg: &rest.Config{ + Host: "https://kubernetes.default.svc", + BearerToken: "test-bearer-token", + }, + } + + // Execute + configYaml, err := k.ConfigurationView(false) + + // Verify + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if configYaml == "" { + t.Error("expected non-empty configuration YAML") + } + + // Check that token configuration is included + if !contains(configYaml, "test-bearer-token") { + t.Error("expected bearer token to be present in the output") + } + }) +} + +// Helper function to check if a string contains a substring +func contains(s, substr string) bool { + return len(s) >= len(substr) && (s == substr || len(substr) == 0 || + (len(s) > len(substr) && (s[:len(substr)] == substr || s[len(s)-len(substr):] == substr || + func() bool { + for i := 1; i <= len(s)-len(substr); i++ { + if s[i:i+len(substr)] == substr { + return true + } + } + return false + }()))) +} From b28860eb767f62f67d45c6ffbca0897ad12d5b4a Mon Sep 17 00:00:00 2001 From: Ziad Date: Wed, 4 Jun 2025 13:04:16 +0000 Subject: [PATCH 2/2] fix nil deref in test --- pkg/kubernetes/configuration_test.go | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/pkg/kubernetes/configuration_test.go b/pkg/kubernetes/configuration_test.go index 86bcccf..d08c13a 100644 --- a/pkg/kubernetes/configuration_test.go +++ b/pkg/kubernetes/configuration_test.go @@ -2,15 +2,16 @@ package kubernetes import ( "errors" - "k8s.io/client-go/rest" - "k8s.io/client-go/tools/clientcmd/api" - v1 "k8s.io/client-go/tools/clientcmd/api/v1" "os" "path" "runtime" - "sigs.k8s.io/yaml" "strings" "testing" + + "k8s.io/client-go/rest" + "k8s.io/client-go/tools/clientcmd/api" + v1 "k8s.io/client-go/tools/clientcmd/api/v1" + "sigs.k8s.io/yaml" ) func TestKubernetes_IsInCluster(t *testing.T) { @@ -171,6 +172,9 @@ func TestConfigurationViewWithOIDC(t *testing.T) { Kubeconfig: "", } + // Initialize cfg directly to prevent nil pointer dereference + k.cfg, _ = InClusterConfig() + // Call ConfigurationView configYaml, err := k.ConfigurationView(true) if err != nil { @@ -245,6 +249,9 @@ func TestConfigurationViewWithExecProvider(t *testing.T) { Kubeconfig: "", } + // Initialize cfg directly to prevent nil pointer dereference + k.cfg, _ = InClusterConfig() + // Call ConfigurationView configYaml, err := k.ConfigurationView(true) if err != nil {