Fix: vela up command to behave similarly with kubectl (#6809)

* Fix: vela up command to behave similarly with kubectl

Signed-off-by: Chaitanya Reddy Onteddu <chaitanyareddy0702@gmail.com>

* Chore: Update the unit test cases and add a new test cases

Signed-off-by: Chaitanya Reddy Onteddu <chaitanyareddy0702@gmail.com>

---------

Signed-off-by: Chaitanya Reddy Onteddu <chaitanyareddy0702@gmail.com>
Co-authored-by: Chaitanya Reddy Onteddu <co@guidewire.com>

pkg/cmd/utils.go

@@ -49,6 +49,31 @@ func GetNamespace(_ Factory, cmd *cobra.Command) string {
 	return envMeta.Namespace
 }
 
+// GetNamespaceAndSource gets namespace from command flags and env and also returns source of the namespace
+func GetNamespaceAndSource(cmd *cobra.Command) (string, string) {
+	namespace, err := cmd.Flags().GetString(flagNamespace)
+	cmdutil.CheckErr(err)
+	if namespace != "" {
+		return namespace, "namespaceflag"
+	}
+	// find namespace from env
+	envName, err := cmd.Flags().GetString(flagEnv)
+	if err != nil {
+		// ignore env if the command does not use the flag
+		return "", "defaulted"
+	}
+	var envMeta *types.EnvMeta
+	if envName != "" {
+		envMeta, err = env.GetEnvByName(envName)
+	} else {
+		envMeta, err = env.GetCurrentEnv()
+	}
+	if err != nil {
+		return "", "defaulted"
+	}
+	return envMeta.Namespace, "env"
+}
+
 // GetGroup get group from command flags
 func GetGroup(cmd *cobra.Command) string {
 	group, err := cmd.Flags().GetString(flagGroup)

references/cli/up.go

@@ -46,15 +46,16 @@ import (
 
 // UpCommandOptions command args for vela up
 type UpCommandOptions struct {
-	AppName        string
-	Namespace      string
-	File           string
-	PublishVersion string
-	RevisionName   string
-	ShardID        string
-	Debug          bool
-	Wait           bool
-	WaitTimeout    string
+	AppName         string
+	Namespace       string
+	File            string
+	PublishVersion  string
+	RevisionName    string
+	ShardID         string
+	Debug           bool
+	Wait            bool
+	WaitTimeout     string
+	NamespaceSource string
 }
 
 // Complete fill the args for vela up
@@ -62,7 +63,7 @@ func (opt *UpCommandOptions) Complete(f velacmd.Factory, cmd *cobra.Command, arg
 	if len(args) > 0 {
 		opt.AppName = args[0]
 	}
-	opt.Namespace = velacmd.GetNamespace(f, cmd)
+	opt.Namespace, opt.NamespaceSource = velacmd.GetNamespaceAndSource(cmd)
 }
 
 // Validate if vela up args is valid, interrupt the command
@@ -184,7 +185,14 @@ func (opt *UpCommandOptions) deployApplicationFromFile(f velacmd.Factory, cmd *c
 
 		// Override namespace if namespace flag is set. We should check if namespace is `default` or not
 		// since GetFlagNamespaceOrEnv returns default namespace when failed to get current env.
-		if opt.Namespace != "" && opt.Namespace != types.DefaultAppNamespace {
+		// If the Namespace mentioned in the flag and the file are different, returns an error.
+		// Precedence is as follows: flag/file > env
+
+		if app.Namespace != "" && opt.Namespace != "" && opt.NamespaceSource == "namespaceflag" && app.Namespace != opt.Namespace {
+			return errors.Errorf("application namespace %s is different from the namespace %s specified by flag", app.Namespace, opt.Namespace)
+		}
+
+		if opt.Namespace != "" && opt.Namespace != types.DefaultAppNamespace && app.Namespace == "" {
 			app.SetNamespace(opt.Namespace)
 		}
 		if opt.PublishVersion != "" {

references/cli/up_test.go

@@ -37,7 +37,6 @@ import (
 )
 
 func TestUp(t *testing.T) {
-
 	app := &v1beta1.Application{}
 	app.Name = "app-up"
 	msg := common.Info(app)
@@ -93,20 +92,6 @@ spec:
 			namespace:         "",
 			expectedNamespace: "vela-apps",
 		},
-		"override application namespace": {
-			application: `
-apiVersion: core.oam.dev/v1beta1
-kind: Application
-metadata:
-  name: first-vela-app
-  namespace: vela-apps
-spec:
-  components: []
-`,
-			applicationName:   "first-vela-app",
-			namespace:         "overridden-namespace",
-			expectedNamespace: "overridden-namespace",
-		},
 	}
 
 	for name, c := range cases {
@@ -150,3 +135,67 @@ spec:
 		})
 	}
 }
+
+func TestUpConflictingNamespace(t *testing.T) {
+	cases := map[string]struct {
+		application string
+		namespace   string
+	}{
+		"conflicting namespace": {
+			application: `
+apiVersion: core.oam.dev/v1beta1
+kind: Application
+metadata:
+  name: conflicting-namespace-app
+  namespace: default
+spec:
+  components: []
+`,
+			namespace: "conflicting-namespace",
+		},
+	}
+
+	for name, c := range cases {
+		t.Run(name, func(t *testing.T) {
+			args := initArgs()
+			kc, err := args.GetClient()
+			require.NoError(t, err)
+
+			af, err := os.CreateTemp(os.TempDir(), "conflicting-namespace-*.yaml")
+			require.NoError(t, err)
+			defer func() {
+				_ = af.Close()
+				_ = os.Remove(af.Name())
+			}()
+			_, err = af.WriteString(c.application)
+			require.NoError(t, err)
+
+			// Ensure namespace
+			require.NoError(t, kc.Create(context.TODO(), &corev1.Namespace{
+				ObjectMeta: metav1.ObjectMeta{Name: c.namespace},
+			}))
+
+			var buf bytes.Buffer
+			cmd := NewUpCommand(velacmd.NewDelegateFactory(args.GetClient, args.GetConfig), "", args, util.IOStreams{In: os.Stdin, Out: os.Stdout, ErrOut: os.Stderr})
+			cmd.SetOut(&buf)
+			cmd.SetErr(&buf)
+			o := &UpCommandOptions{}
+			o.Complete(velacmd.NewDelegateFactory(args.GetClient, args.GetConfig), cmd, []string{})
+			if c.namespace != "" {
+				require.NoError(t, cmd.Flags().Set(FlagNamespace, c.namespace))
+			}
+			require.NoError(t, cmd.Flags().Set("file", af.Name()))
+			o.File = af.Name()
+			err = o.Validate()
+			if err != nil {
+				t.Logf("Validation error: %v", err)
+				t.FailNow()
+			}
+			o.Complete(velacmd.NewDelegateFactory(args.GetClient, args.GetConfig), cmd, []string{})
+			// This will return the actual error instead of calling os.Exit()
+			err = o.Run(velacmd.NewDelegateFactory(args.GetClient, args.GetConfig), cmd)
+			require.Error(t, err)
+			assert.Contains(t, err.Error(), "application namespace")
+		})
+	}
+}