Fix panic when listing packages with missing repository in cache (kptdev#459)

* Add robustness for nil objects to avoid panic

* Add robustness for nil objects to avoid panic

Author: kushnaidu

pkg/cache/dbcache/dbpackagerevision.go

@@ -125,6 +125,10 @@ func (pr *dbPackageRevision) UpdateLifecycle(ctx context.Context, newLifecycle p
 	_, span := tracer.Start(ctx, "dbPackageRevision::UpdateLifecycle", trace.WithAttributes())
 	defer span.End()
 
+	if pr.repo == nil {
+		return fmt.Errorf("cannot update lifecycle for package revision %s: no associated repository", pr.KubeObjectName())
+	}
+
 	if pr.lifecycle == porchapi.PackageRevisionLifecycleProposed && newLifecycle == porchapi.PackageRevisionLifecyclePublished {
 		if err := pr.publishPR(ctx, newLifecycle); err != nil {
 			pr.pkgRevKey.Revision = 0
@@ -142,6 +146,15 @@ func (pr *dbPackageRevision) GetPackageRevision(ctx context.Context) (*porchapi.
 	_, span := tracer.Start(ctx, "dbPackageRevision::GetPackageRevision", trace.WithAttributes())
 	defer span.End()
 
+	if pr == nil {
+		return nil, fmt.Errorf("invalid package revision: nil object")
+	}
+
+	if pr.repo == nil {
+		klog.Warningf("package revision %+v has nil repository, skipping", pr.Key())
+		return nil, fmt.Errorf("package revision %s has no associated repository (may be deleted or not yet cached)", pr.KubeObjectName())
+	}
+
 	readPR, err := pkgRevReadFromDB(ctx, pr.Key(), false)
 	if err != nil {
 		if pr.GetMeta().DeletionTimestamp != nil || strings.Contains(err.Error(), "sql: no rows in result set") {
@@ -152,14 +165,14 @@ func (pr *dbPackageRevision) GetPackageRevision(ctx context.Context) (*porchapi.
 		}
 	}
 
-	_, upstreamLock, _ := pr.GetUpstreamLock(ctx)
-	_, selfLock, _ := pr.GetLock(ctx)
+	_, upstreamLock, _ := readPR.GetUpstreamLock(ctx)
+	_, selfLock, _ := readPR.GetLock(ctx)
 	kf, _ := readPR.GetKptfile(ctx)
 
 	status := porchapi.PackageRevisionStatus{
 		UpstreamLock: repository.KptUpstreamLock2APIUpstreamLock(upstreamLock),
 		SelfLock:     repository.KptUpstreamLock2APIUpstreamLock(selfLock),
-		Deployment:   pr.repo.deployment,
+		Deployment:   readPR.repo.deployment,
 		Conditions:   repository.ToAPIConditions(kf),
 	}
 

pkg/cache/dbcache/dbpackagerevisionsql.go

@@ -305,7 +305,18 @@ func pkgRevScanRowsFromDB(ctx context.Context, rows *sql.Rows) ([]*dbPackageRevi
 			return nil, err
 		}
 
-		pkgRev.repo = cachetypes.CacheInstance.GetRepository(pkgRev.pkgRevKey.PkgKey.RepoKey).(*dbRepository)
+		repo := cachetypes.CacheInstance.GetRepository(pkgRev.pkgRevKey.PkgKey.RepoKey)
+		if repo != nil {
+			if dbRepo, ok := repo.(*dbRepository); ok {
+				pkgRev.repo = dbRepo
+			} else {
+				klog.Warningf("pkgRevScanRowsFromDB: repository %+v is not a dbRepository for package revision %s", pkgRev.pkgRevKey.PkgKey.RepoKey, prK8SName)
+				continue
+			}
+		} else {
+			klog.V(4).Infof("pkgRevScanRowsFromDB: repository %+v not found in cache for package revision %s", pkgRev.pkgRevKey.PkgKey.RepoKey, prK8SName)
+			continue
+		}
 		pkgRev.pkgRevKey.PkgKey.Package = repository.K8SName2PkgName(pkgK8SName)
 		pkgRev.pkgRevKey.WorkspaceName = repository.K8SName2PkgRevWSName(pkgK8SName, prK8SName)
 		setValueFromJSON(metaAsJSON, &pkgRev.meta)

pkg/cache/dbcache/dbrepository.go

@@ -148,6 +148,10 @@ func (r *dbRepository) ListPackageRevisions(ctx context.Context, filter reposito
 
 	genericPkgRevs := make([]repository.PackageRevision, len(foundPkgRevs))
 	for i, pkgRev := range foundPkgRevs {
+		if pkgRev.repo == nil {
+			klog.V(4).Infof("ListPackageRevisions: skipping package revision %+v with nil repository", pkgRev.Key())
+			continue
+		}
 		genericPkgRev := repository.PackageRevision(pkgRev)
 		if filter.MatchesLabels(ctx, genericPkgRev) {
 			genericPkgRevs[i] = genericPkgRev

pkg/registry/porch/packagerevision.go

@@ -88,7 +88,9 @@ func (r *packageRevisions) List(ctx context.Context, options *metainternalversio
 	if err := r.listPackageRevisions(ctx, *filter, func(ctx context.Context, p repository.PackageRevision) error {
 		item, err := p.GetPackageRevision(ctx)
 		if err != nil {
-			return err
+			// Skip package revisions that fail to fetch (stale cache, deleted, etc.)
+			klog.Warningf("Failed to fetch package revision %s during list, skipping: %v", p.KubeObjectName(), err)
+			return nil
 		}
 		result.Items = append(result.Items, *item)
 		return nil

pkg/registry/porch/packagerevision_test.go

@@ -150,6 +150,7 @@ func TestList(t *testing.T) {
 	mockEngine.On("ListPackageRevisions", mock.Anything, mock.Anything, mock.Anything).Return([]repository.PackageRevision{
 		mockPkgRev,
 	}, nil)
+	mockPkgRev.On("KubeObjectName").Return("test-package").Maybe()
 	mockPkgRev.On("GetPackageRevision", mock.Anything).Return(nil, errors.New("error getting API package revision")).Once()
 	result, err = packagerevisions.List(context.TODO(), &internalversion.ListOptions{})
 	assert.NoError(t, err)