From 47c3c483ef72f6f44a892c92b8310d9fea3801e3 Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Mon, 29 Sep 2025 16:43:40 +0200 Subject: [PATCH 1/2] fix support for workspace paths On-behalf-of: @SAP christoph.mewes@sap.com --- internal/controller/apiexport/controller.go | 29 ++++++++++--- internal/controller/apiexport/reconciler.go | 41 ++++++++++++++----- internal/controller/syncmanager/controller.go | 5 +++ test/utils/fixtures.go | 10 +++++ 4 files changed, 69 insertions(+), 16 deletions(-) diff --git a/internal/controller/apiexport/controller.go b/internal/controller/apiexport/controller.go index d51ef23..1cd144d 100644 --- a/internal/controller/apiexport/controller.go +++ b/internal/controller/apiexport/controller.go @@ -128,7 +128,7 @@ func (r *Reconciler) reconcile(ctx context.Context) error { // for each PR, we note down the created ARS and also the GVKs of related resources arsList := sets.New[string]() - claimedResources := sets.New[string]() + claimedResources := sets.New[kcpdevv1alpha1.GroupResource]() // PublishedResources use kinds, but the PermissionClaims use resource names (plural), // so we must translate accordingly @@ -139,7 +139,17 @@ func (r *Reconciler) reconcile(ctx context.Context) error { // to evaluate the namespace filter, the agent needs to fetch the namespace if filter := pubResource.Spec.Filter; filter != nil && filter.Namespace != nil { - claimedResources.Insert("namespaces") + claimedResources.Insert(kcpdevv1alpha1.GroupResource{ + Group: "", + Resource: "namespaces", + }) + } + + if pubResource.Spec.EnableWorkspacePaths { + claimedResources.Insert(kcpdevv1alpha1.GroupResource{ + Group: "core.kcp.io", + Resource: "logicalclusters", + }) } for _, rr := range pubResource.Spec.Related { @@ -150,18 +160,27 @@ func (r *Reconciler) reconcile(ctx context.Context) error { return fmt.Errorf("unknown related resource kind %q: %w", rr.Kind, err) } - claimedResources.Insert(resource.Resource) + claimedResources.Insert(kcpdevv1alpha1.GroupResource{ + Group: "", + Resource: resource.Resource, + }) } } // Related resources (Secrets, ConfigMaps) are namespaced and so the Sync Agent will // always need to be able to see and manage namespaces. if claimedResources.Len() > 0 { - claimedResources.Insert("namespaces") + claimedResources.Insert(kcpdevv1alpha1.GroupResource{ + Group: "", + Resource: "namespaces", + }) } // We always want to create events. - claimedResources.Insert("events") + claimedResources.Insert(kcpdevv1alpha1.GroupResource{ + Group: "", + Resource: "events", + }) if arsList.Len() == 0 { r.log.Debug("No ready PublishedResources available.") diff --git a/internal/controller/apiexport/reconciler.go b/internal/controller/apiexport/reconciler.go index cc83272..16aee9c 100644 --- a/internal/controller/apiexport/reconciler.go +++ b/internal/controller/apiexport/reconciler.go @@ -17,6 +17,7 @@ limitations under the License. package apiexport import ( + "fmt" "slices" "strings" @@ -36,7 +37,7 @@ import ( // by a controller in kcp. Make sure you don't create a reconciling conflict! func (r *Reconciler) createAPIExportReconciler( availableResourceSchemas sets.Set[string], - claimedResourceKinds sets.Set[string], + claimedResourceKinds sets.Set[kcpdevv1alpha1.GroupResource], agentName string, apiExportName string, recorder record.EventRecorder, @@ -59,28 +60,38 @@ func (r *Reconciler) createAPIExportReconciler( // only ensure the ones originating from the published resources; // step 1 is to collect all existing claims with the same properties // as ours. - existingClaims := sets.New[string]() + existingClaims := sets.New[kcpdevv1alpha1.GroupResource]() for _, claim := range existing.Spec.PermissionClaims { - if claim.All && claim.Group == "" && len(claim.ResourceSelector) == 0 { - existingClaims.Insert(claim.Resource) + if claim.All && len(claim.ResourceSelector) == 0 { + existingClaims.Insert(claim.GroupResource) } } missingClaims := claimedResourceKinds.Difference(existingClaims) + claimsToAdd := missingClaims.UnsortedList() + slices.SortStableFunc(claimsToAdd, func(a, b kcpdevv1alpha1.GroupResource) int { + if a.Group != b.Group { + return strings.Compare(a.Group, b.Group) + } + + return strings.Compare(a.Resource, b.Resource) + }) + // add our missing claims - for _, claimed := range sets.List(missingClaims) { + for _, claimed := range claimsToAdd { existing.Spec.PermissionClaims = append(existing.Spec.PermissionClaims, kcpdevv1alpha1.PermissionClaim{ - GroupResource: kcpdevv1alpha1.GroupResource{ - Group: "", - Resource: claimed, - }, - All: true, + GroupResource: claimed, + All: true, }) } if missingClaims.Len() > 0 { - recorder.Eventf(existing, corev1.EventTypeNormal, "AddingPermissionClaims", "Added new permission claim(s) for all %s.", strings.Join(sets.List(missingClaims), ", ")) + claims := make([]string, 0, len(claimsToAdd)) + for _, claimed := range claimsToAdd { + claims = append(claims, groupResourceToString(claimed)) + } + recorder.Eventf(existing, corev1.EventTypeNormal, "AddingPermissionClaims", "Added new permission claim(s) for all %s.", strings.Join(claims, ", ")) } // prevent reconcile loops by ensuring a stable order @@ -101,6 +112,14 @@ func (r *Reconciler) createAPIExportReconciler( } } +func groupResourceToString(gr kcpdevv1alpha1.GroupResource) string { + if gr.Group == "" { + return gr.Resource + } + + return fmt.Sprintf("%s/%s", gr.Group, gr.Resource) +} + func mergeResourceSchemas(existing []string, configured sets.Set[string]) []string { var result []string diff --git a/internal/controller/syncmanager/controller.go b/internal/controller/syncmanager/controller.go index 60c3855..e159d44 100644 --- a/internal/controller/syncmanager/controller.go +++ b/internal/controller/syncmanager/controller.go @@ -29,6 +29,7 @@ import ( syncagentv1alpha1 "github.com/kcp-dev/api-syncagent/sdk/apis/syncagent/v1alpha1" kcpapisv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha1" + kcpcorev1alpha1 "github.com/kcp-dev/kcp/sdk/apis/core/v1alpha1" apiexportprovider "github.com/kcp-dev/multicluster-provider/apiexport" mccontroller "sigs.k8s.io/multicluster-runtime/pkg/controller" mcmanager "sigs.k8s.io/multicluster-runtime/pkg/manager" @@ -248,6 +249,10 @@ func (r *Reconciler) ensureManager(log *zap.SugaredLogger, vwURL string) error { return fmt.Errorf("failed to register scheme %s: %w", kcpapisv1alpha1.SchemeGroupVersion, err) } + if err := kcpcorev1alpha1.AddToScheme(scheme); err != nil { + return fmt.Errorf("failed to register scheme %s: %w", kcpcorev1alpha1.SchemeGroupVersion, err) + } + if r.vwProvider == nil { log.Debug("Setting up APIExport provider…") diff --git a/test/utils/fixtures.go b/test/utils/fixtures.go index b862295..45a8711 100644 --- a/test/utils/fixtures.go +++ b/test/utils/fixtures.go @@ -287,6 +287,16 @@ func BindToAPIExport(t *testing.T, ctx context.Context, client ctrlruntimeclient }, State: kcpapisv1alpha1.ClaimAccepted, }, + { + PermissionClaim: kcpapisv1alpha1.PermissionClaim{ + GroupResource: kcpapisv1alpha1.GroupResource{ + Group: "core.kcp.io", + Resource: "logicalclusters", + }, + All: true, + }, + State: kcpapisv1alpha1.ClaimAccepted, + }, }, }, } From 8ff47bb3565e848a815187178acb0447c93ee55e Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Mon, 29 Sep 2025 16:49:47 +0200 Subject: [PATCH 2/2] add basic e2e test for path support On-behalf-of: @SAP christoph.mewes@sap.com --- Makefile | 2 +- test/e2e/sync/primary_test.go | 100 ++++++++++++++++++++++++++++++++++ 2 files changed, 101 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index ad61e4f..2d41a83 100644 --- a/Makefile +++ b/Makefile @@ -106,7 +106,7 @@ $(YQ): yq_* KCP = _tools/kcp -KCP_VERSION ?= 0.28.1 +export KCP_VERSION ?= 0.28.1 .PHONY: $(KCP) $(KCP): diff --git a/test/e2e/sync/primary_test.go b/test/e2e/sync/primary_test.go index e691b83..42293e0 100644 --- a/test/e2e/sync/primary_test.go +++ b/test/e2e/sync/primary_test.go @@ -739,3 +739,103 @@ spec: t.Fatalf("Failed to wait for object to be synced down: %v", err) } } + +func TestSyncWithWorkspacePath(t *testing.T) { + const ( + apiExportName = "kcp.example.com" + kcpGroupName = "kcp.example.com" + orgWorkspace = "sync-with-paths" + ) + + ctx := t.Context() + ctrlruntime.SetLogger(logr.Discard()) + + // setup a test environment in kcp + orgKubconfig := utils.CreateOrganization(t, ctx, orgWorkspace, apiExportName) + + // start a service cluster + envtestKubeconfig, envtestClient, _ := utils.RunEnvtest(t, []string{ + "test/crds/crontab.yaml", + }) + + // publish Crontabs and Backups + t.Logf("Publishing CRDs…") + prCrontabs := &syncagentv1alpha1.PublishedResource{ + ObjectMeta: metav1.ObjectMeta{ + Name: "publish-crontabs", + }, + Spec: syncagentv1alpha1.PublishedResourceSpec{ + Resource: syncagentv1alpha1.SourceResourceDescriptor{ + APIGroup: "example.com", + Version: "v1", + Kind: "CronTab", + }, + // These rules make finding the local object easier, but should not be used in production. + Naming: &syncagentv1alpha1.ResourceNaming{ + Name: "{{ .Object.metadata.name }}", + Namespace: "synced-{{ .Object.metadata.namespace }}", + }, + Projection: &syncagentv1alpha1.ResourceProjection{ + Group: kcpGroupName, + }, + EnableWorkspacePaths: true, + }, + } + + if err := envtestClient.Create(ctx, prCrontabs); err != nil { + t.Fatalf("Failed to create PublishedResource: %v", err) + } + + // start the agent in the background to update the APIExport with the CronTabs API + utils.RunAgent(ctx, t, "bob", orgKubconfig, envtestKubeconfig, apiExportName) + + // wait until the API is available + kcpClusterClient := utils.GetKcpAdminClusterClient(t) + + teamClusterPath := logicalcluster.NewPath("root").Join(orgWorkspace).Join("team-1") + teamClient := kcpClusterClient.Cluster(teamClusterPath) + + utils.WaitForBoundAPI(t, ctx, teamClient, schema.GroupVersionResource{ + Group: kcpGroupName, + Version: "v1", + Resource: "crontabs", + }) + + // create a Crontab object in a team workspace + t.Log("Creating CronTab in kcp…") + crontab := utils.YAMLToUnstructured(t, ` +apiVersion: kcp.example.com/v1 +kind: CronTab +metadata: + namespace: default + name: my-crontab +spec: + cronSpec: '* * *' + image: ubuntu:latest +`) + + if err := teamClient.Create(ctx, crontab); err != nil { + t.Fatalf("Failed to create CronTab in kcp: %v", err) + } + + // wait for the agent to sync the object down into the service cluster + + t.Logf("Wait for CronTab to be synced…") + copy := &unstructured.Unstructured{} + copy.SetAPIVersion("example.com/v1") + copy.SetKind("CronTab") + + err := wait.PollUntilContextTimeout(ctx, 500*time.Millisecond, 30*time.Second, false, func(ctx context.Context) (done bool, err error) { + copyKey := types.NamespacedName{Namespace: "synced-default", Name: "my-crontab"} + return envtestClient.Get(ctx, copyKey, copy) == nil, nil + }) + if err != nil { + t.Fatalf("Failed to wait for object to be synced down: %v", err) + } + + ann := "syncagent.kcp.io/remote-object-workspace-path" + + if value := copy.GetAnnotations()[ann]; value != teamClusterPath.String() { + t.Fatalf("Expected %s annotation to be %q, but is %q.", ann, teamClusterPath.String(), value) + } +}