since there is no webhook, validate the PublishedResource at runtime

This isn't great, but the cheapest option at the moment.

On-behalf-of: @SAP [email protected]

Author: xrstf

internal/admission/publishedresource/validation.go

@@ -0,0 +1,61 @@
+package publishedresource
+
+import (
+	"errors"
+	"fmt"
+
+	syncagentv1alpha1 "github.com/kcp-dev/api-syncagent/sdk/apis/syncagent/v1alpha1"
+	"k8s.io/apimachinery/pkg/util/sets"
+)
+
+func ValidatePublishedResource(pr *syncagentv1alpha1.PublishedResource) error {
+	identifiers := sets.New[string]()
+
+	for _, rr := range pr.Spec.Related {
+		if identifiers.Has(rr.Identifier) {
+			return fmt.Errorf("multiple related resources use the identifier %q", rr.Identifier)
+		}
+
+		if err := validateRelatedResource(rr); err != nil {
+			return fmt.Errorf("related resource %q is invalid: %w", rr.Identifier, err)
+		}
+
+		identifiers.Insert(rr.Identifier)
+	}
+
+	return nil
+}
+
+func validateRelatedResource(rr syncagentv1alpha1.RelatedResourceSpec) error {
+	if err := validateRelatedResourceSourceSpec(rr.Source.Name); err != nil {
+		return fmt.Errorf("%s for the name source", err)
+	}
+
+	if rr.Source.Name.Selector != nil && (rr.Naming == nil || rr.Naming.Name == "") {
+		return errors.New("must configure a naming rule for the related object's name if a label selector is used")
+	}
+
+	if ns := rr.Source.Namespace; ns != nil {
+		if err := validateRelatedResourceSourceSpec(*ns); err != nil {
+			return fmt.Errorf("%s for the namespace source", err)
+		}
+
+		if ns.Selector != nil && (rr.Naming == nil || rr.Naming.Namespace == "") {
+			return errors.New("must configure a naming rule for the related object's namespace if a label selector is used")
+		}
+	}
+
+	return nil
+}
+
+func validateRelatedResourceSourceSpec(ss syncagentv1alpha1.RelatedResourceSourceSpec) error {
+	if ss.Selector == nil && ss.Reference == nil {
+		return errors.New("must use either selector or reference")
+	}
+
+	if ss.Selector != nil && ss.Reference != nil {
+		return errors.New("cannot use both selector and reference")
+	}
+
+	return nil
+}

internal/controller/syncmanager/controller.go

@@ -24,6 +24,7 @@ import (
 	"github.com/kcp-dev/logicalcluster/v3"
 	"go.uber.org/zap"
 
+	"github.com/kcp-dev/api-syncagent/internal/admission/publishedresource"
 	"github.com/kcp-dev/api-syncagent/internal/controller/sync"
 	"github.com/kcp-dev/api-syncagent/internal/controller/syncmanager/lifecycle"
 	"github.com/kcp-dev/api-syncagent/internal/controllerutil"
@@ -274,6 +275,16 @@ func (r *Reconciler) ensureSyncControllers(ctx context.Context, log *zap.Sugared
 			continue
 		}
 
+		// If the PR is invalid, do not even start a controller for it. Since the $key contains
+		// the resource version, if the invalidt PR is fixed, a new controller can be spawned.
+		// In the future, validation issues like this should be caught by a validation webhook
+		// or more advanced CEL-based validations on the CRD itself, rather than validating in
+		// a controller.
+		if err := publishedresource.ValidatePublishedResource(&pubRes); err != nil {
+			log.Errorw("Not starting controller for published resource because of errors", zap.Error(err), "pr", pubRes.Name)
+			continue
+		}
+
 		log.Infow("Starting new sync controller…", "key", key)
 
 		// create the sync controller;