Allow users to unset git flags by passing them as empty string

pkg/apis/cartographer/v1alpha1/workload_helpers.go

@@ -327,24 +327,22 @@ func (w *WorkloadSpec) ResetSource() {
 
 func (w *WorkloadSpec) MergeGit(git GitSource) {
 	stash := w.Source
+	image := w.Image
 	w.ResetSource()
 
-	w.Source = &Source{
-		Git: &git,
-	}
-	if stash != nil && stash.Git != nil {
-		w.Source.Subpath = stash.Subpath
-		if w.Source.Git.URL == "" {
-			w.Source.Git.URL = stash.Git.URL
-		}
-		if w.Source.Git.Ref.Branch == "" {
-			w.Source.Git.Ref.Branch = stash.Git.Ref.Branch
-		}
-		if w.Source.Git.Ref.Tag == "" {
-			w.Source.Git.Ref.Tag = stash.Git.Ref.Tag
+	// if workload already has a source, when changing to git source it needs to be validated
+	// that there's a git url
+	sourceExists := (stash != nil && stash.Image != "") || image != ""
+	// if workload is brand new, to set git source it needs to be validated that
+	// git repo was set
+	isNewWorkload := git.URL == "" && stash == nil
+
+	if git.URL != "" || sourceExists || isNewWorkload {
+		w.Source = &Source{
+			Git: &git,
 		}
-		if w.Source.Git.Ref.Commit == "" {
-			w.Source.Git.Ref.Commit = stash.Git.Ref.Commit
+		if stash != nil && stash.Git != nil {
+			w.Source.Subpath = stash.Subpath
 		}
 	}
 }

pkg/apis/cartographer/v1alpha1/workload_test.go

@@ -1435,6 +1435,10 @@ func TestWorkloadSpec_MergeGit(t *testing.T) {
 		},
 		git: GitSource{
 			URL: "git@github.com:example/repo.git",
+			Ref: GitRef{
+				Branch: "main",
+				Tag:    "v1.0.0",
+			},
 		},
 		want: &WorkloadSpec{
 			Source: &Source{
@@ -1461,6 +1465,7 @@ func TestWorkloadSpec_MergeGit(t *testing.T) {
 			},
 		},
 		git: GitSource{
+			URL: "git@github.com:example/repo.git",
 			Ref: GitRef{
 				Tag: "v1.0.1",
 			},
@@ -1470,8 +1475,7 @@ func TestWorkloadSpec_MergeGit(t *testing.T) {
 				Git: &GitSource{
 					URL: "git@github.com:example/repo.git",
 					Ref: GitRef{
-						Branch: "main",
-						Tag:    "v1.0.1",
+						Tag: "v1.0.1",
 					},
 				},
 			},
@@ -1491,7 +1495,10 @@ func TestWorkloadSpec_MergeGit(t *testing.T) {
 			},
 		},
 		git: GitSource{
+			URL: "git@github.com:example/repo.git",
 			Ref: GitRef{
+				Branch: "main",
+				Tag:    "v1.0.0",
 				Commit: "efgh5678",
 			},
 		},
@@ -1525,6 +1532,8 @@ func TestWorkloadSpec_MergeGit(t *testing.T) {
 			URL: "git@github.com:example/repo.git",
 			Ref: GitRef{
 				Branch: "my-new-branch",
+				Tag:    "v1.0.0",
+				Commit: "abcd1234",
 			},
 		},
 		want: &WorkloadSpec{
@@ -1580,13 +1589,15 @@ func TestWorkloadSpec_MergeGit(t *testing.T) {
 				Git: &GitSource{
 					URL: "git@github.com:example/repo.git",
 					Ref: GitRef{
-						Tag: "v1.0.0",
+						Branch: "my-bad-branch",
+						Tag:    "v1.0.0",
 					},
 				},
 				Subpath: "my-subpath",
 			},
 		},
 		git: GitSource{
+			URL: "git@github.com:example/repo.git",
 			Ref: GitRef{
 				Branch: "main",
 			},
@@ -1596,7 +1607,6 @@ func TestWorkloadSpec_MergeGit(t *testing.T) {
 				Git: &GitSource{
 					URL: "git@github.com:example/repo.git",
 					Ref: GitRef{
-						Tag:    "v1.0.0",
 						Branch: "main",
 					},
 				},
@@ -1627,6 +1637,23 @@ func TestWorkloadSpec_MergeGit(t *testing.T) {
 				},
 			},
 		},
+	}, {
+		name: "delete source when setting repo to empty string",
+		seed: &WorkloadSpec{
+			Source: &Source{
+				Git: &GitSource{
+					URL: "git@github.com:example/repo.git",
+					Ref: GitRef{
+						Branch: "main",
+					},
+				},
+				Subpath: "my-subpath",
+			},
+		},
+		git: GitSource{
+			URL: "",
+		},
+		want: &WorkloadSpec{},
 	}}
 
 	for _, test := range tests {

pkg/commands/workload.go

@@ -289,16 +289,7 @@ func (opts *WorkloadOptions) ApplyOptionsToWorkload(ctx context.Context, workloa
 		workload.Spec.RemoveParam("live-update")
 	}
 
-	if opts.GitRepo != "" || opts.GitBranch != "" || opts.GitCommit != "" || opts.GitTag != "" {
-		workload.Spec.MergeGit(cartov1alpha1.GitSource{
-			URL: opts.GitRepo,
-			Ref: cartov1alpha1.GitRef{
-				Branch: opts.GitBranch,
-				Commit: opts.GitCommit,
-				Tag:    opts.GitTag,
-			},
-		})
-	}
+	opts.checkGitValues(ctx, workload)
 
 	if opts.SourceImage != "" {
 		workload.Spec.MergeSourceImage(opts.SourceImage)
@@ -391,6 +382,46 @@ func (opts *WorkloadOptions) ApplyOptionsToWorkload(ctx context.Context, workloa
 	return ctx
 }
 
+func (opts *WorkloadOptions) checkGitValues(ctx context.Context, workload *cartov1alpha1.Workload) {
+	isGitSource := false
+	var gitRepo, gitBranch, gitCommit, gitTag string
+
+	if workload != nil && workload.Spec.Source != nil && workload.Spec.Source.Git != nil {
+		gitRepo = workload.Spec.Source.Git.URL
+		gitBranch = workload.Spec.Source.Git.Ref.Branch
+		gitCommit = workload.Spec.Source.Git.Ref.Commit
+		gitTag = workload.Spec.Source.Git.Ref.Tag
+	}
+
+	if cli.CommandFromContext(ctx).Flags().Changed(cli.StripDash(flags.GitRepoFlagName)) {
+		isGitSource = true
+		gitRepo = opts.GitRepo
+	}
+	if cli.CommandFromContext(ctx).Flags().Changed(cli.StripDash(flags.GitBranchFlagName)) {
+		isGitSource = true
+		gitBranch = opts.GitBranch
+	}
+	if cli.CommandFromContext(ctx).Flags().Changed(cli.StripDash(flags.GitCommitFlagName)) {
+		isGitSource = true
+		gitCommit = opts.GitCommit
+	}
+	if cli.CommandFromContext(ctx).Flags().Changed(cli.StripDash(flags.GitTagFlagName)) {
+		isGitSource = true
+		gitTag = opts.GitTag
+	}
+
+	if isGitSource {
+		workload.Spec.MergeGit(cartov1alpha1.GitSource{
+			URL: gitRepo,
+			Ref: cartov1alpha1.GitRef{
+				Branch: gitBranch,
+				Commit: gitCommit,
+				Tag:    gitTag,
+			},
+		})
+	}
+}
+
 // PublishLocalSource packages the specified source code in the --local-path flag and creates an image
 // that will be eventually published to the registry specified in the --source-image flag.
 // Returns a boolean that indicates if user does actually want to publish the image and an error in case of failure