Refactor PackageRevision controller code for introduction of subpackage clone and upgrade

controllers/packagerevisions/pkg/controllers/packagerevision/clone.go

@@ -0,0 +1,121 @@
+// Copyright 2026 The kpt Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package packagerevision
+
+import (
+	"context"
+	"fmt"
+	"path"
+
+	kptfilev1 "github.com/kptdev/kpt/api/kptfile/v1"
+	"github.com/kptdev/kpt/pkg/lib/kptops"
+	porchv1alpha2 "github.com/kptdev/porch/api/porch/v1alpha2"
+	"github.com/kptdev/porch/pkg/repository"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/log"
+)
+
+// clonePackage reads the source package referenced by CloneFrom and returns its resources
+// with Kptfile upstream/upstreamLock updated.
+// Currently only supports upstreamRef (registered repo). Raw git URL is not yet implemented.
+func (r *PackageRevisionReconciler) clonePackage(ctx context.Context, pr *porchv1alpha2.PackageRevision) (map[string]string, error) {
+	cloneFrom := r.getCloneFrom(pr)
+
+	if cloneFrom.UpstreamRef != nil {
+		return r.cloneFromUpstreamRef(ctx, pr, cloneFrom.UpstreamRef)
+	}
+	if cloneFrom.Git != nil {
+		return r.cloneFromGit(ctx, pr, cloneFrom.Git)
+	}
+	return nil, fmt.Errorf("clone source must specify either upstreamRef or git")
+}
+
+func (r *PackageRevisionReconciler) cloneFromUpstreamRef(ctx context.Context, pr *porchv1alpha2.PackageRevision, ref *porchv1alpha2.PackageRevisionRef) (map[string]string, error) {
+	log := log.FromContext(ctx)
+	var sourcePR porchv1alpha2.PackageRevision
+	if err := r.Get(ctx, client.ObjectKey{Namespace: pr.Namespace, Name: ref.Name}, &sourcePR); err != nil {
+		return nil, fmt.Errorf("failed to get upstream package %q: %w", ref.Name, err)
+	}
+
+	if !porchv1alpha2.LifecycleIsPublished(sourcePR.Spec.Lifecycle) {
+		return nil, fmt.Errorf("upstream package %q must be published", ref.Name)
+	}
+
+	log.V(1).Info("cloning from upstream ref", "upstream", ref.Name)
+
+	repoKey := repository.RepositoryKey{Namespace: pr.Namespace, Name: sourcePR.Spec.RepositoryName}
+	content, err := r.ContentCache.GetPackageContent(ctx, repoKey, sourcePR.Spec.PackageName, sourcePR.Spec.WorkspaceName)
+	if err != nil {
+		return nil, fmt.Errorf("failed to get upstream package content: %w", err)
+	}
+
+	resources, err := content.GetResourceContents(ctx)
+	if err != nil {
+		return nil, fmt.Errorf("failed to read upstream resources: %w", err)
+	}
+
+	upstream, lock, err := content.GetLock(ctx)
+	if err != nil {
+		return nil, fmt.Errorf("failed to get upstream lock for %q: %w", ref.Name, err)
+	}
+
+	if err := kptops.UpdateKptfileUpstream(r.getClonePackagename(pr), resources, upstream, lock); err != nil {
+		return nil, fmt.Errorf("failed to update Kptfile upstream: %w", err)
+	}
+
+	return resources, nil
+}
+
+func (r *PackageRevisionReconciler) cloneFromGit(ctx context.Context, pr *porchv1alpha2.PackageRevision, gitSpec *porchv1alpha2.GitPackage) (map[string]string, error) {
+	log.FromContext(ctx).V(1).Info("cloning from git", "repo", gitSpec.Repo, "ref", gitSpec.Ref, "directory", gitSpec.Directory)
+	resources, lock, err := r.ExternalPackageFetcher.FetchExternalGitPackage(ctx, gitSpec, pr.Namespace)
+	if err != nil {
+		return nil, fmt.Errorf("failed to fetch from git: %w", err)
+	}
+
+	if err := kptops.UpdateKptfileUpstream(r.getClonePackagename(pr), resources, kptfilev1.Upstream{
+		Type: kptfilev1.GitOrigin,
+		Git: &kptfilev1.Git{
+			Repo:      lock.Repo,
+			Directory: lock.Directory,
+			Ref:       lock.Ref,
+		},
+	}, kptfilev1.Locator{
+		Type: kptfilev1.GitOrigin,
+		Git:  &lock,
+	}); err != nil {
+		return nil, fmt.Errorf("failed to update Kptfile upstream: %w", err)
+	}
+
+	return resources, nil
+}
+
+// getCloneFrom returns the upstream package for a clone in the case of a source clone or a subpackage
+// operation clone
+func (r *PackageRevisionReconciler) getCloneFrom(pr *porchv1alpha2.PackageRevision) *porchv1alpha2.UpstreamPackage {
+	if pr.Status.CreationSource != "" && pr.Spec.SubpackageOperation != nil && pr.Spec.SubpackageOperation.CloneFrom != nil {
+		return pr.Spec.SubpackageOperation.CloneFrom
+	}
+	return pr.Spec.Source.CloneFrom
+}
+
+// getClonePackagename returns the package name of a clone in the case of a source clone or a subpackage
+// operation clone
+func (r *PackageRevisionReconciler) getClonePackagename(pr *porchv1alpha2.PackageRevision) string {
+	if pr.Status.CreationSource != "" && pr.Spec.SubpackageOperation != nil && pr.Spec.SubpackageOperation.CloneFrom != nil {
+		return path.Base(pr.Spec.SubpackageOperation.SubpackageDir)
+	}
+	return pr.Spec.PackageName
+}

controllers/packagerevisions/pkg/controllers/packagerevision/clone_test.go

@@ -0,0 +1,120 @@
+package packagerevision
+
+import (
+	"testing"
+
+	porchv1alpha2 "github.com/kptdev/porch/api/porch/v1alpha2"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestGetCloneFromSourceClone(t *testing.T) {
+	r := &PackageRevisionReconciler{}
+	pr := &porchv1alpha2.PackageRevision{
+		Spec: porchv1alpha2.PackageRevisionSpec{
+			Source: &porchv1alpha2.PackageSource{
+				CloneFrom: &porchv1alpha2.UpstreamPackage{
+					UpstreamRef: &porchv1alpha2.PackageRevisionRef{Name: "source.pkg.v1"},
+				},
+			},
+		},
+	}
+
+	result := r.getCloneFrom(pr)
+	assert.Equal(t, "source.pkg.v1", result.UpstreamRef.Name)
+}
+
+func TestGetCloneFromSubpackageClone(t *testing.T) {
+	r := &PackageRevisionReconciler{}
+	pr := &porchv1alpha2.PackageRevision{
+		Spec: porchv1alpha2.PackageRevisionSpec{
+			Source: &porchv1alpha2.PackageSource{
+				CloneFrom: &porchv1alpha2.UpstreamPackage{
+					UpstreamRef: &porchv1alpha2.PackageRevisionRef{Name: "source.pkg.v1"},
+				},
+			},
+			SubpackageOperation: &porchv1alpha2.SubpackageOperation{
+				SubpackageDir: "my-subpkg",
+				CloneFrom: &porchv1alpha2.UpstreamPackage{
+					UpstreamRef: &porchv1alpha2.PackageRevisionRef{Name: "subpkg-upstream.pkg.v1"},
+				},
+			},
+		},
+		Status: porchv1alpha2.PackageRevisionStatus{
+			CreationSource: "init",
+		},
+	}
+
+	result := r.getCloneFrom(pr)
+	assert.Equal(t, "subpkg-upstream.pkg.v1", result.UpstreamRef.Name)
+}
+
+func TestGetCloneFromSubpackageCloneWithoutCreationSource(t *testing.T) {
+	// Without CreationSource set, getCloneFrom should fall back to Source.CloneFrom
+	r := &PackageRevisionReconciler{}
+	pr := &porchv1alpha2.PackageRevision{
+		Spec: porchv1alpha2.PackageRevisionSpec{
+			Source: &porchv1alpha2.PackageSource{
+				CloneFrom: &porchv1alpha2.UpstreamPackage{
+					UpstreamRef: &porchv1alpha2.PackageRevisionRef{Name: "source.pkg.v1"},
+				},
+			},
+			SubpackageOperation: &porchv1alpha2.SubpackageOperation{
+				SubpackageDir: "my-subpkg",
+				CloneFrom: &porchv1alpha2.UpstreamPackage{
+					UpstreamRef: &porchv1alpha2.PackageRevisionRef{Name: "subpkg-upstream.pkg.v1"},
+				},
+			},
+		},
+	}
+
+	result := r.getCloneFrom(pr)
+	assert.Equal(t, "source.pkg.v1", result.UpstreamRef.Name)
+}
+
+func TestGetClonePackagenameSource(t *testing.T) {
+	r := &PackageRevisionReconciler{}
+	pr := &porchv1alpha2.PackageRevision{
+		Spec: porchv1alpha2.PackageRevisionSpec{
+			PackageName: "my-pkg",
+		},
+	}
+
+	assert.Equal(t, "my-pkg", r.getClonePackagename(pr))
+}
+
+func TestGetClonePackagenameSubpackage(t *testing.T) {
+	r := &PackageRevisionReconciler{}
+	pr := &porchv1alpha2.PackageRevision{
+		Spec: porchv1alpha2.PackageRevisionSpec{
+			PackageName: "my-pkg",
+			SubpackageOperation: &porchv1alpha2.SubpackageOperation{
+				SubpackageDir: "level1/level2/my-subpkg",
+				CloneFrom: &porchv1alpha2.UpstreamPackage{
+					UpstreamRef: &porchv1alpha2.PackageRevisionRef{Name: "upstream.pkg.v1"},
+				},
+			},
+		},
+		Status: porchv1alpha2.PackageRevisionStatus{
+			CreationSource: "init",
+		},
+	}
+
+	assert.Equal(t, "my-subpkg", r.getClonePackagename(pr))
+}
+
+func TestGetClonePackagenameSubpackageWithoutCreationSource(t *testing.T) {
+	r := &PackageRevisionReconciler{}
+	pr := &porchv1alpha2.PackageRevision{
+		Spec: porchv1alpha2.PackageRevisionSpec{
+			PackageName: "my-pkg",
+			SubpackageOperation: &porchv1alpha2.SubpackageOperation{
+				SubpackageDir: "level1/my-subpkg",
+				CloneFrom: &porchv1alpha2.UpstreamPackage{
+					UpstreamRef: &porchv1alpha2.PackageRevisionRef{Name: "upstream.pkg.v1"},
+				},
+			},
+		},
+	}
+
+	assert.Equal(t, "my-pkg", r.getClonePackagename(pr))
+}

controllers/packagerevisions/pkg/controllers/packagerevision/packagerevision_controller.go

@@ -190,7 +190,7 @@ func resultOrDefault(result *ctrl.Result) ctrl.Result {
 // Returns (result, nil) if source was applied and status was updated.
 // Returns (nil, err) on failure.
 func (r *PackageRevisionReconciler) reconcileSource(ctx context.Context, pr *porchv1alpha2.PackageRevision, repoKey repository.RepositoryKey) (*ctrl.Result, error) {
-	resources, creationSource, err := r.applySource(ctx, pr)
+	resources, sourceOperationType, err := r.applySource(ctx, pr)
 	if err != nil {
 		return nil, r.setSourceFailed(ctx, pr, err)
 	}
@@ -199,15 +199,29 @@ func (r *PackageRevisionReconciler) reconcileSource(ctx context.Context, pr *por
 	}
 
 	log := log.FromContext(ctx)
-	log.Info("applying source", "type", creationSource, "name", pr.Name)
+	log.Info("applying source", "type", sourceOperationType, "name", pr.Name)
 
 	// TODO: CreateNewDraft always receives lifecycle=Draft — consider removing the lifecycle parameter from the interface.
 	draft, err := r.ContentCache.CreateNewDraft(ctx, repoKey, pr.Spec.PackageName, pr.Spec.WorkspaceName, string(porchv1alpha2.PackageRevisionLifecycleDraft))
 	if err != nil {
 		return nil, r.setSourceFailed(ctx, pr, fmt.Errorf("create draft: %w", err))
 	}
 
-	if err := draft.UpdateResources(ctx, resources, creationSource); err != nil {
+	return r.finalizeDraftAndUpdateStatus(ctx, pr, repoKey, draft, resources, sourceOperationType)
+}
+
+// finalizeDraftAndUpdateStatus completes the draft operation by updating resources,
+// closing the draft, and updating the package revision status.
+func (r *PackageRevisionReconciler) finalizeDraftAndUpdateStatus(
+	ctx context.Context,
+	pr *porchv1alpha2.PackageRevision,
+	repoKey repository.RepositoryKey,
+	draft repository.PackageRevisionDraftSlim,
+	resources map[string]string,
+	operationType string) (*ctrl.Result, error) {
+	log := log.FromContext(ctx)
+
+	if err := draft.UpdateResources(ctx, resources, operationType); err != nil {
 		return nil, r.setSourceFailed(ctx, pr, fmt.Errorf("update resources: %w", err))
 	}
 
@@ -221,13 +235,11 @@ func (r *PackageRevisionReconciler) reconcileSource(ctx context.Context, pr *por
 		log.Error(err, "failed to read back package content after source execution")
 	}
 
-	r.updateStatus(ctx, pr, content, creationSource,
-		readyCondition(pr.Generation, metav1.ConditionFalse, porchv1alpha2.ReasonPending, "awaiting render"),
-	)
+	r.updateStatus(ctx, pr, content, operationType,
+		readyCondition(pr.Generation, metav1.ConditionFalse, porchv1alpha2.ReasonPending, "awaiting render"))
 	// Set Rendered=Unknown via the render field manager.
 	r.updateRenderStatus(ctx, pr, "", "",
-		renderedCondition(pr.Generation, metav1.ConditionUnknown, porchv1alpha2.ReasonPending, "awaiting render"),
-	)
+		renderedCondition(pr.Generation, metav1.ConditionUnknown, porchv1alpha2.ReasonPending, "awaiting render"))
 	r.ensureLatestRevisionLabel(ctx, pr)
 
 	result := ctrl.Result{Requeue: true}