fix:Adds validations to check multiple sources via flags on workload… (#568)

* fix: Adds validations to check multiple sources via flags on workload create or update

* fix: review comments

Author: shaheerkootteeri

pkg/apis/cartographer/v1alpha1/zz_generated.deepcopy.go

@@ -17,6 +17,9 @@ limitations under the License.
 package validation
 
 import (
+	"fmt"
+	"strings"
+
 	k8sfield "k8s.io/apimachinery/pkg/util/validation/field"
 )
 
@@ -25,3 +28,9 @@ func ErrMissingFieldWithDetail(field string, detail string) FieldErrors {
 		k8sfield.Required(k8sfield.NewPath(field), detail),
 	}
 }
+
+func ErrMultipleSources(names ...string) FieldErrors {
+	return FieldErrors{
+		k8sfield.Required(k8sfield.NewPath(fmt.Sprintf("[%s]", strings.Join(names, ", "))), "expected exactly one, got multiple"),
+	}
+}

pkg/cli-runtime/validation/fielderrors.go

@@ -17,6 +17,8 @@ limitations under the License.
 package validation_test
 
 import (
+	"fmt"
+	"strings"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -56,3 +58,41 @@ func TestErrMissingFieldWithDetail(t *testing.T) {
 		})
 	}
 }
+
+func TestErrMultipleSources(t *testing.T) {
+	tests := []struct {
+		testName string
+		sources  []string
+		expected validation.FieldErrors
+	}{
+		{
+			testName: "multiple sources",
+			expected: validation.FieldErrors{
+				k8sfield.Required(k8sfield.NewPath(fmt.Sprintf("[%s]", strings.Join([]string{flags.GitFlagWildcard, flags.ImageFlagName}, ", "))), "expected exactly one, got multiple"),
+			},
+			sources: append([]string{}, flags.GitFlagWildcard, flags.ImageFlagName),
+		}, {
+			testName: "single source",
+			expected: validation.FieldErrors{
+				k8sfield.Required(k8sfield.NewPath(fmt.Sprintf("[%s]", strings.Join([]string{flags.GitFlagWildcard}, ", "))), "expected exactly one, got multiple"),
+			},
+			sources: append([]string{}, flags.GitFlagWildcard),
+		}, {
+			testName: "Empty source",
+			expected: validation.FieldErrors{
+				k8sfield.Required(k8sfield.NewPath(fmt.Sprintf("[%s]", strings.Join([]string{}, ", "))), "expected exactly one, got multiple"),
+			},
+			sources: []string{},
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.testName, func(t *testing.T) {
+			expected := test.expected
+			actual := validation.ErrMultipleSources(test.sources...)
+			if diff := cmp.Diff(expected, actual); diff != "" {
+				t.Errorf("%s() = (-expected, +actual): %s", test.testName, diff)
+			}
+		})
+	}
+}

pkg/cli-runtime/validation/fielderrors_test.go

@@ -64,6 +64,9 @@ const (
 	AnnotationReservedKey     = "annotations"
 	MavenOverwrittenNoticeMsg = "Maven configuration flags have overwritten values provided by \"--params-yaml\"."
 	WebTypeReservedKey        = "web"
+	MavenFlagWildcard         = "--maven*"
+	// --source-image can be a source for workload without local path and vice versa.
+	LocalPathAndSource = "--local-path with/without --source-image"
 )
 
 const (
@@ -153,6 +156,8 @@ type WorkloadOptions struct {
 
 func (opts *WorkloadOptions) Validate(ctx context.Context) validation.FieldErrors {
 	errs := validation.FieldErrors{}
+	var mavenSource bool
+	sources := []string{}
 
 	errs = errs.Also(validation.K8sName(opts.Namespace, flags.NamespaceFlagName))
 	if opts.FilePath == "" {
@@ -200,6 +205,36 @@ func (opts *WorkloadOptions) Validate(ctx context.Context) validation.FieldError
 		errs = errs.Also(validation.Enum(opts.Output, flags.OutputFlagName, []string{printer.OutputFormatJson, printer.OutputFormatYaml, printer.OutputFormatYml}))
 	}
 
+	// validating sources as the source options are mutually exclusive
+	if opts.MavenArtifact != "" || opts.MavenVersion != "" || opts.MavenGroup != "" || opts.MavenType != "" {
+		mavenSource = true
+	}
+	if !mavenSource {
+		for _, p := range opts.ParamsYaml {
+			kv := parsers.DeletableKeyValue(p)
+			if len(kv) != 1 {
+				if kv[0] == cartov1alpha1.WorkloadMavenParam {
+					mavenSource = true
+					break
+				}
+			}
+		}
+	}
+	if mavenSource {
+		sources = append(sources, MavenFlagWildcard)
+	}
+	if opts.LocalPath != "" || opts.SourceImage != "" {
+		sources = append(sources, LocalPathAndSource)
+	}
+	if opts.Image != "" {
+		sources = append(sources, flags.ImageFlagName)
+	}
+	if opts.GitRepo != "" || opts.GitBranch != "" || opts.GitCommit != "" || opts.GitTag != "" {
+		sources = append(sources, flags.GitFlagWildcard)
+	}
+	if len(sources) > 1 {
+		errs = errs.Also(validation.ErrMultipleSources(sources...))
+	}
 	return errs
 }
 

pkg/commands/workload.go

@@ -145,6 +145,23 @@ func TestWorkloadApplyOptionsValidate(t *testing.T) {
 			},
 			ExpectFieldErrors: validation.EnumInvalidValue("invalid", flags.UpdateStrategyFlagName, []string{"merge", "replace"}),
 		},
+		{
+			Name: "apply with multiple sources",
+			Validatable: &commands.WorkloadApplyOptions{
+				WorkloadOptions: commands.WorkloadOptions{
+					Namespace:     "default",
+					Name:          "my-resource",
+					GitRepo:       "https://example.com/repo.git",
+					GitBranch:     "main",
+					SourceImage:   "repo.example/image:tag",
+					Image:         "repo.example/image:tag",
+					MavenArtifact: "hello-world",
+					MavenType:     "jar",
+					MavenVersion:  "0.0.1",
+				},
+			},
+			ExpectFieldErrors: validation.ErrMultipleSources(commands.MavenFlagWildcard, commands.LocalPathAndSource, flags.ImageFlagName, flags.GitFlagWildcard),
+		},
 	}
 
 	table.Run(t)

pkg/commands/workload_apply_test.go

@@ -92,6 +92,22 @@ func TestWorkloadCreateOptionsValidate(t *testing.T) {
 			},
 			ExpectFieldErrors: validation.ErrInvalidArrayValue("FOO", flags.BuildEnvFlagName, 0),
 		},
+		{
+			Name: "apply with multiple sources",
+			Validatable: &commands.WorkloadCreateOptions{
+				WorkloadOptions: commands.WorkloadOptions{
+					Namespace:     "default",
+					Name:          "my-resource",
+					GitRepo:       "https://example.com/repo.git",
+					GitBranch:     "main",
+					SourceImage:   "repo.example/image:tag",
+					MavenArtifact: "hello-world",
+					MavenType:     "jar",
+					MavenVersion:  "0.0.1",
+				},
+			},
+			ExpectFieldErrors: validation.ErrMultipleSources(commands.MavenFlagWildcard, commands.LocalPathAndSource, flags.GitFlagWildcard),
+		},
 	}
 
 	table.Run(t)

pkg/commands/workload_create_test.go

@@ -435,7 +435,24 @@ func TestWorkloadOptionsValidate(t *testing.T) {
 				SourceImage: "repo.example/image:tag",
 				Image:       "repo.example/image:tag",
 			},
-			ShouldValidate: true,
+			ShouldValidate:    false,
+			ExpectFieldErrors: validation.ErrMultipleSources(commands.LocalPathAndSource, flags.ImageFlagName, flags.GitFlagWildcard),
+		},
+		{
+			Name: "all sources including maven",
+			Validatable: &commands.WorkloadOptions{
+				Namespace:     "default",
+				Name:          "my-resource",
+				GitRepo:       "https://example.com/repo.git",
+				GitBranch:     "main",
+				SourceImage:   "repo.example/image:tag",
+				Image:         "repo.example/image:tag",
+				MavenArtifact: "hello-world",
+				MavenType:     "jar",
+				MavenVersion:  "0.0.1",
+			},
+			ShouldValidate:    false,
+			ExpectFieldErrors: validation.ErrMultipleSources(commands.MavenFlagWildcard, commands.LocalPathAndSource, flags.ImageFlagName, flags.GitFlagWildcard),
 		},
 		{
 			Name: "wait",