Merge branch 'main' into introduce-jwks-url

Author: ardaguclu

.gitignore

@@ -1,3 +1,4 @@
+_output/
 .idea/
 .vscode/
 .docusaurus/

Makefile

@@ -15,6 +15,9 @@ LD_FLAGS = -s -w \
 	-X '$(PACKAGE)/pkg/version.BinaryName=$(BINARY_NAME)'
 COMMON_BUILD_ARGS = -ldflags "$(LD_FLAGS)"
 
+GOLANGCI_LINT = $(shell pwd)/_output/tools/bin/golangci-lint
+GOLANGCI_LINT_VERSION ?= v2.2.2
+
 # NPM version should not append the -dirty flag
 NPM_VERSION ?= $(shell echo $(shell git describe --tags --always) | sed 's/^v//')
 OSES = darwin linux windows
@@ -97,3 +100,14 @@ format: ## Format the code
 .PHONY: tidy
 tidy: ## Tidy up the go modules
 	go mod tidy
+
+.PHONY: golangci-lint
+golangci-lint: ## Download and install golangci-lint if not already installed
+		@[ -f $(GOLANGCI_LINT) ] || { \
+    	set -e ;\
+    	curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(shell dirname $(GOLANGCI_LINT)) $(GOLANGCI_LINT_VERSION) ;\
+    	}
+
+.PHONY: lint
+lint: golangci-lint ## Lint the code
+	$(GOLANGCI_LINT) run --verbose --print-resources-usage

pkg/helm/helm.go

@@ -45,11 +45,14 @@ func (h *Helm) Install(ctx context.Context, chart string, values map[string]inte
 	install.Timeout = 5 * time.Minute
 	install.DryRun = false
 
-	chartRequested, err := install.ChartPathOptions.LocateChart(chart, cli.New())
+	chartRequested, err := install.LocateChart(chart, cli.New())
 	if err != nil {
 		return "", err
 	}
 	chartLoaded, err := loader.Load(chartRequested)
+	if err != nil {
+		return "", err
+	}
 
 	installedRelease, err := install.RunWithContext(ctx, chartLoaded, values)
 	if err != nil {

pkg/http/http_test.go

@@ -4,11 +4,13 @@ import (
 	"bufio"
 	"bytes"
 	"context"
+	"flag"
 	"fmt"
 	"net"
 	"net/http"
 	"os"
 	"path/filepath"
+	"regexp"
 	"strings"
 	"testing"
 	"time"
@@ -47,7 +49,10 @@ func (c *httpContext) beforeEach() {
 	_ = os.Setenv("KUBECONFIG", kubeConfig)
 	// Capture logging
 	c.klogState = klog.CaptureState()
-	klog.SetLogger(textlogger.NewLogger(textlogger.NewConfig(textlogger.Verbosity(1), textlogger.Output(&c.logBuffer))))
+	flags := flag.NewFlagSet("test", flag.ContinueOnError)
+	klog.InitFlags(flags)
+	_ = flags.Set("v", "5")
+	klog.SetLogger(textlogger.NewLogger(textlogger.NewConfig(textlogger.Verbosity(5), textlogger.Output(&c.logBuffer))))
 	// Start server in random port
 	ln, err := net.Listen("tcp", "0.0.0.0:0")
 	if err != nil {
@@ -247,3 +252,29 @@ func TestWellKnownOAuthProtectedResource(t *testing.T) {
 		})
 	})
 }
+
+func TestMiddlewareLogging(t *testing.T) {
+	testCase(t, func(ctx *httpContext) {
+		_, _ = http.Get(fmt.Sprintf("http://%s/.well-known/oauth-protected-resource", ctx.httpAddress))
+		t.Run("Logs HTTP requests and responses", func(t *testing.T) {
+			if !strings.Contains(ctx.logBuffer.String(), "GET /.well-known/oauth-protected-resource 200") {
+				t.Errorf("Expected log entry for GET /.well-known/oauth-protected-resource, got: %s", ctx.logBuffer.String())
+			}
+		})
+		t.Run("Logs HTTP request duration", func(t *testing.T) {
+			expected := `"GET /.well-known/oauth-protected-resource 200 (.+)"`
+			m := regexp.MustCompile(expected).FindStringSubmatch(ctx.logBuffer.String())
+			if len(m) != 2 {
+				t.Fatalf("Expected log entry to contain duration, got %s", ctx.logBuffer.String())
+			}
+			duration, err := time.ParseDuration(m[1])
+			if err != nil {
+				t.Fatalf("Failed to parse duration from log entry: %v", err)
+			}
+			if duration < 0 {
+				t.Errorf("Expected duration to be non-negative, got %v", duration)
+			}
+		})
+	})
+
+}

pkg/kubernetes-mcp-server/cmd/root.go

@@ -119,16 +119,16 @@ func NewMCPServer(streams genericiooptions.IOStreams) *cobra.Command {
 	cmd.Flags().BoolVar(&o.ReadOnly, "read-only", o.ReadOnly, "If true, only tools annotated with readOnlyHint=true are exposed")
 	cmd.Flags().BoolVar(&o.DisableDestructive, "disable-destructive", o.DisableDestructive, "If true, tools annotated with destructiveHint=true are disabled")
 	cmd.Flags().BoolVar(&o.RequireOAuth, "require-oauth", o.RequireOAuth, "If true, requires OAuth authorization as defined in the Model Context Protocol (MCP) specification. This flag is ignored if transport type is stdio")
-	cmd.Flags().MarkHidden("require-oauth")
+	_ = cmd.Flags().MarkHidden("require-oauth")
 	cmd.Flags().StringVar(&o.AuthorizationURL, "authorization-url", o.AuthorizationURL, "OAuth authorization server URL for protected resource endpoint. If not provided, the Kubernetes API server host will be used. Only valid if require-oauth is enabled.")
-	cmd.Flags().MarkHidden("authorization-url")
+  _ = cmd.Flags().MarkHidden("authorization-url")
 	cmd.Flags().StringVar(&o.JwksURL, "jwks-url", o.JwksURL, "OAuth JWKS server URL for protected resource endpoint. Only valid if require-oauth is enabled.")
-	cmd.Flags().MarkHidden("jwks-url")
+	_ = cmd.Flags().MarkHidden("jwks-url")
 	cmd.Flags().StringVar(&o.ServerURL, "server-url", o.ServerURL, "Server URL of this application. Optional. If set, this url will be served in protected resource metadata endpoint and tokens will be validated with this audience. If not set, expected audience is kubernetes-mcp-server. Only valid if require-oauth is enabled.")
-	cmd.Flags().MarkHidden("server-url")
+	_ = cmd.Flags().MarkHidden("server-url")
 	cmd.Flags().StringVar(&o.CertificateAuthority, "certificate-authority", o.CertificateAuthority, "Certificate authority path to verify certificates. Optional. Only valid if require-oauth is enabled.")
-	cmd.Flags().MarkHidden("certificate-authority")
-
+	_ = cmd.Flags().MarkHidden("certificate-authority")
+	
 	return cmd
 }
 
@@ -257,11 +257,11 @@ func (m *MCPServerOptions) Validate() error {
 func (m *MCPServerOptions) Run() error {
 	profile := mcp.ProfileFromString(m.Profile)
 	if profile == nil {
-		return fmt.Errorf("Invalid profile name: %s, valid names are: %s\n", m.Profile, strings.Join(mcp.ProfileNames, ", "))
+		return fmt.Errorf("invalid profile name: %s, valid names are: %s", m.Profile, strings.Join(mcp.ProfileNames, ", "))
 	}
 	listOutput := output.FromString(m.StaticConfig.ListOutput)
 	if listOutput == nil {
-		return fmt.Errorf("Invalid output name: %s, valid names are: %s\n", m.StaticConfig.ListOutput, strings.Join(output.Names, ", "))
+		return fmt.Errorf("invalid output name: %s, valid names are: %s", m.StaticConfig.ListOutput, strings.Join(output.Names, ", "))
 	}
 	klog.V(1).Info("Starting kubernetes-mcp-server")
 	klog.V(1).Infof(" - Config: %s", m.ConfigPath)
@@ -314,7 +314,7 @@ func (m *MCPServerOptions) Run() error {
 		StaticConfig: m.StaticConfig,
 	})
 	if err != nil {
-		return fmt.Errorf("Failed to initialize MCP server: %w\n", err)
+		return fmt.Errorf("failed to initialize MCP server: %w", err)
 	}
 	defer mcpServer.Close()
 

pkg/kubernetes/configuration.go

@@ -106,6 +106,7 @@ func (m *Manager) ConfigurationView(minify bool) (runtime.Object, error) {
 			return nil, err
 		}
 	}
+	//nolint:staticcheck
 	if err = clientcmdapi.FlattenConfig(&cfg); err != nil {
 		// ignore error
 		//return "", err

pkg/kubernetes/impersonate_roundtripper.go

@@ -2,10 +2,12 @@ package kubernetes
 
 import "net/http"
 
+// nolint:unused
 type impersonateRoundTripper struct {
 	delegate http.RoundTripper
 }
 
+// nolint:unused
 func (irt *impersonateRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
 	// TODO: Solution won't work with discoveryclient which uses context.TODO() instead of the passed-in context
 	if v, ok := req.Context().Value(OAuthAuthorizationHeader).(string); ok {

pkg/kubernetes/kubernetes.go

@@ -26,9 +26,11 @@ import (
 	_ "k8s.io/client-go/plugin/pkg/client/auth/oidc"
 )
 
+type HeaderKey string
+
 const (
-	CustomAuthorizationHeader = "kubernetes-authorization"
-	OAuthAuthorizationHeader  = "Authorization"
+	CustomAuthorizationHeader = HeaderKey("kubernetes-authorization")
+	OAuthAuthorizationHeader  = HeaderKey("Authorization")
 
 	CustomUserAgent = "kubernetes-mcp-server/bearer-token-auth"
 )
@@ -155,10 +157,10 @@ func (m *Manager) Derived(ctx context.Context) (*Kubernetes, error) {
 		APIPath: m.cfg.APIPath,
 		// Copy only server verification TLS settings (CA bundle and server name)
 		TLSClientConfig: rest.TLSClientConfig{
-			Insecure:   m.cfg.TLSClientConfig.Insecure,
-			ServerName: m.cfg.TLSClientConfig.ServerName,
-			CAFile:     m.cfg.TLSClientConfig.CAFile,
-			CAData:     m.cfg.TLSClientConfig.CAData,
+			Insecure:   m.cfg.Insecure,
+			ServerName: m.cfg.ServerName,
+			CAFile:     m.cfg.CAFile,
+			CAData:     m.cfg.CAData,
 		},
 		BearerToken: strings.TrimPrefix(authorization, "Bearer "),
 		// pass custom UserAgent to identify the client

pkg/kubernetes/kubernetes_test.go

@@ -137,17 +137,17 @@ users:
 			t.Errorf("expected Timeout %v, got %v", originalCfg.Timeout, derivedCfg.Timeout)
 		}
 
-		if derivedCfg.TLSClientConfig.Insecure != originalCfg.TLSClientConfig.Insecure {
-			t.Errorf("expected TLS Insecure %v, got %v", originalCfg.TLSClientConfig.Insecure, derivedCfg.TLSClientConfig.Insecure)
+		if derivedCfg.Insecure != originalCfg.Insecure {
+			t.Errorf("expected TLS Insecure %v, got %v", originalCfg.Insecure, derivedCfg.Insecure)
 		}
-		if derivedCfg.TLSClientConfig.ServerName != originalCfg.TLSClientConfig.ServerName {
-			t.Errorf("expected TLS ServerName %s, got %s", originalCfg.TLSClientConfig.ServerName, derivedCfg.TLSClientConfig.ServerName)
+		if derivedCfg.ServerName != originalCfg.ServerName {
+			t.Errorf("expected TLS ServerName %s, got %s", originalCfg.ServerName, derivedCfg.ServerName)
 		}
-		if derivedCfg.TLSClientConfig.CAFile != originalCfg.TLSClientConfig.CAFile {
-			t.Errorf("expected TLS CAFile %s, got %s", originalCfg.TLSClientConfig.CAFile, derivedCfg.TLSClientConfig.CAFile)
+		if derivedCfg.CAFile != originalCfg.CAFile {
+			t.Errorf("expected TLS CAFile %s, got %s", originalCfg.CAFile, derivedCfg.CAFile)
 		}
-		if string(derivedCfg.TLSClientConfig.CAData) != string(originalCfg.TLSClientConfig.CAData) {
-			t.Errorf("expected TLS CAData %s, got %s", string(originalCfg.TLSClientConfig.CAData), string(derivedCfg.TLSClientConfig.CAData))
+		if string(derivedCfg.CAData) != string(originalCfg.CAData) {
+			t.Errorf("expected TLS CAData %s, got %s", string(originalCfg.CAData), string(derivedCfg.CAData))
 		}
 
 		if derivedCfg.BearerToken != testBearerToken {
@@ -160,17 +160,17 @@ users:
 		// Verify that sensitive fields are NOT copied to prevent credential leakage
 		// The derived config should only use the bearer token from the Authorization header
 		// and not inherit any authentication credentials from the original kubeconfig
-		if derivedCfg.TLSClientConfig.CertFile != "" {
-			t.Errorf("expected TLS CertFile to be empty, got %s", derivedCfg.TLSClientConfig.CertFile)
+		if derivedCfg.CertFile != "" {
+			t.Errorf("expected TLS CertFile to be empty, got %s", derivedCfg.CertFile)
 		}
-		if derivedCfg.TLSClientConfig.KeyFile != "" {
-			t.Errorf("expected TLS KeyFile to be empty, got %s", derivedCfg.TLSClientConfig.KeyFile)
+		if derivedCfg.KeyFile != "" {
+			t.Errorf("expected TLS KeyFile to be empty, got %s", derivedCfg.KeyFile)
 		}
-		if len(derivedCfg.TLSClientConfig.CertData) != 0 {
-			t.Errorf("expected TLS CertData to be empty, got %v", derivedCfg.TLSClientConfig.CertData)
+		if len(derivedCfg.CertData) != 0 {
+			t.Errorf("expected TLS CertData to be empty, got %v", derivedCfg.CertData)
 		}
-		if len(derivedCfg.TLSClientConfig.KeyData) != 0 {
-			t.Errorf("expected TLS KeyData to be empty, got %v", derivedCfg.TLSClientConfig.KeyData)
+		if len(derivedCfg.KeyData) != 0 {
+			t.Errorf("expected TLS KeyData to be empty, got %v", derivedCfg.KeyData)
 		}
 
 		if derivedCfg.Username != "" {

pkg/mcp/common_test.go

@@ -1,13 +1,18 @@
 package mcp
 
 import (
+	"bytes"
 	"context"
 	"encoding/json"
+	"flag"
 	"fmt"
+	"k8s.io/klog/v2"
+	"k8s.io/klog/v2/textlogger"
 	"net/http/httptest"
 	"os"
 	"path/filepath"
 	"runtime"
+	"strconv"
 	"testing"
 	"time"
 
@@ -25,12 +30,9 @@ import (
 	apiextensionsv1spec "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime/schema"
-	"k8s.io/apimachinery/pkg/runtime/serializer"
 	"k8s.io/apimachinery/pkg/watch"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
-	"k8s.io/client-go/scale"
 	"k8s.io/client-go/tools/clientcmd"
 	"k8s.io/client-go/tools/clientcmd/api"
 	toolswatch "k8s.io/client-go/tools/watch"
@@ -71,7 +73,7 @@ func TestMain(m *testing.M) {
 	}
 	envTestEnv.CheckCoherence()
 	workflows.Use{}.Do(envTestEnv)
-	versionDir := envTestEnv.Platform.Platform.BaseName(*envTestEnv.Version.AsConcrete())
+	versionDir := envTestEnv.Platform.BaseName(*envTestEnv.Version.AsConcrete())
 	envTest = &envtest.Environment{
 		BinaryAssetsDirectory: filepath.Join(envTestDir, "k8s", versionDir),
 	}
@@ -100,6 +102,7 @@ func TestMain(m *testing.M) {
 type mcpContext struct {
 	profile    Profile
 	listOutput output.Output
+	logLevel   int
 
 	staticConfig  *config.StaticConfig
 	clientOptions []transport.ClientOption
@@ -111,6 +114,8 @@ type mcpContext struct {
 	mcpServer     *Server
 	mcpHttpServer *httptest.Server
 	mcpClient     *client.Client
+	klogState     klog.State
+	logBuffer     bytes.Buffer
 }
 
 func (c *mcpContext) beforeEach(t *testing.T) {
@@ -133,6 +138,13 @@ func (c *mcpContext) beforeEach(t *testing.T) {
 	if c.before != nil {
 		c.before(c)
 	}
+	// Set up logging
+	c.klogState = klog.CaptureState()
+	flags := flag.NewFlagSet("test", flag.ContinueOnError)
+	klog.InitFlags(flags)
+	_ = flags.Set("v", strconv.Itoa(c.logLevel))
+	klog.SetLogger(textlogger.NewLogger(textlogger.NewConfig(textlogger.Verbosity(c.logLevel), textlogger.Output(&c.logBuffer))))
+	// MCP Server
 	if c.mcpServer, err = NewServer(Configuration{
 		Profile:      c.profile,
 		ListOutput:   c.listOutput,
@@ -146,6 +158,7 @@ func (c *mcpContext) beforeEach(t *testing.T) {
 		t.Fatal(err)
 		return
 	}
+	// MCP Client
 	if err = c.mcpClient.Start(c.ctx); err != nil {
 		t.Fatal(err)
 		return
@@ -168,6 +181,7 @@ func (c *mcpContext) afterEach() {
 	c.mcpServer.Close()
 	_ = c.mcpClient.Close()
 	c.mcpHttpServer.Close()
+	c.klogState.Restore()
 }
 
 func testCase(t *testing.T, test func(c *mcpContext)) {
@@ -190,9 +204,9 @@ func (c *mcpContext) withKubeConfig(rc *rest.Config) *api.Config {
 	fakeConfig.AuthInfos["additional-auth"] = api.NewAuthInfo()
 	if rc != nil {
 		fakeConfig.Clusters["fake"].Server = rc.Host
-		fakeConfig.Clusters["fake"].CertificateAuthorityData = rc.TLSClientConfig.CAData
-		fakeConfig.AuthInfos["fake"].ClientKeyData = rc.TLSClientConfig.KeyData
-		fakeConfig.AuthInfos["fake"].ClientCertificateData = rc.TLSClientConfig.CertData
+		fakeConfig.Clusters["fake"].CertificateAuthorityData = rc.CAData
+		fakeConfig.AuthInfos["fake"].ClientKeyData = rc.KeyData
+		fakeConfig.AuthInfos["fake"].ClientCertificateData = rc.CertData
 	}
 	fakeConfig.Contexts["fake-context"] = api.NewContext()
 	fakeConfig.Contexts["fake-context"].Cluster = "fake"
@@ -264,18 +278,6 @@ func (c *mcpContext) newKubernetesClient() *kubernetes.Clientset {
 	return kubernetes.NewForConfigOrDie(envTestRestConfig)
 }
 
-func (c *mcpContext) newRestClient(groupVersion *schema.GroupVersion) *rest.RESTClient {
-	config := *envTestRestConfig
-	config.GroupVersion = groupVersion
-	config.APIPath = "/api"
-	config.NegotiatedSerializer = serializer.NewCodecFactory(scale.NewScaleConverter().Scheme()).WithoutConversion()
-	rc, err := rest.RESTClientFor(&config)
-	if err != nil {
-		panic(err)
-	}
-	return rc
-}
-
 // newApiExtensionsClient creates a new ApiExtensions client with the envTest kubeconfig
 func (c *mcpContext) newApiExtensionsClient() *apiextensionsv1.ApiextensionsV1Client {
 	return apiextensionsv1.NewForConfigOrDie(envTestRestConfig)
@@ -286,6 +288,9 @@ func (c *mcpContext) crdApply(resource string) error {
 	apiExtensionsV1Client := c.newApiExtensionsClient()
 	var crd = &apiextensionsv1spec.CustomResourceDefinition{}
 	err := json.Unmarshal([]byte(resource), crd)
+	if err != nil {
+		return fmt.Errorf("failed to create CRD %v", err)
+	}
 	_, err = apiExtensionsV1Client.CustomResourceDefinitions().Create(c.ctx, crd, metav1.CreateOptions{})
 	if err != nil {
 		return fmt.Errorf("failed to create CRD %v", err)