add apiVersion and identityHash to pubres CRD

On-behalf-of: @SAP christoph.mewes@sap.com

Author: xrstf

deploy/crd/kcp.io/syncagent.kcp.io_publishedresources.yaml

@@ -349,17 +349,35 @@ spec:
                       type: object
                   type: object
                 related:
+                  description: |-
+                    Related configures additional resources that semantically belong to the synced
+                    resource, like a Secret containing generated credentials. Related objects are
+                    synced along the main resource.
                   items:
                     properties:
+                      group:
+                        description: |-
+                          Group is the API group of the related resource. This should be left blank for resources
+                          in the core API group.
+                        type: string
                       identifier:
                         description: |-
                           Identifier is a unique name for this related resource. The name must be unique within one
                           PublishedResource and is the key by which consumers (end users) can identify and consume the
                           related resource. Common names are "connection-details" or "credentials".
                           The identifier must be an alphanumeric string.
                         type: string
+                      identityHash:
+                        description: |-
+                          IdentityHash is the identity hash of a kcp APIExport, in case the given Kind is
+                          provided by an APIExport and not Kube-native.
+                        type: string
                       kind:
-                        description: ConfigMap or Secret
+                        description: |-
+                          Kind is the object kind of the related resource (for example "Secret").
+
+                          Deprecated: Use "Resource" instead. This field is limited to "ConfigMap" and "Secret" and will
+                          be removed in the future. Kind and Resource cannot be specified at the same time.
                         type: string
                       mutation:
                         description: |-
@@ -686,9 +704,33 @@ spec:
                           - service
                           - kcp
                         type: string
+                      projection:
+                        description: |-
+                          Projection is used to change the GVK of a related resource on the opposite side of
+                          its origin.
+                          All fields in the projection are optional. If a field is set, it will overwrite
+                          that field in the GVK.
+                        properties:
+                          group:
+                            description: The API group, for example "myservice.example.com". Leave empty to not modify the API group.
+                            type: string
+                          resource:
+                            description: The resource name, for example "databases". Leave empty to not modify the resource.
+                            type: string
+                          version:
+                            description: The API version, for example "v1beta1". Leave empty to not modify the version.
+                            type: string
+                        type: object
+                      resource:
+                        description: Resource is the name of the related resource (for example "secrets").
+                        type: string
+                      version:
+                        description: |-
+                          Version is the API version of the related resource. This can be left blank to automatically
+                          use the preferred version.
+                        type: string
                     required:
                       - identifier
-                      - kind
                       - object
                       - origin
                     type: object
@@ -723,6 +765,22 @@ spec:
                     - apiGroup
                     - kind
                   type: object
+                synchronization:
+                  description: Synchronization allows to configure how the syncagent processes this resource.
+                  properties:
+                    enabled:
+                      description: |-
+                        Enabled can be used to toggle the synchronization as a whole. When set to
+                        false, the syncagent will only copy the CRD and include it in the APIExport,
+                        but not will attempt to synchronize objects of this resource from the kcp
+                        workspaces to the provider.
+                        Synchronization must be disabled for resources that are used as related
+                        resources for other PublishedResources. Otherwise the syncagent would
+                        potentially loop and never finish processing an object.
+                      type: boolean
+                  required:
+                    - enabled
+                  type: object
               required:
                 - resource
               type: object

hack/update-codegen-sdk.sh

@@ -39,6 +39,10 @@ $GOBIN/controller-gen \
   "object:headerFile=$BOILERPLATE_HEADER" \
   paths=./internal/sync/apis/...
 
+$GOBIN/controller-gen \
+  "object:headerFile=$BOILERPLATE_HEADER" \
+  paths=./test/crds/...
+
 cd sdk
 rm -rf -- applyconfiguration clientset informers listers
 

internal/controller/apiexport/controller.go

@@ -26,13 +26,16 @@ import (
 
 	"github.com/kcp-dev/api-syncagent/internal/controllerutil"
 	predicateutil "github.com/kcp-dev/api-syncagent/internal/controllerutil/predicate"
+	"github.com/kcp-dev/api-syncagent/internal/projection"
 	"github.com/kcp-dev/api-syncagent/internal/resources/reconciling"
+	"github.com/kcp-dev/api-syncagent/internal/validation"
 	syncagentv1alpha1 "github.com/kcp-dev/api-syncagent/sdk/apis/syncagent/v1alpha1"
 
 	kcpdevv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha1"
 
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/client-go/tools/record"
 	"sigs.k8s.io/controller-runtime/pkg/builder"
@@ -109,10 +112,16 @@ func Add(
 
 func (r *Reconciler) Reconcile(ctx context.Context, request reconcile.Request) (reconcile.Result, error) {
 	r.log.Debug("Processing")
-	return reconcile.Result{}, r.reconcile(ctx)
+
+	apiExport := &kcpdevv1alpha1.APIExport{}
+	if err := r.kcpClient.Get(ctx, types.NamespacedName{Name: r.apiExportName}, apiExport); err != nil {
+		return reconcile.Result{}, ctrlruntimeclient.IgnoreNotFound(err)
+	}
+
+	return reconcile.Result{}, r.reconcile(ctx, apiExport)
 }
 
-func (r *Reconciler) reconcile(ctx context.Context) error {
+func (r *Reconciler) reconcile(ctx context.Context, apiExport *kcpdevv1alpha1.APIExport) error {
 	// find all PublishedResources
 	pubResources := &syncagentv1alpha1.PublishedResourceList{}
 	if err := r.localClient.List(ctx, pubResources, &ctrlruntimeclient.ListOptions{
@@ -121,75 +130,99 @@ func (r *Reconciler) reconcile(ctx context.Context) error {
 		return fmt.Errorf("failed to list PublishedResources: %w", err)
 	}
 
-	// filter out those PRs that have not yet been processed into an ARS
+	// filter out those PRs that are invalid; we keep those that are not yet converted into ARS,
+	// just to reduce the amount of re-reconciles when the agent processes a number of PRs in a row
+	// and would constantly update the APIExport; instead we rely on kcp to handle the eventual
+	// consistency.
+	// This allows us to already determine all resources we "own", which helps us in
+	// bilding the proper permission claims if one of the related resources is using a resource
+	// type managed via PublishedResource. Otherwise the controller might not see the PR for the
+	// related resource and temporarily incorrectly assume it needs to add a permission claim.
 	filteredPubResources := slices.DeleteFunc(pubResources.Items, func(pr syncagentv1alpha1.PublishedResource) bool {
-		return pr.Status.ResourceSchemaName == ""
+		// TODO: Turn this into a webhook or CEL expressions.
+		err := validation.ValidatePublishedResource(&pr)
+		if err != nil {
+			r.log.With("pr", pr.Name, "error", err).Warn("Ignoring invalid PublishedResource.")
+		}
+
+		return err != nil
 	})
 
 	// for each PR, we note down the created ARS and also the GVKs of related resources
-	arsList := sets.New[string]()
-	claimedResources := sets.New[kcpdevv1alpha1.GroupResource]()
+	newARSList := sets.New[string]()
+	for _, pubResource := range filteredPubResources {
+		newARSList.Insert(pubResource.Status.ResourceSchemaName)
+	}
 
-	// PublishedResources use kinds, but the PermissionClaims use resource names (plural),
-	// so we must translate accordingly
-	mapper := r.kcpClient.RESTMapper()
+	// To determine if the GVR of a related resource needs to be listed as a permission claim,
+	// we first need to figure out all the GVRs our APIExport contains. This is not just the
+	// list of ARS we just built, but also potentially other ARS's that exist in the APIExport
+	// and that we would not touch.
+	allARSList := mergeResourceSchemas(apiExport.Spec.LatestResourceSchemas, newARSList)
+
+	// turn the flat list of schema names ("version.resource.group") into a lookup table consisting
+	// of group/resource only
+	ourOwnResources := sets.New[schema.GroupResource]()
+	for _, schemaName := range allARSList {
+		gvr, err := parseSchemaName(schemaName)
+		if err != nil {
+			return fmt.Errorf("failed to assemble own resources: %w", err)
+		}
 
-	for _, pubResource := range filteredPubResources {
-		arsList.Insert(pubResource.Status.ResourceSchemaName)
+		ourOwnResources.Insert(gvr.GroupResource())
+	}
+
+	// Now we can finally assemble the list of required permission claims.
+	claimedResources := sets.New[permissionClaim]()
 
+	for _, pubResource := range filteredPubResources {
 		// to evaluate the namespace filter, the agent needs to fetch the namespace
 		if filter := pubResource.Spec.Filter; filter != nil && filter.Namespace != nil {
-			claimedResources.Insert(kcpdevv1alpha1.GroupResource{
+			claimedResources.Insert(permissionClaim{
 				Group:    "",
 				Resource: "namespaces",
 			})
 		}
 
 		if pubResource.Spec.EnableWorkspacePaths {
-			claimedResources.Insert(kcpdevv1alpha1.GroupResource{
+			claimedResources.Insert(permissionClaim{
 				Group:    "core.kcp.io",
 				Resource: "logicalclusters",
 			})
 		}
 
+		// Add a claim for every foreign (!) related resource, but make sure to
+		// project the GVR of related resources to their kcp-side equivalent first
+		// if they originate on the service cluster.
 		for _, rr := range pubResource.Spec.Related {
-			resource, err := mapper.ResourceFor(schema.GroupVersionResource{
-				Resource: rr.Kind,
-			})
-			if err != nil {
-				return fmt.Errorf("unknown related resource kind %q: %w", rr.Kind, err)
-			}
+			kcpGVR := projection.RelatedResourceKcpGVR(&rr)
 
-			claimedResources.Insert(kcpdevv1alpha1.GroupResource{
+			// We always want to see namespaces, for simplicity. Technically if we knew the scope
+			// of every related resource, we could determine if a namespace claim is truly necessary.
+			claimedResources.Insert(permissionClaim{
 				Group:    "",
-				Resource: resource.Resource,
+				Resource: "namespaces",
 			})
-		}
-	}
 
-	// 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(kcpdevv1alpha1.GroupResource{
-			Group:    "",
-			Resource: "namespaces",
-		})
+			if !ourOwnResources.Has(kcpGVR.GroupResource()) {
+				claimedResources.Insert(permissionClaim{
+					Group:        kcpGVR.Group,
+					Resource:     kcpGVR.Resource,
+					IdentityHash: rr.IdentityHash,
+				})
+			}
+		}
 	}
 
 	// We always want to create events.
-	claimedResources.Insert(kcpdevv1alpha1.GroupResource{
+	claimedResources.Insert(permissionClaim{
 		Group:    "",
 		Resource: "events",
 	})
 
-	if arsList.Len() == 0 {
-		r.log.Debug("No ready PublishedResources available.")
-		return nil
-	}
-
 	// reconcile an APIExport in kcp
 	factories := []reconciling.NamedAPIExportReconcilerFactory{
-		r.createAPIExportReconciler(arsList, claimedResources, r.agentName, r.apiExportName, r.recorder),
+		r.createAPIExportReconciler(newARSList, claimedResources, r.agentName, r.apiExportName, r.recorder),
 	}
 
 	if err := reconciling.ReconcileAPIExports(ctx, factories, "", r.kcpClient); err != nil {