From fc732184a27d2253263688e48270ab2caeed0d85 Mon Sep 17 00:00:00 2001 From: liamfallon Date: Wed, 13 May 2026 16:36:13 +0100 Subject: [PATCH 01/15] Rebased due to move to kptdev Signed-off-by: liamfallon --- api/generated/openapi/zz_generated.openapi.go | 14 +++ api/porch/types.go | 8 ++ api/porch/v1alpha1/types.go | 8 ++ api/porch/v1alpha1/zz_generated.conversion.go | 4 + api/porch/v1alpha2/packagerevision_types.go | 2 +- .../porch.kpt.dev_packagerevisions.yaml | 86 +++++++++++-------- api/porch/v1alpha2/source_types.go | 15 ++++ api/porch/v1alpha2/util.go | 2 +- api/porch/v1alpha2/util_test.go | 2 +- api/porch/v1alpha2/zz_generated.deepcopy.go | 26 +++++- .../pkg/controllers/packagerevision/source.go | 4 +- .../packagerevision/source_test.go | 58 +++++++++---- pkg/cli/commands/rpkg/clone/v1alpha2.go | 4 +- pkg/cli/commands/rpkg/upgrade/command.go | 8 +- pkg/cli/commands/rpkg/upgrade/v1alpha2.go | 6 +- .../commands/rpkg/upgrade/v1alpha2_test.go | 26 +++--- test/e2e/crd/clone_test.go | 54 ++++++------ test/e2e/crd/helpers_test.go | 8 +- test/e2e/crd/side_by_side_test.go | 18 ++-- 19 files changed, 238 insertions(+), 115 deletions(-) diff --git a/api/generated/openapi/zz_generated.openapi.go b/api/generated/openapi/zz_generated.openapi.go index 70f67388e..b56a3d1b0 100644 --- a/api/generated/openapi/zz_generated.openapi.go +++ b/api/generated/openapi/zz_generated.openapi.go @@ -544,6 +544,13 @@ func schema_porch_api_porch_v1alpha1_PackageCloneTaskSpec(ref common.ReferenceCa Ref: ref("github.com/kptdev/porch/api/porch/v1alpha1.UpstreamPackage"), }, }, + "subpacakge-dir": { + SchemaProps: spec.SchemaProps{ + Description: "`SubpackageDir` is the path to a subdirectory in an existing package revision into which `Upstream` will be cloned as an independent subpackage.", + Type: []string{"string"}, + Format: "", + }, + }, }, }, }, @@ -1169,6 +1176,13 @@ func schema_porch_api_porch_v1alpha1_PackageUpgradeTaskSpec(ref common.Reference Ref: ref("github.com/kptdev/porch/api/porch/v1alpha1.PackageRevisionRef"), }, }, + "subpacakge-dir": { + SchemaProps: spec.SchemaProps{ + Description: "`SubpackageDir` is the path to a subdirectory in the package revision that contains an independent subpackage that is to be upgraded.", + Type: []string{"string"}, + Format: "", + }, + }, "strategy": { SchemaProps: spec.SchemaProps{ Description: "Defines which strategy should be used to update the package. It defaults to 'resource-merge'.\n * resource-merge: Perform a structural comparison of the original /\n updated resources, and merge the changes into the local package.\n * fast-forward: Fail without updating if the local package was modified\n since it was fetched.\n * force-delete-replace: Wipe all the local changes to the package and replace\n it with the remote version.\n * copy-merge: Copy all the remote changes to the local package.", diff --git a/api/porch/types.go b/api/porch/types.go index 9c2739d2c..5269cd391 100644 --- a/api/porch/types.go +++ b/api/porch/types.go @@ -209,6 +209,10 @@ type PackageInitTaskSpec struct { type PackageCloneTaskSpec struct { // `Upstream` is the reference to the upstream package to clone. Upstream UpstreamPackage `json:"upstreamRef,omitempty"` + + // `SubpackageDir` is the path to a subdirectory in an existing package revision + // into which `Upstream` will be cloned as an independent subpackage. + SubpackageDir string `json:"subpacakge-dir,omitempty"` } type PackageMergeStrategy string @@ -226,6 +230,10 @@ type PackageUpgradeTaskSpec struct { // contains all the local changes on top of the `OldUpstream` package revision. LocalPackageRevisionRef PackageRevisionRef `json:"localPackageRevisionRef,omitempty"` + // `SubpackageDir` is the path to a subdirectory that contains an independent + // subpackage that is to be upgraded. + SubpackageDir string `json:"subpacakge-dir,omitempty"` + // Defines which strategy should be used to update the package. It defaults to 'resource-merge'. // * resource-merge: Perform a structural comparison of the original / // updated resources, and merge the changes into the local package. diff --git a/api/porch/v1alpha1/types.go b/api/porch/v1alpha1/types.go index 0fa0ef8ea..86680526b 100644 --- a/api/porch/v1alpha1/types.go +++ b/api/porch/v1alpha1/types.go @@ -209,6 +209,10 @@ type PackageInitTaskSpec struct { type PackageCloneTaskSpec struct { // `Upstream` is the reference to the upstream package to clone. Upstream UpstreamPackage `json:"upstreamRef,omitempty"` + + // `SubpackageDir` is the path to a subdirectory in an existing package revision + // into which `Upstream` will be cloned as an independent subpackage. + SubpackageDir string `json:"subpacakge-dir,omitempty"` } type PackageMergeStrategy string @@ -226,6 +230,10 @@ type PackageUpgradeTaskSpec struct { // contains all the local changes on top of the `OldUpstream` package revision. LocalPackageRevisionRef PackageRevisionRef `json:"localPackageRevisionRef,omitempty"` + // `SubpackageDir` is the path to a subdirectory in the package revision that contains + // an independent subpackage that is to be upgraded. + SubpackageDir string `json:"subpacakge-dir,omitempty"` + // Defines which strategy should be used to update the package. It defaults to 'resource-merge'. // * resource-merge: Perform a structural comparison of the original / // updated resources, and merge the changes into the local package. diff --git a/api/porch/v1alpha1/zz_generated.conversion.go b/api/porch/v1alpha1/zz_generated.conversion.go index ae5168f1e..49c5cbe11 100644 --- a/api/porch/v1alpha1/zz_generated.conversion.go +++ b/api/porch/v1alpha1/zz_generated.conversion.go @@ -623,6 +623,7 @@ func autoConvert_v1alpha1_PackageCloneTaskSpec_To_porch_PackageCloneTaskSpec(in if err := Convert_v1alpha1_UpstreamPackage_To_porch_UpstreamPackage(&in.Upstream, &out.Upstream, s); err != nil { return err } + out.SubpackageDir = in.SubpackageDir return nil } @@ -635,6 +636,7 @@ func autoConvert_porch_PackageCloneTaskSpec_To_v1alpha1_PackageCloneTaskSpec(in if err := Convert_porch_UpstreamPackage_To_v1alpha1_UpstreamPackage(&in.Upstream, &out.Upstream, s); err != nil { return err } + out.SubpackageDir = in.SubpackageDir return nil } @@ -1007,6 +1009,7 @@ func autoConvert_v1alpha1_PackageUpgradeTaskSpec_To_porch_PackageUpgradeTaskSpec if err := Convert_v1alpha1_PackageRevisionRef_To_porch_PackageRevisionRef(&in.LocalPackageRevisionRef, &out.LocalPackageRevisionRef, s); err != nil { return err } + out.SubpackageDir = in.SubpackageDir out.Strategy = porch.PackageMergeStrategy(in.Strategy) return nil } @@ -1026,6 +1029,7 @@ func autoConvert_porch_PackageUpgradeTaskSpec_To_v1alpha1_PackageUpgradeTaskSpec if err := Convert_porch_PackageRevisionRef_To_v1alpha1_PackageRevisionRef(&in.LocalPackageRevisionRef, &out.LocalPackageRevisionRef, s); err != nil { return err } + out.SubpackageDir = in.SubpackageDir out.Strategy = PackageMergeStrategy(in.Strategy) return nil } diff --git a/api/porch/v1alpha2/packagerevision_types.go b/api/porch/v1alpha2/packagerevision_types.go index 4a10267df..5d7ff9990 100644 --- a/api/porch/v1alpha2/packagerevision_types.go +++ b/api/porch/v1alpha2/packagerevision_types.go @@ -210,7 +210,7 @@ type PackageSource struct { Init *PackageInitSpec `json:"init,omitempty"` // CloneFrom copies a package from an upstream source (first time). - CloneFrom *UpstreamPackage `json:"cloneFrom,omitempty"` + Clone *PackageCloneSpec `json:"cloneFrom,omitempty"` // CopyFrom creates a new revision from an existing package in the same repository. CopyFrom *PackageRevisionRef `json:"copyFrom,omitempty"` diff --git a/api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml b/api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml index 23820a4be..763edde43 100644 --- a/api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml +++ b/api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml @@ -130,52 +130,61 @@ spec: description: CloneFrom copies a package from an upstream source (first time). properties: - git: - description: Git upstream package specification. Required - if type is git. + cloneFrom: + description: '`cloneFrom` is the upstream package to clone.' properties: - directory: - description: Directory within the Git repository where - the packages are stored. - type: string - ref: - description: Ref is the git ref containing the package. - Ref can be a branch, tag, or commit SHA. - type: string - repo: - description: |- - Repo is the address of the Git repository, for example: - https://github.com/GoogleCloudPlatform/blueprints.git + git: + description: Git upstream package specification. Required + if type is git. + properties: + directory: + description: Directory within the Git repository where + the packages are stored. + type: string + ref: + description: Ref is the git ref containing the package. + Ref can be a branch, tag, or commit SHA. + type: string + repo: + description: |- + Repo is the address of the Git repository, for example: + https://github.com/GoogleCloudPlatform/blueprints.git + type: string + secretRef: + description: SecretRef is a reference to secret containing + authentication credentials. + properties: + name: + type: string + required: + - name + type: object + required: + - directory + - ref + - repo + type: object + type: + description: Type of the repository (i.e. git). If empty, + upstreamRef will be used. + enum: + - git type: string - secretRef: - description: SecretRef is a reference to secret containing - authentication credentials. + upstreamRef: + description: UpstreamRef is the reference to the package + from a registered repository. properties: name: type: string required: - name type: object - required: - - directory - - ref - - repo type: object - type: - description: Type of the repository (i.e. git). If empty, - upstreamRef will be used. - enum: - - git + subpacakge-dir: + description: |- + `SubpackageDir` is the path to a subdirectory in an existing package revision + into which `Upstream` will be cloned as an independent subpackage. type: string - upstreamRef: - description: UpstreamRef is the reference to the package from - a registered repository. - properties: - name: - type: string - required: - - name - type: object type: object copyFrom: description: CopyFrom creates a new revision from an existing @@ -253,6 +262,11 @@ spec: - force-delete-replace - copy-merge type: string + subpacakge-dir: + description: |- + `SubpackageDir` is the path to a subdirectory that contains an independent + subpackage that is to be upgraded. + type: string type: object type: object x-kubernetes-validations: diff --git a/api/porch/v1alpha2/source_types.go b/api/porch/v1alpha2/source_types.go index b60a05720..4c410e354 100644 --- a/api/porch/v1alpha2/source_types.go +++ b/api/porch/v1alpha2/source_types.go @@ -30,6 +30,17 @@ type PackageInitSpec struct { Site string `json:"site,omitempty"` } +// PackageCloneSpec defines the package clone parameters. +// Used when creating a new package by cloning it from an existing package revision. +type PackageCloneSpec struct { + // `cloneFrom` is the upstream package to clone. + CloneFrom *UpstreamPackage `json:"cloneFrom,omitempty"` + + // `SubpackageDir` is the path to a subdirectory in an existing package revision + // into which `Upstream` will be cloned as an independent subpackage. + SubpackageDir string `json:"subpacakge-dir,omitempty"` +} + // PackageUpgradeSpec defines the package upgrade parameters. // Used when merging changes from a new upstream version into a local package. type PackageUpgradeSpec struct { @@ -45,6 +56,10 @@ type PackageUpgradeSpec struct { // contains all the local changes on top of the OldUpstream package revision. CurrentPackage PackageRevisionRef `json:"currentPackage,omitempty"` + // `SubpackageDir` is the path to a subdirectory that contains an independent + // subpackage that is to be upgraded. + SubpackageDir string `json:"subpacakge-dir,omitempty"` + // Strategy defines which strategy should be used to update the package. It defaults to 'resource-merge'. // * resource-merge: Perform a structural comparison of the original / // updated resources, and merge the changes into the local package. diff --git a/api/porch/v1alpha2/util.go b/api/porch/v1alpha2/util.go index f4c36e346..e29880188 100644 --- a/api/porch/v1alpha2/util.go +++ b/api/porch/v1alpha2/util.go @@ -48,5 +48,5 @@ func IsPackageCreation(pkgRev *PackageRevision) bool { if pkgRev.Spec.Source == nil { return false } - return pkgRev.Spec.Source.Init != nil || pkgRev.Spec.Source.CloneFrom != nil + return pkgRev.Spec.Source.Init != nil || pkgRev.Spec.Source.Clone != nil } diff --git a/api/porch/v1alpha2/util_test.go b/api/porch/v1alpha2/util_test.go index 4da40cf72..69a30c6e2 100644 --- a/api/porch/v1alpha2/util_test.go +++ b/api/porch/v1alpha2/util_test.go @@ -120,7 +120,7 @@ func TestIsPackageCreation(t *testing.T) { }{ {name: "nil source", source: nil, expected: false}, {name: "init", source: &PackageSource{Init: &PackageInitSpec{}}, expected: true}, - {name: "clone", source: &PackageSource{CloneFrom: &UpstreamPackage{}}, expected: true}, + {name: "clone", source: &PackageSource{Clone: &PackageCloneSpec{}}, expected: true}, {name: "copy", source: &PackageSource{CopyFrom: &PackageRevisionRef{}}, expected: false}, {name: "upgrade", source: &PackageSource{Upgrade: &PackageUpgradeSpec{}}, expected: false}, } diff --git a/api/porch/v1alpha2/zz_generated.deepcopy.go b/api/porch/v1alpha2/zz_generated.deepcopy.go index eadcf0402..8f2a22b4b 100644 --- a/api/porch/v1alpha2/zz_generated.deepcopy.go +++ b/api/porch/v1alpha2/zz_generated.deepcopy.go @@ -89,6 +89,26 @@ func (in *NameRef) DeepCopy() *NameRef { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *PackageCloneSpec) DeepCopyInto(out *PackageCloneSpec) { + *out = *in + if in.CloneFrom != nil { + in, out := &in.CloneFrom, &out.CloneFrom + *out = new(UpstreamPackage) + (*in).DeepCopyInto(*out) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PackageCloneSpec. +func (in *PackageCloneSpec) DeepCopy() *PackageCloneSpec { + if in == nil { + return nil + } + out := new(PackageCloneSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PackageCondition) DeepCopyInto(out *PackageCondition) { *out = *in @@ -291,9 +311,9 @@ func (in *PackageSource) DeepCopyInto(out *PackageSource) { *out = new(PackageInitSpec) (*in).DeepCopyInto(*out) } - if in.CloneFrom != nil { - in, out := &in.CloneFrom, &out.CloneFrom - *out = new(UpstreamPackage) + if in.Clone != nil { + in, out := &in.Clone, &out.Clone + *out = new(PackageCloneSpec) (*in).DeepCopyInto(*out) } if in.CopyFrom != nil { diff --git a/controllers/packagerevisions/pkg/controllers/packagerevision/source.go b/controllers/packagerevisions/pkg/controllers/packagerevision/source.go index 18c0d450b..bd52b0a09 100644 --- a/controllers/packagerevisions/pkg/controllers/packagerevision/source.go +++ b/controllers/packagerevisions/pkg/controllers/packagerevision/source.go @@ -49,7 +49,7 @@ func (r *PackageRevisionReconciler) applySource(ctx context.Context, pr *porchv1 case pr.Spec.Source.Init != nil: resources, err := initPackage(ctx, pr.Spec.PackageName, pr.Spec.Source.Init) return resources, "init", err - case pr.Spec.Source.CloneFrom != nil: + case pr.Spec.Source.Clone != nil: resources, err := r.clonePackage(ctx, pr) return resources, "clone", err case pr.Spec.Source.CopyFrom != nil: @@ -120,7 +120,7 @@ func (r *PackageRevisionReconciler) copyPackage(ctx context.Context, pr *porchv1 // 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 := pr.Spec.Source.CloneFrom + cloneFrom := pr.Spec.Source.Clone.CloneFrom if cloneFrom.UpstreamRef != nil { return r.cloneFromUpstreamRef(ctx, pr, cloneFrom.UpstreamRef) diff --git a/controllers/packagerevisions/pkg/controllers/packagerevision/source_test.go b/controllers/packagerevisions/pkg/controllers/packagerevision/source_test.go index e961f6766..e796ea146 100644 --- a/controllers/packagerevisions/pkg/controllers/packagerevision/source_test.go +++ b/controllers/packagerevisions/pkg/controllers/packagerevision/source_test.go @@ -311,8 +311,10 @@ func TestApplySourceCloneUpstreamRef(t *testing.T) { RepositoryName: "my-repo", WorkspaceName: "v1", Source: &porchv1alpha2.PackageSource{ - CloneFrom: &porchv1alpha2.UpstreamPackage{ - UpstreamRef: &porchv1alpha2.PackageRevisionRef{Name: "upstream-repo.upstream-pkg.v1"}, + Clone: &porchv1alpha2.PackageCloneSpec{ + CloneFrom: &porchv1alpha2.UpstreamPackage{ + UpstreamRef: &porchv1alpha2.PackageRevisionRef{Name: "upstream-repo.upstream-pkg.v1"}, + }, }, }, }, @@ -346,8 +348,10 @@ func TestApplySourceCloneUpstreamRefNotPublished(t *testing.T) { RepositoryName: "my-repo", WorkspaceName: "v1", Source: &porchv1alpha2.PackageSource{ - CloneFrom: &porchv1alpha2.UpstreamPackage{ - UpstreamRef: &porchv1alpha2.PackageRevisionRef{Name: "upstream-repo.pkg.v1"}, + Clone: &porchv1alpha2.PackageCloneSpec{ + CloneFrom: &porchv1alpha2.UpstreamPackage{ + UpstreamRef: &porchv1alpha2.PackageRevisionRef{Name: "upstream-repo.pkg.v1"}, + }, }, }, }, @@ -371,8 +375,10 @@ func TestApplySourceCloneUpstreamRefNotFound(t *testing.T) { RepositoryName: "my-repo", WorkspaceName: "v1", Source: &porchv1alpha2.PackageSource{ - CloneFrom: &porchv1alpha2.UpstreamPackage{ - UpstreamRef: &porchv1alpha2.PackageRevisionRef{Name: "upstream-repo.pkg.v1"}, + Clone: &porchv1alpha2.PackageCloneSpec{ + CloneFrom: &porchv1alpha2.UpstreamPackage{ + UpstreamRef: &porchv1alpha2.PackageRevisionRef{Name: "upstream-repo.pkg.v1"}, + }, }, }, }, @@ -407,8 +413,10 @@ func TestApplySourceCloneGit(t *testing.T) { RepositoryName: "my-repo", WorkspaceName: "v1", Source: &porchv1alpha2.PackageSource{ - CloneFrom: &porchv1alpha2.UpstreamPackage{ - Git: gitSpec, + Clone: &porchv1alpha2.PackageCloneSpec{ + CloneFrom: &porchv1alpha2.UpstreamPackage{ + Git: gitSpec, + }, }, }, }, @@ -431,7 +439,9 @@ func TestApplySourceCloneNoSourceSpecified(t *testing.T) { RepositoryName: "my-repo", WorkspaceName: "v1", Source: &porchv1alpha2.PackageSource{ - CloneFrom: &porchv1alpha2.UpstreamPackage{}, + Clone: &porchv1alpha2.PackageCloneSpec{ + CloneFrom: &porchv1alpha2.UpstreamPackage{}, + }, }, }, } @@ -468,8 +478,10 @@ func TestApplySourceCloneUpstreamRefGetContentError(t *testing.T) { RepositoryName: "my-repo", WorkspaceName: "v1", Source: &porchv1alpha2.PackageSource{ - CloneFrom: &porchv1alpha2.UpstreamPackage{ - UpstreamRef: &porchv1alpha2.PackageRevisionRef{Name: "upstream-repo.pkg.v1"}, + Clone: &porchv1alpha2.PackageCloneSpec{ + CloneFrom: &porchv1alpha2.UpstreamPackage{ + UpstreamRef: &porchv1alpha2.PackageRevisionRef{Name: "upstream-repo.pkg.v1"}, + }, }, }, }, @@ -511,8 +523,10 @@ func TestApplySourceCloneUpstreamRefGetLockError(t *testing.T) { RepositoryName: "my-repo", WorkspaceName: "v1", Source: &porchv1alpha2.PackageSource{ - CloneFrom: &porchv1alpha2.UpstreamPackage{ - UpstreamRef: &porchv1alpha2.PackageRevisionRef{Name: "upstream-repo.pkg.v1"}, + Clone: &porchv1alpha2.PackageCloneSpec{ + CloneFrom: &porchv1alpha2.UpstreamPackage{ + UpstreamRef: &porchv1alpha2.PackageRevisionRef{Name: "upstream-repo.pkg.v1"}, + }, }, }, }, @@ -544,8 +558,10 @@ func TestApplySourceCloneGitFetchError(t *testing.T) { RepositoryName: "my-repo", WorkspaceName: "v1", Source: &porchv1alpha2.PackageSource{ - CloneFrom: &porchv1alpha2.UpstreamPackage{ - Git: gitSpec, + Clone: &porchv1alpha2.PackageCloneSpec{ + CloneFrom: &porchv1alpha2.UpstreamPackage{ + Git: gitSpec, + }, }, }, }, @@ -586,8 +602,10 @@ func TestApplySourceCloneUpstreamRefGetResourcesError(t *testing.T) { RepositoryName: "my-repo", WorkspaceName: "v1", Source: &porchv1alpha2.PackageSource{ - CloneFrom: &porchv1alpha2.UpstreamPackage{ - UpstreamRef: &porchv1alpha2.PackageRevisionRef{Name: "upstream-repo.pkg.v1"}, + Clone: &porchv1alpha2.PackageCloneSpec{ + CloneFrom: &porchv1alpha2.UpstreamPackage{ + UpstreamRef: &porchv1alpha2.PackageRevisionRef{Name: "upstream-repo.pkg.v1"}, + }, }, }, }, @@ -603,8 +621,10 @@ func TestApplySourceCloneIdempotent(t *testing.T) { Spec: porchv1alpha2.PackageRevisionSpec{ PackageName: "pkg", Source: &porchv1alpha2.PackageSource{ - CloneFrom: &porchv1alpha2.UpstreamPackage{ - UpstreamRef: &porchv1alpha2.PackageRevisionRef{Name: "upstream-repo.pkg.v1"}, + Clone: &porchv1alpha2.PackageCloneSpec{ + CloneFrom: &porchv1alpha2.UpstreamPackage{ + UpstreamRef: &porchv1alpha2.PackageRevisionRef{Name: "upstream-repo.pkg.v1"}, + }, }, }, }, diff --git a/pkg/cli/commands/rpkg/clone/v1alpha2.go b/pkg/cli/commands/rpkg/clone/v1alpha2.go index bb107ae1e..87b999256 100644 --- a/pkg/cli/commands/rpkg/clone/v1alpha2.go +++ b/pkg/cli/commands/rpkg/clone/v1alpha2.go @@ -154,7 +154,9 @@ func (r *v1alpha2Runner) runE(cmd *cobra.Command, _ []string) error { RepositoryName: r.repository, Lifecycle: porchv1alpha2.PackageRevisionLifecycleDraft, Source: &porchv1alpha2.PackageSource{ - CloneFrom: &r.upstream, + Clone: &porchv1alpha2.PackageCloneSpec{ + CloneFrom: &r.upstream, + }, }, }, } diff --git a/pkg/cli/commands/rpkg/upgrade/command.go b/pkg/cli/commands/rpkg/upgrade/command.go index d90194fc7..171c7a486 100644 --- a/pkg/cli/commands/rpkg/upgrade/command.go +++ b/pkg/cli/commands/rpkg/upgrade/command.go @@ -62,6 +62,7 @@ func newRunner(ctx context.Context, rcg *genericclioptions.ConfigFlags) *runner } r.Command.Flags().IntVar(&r.revision, "revision", 0, "Revision of the upstream package to upgrade to.") r.Command.Flags().StringVar(&r.workspace, "workspace", "", "Workspace name of the upgrade package revision.") + r.Command.Flags().StringVar(&r.strategy, "subpackage-dir", "", "Path to a subpackage directory within the package revision to upgrade, if omitted, the entire pacakge revision is upgraded.") r.Command.Flags().StringVar(&r.strategy, "strategy", "resource-merge", "Strategy to use for the upgrade. Options: resource-merge (default), fast-forward, force-delete-replace, copy-merge.") r.Command.Flags().StringVar(&r.discover, "discover", "", `If set, search for available updates instead of performing an update. @@ -76,9 +77,10 @@ type runner struct { client client.Client Command *cobra.Command - revision int // Target package revision - workspace string - strategy string // Merge strategy to use, default is "resource-merge" + revision int // Target package revision + workspace string + subpackageDir string // The subpackage to upgrade + strategy string // Merge strategy to use, default is "resource-merge" discover string // If set, discover updates rather than do updates diff --git a/pkg/cli/commands/rpkg/upgrade/v1alpha2.go b/pkg/cli/commands/rpkg/upgrade/v1alpha2.go index e80be9c61..5e317ab9d 100644 --- a/pkg/cli/commands/rpkg/upgrade/v1alpha2.go +++ b/pkg/cli/commands/rpkg/upgrade/v1alpha2.go @@ -281,9 +281,9 @@ func (r *v1alpha2Runner) findUpstreamName(pr *porchv1alpha2.PackageRevision) str return "" } switch { - case pr.Spec.Source.CloneFrom != nil: - if pr.Spec.Source.CloneFrom.UpstreamRef != nil { - return pr.Spec.Source.CloneFrom.UpstreamRef.Name + case pr.Spec.Source.Clone != nil: + if pr.Spec.Source.Clone.CloneFrom.UpstreamRef != nil { + return pr.Spec.Source.Clone.CloneFrom.UpstreamRef.Name } // Git URL clone — no upstream PR name in spec. Match via selfLock. if up := r.findUpstreamBySelfLock(pr.Status.UpstreamLock); up != nil { diff --git a/pkg/cli/commands/rpkg/upgrade/v1alpha2_test.go b/pkg/cli/commands/rpkg/upgrade/v1alpha2_test.go index ae4dda9c1..583d3cab2 100644 --- a/pkg/cli/commands/rpkg/upgrade/v1alpha2_test.go +++ b/pkg/cli/commands/rpkg/upgrade/v1alpha2_test.go @@ -105,8 +105,10 @@ func makeV2Pr(ns, repo, pkg string, revision int, lc porchv1alpha2.PackageRevisi func cloneSource(upstreamRefName string) *porchv1alpha2.PackageSource { return &porchv1alpha2.PackageSource{ - CloneFrom: &porchv1alpha2.UpstreamPackage{ - UpstreamRef: &porchv1alpha2.PackageRevisionRef{Name: upstreamRefName}, + Clone: &porchv1alpha2.PackageCloneSpec{ + CloneFrom: &porchv1alpha2.UpstreamPackage{ + UpstreamRef: &porchv1alpha2.PackageRevisionRef{Name: upstreamRefName}, + }, }, } } @@ -300,11 +302,13 @@ func TestV1Alpha2FindUpstreamNameGitURLClone(t *testing.T) { Spec: porchv1alpha2.PackageRevisionSpec{ Lifecycle: porchv1alpha2.PackageRevisionLifecyclePublished, Source: &porchv1alpha2.PackageSource{ - CloneFrom: &porchv1alpha2.UpstreamPackage{ - Type: porchv1alpha2.RepositoryTypeGit, - Git: &porchv1alpha2.GitPackage{ - Repo: "https://github.com/user/repo", - Ref: "orig/v1", + Clone: &porchv1alpha2.PackageCloneSpec{ + CloneFrom: &porchv1alpha2.UpstreamPackage{ + Type: porchv1alpha2.RepositoryTypeGit, + Git: &porchv1alpha2.GitPackage{ + Repo: "https://github.com/user/repo", + Ref: "orig/v1", + }, }, }, }, @@ -320,9 +324,11 @@ func TestV1Alpha2FindUpstreamNameGitURLClone(t *testing.T) { Spec: porchv1alpha2.PackageRevisionSpec{ Lifecycle: porchv1alpha2.PackageRevisionLifecyclePublished, Source: &porchv1alpha2.PackageSource{ - CloneFrom: &porchv1alpha2.UpstreamPackage{ - Type: porchv1alpha2.RepositoryTypeGit, - Git: &porchv1alpha2.GitPackage{Repo: "https://other.com/repo"}, + Clone: &porchv1alpha2.PackageCloneSpec{ + CloneFrom: &porchv1alpha2.UpstreamPackage{ + Type: porchv1alpha2.RepositoryTypeGit, + Git: &porchv1alpha2.GitPackage{Repo: "https://other.com/repo"}, + }, }, }, }, diff --git a/test/e2e/crd/clone_test.go b/test/e2e/crd/clone_test.go index 1cd8d21f0..d0b9c78c6 100644 --- a/test/e2e/crd/clone_test.go +++ b/test/e2e/crd/clone_test.go @@ -87,14 +87,16 @@ var _ = Describe("Clone", Ordered, Label("lifecycle"), func() { By("cloning basens with leading slash in git directory") pr := newPackageRevision(env.Namespace, downstreamRepo, "basens-ls", "v1", func(pr *porchv1alpha2.PackageRevision) { pr.Spec.Source = &porchv1alpha2.PackageSource{ - CloneFrom: &porchv1alpha2.UpstreamPackage{ - Type: porchv1alpha2.RepositoryTypeGit, - Git: &porchv1alpha2.GitPackage{ - Repo: giteaRepoURL(upstreamRepo), - Ref: "basens/v1", - Directory: "/basens", - SecretRef: porchv1alpha2.SecretRef{ - Name: downstreamRepo + "-auth", + Clone: &porchv1alpha2.PackageCloneSpec{ + CloneFrom: &porchv1alpha2.UpstreamPackage{ + Type: porchv1alpha2.RepositoryTypeGit, + Git: &porchv1alpha2.GitPackage{ + Repo: giteaRepoURL(upstreamRepo), + Ref: "basens/v1", + Directory: "/basens", + SecretRef: porchv1alpha2.SecretRef{ + Name: downstreamRepo + "-auth", + }, }, }, }, @@ -111,14 +113,16 @@ var _ = Describe("Clone", Ordered, Label("lifecycle"), func() { By("creating a clone from raw git URL") pr := newPackageRevision(env.Namespace, downstreamRepo, "basens", "git-clone", func(pr *porchv1alpha2.PackageRevision) { pr.Spec.Source = &porchv1alpha2.PackageSource{ - CloneFrom: &porchv1alpha2.UpstreamPackage{ - Type: porchv1alpha2.RepositoryTypeGit, - Git: &porchv1alpha2.GitPackage{ - Repo: giteaRepoURL(upstreamRepo), - Ref: "basens/v1", - Directory: "/basens", - SecretRef: porchv1alpha2.SecretRef{ - Name: downstreamRepo + "-auth", + Clone: &porchv1alpha2.PackageCloneSpec{ + CloneFrom: &porchv1alpha2.UpstreamPackage{ + Type: porchv1alpha2.RepositoryTypeGit, + Git: &porchv1alpha2.GitPackage{ + Repo: giteaRepoURL(upstreamRepo), + Ref: "basens/v1", + Directory: "/basens", + SecretRef: porchv1alpha2.SecretRef{ + Name: downstreamRepo + "-auth", + }, }, }, }, @@ -335,14 +339,16 @@ var _ = Describe("Clone", Ordered, Label("lifecycle"), func() { By("cloning using bearer token auth") pr := newPackageRevision(env.Namespace, downstreamRepo, "basens", "token-clone", func(pr *porchv1alpha2.PackageRevision) { pr.Spec.Source = &porchv1alpha2.PackageSource{ - CloneFrom: &porchv1alpha2.UpstreamPackage{ - Type: porchv1alpha2.RepositoryTypeGit, - Git: &porchv1alpha2.GitPackage{ - Repo: giteaRepoURL("test-blueprints"), - Ref: "basens/v1", - Directory: "/basens", - SecretRef: porchv1alpha2.SecretRef{ - Name: "bearer-token-secret", + Clone: &porchv1alpha2.PackageCloneSpec{ + CloneFrom: &porchv1alpha2.UpstreamPackage{ + Type: porchv1alpha2.RepositoryTypeGit, + Git: &porchv1alpha2.GitPackage{ + Repo: giteaRepoURL("test-blueprints"), + Ref: "basens/v1", + Directory: "/basens", + SecretRef: porchv1alpha2.SecretRef{ + Name: "bearer-token-secret", + }, }, }, }, diff --git a/test/e2e/crd/helpers_test.go b/test/e2e/crd/helpers_test.go index 687eeeca2..5f57dc3d1 100644 --- a/test/e2e/crd/helpers_test.go +++ b/test/e2e/crd/helpers_test.go @@ -321,9 +321,11 @@ func withInitFull(description string, keywords []string, site string) prOption { func withCloneFromRef(upstreamCRDName string) prOption { return func(pr *porchv1alpha2.PackageRevision) { pr.Spec.Source = &porchv1alpha2.PackageSource{ - CloneFrom: &porchv1alpha2.UpstreamPackage{ - UpstreamRef: &porchv1alpha2.PackageRevisionRef{ - Name: upstreamCRDName, + Clone: &porchv1alpha2.PackageCloneSpec{ + CloneFrom: &porchv1alpha2.UpstreamPackage{ + UpstreamRef: &porchv1alpha2.PackageRevisionRef{ + Name: upstreamCRDName, + }, }, }, } diff --git a/test/e2e/crd/side_by_side_test.go b/test/e2e/crd/side_by_side_test.go index 3692f9cc6..e8e11d5a9 100644 --- a/test/e2e/crd/side_by_side_test.go +++ b/test/e2e/crd/side_by_side_test.go @@ -263,14 +263,16 @@ var _ = Describe("SideBySide", Ordered, Label("migration"), func() { By("cloning into the v1alpha2 repo via git URL") pr := newPackageRevision(env.Namespace, v2Repo, "cross-clone", "v1", func(pr *porchv1alpha2.PackageRevision) { pr.Spec.Source = &porchv1alpha2.PackageSource{ - CloneFrom: &porchv1alpha2.UpstreamPackage{ - Type: porchv1alpha2.RepositoryTypeGit, - Git: &porchv1alpha2.GitPackage{ - Repo: giteaRepoURL(v1Repo), - Ref: v1Pr.gitRef, - Directory: "/" + v1Pr.packageName, - SecretRef: porchv1alpha2.SecretRef{ - Name: v2Repo + "-auth", + Clone: &porchv1alpha2.PackageCloneSpec{ + CloneFrom: &porchv1alpha2.UpstreamPackage{ + Type: porchv1alpha2.RepositoryTypeGit, + Git: &porchv1alpha2.GitPackage{ + Repo: giteaRepoURL(v1Repo), + Ref: v1Pr.gitRef, + Directory: "/" + v1Pr.packageName, + SecretRef: porchv1alpha2.SecretRef{ + Name: v2Repo + "-auth", + }, }, }, }, From ed48b1cc1ea4c8e055a6601ee3135bedaa872b40 Mon Sep 17 00:00:00 2001 From: liamfallon Date: Thu, 7 May 2026 16:39:21 +0100 Subject: [PATCH 02/15] Revert subpckage-dir parameter on upgrade Signed-off-by: liamfallon --- pkg/cli/commands/rpkg/upgrade/command.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/pkg/cli/commands/rpkg/upgrade/command.go b/pkg/cli/commands/rpkg/upgrade/command.go index 171c7a486..d90194fc7 100644 --- a/pkg/cli/commands/rpkg/upgrade/command.go +++ b/pkg/cli/commands/rpkg/upgrade/command.go @@ -62,7 +62,6 @@ func newRunner(ctx context.Context, rcg *genericclioptions.ConfigFlags) *runner } r.Command.Flags().IntVar(&r.revision, "revision", 0, "Revision of the upstream package to upgrade to.") r.Command.Flags().StringVar(&r.workspace, "workspace", "", "Workspace name of the upgrade package revision.") - r.Command.Flags().StringVar(&r.strategy, "subpackage-dir", "", "Path to a subpackage directory within the package revision to upgrade, if omitted, the entire pacakge revision is upgraded.") r.Command.Flags().StringVar(&r.strategy, "strategy", "resource-merge", "Strategy to use for the upgrade. Options: resource-merge (default), fast-forward, force-delete-replace, copy-merge.") r.Command.Flags().StringVar(&r.discover, "discover", "", `If set, search for available updates instead of performing an update. @@ -77,10 +76,9 @@ type runner struct { client client.Client Command *cobra.Command - revision int // Target package revision - workspace string - subpackageDir string // The subpackage to upgrade - strategy string // Merge strategy to use, default is "resource-merge" + revision int // Target package revision + workspace string + strategy string // Merge strategy to use, default is "resource-merge" discover string // If set, discover updates rather than do updates From 2b06dc15d0f324b63ce93c9909fd503ee7ddab56 Mon Sep 17 00:00:00 2001 From: liamfallon Date: Thu, 7 May 2026 16:54:52 +0100 Subject: [PATCH 03/15] Addrees review comments Signed-off-by: liamfallon --- api/generated/openapi/zz_generated.openapi.go | 4 ++-- api/porch/types.go | 4 ++-- api/porch/v1alpha1/types.go | 4 ++-- api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml | 4 ++-- api/porch/v1alpha2/source_types.go | 4 ++-- .../pkg/controllers/packagerevision/source.go | 4 ++++ pkg/cli/commands/rpkg/upgrade/v1alpha2.go | 2 +- 7 files changed, 15 insertions(+), 11 deletions(-) diff --git a/api/generated/openapi/zz_generated.openapi.go b/api/generated/openapi/zz_generated.openapi.go index b56a3d1b0..cb69214e9 100644 --- a/api/generated/openapi/zz_generated.openapi.go +++ b/api/generated/openapi/zz_generated.openapi.go @@ -544,7 +544,7 @@ func schema_porch_api_porch_v1alpha1_PackageCloneTaskSpec(ref common.ReferenceCa Ref: ref("github.com/kptdev/porch/api/porch/v1alpha1.UpstreamPackage"), }, }, - "subpacakge-dir": { + "subpackage-dir": { SchemaProps: spec.SchemaProps{ Description: "`SubpackageDir` is the path to a subdirectory in an existing package revision into which `Upstream` will be cloned as an independent subpackage.", Type: []string{"string"}, @@ -1176,7 +1176,7 @@ func schema_porch_api_porch_v1alpha1_PackageUpgradeTaskSpec(ref common.Reference Ref: ref("github.com/kptdev/porch/api/porch/v1alpha1.PackageRevisionRef"), }, }, - "subpacakge-dir": { + "subpackage-dir": { SchemaProps: spec.SchemaProps{ Description: "`SubpackageDir` is the path to a subdirectory in the package revision that contains an independent subpackage that is to be upgraded.", Type: []string{"string"}, diff --git a/api/porch/types.go b/api/porch/types.go index 5269cd391..5490b96c5 100644 --- a/api/porch/types.go +++ b/api/porch/types.go @@ -212,7 +212,7 @@ type PackageCloneTaskSpec struct { // `SubpackageDir` is the path to a subdirectory in an existing package revision // into which `Upstream` will be cloned as an independent subpackage. - SubpackageDir string `json:"subpacakge-dir,omitempty"` + SubpackageDir string `json:"subpackage-dir,omitempty"` } type PackageMergeStrategy string @@ -232,7 +232,7 @@ type PackageUpgradeTaskSpec struct { // `SubpackageDir` is the path to a subdirectory that contains an independent // subpackage that is to be upgraded. - SubpackageDir string `json:"subpacakge-dir,omitempty"` + SubpackageDir string `json:"subpackage-dir,omitempty"` // Defines which strategy should be used to update the package. It defaults to 'resource-merge'. // * resource-merge: Perform a structural comparison of the original / diff --git a/api/porch/v1alpha1/types.go b/api/porch/v1alpha1/types.go index 86680526b..6aa505de8 100644 --- a/api/porch/v1alpha1/types.go +++ b/api/porch/v1alpha1/types.go @@ -212,7 +212,7 @@ type PackageCloneTaskSpec struct { // `SubpackageDir` is the path to a subdirectory in an existing package revision // into which `Upstream` will be cloned as an independent subpackage. - SubpackageDir string `json:"subpacakge-dir,omitempty"` + SubpackageDir string `json:"subpackage-dir,omitempty"` } type PackageMergeStrategy string @@ -232,7 +232,7 @@ type PackageUpgradeTaskSpec struct { // `SubpackageDir` is the path to a subdirectory in the package revision that contains // an independent subpackage that is to be upgraded. - SubpackageDir string `json:"subpacakge-dir,omitempty"` + SubpackageDir string `json:"subpackage-dir,omitempty"` // Defines which strategy should be used to update the package. It defaults to 'resource-merge'. // * resource-merge: Perform a structural comparison of the original / diff --git a/api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml b/api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml index 763edde43..fa81ad299 100644 --- a/api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml +++ b/api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml @@ -180,7 +180,7 @@ spec: - name type: object type: object - subpacakge-dir: + subpackage-dir: description: |- `SubpackageDir` is the path to a subdirectory in an existing package revision into which `Upstream` will be cloned as an independent subpackage. @@ -262,7 +262,7 @@ spec: - force-delete-replace - copy-merge type: string - subpacakge-dir: + subpackage-dir: description: |- `SubpackageDir` is the path to a subdirectory that contains an independent subpackage that is to be upgraded. diff --git a/api/porch/v1alpha2/source_types.go b/api/porch/v1alpha2/source_types.go index 4c410e354..ac58ce958 100644 --- a/api/porch/v1alpha2/source_types.go +++ b/api/porch/v1alpha2/source_types.go @@ -38,7 +38,7 @@ type PackageCloneSpec struct { // `SubpackageDir` is the path to a subdirectory in an existing package revision // into which `Upstream` will be cloned as an independent subpackage. - SubpackageDir string `json:"subpacakge-dir,omitempty"` + SubpackageDir string `json:"subpackage-dir,omitempty"` } // PackageUpgradeSpec defines the package upgrade parameters. @@ -58,7 +58,7 @@ type PackageUpgradeSpec struct { // `SubpackageDir` is the path to a subdirectory that contains an independent // subpackage that is to be upgraded. - SubpackageDir string `json:"subpacakge-dir,omitempty"` + SubpackageDir string `json:"subpackage-dir,omitempty"` // Strategy defines which strategy should be used to update the package. It defaults to 'resource-merge'. // * resource-merge: Perform a structural comparison of the original / diff --git a/controllers/packagerevisions/pkg/controllers/packagerevision/source.go b/controllers/packagerevisions/pkg/controllers/packagerevision/source.go index bd52b0a09..8d90a84d5 100644 --- a/controllers/packagerevisions/pkg/controllers/packagerevision/source.go +++ b/controllers/packagerevisions/pkg/controllers/packagerevision/source.go @@ -122,6 +122,10 @@ func (r *PackageRevisionReconciler) copyPackage(ctx context.Context, pr *porchv1 func (r *PackageRevisionReconciler) clonePackage(ctx context.Context, pr *porchv1alpha2.PackageRevision) (map[string]string, error) { cloneFrom := pr.Spec.Source.Clone.CloneFrom + if cloneFrom == nil { + return nil, fmt.Errorf("a clone source must be specified") + } + if cloneFrom.UpstreamRef != nil { return r.cloneFromUpstreamRef(ctx, pr, cloneFrom.UpstreamRef) } diff --git a/pkg/cli/commands/rpkg/upgrade/v1alpha2.go b/pkg/cli/commands/rpkg/upgrade/v1alpha2.go index 5e317ab9d..7083e4b26 100644 --- a/pkg/cli/commands/rpkg/upgrade/v1alpha2.go +++ b/pkg/cli/commands/rpkg/upgrade/v1alpha2.go @@ -282,7 +282,7 @@ func (r *v1alpha2Runner) findUpstreamName(pr *porchv1alpha2.PackageRevision) str } switch { case pr.Spec.Source.Clone != nil: - if pr.Spec.Source.Clone.CloneFrom.UpstreamRef != nil { + if pr.Spec.Source.Clone.CloneFrom != nil && pr.Spec.Source.Clone.CloneFrom.UpstreamRef != nil { return pr.Spec.Source.Clone.CloneFrom.UpstreamRef.Name } // Git URL clone — no upstream PR name in spec. Match via selfLock. From 9f47c7c7c204f7db972d1be354667ea597b48089 Mon Sep 17 00:00:00 2001 From: liamfallon Date: Thu, 7 May 2026 17:08:13 +0100 Subject: [PATCH 04/15] Address copilot comments Signed-off-by: liamfallon --- api/generated/openapi/zz_generated.openapi.go | 4 ++-- api/porch/types.go | 4 ++-- api/porch/v1alpha1/types.go | 4 ++-- api/porch/v1alpha2/packagerevision_types.go | 2 +- api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml | 4 ++-- api/porch/v1alpha2/source_types.go | 6 +++--- 6 files changed, 12 insertions(+), 12 deletions(-) diff --git a/api/generated/openapi/zz_generated.openapi.go b/api/generated/openapi/zz_generated.openapi.go index cb69214e9..dd8c218b8 100644 --- a/api/generated/openapi/zz_generated.openapi.go +++ b/api/generated/openapi/zz_generated.openapi.go @@ -544,7 +544,7 @@ func schema_porch_api_porch_v1alpha1_PackageCloneTaskSpec(ref common.ReferenceCa Ref: ref("github.com/kptdev/porch/api/porch/v1alpha1.UpstreamPackage"), }, }, - "subpackage-dir": { + "subpackageDir": { SchemaProps: spec.SchemaProps{ Description: "`SubpackageDir` is the path to a subdirectory in an existing package revision into which `Upstream` will be cloned as an independent subpackage.", Type: []string{"string"}, @@ -1176,7 +1176,7 @@ func schema_porch_api_porch_v1alpha1_PackageUpgradeTaskSpec(ref common.Reference Ref: ref("github.com/kptdev/porch/api/porch/v1alpha1.PackageRevisionRef"), }, }, - "subpackage-dir": { + "subpackageDir": { SchemaProps: spec.SchemaProps{ Description: "`SubpackageDir` is the path to a subdirectory in the package revision that contains an independent subpackage that is to be upgraded.", Type: []string{"string"}, diff --git a/api/porch/types.go b/api/porch/types.go index 5490b96c5..f5387533e 100644 --- a/api/porch/types.go +++ b/api/porch/types.go @@ -212,7 +212,7 @@ type PackageCloneTaskSpec struct { // `SubpackageDir` is the path to a subdirectory in an existing package revision // into which `Upstream` will be cloned as an independent subpackage. - SubpackageDir string `json:"subpackage-dir,omitempty"` + SubpackageDir string `json:"subpackageDir,omitempty"` } type PackageMergeStrategy string @@ -232,7 +232,7 @@ type PackageUpgradeTaskSpec struct { // `SubpackageDir` is the path to a subdirectory that contains an independent // subpackage that is to be upgraded. - SubpackageDir string `json:"subpackage-dir,omitempty"` + SubpackageDir string `json:"subpackageDir,omitempty"` // Defines which strategy should be used to update the package. It defaults to 'resource-merge'. // * resource-merge: Perform a structural comparison of the original / diff --git a/api/porch/v1alpha1/types.go b/api/porch/v1alpha1/types.go index 6aa505de8..1280dd442 100644 --- a/api/porch/v1alpha1/types.go +++ b/api/porch/v1alpha1/types.go @@ -212,7 +212,7 @@ type PackageCloneTaskSpec struct { // `SubpackageDir` is the path to a subdirectory in an existing package revision // into which `Upstream` will be cloned as an independent subpackage. - SubpackageDir string `json:"subpackage-dir,omitempty"` + SubpackageDir string `json:"subpackageDir,omitempty"` } type PackageMergeStrategy string @@ -232,7 +232,7 @@ type PackageUpgradeTaskSpec struct { // `SubpackageDir` is the path to a subdirectory in the package revision that contains // an independent subpackage that is to be upgraded. - SubpackageDir string `json:"subpackage-dir,omitempty"` + SubpackageDir string `json:"subpackageDir,omitempty"` // Defines which strategy should be used to update the package. It defaults to 'resource-merge'. // * resource-merge: Perform a structural comparison of the original / diff --git a/api/porch/v1alpha2/packagerevision_types.go b/api/porch/v1alpha2/packagerevision_types.go index 5d7ff9990..09d051541 100644 --- a/api/porch/v1alpha2/packagerevision_types.go +++ b/api/porch/v1alpha2/packagerevision_types.go @@ -210,7 +210,7 @@ type PackageSource struct { Init *PackageInitSpec `json:"init,omitempty"` // CloneFrom copies a package from an upstream source (first time). - Clone *PackageCloneSpec `json:"cloneFrom,omitempty"` + Clone *PackageCloneSpec `json:"clone,omitempty"` // CopyFrom creates a new revision from an existing package in the same repository. CopyFrom *PackageRevisionRef `json:"copyFrom,omitempty"` diff --git a/api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml b/api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml index fa81ad299..d0052460c 100644 --- a/api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml +++ b/api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml @@ -180,7 +180,7 @@ spec: - name type: object type: object - subpackage-dir: + subpackageDir: description: |- `SubpackageDir` is the path to a subdirectory in an existing package revision into which `Upstream` will be cloned as an independent subpackage. @@ -262,7 +262,7 @@ spec: - force-delete-replace - copy-merge type: string - subpackage-dir: + subpackageDir: description: |- `SubpackageDir` is the path to a subdirectory that contains an independent subpackage that is to be upgraded. diff --git a/api/porch/v1alpha2/source_types.go b/api/porch/v1alpha2/source_types.go index ac58ce958..268908be6 100644 --- a/api/porch/v1alpha2/source_types.go +++ b/api/porch/v1alpha2/source_types.go @@ -37,8 +37,8 @@ type PackageCloneSpec struct { CloneFrom *UpstreamPackage `json:"cloneFrom,omitempty"` // `SubpackageDir` is the path to a subdirectory in an existing package revision - // into which `Upstream` will be cloned as an independent subpackage. - SubpackageDir string `json:"subpackage-dir,omitempty"` + // into which `CloneFrom` will be cloned as an independent subpackage. + SubpackageDir string `json:"subpackageDir,omitempty"` } // PackageUpgradeSpec defines the package upgrade parameters. @@ -58,7 +58,7 @@ type PackageUpgradeSpec struct { // `SubpackageDir` is the path to a subdirectory that contains an independent // subpackage that is to be upgraded. - SubpackageDir string `json:"subpackage-dir,omitempty"` + SubpackageDir string `json:"subpackageDir,omitempty"` // Strategy defines which strategy should be used to update the package. It defaults to 'resource-merge'. // * resource-merge: Perform a structural comparison of the original / From 9b9c66488ec7baef224a3f429d61e14c3b3d5c2d Mon Sep 17 00:00:00 2001 From: liamfallon Date: Mon, 11 May 2026 17:01:41 +0100 Subject: [PATCH 05/15] Correct annotations for code generation Signed-off-by: liamfallon --- api/porch/v1alpha2/packagerevision_types.go | 2 +- api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/api/porch/v1alpha2/packagerevision_types.go b/api/porch/v1alpha2/packagerevision_types.go index 09d051541..4d266773a 100644 --- a/api/porch/v1alpha2/packagerevision_types.go +++ b/api/porch/v1alpha2/packagerevision_types.go @@ -204,7 +204,7 @@ type PackageRevisionStatus struct { // PackageSource specifies how a package was created. // Exactly one field must be set. -// +kubebuilder:validation:XValidation:rule="[has(self.init), has(self.cloneFrom), has(self.copyFrom), has(self.upgrade)].filter(x, x).size() == 1",message="exactly one of init, cloneFrom, copyFrom, or upgrade must be set" +// +kubebuilder:validation:XValidation:rule="[has(self.init), has(self.clone), has(self.copyFrom), has(self.upgrade)].filter(x, x).size() == 1",message="exactly one of init, cloneFrom, copyFrom, or upgrade must be set" type PackageSource struct { // Init creates a brand new package from scratch. Init *PackageInitSpec `json:"init,omitempty"` diff --git a/api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml b/api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml index d0052460c..db0c4dd60 100644 --- a/api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml +++ b/api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml @@ -126,7 +126,7 @@ spec: For packages discovered from git (existed before Porch), Source will be nil. This field is immutable after creation. properties: - cloneFrom: + clone: description: CloneFrom copies a package from an upstream source (first time). properties: @@ -183,7 +183,7 @@ spec: subpackageDir: description: |- `SubpackageDir` is the path to a subdirectory in an existing package revision - into which `Upstream` will be cloned as an independent subpackage. + into which `CloneFrom` will be cloned as an independent subpackage. type: string type: object copyFrom: @@ -272,8 +272,8 @@ spec: x-kubernetes-validations: - message: exactly one of init, cloneFrom, copyFrom, or upgrade must be set - rule: '[has(self.init), has(self.cloneFrom), has(self.copyFrom), - has(self.upgrade)].filter(x, x).size() == 1' + rule: '[has(self.init), has(self.clone), has(self.copyFrom), has(self.upgrade)].filter(x, + x).size() == 1' workspaceName: description: WorkspaceName is a short, unique description of the changes contained in this package revision. From 52e7f184f854b40a9ad5dc06e56adba47cc482dc Mon Sep 17 00:00:00 2001 From: liamfallon Date: Mon, 11 May 2026 17:17:36 +0100 Subject: [PATCH 06/15] Address CoPilot comments Signed-off-by: liamfallon --- api/porch/v1alpha2/packagerevision_types.go | 2 +- api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml | 4 ++-- api/porch/v1alpha2/source_types.go | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/api/porch/v1alpha2/packagerevision_types.go b/api/porch/v1alpha2/packagerevision_types.go index 4d266773a..20c77016c 100644 --- a/api/porch/v1alpha2/packagerevision_types.go +++ b/api/porch/v1alpha2/packagerevision_types.go @@ -204,7 +204,7 @@ type PackageRevisionStatus struct { // PackageSource specifies how a package was created. // Exactly one field must be set. -// +kubebuilder:validation:XValidation:rule="[has(self.init), has(self.clone), has(self.copyFrom), has(self.upgrade)].filter(x, x).size() == 1",message="exactly one of init, cloneFrom, copyFrom, or upgrade must be set" +// +kubebuilder:validation:XValidation:rule="[has(self.init), has(self.clone), has(self.copyFrom), has(self.upgrade)].filter(x, x).size() == 1",message="exactly one of init, clone, copyFrom, or upgrade must be set" type PackageSource struct { // Init creates a brand new package from scratch. Init *PackageInitSpec `json:"init,omitempty"` diff --git a/api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml b/api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml index db0c4dd60..ff6e665e5 100644 --- a/api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml +++ b/api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml @@ -270,8 +270,8 @@ spec: type: object type: object x-kubernetes-validations: - - message: exactly one of init, cloneFrom, copyFrom, or upgrade must - be set + - message: exactly one of init, clone, copyFrom, or upgrade must be + set rule: '[has(self.init), has(self.clone), has(self.copyFrom), has(self.upgrade)].filter(x, x).size() == 1' workspaceName: diff --git a/api/porch/v1alpha2/source_types.go b/api/porch/v1alpha2/source_types.go index 268908be6..09afba8c8 100644 --- a/api/porch/v1alpha2/source_types.go +++ b/api/porch/v1alpha2/source_types.go @@ -16,7 +16,7 @@ package v1alpha2 // Package creation source specifications. // In v1alpha2, the creation source is specified directly via PackageSource fields. -// Exactly one of Init, CloneFrom, CopyFrom, or Upgrade must be set when creating a PackageRevision. +// Exactly one of Init, clone, CopyFrom, or Upgrade must be set when creating a PackageRevision. // These fields are immutable after creation. // PackageInitSpec defines the package initialization parameters. From cfa820f0dbe815863ae5cb828b26350fff5f10be Mon Sep 17 00:00:00 2001 From: liamfallon Date: Mon, 11 May 2026 17:24:10 +0100 Subject: [PATCH 07/15] Address CoPilot comments Signed-off-by: liamfallon --- api/porch/v1alpha2/packagerevision_types.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/porch/v1alpha2/packagerevision_types.go b/api/porch/v1alpha2/packagerevision_types.go index 20c77016c..f254ed1f7 100644 --- a/api/porch/v1alpha2/packagerevision_types.go +++ b/api/porch/v1alpha2/packagerevision_types.go @@ -209,7 +209,7 @@ type PackageSource struct { // Init creates a brand new package from scratch. Init *PackageInitSpec `json:"init,omitempty"` - // CloneFrom copies a package from an upstream source (first time). + // Clone copies a package from an upstream source (first time). Clone *PackageCloneSpec `json:"clone,omitempty"` // CopyFrom creates a new revision from an existing package in the same repository. From 6e6b8a41e6fbba73c45f95562feb96fa2dabae70 Mon Sep 17 00:00:00 2001 From: liamfallon Date: Mon, 11 May 2026 17:27:30 +0100 Subject: [PATCH 08/15] Address CoPilot comments Signed-off-by: liamfallon --- api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml b/api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml index ff6e665e5..9d0468d85 100644 --- a/api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml +++ b/api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml @@ -127,8 +127,8 @@ spec: This field is immutable after creation. properties: clone: - description: CloneFrom copies a package from an upstream source - (first time). + description: Clone copies a package from an upstream source (first + time). properties: cloneFrom: description: '`cloneFrom` is the upstream package to clone.' From 04972aaf1baac3b9220f8b41329f0c9b649fb0b7 Mon Sep 17 00:00:00 2001 From: liamfallon Date: Mon, 11 May 2026 17:56:52 +0100 Subject: [PATCH 09/15] Address CoPilot comments Signed-off-by: liamfallon --- api/porch/v1alpha2/packagerevision_types.go | 1 + api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml | 2 ++ .../pkg/controllers/packagerevision/source.go | 4 ++-- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/api/porch/v1alpha2/packagerevision_types.go b/api/porch/v1alpha2/packagerevision_types.go index f254ed1f7..2aa29aa1e 100644 --- a/api/porch/v1alpha2/packagerevision_types.go +++ b/api/porch/v1alpha2/packagerevision_types.go @@ -205,6 +205,7 @@ type PackageRevisionStatus struct { // PackageSource specifies how a package was created. // Exactly one field must be set. // +kubebuilder:validation:XValidation:rule="[has(self.init), has(self.clone), has(self.copyFrom), has(self.upgrade)].filter(x, x).size() == 1",message="exactly one of init, clone, copyFrom, or upgrade must be set" +// +kubebuilder:validation:XValidation:rule="!has(self.clone) || has(self.clone.cloneFrom)",message="clone.cloneFrom must be set when clone is specified" type PackageSource struct { // Init creates a brand new package from scratch. Init *PackageInitSpec `json:"init,omitempty"` diff --git a/api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml b/api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml index 9d0468d85..6f858de84 100644 --- a/api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml +++ b/api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml @@ -274,6 +274,8 @@ spec: set rule: '[has(self.init), has(self.clone), has(self.copyFrom), has(self.upgrade)].filter(x, x).size() == 1' + - message: clone.cloneFrom must be set when clone is specified + rule: '!has(self.clone) || has(self.clone.cloneFrom)' workspaceName: description: WorkspaceName is a short, unique description of the changes contained in this package revision. diff --git a/controllers/packagerevisions/pkg/controllers/packagerevision/source.go b/controllers/packagerevisions/pkg/controllers/packagerevision/source.go index 8d90a84d5..afec5d5c1 100644 --- a/controllers/packagerevisions/pkg/controllers/packagerevision/source.go +++ b/controllers/packagerevisions/pkg/controllers/packagerevision/source.go @@ -118,12 +118,12 @@ func (r *PackageRevisionReconciler) copyPackage(ctx context.Context, pr *porchv1 // 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. +// Supports cloning from either an upstreamRef (registered repo) or a git source. func (r *PackageRevisionReconciler) clonePackage(ctx context.Context, pr *porchv1alpha2.PackageRevision) (map[string]string, error) { cloneFrom := pr.Spec.Source.Clone.CloneFrom if cloneFrom == nil { - return nil, fmt.Errorf("a clone source must be specified") + return nil, fmt.Errorf("a clone source must be specified at spec.source.clone.cloneFrom") } if cloneFrom.UpstreamRef != nil { From a814834740fd81a653b94cc23e15831198c5457c Mon Sep 17 00:00:00 2001 From: liamfallon Date: Tue, 12 May 2026 13:39:37 +0100 Subject: [PATCH 10/15] Addressed copilot comments Signed-off-by: liamfallon --- api/porch/v1alpha2/packagerevision_types.go | 1 + api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml | 4 ++++ api/porch/v1alpha2/source_types.go | 2 +- 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/api/porch/v1alpha2/packagerevision_types.go b/api/porch/v1alpha2/packagerevision_types.go index 2aa29aa1e..fb5e4382e 100644 --- a/api/porch/v1alpha2/packagerevision_types.go +++ b/api/porch/v1alpha2/packagerevision_types.go @@ -206,6 +206,7 @@ type PackageRevisionStatus struct { // Exactly one field must be set. // +kubebuilder:validation:XValidation:rule="[has(self.init), has(self.clone), has(self.copyFrom), has(self.upgrade)].filter(x, x).size() == 1",message="exactly one of init, clone, copyFrom, or upgrade must be set" // +kubebuilder:validation:XValidation:rule="!has(self.clone) || has(self.clone.cloneFrom)",message="clone.cloneFrom must be set when clone is specified" +// +kubebuilder:validation:XValidation:rule="!has(self.clone) || has(self.clone.cloneFrom.upstreamRef) || has(self.clone.cloneFrom.git)",message="clone.cloneFrom must specify either upstreamRef or git when clone is specified" type PackageSource struct { // Init creates a brand new package from scratch. Init *PackageInitSpec `json:"init,omitempty"` diff --git a/api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml b/api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml index 6f858de84..87d111463 100644 --- a/api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml +++ b/api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml @@ -276,6 +276,10 @@ spec: x).size() == 1' - message: clone.cloneFrom must be set when clone is specified rule: '!has(self.clone) || has(self.clone.cloneFrom)' + - message: clone.cloneFrom must specify either upstreamRef or git + when clone is specified + rule: '!has(self.clone) || has(self.clone.cloneFrom.upstreamRef) + || has(self.clone.cloneFrom.git)' workspaceName: description: WorkspaceName is a short, unique description of the changes contained in this package revision. diff --git a/api/porch/v1alpha2/source_types.go b/api/porch/v1alpha2/source_types.go index 09afba8c8..ffb18d10c 100644 --- a/api/porch/v1alpha2/source_types.go +++ b/api/porch/v1alpha2/source_types.go @@ -16,7 +16,7 @@ package v1alpha2 // Package creation source specifications. // In v1alpha2, the creation source is specified directly via PackageSource fields. -// Exactly one of Init, clone, CopyFrom, or Upgrade must be set when creating a PackageRevision. +// Exactly one of Init, Clone, CopyFrom, or Upgrade must be set when creating a PackageRevision. // These fields are immutable after creation. // PackageInitSpec defines the package initialization parameters. From 8cdac6a3818186c30714424c08adce8a906fc273 Mon Sep 17 00:00:00 2001 From: liamfallon Date: Wed, 13 May 2026 16:37:47 +0100 Subject: [PATCH 11/15] Rebased due to move to kptdev Signed-off-by: liamfallon --- api/porch/v1alpha2/packagerevision_types.go | 6 +++--- .../v1alpha2/porch.kpt.dev_packagerevisions.yaml | 11 +++++------ api/porch/v1alpha2/source_types.go | 2 +- api/porch/v1alpha2/util_test.go | 2 +- api/porch/v1alpha2/zz_generated.deepcopy.go | 4 ++-- .../pkg/controllers/packagerevision/source.go | 6 +++--- .../pkg/controllers/packagerevision/source_test.go | 10 +++++----- pkg/cli/commands/rpkg/copy/v1alpha2.go | 2 +- pkg/cli/commands/rpkg/upgrade/v1alpha2.go | 4 ++-- pkg/cli/commands/rpkg/upgrade/v1alpha2_test.go | 2 +- test/e2e/crd/copy_test.go | 8 ++++---- test/e2e/crd/helpers_test.go | 4 ++-- test/e2e/crd/lifecycle_test.go | 4 ++-- test/e2e/crd/validation_test.go | 2 +- 14 files changed, 33 insertions(+), 34 deletions(-) diff --git a/api/porch/v1alpha2/packagerevision_types.go b/api/porch/v1alpha2/packagerevision_types.go index fb5e4382e..785cebdc8 100644 --- a/api/porch/v1alpha2/packagerevision_types.go +++ b/api/porch/v1alpha2/packagerevision_types.go @@ -204,7 +204,7 @@ type PackageRevisionStatus struct { // PackageSource specifies how a package was created. // Exactly one field must be set. -// +kubebuilder:validation:XValidation:rule="[has(self.init), has(self.clone), has(self.copyFrom), has(self.upgrade)].filter(x, x).size() == 1",message="exactly one of init, clone, copyFrom, or upgrade must be set" +// +kubebuilder:validation:XValidation:rule="[has(self.init), has(self.clone), has(self.copy), has(self.upgrade)].filter(x, x).size() == 1",message="exactly one of init, clone, copy, or upgrade must be set" // +kubebuilder:validation:XValidation:rule="!has(self.clone) || has(self.clone.cloneFrom)",message="clone.cloneFrom must be set when clone is specified" // +kubebuilder:validation:XValidation:rule="!has(self.clone) || has(self.clone.cloneFrom.upstreamRef) || has(self.clone.cloneFrom.git)",message="clone.cloneFrom must specify either upstreamRef or git when clone is specified" type PackageSource struct { @@ -214,8 +214,8 @@ type PackageSource struct { // Clone copies a package from an upstream source (first time). Clone *PackageCloneSpec `json:"clone,omitempty"` - // CopyFrom creates a new revision from an existing package in the same repository. - CopyFrom *PackageRevisionRef `json:"copyFrom,omitempty"` + // Copy creates a new revision from an existing package in the same repository. + Copy *PackageRevisionRef `json:"copy,omitempty"` // Upgrade merges changes from a new upstream version into a local package. Upgrade *PackageUpgradeSpec `json:"upgrade,omitempty"` diff --git a/api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml b/api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml index 87d111463..a393cfc60 100644 --- a/api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml +++ b/api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml @@ -186,9 +186,9 @@ spec: into which `CloneFrom` will be cloned as an independent subpackage. type: string type: object - copyFrom: - description: CopyFrom creates a new revision from an existing - package in the same repository. + copy: + description: Copy creates a new revision from an existing package + in the same repository. properties: name: type: string @@ -270,9 +270,8 @@ spec: type: object type: object x-kubernetes-validations: - - message: exactly one of init, clone, copyFrom, or upgrade must be - set - rule: '[has(self.init), has(self.clone), has(self.copyFrom), has(self.upgrade)].filter(x, + - message: exactly one of init, clone, copy, or upgrade must be set + rule: '[has(self.init), has(self.clone), has(self.copy), has(self.upgrade)].filter(x, x).size() == 1' - message: clone.cloneFrom must be set when clone is specified rule: '!has(self.clone) || has(self.clone.cloneFrom)' diff --git a/api/porch/v1alpha2/source_types.go b/api/porch/v1alpha2/source_types.go index ffb18d10c..11662a6dd 100644 --- a/api/porch/v1alpha2/source_types.go +++ b/api/porch/v1alpha2/source_types.go @@ -16,7 +16,7 @@ package v1alpha2 // Package creation source specifications. // In v1alpha2, the creation source is specified directly via PackageSource fields. -// Exactly one of Init, Clone, CopyFrom, or Upgrade must be set when creating a PackageRevision. +// Exactly one of Init, Clone, Copy, or Upgrade must be set when creating a PackageRevision. // These fields are immutable after creation. // PackageInitSpec defines the package initialization parameters. diff --git a/api/porch/v1alpha2/util_test.go b/api/porch/v1alpha2/util_test.go index 69a30c6e2..71036ba31 100644 --- a/api/porch/v1alpha2/util_test.go +++ b/api/porch/v1alpha2/util_test.go @@ -121,7 +121,7 @@ func TestIsPackageCreation(t *testing.T) { {name: "nil source", source: nil, expected: false}, {name: "init", source: &PackageSource{Init: &PackageInitSpec{}}, expected: true}, {name: "clone", source: &PackageSource{Clone: &PackageCloneSpec{}}, expected: true}, - {name: "copy", source: &PackageSource{CopyFrom: &PackageRevisionRef{}}, expected: false}, + {name: "copy", source: &PackageSource{Copy: &PackageRevisionRef{}}, expected: false}, {name: "upgrade", source: &PackageSource{Upgrade: &PackageUpgradeSpec{}}, expected: false}, } diff --git a/api/porch/v1alpha2/zz_generated.deepcopy.go b/api/porch/v1alpha2/zz_generated.deepcopy.go index 8f2a22b4b..392f98a3e 100644 --- a/api/porch/v1alpha2/zz_generated.deepcopy.go +++ b/api/porch/v1alpha2/zz_generated.deepcopy.go @@ -316,8 +316,8 @@ func (in *PackageSource) DeepCopyInto(out *PackageSource) { *out = new(PackageCloneSpec) (*in).DeepCopyInto(*out) } - if in.CopyFrom != nil { - in, out := &in.CopyFrom, &out.CopyFrom + if in.Copy != nil { + in, out := &in.Copy, &out.Copy *out = new(PackageRevisionRef) **out = **in } diff --git a/controllers/packagerevisions/pkg/controllers/packagerevision/source.go b/controllers/packagerevisions/pkg/controllers/packagerevision/source.go index afec5d5c1..649b8f5bf 100644 --- a/controllers/packagerevisions/pkg/controllers/packagerevision/source.go +++ b/controllers/packagerevisions/pkg/controllers/packagerevision/source.go @@ -52,7 +52,7 @@ func (r *PackageRevisionReconciler) applySource(ctx context.Context, pr *porchv1 case pr.Spec.Source.Clone != nil: resources, err := r.clonePackage(ctx, pr) return resources, "clone", err - case pr.Spec.Source.CopyFrom != nil: + case pr.Spec.Source.Copy != nil: resources, err := r.copyPackage(ctx, pr) return resources, "copy", err case pr.Spec.Source.Upgrade != nil: @@ -85,11 +85,11 @@ func initPackage(ctx context.Context, pkgName string, spec *porchv1alpha2.Packag return readFsToMap(fs) } -// copyPackage reads the source package referenced by CopyFrom and returns its resources. +// copyPackage reads the source package referenced by Copy and returns its resources. // Validates the source is from the same repository and is published. func (r *PackageRevisionReconciler) copyPackage(ctx context.Context, pr *porchv1alpha2.PackageRevision) (map[string]string, error) { log := log.FromContext(ctx) - sourceRef := pr.Spec.Source.CopyFrom + sourceRef := pr.Spec.Source.Copy var sourcePR porchv1alpha2.PackageRevision if err := r.Get(ctx, client.ObjectKey{Namespace: pr.Namespace, Name: sourceRef.Name}, &sourcePR); err != nil { diff --git a/controllers/packagerevisions/pkg/controllers/packagerevision/source_test.go b/controllers/packagerevisions/pkg/controllers/packagerevision/source_test.go index e796ea146..24e4f2a9a 100644 --- a/controllers/packagerevisions/pkg/controllers/packagerevision/source_test.go +++ b/controllers/packagerevisions/pkg/controllers/packagerevision/source_test.go @@ -153,7 +153,7 @@ func TestApplySourceCopy(t *testing.T) { RepositoryName: "repo", WorkspaceName: "v2", Source: &porchv1alpha2.PackageSource{ - CopyFrom: &porchv1alpha2.PackageRevisionRef{Name: "repo.pkg.v1"}, + Copy: &porchv1alpha2.PackageRevisionRef{Name: "repo.pkg.v1"}, }, }, } @@ -183,7 +183,7 @@ func TestApplySourceCopyDifferentRepo(t *testing.T) { RepositoryName: "repo", WorkspaceName: "v2", Source: &porchv1alpha2.PackageSource{ - CopyFrom: &porchv1alpha2.PackageRevisionRef{Name: "other-repo.pkg.v1"}, + Copy: &porchv1alpha2.PackageRevisionRef{Name: "other-repo.pkg.v1"}, }, }, } @@ -212,7 +212,7 @@ func TestApplySourceCopyNotPublished(t *testing.T) { RepositoryName: "repo", WorkspaceName: "v2", Source: &porchv1alpha2.PackageSource{ - CopyFrom: &porchv1alpha2.PackageRevisionRef{Name: "repo.pkg.v1"}, + Copy: &porchv1alpha2.PackageRevisionRef{Name: "repo.pkg.v1"}, }, }, } @@ -235,7 +235,7 @@ func TestApplySourceCopyNotFound(t *testing.T) { RepositoryName: "repo", WorkspaceName: "v2", Source: &porchv1alpha2.PackageSource{ - CopyFrom: &porchv1alpha2.PackageRevisionRef{Name: "repo.pkg.v1"}, + Copy: &porchv1alpha2.PackageRevisionRef{Name: "repo.pkg.v1"}, }, }, } @@ -264,7 +264,7 @@ func TestApplySourceCopyDifferentPackageName(t *testing.T) { RepositoryName: "repo", WorkspaceName: "v2", Source: &porchv1alpha2.PackageSource{ - CopyFrom: &porchv1alpha2.PackageRevisionRef{Name: "repo.other-pkg.v1"}, + Copy: &porchv1alpha2.PackageRevisionRef{Name: "repo.other-pkg.v1"}, }, }, } diff --git a/pkg/cli/commands/rpkg/copy/v1alpha2.go b/pkg/cli/commands/rpkg/copy/v1alpha2.go index 3087fd76a..b3ebea2e6 100644 --- a/pkg/cli/commands/rpkg/copy/v1alpha2.go +++ b/pkg/cli/commands/rpkg/copy/v1alpha2.go @@ -94,7 +94,7 @@ func (r *v1alpha2Runner) runE(cmd *cobra.Command, _ []string) error { RepositoryName: source.Spec.RepositoryName, Lifecycle: porchv1alpha2.PackageRevisionLifecycleDraft, Source: &porchv1alpha2.PackageSource{ - CopyFrom: &porchv1alpha2.PackageRevisionRef{Name: r.sourceName}, + Copy: &porchv1alpha2.PackageRevisionRef{Name: r.sourceName}, }, }, } diff --git a/pkg/cli/commands/rpkg/upgrade/v1alpha2.go b/pkg/cli/commands/rpkg/upgrade/v1alpha2.go index 7083e4b26..1a0ad5bcc 100644 --- a/pkg/cli/commands/rpkg/upgrade/v1alpha2.go +++ b/pkg/cli/commands/rpkg/upgrade/v1alpha2.go @@ -290,8 +290,8 @@ func (r *v1alpha2Runner) findUpstreamName(pr *porchv1alpha2.PackageRevision) str return up.Name } return "" - case pr.Spec.Source.CopyFrom != nil: - if source := r.findPackageRevision(pr.Spec.Source.CopyFrom.Name); source != nil { + case pr.Spec.Source.Copy != nil: + if source := r.findPackageRevision(pr.Spec.Source.Copy.Name); source != nil { return r.findUpstreamName(source) } return "" diff --git a/pkg/cli/commands/rpkg/upgrade/v1alpha2_test.go b/pkg/cli/commands/rpkg/upgrade/v1alpha2_test.go index 583d3cab2..e0907e393 100644 --- a/pkg/cli/commands/rpkg/upgrade/v1alpha2_test.go +++ b/pkg/cli/commands/rpkg/upgrade/v1alpha2_test.go @@ -257,7 +257,7 @@ func TestV1Alpha2FindUpstreamName(t *testing.T) { Spec: porchv1alpha2.PackageRevisionSpec{ Lifecycle: porchv1alpha2.PackageRevisionLifecyclePublished, Source: &porchv1alpha2.PackageSource{ - CopyFrom: &porchv1alpha2.PackageRevisionRef{Name: "local.clone.v1"}, + Copy: &porchv1alpha2.PackageRevisionRef{Name: "local.clone.v1"}, }, }, } diff --git a/test/e2e/crd/copy_test.go b/test/e2e/crd/copy_test.go index c24814de1..322401092 100644 --- a/test/e2e/crd/copy_test.go +++ b/test/e2e/crd/copy_test.go @@ -36,7 +36,7 @@ var _ = Describe("Copy", Ordered, Label("lifecycle"), func() { publishPackage(env.Ctx, src) By("creating a copy to a new workspace") - dst := newPackageRevision(env.Namespace, env.RepoName, "copy-pkg", "v2", withCopyFrom(src.Name)) + dst := newPackageRevision(env.Namespace, env.RepoName, "copy-pkg", "v2", withCopy(src.Name)) Expect(k8sClient.Create(env.Ctx, dst)).To(Succeed()) waitForReady(env.Ctx, dst) @@ -58,7 +58,7 @@ var _ = Describe("Copy", Ordered, Label("lifecycle"), func() { publishPackage(env.Ctx, src) By("copying to v2 and publishing") - dst := newPackageRevision(env.Namespace, env.RepoName, "pub-copy", "v2", withCopyFrom(src.Name)) + dst := newPackageRevision(env.Namespace, env.RepoName, "pub-copy", "v2", withCopy(src.Name)) Expect(k8sClient.Create(env.Ctx, dst)).To(Succeed()) waitForReady(env.Ctx, dst) publishPackage(env.Ctx, dst) @@ -84,7 +84,7 @@ var _ = Describe("Copy", Ordered, Label("lifecycle"), func() { publishPackage(env.Ctx, src) By("attempting to copy into a different package name") - bad := newPackageRevision(env.Namespace, env.RepoName, "wrong-pkg", "v1", withCopyFrom(src.Name)) + bad := newPackageRevision(env.Namespace, env.RepoName, "wrong-pkg", "v1", withCopy(src.Name)) Expect(k8sClient.Create(env.Ctx, bad)).To(Succeed()) waitForReadyFalse(env.Ctx, bad) @@ -98,7 +98,7 @@ var _ = Describe("Copy", Ordered, Label("lifecycle"), func() { It("should fail copy from non-existent source", func() { By("attempting to copy from a source that doesn't exist") bad := newPackageRevision(env.Namespace, env.RepoName, "no-src", "v1", - withCopyFrom(crdName(env.RepoName, "no-src", "does-not-exist"))) + withCopy(crdName(env.RepoName, "no-src", "does-not-exist"))) Expect(k8sClient.Create(env.Ctx, bad)).To(Succeed()) By("verifying Ready=False") diff --git a/test/e2e/crd/helpers_test.go b/test/e2e/crd/helpers_test.go index 5f57dc3d1..b74f67840 100644 --- a/test/e2e/crd/helpers_test.go +++ b/test/e2e/crd/helpers_test.go @@ -332,10 +332,10 @@ func withCloneFromRef(upstreamCRDName string) prOption { } } -func withCopyFrom(sourceCRDName string) prOption { +func withCopy(sourceCRDName string) prOption { return func(pr *porchv1alpha2.PackageRevision) { pr.Spec.Source = &porchv1alpha2.PackageSource{ - CopyFrom: &porchv1alpha2.PackageRevisionRef{ + Copy: &porchv1alpha2.PackageRevisionRef{ Name: sourceCRDName, }, } diff --git a/test/e2e/crd/lifecycle_test.go b/test/e2e/crd/lifecycle_test.go index 8b41f62be..fe8376c53 100644 --- a/test/e2e/crd/lifecycle_test.go +++ b/test/e2e/crd/lifecycle_test.go @@ -274,7 +274,7 @@ var _ = Describe("Lifecycle", Ordered, Label("lifecycle"), func() { }).WithTimeout(defaultTimeout).Should(Succeed()) By("creating and publishing v2 (copy of v1)") - pr2 := newPackageRevision(env.Namespace, env.RepoName, "latest-pkg", "v2", withCopyFrom(pr1.Name)) + pr2 := newPackageRevision(env.Namespace, env.RepoName, "latest-pkg", "v2", withCopy(pr1.Name)) Expect(k8sClient.Create(env.Ctx, pr2)).To(Succeed()) waitForReady(env.Ctx, pr2) publishPackage(env.Ctx, pr2) @@ -324,7 +324,7 @@ var _ = Describe("Lifecycle", Ordered, Label("lifecycle"), func() { Expect(pr1.Status.Revision).To(Equal(1)) By("copying to v2 and publishing") - pr2 := newPackageRevision(env.Namespace, env.RepoName, "sub/folder/pkg", "v2", withCopyFrom(pr1.Name)) + pr2 := newPackageRevision(env.Namespace, env.RepoName, "sub/folder/pkg", "v2", withCopy(pr1.Name)) pr2.Name = crdName(env.RepoName, "sub.folder.pkg", "v2") Expect(k8sClient.Create(env.Ctx, pr2)).To(Succeed()) waitForReady(env.Ctx, pr2) diff --git a/test/e2e/crd/validation_test.go b/test/e2e/crd/validation_test.go index 7786e1ce6..d62cd55b8 100644 --- a/test/e2e/crd/validation_test.go +++ b/test/e2e/crd/validation_test.go @@ -42,7 +42,7 @@ var _ = Describe("Validation", Ordered, Label("infra"), func() { Init: &porchv1alpha2.PackageInitSpec{ Description: "should fail", }, - CopyFrom: &porchv1alpha2.PackageRevisionRef{ + Copy: &porchv1alpha2.PackageRevisionRef{ Name: "does-not-matter", }, } From 5e3e89908a26dadae97790a1369257a3f850cdbb Mon Sep 17 00:00:00 2001 From: liamfallon Date: Tue, 12 May 2026 16:23:12 +0100 Subject: [PATCH 12/15] Address copilot comments Signed-off-by: liamfallon --- .../pkg/controllers/packagerevision/source.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/controllers/packagerevisions/pkg/controllers/packagerevision/source.go b/controllers/packagerevisions/pkg/controllers/packagerevision/source.go index 649b8f5bf..d4a5f5612 100644 --- a/controllers/packagerevisions/pkg/controllers/packagerevision/source.go +++ b/controllers/packagerevisions/pkg/controllers/packagerevision/source.go @@ -116,8 +116,8 @@ func (r *PackageRevisionReconciler) copyPackage(ctx context.Context, pr *porchv1 return content.GetResourceContents(ctx) } -// clonePackage reads the source package referenced by CloneFrom and returns its resources -// with Kptfile upstream/upstreamLock updated. +// clonePackage reads the source package referenced by spec.source.clone.cloneFrom +// and returns its resources with Kptfile upstream/upstreamLock updated. // Supports cloning from either an upstreamRef (registered repo) or a git source. func (r *PackageRevisionReconciler) clonePackage(ctx context.Context, pr *porchv1alpha2.PackageRevision) (map[string]string, error) { cloneFrom := pr.Spec.Source.Clone.CloneFrom From 5906dad3e2f326f3917c75a486abc0beb17b681f Mon Sep 17 00:00:00 2001 From: liamfallon Date: Wed, 13 May 2026 06:36:23 +0100 Subject: [PATCH 13/15] Add small change to trigger CI on kptdev/porch Signed-off-by: liamfallon --- api/porch/v1alpha2/source_types.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/porch/v1alpha2/source_types.go b/api/porch/v1alpha2/source_types.go index 11662a6dd..dee5afc08 100644 --- a/api/porch/v1alpha2/source_types.go +++ b/api/porch/v1alpha2/source_types.go @@ -15,7 +15,7 @@ package v1alpha2 // Package creation source specifications. -// In v1alpha2, the creation source is specified directly via PackageSource fields. +// In v1alpha2, the creation source is specified directly using PackageSource fields. // Exactly one of Init, Clone, Copy, or Upgrade must be set when creating a PackageRevision. // These fields are immutable after creation. From 54d9ceecaa381fb5ca89533bd37c648b9ab7e325 Mon Sep 17 00:00:00 2001 From: liamfallon Date: Wed, 13 May 2026 06:55:17 +0100 Subject: [PATCH 14/15] Address copilot comments Signed-off-by: liamfallon --- api/porch/v1alpha2/packagerevision_types.go | 1 + 1 file changed, 1 insertion(+) diff --git a/api/porch/v1alpha2/packagerevision_types.go b/api/porch/v1alpha2/packagerevision_types.go index 785cebdc8..a643dc1cb 100644 --- a/api/porch/v1alpha2/packagerevision_types.go +++ b/api/porch/v1alpha2/packagerevision_types.go @@ -207,6 +207,7 @@ type PackageRevisionStatus struct { // +kubebuilder:validation:XValidation:rule="[has(self.init), has(self.clone), has(self.copy), has(self.upgrade)].filter(x, x).size() == 1",message="exactly one of init, clone, copy, or upgrade must be set" // +kubebuilder:validation:XValidation:rule="!has(self.clone) || has(self.clone.cloneFrom)",message="clone.cloneFrom must be set when clone is specified" // +kubebuilder:validation:XValidation:rule="!has(self.clone) || has(self.clone.cloneFrom.upstreamRef) || has(self.clone.cloneFrom.git)",message="clone.cloneFrom must specify either upstreamRef or git when clone is specified" +// +kubebuilder:validation:XValidation:rule="!has(self.clone) || (has(self.clone.cloneFrom.upstreamRef) && !has(self.clone.cloneFrom.git)) || (!has(self.clone.cloneFrom.upstreamRef) && has(self.clone.cloneFrom.git))",message="clone.cloneFrom must specify exactly one of upstreamRef or git when clone is specified" type PackageSource struct { // Init creates a brand new package from scratch. Init *PackageInitSpec `json:"init,omitempty"` From 50c9952bc7bff2853d4c217f323bd8b2a1079641 Mon Sep 17 00:00:00 2001 From: liamfallon Date: Wed, 13 May 2026 17:47:27 +0100 Subject: [PATCH 15/15] Address CoPilot comments Signed-off-by: liamfallon --- api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml b/api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml index a393cfc60..602a897f8 100644 --- a/api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml +++ b/api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml @@ -279,6 +279,11 @@ spec: when clone is specified rule: '!has(self.clone) || has(self.clone.cloneFrom.upstreamRef) || has(self.clone.cloneFrom.git)' + - message: clone.cloneFrom must specify exactly one of upstreamRef + or git when clone is specified + rule: '!has(self.clone) || (has(self.clone.cloneFrom.upstreamRef) + && !has(self.clone.cloneFrom.git)) || (!has(self.clone.cloneFrom.upstreamRef) + && has(self.clone.cloneFrom.git))' workspaceName: description: WorkspaceName is a short, unique description of the changes contained in this package revision.