Repository: GoogleContainerTools/kpt
Pull request #3130 (2022-05-11 19:17:35)

simplify rpkg copy: disallow editing outside repository and package name changes
Simplify `kpt alpha rpkg copy`
- allow copying only within repository
- disallow package name change (so `copy` only creates new revision of the same package)
- added a TODO for defaulting the revision number to latest+1 which I will tackle in a followup PR. I had some questions about this task, which I left as a comment in the issue filed. 

Before:
`kpt alpha rpkg copy deployments-11ca1db650fa4bfa33deeb7f488fbdc50cdb3b82 foo --namespace default --repository deployments --revision v1`

After: 
`kpt alpha rpkg copy deployments-11ca1db650fa4bfa33deeb7f488fbdc50cdb3b82 --namespace default --revision v1`

PR to update docs: https://github.com/GoogleContainerTools/kpt/pull/3135

mortent: Other than the small issues mentioned by @yuwenma, this looks good to me.

Patch:
diff --git a/go.mod b/go.mod
index bb63ab7f..d5982ab0 100644
--- a/go.mod
+++ b/go.mod
@@ -58,11 +58,17 @@ require (
 	github.com/go-openapi/jsonreference v0.19.6 // indirect
 	github.com/go-openapi/swag v0.21.1 // indirect
 	github.com/gogo/protobuf v1.3.2 // indirect
+	github.com/golang/glog v1.0.0 // indirect
 	github.com/golang/protobuf v1.5.2 // indirect
 	github.com/google/btree v1.0.1 // indirect
 	github.com/google/gnostic v0.5.7-v3refs // indirect
 	github.com/google/gofuzz v1.1.0 // indirect
 	github.com/google/uuid v1.3.0 // indirect
+	github.com/gorilla/context v0.0.0-20160226214623-1ea25387ff6f // indirect
+	github.com/gorilla/mux v1.8.0 // indirect
+	github.com/gorilla/securecookie v0.0.0-20160422134519-667fe4e3466a // indirect
+	github.com/gorilla/sessions v0.0.0-20160922145804-ca9ada445741 // indirect
+	github.com/gorilla/websocket v1.4.2 // indirect
 	github.com/gregjones/httpcache v0.0.0-20180305231024-9cad4c3443a7 // indirect
 	github.com/imdario/mergo v0.3.12 // indirect
 	github.com/inconshreveable/mousetrap v1.0.0 // indirect
@@ -79,6 +85,7 @@ require (
 	github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
 	github.com/modern-go/reflect2 v1.0.2 // indirect
 	github.com/monochromegane/go-gitignore v0.0.0-20200626010858-205db1a8cc00 // indirect
+	github.com/monopole/mdrip v1.0.2 // indirect
 	github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
 	github.com/onsi/gomega v1.17.0 // indirect
 	github.com/peterbourgon/diskv v2.0.1+incompatible // indirect
diff --git a/go.sum b/go.sum
index e1c2e83f..dd1ea8f2 100644
--- a/go.sum
+++ b/go.sum
@@ -244,6 +244,7 @@ github.com/gogo/protobuf v1.3.1/go.mod h1:SlYgWuQ5SjCEi6WLHjHCa1yvBfUnHcTbrrZtXP
 github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q=
 github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69NZV8Q=
 github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q=
+github.com/golang/glog v1.0.0 h1:nfP3RFugxnNRyKgeWd4oI1nYvXpxrx8ck8ZrcizshdQ=
 github.com/golang/glog v1.0.0/go.mod h1:EWib/APOK0SL3dFbYqvxE3UYd8E6s1ouQ7iEp/0LWV4=
 github.com/golang/groupcache v0.0.0-20190129154638-5b532d6fd5ef/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc=
 github.com/golang/groupcache v0.0.0-20190702054246-869f871628b6/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc=
@@ -337,8 +338,18 @@ github.com/googleapis/gax-go/v2 v2.1.1/go.mod h1:hddJymUZASv3XPyGkUpKj8pPO47Rmb0
 github.com/googleapis/gnostic v0.5.1/go.mod h1:6U4PtQXGIEt/Z3h5MAT7FNofLnw9vXk2cUuW7uA/OeU=
 github.com/googleapis/gnostic v0.5.5/go.mod h1:7+EbHbldMins07ALC74bsA81Ovc97DwqyJO1AENw9kA=
 github.com/gopherjs/gopherjs v0.0.0-20181017120253-0766667cb4d1/go.mod h1:wJfORRmW1u3UXTncJ5qlYoELFm8eSnnEO6hX4iZ3EWY=
+github.com/gorilla/context v0.0.0-20160226214623-1ea25387ff6f h1:9oNbS1z4rVpbnkHBdPZU4jo9bSmrLpII768arSyMFgk=
+github.com/gorilla/context v0.0.0-20160226214623-1ea25387ff6f/go.mod h1:kBGZzfjB9CEq2AlWe17Uuf7NDRt0dE0s8S51q0aT7Yg=
+github.com/gorilla/mux v1.6.0/go.mod h1:1lud6UwP+6orDFRuTfBEV8e9/aOM/c4fVVCaMa2zaAs=
+github.com/gorilla/mux v1.8.0 h1:i40aqfkR1h2SlN9hojwV5ZA91wcXFOvkdNIeFDP5koI=
 github.com/gorilla/mux v1.8.0/go.mod h1:DVbg23sWSpFRCP0SfiEN6jmj59UnW/n46BH5rLB71So=
+github.com/gorilla/securecookie v0.0.0-20160422134519-667fe4e3466a h1:YH0IojQwndMQdeRWdw1aPT8bkbiWaYR3WD+Zf5e09DU=
+github.com/gorilla/securecookie v0.0.0-20160422134519-667fe4e3466a/go.mod h1:ra0sb63/xPlUeL+yeDciTfxMRAA+MP+HVt/4epWDjd4=
+github.com/gorilla/sessions v0.0.0-20160922145804-ca9ada445741 h1:OuuPl66BpF1q3OEkaPpp+VfzxrBBY62ATGdWqql/XX8=
+github.com/gorilla/sessions v0.0.0-20160922145804-ca9ada445741/go.mod h1:+WVp8kdw6VhyKExm03PAMRn2ZxnPtm58pV0dBVPdhHE=
+github.com/gorilla/websocket v1.2.0/go.mod h1:E7qHFY5m1UJ88s3WnNqhKjPHQ0heANvMoAMk2YaljkQ=
 github.com/gorilla/websocket v1.4.0/go.mod h1:E7qHFY5m1UJ88s3WnNqhKjPHQ0heANvMoAMk2YaljkQ=
+github.com/gorilla/websocket v1.4.2 h1:+/TMaTYc4QFitKJxsQ7Yye35DkWvkdLcvGKqM+x0Ufc=
 github.com/gorilla/websocket v1.4.2/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE=
 github.com/gregjones/httpcache v0.0.0-20180305231024-9cad4c3443a7 h1:pdN6V1QBWetyv/0+wjACpqVH+eVULgEjkurDLq3goeM=
 github.com/gregjones/httpcache v0.0.0-20180305231024-9cad4c3443a7/go.mod h1:FecbI9+v66THATjSRHfNgh1IVFe/9kFxbXtjV0ctIMA=
@@ -488,6 +499,8 @@ github.com/modern-go/reflect2 v1.0.2 h1:xBagoLtFs94CBntxluKeaWgTMpvLxC4ur3nMaC9G
 github.com/modern-go/reflect2 v1.0.2/go.mod h1:yWuevngMOJpCy52FWWMvUC8ws7m/LJsjYzDa0/r8luk=
 github.com/monochromegane/go-gitignore v0.0.0-20200626010858-205db1a8cc00 h1:n6/2gBQ3RWajuToeY6ZtZTIKv2v7ThUy5KKusIT0yc0=
 github.com/monochromegane/go-gitignore v0.0.0-20200626010858-205db1a8cc00/go.mod h1:Pm3mSP3c5uWn86xMLZ5Sa7JB9GsEZySvHYXCTK4E9q4=
+github.com/monopole/mdrip v1.0.2 h1:0ycJ0vJuBisqo5ULXbZZHbHk7ehOXmG/6M/YsFAs1ZA=
+github.com/monopole/mdrip v1.0.2/go.mod h1:KMw1TEew7vaYuLAExOcvdU9tx3t/DR6ngxFJzWXux/0=
 github.com/munnerz/goautoneg v0.0.0-20120707110453-a547fc61f48d/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ=
 github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 h1:C3w9PqII01/Oq1c1nUAm88MOHcQC9l5mIlSMApZMrHA=
 github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ=
diff --git a/internal/cmdrpkgcopy/command.go b/internal/cmdrpkgcopy/command.go
index c1289e3c..1b800172 100644
--- a/internal/cmdrpkgcopy/command.go
+++ b/internal/cmdrpkgcopy/command.go
@@ -24,6 +24,7 @@ import (
 	porchapi "github.com/GoogleContainerTools/kpt/porch/api/porch/v1alpha1"
 	"github.com/spf13/cobra"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/cli-runtime/pkg/genericclioptions"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
@@ -52,9 +53,8 @@ func newRunner(ctx context.Context, rcg *genericclioptions.ConfigFlags) *runner
 		Hidden:  porch.HidePorchCommands,
 	}
 	r.Command = c
-
-	c.Flags().StringVar(&r.repository, "repository", "", "Repository in which the copy will be created.")
-	c.Flags().StringVar(&r.revision, "revision", "v1", "Revision of the copied package.")
+	// TODO (natasha41575): Make the default "latest+1"
+	c.Flags().StringVar(&r.revision, "revision", "", "Revision of the copied package.")
 
 	return r
 }
@@ -67,9 +67,7 @@ type runner struct {
 
 	copy porchapi.PackageEditTaskSpec
 
-	repository string // Target repository
-	revision   string // Target package revision
-	target     string // Target package name
+	revision string // Target package revision
 }
 
 func (r *runner) preRunE(cmd *cobra.Command, args []string) error {
@@ -80,29 +78,32 @@ func (r *runner) preRunE(cmd *cobra.Command, args []string) error {
 	}
 	r.client = client
 
-	if len(args) < 2 {
-		return errors.E(op, fmt.Errorf("SOURCE_PACKAGE and NAME are required positional arguments; %d provided", len(args)))
+	if len(args) < 1 {
+		return errors.E(op, fmt.Errorf("SOURCE_PACKAGE is a required positional argument"))
 	}
-
-	if r.repository == "" {
-		return errors.E(op, fmt.Errorf("--repository is required to specify target repository"))
+	if len(args) > 1 {
+		return errors.E(op, fmt.Errorf("too many arguments; SOURCE_PACKAGE is the only accepted positional arguments"))
 	}
 
-	source := args[0]
-	target := args[1]
+	// TODO(natasha41575): This is temporarily required until we can set a default value for the revision. Now that we are disallowing
+	//   package name changes or editing outside the repository, the copy needs to have a new revision number.
+	if r.revision == "" {
+		return errors.E(op, fmt.Errorf("--revision is a required flag"))
+	}
 
 	r.copy.Source = &porchapi.PackageRevisionRef{
-		Name: source,
+		Name: args[0],
 	}
-
-	r.target = target
-
 	return nil
 }
 
 func (r *runner) runE(cmd *cobra.Command, args []string) error {
 	const op errors.Op = command + ".runE"
 
+	revisionSpec, err := r.getPackageRevisionSpec()
+	if err != nil {
+		return errors.E(op, err)
+	}
 	pr := &porchapi.PackageRevision{
 		TypeMeta: metav1.TypeMeta{
 			Kind:       "PackageRevision",
@@ -111,22 +112,43 @@ func (r *runner) runE(cmd *cobra.Command, args []string) error {
 		ObjectMeta: metav1.ObjectMeta{
 			Namespace: *r.cfg.Namespace,
 		},
-		Spec: porchapi.PackageRevisionSpec{
-			PackageName:    r.target,
-			Revision:       r.revision,
-			RepositoryName: r.repository,
-			Tasks: []porchapi.Task{
-				{
-					Type: porchapi.TaskTypeEdit,
-					Edit: &r.copy,
-				},
-			},
-		},
+		Spec: *revisionSpec,
 	}
 	if err := r.client.Create(r.ctx, pr); err != nil {
 		return errors.E(op, err)
 	}
-
 	fmt.Fprintf(cmd.OutOrStdout(), "%s created", pr.Name)
 	return nil
 }
+
+func (r *runner) getPackageRevisionSpec() (*porchapi.PackageRevisionSpec, error) {
+	newScheme := runtime.NewScheme()
+	if err := porchapi.SchemeBuilder.AddToScheme(newScheme); err != nil {
+		return nil, err
+	}
+	restClient, err := porch.CreateRESTClient(r.cfg)
+	if err != nil {
+		return nil, err
+	}
+
+	result := porchapi.PackageRevision{}
+	err = restClient.
+		Get().
+		Namespace(*r.cfg.Namespace).
+		Resource("packagerevisions").
+		Name(r.copy.Source.Name).
+		Do(r.ctx).
+		Into(&result)
+	if err != nil {
+		return nil, err
+	}
+
+	// TODO(natasha41575): Set a default revision of "latest + 1"
+	spec := &porchapi.PackageRevisionSpec{
+		PackageName:    result.Spec.PackageName,
+		Revision:       r.revision,
+		RepositoryName: result.Spec.RepositoryName,
+	}
+	spec.Tasks = []porchapi.Task{{Type: porchapi.TaskTypeEdit, Edit: &r.copy}}
+	return spec, nil
+}
diff --git a/porch/go.sum b/porch/go.sum
index 623f593c..5c2e7546 100644
--- a/porch/go.sum
+++ b/porch/go.sum
@@ -584,10 +584,15 @@ github.com/googleapis/gnostic v0.4.1/go.mod h1:LRhVm6pbyptWbWbuZ38d1eyptfvIytN3i
 github.com/googleapis/gnostic v0.5.1/go.mod h1:6U4PtQXGIEt/Z3h5MAT7FNofLnw9vXk2cUuW7uA/OeU=
 github.com/googleapis/gnostic v0.5.5/go.mod h1:7+EbHbldMins07ALC74bsA81Ovc97DwqyJO1AENw9kA=
 github.com/gopherjs/gopherjs v0.0.0-20181017120253-0766667cb4d1/go.mod h1:wJfORRmW1u3UXTncJ5qlYoELFm8eSnnEO6hX4iZ3EWY=
+github.com/gorilla/context v0.0.0-20160226214623-1ea25387ff6f/go.mod h1:kBGZzfjB9CEq2AlWe17Uuf7NDRt0dE0s8S51q0aT7Yg=
 github.com/gorilla/handlers v0.0.0-20150720190736-60c7bfde3e33/go.mod h1:Qkdc/uu4tH4g6mTK6auzZ766c4CA0Ng8+o/OAirnOIQ=
+github.com/gorilla/mux v1.6.0/go.mod h1:1lud6UwP+6orDFRuTfBEV8e9/aOM/c4fVVCaMa2zaAs=
 github.com/gorilla/mux v1.7.2/go.mod h1:1lud6UwP+6orDFRuTfBEV8e9/aOM/c4fVVCaMa2zaAs=
 github.com/gorilla/mux v1.8.0/go.mod h1:DVbg23sWSpFRCP0SfiEN6jmj59UnW/n46BH5rLB71So=
+github.com/gorilla/securecookie v0.0.0-20160422134519-667fe4e3466a/go.mod h1:ra0sb63/xPlUeL+yeDciTfxMRAA+MP+HVt/4epWDjd4=
+github.com/gorilla/sessions v0.0.0-20160922145804-ca9ada445741/go.mod h1:+WVp8kdw6VhyKExm03PAMRn2ZxnPtm58pV0dBVPdhHE=
 github.com/gorilla/websocket v0.0.0-20170926233335-4201258b820c/go.mod h1:E7qHFY5m1UJ88s3WnNqhKjPHQ0heANvMoAMk2YaljkQ=
+github.com/gorilla/websocket v1.2.0/go.mod h1:E7qHFY5m1UJ88s3WnNqhKjPHQ0heANvMoAMk2YaljkQ=
 github.com/gorilla/websocket v1.4.0/go.mod h1:E7qHFY5m1UJ88s3WnNqhKjPHQ0heANvMoAMk2YaljkQ=
 github.com/gorilla/websocket v1.4.2 h1:+/TMaTYc4QFitKJxsQ7Yye35DkWvkdLcvGKqM+x0Ufc=
 github.com/gorilla/websocket v1.4.2/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE=
@@ -783,6 +788,7 @@ github.com/mohae/deepcopy v0.0.0-20170929034955-c48cc78d4826 h1:RWengNIwukTxcDr9
 github.com/mohae/deepcopy v0.0.0-20170929034955-c48cc78d4826/go.mod h1:TaXosZuwdSHYgviHp1DAtfrULt5eUgsSMsZf+YrPgl8=
 github.com/monochromegane/go-gitignore v0.0.0-20200626010858-205db1a8cc00 h1:n6/2gBQ3RWajuToeY6ZtZTIKv2v7ThUy5KKusIT0yc0=
 github.com/monochromegane/go-gitignore v0.0.0-20200626010858-205db1a8cc00/go.mod h1:Pm3mSP3c5uWn86xMLZ5Sa7JB9GsEZySvHYXCTK4E9q4=
+github.com/monopole/mdrip v1.0.2/go.mod h1:KMw1TEew7vaYuLAExOcvdU9tx3t/DR6ngxFJzWXux/0=
 github.com/morikuni/aec v1.0.0/go.mod h1:BbKIizmSmc5MMPqRYbxO4ZU0S0+P200+tUnFx7PXmsc=
 github.com/mrunalp/fileutils v0.5.0/go.mod h1:M1WthSahJixYnrXQl/DFQuteStB1weuxD2QJNHXfbSQ=
 github.com/munnerz/goautoneg v0.0.0-20120707110453-a547fc61f48d/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ=
diff --git a/porch/pkg/engine/edit.go b/porch/pkg/engine/edit.go
index 212970eb..6d96e1d6 100644
--- a/porch/pkg/engine/edit.go
+++ b/porch/pkg/engine/edit.go
@@ -19,14 +19,12 @@ import (
 	"fmt"
 
 	api "github.com/GoogleContainerTools/kpt/porch/api/porch/v1alpha1"
-	"github.com/GoogleContainerTools/kpt/porch/pkg/kpt"
 	"github.com/GoogleContainerTools/kpt/porch/pkg/repository"
 	"go.opentelemetry.io/otel/trace"
 )
 
 type editPackageMutation struct {
 	task              *api.Task
-	name              string
 	namespace         string
 	cad               CaDEngine
 	referenceResolver ReferenceResolver
@@ -48,11 +46,6 @@ func (m *editPackageMutation) Apply(ctx context.Context, resources repository.Pa
 		return repository.PackageResources{}, nil, fmt.Errorf("failed to fetch resources for package %q: %w", sourceRef.Name, err)
 	}
 
-	// Update Kptfile
-	if err := kpt.UpdateKptfileName(m.name, sourceResources.Spec.Resources); err != nil {
-		return repository.PackageResources{}, nil, fmt.Errorf("failed to update package name %q: %w", sourceRef.Name, err)
-	}
-
 	return repository.PackageResources{
 		Contents: sourceResources.Spec.Resources,
 	}, &api.Task{}, nil
diff --git a/porch/pkg/engine/engine.go b/porch/pkg/engine/engine.go
index aab55621..695908db 100644
--- a/porch/pkg/engine/engine.go
+++ b/porch/pkg/engine/engine.go
@@ -193,7 +193,6 @@ func (cad *cadEngine) mapTaskToMutation(ctx context.Context, obj *api.PackageRev
 		return &editPackageMutation{
 			task:              task,
 			namespace:         obj.Namespace,
-			name:              obj.Spec.PackageName,
 			cad:               cad,
 			referenceResolver: cad.referenceResolver,
 		}, nil


Test patch:
diff --git a/e2e/testdata/porch/rpkg-edit/config.yaml b/e2e/testdata/porch/rpkg-copy/config.yaml
similarity index 84%
rename from e2e/testdata/porch/rpkg-edit/config.yaml
rename to e2e/testdata/porch/rpkg-copy/config.yaml
index 9bcc3bb4..7f0f6cff 100644
--- a/e2e/testdata/porch/rpkg-edit/config.yaml
+++ b/e2e/testdata/porch/rpkg-copy/config.yaml
@@ -3,14 +3,14 @@ commands:
       - alpha
       - repo
       - register
-      - --namespace=rpkg-edit
+      - --namespace=rpkg-copy
       - --name=git
       - http://git-server.test-git-namespace.svc.cluster.local:8080
   - args:
       - alpha
       - rpkg
       - clone
-      - --namespace=rpkg-edit
+      - --namespace=rpkg-copy
       - https://github.com/platkrm/test-blueprints.git
       - --directory=basens
       - --ref=basens/v1
@@ -21,18 +21,17 @@ commands:
   - args:
       - alpha
       - rpkg
-      - edit
-      - --namespace=rpkg-edit
-      - --repository=git
+      - copy
+      - --namespace=rpkg-copy
+      - --revision=v1
       - git-f1fbc2b72ee70b2f34e64c2630031d0560e6cf48
-      - basens-new-edit
-    stdout: git-85f4237aefaf996af6ce27fe0993d1ccb9a8c9ef created
+    stdout: git-66579ccd78cf67f80a3cafaf63dc121db4d92b83 created
   - args:
       - alpha
       - rpkg
       - pull
-      - --namespace=rpkg-edit
-      - git-85f4237aefaf996af6ce27fe0993d1ccb9a8c9ef
+      - --namespace=rpkg-copy
+      - git-66579ccd78cf67f80a3cafaf63dc121db4d92b83
     stdout: |
       apiVersion: config.kubernetes.io/v1
       items:
@@ -46,7 +45,7 @@ commands:
             config.kubernetes.io/path: Kptfile
             internal.config.kubernetes.io/index: "0"
             internal.config.kubernetes.io/path: Kptfile
-          name: basens-new-edit
+          name: basens-edit
         upstream:
           git:
             directory: basens
diff --git a/porch/pkg/engine/edit_test.go b/porch/pkg/engine/edit_test.go
index 4ef3b315..ec5b4e83 100644
--- a/porch/pkg/engine/edit_test.go
+++ b/porch/pkg/engine/edit_test.go
@@ -70,7 +70,6 @@ info:
 			},
 		},
 		namespace:         "test-namespace",
-		name:              "test-package",
 		referenceResolver: &fakeReferenceResolver{},
 		cad:               cad,
 	}
@@ -84,7 +83,7 @@ info:
 apiVersion: kpt.dev/v1
 kind: Kptfile
 metadata:
-  name: test-package
+  name: example
   annotations:
     config.kubernetes.io/local-config: "true"
 info: