Refactor PackageRevision controller code for introduction of subpackage clone and upgrade

[liamfallon]

Refactor PackageRevision controller code for introduction of subpackage clone and upgrade

---

Description

• What changed: The PacakgeRevision controller code is refactored to make it easier to implement subpackage operations. The clone and upgrade sub-operations are separated from the source handing into their own separate files. A util file is added to hold utility functions that are independent of source and sobpackage operations
• Why it’s needed: Subpackage support will soon be implemented on the v1alpha2/v1 API, this refactoring work is to make that introduction easier and also to reduce the amount of code in the subpackage implementation PR proper
• How it works: The controller works exactly as before.

---

Related Issue(s)

• Partial implementation of Implement support in porchctl for independent subpackages for the v1alpha2/v1 API #998

---

Type of Change

• Bug fix
• New feature
• Enhancement
• Refactor
• Documentation
• Tests
• Other: ________

---

Checklist

• Code follows project style guidelines
• Self-reviewed changes
• Tests added/updated
• Documentation added/updated
• All tests and gating checks pass

---

AI Disclosure

• I have used AI in the creation of this PR.

[netlify]

✅ Deploy Preview for kpt-porch ready!

Name	Link
🔨 Latest commit	6c38d49
🔍 Latest deploy log	https://app.netlify.com/projects/kpt-porch/deploys/6a69bcd5cb5ba400087269ff
😎 Deploy Preview	https://deploy-preview-1139--kpt-porch.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[Copilot]

Pull request overview

Note

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Refactors the PackageRevision controller code to split out clone/upgrade logic and introduce shared helpers in preparation for subpackage clone/upgrade support.

Changes:

• Extracted clone and upgrade logic into new clone.go / upgrade.go files.
• Added util.go for shared controller helper functions (resource reading, PR lookup, Kptfile status stripping).
• Updated controller reconcile flow to use the refactored source operation type naming and new helper function.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
controllers/packagerevisions/pkg/controllers/packagerevision/util.go	Adds shared helpers for PR lookup, resource reading, and Kptfile status stripping.
controllers/packagerevisions/pkg/controllers/packagerevision/upgrade.go	Extracts upgrade logic and adds subpackage upgrade resource selection.
controllers/packagerevisions/pkg/controllers/packagerevision/upgrade_test.go	Adds unit tests for upgrade helper selection and subpackage resource slicing.
controllers/packagerevisions/pkg/controllers/packagerevision/clone.go	Extracts clone logic and adds subpackage clone name/source helpers.
controllers/packagerevisions/pkg/controllers/packagerevision/clone_test.go	Adds unit tests for clone helper selection and naming behavior.
controllers/packagerevisions/pkg/controllers/packagerevision/source.go	Removes clone/upgrade/util helpers that were moved into dedicated files.
controllers/packagerevisions/pkg/controllers/packagerevision/packagerevision_controller.go	Renames/threads through operation type and introduces finalizeDraftAndUpdateStatus.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (7)

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

• reconcileSource updates and closes the draft, then immediately calls finalizeDraftAndUpdateStatus which updates and closes the same draft again. This double-close is likely to fail (or at least does redundant work) and can prevent source application from succeeding.

	if err := draft.UpdateResources(ctx, resources, sourceOperationType); err != nil {
		return nil, r.setSourceFailed(ctx, pr, fmt.Errorf("update resources: %w", err))
	}

	if err := r.ContentCache.CloseDraft(ctx, repoKey, draft, 0); err != nil {
		return nil, r.setSourceFailed(ctx, pr, fmt.Errorf("close draft: %w", err))
	}

	return r.finalizeDraftAndUpdateStatus(ctx, pr, repoKey, draft, resources, sourceOperationType)

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

• After removing the duplicate v1alpha2 import, this return type should also use the remaining porchv1alpha2 import alias to avoid an undefined identifier.

func (r *PackageRevisionReconciler) getCloneFrom(pr *porchv1alpha2.PackageRevision) *v1alpha2.UpstreamPackage {

controllers/packagerevisions/pkg/controllers/packagerevision/upgrade.go:25

• This file imports the same package path twice (github.com/kptdev/porch/api/porch/v1alpha2), once unnamed and once as porchv1alpha2. Go rejects duplicate imports, so this will not compile.

	"github.com/kptdev/kpt/pkg/lib/kptops"
	"github.com/kptdev/porch/api/porch/v1alpha2"
	porchv1alpha2 "github.com/kptdev/porch/api/porch/v1alpha2"
	"github.com/kptdev/porch/pkg/repository"

controllers/packagerevisions/pkg/controllers/packagerevision/upgrade.go:115

• After removing the duplicate v1alpha2 import, this return type should also use the remaining porchv1alpha2 import alias to avoid an undefined identifier.

func (r *PackageRevisionReconciler) getUpgrade(pr *porchv1alpha2.PackageRevision) *v1alpha2.PackageUpgradeSpec {

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

• The test comment says it should fall back to Source.CloneFrom when CreationSource is unset, but the assertion expects the subpackage value. This is misleading for future readers/maintainers.

	// Without CreationSource set, getCloneFrom should fall back to Source.CloneFrom

controllers/packagerevisions/pkg/controllers/packagerevision/util.go:33

• Comment says this validates the PackageRevision is published, but the function enforces the Draft lifecycle. This mismatch makes the helper easy to misuse.

// getDraftPackageRevision looks up a PackageRevision CRD and validates it is published.

controllers/packagerevisions/pkg/controllers/packagerevision/upgrade.go:119

• upgradePackage is only called from applySource, and applySource returns early when pr.Status.CreationSource != "" (source.go:34-36). That means the pr.Status.CreationSource != "" gate here makes the subpackage-upgrade path effectively unreachable in the current controller flow.

func (r *PackageRevisionReconciler) getUpgrade(pr *porchv1alpha2.PackageRevision) *v1alpha2.PackageUpgradeSpec {
	if pr.Status.CreationSource != "" && pr.Spec.SubpackageOperation != nil && pr.Spec.SubpackageOperation.Upgrade != nil {
		return pr.Spec.SubpackageOperation.Upgrade
	}
	return pr.Spec.Source.Upgrade

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
89.4% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud