Merge branch 'nephio-project:main' into creation-timestamp-fix

pkg/cache/memory/repository.go

@@ -54,7 +54,7 @@ type cachedRepository struct {
 
 	lastVersion string
 
-	mutex                  sync.Mutex
+	mutex                  sync.RWMutex
 	cachedPackageRevisions map[repository.PackageRevisionKey]*cachedPackageRevision
 	cachedPackages         map[repository.PackageKey]*cachedPackage
 	// Error encountered on repository refresh by the refresh goroutine.
@@ -108,8 +108,8 @@ func (r *cachedRepository) ListPackageRevisions(ctx context.Context, filter repo
 }
 
 func (r *cachedRepository) getRefreshError() error {
-	r.mutex.Lock()
-	defer r.mutex.Unlock()
+	r.mutex.RLock()
+	defer r.mutex.RUnlock()
 
 	// TODO: This should also check r.refreshPkgsError when
 	//   the package resource is fully supported.
@@ -123,8 +123,8 @@ func (r *cachedRepository) getPackageRevisions(ctx context.Context, filter repos
 	if err != nil {
 		return nil, err
 	}
-	r.mutex.Lock()
-	defer r.mutex.Unlock()
+	r.mutex.RLock()
+	defer r.mutex.RUnlock()
 	return toPackageRevisionSlice(ctx, packageRevisions, filter), nil
 }
 
@@ -134,16 +134,16 @@ func (r *cachedRepository) getPackages(ctx context.Context, filter repository.Li
 	if err != nil {
 		return nil, err
 	}
-	r.mutex.Lock()
-	defer r.mutex.Unlock()
+	r.mutex.RLock()
+	defer r.mutex.RUnlock()
 	return toPackageSlice(packages, filter), nil
 }
 
 // getCachedPackages returns cachedPackages; fetching it if not cached or if forceRefresh.
 // mutex must be held.
 func (r *cachedRepository) getCachedPackages(ctx context.Context, forceRefresh bool) (map[repository.PackageKey]*cachedPackage, map[repository.PackageRevisionKey]*cachedPackageRevision, error) {
 	// must hold mutex
-
+	
 	r.mutex.Lock()
 	packages := r.cachedPackages
 	packageRevisions := r.cachedPackageRevisions
@@ -157,6 +157,7 @@ func (r *cachedRepository) getCachedPackages(ctx context.Context, forceRefresh b
 			// TODO: Figure out a way to do this without the cache layer
 			//  needing to know what type of repo we are working with.
 			if err := gitRepo.UpdateDeletionProposedCache(); err != nil {
+				r.mutex.Unlock()
 				return nil, nil, err
 			}
 		}

pkg/engine/engine.go

@@ -198,6 +198,10 @@ func (cad *cadEngine) CreatePackageRevision(ctx context.Context, repositoryObj *
 	}
 	pkgRevMeta, err = cad.metadataStore.Create(ctx, pkgRevMeta, repositoryObj.Name, repoPkgRev.UID())
 	if err != nil {
+		if (apierrors.IsUnauthorized(err) || apierrors.IsForbidden(err)) && repository.AnyBlockOwnerDeletionSet(obj) {
+			return nil, fmt.Errorf("failed to create internal PackageRev object, because blockOwnerDeletion is enabled for some ownerReference "+
+				"(it is likely that the serviceaccount of porch-server does not have the rights to update finalizers in the owner object): %w", err)
+		}
 		return nil, err
 	}
 	repoPkgRev.SetMeta(pkgRevMeta)
@@ -318,6 +322,10 @@ func (cad *cadEngine) UpdatePackageRevision(ctx context.Context, version string,
 
 	err = cad.updatePkgRevMeta(ctx, repoPkgRev, newObj)
 	if err != nil {
+		if (apierrors.IsUnauthorized(err) || apierrors.IsForbidden(err)) && repository.AnyBlockOwnerDeletionSet(newObj) {
+			return nil, fmt.Errorf("failed to update internal PackageRev object, because blockOwnerDeletion is enabled for some ownerReference "+
+				"(it is likely that the serviceaccount of porch-server does not have the rights to update finalizers in the owner object): %w", err)
+		}
 		return nil, err
 	}
 

pkg/repository/util.go

@@ -26,6 +26,7 @@ import (
 	"go.opentelemetry.io/otel"
 	"go.opentelemetry.io/otel/trace"
 	"golang.org/x/mod/semver"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 var tracer = otel.Tracer("repository/util")
@@ -136,3 +137,14 @@ func ValidateWorkspaceName(workspace api.WorkspaceName) error {
 
 	return nil
 }
+
+// AnyBlockOwnerDeletionSet checks whether there are any ownerReferences in the Object
+// which have blockOwnerDeletion enabled (meaning either nil or true).
+func AnyBlockOwnerDeletionSet(obj client.Object) bool {
+	for _, owner := range obj.GetOwnerReferences() {
+		if owner.BlockOwnerDeletion == nil || *owner.BlockOwnerDeletion {
+			return true
+		}
+	}
+	return false
+}