removed strategy from clone task (#371)

Signed-off-by: lapentafd <francesco.lapenta@est.tech>

Author: lapentad

api/generated/openapi/zz_generated.openapi.go

@@ -199,16 +199,6 @@ type PackageCloneTaskSpec struct {
 
 	// `Upstream` is the reference to the upstream package to clone.
 	Upstream UpstreamPackage `json:"upstreamRef,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.
-	//  * fast-forward: Fail without updating if the local package was modified
-	//    since it was fetched.
-	//  * force-delete-replace: Wipe all the local changes to the package and replace
-	//    it with the remote version.
-	//  * copy-merge: Copy all the remote changes to the local package.
-	Strategy PackageMergeStrategy `json:"strategy,omitempty"`
 }
 
 type PackageMergeStrategy string

api/porch/types.go

@@ -200,16 +200,6 @@ type PackageCloneTaskSpec struct {
 
 	// `Upstream` is the reference to the upstream package to clone.
 	Upstream UpstreamPackage `json:"upstreamRef,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.
-	//  * fast-forward: Fail without updating if the local package was modified
-	//    since it was fetched.
-	//  * force-delete-replace: Wipe all the local changes to the package and replace
-	//    it with the remote version.
-	//  * copy-merge: Copy all the remote changes to the local package.
-	Strategy PackageMergeStrategy `json:"strategy,omitempty"`
 }
 
 type PackageMergeStrategy string

api/porch/v1alpha1/types.go

@@ -35,14 +35,7 @@ const (
 	command = "cmdrpkgclone"
 )
 
-var (
-	strategies = []string{
-		string(porchapi.ResourceMerge),
-		string(porchapi.FastForward),
-		string(porchapi.ForceDeleteReplace),
-		string(porchapi.CopyMerge),
-	}
-)
+
 
 func NewCommand(ctx context.Context, rcg *genericclioptions.ConfigFlags) *cobra.Command {
 	return newRunner(ctx, rcg).Command
@@ -64,8 +57,6 @@ func newRunner(ctx context.Context, rcg *genericclioptions.ConfigFlags) *runner
 	}
 	r.Command = c
 
-	c.Flags().StringVar(&r.strategy, "strategy", string(porchapi.ResourceMerge),
-		"update strategy that should be used when updating this package; one of: "+strings.Join(strategies, ","))
 	c.Flags().StringVar(&r.directory, "directory", "", "Directory within the repository where the upstream package is located.")
 	c.Flags().StringVar(&r.ref, "ref", "", "Branch in the repository where the upstream package is located.")
 	c.Flags().StringVar(&r.repository, "repository", "", "Repository to which package will be cloned (downstream repository).")
@@ -84,7 +75,6 @@ type runner struct {
 	clone porchapi.PackageCloneTaskSpec
 
 	// Flags
-	strategy   string
 	directory  string
 	ref        string
 	repository string // Target repository
@@ -101,12 +91,6 @@ func (r *runner) preRunE(_ *cobra.Command, args []string) error {
 	}
 	r.client = client
 
-	mergeStrategy, err := toMergeStrategy(r.strategy)
-	if err != nil {
-		return errors.E(op, err)
-	}
-	r.clone.Strategy = mergeStrategy
-
 	if len(args) < 2 {
 		return errors.E(op, fmt.Errorf("SOURCE_PACKAGE and NAME are required positional arguments; %d provided", len(args)))
 	}
@@ -218,18 +202,3 @@ func (r *runner) runE(cmd *cobra.Command, _ []string) error {
 	fmt.Fprintf(cmd.OutOrStdout(), "%s created\n", pr.Name)
 	return nil
 }
-
-func toMergeStrategy(strategy string) (porchapi.PackageMergeStrategy, error) {
-	switch strategy {
-	case string(porchapi.ResourceMerge):
-		return porchapi.ResourceMerge, nil
-	case string(porchapi.FastForward):
-		return porchapi.FastForward, nil
-	case string(porchapi.ForceDeleteReplace):
-		return porchapi.ForceDeleteReplace, nil
-	case string(porchapi.CopyMerge):
-		return porchapi.CopyMerge, nil
-	default:
-		return "", fmt.Errorf("invalid strategy: %q", strategy)
-	}
-}

api/porch/v1alpha1/zz_generated.conversion.go

@@ -114,32 +114,6 @@ func TestCmd(t *testing.T) {
 	}
 }
 
-func TestToMergeStrategy(t *testing.T) {
-	tests := []struct {
-		input    string
-		expected porchapi.PackageMergeStrategy
-		hasError bool
-	}{
-		{input: string(porchapi.ResourceMerge), expected: porchapi.ResourceMerge, hasError: false},
-		{input: string(porchapi.FastForward), expected: porchapi.FastForward, hasError: false},
-		{input: string(porchapi.ForceDeleteReplace), expected: porchapi.ForceDeleteReplace, hasError: false},
-		{input: string(porchapi.CopyMerge), expected: porchapi.CopyMerge, hasError: false},
-		{input: "invalid-strategy", expected: "", hasError: true},
-	}
-
-	for _, test := range tests {
-		t.Run(test.input, func(t *testing.T) {
-			result, err := toMergeStrategy(test.input)
-			if test.hasError {
-				assert.Error(t, err)
-			} else {
-				assert.NoError(t, err)
-				assert.Equal(t, test.expected, result)
-			}
-		})
-	}
-}
-
 func TestPreRunE(t *testing.T) {
 	tests := []struct {
 		name      string
@@ -150,7 +124,7 @@ func TestPreRunE(t *testing.T) {
 		{
 			name:      "Missing arguments",
 			args:      []string{"source-package"},
-			flags:     map[string]string{"repository": "test-repo", "workspace": "test-workspace", "strategy": "copy-merge"},
+			flags:     map[string]string{"repository": "test-repo", "workspace": "test-workspace"},
 			expectErr: true,
 		},
 		{
@@ -159,12 +133,6 @@ func TestPreRunE(t *testing.T) {
 			flags:     map[string]string{"repository": "", "workspace": ""},
 			expectErr: true,
 		},
-		{
-			name:      "Invalid strategy",
-			args:      []string{"source-package", "target-package"},
-			flags:     map[string]string{"repository": "test-repo", "workspace": "test-workspace", "strategy": "invalid-strategy"},
-			expectErr: true,
-		},
 	}
 
 	for _, test := range tests {
@@ -173,7 +141,6 @@ func TestPreRunE(t *testing.T) {
 			r := &runner{
 				ctx:        context.Background(),
 				cfg:        &genericclioptions.ConfigFlags{},
-				strategy:   test.flags["strategy"],
 				repository: test.flags["repository"],
 				workspace:  test.flags["workspace"],
 			}

pkg/cli/commands/rpkg/clone/command.go

@@ -76,25 +76,15 @@ Flags:
   --workspace
     The workspace name assigned to the new downstream package revision. The default value is v1.
 
-  --strategy
-    Update strategy that should be used when updating the new
-    package revision. Must be one of:
-    =resource-merge: (Default) Perform a structural comparison of the original
-      updated resources, and merge the changes into the local package.
-    =fast-forward: Fail without updating if the local package was modified since it was fetched.
-    =force-delete-replace: Wipe all the local changes to the package and replace
-      it with the remote version.
-    =copy-merge: Copy all the remote changes to the local package.
-
   --secret-ref
     Name of the secret containing basic authentication used to authenticate with the upstream repository (git-only).
     Naturally, this secret has to exist in the kubernetes cluster and must be in the namespace
     where the package revision is to be created.
 `
 var CloneExamples = `
   # clone the 'example-repo.example-package-name.example-workspace' package and create a new package revision called
-  # 'example-package-name-2' in the 'example-repo-2' repository with a new workspace named 'example-workspace-2' and 'force-delete-replace' update strategy.
-  $ porchctl rpkg clone example-repo.example-package-name.example-workspace example-package-name-2 --repository=example-repo-2 --workspace=example-workspace-2 --strategy=force-delete-replace
+  # 'example-package-name-2' in the 'example-repo-2' repository with a new workspace named 'example-workspace-2'.
+  $ porchctl rpkg clone example-repo.example-package-name.example-workspace example-package-name-2 --repository=example-repo-2 --workspace=example-workspace-2
 
   # clone the git repository at 'https://github.com/repo/blueprint.git' at reference 'base/v0' and in directory base. The new
   # package revision will be created in repository 'blueprint' and namespace 'default'.

pkg/cli/commands/rpkg/clone/command_test.go

@@ -470,7 +470,6 @@ func TestCreateCloneTaskValidation(t *testing.T) {
 				{
 					Type: porchapi.TaskTypeClone,
 					Clone: &porchapi.PackageCloneTaskSpec{
-						Strategy: porchapi.ResourceMerge,
 						Upstream: porchapi.UpstreamPackage{
 							Type: porchapi.RepositoryTypeGit,
 							Git: &porchapi.GitPackage{

pkg/cli/commands/rpkg/docs/docs.go

@@ -296,7 +296,6 @@ func TestUpdateStrategy(t *testing.T) {
 										Directory: "/path/to/package",
 									},
 								},
-								Strategy: "copy-merge",
 							},
 						},
 					},

pkg/engine/engine_test.go

@@ -42,7 +42,6 @@ type genericTaskHandler struct {
 	repoOpener                 repository.RepositoryOpener
 	credentialResolver         repository.CredentialResolver
 	referenceResolver          repository.ReferenceResolver
-	cloneStrategy              porchapi.PackageMergeStrategy
 	repoOperationRetryAttempts int
 }
 
@@ -79,11 +78,6 @@ func (th *genericTaskHandler) ApplyTask(ctx context.Context, draft repository.Pa
 		return pkgerrors.New("task list must contain exactly 1 task")
 	}
 
-	if cloneTask := obj.Spec.Tasks[0].Clone; cloneTask != nil {
-		klog.Infof("Clone strategy is %s", cloneTask.Strategy)
-		th.cloneStrategy = cloneTask.Strategy
-	}
-
 	mut, err := th.mapTaskToMutation(obj, &obj.Spec.Tasks[0], repositoryObj.Spec.Deployment, packageConfig)
 	if err != nil {
 		return err