add additional info to error when blockOwnerDeletion is not false,

introduce many new fake types for future mocking

pkg/apiserver/apiserver.go

@@ -29,6 +29,7 @@ import (
 	"github.com/nephio-project/porch/pkg/cache"
 	memorycache "github.com/nephio-project/porch/pkg/cache/memory"
 	"github.com/nephio-project/porch/pkg/engine"
+	engineutils "github.com/nephio-project/porch/pkg/engine/utils"
 	"github.com/nephio-project/porch/pkg/meta"
 	"github.com/nephio-project/porch/pkg/registry/porch"
 	"google.golang.org/api/option"
@@ -226,7 +227,7 @@ func (c completedConfig) New() (*PorchServer, error) {
 	referenceResolver := porch.NewReferenceResolver(coreClient)
 	userInfoProvider := &porch.ApiserverUserInfoProvider{}
 
-	watcherMgr := engine.NewWatcherManager()
+	watcherMgr := engineutils.NewWatcherManager()
 
 	memoryCache := memorycache.NewCache(c.ExtraConfig.CacheDirectory, c.ExtraConfig.RepoSyncFrequency, c.ExtraConfig.UseUserDefinedCaBundle, memorycache.CacheOptions{
 		CredentialResolver: credentialResolver,

pkg/cache/fake/cache.go

@@ -0,0 +1,22 @@
+package fake
+
+import (
+	"context"
+
+	configapi "github.com/nephio-project/porch/api/porchconfig/v1alpha1"
+	"github.com/nephio-project/porch/pkg/cache"
+	"github.com/nephio-project/porch/pkg/repository"
+	"github.com/nephio-project/porch/pkg/repository/fake"
+)
+
+type FakeCache struct{}
+
+var _ cache.Cache = &FakeCache{}
+
+func (m *FakeCache) OpenRepository(context.Context, *configapi.Repository) (repository.Repository, error) {
+	return &fake.FakeRepository{}, nil
+}
+
+func (m *FakeCache) CloseRepository(context.Context, *configapi.Repository, []configapi.Repository) error {
+	return nil
+}

pkg/engine/engine.go

@@ -23,7 +23,8 @@ import (
 
 	api "github.com/nephio-project/porch/api/porch/v1alpha1"
 	configapi "github.com/nephio-project/porch/api/porchconfig/v1alpha1"
-	cache "github.com/nephio-project/porch/pkg/cache"
+	"github.com/nephio-project/porch/pkg/cache"
+	"github.com/nephio-project/porch/pkg/engine/utils"
 	"github.com/nephio-project/porch/pkg/meta"
 	"github.com/nephio-project/porch/pkg/repository"
 	"github.com/nephio-project/porch/pkg/task"
@@ -44,7 +45,7 @@ const (
 
 type CaDEngine interface {
 	// ObjectCache() is a cache of all our objects.
-	ObjectCache() WatcherManager
+	ObjectCache() utils.WatcherManager
 
 	UpdatePackageResources(ctx context.Context, repositoryObj *configapi.Repository, oldPackage repository.PackageRevision, old, new *api.PackageRevisionResources) (repository.PackageRevision, *api.RenderStatus, error)
 
@@ -77,14 +78,14 @@ type cadEngine struct {
 
 	userInfoProvider repository.UserInfoProvider
 	metadataStore    meta.MetadataStore
-	watcherManager   *watcherManager
+	watcherManager   utils.WatcherManager
 	taskHandler      task.TaskHandler
 }
 
 var _ CaDEngine = &cadEngine{}
 
 // ObjectCache is a cache of all our objects.
-func (cad *cadEngine) ObjectCache() WatcherManager {
+func (cad *cadEngine) ObjectCache() utils.WatcherManager {
 	return cad.watcherManager
 }
 
@@ -198,6 +199,9 @@ 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)) && anyBlockOwnerDeletionSet(obj) {
+			return nil, fmt.Errorf("failed to create PackageRev because blockOwnerDeletion is enabled for some ownerReference: %w", err)
+		}
 		return nil, err
 	}
 	repoPkgRev.SetMeta(pkgRevMeta)
@@ -206,6 +210,15 @@ func (cad *cadEngine) CreatePackageRevision(ctx context.Context, repositoryObj *
 	return repoPkgRev, nil
 }
 
+func anyBlockOwnerDeletionSet(pr *api.PackageRevision) bool {
+	for _, owner := range pr.OwnerReferences {
+		if owner.BlockOwnerDeletion == nil || *owner.BlockOwnerDeletion {
+			return true
+		}
+	}
+	return false
+}
+
 // The workspaceName must be unique, because it used to generate the package revision's metadata.name.
 func ensureUniqueWorkspaceName(obj *api.PackageRevision, existingRevs []repository.PackageRevision) error {
 	for _, r := range existingRevs {
@@ -273,6 +286,9 @@ 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)) && anyBlockOwnerDeletionSet(newObj) {
+				return nil, fmt.Errorf("failed to update PackageRev because blockOwnerDeletion is set for some ownerReference: %w", err)
+			}
 			return nil, err
 		}
 		sent := cad.watcherManager.NotifyPackageRevisionChange(watch.Modified, repoPkgRev)

pkg/engine/options.go

@@ -19,6 +19,7 @@ import (
 
 	"github.com/nephio-project/porch/internal/kpt/fnruntime"
 	"github.com/nephio-project/porch/pkg/cache"
+	engineutils "github.com/nephio-project/porch/pkg/engine/utils"
 	"github.com/nephio-project/porch/pkg/kpt"
 	"github.com/nephio-project/porch/pkg/kpt/fn"
 	"github.com/nephio-project/porch/pkg/meta"
@@ -129,7 +130,7 @@ func WithMetadataStore(metadataStore meta.MetadataStore) EngineOption {
 	})
 }
 
-func WithWatcherManager(watcherManager *watcherManager) EngineOption {
+func WithWatcherManager(watcherManager engineutils.WatcherManager) EngineOption {
 	return EngineOptionFunc(func(engine *cadEngine) error {
 		engine.watcherManager = watcherManager
 		return nil

pkg/engine/utils/fake/watchermanager.go

@@ -0,0 +1,21 @@
+package fake
+
+import (
+	"context"
+
+	"github.com/nephio-project/porch/pkg/engine/utils"
+	"github.com/nephio-project/porch/pkg/repository"
+	"k8s.io/apimachinery/pkg/watch"
+)
+
+type FakeWatcherManager struct{}
+
+var _ utils.WatcherManager = &FakeWatcherManager{}
+
+func (m *FakeWatcherManager) WatchPackageRevisions(context.Context, repository.ListPackageRevisionFilter, utils.ObjectWatcher) error {
+	return nil
+}
+
+func (m *FakeWatcherManager) NotifyPackageRevisionChange(watch.EventType, repository.PackageRevision) int {
+	return 0
+}

pkg/engine/safejoin.go → pkg/engine/utils/safejoin.go

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package engine
+package utils
 
 import (
 	"fmt"
@@ -22,7 +22,7 @@ import (
 
 // Relevant: https://github.com/golang/go/issues/20126
 
-func filepathSafeJoin(dir string, relative string) (string, error) {
+func FilepathSafeJoin(dir string, relative string) (string, error) {
 	p := filepath.Join(dir, relative)
 	p = filepath.Clean(p)
 

pkg/engine/safejoin_test.go → pkg/engine/utils/safejoin_test.go

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package engine
+package utils
 
 import (
 	"fmt"
@@ -82,7 +82,7 @@ func TestSafeJoin(t *testing.T) {
 
 	for _, g := range grid {
 		t.Run(fmt.Sprintf("%#v", g), func(t *testing.T) {
-			got, err := filepathSafeJoin(g.base, g.relative)
+			got, err := FilepathSafeJoin(g.base, g.relative)
 			if g.wantError {
 				if err == nil {
 					t.Errorf("got %q and nil error, want error", got)

pkg/engine/watchermanager.go → pkg/engine/utils/watchermanager.go

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package engine
+package utils
 
 import (
 	"context"
@@ -23,17 +23,18 @@ import (
 	"k8s.io/klog/v2"
 )
 
-// ObjectCache caches objects across repositories, and allows for watching.
+// WatcherManager caches objects across repositories, and allows for watching.
 type WatcherManager interface {
 	WatchPackageRevisions(ctx context.Context, filter repository.ListPackageRevisionFilter, callback ObjectWatcher) error
+	NotifyPackageRevisionChange(eventType watch.EventType, obj repository.PackageRevision) int
 }
 
-// PackageRevisionWatcher is the callback interface for watchers.
+// ObjectWatcher is the callback interface for watchers.
 type ObjectWatcher interface {
 	OnPackageRevisionChange(eventType watch.EventType, obj repository.PackageRevision) bool
 }
 
-func NewWatcherManager() *watcherManager {
+func NewWatcherManager() WatcherManager {
 	return &watcherManager{}
 }
 
@@ -59,7 +60,7 @@ type watcher struct {
 	filter repository.ListPackageRevisionFilter
 }
 
-// WatchPackageRevision adds a change-listener that will be called for all changes.
+// WatchPackageRevisions adds a change-listener that will be called for all changes.
 func (r *watcherManager) WatchPackageRevisions(ctx context.Context, filter repository.ListPackageRevisionFilter, callback ObjectWatcher) error {
 	r.mutex.Lock()
 	defer r.mutex.Unlock()
@@ -93,7 +94,7 @@ func (r *watcherManager) WatchPackageRevisions(ctx context.Context, filter repos
 	return nil
 }
 
-// notifyPackageRevisionChange is called to send a change notification to all interested listeners.
+// NotifyPackageRevisionChange is called to send a change notification to all interested listeners.
 func (r *watcherManager) NotifyPackageRevisionChange(eventType watch.EventType, obj repository.PackageRevision) int {
 	r.mutex.Lock()
 	defer r.mutex.Unlock()

pkg/meta/fake/noopstore.go

@@ -0,0 +1,36 @@
+package fake
+
+import (
+	"context"
+
+	configapi "github.com/nephio-project/porch/api/porchconfig/v1alpha1"
+	"github.com/nephio-project/porch/pkg/meta"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
+)
+
+type NoopMetadataStore struct {
+	Metas []metav1.ObjectMeta
+}
+
+var _ meta.MetadataStore = &NoopMetadataStore{}
+
+func (ms *NoopMetadataStore) Get(ctx context.Context, namespacedName types.NamespacedName) (metav1.ObjectMeta, error) {
+	return metav1.ObjectMeta{Name: namespacedName.Name, Namespace: namespacedName.Namespace}, nil
+}
+
+func (ms *NoopMetadataStore) List(context.Context, *configapi.Repository) ([]metav1.ObjectMeta, error) {
+	return ms.Metas, nil
+}
+
+func (ms *NoopMetadataStore) Create(ctx context.Context, pkgRevMeta metav1.ObjectMeta, repoName string, pkgRevUID types.UID) (metav1.ObjectMeta, error) {
+	return pkgRevMeta, nil
+}
+
+func (ms *NoopMetadataStore) Update(ctx context.Context, pkgRevMeta metav1.ObjectMeta) (metav1.ObjectMeta, error) {
+	return pkgRevMeta, nil
+}
+
+func (ms *NoopMetadataStore) Delete(ctx context.Context, namespacedName types.NamespacedName, clearFinalizer bool) (metav1.ObjectMeta, error) {
+	return metav1.ObjectMeta{Name: namespacedName.Name, Namespace: namespacedName.Namespace}, nil
+}

pkg/meta/store.go

@@ -54,7 +54,7 @@ type MetadataStore interface {
 
 var _ MetadataStore = &crdMetadataStore{}
 
-func NewCrdMetadataStore(coreClient client.Client) *crdMetadataStore {
+func NewCrdMetadataStore(coreClient client.Client) MetadataStore {
 	return &crdMetadataStore{
 		coreClient: coreClient,
 	}

pkg/registry/porch/packagecommon.go

@@ -23,6 +23,7 @@ import (
 	api "github.com/nephio-project/porch/api/porch/v1alpha1"
 	configapi "github.com/nephio-project/porch/api/porchconfig/v1alpha1"
 	"github.com/nephio-project/porch/pkg/engine"
+	engineutils "github.com/nephio-project/porch/pkg/engine/utils"
 	"github.com/nephio-project/porch/pkg/repository"
 	"github.com/nephio-project/porch/pkg/util"
 	"go.opentelemetry.io/otel/trace"
@@ -144,7 +145,7 @@ func (r *packageCommon) listPackages(ctx context.Context, filter packageFilter,
 	return nil
 }
 
-func (r *packageCommon) watchPackages(ctx context.Context, filter packageRevisionFilter, callback engine.ObjectWatcher) error {
+func (r *packageCommon) watchPackages(ctx context.Context, filter packageRevisionFilter, callback engineutils.ObjectWatcher) error {
 	if err := r.cad.ObjectCache().WatchPackageRevisions(ctx, filter.ListPackageRevisionFilter, callback); err != nil {
 		return err
 	}

pkg/registry/porch/watch.go

@@ -19,7 +19,7 @@ import (
 	"fmt"
 	"sync"
 
-	"github.com/nephio-project/porch/pkg/engine"
+	engineutils "github.com/nephio-project/porch/pkg/engine/utils"
 	"github.com/nephio-project/porch/pkg/repository"
 	"go.opentelemetry.io/otel/trace"
 	metainternalversion "k8s.io/apimachinery/pkg/apis/meta/internalversion"
@@ -92,7 +92,7 @@ func (w *watcher) ResultChan() <-chan watch.Event {
 }
 
 type packageReader interface {
-	watchPackages(ctx context.Context, filter packageRevisionFilter, callback engine.ObjectWatcher) error
+	watchPackages(ctx context.Context, filter packageRevisionFilter, callback engineutils.ObjectWatcher) error
 	listPackageRevisions(ctx context.Context, filter packageRevisionFilter, selector labels.Selector, callback func(ctx context.Context, p repository.PackageRevision) error) error
 }
 

pkg/registry/porch/watch_test.go

@@ -21,7 +21,7 @@ import (
 	"time"
 
 	"github.com/nephio-project/porch/api/porch/v1alpha1"
-	"github.com/nephio-project/porch/pkg/engine"
+	engineutils "github.com/nephio-project/porch/pkg/engine/utils"
 	"github.com/nephio-project/porch/pkg/repository"
 	"github.com/nephio-project/porch/pkg/repository/fake"
 	metainternalversion "k8s.io/apimachinery/pkg/apis/meta/internalversion"
@@ -88,10 +88,10 @@ func TestWatcherClose(t *testing.T) {
 
 type fakePackageReader struct {
 	sync.WaitGroup
-	callback engine.ObjectWatcher
+	callback engineutils.ObjectWatcher
 }
 
-func (f *fakePackageReader) watchPackages(ctx context.Context, filter packageRevisionFilter, callback engine.ObjectWatcher) error {
+func (f *fakePackageReader) watchPackages(ctx context.Context, filter packageRevisionFilter, callback engineutils.ObjectWatcher) error {
 	f.callback = callback
 	f.Done()
 	return nil

pkg/repository/fake/package.go

@@ -0,0 +1,30 @@
+package fake
+
+import (
+	"github.com/nephio-project/porch/api/porch/v1alpha1"
+	"github.com/nephio-project/porch/pkg/repository"
+)
+
+type FakePackage struct {
+	Name           string
+	PackageKey     repository.PackageKey
+	LatestRevision string
+}
+
+var _ repository.Package = &FakePackage{}
+
+func (pkg *FakePackage) KubeObjectName() string {
+	return pkg.Name
+}
+
+func (pkg *FakePackage) Key() repository.PackageKey {
+	return pkg.PackageKey
+}
+
+func (pkg *FakePackage) GetPackage() *v1alpha1.PorchPackage {
+	panic("not implemented")
+}
+
+func (pkg *FakePackage) GetLatestRevision() string {
+	return pkg.LatestRevision
+}