Merge pull request #3726 from olamilekan000/add-path-to-cachedresourcerefrence-api

kcp-ci-bot · web-flow · commit a6829d3f39fb · 2025-11-26T15:42:12.000+01:00
add path to cachedresource api
diff --git a/config/crds/cache.kcp.io_cachedresourceendpointslices.yaml b/config/crds/cache.kcp.io_cachedresourceendpointslices.yaml
@@ -59,6 +59,12 @@ spec:
                     description: name is the name of the CachedResource the reference
                       points to.
                     type: string
+                  path:
+                    description: |-
+                      path is a logical cluster path where the CachedResource is defined. If empty,
+                      the CachedResource is assumed to be co-located with the referencing resource.
+                    pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(:[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
+                    type: string
                 required:
                 - name
                 type: object
diff --git a/config/root-phase0/apiexport-cache.kcp.io.yaml b/config/root-phase0/apiexport-cache.kcp.io.yaml
@@ -7,7 +7,7 @@ spec:
   resources:
   - group: cache.kcp.io
     name: cachedresourceendpointslices
-    schema: v251008-c4825908e.cachedresourceendpointslices.cache.kcp.io
+    schema: v251120-d8e87a979.cachedresourceendpointslices.cache.kcp.io
     storage:
       crd: {}
   - group: cache.kcp.io
diff --git a/config/root-phase0/apiresourceschema-cachedresourceendpointslices.cache.kcp.io.yaml b/config/root-phase0/apiresourceschema-cachedresourceendpointslices.cache.kcp.io.yaml
@@ -2,7 +2,7 @@ apiVersion: apis.kcp.io/v1alpha1
 kind: APIResourceSchema
 metadata:
   creationTimestamp: null
-  name: v251008-c4825908e.cachedresourceendpointslices.cache.kcp.io
+  name: v251120-d8e87a979.cachedresourceendpointslices.cache.kcp.io
 spec:
   group: cache.kcp.io
   names:
@@ -56,6 +56,12 @@ spec:
                   description: name is the name of the CachedResource the reference
                     points to.
                   type: string
+                path:
+                  description: |-
+                    path is a logical cluster path where the CachedResource is defined. If empty,
+                    the CachedResource is assumed to be co-located with the referencing resource.
+                  pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(:[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
+                  type: string
               required:
               - name
               type: object
diff --git a/pkg/admission/pathannotation/pathannotation_admission.go b/pkg/admission/pathannotation/pathannotation_admission.go
@@ -29,6 +29,7 @@ import (
 
 	"github.com/kcp-dev/logicalcluster/v3"
 	apisv1alpha2 "github.com/kcp-dev/sdk/apis/apis/v1alpha2"
+	cachev1alpha1 "github.com/kcp-dev/sdk/apis/cache/v1alpha1"
 	"github.com/kcp-dev/sdk/apis/core"
 	corev1alpha1 "github.com/kcp-dev/sdk/apis/core/v1alpha1"
 	tenancyv1alpha1 "github.com/kcp-dev/sdk/apis/tenancy/v1alpha1"
@@ -69,6 +70,7 @@ type pathAnnotationPlugin struct {
 
 var pathAnnotationResources = sets.New[string](
 	apisv1alpha2.Resource("apiexports").String(),
+	cachev1alpha1.Resource("cachedresources").String(),
 	tenancyv1alpha1.Resource("workspacetypes").String(),
 )
 
diff --git a/pkg/admission/pathannotation/pathannotation_admission_test.go b/pkg/admission/pathannotation/pathannotation_admission_test.go
@@ -31,6 +31,7 @@ import (
 	"github.com/kcp-dev/logicalcluster/v3"
 	apisv1alpha1 "github.com/kcp-dev/sdk/apis/apis/v1alpha1"
 	apisv1alpha2 "github.com/kcp-dev/sdk/apis/apis/v1alpha2"
+	cachev1alpha1 "github.com/kcp-dev/sdk/apis/cache/v1alpha1"
 	"github.com/kcp-dev/sdk/apis/core"
 	corev1alpha1 "github.com/kcp-dev/sdk/apis/core/v1alpha1"
 	tenancyv1alpha1 "github.com/kcp-dev/sdk/apis/tenancy/v1alpha1"
@@ -133,6 +134,15 @@ func TestPathAnnotationAdmit(t *testing.T) {
 			getLogicalCluster:       getCluster("foo"),
 			validateAdmissionObject: objectHasPathAnnotation("root:foo"),
 		},
+		{
+			name:                    "happy path: a CachedResource is annotated with a path",
+			admissionVerb:           admission.Create,
+			admissionResource:       cachev1alpha1.SchemeGroupVersion.WithResource("cachedresources"),
+			admissionObject:         &cachev1alpha1.CachedResource{},
+			admissionContext:        admissionContextFor("foo"),
+			getLogicalCluster:       getCluster("foo"),
+			validateAdmissionObject: objectHasPathAnnotation("root:foo"),
+		},
 
 		{
 			name:                    "happy path: a WorkspaceType is annotated with a path",
@@ -239,6 +249,15 @@ func TestPathAnnotationValidate(t *testing.T) {
 			admissionContext:  admissionContextFor("foo"),
 			getLogicalCluster: getCluster("foo"),
 		},
+		{
+			name:              "a CachedResource with incorrect path annotation is NOT admitted",
+			admissionVerb:     admission.Create,
+			admissionResource: cachev1alpha1.SchemeGroupVersion.WithResource("cachedresources"),
+			admissionObject:   &cachev1alpha1.CachedResource{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{core.LogicalClusterPathAnnotationKey: "root:bar"}}},
+			admissionContext:  admissionContextFor("foo"),
+			getLogicalCluster: getCluster("foo"),
+			expectError:       true,
+		},
 	}
 
 	for _, scenario := range scenarios {
diff --git a/pkg/openapi/zz_generated.openapi.go b/pkg/openapi/zz_generated.openapi.go
diff --git a/pkg/reconciler/cache/cachedresourceendpointslice/cachedresourceendpointslice_indexers.go b/pkg/reconciler/cache/cachedresourceendpointslice/cachedresourceendpointslice_indexers.go
@@ -60,6 +60,14 @@ func IndexCachedResourceEndpointSliceByCachedResourceFunc(obj interface{}) ([]st
 	}
 
 	pathLocal := logicalcluster.From(slice).Path()
-	// TODO(gman0): add an optional external path index key once we add "CachedResourceEndpointSlice.spec.cachedResource.path".
-	return []string{pathLocal.Join(slice.Spec.CachedResource.Name).String()}, nil
+	keys := []string{pathLocal.Join(slice.Spec.CachedResource.Name).String()}
+
+	if refPath := logicalcluster.NewPath(slice.Spec.CachedResource.Path); !refPath.Empty() {
+		key := refPath.Join(slice.Spec.CachedResource.Name).String()
+		if key != keys[0] {
+			keys = append(keys, key)
+		}
+	}
+
+	return keys, nil
 }
diff --git a/pkg/reconciler/cache/cachedresourceendpointslice/cachedresourceendpointslice_reconcile.go b/pkg/reconciler/cache/cachedresourceendpointslice/cachedresourceendpointslice_reconcile.go
@@ -30,7 +30,12 @@ import (
 )
 
 func (c *controller) reconcile(ctx context.Context, endpoints *cachev1alpha1.CachedResourceEndpointSlice) error {
-	_, err := c.getCachedResource(logicalcluster.From(endpoints).Path(), endpoints.Spec.CachedResource.Name)
+	cachedResourcePath := logicalcluster.NewPath(endpoints.Spec.CachedResource.Path)
+	if cachedResourcePath.Empty() {
+		cachedResourcePath = logicalcluster.From(endpoints).Path()
+	}
+
+	_, err := c.getCachedResource(cachedResourcePath, endpoints.Spec.CachedResource.Name)
 	if err != nil {
 		if apierrors.IsNotFound(err) {
 			// Don't keep the endpoints if the CachedResource has been deleted.
@@ -41,7 +46,7 @@ func (c *controller) reconcile(ctx context.Context, endpoints *cachev1alpha1.Cac
 				cachev1alpha1.CachedResourceNotFoundReason,
 				conditionsv1alpha1.ConditionSeverityError,
 				"Error getting CachedResource %s|%s",
-				logicalcluster.From(endpoints),
+				cachedResourcePath,
 				endpoints.Spec.CachedResource.Name,
 			)
 			// No need to try again.
@@ -53,7 +58,7 @@ func (c *controller) reconcile(ctx context.Context, endpoints *cachev1alpha1.Cac
 				cachev1alpha1.InternalErrorReason,
 				conditionsv1alpha1.ConditionSeverityError,
 				"Error getting CachedResource %s|%s",
-				logicalcluster.From(endpoints),
+				cachedResourcePath,
 				endpoints.Spec.CachedResource.Name,
 			)
 			return err
diff --git a/pkg/reconciler/cache/cachedresourceendpointslice/cachedresourceendpointslice_reconcile_test.go b/pkg/reconciler/cache/cachedresourceendpointslice/cachedresourceendpointslice_reconcile_test.go
@@ -35,6 +35,7 @@ import (
 )
 
 func TestReconcile(t *testing.T) {
+	const workspacePath = "root:org:ws"
 	tests := map[string]struct {
 		keyMissing                bool
 		cachedResourceMissing     bool
@@ -49,29 +50,45 @@ func TestReconcile(t *testing.T) {
 		wantPartitionValid         bool
 		wantCachedResourceNotValid bool
 		wantPartitionNotValid      bool
+
+		sliceMutator               func(slice *cachev1alpha1.CachedResourceEndpointSlice)
+		expectedCachedResourcePath string
 	}{
 		"CachedResourceValid set to false when CachedResource is missing": {
 			cachedResourceMissing:      true,
 			errorReason:                cachev1alpha1.CachedResourceNotFoundReason,
 			wantCachedResourceNotValid: true,
+			expectedCachedResourcePath: workspacePath,
 		},
 		"CachedResourceValid set to false if an internal error happens when fetching the CachedResource": {
 			cachedResourceInternalErr:  true,
 			wantError:                  true,
 			errorReason:                cachev1alpha1.InternalErrorReason,
 			wantCachedResourceNotValid: true,
+			expectedCachedResourcePath: workspacePath,
 		},
 		"PartitionValid set to false when the Partition is missing": {
-			partitionMissing:      true,
-			errorReason:           cachev1alpha1.PartitionInvalidReferenceReason,
-			wantPartitionNotValid: true,
+			partitionMissing:           true,
+			errorReason:                cachev1alpha1.PartitionInvalidReferenceReason,
+			wantPartitionNotValid:      true,
+			expectedCachedResourcePath: workspacePath,
+		},
+		"CachedResource lookup uses referenced path when provided": {
+			sliceMutator: func(slice *cachev1alpha1.CachedResourceEndpointSlice) {
+				slice.Spec.CachedResource.Path = "root:org:external"
+			},
+			expectedCachedResourcePath: "root:org:external",
+			wantCachedResourceValid:    true,
+			wantPartitionValid:         true,
 		},
 	}
 
 	for name, tc := range tests {
 		t.Run(name, func(t *testing.T) {
+			var gotCachedResourcePath string
 			c := &controller{
 				getCachedResource: func(path logicalcluster.Path, name string) (*cachev1alpha1.CachedResource, error) {
+					gotCachedResourcePath = path.String()
 					if tc.cachedResourceMissing {
 						return nil, apierrors.NewNotFound(cachev1alpha1.Resource("CachedResource"), name)
 					} else if tc.cachedResourceInternalErr {
@@ -113,7 +130,7 @@ func TestReconcile(t *testing.T) {
 			cachedResourceEndpointSlice := &cachev1alpha1.CachedResourceEndpointSlice{
 				ObjectMeta: metav1.ObjectMeta{
 					Annotations: map[string]string{
-						logicalcluster.AnnotationKey: "root:org:ws",
+						logicalcluster.AnnotationKey: workspacePath,
 					},
 					Name: "my-slice",
 				},
@@ -124,6 +141,9 @@ func TestReconcile(t *testing.T) {
 					Partition: "my-partition",
 				},
 			}
+			if tc.sliceMutator != nil {
+				tc.sliceMutator(cachedResourceEndpointSlice)
+			}
 			err := c.reconcile(context.Background(), cachedResourceEndpointSlice)
 			if tc.wantError {
 				require.Error(t, err, "expected an error")
@@ -164,6 +184,10 @@ func TestReconcile(t *testing.T) {
 					conditions.TrueCondition(cachev1alpha1.PartitionValid),
 				)
 			}
+
+			if tc.expectedCachedResourcePath != "" {
+				require.Equal(t, tc.expectedCachedResourcePath, gotCachedResourcePath, "unexpected cached resource lookup path")
+			}
 		})
 	}
 }
diff --git a/pkg/reconciler/cache/cachedresourceendpointsliceurls/cachedresourceendpointsliceurls_controller.go b/pkg/reconciler/cache/cachedresourceendpointsliceurls/cachedresourceendpointsliceurls_controller.go
@@ -110,8 +110,8 @@ func NewController(
 		getMyShard: func() (*corev1alpha1.Shard, error) {
 			return globalShardClusterInformer.Cluster(core.RootCluster).Lister().Get(shardName)
 		},
-		getCachedResource: func(cluster logicalcluster.Name, name string) (*cachev1alpha1.CachedResource, error) {
-			return indexers.ByPathAndName[*cachev1alpha1.CachedResource](cachev1alpha1.Resource("cachedresources"), globalCachedResourcelusterInformer.Informer().GetIndexer(), cluster.Path(), name)
+		getCachedResource: func(path logicalcluster.Path, name string) (*cachev1alpha1.CachedResource, error) {
+			return indexers.ByPathAndName[*cachev1alpha1.CachedResource](cachev1alpha1.Resource("cachedresources"), globalCachedResourcelusterInformer.Informer().GetIndexer(), path, name)
 		},
 		getCachedResourceEndpointSlice: func(cluster logicalcluster.Name, name string) (*cachev1alpha1.CachedResourceEndpointSlice, error) {
 			obj, err := indexers.ByPathAndNameWithFallback[*cachev1alpha1.CachedResourceEndpointSlice](cachev1alpha1.Resource("cachedresourceendpointslices"), localCachedResourceEndpointSliceClusterInformer.Informer().GetIndexer(), globalCachedResourceEndpointSliceClusterInformer.Informer().GetIndexer(), cluster.Path(), name)
@@ -242,7 +242,7 @@ type controller struct {
 	listAPIExportsByCachedResourceIdentityAndGR func(identityHash string, gr schema.GroupResource) ([]*apisv1alpha2.APIExport, error)
 	listAPIBindingsByAPIExports                 func(exports []*apisv1alpha2.APIExport) ([]*apisv1alpha2.APIBinding, error)
 	getMyShard                                  func() (*corev1alpha1.Shard, error)
-	getCachedResource                           func(cluster logicalcluster.Name, name string) (*cachev1alpha1.CachedResource, error)
+	getCachedResource                           func(path logicalcluster.Path, name string) (*cachev1alpha1.CachedResource, error)
 	getCachedResourceEndpointSlice              func(cluster logicalcluster.Name, name string) (*cachev1alpha1.CachedResourceEndpointSlice, error)
 	patchAPIExportEndpointSlice                 func(ctx context.Context, cluster logicalcluster.Path, patch *cachev1alpha1apply.CachedResourceEndpointSliceApplyConfiguration) error
 	patchCachedResourceEndpointSlice            func(ctx context.Context, cluster logicalcluster.Path, patch *cachev1alpha1apply.CachedResourceEndpointSliceApplyConfiguration) error
diff --git a/pkg/reconciler/cache/cachedresourceendpointsliceurls/cachedresourceendpointsliceurls_reconciler.go b/pkg/reconciler/cache/cachedresourceendpointsliceurls/cachedresourceendpointsliceurls_reconciler.go
@@ -56,7 +56,7 @@ type endpointsReconciler struct {
 	listAPIExportsByCachedResourceIdentityAndGR func(identityHash string, gr schema.GroupResource) ([]*apisv1alpha2.APIExport, error)
 	listAPIBindingsByAPIExports                 func(exports []*apisv1alpha2.APIExport) ([]*apisv1alpha2.APIBinding, error)
 	getMyShard                                  func() (*corev1alpha1.Shard, error)
-	getCachedResource                           func(cluster logicalcluster.Name, name string) (*cachev1alpha1.CachedResource, error)
+	getCachedResource                           func(path logicalcluster.Path, name string) (*cachev1alpha1.CachedResource, error)
 	patchAPIExportEndpointSlice                 func(ctx context.Context, cluster logicalcluster.Path, patch *cachev1alpha1apply.CachedResourceEndpointSliceApplyConfiguration) error
 }
 
@@ -105,7 +105,12 @@ func (r *endpointsReconciler) updateEndpoints(ctx context.Context, slice *cachev
 		return nil, nil
 	}
 
-	cr, err := r.getCachedResource(logicalcluster.From(slice), slice.Spec.CachedResource.Name)
+	cachedResourcePath := logicalcluster.NewPath(slice.Spec.CachedResource.Path)
+	if cachedResourcePath.Empty() {
+		cachedResourcePath = logicalcluster.From(slice).Path()
+	}
+
+	cr, err := r.getCachedResource(cachedResourcePath, slice.Spec.CachedResource.Name)
 	if err != nil {
 		return nil, err
 	}
diff --git a/pkg/reconciler/cache/cachedresources/cachedresources_reconcile_endpointslice.go b/pkg/reconciler/cache/cachedresources/cachedresources_reconcile_endpointslice.go
@@ -25,6 +25,7 @@ import (
 
 	"github.com/kcp-dev/logicalcluster/v3"
 	cachev1alpha1 "github.com/kcp-dev/sdk/apis/cache/v1alpha1"
+	"github.com/kcp-dev/sdk/apis/core"
 )
 
 // endpointSlice creates CachedResourceEndopointSlice for the published resource.
@@ -43,6 +44,7 @@ func (r *endpointSlice) reconcile(ctx context.Context, cachedResource *cachev1al
 	}
 
 	clusterName := logicalcluster.From(cachedResource)
+	clusterPath := cachedResource.Annotations[core.LogicalClusterPathAnnotationKey]
 
 	_, err := r.getEndpointSlice(ctx, clusterName, cachedResource.Name)
 	if err != nil {
@@ -62,6 +64,7 @@ func (r *endpointSlice) reconcile(ctx context.Context, cachedResource *cachev1al
 				},
 				Spec: cachev1alpha1.CachedResourceEndpointSliceSpec{
 					CachedResource: cachev1alpha1.CachedResourceReference{
+						Path: clusterPath,
 						Name: cachedResource.Name,
 					},
 				},
diff --git a/staging/src/github.com/kcp-dev/sdk/apis/cache/v1alpha1/types_cachedresource.go b/staging/src/github.com/kcp-dev/sdk/apis/cache/v1alpha1/types_cachedresource.go
@@ -179,6 +179,13 @@ type ResourceCount struct {
 
 // CachedResourceReference is a reference to a CachedResource.
 type CachedResourceReference struct {
+	// path is a logical cluster path where the CachedResource is defined. If empty,
+	// the CachedResource is assumed to be co-located with the referencing resource.
+	//
+	// +optional
+	// +kubebuilder:validation:Pattern:="^[a-z0-9]([-a-z0-9]*[a-z0-9])?(:[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$"
+	Path string `json:"path,omitempty"`
+
 	// name is the name of the CachedResource the reference points to.
 	//
 	// +required
diff --git a/staging/src/github.com/kcp-dev/sdk/client/applyconfiguration/cache/v1alpha1/cachedresourcereference.go b/staging/src/github.com/kcp-dev/sdk/client/applyconfiguration/cache/v1alpha1/cachedresourcereference.go
diff --git a/test/e2e/reconciler/cachedresourceendpointslice/cachedresourceendpointslice_test.go b/test/e2e/reconciler/cachedresourceendpointslice/cachedresourceendpointslice_test.go
