Add a Devfile dependency for commands (#7063)

* Define Devfile dependency for commands * Remove MarkDevfileNotNeeded * More commands not using Devfile * Tests with invalid devfiles
redhat-developer · Sep 4, 2023 · a658121 · a658121
1 parent 03c109a
commit a658121
Show file tree

Hide file tree

Showing 22 changed files with 135 additions and 38 deletions.
diff --git a/pkg/odo/cli/alizer/alizer.go b/pkg/odo/cli/alizer/alizer.go
@@ -35,6 +35,10 @@ func (o *AlizerOptions) SetClientset(clientset *clientset.Clientset) {
 	o.clientset = clientset
 }
 
+func (o *AlizerOptions) UseDevfile(ctx context.Context, cmdline cmdline.Cmdline, args []string) bool {
+	return false
+}
+
 func (o *AlizerOptions) Complete(ctx context.Context, cmdline cmdline.Cmdline, args []string) (err error) {
 	return nil
 }
@@ -80,7 +84,6 @@ func NewCmdAlizer(name, fullName string, testClientset clientset.Clientset) *cob
 	}
 	clientset.Add(alizerCmd, clientset.ALIZER, clientset.FILESYSTEM)
 	util.SetCommandGroup(alizerCmd, util.UtilityGroup)
-	genericclioptions.MarkDevfileNotNeeded(alizerCmd)
 	commonflags.UseOutputFlag(alizerCmd)
 	alizerCmd.SetUsageTemplate(odoutil.CmdUsageTemplate)
 	return alizerCmd

diff --git a/pkg/odo/cli/delete/component/component.go b/pkg/odo/cli/delete/component/component.go
@@ -84,6 +84,10 @@ func (o *ComponentOptions) SetClientset(clientset *clientset.Clientset) {
 	o.clientset = clientset
 }
 
+func (o *ComponentOptions) UseDevfile(ctx context.Context, cmdline cmdline.Cmdline, args []string) bool {
+	return o.name == ""
+}
+
 func (o *ComponentOptions) Complete(ctx context.Context, cmdline cmdline.Cmdline, args []string) (err error) {
 	switch api.RunningMode(o.runningInFlag) {
 	case api.RunningModeDev:

diff --git a/pkg/odo/cli/describe/binding.go b/pkg/odo/cli/describe/binding.go
@@ -51,6 +51,10 @@ func (o *BindingOptions) SetClientset(clientset *clientset.Clientset) {
 	o.clientset = clientset
 }
 
+func (o *BindingOptions) UseDevfile(ctx context.Context, cmdline cmdline.Cmdline, args []string) bool {
+	return o.nameFlag == ""
+}
+
 func (o *BindingOptions) Complete(ctx context.Context, cmdline cmdline.Cmdline, args []string) (err error) {
 	if o.nameFlag == "" {
 		devfileObj := odocontext.GetEffectiveDevfileObj(ctx)

diff --git a/pkg/odo/cli/describe/component.go b/pkg/odo/cli/describe/component.go
@@ -61,6 +61,10 @@ func (o *ComponentOptions) SetClientset(clientset *clientset.Clientset) {
 	o.clientset = clientset
 }
 
+func (o *ComponentOptions) UseDevfile(ctx context.Context, cmdline cmdline.Cmdline, args []string) bool {
+	return o.nameFlag == ""
+}
+
 func (o *ComponentOptions) Complete(ctx context.Context, cmdline cmdline.Cmdline, args []string) (err error) {
 	platform := fcontext.GetPlatform(ctx, commonflags.PlatformCluster)
 
@@ -329,7 +333,7 @@ func NewCmdComponent(ctx context.Context, name, fullName string, testClientset c
 	}
 	componentCmd.Flags().StringVar(&o.nameFlag, "name", "", "Name of the component to describe, optional. By default, the component in the local devfile is described")
 	componentCmd.Flags().StringVar(&o.namespaceFlag, "namespace", "", "Namespace in which to find the component to describe, optional. By default, the current namespace defined in kubeconfig is used")
-	clientset.Add(componentCmd, clientset.KUBERNETES_NULLABLE, clientset.STATE)
+	clientset.Add(componentCmd, clientset.KUBERNETES_NULLABLE, clientset.STATE, clientset.FILESYSTEM)
 	if feature.IsEnabled(ctx, feature.GenericPlatformFlag) {
 		clientset.Add(componentCmd, clientset.PODMAN_NULLABLE)
 	}

diff --git a/pkg/odo/cli/init/init.go b/pkg/odo/cli/init/init.go
@@ -88,6 +88,10 @@ func (o *InitOptions) SetClientset(clientset *clientset.Clientset) {
 	o.clientset = clientset
 }
 
+func (o *InitOptions) UseDevfile(ctx context.Context, cmdline cmdline.Cmdline, args []string) bool {
+	return false
+}
+
 // Complete will build the parameters for init, using different backends based on the flags set,
 // either by using flags or interactively if no flag is passed
 // Complete will return an error immediately if the current working directory is not empty

diff --git a/pkg/odo/cli/list/services/services.go b/pkg/odo/cli/list/services/services.go
@@ -56,6 +56,10 @@ func (o *ServiceListOptions) SetClientset(clientset *clientset.Clientset) {
 	o.clientset = clientset
 }
 
+func (o *ServiceListOptions) UseDevfile(ctx context.Context, cmdline cmdline.Cmdline, args []string) bool {
+	return false
+}
+
 func (o *ServiceListOptions) Complete(ctx context.Context, cmdline cmdline.Cmdline, _ []string) error {
 	if o.namespaceFlag == "" && !o.allNamespacesFlag {
 		o.namespaceFlag = odocontext.GetNamespace(ctx)

diff --git a/pkg/odo/cli/preference/add/registry.go b/pkg/odo/cli/preference/add/registry.go
@@ -58,6 +58,10 @@ func (o *RegistryOptions) SetClientset(clientset *clientset.Clientset) {
 	o.clientset = clientset
 }
 
+func (o *RegistryOptions) UseDevfile(ctx context.Context, cmdline cmdline.Cmdline, args []string) bool {
+	return false
+}
+
 // Complete completes RegistryOptions after they've been created
 func (o *RegistryOptions) Complete(ctx context.Context, cmdline cmdline.Cmdline, args []string) (err error) {
 	o.operation = "add"

diff --git a/pkg/odo/cli/preference/remove/registry.go b/pkg/odo/cli/preference/remove/registry.go
@@ -56,6 +56,10 @@ func (o *RegistryOptions) SetClientset(clientset *clientset.Clientset) {
 	o.clientset = clientset
 }
 
+func (o *RegistryOptions) UseDevfile(ctx context.Context, cmdline cmdline.Cmdline, args []string) bool {
+	return false
+}
+
 // Complete completes RegistryOptions after they've been created
 func (o *RegistryOptions) Complete(ctx context.Context, cmdline cmdline.Cmdline, args []string) (err error) {
 	o.operation = "remove"

diff --git a/pkg/odo/cli/preference/set.go b/pkg/odo/cli/preference/set.go
@@ -53,6 +53,10 @@ func (o *SetOptions) SetClientset(clientset *clientset.Clientset) {
 	o.clientset = clientset
 }
 
+func (o *SetOptions) UseDevfile(ctx context.Context, cmdline cmdline.Cmdline, args []string) bool {
+	return false
+}
+
 // Complete completes SetOptions after they've been created
 func (o *SetOptions) Complete(ctx context.Context, cmdline cmdline.Cmdline, args []string) (err error) {
 	o.paramName = strings.ToLower(args[0])

diff --git a/pkg/odo/cli/preference/unset.go b/pkg/odo/cli/preference/unset.go
@@ -52,6 +52,10 @@ func (o *UnsetOptions) SetClientset(clientset *clientset.Clientset) {
 	o.clientset = clientset
 }
 
+func (o *UnsetOptions) UseDevfile(ctx context.Context, cmdline cmdline.Cmdline, args []string) bool {
+	return false
+}
+
 // Complete completes UnsetOptions after they've been created
 func (o *UnsetOptions) Complete(ctx context.Context, cmdline cmdline.Cmdline, args []string) (err error) {
 	o.paramName = strings.ToLower(args[0])

diff --git a/pkg/odo/cli/preference/view.go b/pkg/odo/cli/preference/view.go
@@ -41,6 +41,10 @@ func (o *ViewOptions) SetClientset(clientset *clientset.Clientset) {
 	o.clientset = clientset
 }
 
+func (o *ViewOptions) UseDevfile(ctx context.Context, cmdline cmdline.Cmdline, args []string) bool {
+	return false
+}
+
 // Complete completes ViewOptions after they've been created
 func (o *ViewOptions) Complete(ctx context.Context, cmdline cmdline.Cmdline, args []string) (err error) {
 	return

diff --git a/pkg/odo/cli/registry/registry.go b/pkg/odo/cli/registry/registry.go
@@ -67,6 +67,10 @@ func (o *ListOptions) SetClientset(clientset *clientset.Clientset) {
 	o.clientset = clientset
 }
 
+func (o *ListOptions) UseDevfile(ctx context.Context, cmdline cmdline.Cmdline, args []string) bool {
+	return false
+}
+
 // Complete completes ListOptions after they've been created
 func (o *ListOptions) Complete(ctx context.Context, cmdline cmdline.Cmdline, args []string) (err error) {
 

diff --git a/pkg/odo/cli/set/namespace/namespace.go b/pkg/odo/cli/set/namespace/namespace.go
@@ -56,6 +56,10 @@ func (so *SetOptions) SetClientset(clientset *clientset.Clientset) {
 	so.clientset = clientset
 }
 
+func (o *SetOptions) UseDevfile(ctx context.Context, cmdline cmdline.Cmdline, args []string) bool {
+	return false
+}
+
 // Complete completes SetOptions after they've been created
 func (so *SetOptions) Complete(ctx context.Context, cmdline cmdline.Cmdline, args []string) (err error) {
 	so.namespaceName = args[0]

diff --git a/pkg/odo/genericclioptions/devfile.go b/pkg/odo/genericclioptions/devfile.go
@@ -15,17 +15,6 @@ import (
 	odoutil "github.com/redhat-developer/odo/pkg/util"
 )
 
-// MarkDevfileNotNeeded annotates the provided command such that it does not require a valid Devfile
-// to be present in the current directory.
-// A corollary to this is that commands annotated as such will not have any Devfile parsed in their root context,
-// even if there is a local "devfile.yaml" in the current directory.
-func MarkDevfileNotNeeded(cmd *cobra.Command) {
-	if cmd.Annotations == nil {
-		cmd.Annotations = map[string]string{}
-	}
-	cmd.Annotations["devfile-not-needed"] = "true"
-}
-
 func getDevfileInfo(cmd *cobra.Command, fsys filesystem.Filesystem, workingDir string, variables map[string]string, imageRegistry string) (
 	devfilePath string,
 	devfileObj *parser.DevfileObj,
@@ -34,8 +23,7 @@ func getDevfileInfo(cmd *cobra.Command, fsys filesystem.Filesystem, workingDir s
 ) {
 	devfilePath = location.DevfileLocation(fsys, workingDir)
 	isDevfile := odoutil.CheckPathExists(fsys, devfilePath)
-	requiresValidDevfile := cmd.Annotations["devfile-not-needed"] != "true"
-	if requiresValidDevfile && isDevfile {
+	if isDevfile {
 		devfilePath, err = dfutil.GetAbsPath(devfilePath)
 		if err != nil {
 			return "", nil, "", err

diff --git a/pkg/odo/genericclioptions/runnable.go b/pkg/odo/genericclioptions/runnable.go
@@ -71,6 +71,14 @@ type JsonOutputter interface {
 	RunForJsonOutput(ctx context.Context) (result interface{}, err error)
 }
 
+// DevfileUser must be implemented by commands that use Devfile depending on arguments, or
+// commands that depend on FS but do not use Devfile.
+// If the interface is not implemented and the command depends on FS, the command is expected to use Devfile
+type DevfileUser interface {
+	// UseDevfile returns true if the command with the specified cmdline and args needs to have access to the Devfile
+	UseDevfile(ctx context.Context, cmdline cmdline.Cmdline, args []string) bool
+}
+
 const (
 	// defaultAppName is the default name of the application when an application name is not provided
 	defaultAppName = "app"
@@ -248,16 +256,23 @@ func GenericRun(o Runnable, testClientset clientset.Clientset, cmd *cobra.Comman
 			}
 		}
 
-		var devfilePath, componentName string
-		var devfileObj *parser.DevfileObj
-		devfilePath, devfileObj, componentName, err = getDevfileInfo(cmd, deps.FS, cwd, variables, userConfig.GetImageRegistry())
-		if err != nil {
-			startTelemetry(cmd, err, startTime)
-			return err
+		useDevfile := true
+		if devfileUser, ok := o.(DevfileUser); ok {
+			useDevfile = devfileUser.UseDevfile(ctx, cmdLineObj, args)
+		}
+
+		if useDevfile {
+			var devfilePath, componentName string
+			var devfileObj *parser.DevfileObj
+			devfilePath, devfileObj, componentName, err = getDevfileInfo(cmd, deps.FS, cwd, variables, userConfig.GetImageRegistry())
+			if err != nil {
+				startTelemetry(cmd, err, startTime)
+				return err
+			}
+			ctx = odocontext.WithDevfilePath(ctx, devfilePath)
+			ctx = odocontext.WithEffectiveDevfileObj(ctx, devfileObj)
+			ctx = odocontext.WithComponentName(ctx, componentName)
 		}
-		ctx = odocontext.WithDevfilePath(ctx, devfilePath)
-		ctx = odocontext.WithEffectiveDevfileObj(ctx, devfileObj)
-		ctx = odocontext.WithComponentName(ctx, componentName)
 	}
 
 	// Run completion, validation and run.

diff --git a/tests/helper/helper_filesystem.go b/tests/helper/helper_filesystem.go
@@ -319,3 +319,15 @@ func AppendToFile(filepath string, s string) error {
 	}
 	return nil
 }
+
+func CreateInvalidDevfile(dir string) {
+	devfilePath := filepath.Join(dir, "devfile.yaml")
+	err := CreateFileWithContent(devfilePath, "invalid")
+	Expect(err).ToNot(HaveOccurred())
+}
+
+func DeleteInvalidDevfile(dir string) {
+	devfilePath := filepath.Join(dir, "devfile.yaml")
+	err := os.Remove(devfilePath)
+	Expect(err).ToNot(HaveOccurred())
+}
diff --git a/tests/integration/cmd_delete_test.go b/tests/integration/cmd_delete_test.go
@@ -149,6 +149,8 @@ var _ = Describe("odo delete command tests", func() {
 										Eventually(string(commonVar.CliRunner.Run(getSVCArgs...).Out.Contents()), 60, 3).ShouldNot(ContainSubstring(serviceName))
 									})
 									It("should output that there are no resources to be deleted", func() {
+										helper.CreateInvalidDevfile(commonVar.Context)
+										helper.Chdir(commonVar.Context)
 										args := []string{"delete", "component", "--name", cmpName, "--namespace", commonVar.Project}
 										if runningIn != "" {
 											args = append(args, "--running-in", runningIn)

diff --git a/tests/integration/cmd_describe_component_test.go b/tests/integration/cmd_describe_component_test.go
@@ -69,6 +69,7 @@ var _ = Describe("odo describe component command tests", func() {
 
 	It("should fail, with default cluster mode", func() {
 		By("running odo describe component -o json with an unknown name", func() {
+			helper.CreateInvalidDevfile(commonVar.Context)
 			res := helper.Cmd("odo", "describe", "component", "--name", "unknown-name", "-o", "json").ShouldFail()
 			stdout, stderr := res.Out(), res.Err()
 			Expect(helper.IsJSON(stderr)).To(BeTrue())
@@ -77,6 +78,7 @@ var _ = Describe("odo describe component command tests", func() {
 		})
 
 		By("running odo describe component with an unknown name", func() {
+			helper.CreateInvalidDevfile(commonVar.Context)
 			res := helper.Cmd("odo", "describe", "component", "--name", "unknown-name").ShouldFail()
 			stdout, stderr := res.Out(), res.Err()
 			Expect(stdout).To(BeEmpty())
@@ -86,6 +88,7 @@ var _ = Describe("odo describe component command tests", func() {
 
 	It("should fail, with cluster", func() {
 		By("running odo describe component -o json with an unknown name", func() {
+			helper.CreateInvalidDevfile(commonVar.Context)
 			res := helper.Cmd("odo", "describe", "component", "--name", "unknown-name", "--platform", "cluster", "-o", "json").ShouldFail()
 			stdout, stderr := res.Out(), res.Err()
 			Expect(helper.IsJSON(stderr)).To(BeTrue())
@@ -94,6 +97,7 @@ var _ = Describe("odo describe component command tests", func() {
 		})
 
 		By("running odo describe component with an unknown name", func() {
+			helper.CreateInvalidDevfile(commonVar.Context)
 			res := helper.Cmd("odo", "describe", "component", "--name", "unknown-name", "--platform", "cluster").ShouldFail()
 			stdout, stderr := res.Out(), res.Err()
 			Expect(stdout).To(BeEmpty())
@@ -103,6 +107,7 @@ var _ = Describe("odo describe component command tests", func() {
 
 	It("should fail, with podman", Label(helper.LabelPodman), func() {
 		By("running odo describe component -o json with an unknown name", func() {
+			helper.CreateInvalidDevfile(commonVar.Context)
 			res := helper.Cmd("odo", "describe", "component", "--name", "unknown-name", "--platform", "podman", "-o", "json").
 				ShouldFail()
 			stdout, stderr := res.Out(), res.Err()
@@ -112,6 +117,7 @@ var _ = Describe("odo describe component command tests", func() {
 		})
 
 		By("running odo describe component with an unknown name", func() {
+			helper.CreateInvalidDevfile(commonVar.Context)
 			stderr := helper.Cmd("odo", "describe", "component", "--name", "unknown-name", "--platform", "podman").
 				ShouldFail().Err()
 			Expect(stderr).To(ContainSubstring("no component found with name \"unknown-name\""))
@@ -257,8 +263,10 @@ var _ = Describe("odo describe component command tests", func() {
 
 		It("should not describe the component from another directory, with default cluster mode", func() {
 			By("running with json output", func() {
-				err := os.Chdir("/")
-				Expect(err).NotTo(HaveOccurred())
+				otherDir := filepath.Join(commonVar.Context, "tmp")
+				helper.MakeDir(otherDir)
+				helper.Chdir(otherDir)
+				helper.CreateInvalidDevfile(otherDir)
 				res := helper.Cmd("odo", "describe", "component", "--name", cmpName, "-o", "json").ShouldFail()
 				stdout, stderr := res.Out(), res.Err()
 				Expect(helper.IsJSON(stderr)).To(BeTrue())
@@ -267,8 +275,10 @@ var _ = Describe("odo describe component command tests", func() {
 			})
 
 			By("running with default output", func() {
-				err := os.Chdir("/")
-				Expect(err).NotTo(HaveOccurred())
+				otherDir := filepath.Join(commonVar.Context, "tmp")
+				helper.MakeDir(otherDir)
+				helper.Chdir(otherDir)
+				helper.CreateInvalidDevfile(otherDir)
 				res := helper.Cmd("odo", "describe", "component", "--name", cmpName).ShouldFail()
 				stdout, stderr := res.Out(), res.Err()
 				Expect(stdout).To(BeEmpty())
@@ -278,8 +288,10 @@ var _ = Describe("odo describe component command tests", func() {
 
 		It("should not describe the component from another directory, with cluster", func() {
 			By("running with json output", func() {
-				err := os.Chdir("/")
-				Expect(err).NotTo(HaveOccurred())
+				otherDir := filepath.Join(commonVar.Context, "tmp")
+				helper.MakeDir(otherDir)
+				helper.Chdir(otherDir)
+				helper.CreateInvalidDevfile(otherDir)
 				res := helper.Cmd("odo", "describe", "component", "--name", cmpName, "-o", "json", "--platform", "cluster").ShouldFail()
 				stdout, stderr := res.Out(), res.Err()
 				Expect(helper.IsJSON(stderr)).To(BeTrue())
@@ -288,8 +300,10 @@ var _ = Describe("odo describe component command tests", func() {
 			})
 
 			By("running with default output", func() {
-				err := os.Chdir("/")
-				Expect(err).NotTo(HaveOccurred())
+				otherDir := filepath.Join(commonVar.Context, "tmp")
+				helper.MakeDir(otherDir)
+				helper.Chdir(otherDir)
+				helper.CreateInvalidDevfile(otherDir)
 				res := helper.Cmd("odo", "describe", "component", "--name", cmpName, "--platform", "cluster").ShouldFail()
 				stdout, stderr := res.Out(), res.Err()
 				Expect(stdout).To(BeEmpty())
@@ -547,10 +561,12 @@ var _ = Describe("odo describe component command tests", func() {
 						}
 					})
 					By("checking the human readable output with component name", func() {
+						helper.CreateInvalidDevfile(commonVar.Context)
 						out := helper.Cmd("odo", "describe", "component", "--name", componentName).ShouldPass().Out()
 						helper.MatchAllInOutput(out, ctx.matchOutput)
 					})
 					By("checking the machine readable output with component name", func() {
+						helper.CreateInvalidDevfile(commonVar.Context)
 						out := helper.Cmd("odo", "describe", "component", "--name", componentName, "-o", "json").ShouldPass().Out()
 						for key, value := range ctx.matchJSONOutput {
 							helper.JsonPathContentContain(out, key, value)