Merge 2dc0119 into 72d632b

pkg/alizer/alizer.go

@@ -1,9 +1,15 @@
 package alizer
 
 import (
+	"fmt"
+	"os"
+	"path/filepath"
+
 	"github.com/redhat-developer/alizer/go/pkg/apis/recognizer"
 	"github.com/redhat-developer/odo/pkg/api"
 	"github.com/redhat-developer/odo/pkg/registry"
+	"github.com/redhat-developer/odo/pkg/util"
+	"k8s.io/klog"
 )
 
 type Alizer struct {
@@ -47,3 +53,65 @@ func GetDevfileLocationFromDetection(typ recognizer.DevFileType, registry api.Re
 		DevfileRegistry: registry.Name,
 	}
 }
+
+// DetectName retrieves the name of the project (if available)
+// If source code is detected:
+// 1. Detect the name (pom.xml for java, package.json for nodejs, etc.)
+// 2. If unable to detect the name, use the directory name
+//
+// If no source is detected:
+// 1. Use the directory name
+//
+// Last step. Sanitize the name so it's valid for a component name
+
+// Use:
+// import "github.com/redhat-developer/alizer/pkg/apis/recognizer"
+// components, err := recognizer.DetectComponents("./")
+
+// In order to detect the name, the name will first try to find out the name based on the program (pom.xml, etc.) but then if not, it will use the dir name.
+func DetectName(path string) (string, error) {
+	if path == "" {
+		return "", fmt.Errorf("path is empty")
+	}
+
+	// Check if the path exists using os.Stat
+	_, err := os.Stat(path)
+	if err != nil {
+		return "", err
+	}
+
+	// Step 1.
+	// Get the name of the directory from the devfile absolute path
+	// Use that path with Alizer to get the name of the project,
+	// if unable to find the name, we will use the directory name
+	components, err := recognizer.DetectComponents(path)
+	if err != nil {
+		return "", err
+	}
+	klog.V(4).Infof("Found components: %v", components)
+
+	// Take the first name that is found
+	var detectedName string
+	if len(components) > 0 {
+		detectedName = components[0].Name
+	}
+
+	// Step 2. If unable to detect the name, we will use the directory name.
+	// Alizer will not correctly default to the directory name when unable to detect it via pom.xml, package.json, etc.
+	// So we must do it ourselves
+	if detectedName == "" {
+		// Using filepath.Dir may be redundant, but it's to also check in case we passed in a filename (which shouldn't be passed in anyways)
+		detectedName = filepath.Base(filepath.Dir(path))
+	}
+
+	// Step 3.  Sanitize the name
+	// Make sure that detectedName conforms with Kubernetes naming rules
+	// If not, we will use the directory name
+	name := util.GetDNS1123Name(detectedName)
+	klog.V(4).Infof("Path: %s, Detected name: %s, Sanitized name: %s", path, detectedName, name)
+	if name == "" {
+		return "", fmt.Errorf("unable to sanitize name to DNS1123 format: %q", name)
+	}
+
+	return name, nil
+}

pkg/binding/add.go

@@ -10,6 +10,7 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 
+	"github.com/redhat-developer/odo/pkg/component"
 	sboApi "github.com/redhat-developer/service-binding-operator/apis/binding/v1alpha1"
 
 	"github.com/redhat-developer/odo/pkg/binding/asker"
@@ -112,7 +113,12 @@ func (o *BindingClient) AddBindingToDevfile(
 		return obj, err
 	}
 
-	deploymentName := fmt.Sprintf("%s-app", obj.GetMetadataName())
+	componentName, err := component.GatherName(obj)
+	if err != nil {
+		return obj, err
+	}
+
+	deploymentName := fmt.Sprintf("%s-app", componentName)
 	deploymentGVK, err := o.kubernetesClient.GetDeploymentAPIVersion()
 	if err != nil {
 		return obj, err

pkg/component/component.go

@@ -3,7 +3,6 @@ package component
 import (
 	"fmt"
 	"io"
-	"path/filepath"
 	"sort"
 	"strings"
 
@@ -12,15 +11,17 @@ import (
 	"github.com/devfile/library/pkg/devfile/parser"
 	"github.com/devfile/library/pkg/devfile/parser/data"
 	dfutil "github.com/devfile/library/pkg/util"
+	"k8s.io/klog"
 
+	"github.com/redhat-developer/odo/pkg/alizer"
 	"github.com/redhat-developer/odo/pkg/api"
 	"github.com/redhat-developer/odo/pkg/kclient"
 	"github.com/redhat-developer/odo/pkg/labels"
 	odolabels "github.com/redhat-developer/odo/pkg/labels"
+	"github.com/redhat-developer/odo/pkg/util"
 
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
-	"k8s.io/klog"
 )
 
 const (
@@ -42,32 +43,23 @@ func GetComponentTypeFromDevfileMetadata(metadata devfile.DevfileMetadata) strin
 	return componentType
 }
 
-// GatherName parses the Devfile and retrieves an appropriate name in two ways.
-// 1. If metadata.name exists, we use it
-// 2. If metadata.name does NOT exist, we use the folder name where the devfile.yaml is located
-func GatherName(devObj parser.DevfileObj, devfilePath string) (string, error) {
-
-	metadata := devObj.Data.GetMetadata()
-
-	klog.V(4).Infof("metadata.Name: %s", metadata.Name)
-
-	// 1. Use metadata.name if it exists
-	if metadata.Name != "" {
-
-		// Remove any suffix's that end with `-`. This is because many Devfile's use the original v1 Devfile pattern of
-		// having names such as "foo-bar-" in order to prepend container names such as "foo-bar-container1"
-		return strings.TrimSuffix(metadata.Name, "-"), nil
+// GatherName returns the name of the component.
+//// If a name is set in the Devfile metadata (which is optional) and is not blank, it returns that.
+//// Otherwise, it uses Alizer to detect the name of the component, from the project build tools (pom.xml, package.json, ...),
+//// or from the component directory name.
+func GatherName(devfileObj parser.DevfileObj) (string, error) {
+	devfilePath := devfileObj.Ctx.GetAbsPath()
+	name := devfileObj.GetMetadataName()
+	if name == "" || strings.TrimSpace(name) == "" {
+		//name already sanitized in alizer.DetectName
+		return alizer.DetectName(devfilePath)
 	}
 
-	// 2. Use the folder name as a last resort if nothing else exists
-	sourcePath, err := dfutil.GetAbsPath(devfilePath)
-	if err != nil {
-		return "", fmt.Errorf("unable to get source path: %w", err)
-	}
-	klog.V(4).Infof("Source path: %s", sourcePath)
-	klog.V(4).Infof("devfile dir: %s", filepath.Dir(sourcePath))
+	//sanitize the name
+	s := util.GetDNS1123Name(name)
+	klog.V(3).Infof("For Devfile at path %q, name of component is %q, and sanitized name is %q", devfilePath, name, s)
 
-	return filepath.Base(filepath.Dir(sourcePath)), nil
+	return s, nil
 }
 
 // Exists checks whether a component with the given name exists in the current application or not

pkg/component/component_test.go

@@ -2,18 +2,30 @@ package component
 
 import (
 	"errors"
+	"os"
+	"path"
+	"path/filepath"
 	"reflect"
 	"testing"
 
 	devfilepkg "github.com/devfile/api/v2/pkg/devfile"
+	"github.com/devfile/library/pkg/devfile"
+	"github.com/devfile/library/pkg/devfile/parser"
+	devfileCtx "github.com/devfile/library/pkg/devfile/parser/context"
+	"github.com/devfile/library/pkg/devfile/parser/data"
+	"github.com/devfile/library/pkg/testingutil/filesystem"
+	dfutil "github.com/devfile/library/pkg/util"
 	"github.com/golang/mock/gomock"
 	"github.com/kylelemons/godebug/pretty"
 
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+
 	"github.com/redhat-developer/odo/pkg/kclient"
 	"github.com/redhat-developer/odo/pkg/labels"
+	"github.com/redhat-developer/odo/pkg/testingutil"
+	"github.com/redhat-developer/odo/pkg/util"
 
 	"github.com/redhat-developer/odo/pkg/api"
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 )
 
 func TestListAllClusterComponents(t *testing.T) {
@@ -321,3 +333,117 @@ func TestGetRunningModes(t *testing.T) {
 		})
 	}
 }
+
+func TestGatherName(t *testing.T) {
+	type devfileProvider func() (parser.DevfileObj, string, error)
+	fakeDevfileWithNameProvider := func(name string) devfileProvider {
+		return func() (parser.DevfileObj, string, error) {
+			dData, err := data.NewDevfileData(string(data.APISchemaVersion220))
+			if err != nil {
+				return parser.DevfileObj{}, "", err
+			}
+			dData.SetMetadata(devfilepkg.DevfileMetadata{Name: name})
+			return parser.DevfileObj{
+				Ctx:  devfileCtx.FakeContext(filesystem.NewFakeFs(), parser.OutputDevfileYamlPath),
+				Data: dData,
+			}, "", nil
+		}
+	}
+
+	fs := filesystem.DefaultFs{}
+	//realDevfileWithNameProvider creates a real temporary directory and writes a devfile with the given name to it.
+	//It is the responsibility of the caller to remove the directory.
+	realDevfileWithNameProvider := func(name string) devfileProvider {
+		return func() (parser.DevfileObj, string, error) {
+			dir, err := fs.TempDir("", "Component_GatherName_")
+			if err != nil {
+				return parser.DevfileObj{}, dir, err
+			}
+
+			originalDevfile := testingutil.GetTestDevfileObjFromFile("devfile.yaml")
+			originalDevfilePath := originalDevfile.Ctx.GetAbsPath()
+
+			stat, err := os.Stat(originalDevfilePath)
+			if err != nil {
+				return parser.DevfileObj{}, dir, err
+			}
+			dPath := path.Join(dir, "devfile.yaml")
+			err = dfutil.CopyFile(originalDevfilePath, dPath, stat)
+			if err != nil {
+				return parser.DevfileObj{}, dir, err
+			}
+
+			var d parser.DevfileObj
+			d, _, err = devfile.ParseDevfileAndValidate(parser.ParserArgs{Path: dPath})
+			if err != nil {
+				return parser.DevfileObj{}, dir, err
+			}
+
+			err = d.SetMetadataName(name)
+
+			return d, dir, err
+		}
+	}
+
+	wantDevfileDirectoryName := func(d parser.DevfileObj) string {
+		return util.GetDNS1123Name(filepath.Base(filepath.Dir(d.Ctx.GetAbsPath())))
+	}
+
+	for _, tt := range []struct {
+		name                string
+		devfileProviderFunc devfileProvider
+		wantErr             bool
+		want                func(parser.DevfileObj) string
+	}{
+		{
+			name:                "compliant name",
+			devfileProviderFunc: fakeDevfileWithNameProvider("my-component-name"),
+			want:                func(parser.DevfileObj) string { return "my-component-name" },
+		},
+		{
+			name:                "un-sanitized name",
+			devfileProviderFunc: fakeDevfileWithNameProvider("name with spaces"),
+			want:                func(parser.DevfileObj) string { return "name-with-spaces" },
+		},
+		{
+			name:                "all numeric name",
+			devfileProviderFunc: fakeDevfileWithNameProvider("123456789"),
+			// "x" prefix added by util.GetDNS1123Name
+			want: func(parser.DevfileObj) string { return "x123456789" },
+		},
+		{
+			name:                "no name",
+			devfileProviderFunc: realDevfileWithNameProvider(""),
+			want:                wantDevfileDirectoryName,
+		},
+		{
+			name:                "blank name",
+			devfileProviderFunc: realDevfileWithNameProvider("   "),
+			want:                wantDevfileDirectoryName,
+		},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			d, dir, dErr := tt.devfileProviderFunc()
+			if dir != "" {
+				defer func(fs filesystem.DefaultFs, path string) {
+					if err := fs.RemoveAll(path); err != nil {
+						t.Logf("error while attempting to remove temporary directory %q: %v", path, err)
+					}
+				}(fs, dir)
+			}
+			if dErr != nil {
+				t.Errorf("error when building test Devfile object: %v", dErr)
+				return
+			}
+
+			got, err := GatherName(d)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("error = %v, wantErr %v", err, tt.wantErr)
+			}
+			want := tt.want(d)
+			if !reflect.DeepEqual(got, want) {
+				t.Errorf("GatherName() = %q, want = %q", got, want)
+			}
+		})
+	}
+}

pkg/component/delete/delete.go

@@ -95,7 +95,12 @@ func (do DeleteComponentClient) ListResourcesToDeleteFromDevfile(devfileObj pars
 	if mode == odolabels.ComponentDevMode || mode == odolabels.ComponentAnyMode {
 		// Inner Loop
 		// Fetch the deployment of the devfile component
-		componentName := devfileObj.GetMetadataName()
+		var componentName string
+		componentName, err = component.GatherName(devfileObj)
+		if err != nil {
+			return isInnerLoopDeployed, resources, err
+		}
+
 		var deploymentName string
 		deploymentName, err = util.NamespaceKubernetesObject(componentName, appName)
 		if err != nil {
@@ -155,7 +160,12 @@ func (do *DeleteComponentClient) ExecutePreStopEvents(devfileObj parser.DevfileO
 	if !libdevfile.HasPreStopEvents(devfileObj) {
 		return nil
 	}
-	componentName := devfileObj.GetMetadataName()
+
+	componentName, err := component.GatherName(devfileObj)
+	if err != nil {
+		return err
+	}
+
 	klog.V(4).Infof("Gathering information for component: %q", componentName)
 
 	klog.V(3).Infof("Checking component status for %q", componentName)