Support mux patterns in VW framework

pkg/server/proxy/lookup.go

@@ -37,12 +37,6 @@ import (
 	kcpauthorization "github.com/kcp-dev/kcp/pkg/authorization"
 	"github.com/kcp-dev/kcp/pkg/index"
 	"github.com/kcp-dev/kcp/pkg/server/proxy/types"
-	"github.com/kcp-dev/kcp/pkg/virtual/admin"
-	"github.com/kcp-dev/kcp/pkg/virtual/apiresourceschema"
-	"github.com/kcp-dev/kcp/pkg/virtual/initializingworkspaces"
-	"github.com/kcp-dev/kcp/pkg/virtual/migratingworkspaces"
-	"github.com/kcp-dev/kcp/pkg/virtual/replication"
-	"github.com/kcp-dev/kcp/pkg/virtual/terminatingworkspaces"
 )
 
 func WithClusterResolver(delegate http.Handler, mappings []types.PathMapping, idx index.Index) http.Handler {
@@ -195,48 +189,9 @@ func newMappingHandler(delegate http.Handler, idx index.Index) http.HandlerFunc
 	}
 }
 
-// apiExportVirtualWorkspaceName is the apiexport virtual workspace's name,
-// builder.VirtualWorkspaceName in pkg/virtual/apiexport/builder, a package too
-// heavy to import here. A test keeps the two equal.
-const apiExportVirtualWorkspaceName = "apiexport"
-
-// virtualWorkspaceClusterOffsets is, for each built-in virtual workspace, the
-// number of path segments its URLs carry between /services/<name>/ and the
-// clusters/<cluster> segment, as its shard-side path resolver parses them:
-//
-//	/services/apiexport/<export cluster>/<export name>/clusters/<cluster>/...
-//	/services/replication/<slice cluster>/<slice name>/clusters/<cluster>/...
-//	/services/apiresourceschema/<consumer cluster>/clusters/<cluster>/...
-//	/services/initializingworkspaces/<initializer>/clusters/<cluster>/...
-//	/services/terminatingworkspaces/<terminator>/clusters/<cluster>/...
-//	/services/admin/clusters/<cluster>/...
-//	/services/migratingworkspaces/clusters/<cluster>/...
-var virtualWorkspaceClusterOffsets = map[string]int{
-	apiExportVirtualWorkspaceName:               2,
-	replication.VirtualWorkspaceName:            2,
-	apiresourceschema.VirtualWorkspaceName:      1,
-	initializingworkspaces.VirtualWorkspaceName: 1,
-	terminatingworkspaces.VirtualWorkspaceName:  1,
-	admin.VirtualWorkspaceName:                  0,
-	migratingworkspaces.VirtualWorkspaceName:    0,
-}
-
 // extractClusterFromPath extracts the cluster name from URLs containing /clusters/<cluster>/
 // For example: /services/foo/clusters/root:orgs:bob/apis/... -> root:orgs:bob.
-//
-// For a built-in virtual workspace the cluster is read where that workspace's
-// URL layout puts it, so a segment before it named "clusters" — an APIExport
-// or a CachedResource of that name — is not taken for the cluster. Other
-// virtual workspaces' layouts are unknown here, and their cluster is the
-// segment after the first /clusters/.
 func extractClusterFromPath(path string) string {
-	if rest, ok := strings.CutPrefix(path, "/services/"); ok {
-		name, rest, _ := strings.Cut(rest, "/")
-		if offset, known := virtualWorkspaceClusterOffsets[name]; known {
-			return clusterAtOffset(rest, offset)
-		}
-	}
-
 	const clustersPrefix = "/clusters/"
 	idx := strings.Index(path, clustersPrefix)
 	if idx == -1 {
@@ -254,18 +209,6 @@ func extractClusterFromPath(path string) string {
 	return rest[:endIdx]
 }
 
-// clusterAtOffset returns the cluster of a virtual workspace path, given the
-// path after /services/<name>/ and the number of segments before its
-// clusters/<cluster> segment. A path without clusters/<cluster> at that
-// position names no cluster.
-func clusterAtOffset(path string, offset int) string {
-	segments := strings.SplitN(path, "/", offset+3)
-	if len(segments) < offset+2 || segments[offset] != "clusters" {
-		return ""
-	}
-	return segments[offset+1]
-}
-
 func resolveClusterName(w http.ResponseWriter, req *http.Request, idx index.Index, clusterName string) (*http.Request, *index.Result) {
 	ctx := req.Context()
 	logger := klog.FromContext(ctx)

pkg/server/proxy/lookup_test.go

@@ -26,7 +26,6 @@ import (
 	"github.com/kcp-dev/logicalcluster/v3"
 
 	"github.com/kcp-dev/kcp/pkg/index/indextest"
-	apiexportbuilder "github.com/kcp-dev/kcp/pkg/virtual/apiexport/builder"
 )
 
 func TestClusterResolveHandlerMigration(t *testing.T) {
@@ -203,91 +202,6 @@ func TestExtractClusterFromPath(t *testing.T) {
 			path:     "/services/foo/clusters/root/apis/v1",
 			expected: "root",
 		},
-		{
-			name:     "replication VW, cached resource named clusters",
-			path:     "/services/replication/1oget0q1249b2vcy/clusters/clusters/385doly4poks8a45/apis/wildwest.dev/v1alpha1/clusters",
-			expected: "385doly4poks8a45",
-		},
-		{
-			name:     "replication VW, cached resource named clusters, at the cluster's root",
-			path:     "/services/replication/1oget0q1249b2vcy/clusters/clusters/385doly4poks8a45",
-			expected: "385doly4poks8a45",
-		},
-		{
-			name:     "replication VW, cached resource named clusters, non-API endpoint",
-			path:     "/services/replication/1oget0q1249b2vcy/clusters/clusters/385doly4poks8a45/healthz",
-			expected: "385doly4poks8a45",
-		},
-		{
-			name:     "apiexport VW, export named clusters",
-			path:     "/services/apiexport/root:providers/clusters/clusters/root:orgs:bob/api/v1/namespaces",
-			expected: "root:orgs:bob",
-		},
-		{
-			name:     "apiexport VW, export named clusters, wildcard cluster",
-			path:     "/services/apiexport/root:providers/clusters/clusters/*/apis/wildwest.dev/v1alpha1/clusters",
-			expected: "*",
-		},
-		{
-			name:     "apiexport VW, export named clusters, trailing slash after the cluster",
-			path:     "/services/apiexport/root:providers/clusters/clusters/root:orgs:bob/",
-			expected: "root:orgs:bob",
-		},
-		{
-			name:     "apiexport VW, export and its workspace both named clusters",
-			path:     "/services/apiexport/clusters/clusters/clusters/root:orgs:bob/apis/wildwest.dev/v1alpha1/clusters",
-			expected: "root:orgs:bob",
-		},
-		{
-			name:     "apiexport VW, a later clusters segment does not move the cluster",
-			path:     "/services/apiexport/root:providers/export/clusters/root:orgs:a/x/clusters/root:orgs:b/apis/v1",
-			expected: "root:orgs:a",
-		},
-		{
-			name:     "apiexport VW without a clusters segment where its layout puts one",
-			path:     "/services/apiexport/root:providers/export/notclusters/root:orgs:bob/apis/v1",
-			expected: "",
-		},
-		{
-			name:     "apiexport VW cut short before its cluster",
-			path:     "/services/apiexport/root:providers/export/clusters",
-			expected: "",
-		},
-		{
-			name:     "initializingworkspaces VW",
-			path:     "/services/initializingworkspaces/root:org:custom/clusters/2x7k9fqp1abcd/apis/core.kcp.io/v1alpha1/logicalclusters",
-			expected: "2x7k9fqp1abcd",
-		},
-		{
-			name:     "terminatingworkspaces VW",
-			path:     "/services/terminatingworkspaces/root:org:custom/clusters/2x7k9fqp1abcd/apis/core.kcp.io/v1alpha1/logicalclusters",
-			expected: "2x7k9fqp1abcd",
-		},
-		{
-			name:     "apiresourceschema VW",
-			path:     "/services/apiresourceschema/root:consumer/clusters/*/apis/apis.kcp.io/v1alpha1/apiresourceschemas",
-			expected: "*",
-		},
-		{
-			name:     "admin VW",
-			path:     "/services/admin/clusters/root:orgs:bob/apis/core.kcp.io/v1alpha1/logicalclusters",
-			expected: "root:orgs:bob",
-		},
-		{
-			name:     "migratingworkspaces VW",
-			path:     "/services/migratingworkspaces/clusters/2x7k9fqp1abcd/apis/v1",
-			expected: "2x7k9fqp1abcd",
-		},
-		{
-			name:     "unknown VW, resource named clusters after the cluster",
-			path:     "/services/foo/clusters/root:orgs:bob/apis/wildwest.dev/v1alpha1/clusters/cluster-a",
-			expected: "root:orgs:bob",
-		},
-		{
-			name:     "unknown VW keeps the first segment after /clusters/, even an empty one",
-			path:     "/services/foo/clusters//clusters/root:orgs:bob/apis/v1",
-			expected: "",
-		},
 	}
 
 	for _, tt := range tests {
@@ -300,10 +214,3 @@ func TestExtractClusterFromPath(t *testing.T) {
 		})
 	}
 }
-
-func TestAPIExportVirtualWorkspaceName(t *testing.T) {
-	t.Parallel()
-	if apiExportVirtualWorkspaceName != apiexportbuilder.VirtualWorkspaceName {
-		t.Errorf("apiExportVirtualWorkspaceName = %q, want the apiexport virtual workspace's name %q", apiExportVirtualWorkspaceName, apiexportbuilder.VirtualWorkspaceName)
-	}
-}

pkg/virtual/admin/builder/build.go

@@ -19,14 +19,13 @@ package builder
 import (
 	"context"
 	"errors"
-	"strings"
+	"path"
 	"sync"
 
 	genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
 	genericapiserver "k8s.io/apiserver/pkg/server"
 
 	kcpdynamic "github.com/kcp-dev/client-go/dynamic"
-	"github.com/kcp-dev/logicalcluster/v3"
 	adminv1alpha1 "github.com/kcp-dev/sdk/apis/admin/v1alpha1"
 	"github.com/kcp-dev/sdk/apis/core"
 	corev1alpha1 "github.com/kcp-dev/sdk/apis/core/v1alpha1"
@@ -46,6 +45,16 @@ import (
 // VW serves one fixed API set regardless of the caller.
 const apiDomainKey = dynamiccontext.APIDomainKey(admin.VirtualWorkspaceName)
 
+// pathPatterns returns the mux patterns of the virtual workspace:
+// rootPathPrefix itself and, for cluster-aware clients, below an optional
+// clusters/<cluster> segment. The views are identical for every value.
+func pathPatterns(rootPathPrefix string) []string {
+	return []string{
+		path.Clean(rootPathPrefix),
+		path.Join(rootPathPrefix, "clusters/{cluster}"),
+	}
+}
+
 // BuildVirtualWorkspace builds the Admin workspace virtual workspace. It
 // serves two cache-backed, installation-wide views: every shard's Shard
 // object, writable for the allow-listed operational fields, and the
@@ -65,22 +74,11 @@ func BuildVirtualWorkspace(
 	identities *apiidentity.Projector,
 	identitiesSynced func() bool,
 ) ([]rootapiserver.NamedVirtualWorkspace, error) {
-	if !strings.HasSuffix(rootPathPrefix, "/") {
-		rootPathPrefix += "/"
-	}
 	readyCh := make(chan struct{})
 
 	vw := &virtualworkspacesdynamic.DynamicVirtualWorkspace{
-		RootPathResolver: framework.RootPathResolverFunc(func(urlPath string, ctx context.Context) (accepted bool, prefixToStrip string, completedContext context.Context) {
-			cluster, prefixToStrip, ok := digestURL(urlPath, rootPathPrefix)
-			if !ok {
-				return false, "", ctx
-			}
-			completedContext = genericapirequest.WithCluster(ctx, cluster)
-			completedContext = dynamiccontext.WithAPIDomainKey(completedContext, apiDomainKey)
-			return true, prefixToStrip, completedContext
-		}),
-		Authorizer: newAuthorizer(),
+		RootPathResolver: framework.RootPathResolverFunc(resolveRootPath),
+		Authorizer:       newAuthorizer(),
 		ReadyChecker: framework.ReadyFunc(func() error {
 			select {
 			case <-readyCh:
@@ -109,46 +107,18 @@ func BuildVirtualWorkspace(
 	}
 
 	return []rootapiserver.NamedVirtualWorkspace{
-		{Name: admin.VirtualWorkspaceName, VirtualWorkspace: vw},
+		{Name: admin.VirtualWorkspaceName, Patterns: pathPatterns(rootPathPrefix), VirtualWorkspace: vw},
 	}, nil
 }
 
-// digestURL accepts /services/admin and everything below it. An optional
-// /clusters/<name-or-*> segment (added by cluster-aware clients) is parsed
-// and stripped; the views are identical for every value.
-func digestURL(urlPath, rootPathPrefix string) (cluster genericapirequest.Cluster, logicalPath string, accepted bool) {
-	if !strings.HasPrefix(urlPath, rootPathPrefix) {
-		return genericapirequest.Cluster{}, "", false
+// resolveRootPath presents the cluster-less aggregate view under the root
+// cluster name, so cluster-scoped machinery has a stable, valid cluster,
+// unless the request names a cluster.
+func resolveRootPath(ctx context.Context, match framework.PathMatch) (accepted bool, prefixToStrip string, completedContext context.Context) {
+	if match.Value(framework.ClusterWildcard) == "" {
+		ctx = genericapirequest.WithCluster(ctx, genericapirequest.Cluster{Name: core.RootCluster})
 	}
-
-	prefix := strings.TrimSuffix(rootPathPrefix, "/")
-	realPath := strings.TrimPrefix(urlPath, prefix)
-	if realPath != "" && !strings.HasPrefix(realPath, "/") {
-		return genericapirequest.Cluster{}, "", false
-	}
-
-	// the aggregate view is cluster-less; present it under the root cluster
-	// name so cluster-scoped machinery has a stable, valid cluster.
-	cluster = genericapirequest.Cluster{Name: core.RootCluster}
-
-	if withoutClustersPrefix, ok := strings.CutPrefix(realPath, "/clusters/"); ok {
-		parts := strings.SplitN(withoutClustersPrefix, "/", 2)
-		path := logicalcluster.NewPath(parts[0])
-		if path == logicalcluster.Wildcard {
-			cluster = genericapirequest.Cluster{Wildcard: true}
-		} else if name, ok := path.Name(); ok {
-			cluster = genericapirequest.Cluster{Name: name}
-		} else {
-			return genericapirequest.Cluster{}, "", false
-		}
-		realPath = "/"
-		if len(parts) > 1 {
-			realPath += parts[1]
-		}
-		return cluster, strings.TrimSuffix(urlPath, realPath), true
-	}
-
-	return cluster, prefix, true
+	return true, match.PrefixToStrip(), dynamiccontext.WithAPIDomainKey(ctx, apiDomainKey)
 }
 
 // adminAPIDefinitionSetProvider serves the fixed API set of this virtual