diff --git a/pkg/services/apiserver/auth/authorizer/authorizer.go b/pkg/services/apiserver/auth/authorizer/authorizer.go index dab8167deb0..b0dcbbc7872 100644 --- a/pkg/services/apiserver/auth/authorizer/authorizer.go +++ b/pkg/services/apiserver/auth/authorizer/authorizer.go @@ -3,7 +3,6 @@ package authorizer import ( "context" - "github.com/grafana/grafana/pkg/setting" "k8s.io/apimachinery/pkg/runtime/schema" k8suser "k8s.io/apiserver/pkg/authentication/user" "k8s.io/apiserver/pkg/authorization/authorizer" @@ -29,9 +28,9 @@ type GrafanaAuthorizer struct { // 4. We check authorizer that is configured speficially for an api. // 5. As a last fallback we check Role, this will only happen if an api have not configured // an authorizer or return authorizer.DecisionNoOpinion -func NewGrafanaBuiltInSTAuthorizer(cfg *setting.Cfg) *GrafanaAuthorizer { +func NewGrafanaBuiltInSTAuthorizer() *GrafanaAuthorizer { authorizers := []authorizer.Authorizer{ - newImpersonationAuthorizer(), + NewImpersonationAuthorizer(), authorizerfactory.NewPrivilegedGroups(k8suser.SystemPrivilegedGroup), newNamespaceAuthorizer(), } diff --git a/pkg/services/apiserver/auth/authorizer/impersonation.go b/pkg/services/apiserver/auth/authorizer/impersonation.go index c736173bd5f..067d364ef56 100644 --- a/pkg/services/apiserver/auth/authorizer/impersonation.go +++ b/pkg/services/apiserver/auth/authorizer/impersonation.go @@ -8,7 +8,7 @@ import ( var _ authorizer.Authorizer = (*impersonationAuthorizer)(nil) -func newImpersonationAuthorizer() *impersonationAuthorizer { +func NewImpersonationAuthorizer() *impersonationAuthorizer { return &impersonationAuthorizer{} } diff --git a/pkg/services/apiserver/builder/helper.go b/pkg/services/apiserver/builder/helper.go index c535443a91e..774f194e2a8 100644 --- a/pkg/services/apiserver/builder/helper.go +++ b/pkg/services/apiserver/builder/helper.go @@ -76,19 +76,7 @@ var PathRewriters = []filters.PathRewriter{ func GetDefaultBuildHandlerChainFunc(builders []APIGroupBuilder, reg prometheus.Registerer) BuildHandlerChainFunc { return func(delegateHandler http.Handler, c *genericapiserver.Config) http.Handler { - requestHandler, err := GetCustomRoutesHandler( - delegateHandler, - c.LoopbackClientConfig, - builders, - reg, - c.MergedResourceConfig, - ) - if err != nil { - panic(fmt.Sprintf("could not build the request handler for specified API builders: %s", err.Error())) - } - - // Needs to run last in request chain to function as expected, hence we register it first. - handler := filters.WithTracingHTTPLoggingAttributes(requestHandler) + handler := filters.WithTracingHTTPLoggingAttributes(delegateHandler) // filters.WithRequester needs to be after the K8s chain because it depends on the K8s user in context handler = filters.WithRequester(handler) diff --git a/pkg/services/apiserver/builder/request_handler.go b/pkg/services/apiserver/builder/request_handler.go index 50761f1d42c..4e5a355f453 100644 --- a/pkg/services/apiserver/builder/request_handler.go +++ b/pkg/services/apiserver/builder/request_handler.go @@ -3,146 +3,306 @@ package builder import ( "fmt" "net/http" + "strings" + "github.com/emicklei/go-restful/v3" "github.com/gorilla/mux" "github.com/prometheus/client_golang/prometheus" serverstorage "k8s.io/apiserver/pkg/server/storage" - restclient "k8s.io/client-go/rest" klog "k8s.io/klog/v2" "k8s.io/kube-openapi/pkg/spec3" ) -type requestHandler struct { - router *mux.Router +// convertHandlerToRouteFunction converts an http.HandlerFunc to a restful.RouteFunction +// It extracts path parameters from restful.Request and populates them in the request context +// so that mux.Vars can read them (for backward compatibility with handlers that use mux.Vars) +func convertHandlerToRouteFunction(handler http.HandlerFunc) restful.RouteFunction { + return func(req *restful.Request, resp *restful.Response) { + // Extract path parameters from restful.Request and populate mux.Vars + // This is needed for backward compatibility with handlers that use mux.Vars(r) + vars := make(map[string]string) + + // Get all path parameters from the restful.Request + // The restful.Request has PathParameters() method that returns a map + pathParams := req.PathParameters() + for key, value := range pathParams { + vars[key] = value + } + + // Set the vars in the request context using mux.SetURLVars + // This makes mux.Vars(r) work correctly + if len(vars) > 0 { + req.Request = mux.SetURLVars(req.Request, vars) + } + + handler(resp.ResponseWriter, req.Request) + } } -func GetCustomRoutesHandler(delegateHandler http.Handler, restConfig *restclient.Config, builders []APIGroupBuilder, metricsRegistry prometheus.Registerer, apiResourceConfig *serverstorage.ResourceConfig) (http.Handler, error) { - useful := false // only true if any routes exist anywhere - router := mux.NewRouter() +// AugmentWebServicesWithCustomRoutes adds custom routes from builders to existing WebServices +// in the container. +func AugmentWebServicesWithCustomRoutes( + container *restful.Container, + builders []APIGroupBuilder, + metricsRegistry prometheus.Registerer, + apiResourceConfig *serverstorage.ResourceConfig, +) error { + if container == nil { + return fmt.Errorf("container cannot be nil") + } metrics := NewCustomRouteMetrics(metricsRegistry) - for _, builder := range builders { - provider, ok := builder.(APIGroupRouteProvider) + // Build a map of existing WebServices by root path + existingWebServices := make(map[string]*restful.WebService) + for _, ws := range container.RegisteredWebServices() { + existingWebServices[ws.RootPath()] = ws + } + + for _, b := range builders { + provider, ok := b.(APIGroupRouteProvider) if !ok || provider == nil { continue } - for _, gv := range GetGroupVersions(builder) { - // filter out api groups that are disabled in APIEnablementOptions + for _, gv := range GetGroupVersions(b) { + // Filter out disabled API groups gvr := gv.WithResource("") if apiResourceConfig != nil && !apiResourceConfig.ResourceEnabled(gvr) { - klog.InfoS("Skipping custom route handler for disabled group version", "gv", gv.String()) + klog.InfoS("Skipping custom routes for disabled group version", "gv", gv.String()) continue } + routes := provider.GetAPIRoutes(gv) if routes == nil { continue } - prefix := "/apis/" + gv.String() - - // Root handlers - var sub *mux.Router - for _, route := range routes.Root { - if sub == nil { - sub = router.PathPrefix(prefix).Subrouter() - sub.MethodNotAllowedHandler = &methodNotAllowedHandler{} - } - - useful = true - methods, err := methodsFromSpec(route.Path, route.Spec) - if err != nil { - return nil, err - } - - instrumentedHandler := metrics.InstrumentHandler( - gv.Group, - gv.Version, - route.Path, // Use path as resource identifier - route.Handler, - ) - - sub.HandleFunc("/"+route.Path, instrumentedHandler). - Methods(methods...) + // Find or create WebService for this group version + rootPath := "/apis/" + gv.String() + ws, exists := existingWebServices[rootPath] + if !exists { + // Create a new WebService if one doesn't exist + ws = new(restful.WebService) + ws.Path(rootPath) + container.Add(ws) + existingWebServices[rootPath] = ws } - // Namespace handlers - sub = nil - prefix += "/namespaces/{namespace}" - for _, route := range routes.Namespace { - if sub == nil { - sub = router.PathPrefix(prefix).Subrouter() - sub.MethodNotAllowedHandler = &methodNotAllowedHandler{} - } - - useful = true - methods, err := methodsFromSpec(route.Path, route.Spec) - if err != nil { - return nil, err - } - + // Add root handlers using OpenAPI specs + for _, route := range routes.Root { instrumentedHandler := metrics.InstrumentHandler( gv.Group, gv.Version, - route.Path, // Use path as resource identifier + route.Path, route.Handler, ) + routeFunction := convertHandlerToRouteFunction(instrumentedHandler) - sub.HandleFunc("/"+route.Path, instrumentedHandler). - Methods(methods...) + // Use OpenAPI spec to configure routes properly + if err := addRouteFromSpec(ws, route.Path, route.Spec, routeFunction, false); err != nil { + return fmt.Errorf("failed to add root route %s: %w", route.Path, err) + } + } + + // Add namespace handlers using OpenAPI specs + for _, route := range routes.Namespace { + instrumentedHandler := metrics.InstrumentHandler( + gv.Group, + gv.Version, + route.Path, + route.Handler, + ) + routeFunction := convertHandlerToRouteFunction(instrumentedHandler) + + // Use OpenAPI spec to configure routes properly + if err := addRouteFromSpec(ws, route.Path, route.Spec, routeFunction, true); err != nil { + return fmt.Errorf("failed to add namespace route %s: %w", route.Path, err) + } } } } - if !useful { - return delegateHandler, nil - } - - // Per Gorilla Mux issue here: https://github.com/gorilla/mux/issues/616#issuecomment-798807509 - // default handler must come last - router.PathPrefix("/").Handler(delegateHandler) - - return &requestHandler{ - router: router, - }, nil + return nil } -func (h *requestHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { - h.router.ServeHTTP(w, req) +// addRouteFromSpec adds routes to a WebService using OpenAPI specs +func addRouteFromSpec(ws *restful.WebService, routePath string, pathProps *spec3.PathProps, handler restful.RouteFunction, isNamespaced bool) error { + if pathProps == nil { + return fmt.Errorf("pathProps cannot be nil for route %s", routePath) + } + + // Build the full path (relative to WebService root) + var fullPath string + if isNamespaced { + fullPath = "/namespaces/{namespace}/" + routePath + } else { + fullPath = "/" + routePath + } + + // Add routes for each HTTP method defined in the OpenAPI spec + operations := map[string]*spec3.Operation{ + "GET": pathProps.Get, + "POST": pathProps.Post, + "PUT": pathProps.Put, + "PATCH": pathProps.Patch, + "DELETE": pathProps.Delete, + } + + for method, operation := range operations { + if operation == nil { + continue + } + + // Create route builder for this method + var routeBuilder *restful.RouteBuilder + switch method { + case "GET": + routeBuilder = ws.GET(fullPath) + case "POST": + routeBuilder = ws.POST(fullPath) + case "PUT": + routeBuilder = ws.PUT(fullPath) + case "PATCH": + routeBuilder = ws.PATCH(fullPath) + case "DELETE": + routeBuilder = ws.DELETE(fullPath) + } + + // Set operation ID from OpenAPI spec (with K8s verb prefix if needed) + operationID := operation.OperationId + if operationID == "" { + // Generate from path if not specified + operationID = generateOperationNameFromPath(routePath) + } + operationID = prefixRouteIDWithK8sVerbIfNotPresent(operationID, method) + routeBuilder = routeBuilder.Operation(operationID) + + // Add description from OpenAPI spec + if operation.Description != "" { + routeBuilder = routeBuilder.Doc(operation.Description) + } + + // Check if namespace parameter is already in the OpenAPI spec + hasNamespaceParam := false + if operation.Parameters != nil { + for _, param := range operation.Parameters { + if param.Name == "namespace" && param.In == "path" { + hasNamespaceParam = true + break + } + } + } + + // Add namespace parameter for namespaced routes if not already in spec + if isNamespaced && !hasNamespaceParam { + routeBuilder = routeBuilder.Param(restful.PathParameter("namespace", "object name and auth scope, such as for teams and projects")) + } + + // Add parameters from OpenAPI spec + if operation.Parameters != nil { + for _, param := range operation.Parameters { + switch param.In { + case "path": + routeBuilder = routeBuilder.Param(restful.PathParameter(param.Name, param.Description)) + case "query": + routeBuilder = routeBuilder.Param(restful.QueryParameter(param.Name, param.Description)) + case "header": + routeBuilder = routeBuilder.Param(restful.HeaderParameter(param.Name, param.Description)) + } + } + } + + // Note: Request/response schemas are already defined in the OpenAPI spec from builders + // and will be added to the OpenAPI document via addBuilderRoutes in openapi.go. + // We don't duplicate that information here since restful uses the route metadata + // for OpenAPI generation, which is handled separately in this codebase. + + // Register the route with handler + ws.Route(routeBuilder.To(handler)) + } + + return nil } -func methodsFromSpec(slug string, props *spec3.PathProps) ([]string, error) { - if props == nil { - return []string{"GET", "POST", "PUT", "PATCH", "DELETE"}, nil +func prefixRouteIDWithK8sVerbIfNotPresent(operationID string, method string) string { + for _, verb := range allowedK8sVerbs { + if len(operationID) > len(verb) && operationID[:len(verb)] == verb { + return operationID + } } - - methods := make([]string, 0) - if props.Get != nil { - methods = append(methods, "GET") - } - if props.Post != nil { - methods = append(methods, "POST") - } - if props.Put != nil { - methods = append(methods, "PUT") - } - if props.Patch != nil { - methods = append(methods, "PATCH") - } - if props.Delete != nil { - methods = append(methods, "DELETE") - } - - if len(methods) == 0 { - return nil, fmt.Errorf("invalid OpenAPI Spec for slug=%s without any methods in PathProps", slug) - } - - return methods, nil + return fmt.Sprintf("%s%s", httpMethodToK8sVerb[strings.ToUpper(method)], operationID) } -type methodNotAllowedHandler struct{} +var allowedK8sVerbs = []string{ + "get", "log", "read", "replace", "patch", "delete", "deletecollection", "watch", "connect", "proxy", "list", "create", "patch", +} -func (h *methodNotAllowedHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { - w.WriteHeader(405) // method not allowed +var httpMethodToK8sVerb = map[string]string{ + http.MethodGet: "get", + http.MethodPost: "create", + http.MethodPut: "replace", + http.MethodPatch: "patch", + http.MethodDelete: "delete", + http.MethodConnect: "connect", + http.MethodOptions: "connect", // No real equivalent to options and head + http.MethodHead: "connect", +} + +// generateOperationNameFromPath creates an operation name from a route path. +// The operation name is used by the OpenAPI generator and should be descriptive. +// It uses meaningful path segments to create readable yet unique operation names. +// Examples: +// - "/search" -> "Search" +// - "/snapshots/create" -> "SnapshotsCreate" +// - "ofrep/v1/evaluate/flags" -> "OfrepEvaluateFlags" +// - "ofrep/v1/evaluate/flags/{flagKey}" -> "OfrepEvaluateFlagsFlagKey" +func generateOperationNameFromPath(routePath string) string { + // Remove leading slash and split by path segments + parts := strings.Split(strings.TrimPrefix(routePath, "/"), "/") + + // Filter to keep meaningful segments and path parameters + var nameParts []string + skipPrefixes := map[string]bool{ + "namespaces": true, + "apis": true, + } + + for _, part := range parts { + if part == "" { + continue + } + + // Extract parameter name from {paramName} format + if strings.HasPrefix(part, "{") && strings.HasSuffix(part, "}") { + paramName := part[1 : len(part)-1] + // Skip generic parameters like {namespace}, but keep specific ones like {flagKey} + if paramName != "namespace" && paramName != "name" { + nameParts = append(nameParts, strings.ToUpper(paramName[:1])+paramName[1:]) + } + continue + } + + // Skip common prefixes + if skipPrefixes[strings.ToLower(part)] { + continue + } + + // Skip version segments like v1, v0alpha1, v2beta1, etc. + if strings.HasPrefix(strings.ToLower(part), "v") && + (len(part) <= 3 || strings.Contains(strings.ToLower(part), "alpha") || strings.Contains(strings.ToLower(part), "beta")) { + continue + } + + // Capitalize first letter and add to parts + if len(part) > 0 { + nameParts = append(nameParts, strings.ToUpper(part[:1])+part[1:]) + } + } + + if len(nameParts) == 0 { + return "Route" + } + + return strings.Join(nameParts, "") } diff --git a/pkg/services/apiserver/config.go b/pkg/services/apiserver/config.go index 499cdb4df6f..a0a9f007bcd 100644 --- a/pkg/services/apiserver/config.go +++ b/pkg/services/apiserver/config.go @@ -5,7 +5,6 @@ import ( "net" "path/filepath" "strconv" - "strings" "github.com/grafana/grafana/pkg/services/apiserver/options" "github.com/grafana/grafana/pkg/services/featuremgmt" @@ -41,15 +40,6 @@ func applyGrafanaConfig(cfg *setting.Cfg, features featuremgmt.FeatureToggles, o apiserverCfg := cfg.SectionWithEnvOverrides("grafana-apiserver") runtimeConfig := apiserverCfg.Key("runtime_config").String() - runtimeConfigSplit := strings.Split(runtimeConfig, ",") - - // TODO: temporary fix to allow disabling local features service and still being able to use its authz handler - if !cfg.OpenFeature.APIEnabled { - runtimeConfigSplit = append(runtimeConfigSplit, "features.grafana.app/v0alpha1=false") - } - - runtimeConfig = strings.Join(runtimeConfigSplit, ",") - if runtimeConfig != "" { if err := o.APIEnablementOptions.RuntimeConfig.Set(runtimeConfig); err != nil { return fmt.Errorf("failed to set runtime config: %w", err) diff --git a/pkg/services/apiserver/service.go b/pkg/services/apiserver/service.go index 6c92350ec2a..7fd5e51a96b 100644 --- a/pkg/services/apiserver/service.go +++ b/pkg/services/apiserver/service.go @@ -155,7 +155,7 @@ func ProvideService( features: features, rr: rr, builders: []builder.APIGroupBuilder{}, - authorizer: authorizer.NewGrafanaBuiltInSTAuthorizer(cfg), + authorizer: authorizer.NewGrafanaBuiltInSTAuthorizer(), tracing: tracing, db: db, // For Unified storage metrics: reg, @@ -443,6 +443,19 @@ func (s *service) start(ctx context.Context) error { return err } + // Augment existing WebServices with custom routes from builders + // This directly adds routes to existing WebServices using the OpenAPI specs from builders + if server.Handler != nil && server.Handler.GoRestfulContainer != nil { + if err := builder.AugmentWebServicesWithCustomRoutes( + server.Handler.GoRestfulContainer, + builders, + s.metrics, + serverConfig.MergedResourceConfig, + ); err != nil { + return fmt.Errorf("failed to augment web services with custom routes: %w", err) + } + } + // stash the options for later use s.options = o diff --git a/pkg/tests/apis/config_test.go b/pkg/tests/apis/config_test.go deleted file mode 100644 index 665d79a5e50..00000000000 --- a/pkg/tests/apis/config_test.go +++ /dev/null @@ -1,144 +0,0 @@ -package apis - -import ( - "testing" - - "github.com/stretchr/testify/require" - - "github.com/grafana/grafana/pkg/tests/testinfra" - "github.com/grafana/grafana/pkg/util/testutil" -) - -const pluginsDiscoveryJSON = `[ -{ - "version": "v0alpha1", - "freshness": "Current", - "resources": [ - { - "resource": "metas", - "responseKind": { - "group": "", - "kind": "Meta", - "version": "" - }, - "scope": "Namespaced", - "singularResource": "meta", - "subresources": [ - { - "responseKind": { - "group": "", - "kind": "Meta", - "version": "" - }, - "subresource": "status", - "verbs": [ - "get", - "patch", - "update" - ] - } - ], - "verbs": [ - "get", - "list" - ] - }, - { - "resource": "plugins", - "responseKind": { - "group": "", - "kind": "Plugin", - "version": "" - }, - "scope": "Namespaced", - "singularResource": "plugin", - "subresources": [ - { - "responseKind": { - "group": "", - "kind": "Plugin", - "version": "" - }, - "subresource": "status", - "verbs": [ - "get", - "patch", - "update" - ] - } - ], - "verbs": [ - "create", - "delete", - "deletecollection", - "get", - "list", - "patch", - "update", - "watch" - ] - } - ] -} -]` - -func setupHelper(t *testing.T, openFeatureAPIEnabled bool) *K8sTestHelper { - t.Helper() - helper := NewK8sTestHelper(t, testinfra.GrafanaOpts{ - AppModeProduction: true, - DisableAnonymous: true, - APIServerRuntimeConfig: "plugins.grafana.app/v0alpha1=true", - OpenFeatureAPIEnabled: openFeatureAPIEnabled, - }) - t.Cleanup(func() { helper.Shutdown() }) - return helper -} - -func TestIntegrationAPIServerRuntimeConfig(t *testing.T) { - testutil.SkipIntegrationTestInShortMode(t) - - t.Run("discovery with openfeature api enabled", func(t *testing.T) { - helper := setupHelper(t, true) - disco, err := helper.GetGroupVersionInfoJSON("features.grafana.app") - require.NoError(t, err) - require.JSONEq(t, `[ - { - "freshness": "Current", - "resources": [ - { - "resource": "noop", - "responseKind": { - "group": "", - "kind": "Status", - "version": "" - }, - "scope": "Namespaced", - "singularResource": "noop", - "verbs": [ - "get" - ] - } - ], - "version": "v0alpha1" - } - ]`, disco) - - // plugins should still be discoverable - disco, err = helper.GetGroupVersionInfoJSON("plugins.grafana.app") - require.NoError(t, err) - require.JSONEq(t, pluginsDiscoveryJSON, disco) - require.NoError(t, err) - }) - - t.Run("discovery with openfeature api false", func(t *testing.T) { - helper := setupHelper(t, false) - _, err := helper.GetGroupVersionInfoJSON("features.grafana.app") - require.Error(t, err, "expected error when openfeature api is disabled") - - // plugins should still be discoverable - disco, err := helper.GetGroupVersionInfoJSON("plugins.grafana.app") - require.NoError(t, err) - require.JSONEq(t, pluginsDiscoveryJSON, disco) - require.NoError(t, err) - }) -} diff --git a/pkg/tests/testinfra/testinfra.go b/pkg/tests/testinfra/testinfra.go index ebf82f17864..b5e04d65a40 100644 --- a/pkg/tests/testinfra/testinfra.go +++ b/pkg/tests/testinfra/testinfra.go @@ -320,8 +320,9 @@ func CreateGrafDir(t *testing.T, opts GrafanaOpts) (string, string) { require.NoError(t, err) _, err = openFeatureSect.NewKey("enable_api", strconv.FormatBool(opts.OpenFeatureAPIEnabled)) require.NoError(t, err) - if !opts.OpenFeatureAPIEnabled { - _, err = openFeatureSect.NewKey("provider", "static") // in practice, APIEnabled being false goes with features-service type, but trying to make tests work + + if opts.OpenFeatureAPIEnabled { + _, err = openFeatureSect.NewKey("provider", "static") require.NoError(t, err) _, err = openFeatureSect.NewKey("targetingKey", "grafana") require.NoError(t, err)