Do not return an error in odo analyze if current directory contains an invalid Devfile (#6905)

* Add unit test highliging the issue

* Fix 'delete' unit tests

* Pass the filesystem object where it is relevant

* Add a way for CLI commands to indicate whether of not they require a valid Devfile

For the 'analyze' command, this is not required,
so Devfile parsing will be ignored completely.

* Make the fake filesystem return an absolute current dir

Otherwise, some code will assume it is relative,
and try to prepend the current physical directory

cmd/odo/alizer_test.go

@@ -2,10 +2,12 @@ package main
 
 import (
 	"encoding/json"
+	"path/filepath"
 	"strings"
 	"testing"
 
 	"github.com/golang/mock/gomock"
+
 	"github.com/redhat-developer/alizer/go/pkg/apis/model"
 	"github.com/redhat-developer/odo/pkg/alizer"
 	"github.com/redhat-developer/odo/pkg/api"
@@ -18,6 +20,7 @@ func TestOdoAlizer(t *testing.T) {
 	for _, tt := range []struct {
 		name            string
 		clientset       func() clientset.Clientset
+		fsPopulator     func(fs filesystem.Filesystem) error
 		args            []string
 		wantErr         string
 		wantStdout      string
@@ -46,7 +49,7 @@ func TestOdoAlizer(t *testing.T) {
 				ctrl := gomock.NewController(t)
 				fs := filesystem.NewFakeFs()
 				alizerClient := alizer.NewMockClient(ctrl)
-				path := "."
+				path := "/"
 				alizerClient.EXPECT().DetectFramework(gomock.Any(), path).
 					Return(
 						model.DevFileType{
@@ -80,15 +83,77 @@ func TestOdoAlizer(t *testing.T) {
 				checkEqual(t, output[0].ApplicationPorts[1], 3000)
 			},
 		},
+		{
+			name: "analyze should not error out even if there is an invalid Devfile in the current directory",
+			clientset: func() clientset.Clientset {
+				ctrl := gomock.NewController(t)
+				fs := filesystem.NewFakeFs()
+				alizerClient := alizer.NewMockClient(ctrl)
+				path := "/"
+				alizerClient.EXPECT().DetectFramework(gomock.Any(), path).
+					Return(
+						model.DevFileType{
+							Name: "framework-name",
+						},
+						"1.1.1",
+						api.Registry{
+							Name: "TheRegistryName",
+						},
+						nil,
+					)
+				alizerClient.EXPECT().DetectPorts(path).Return([]int{8080, 3000}, nil)
+				alizerClient.EXPECT().DetectName(path).Return("aName", nil)
+				return clientset.Clientset{
+					FS:           fs,
+					AlizerClient: alizerClient,
+				}
+			},
+			fsPopulator: func(fs filesystem.Filesystem) error {
+				cwd, err := fs.Getwd()
+				if err != nil {
+					return err
+				}
+				err = fs.WriteFile(
+					filepath.Join(cwd, "main.go"),
+					[]byte(`package main
+
+import "fmt"
+
+func main() {
+	fmt.Println("Hello World")
+}
+`),
+					0644)
+				if err != nil {
+					return err
+				}
+
+				return fs.WriteFile(filepath.Join(cwd, "devfile.yaml"), []byte("some-invalid-content"), 0644)
+			},
+			args: []string{"analyze", "-o", "json"},
+			checkJsonOutput: func(t *testing.T, b []byte) {
+				var output []api.DetectionResult
+				err := json.Unmarshal(b, &output)
+				if err != nil {
+					t.Fatal(err)
+				}
+				checkEqual(t, output[0].Devfile, "framework-name")
+				checkEqual(t, output[0].DevfileRegistry, "TheRegistryName")
+				checkEqual(t, output[0].Name, "aName")
+				checkEqual(t, output[0].DevfileVersion, "1.1.1")
+				checkEqual(t, output[0].ApplicationPorts[0], 8080)
+				checkEqual(t, output[0].ApplicationPorts[1], 3000)
+			},
+		},
 	} {
 		t.Run(tt.name, func(t *testing.T) {
-			clientset := clientset.Clientset{}
+			cs := clientset.Clientset{}
 			if tt.clientset != nil {
-				clientset = tt.clientset()
+				cs = tt.clientset()
 			}
-			runCommand(t, tt.args, runOptions{}, clientset, nil, func(err error, stdout, stderr string) {
+			runCommand(t, tt.args, runOptions{}, cs, tt.fsPopulator, func(err error, stdout, stderr string) {
 				if (err != nil) != (tt.wantErr != "") {
-					t.Fatalf("errWanted: %v\nGot: %v", tt.wantErr != "", err != nil)
+					t.Fatalf("errWanted: %v\nGot: %v", tt.wantErr != "", err)
 				}
 
 				if tt.wantErr != "" {

cmd/odo/common_test.go

@@ -7,14 +7,15 @@ import (
 	"os"
 	"testing"
 
+	"github.com/sethvargo/go-envconfig"
+	"github.com/spf13/pflag"
+	"k8s.io/klog"
+
 	"github.com/redhat-developer/odo/pkg/config"
 	envcontext "github.com/redhat-developer/odo/pkg/config/context"
 	"github.com/redhat-developer/odo/pkg/odo/cli"
 	"github.com/redhat-developer/odo/pkg/odo/genericclioptions/clientset"
 	"github.com/redhat-developer/odo/pkg/testingutil/filesystem"
-	"github.com/sethvargo/go-envconfig"
-	"github.com/spf13/pflag"
-	"k8s.io/klog"
 )
 
 func resetGlobalFlags() {
@@ -33,7 +34,7 @@ func runCommand(
 	args []string,
 	options runOptions,
 	clientset clientset.Clientset,
-	populateFS func(fs filesystem.Filesystem),
+	populateFS func(fs filesystem.Filesystem) error,
 	f func(err error, stdout, stderr string),
 ) {
 
@@ -52,7 +53,10 @@ func runCommand(
 	}
 
 	if populateFS != nil {
-		populateFS(clientset.FS)
+		err = populateFS(clientset.FS)
+		if err != nil {
+			t.Fatal(err)
+		}
 	}
 
 	ctx := context.Background()

cmd/odo/delete_test.go

@@ -8,16 +8,17 @@ import (
 
 	"github.com/golang/mock/gomock"
 	"github.com/onsi/gomega"
+	appsv1 "k8s.io/api/apps/v1"
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/meta"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+
 	"github.com/redhat-developer/odo/pkg/kclient"
 	"github.com/redhat-developer/odo/pkg/odo/genericclioptions/clientset"
 	"github.com/redhat-developer/odo/pkg/podman"
 	"github.com/redhat-developer/odo/pkg/testingutil/filesystem"
 	"github.com/redhat-developer/odo/pkg/util"
 	"github.com/redhat-developer/odo/tests/helper"
-	appsv1 "k8s.io/api/apps/v1"
-	corev1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/api/meta"
-	"k8s.io/apimachinery/pkg/runtime/schema"
 )
 
 // Context
@@ -82,12 +83,15 @@ var allPlatforms = map[string]platformFunc{
 
 // FS content
 
-type fscontentFunc func(fs filesystem.Filesystem)
+type fscontentFunc func(fs filesystem.Filesystem) error
 
-var noContentFscontent fscontentFunc = func(fs filesystem.Filesystem) {}
+var noContentFscontent fscontentFunc = func(fs filesystem.Filesystem) error {
+	return nil
+}
 
-var nodeJsSourcesFsContent fscontentFunc = func(fs filesystem.Filesystem) {
+var nodeJsSourcesFsContent fscontentFunc = func(fs filesystem.Filesystem) error {
 	helper.CopyExample(filepath.Join("source", "devfiles", "nodejs", "project"), ".")
+	return nil
 }
 
 type fsOptions struct {
@@ -96,7 +100,7 @@ type fsOptions struct {
 }
 
 var nodeJsSourcesAndDevfileFsContent = func(devfilePath string, options fsOptions) fscontentFunc {
-	return func(fs filesystem.Filesystem) {
+	return func(fs filesystem.Filesystem) error {
 		helper.CopyExample(filepath.Join("source", "devfiles", "nodejs", "project"), ".")
 		helper.CopyExampleDevFile(
 			devfilePath,
@@ -110,6 +114,7 @@ var nodeJsSourcesAndDevfileFsContent = func(devfilePath string, options fsOption
 			gomega.Expect(err).NotTo(gomega.HaveOccurred())
 
 		}
+		return nil
 	}
 }
 

pkg/dev/kubedev/kubedev.go

@@ -106,7 +106,7 @@ func (o *DevClient) Start(
 // RegenerateAdapterAndPush get the new devfile and pushes the files to remote pod
 func (o *DevClient) regenerateAdapterAndPush(ctx context.Context, pushParams common.PushParameters, componentStatus *watch.ComponentStatus) error {
 
-	devObj, err := devfile.ParseAndValidateFromFileWithVariables(location.DevfileLocation(""), pushParams.StartOptions.Variables, o.prefClient.GetImageRegistry(), true)
+	devObj, err := devfile.ParseAndValidateFromFileWithVariables(location.DevfileLocation(o.filesystem, ""), pushParams.StartOptions.Variables, o.prefClient.GetImageRegistry(), true)
 	if err != nil {
 		return fmt.Errorf("unable to read devfile: %w", err)
 	}

pkg/dev/podmandev/podmandev.go

@@ -182,7 +182,7 @@ func (o *DevClient) checkVolumesFree(pod *corev1.Pod) error {
 
 func (o *DevClient) watchHandler(ctx context.Context, pushParams common.PushParameters, componentStatus *watch.ComponentStatus) error {
 
-	devObj, err := devfile.ParseAndValidateFromFileWithVariables(location.DevfileLocation(""), pushParams.StartOptions.Variables, o.prefClient.GetImageRegistry(), true)
+	devObj, err := devfile.ParseAndValidateFromFileWithVariables(location.DevfileLocation(o.fs, ""), pushParams.StartOptions.Variables, o.prefClient.GetImageRegistry(), true)
 	if err != nil {
 		return fmt.Errorf("unable to read devfile: %w", err)
 	}

pkg/devfile/location/location.go

@@ -13,9 +13,9 @@ var possibleDevfileNames = [...]string{"devfile.yaml", ".devfile.yaml"}
 
 // DevfileFilenamesProvider checks if the context dir contains devfile with possible names from possibleDevfileNames variable,
 // else it returns "devfile.yaml" as default value.
-func DevfileFilenamesProvider(contexDir string) string {
+func DevfileFilenamesProvider(fsys filesystem.Filesystem, contextDir string) string {
 	for _, devFile := range possibleDevfileNames {
-		if util.CheckPathExists(filepath.Join(contexDir, devFile)) {
+		if util.CheckPathExists(fsys, filepath.Join(contextDir, devFile)) {
 			return devFile
 		}
 	}
@@ -25,12 +25,12 @@ func DevfileFilenamesProvider(contexDir string) string {
 // DevfileLocation returns path to devfile File with approprite devfile File name from possibleDevfileNames variable,
 // if contextDir value is provided as argument then DevfileLocation returns devfile path
 // else it assumes current dir as contextDir and returns appropriate value.
-func DevfileLocation(contexDir string) string {
+func DevfileLocation(fsys filesystem.Filesystem, contextDir string) string {
-	if contexDir == "" {
+	if contextDir == "" {
-		contexDir = "./"
+		contextDir = "./"
 	}
-	devFile := DevfileFilenamesProvider(contexDir)
+	devFile := DevfileFilenamesProvider(fsys, contextDir)
-	return filepath.Join(contexDir, devFile)
+	return filepath.Join(contextDir, devFile)
 }
 
 // IsDevfileName returns true if name is a supported name for a devfile

pkg/odo/cli/add/binding/binding.go

@@ -78,7 +78,7 @@ func (o *AddBindingOptions) Validate(ctx context.Context) (err error) {
 
 func (o *AddBindingOptions) Run(ctx context.Context) error {
 	// Update the raw Devfile only, so we do not break any relationship between parent-child for example
-	withDevfile := odoutil.CheckPathExists(location.DevfileLocation(odocontext.GetWorkingDirectory(ctx)))
+	withDevfile := odoutil.CheckPathExists(o.clientset.FS, location.DevfileLocation(o.clientset.FS, odocontext.GetWorkingDirectory(ctx)))
 	var devfileObj *parser.DevfileObj
 	if withDevfile {
 		rawDevfileObj, err := devfile.ParseAndValidateFromFile(odocontext.GetDevfilePath(ctx), "", false)

pkg/odo/cli/alizer/alizer.go

@@ -80,6 +80,7 @@ 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

pkg/odo/genericclioptions/devfile.go

@@ -5,23 +5,37 @@ import (
 
 	"github.com/devfile/library/v2/pkg/devfile/parser"
 	dfutil "github.com/devfile/library/v2/pkg/util"
+	"github.com/spf13/cobra"
 
 	"github.com/redhat-developer/odo/pkg/component"
 	"github.com/redhat-developer/odo/pkg/devfile"
 	"github.com/redhat-developer/odo/pkg/devfile/location"
 	"github.com/redhat-developer/odo/pkg/devfile/validate"
+	"github.com/redhat-developer/odo/pkg/testingutil/filesystem"
 	odoutil "github.com/redhat-developer/odo/pkg/util"
 )
 
-func getDevfileInfo(workingDir string, variables map[string]string, imageRegistry string) (
+// 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,
 	componentName string,
 	err error,
 ) {
-	devfilePath = location.DevfileLocation(workingDir)
+	devfilePath = location.DevfileLocation(fsys, workingDir)
-	isDevfile := odoutil.CheckPathExists(devfilePath)
+	isDevfile := odoutil.CheckPathExists(fsys, devfilePath)
-	if isDevfile {
+	requiresValidDevfile := cmd.Annotations["devfile-not-needed"] != "true"
+	if requiresValidDevfile && isDevfile {
 		devfilePath, err = dfutil.GetAbsPath(devfilePath)
 		if err != nil {
 			return "", nil, "", err

pkg/odo/genericclioptions/runnable.go

@@ -11,9 +11,8 @@ import (
 	"syscall"
 	"time"
 
-	"gopkg.in/AlecAivazis/survey.v1/terminal"
-
 	"github.com/devfile/library/v2/pkg/devfile/parser"
+	"gopkg.in/AlecAivazis/survey.v1/terminal"
 
 	"github.com/redhat-developer/odo/pkg/machineoutput"
 	"github.com/redhat-developer/odo/pkg/version"
@@ -248,7 +247,7 @@ func GenericRun(o Runnable, testClientset clientset.Clientset, cmd *cobra.Comman
 
 		var devfilePath, componentName string
 		var devfileObj *parser.DevfileObj
-		devfilePath, devfileObj, componentName, err = getDevfileInfo(cwd, variables, userConfig.GetImageRegistry())
+		devfilePath, devfileObj, componentName, err = getDevfileInfo(cmd, deps.FS, cwd, variables, userConfig.GetImageRegistry())
 		if err != nil {
 			startTelemetry(cmd, err, startTime)
 			return err

pkg/registry/registry.go

@@ -435,7 +435,7 @@ func (o RegistryClient) retrieveDevfileDataFromRegistry(ctx context.Context, reg
 	}
 
 	// Get the devfile yaml file from the directory
-	devfileYamlFile := location.DevfileFilenamesProvider(tmpFile)
+	devfileYamlFile := location.DevfileFilenamesProvider(o.fsys, tmpFile)
 
 	// Parse and validate the file and return the devfile data
 	devfileObj, err := devfile.ParseAndValidateFromFile(path.Join(tmpFile, devfileYamlFile), "", true)

pkg/registry/starter_project.go

@@ -3,10 +3,11 @@ package registry
 import (
 	"errors"
 	"fmt"
-	"github.com/redhat-developer/odo/pkg/testingutil/filesystem"
 	"os"
 	"path/filepath"
 
+	"github.com/redhat-developer/odo/pkg/testingutil/filesystem"
+
 	devfilev1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
 	parsercommon "github.com/devfile/library/v2/pkg/devfile/parser/data/v2/common"
 	"github.com/go-git/go-git/v5"
@@ -50,7 +51,7 @@ func DownloadStarterProject(fs filesystem.Filesystem, starterProject *devfilev1.
 	}
 
 	// We will check to see if the project has a valid directory
-	err = util.IsValidProjectDir(path, location.DevfileLocation(""), fs)
+	err = util.IsValidProjectDir(path, location.DevfileLocation(fs, ""), fs)
 	if err != nil {
 		return err
 	}

pkg/testingutil/filesystem/fake_fs.go

@@ -128,7 +128,7 @@ func (fs *fakeFs) RemoveAll(path string) error {
 }
 
 func (fs *fakeFs) Getwd() (dir string, err error) {
-	return ".", nil
+	return "/", nil
 }
 
 // Remove via afero.RemoveAll

pkg/util/util.go

@@ -190,8 +190,8 @@ func removeNonAlphaSuffix(input string) string {
 
 // CheckPathExists checks if a path exists or not
 // TODO(feloy) use from devfile library?
-func CheckPathExists(path string) bool {
+func CheckPathExists(fsys filesystem.Filesystem, path string) bool {
-	if _, err := filesystem.Get().Stat(path); !os.IsNotExist(err) {
+	if _, err := fsys.Stat(path); !errors.Is(err, fs.ErrNotExist) {
 		// path to file does exist
 		return true
 	}

pkg/util/util_test.go

@@ -405,27 +405,38 @@ func TestGetAbsPath(t *testing.T) {
 }
 
 func TestCheckPathExists(t *testing.T) {
-	dir, err := os.CreateTemp("", "")
-	defer os.RemoveAll(dir.Name())
-	if err != nil {
-		return
-	}
 	tests := []struct {
-		fileName string
+		fsProvider func() (filesystem.Filesystem, error)
-		wantBool bool
+		fileName   string
+		wantBool   bool
 	}{
 		{
-			fileName: dir.Name(),
+			fsProvider: func() (filesystem.Filesystem, error) {
+				fakeFs := filesystem.NewFakeFs()
+				_, err := fakeFs.Create("my-file")
+				if err != nil {
+					return nil, err
+				}
+				return fakeFs, nil
+			},
+			fileName: "my-file",
 			wantBool: true,
 		},
 		{
-			fileName: dir.Name() + "-blah",
+			fsProvider: func() (filesystem.Filesystem, error) {
+				return filesystem.NewFakeFs(), nil
+			},
+			fileName: "some-file-blah",
 			wantBool: false,
 		},
 	}
 
 	for _, tt := range tests {
-		exists := CheckPathExists(tt.fileName)
+		fsys, err := tt.fsProvider()
+		if err != nil {
+			t.Fatal(err)
+		}
+		exists := CheckPathExists(fsys, tt.fileName)
 		if tt.wantBool != exists {
 			t.Errorf("the expected value of TestCheckPathExists function is different : %v, got: %v", tt.wantBool, exists)
 		}

pkg/watch/file_watcher.go

@@ -7,9 +7,11 @@ import (
 
 	dfutil "github.com/devfile/library/v2/pkg/util"
 	"github.com/fsnotify/fsnotify"
-	"github.com/redhat-developer/odo/pkg/util"
 	gitignore "github.com/sabhiram/go-gitignore"
 	"k8s.io/klog"
+
+	"github.com/redhat-developer/odo/pkg/testingutil/filesystem"
+	"github.com/redhat-developer/odo/pkg/util"
 )
 
 func getFullSourcesWatcher(path string, fileIgnores []string) (*fsnotify.Watcher, error) {
@@ -38,6 +40,8 @@ func getFullSourcesWatcher(path string, fileIgnores []string) (*fsnotify.Watcher
 // ignores contains the glob rules for matching
 func addRecursiveWatch(watcher *fsnotify.Watcher, rootPath string, path string, ignores []string) error {
 
+	fsys := filesystem.DefaultFs{}
+
 	file, err := os.Stat(path)
 	if err != nil {
 		if os.IsNotExist(err) {
@@ -60,7 +64,7 @@ func addRecursiveWatch(watcher *fsnotify.Watcher, rootPath string, path string,
 			klog.V(4).Infof("adding watch on path %s", path)
 
 			// checking if the file exits before adding the watcher to it
-			if !util.CheckPathExists(path) {
+			if !util.CheckPathExists(fsys, path) {
 				return nil
 			}
 
@@ -76,7 +80,7 @@ func addRecursiveWatch(watcher *fsnotify.Watcher, rootPath string, path string,
 	err = filepath.Walk(path, func(newPath string, info os.FileInfo, err error) error {
 		if err != nil {
 			// Ignore the error if it's a 'path does not exist' error, no need to walk a non-existent path
-			if !util.CheckPathExists(newPath) {
+			if !util.CheckPathExists(fsys, newPath) {
 				klog.V(4).Infof("Walk func received an error for path %s, but the path doesn't exist so this is likely not an error. err: %v", path, err)
 				return nil
 			}
@@ -119,7 +123,7 @@ func addRecursiveWatch(watcher *fsnotify.Watcher, rootPath string, path string,
 		}
 
 		// checking if the file exits before adding the watcher to it
-		if !util.CheckPathExists(path) {
+		if !util.CheckPathExists(fsys, path) {
 			continue
 		}