From c72409c32e453b8b63f3ac36f62cb319c6d405cb Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Thu, 16 Jan 2025 20:29:53 +0100 Subject: [PATCH] reconciler/apibinding: check conflict via bound APIs in bindings, not exports Signed-off-by: Dr. Stefan Schimanski --- pkg/admission/apibinding/validation.go | 2 +- .../apis/apibinding/conflict_checker.go | 39 ++++++++++++------- 2 files changed, 27 insertions(+), 14 deletions(-) diff --git a/pkg/admission/apibinding/validation.go b/pkg/admission/apibinding/validation.go index f5f59b30e6b..a054e79bb2b 100644 --- a/pkg/admission/apibinding/validation.go +++ b/pkg/admission/apibinding/validation.go @@ -54,7 +54,7 @@ func ValidateAPIBindingUpdate(oldBinding, newBinding *apisv1alpha1.APIBinding) f // ValidateAPIBindingReference validates an APIBinding's BindingReference. func ValidateAPIBindingReference(reference apisv1alpha1.BindingReference, path *field.Path) field.ErrorList { allErrs := field.ErrorList{} - + if reference.Export == nil { allErrs = append(allErrs, field.Required(path.Child("export"), "")) } else if reference.Export.Name == "" { diff --git a/pkg/reconciler/apis/apibinding/conflict_checker.go b/pkg/reconciler/apis/apibinding/conflict_checker.go index 76f9df82e7d..ebe85906c94 100644 --- a/pkg/reconciler/apis/apibinding/conflict_checker.go +++ b/pkg/reconciler/apis/apibinding/conflict_checker.go @@ -23,6 +23,7 @@ import ( "github.com/kcp-dev/logicalcluster/v3" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + kerrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/util/sets" apisv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha1" @@ -71,24 +72,38 @@ func newConflictChecker(clusterName logicalcluster.Name, return nil, err } for _, b := range bindings { + // first record the bound resources directly + boundSchemaUIDs := sets.New[string]() + for _, br := range b.Status.BoundResources { + crd, err := ncc.getCRD(SystemBoundCRDsClusterName, string(br.Schema.UID)) + if err != nil { + return nil, err + } + + boundSchemaUIDs.Insert(br.Schema.UID) + + ncc.crds = append(ncc.crds, crd) + ncc.crdToBinding[crd.Name] = b + } + if b.Spec.Reference.Export == nil { - // this should not happen because of validation. - return nil, fmt.Errorf("APIBinding %s|%s has no cluster reference", logicalcluster.From(b), b.Name) + // should never happen due to validation. + continue } + + // to reduce risk of races, we also record the resources in the + // referenced export. path := logicalcluster.NewPath(b.Spec.Reference.Export.Path) if path.Empty() { path = logicalcluster.From(b).Path() } apiExport, err := ncc.getAPIExportByPath(path, b.Spec.Reference.Export.Name) - if err != nil { + if err != nil && !kerrors.IsNotFound(err) { return nil, err + } else if kerrors.IsNotFound(err) { + // export is gone already. + continue } - - boundSchemaUIDs := sets.New[string]() - for _, boundResource := range b.Status.BoundResources { - boundSchemaUIDs.Insert(boundResource.Schema.UID) - } - for _, schemaName := range apiExport.Spec.LatestResourceSchemas { schema, err := ncc.getAPIResourceSchema(logicalcluster.From(apiExport), schemaName) if err != nil { @@ -100,10 +115,6 @@ func newConflictChecker(clusterName logicalcluster.Name, } crd, err := ncc.getCRD(SystemBoundCRDsClusterName, string(schema.UID)) - if err != nil { - return nil, err - } - ncc.crds = append(ncc.crds, crd) ncc.crdToBinding[crd.Name] = b } @@ -121,6 +132,8 @@ func newConflictChecker(clusterName logicalcluster.Name, return ncc, nil } +// Check checks if the given schema from the given APIBinding conflicts with any +// CRD or any other APIBinding. func (ncc *conflictChecker) Check(binding *apisv1alpha1.APIBinding, s *apisv1alpha1.APIResourceSchema) error { for _, crd := range ncc.crds { if other, found := ncc.crdToBinding[crd.Name]; found && other.Name == binding.Name {