From fe48b17f1e8e61dc27b4d37eef3aff6fefb734a4 Mon Sep 17 00:00:00 2001 From: Wendy Arango Date: Thu, 12 Jan 2023 13:50:13 -0500 Subject: [PATCH] feat: Allow users to unset git flags by passing them as empty string Signed-off-by: Wendy Arango --- .../cartographer/v1alpha1/workload_helpers.go | 28 +- .../cartographer/v1alpha1/workload_test.go | 35 ++- pkg/commands/workload.go | 51 ++- pkg/commands/workload_apply_test.go | 293 ++++++++++++++++++ pkg/commands/workload_test.go | 67 ++++ 5 files changed, 445 insertions(+), 29 deletions(-) diff --git a/pkg/apis/cartographer/v1alpha1/workload_helpers.go b/pkg/apis/cartographer/v1alpha1/workload_helpers.go index 4417ccc1f..cfdcaf528 100644 --- a/pkg/apis/cartographer/v1alpha1/workload_helpers.go +++ b/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 } } } diff --git a/pkg/apis/cartographer/v1alpha1/workload_test.go b/pkg/apis/cartographer/v1alpha1/workload_test.go index e8e6f45ad..549fbf65d 100644 --- a/pkg/apis/cartographer/v1alpha1/workload_test.go +++ b/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 { diff --git a/pkg/commands/workload.go b/pkg/commands/workload.go index 5305471f3..e192cf696 100644 --- a/pkg/commands/workload.go +++ b/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 diff --git a/pkg/commands/workload_apply_test.go b/pkg/commands/workload_apply_test.go index 384167399..c4b510ac9 100644 --- a/pkg/commands/workload_apply_test.go +++ b/pkg/commands/workload_apply_test.go @@ -254,6 +254,12 @@ To get status: "tanzu apps workload get my-workload" `, }, + { + Name: "git source without git repo", + Args: []string{workloadName, flags.GitRepoFlagName, "", flags.GitBranchFlagName, gitBranch, flags.SubPathFlagName, "./app", flags.YesFlagName}, + GivenObjects: givenNamespaceDefault, + ShouldError: true, + }, { Name: "create git source with invalid namespace", Args: []string{workloadName, flags.GitRepoFlagName, gitRepo, flags.GitBranchFlagName, gitBranch, flags.NamespaceFlagName, "foo", flags.YesFlagName}, @@ -3578,6 +3584,293 @@ To get status: "tanzu apps workload get my-workload" To see logs: "tanzu apps workload tail my-workload --timestamp --since 1h" To get status: "tanzu apps workload get my-workload" +`, + }, + { + Name: "update - remove git tag by setting to empty in flags", + Args: []string{workloadName, flags.GitTagFlagName, "", flags.YesFlagName}, + GivenObjects: []client.Object{ + parent. + SpecDie( + func(d *diecartov1alpha1.WorkloadSpecDie) { + d.Source(&cartov1alpha1.Source{ + Git: &cartov1alpha1.GitSource{ + URL: "https://github.com/sample-accelerators/spring-petclinic", + Ref: cartov1alpha1.GitRef{ + Branch: "main", + Tag: "v0.1.0", + }, + }, + }) + }), + }, + ExpectUpdates: []client.Object{ + &cartov1alpha1.Workload{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: defaultNamespace, + Name: workloadName, + }, + Spec: cartov1alpha1.WorkloadSpec{ + Source: &cartov1alpha1.Source{ + Git: &cartov1alpha1.GitSource{ + URL: "https://github.com/sample-accelerators/spring-petclinic", + Ref: cartov1alpha1.GitRef{ + Branch: "main", + }, + }, + }, + }, + }, + }, + ExpectOutput: ` +🔎 Update workload: +... + 8, 8 | source: + 9, 9 | git: + 10, 10 | ref: + 11, 11 | branch: main + 12 - | tag: v0.1.0 + 13, 12 | url: https://github.com/sample-accelerators/spring-petclinic +👍 Updated workload "my-workload" + +To see logs: "tanzu apps workload tail my-workload --timestamp --since 1h" +To get status: "tanzu apps workload get my-workload" + +`, + }, + { + Name: "update - remove git branch by setting to empty in flags", + Args: []string{workloadName, flags.GitBranchFlagName, "", flags.YesFlagName}, + GivenObjects: []client.Object{ + parent. + SpecDie( + func(d *diecartov1alpha1.WorkloadSpecDie) { + d.Source(&cartov1alpha1.Source{ + Git: &cartov1alpha1.GitSource{ + URL: "https://github.com/sample-accelerators/spring-petclinic", + Ref: cartov1alpha1.GitRef{ + Branch: "main", + Tag: "v0.1.0", + }, + }, + }) + }), + }, + ExpectUpdates: []client.Object{ + &cartov1alpha1.Workload{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: defaultNamespace, + Name: workloadName, + }, + Spec: cartov1alpha1.WorkloadSpec{ + Source: &cartov1alpha1.Source{ + Git: &cartov1alpha1.GitSource{ + URL: "https://github.com/sample-accelerators/spring-petclinic", + Ref: cartov1alpha1.GitRef{ + Tag: "v0.1.0", + }, + }, + }, + }, + }, + }, + ExpectOutput: ` +🔎 Update workload: +... + 7, 7 |spec: + 8, 8 | source: + 9, 9 | git: + 10, 10 | ref: + 11 - | branch: main + 12, 11 | tag: v0.1.0 + 13, 12 | url: https://github.com/sample-accelerators/spring-petclinic +👍 Updated workload "my-workload" + +To see logs: "tanzu apps workload tail my-workload --timestamp --since 1h" +To get status: "tanzu apps workload get my-workload" + +`, + }, + { + Name: "update - remove git commit by setting to empty in flags", + Args: []string{workloadName, flags.GitCommitFlagName, "", flags.YesFlagName}, + GivenObjects: []client.Object{ + parent. + SpecDie( + func(d *diecartov1alpha1.WorkloadSpecDie) { + d.Source(&cartov1alpha1.Source{ + Git: &cartov1alpha1.GitSource{ + URL: "https://github.com/sample-accelerators/spring-petclinic", + Ref: cartov1alpha1.GitRef{ + Branch: "main", + Commit: "abc1234", + }, + }, + }) + }), + }, + ExpectUpdates: []client.Object{ + &cartov1alpha1.Workload{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: defaultNamespace, + Name: workloadName, + }, + Spec: cartov1alpha1.WorkloadSpec{ + Source: &cartov1alpha1.Source{ + Git: &cartov1alpha1.GitSource{ + URL: "https://github.com/sample-accelerators/spring-petclinic", + Ref: cartov1alpha1.GitRef{ + Branch: "main", + }, + }, + }, + }, + }, + }, + ExpectOutput: ` +🔎 Update workload: +... + 8, 8 | source: + 9, 9 | git: + 10, 10 | ref: + 11, 11 | branch: main + 12 - | commit: abc1234 + 13, 12 | url: https://github.com/sample-accelerators/spring-petclinic +👍 Updated workload "my-workload" + +To see logs: "tanzu apps workload tail my-workload --timestamp --since 1h" +To get status: "tanzu apps workload get my-workload" + +`, + }, + { + Name: "update - change from image to git source", + Args: []string{workloadName, flags.GitRepoFlagName, gitRepo, flags.GitBranchFlagName, gitBranch, flags.YesFlagName}, + GivenObjects: []client.Object{ + parent. + SpecDie( + func(d *diecartov1alpha1.WorkloadSpecDie) { + d.Image("private.repo.domain.com/spring-pet-clinic") + }), + }, + ExpectUpdates: []client.Object{ + &cartov1alpha1.Workload{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: defaultNamespace, + Name: workloadName, + }, + Spec: cartov1alpha1.WorkloadSpec{ + Source: &cartov1alpha1.Source{ + Git: &cartov1alpha1.GitSource{ + URL: "https://example.com/repo.git", + Ref: cartov1alpha1.GitRef{ + Branch: "main", + }, + }, + }, + }, + }, + }, + ExpectOutput: ` +🔎 Update workload: +... + 4, 4 |metadata: + 5, 5 | name: my-workload + 6, 6 | namespace: default + 7, 7 |spec: + 8 - | image: private.repo.domain.com/spring-pet-clinic + 8 + | source: + 9 + | git: + 10 + | ref: + 11 + | branch: main + 12 + | url: https://example.com/repo.git +👍 Updated workload "my-workload" + +To see logs: "tanzu apps workload tail my-workload --timestamp --since 1h" +To get status: "tanzu apps workload get my-workload" + +`, + }, + { + Name: "update - change from image to git source with error", + Args: []string{workloadName, flags.GitBranchFlagName, gitBranch, flags.YesFlagName}, + GivenObjects: []client.Object{ + parent. + SpecDie( + func(d *diecartov1alpha1.WorkloadSpecDie) { + d.Image("private.repo.domain.com/spring-pet-clinic") + }), + }, + ShouldError: true, + }, + { + Name: "update - set all git.ref fields to empty", + Args: []string{workloadName, flags.GitBranchFlagName, "", flags.GitCommitFlagName, "", flags.GitTagFlagName, "", flags.YesFlagName}, + GivenObjects: []client.Object{ + parent. + SpecDie( + func(d *diecartov1alpha1.WorkloadSpecDie) { + d.Source(&cartov1alpha1.Source{ + Git: &cartov1alpha1.GitSource{ + URL: "https://github.com/sample-accelerators/spring-petclinic", + Ref: cartov1alpha1.GitRef{ + Branch: "main", + Commit: "abc1234", + Tag: "v0.1.0", + }, + }, + }) + }), + }, + ShouldError: true, + }, + { + Name: "update - unset source by setting git repo to empty", + Args: []string{workloadName, flags.GitRepoFlagName, "", flags.YesFlagName}, + GivenObjects: []client.Object{ + parent. + SpecDie( + func(d *diecartov1alpha1.WorkloadSpecDie) { + d.Source(&cartov1alpha1.Source{ + Git: &cartov1alpha1.GitSource{ + URL: "https://github.com/sample-accelerators/spring-petclinic", + Ref: cartov1alpha1.GitRef{ + Branch: "main", + Commit: "abc1234", + }, + }, + }) + }), + }, + ExpectUpdates: []client.Object{ + &cartov1alpha1.Workload{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: defaultNamespace, + Name: workloadName, + }, + }, + }, + ExpectOutput: ` +🔎 Update workload: +... + 3, 3 |kind: Workload + 4, 4 |metadata: + 5, 5 | name: my-workload + 6, 6 | namespace: default + 7 - |spec: + 8 - | source: + 9 - | git: + 10 - | ref: + 11 - | branch: main + 12 - | commit: abc1234 + 13 - | url: https://github.com/sample-accelerators/spring-petclinic + 7 + |spec: {} +❗ NOTICE: no source code or image has been specified for this workload. +👍 Updated workload "my-workload" + +To see logs: "tanzu apps workload tail my-workload --timestamp --since 1h" +To get status: "tanzu apps workload get my-workload" + `, }, { diff --git a/pkg/commands/workload_test.go b/pkg/commands/workload_test.go index 8820aa4b2..0d0314cf4 100644 --- a/pkg/commands/workload_test.go +++ b/pkg/commands/workload_test.go @@ -1061,6 +1061,73 @@ func TestWorkloadOptionsApplyOptionsToWorkload(t *testing.T) { }, }, }, + { + name: "set git flags to empty", + args: []string{flags.GitBranchFlagName, "", flags.GitCommitFlagName, ""}, + input: &cartov1alpha1.Workload{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: defaultNamespace, + Name: workloadName, + }, + Spec: cartov1alpha1.WorkloadSpec{ + Source: &cartov1alpha1.Source{ + Git: &cartov1alpha1.GitSource{ + URL: gitRepo, + Ref: cartov1alpha1.GitRef{ + Branch: gitBranch, + Tag: gitTag, + Commit: gitCommit, + }, + }, + }, + }, + }, + expected: &cartov1alpha1.Workload{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: defaultNamespace, + Name: workloadName, + }, + Spec: cartov1alpha1.WorkloadSpec{ + Source: &cartov1alpha1.Source{ + Git: &cartov1alpha1.GitSource{ + URL: gitRepo, + Ref: cartov1alpha1.GitRef{ + Tag: gitTag, + }, + }, + }, + }, + }, + }, + { + name: "set git repo to empty", + args: []string{flags.GitRepoFlagName, ""}, + input: &cartov1alpha1.Workload{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: defaultNamespace, + Name: workloadName, + }, + Spec: cartov1alpha1.WorkloadSpec{ + Source: &cartov1alpha1.Source{ + Git: &cartov1alpha1.GitSource{ + URL: gitRepo, + Ref: cartov1alpha1.GitRef{ + Branch: gitBranch, + Tag: gitTag, + Commit: gitCommit, + }, + }, + }, + }, + }, + expected: &cartov1alpha1.Workload{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: defaultNamespace, + Name: workloadName, + }, + Spec: cartov1alpha1.WorkloadSpec{}, + }, + }, { name: "subPath update with image source", args: []string{flags.SubPathFlagName, subPath},