Fixes Complete() and Validate() code in envinfo.go and also adds comm…

…ents.

pkg/config/url.go

@@ -8,7 +8,8 @@ import (
 	"github.com/openshift/odo/pkg/util"
 )
 
-// GetPorts returns the ports, returns default if nil
+// GetPorts returns the ports stored in the config for the component
+// returns default i.e nil if nil
 func (lc *LocalConfig) GetPorts() []string {
 	if lc.componentSettings.Ports == nil {
 		return nil

pkg/envinfo/envinfo.go

@@ -379,7 +379,8 @@ func (ei *EnvInfo) MatchComponent(name, app, namespace string) bool {
 	return name == ei.GetName() && app == ei.GetApplication() && namespace == ei.GetNamespace()
 }
 
-func (ei *EnvInfo) SetDevfile(devfileObj parser.DevfileObj) {
+// SetDevfileObj sets the devfileObj for the envinfo
+func (ei *EnvInfo) SetDevfileObj(devfileObj parser.DevfileObj) {
 	ei.devfileObj = devfileObj
 }
 

pkg/envinfo/envinfo_test.go

@@ -239,7 +239,7 @@ func TestDeleteURLFromMultipleURLs(t *testing.T) {
 				t.Error(err)
 			}
 			esi.EnvInfo = tt.existingEnvInfo
-			esi.SetDevfile(tt.existingDevfile)
+			esi.SetDevfileObj(tt.existingDevfile)
 
 			oldURLLength := len(esi.ListURLs())
 			err = esi.DeleteURL(tt.deleteParam)

pkg/envinfo/url.go

@@ -49,27 +49,43 @@ func (ei *EnvInfo) CompleteURL(url *localConfigProvider.LocalURL) error {
 	// add leading / to path, if the path provided is empty, it will be set to / which is the default value of path
 	url.Path = "/" + url.Path
 
+	// get the port if not provided
+	if url.Port == -1 {
+		var err error
+		url.Port, err = util.GetValidPortNumber(ei.GetName(), url.Port, ei.GetPorts())
+		if err != nil {
+			return err
+		}
+	}
+
+	// get the name for the URL if not provided
+	if len(url.Name) == 0 {
+		url.Name = util.GetURLName(ei.GetName(), url.Port)
+	}
+
 	containerComponents := ei.devfileObj.Data.GetDevfileContainerComponents()
 	if len(containerComponents) == 0 {
-		return nil
+		return fmt.Errorf("no valid components found in the devfile")
 	}
 
+	// if a container name is provided for the URL, return
 	if len(url.Container) > 0 {
 		return nil
 	}
 
 	containerPortMap := make(map[int]string)
 	portMap := make(map[string]bool)
+
+	// if a container name for the URL is not provided
+	// use a container which uses the given URL port in one of it's endpoints
 	for _, component := range containerComponents {
 		for _, endpoint := range component.Container.Endpoints {
 			containerPortMap[endpoint.TargetPort] = component.Name
 			portMap[strconv.FormatInt(int64(endpoint.TargetPort), 10)] = true
 		}
 	}
 	if containerName, exist := containerPortMap[url.Port]; exist {
-		if len(url.Container) == 0 {
-			url.Container = containerName
-		}
+		url.Container = containerName
 	}
 
 	// container is not provided, or the specified port is not being used under any containers
@@ -78,19 +94,6 @@ func (ei *EnvInfo) CompleteURL(url *localConfigProvider.LocalURL) error {
 		url.Container = containerComponents[0].Name
 	}
 
-	if url.Port == -1 {
-		var err error
-		url.Port, err = util.GetValidPortNumber(ei.GetName(), url.Port, ei.GetPorts())
-		if err != nil {
-			return err
-		}
-	}
-
-	// get the name
-	if len(url.Name) == 0 {
-		url.Name = util.GetURLName(ei.GetName(), url.Port)
-	}
-
 	return nil
 }
 
@@ -137,6 +140,7 @@ func (ei *EnvInfo) ValidateURL(url localConfigProvider.LocalURL) error {
 	if url.TLSSecret != "" && (url.Kind != localConfigProvider.INGRESS || !url.Secure) {
 		errorList = append(errorList, "TLS secret is only available for secure URLs of Ingress kind")
 	}
+
 	// check if a host is provided for route based URLs
 	if len(url.Host) > 0 {
 		if url.Kind == localConfigProvider.ROUTE {
@@ -148,10 +152,27 @@ func (ei *EnvInfo) ValidateURL(url localConfigProvider.LocalURL) error {
 	} else if url.Kind == localConfigProvider.INGRESS {
 		errorList = append(errorList, "host must be provided in order to create URLS of Ingress Kind")
 	}
-	if len(url.Protocol) > 0 && (strings.ToLower(url.Protocol) != string(devfilev1.HTTPEndpointProtocol) && strings.ToLower(url.Protocol) != string(devfilev1.HTTPSEndpointProtocol) && strings.ToLower(url.Protocol) != string(devfilev1.WSEndpointProtocol) &&
-		strings.ToLower(url.Protocol) != string(devfilev1.WSSEndpointProtocol) && strings.ToLower(url.Protocol) != string(devfilev1.TCPEndpointProtocol) && strings.ToLower(url.Protocol) != string(devfilev1.UDPEndpointProtocol)) {
-		errorList = append(errorList, fmt.Sprintf("endpoint protocol only supports %v|%v|%v|%v|%v|%v", devfilev1.HTTPEndpointProtocol, devfilev1.HTTPSEndpointProtocol, devfilev1.WSSEndpointProtocol, devfilev1.WSEndpointProtocol, devfilev1.TCPEndpointProtocol, devfilev1.UDPEndpointProtocol))
+
+	// check the protocol of the URL
+	if len(url.Protocol) > 0 {
+		switch strings.ToLower(url.Protocol) {
+		case string(devfilev1.HTTPEndpointProtocol):
+			break
+		case string(devfilev1.HTTPSEndpointProtocol):
+			break
+		case string(devfilev1.WSEndpointProtocol):
+			break
+		case string(devfilev1.WSSEndpointProtocol):
+			break
+		case string(devfilev1.TCPEndpointProtocol):
+			break
+		case string(devfilev1.UDPEndpointProtocol):
+			break
+		default:
+			errorList = append(errorList, fmt.Sprintf("endpoint protocol only supports %v|%v|%v|%v|%v|%v", devfilev1.HTTPEndpointProtocol, devfilev1.HTTPSEndpointProtocol, devfilev1.WSSEndpointProtocol, devfilev1.WSEndpointProtocol, devfilev1.TCPEndpointProtocol, devfilev1.UDPEndpointProtocol))
+		}
 	}
+
 	for _, localURL := range ei.ListURLs() {
 		if url.Name == localURL.Name {
 			errorList = append(errorList, fmt.Sprintf("URL %s already exists", url.Name))

pkg/envinfo/url_test.go

@@ -181,6 +181,7 @@ func TestEnvInfo_CompleteURL(t *testing.T) {
 				Secure: false,
 				Path:   "/",
 			},
+			wantErr: true,
 		},
 		{
 			name: "case 9: complete the url name if not provided",

pkg/odo/cli/url/create.go

@@ -60,11 +60,11 @@ type URLCreateOptions struct {
 	urlPort     int
 	secureURL   bool
 	now         bool
-	host        string
-	tlsSecret   string
-	path        string
-	protocol    string
-	container   string
+	host        string // host of the URL
+	tlsSecret   string // tlsSecret is the secret to te used by the URL
+	path        string // path of the URL
+	protocol    string // protocol of the URL
+	container   string // container to which the URL belongs
 	wantIngress bool
 	url         localConfigProvider.LocalURL
 }
@@ -76,10 +76,11 @@ func NewURLCreateOptions() *URLCreateOptions {
 
 // Complete completes URLCreateOptions after they've been Created
 func (o *URLCreateOptions) Complete(_ string, cmd *cobra.Command, args []string) (err error) {
-	o.Context, err = genericclioptions.New(genericclioptions.ContextOptions{
+	o.Context, err = genericclioptions.New(genericclioptions.CreateParameters{
 		Cmd:              cmd,
 		DevfilePath:      o.DevfilePath,
 		ComponentContext: o.GetComponentContext(),
+		IsNow:            o.now,
 	})
 
 	if err != nil {

pkg/odo/cli/url/delete.go

@@ -38,7 +38,7 @@ func NewURLDeleteOptions() *URLDeleteOptions {
 
 // Complete completes URLDeleteOptions after they've been Deleted
 func (o *URLDeleteOptions) Complete(name string, cmd *cobra.Command, args []string) (err error) {
-	o.Context, err = genericclioptions.New(genericclioptions.ContextOptions{
+	o.Context, err = genericclioptions.New(genericclioptions.CreateParameters{
 		Cmd:              cmd,
 		DevfilePath:      o.DevfilePath,
 		ComponentContext: o.GetComponentContext(),

pkg/odo/genericclioptions/context.go

@@ -50,52 +50,54 @@ type internalCxt struct {
 	LocalConfigProvider localConfigProvider.LocalConfigProvider
 }
 
-type ContextOptions struct {
+// CreateParameters defines the options which can be provided while creating the context
+type CreateParameters struct {
 	Cmd              *cobra.Command
 	DevfilePath      string
 	ComponentContext string
 	IsNow            bool
 }
 
-func New(options ContextOptions, toggles ...bool) (*Context, error) {
-	options.DevfilePath = completeDevfilePath(options.ComponentContext, options.DevfilePath)
+// New creates a context based on the given parameters
+func New(parameters CreateParameters, toggles ...bool) (context *Context, err error) {
+	parameters.DevfilePath = completeDevfilePath(parameters.ComponentContext, parameters.DevfilePath)
 
-	var context *Context
-	isDevfile := odoutil.CheckPathExists(options.DevfilePath)
+	isDevfile := odoutil.CheckPathExists(parameters.DevfilePath)
 	if isDevfile {
-		context = NewDevfileContext(options.Cmd)
-		context.ComponentContext = options.ComponentContext
+		context = NewDevfileContext(parameters.Cmd)
+		context.ComponentContext = parameters.ComponentContext
 
-		err := context.InitEnvInfoFromContext()
+		err = context.InitEnvInfoFromContext()
 		if err != nil {
 			return nil, err
 		}
 
 		// Parse devfile and validate
-		devObj, err := devfile.ParseAndValidate(options.DevfilePath)
+		devObj, err := devfile.ParseAndValidate(parameters.DevfilePath)
 		if err != nil {
-			return context, fmt.Errorf("failed to parse the devfile %s, with error: %s", options.DevfilePath, err)
+			return context, fmt.Errorf("failed to parse the devfile %s, with error: %s", parameters.DevfilePath, err)
 		}
 		err = validate.ValidateDevfileData(devObj.Data)
 		if err != nil {
 			return context, err
 		}
-		context.EnvSpecificInfo.SetDevfile(devObj)
+
+		context.EnvSpecificInfo.SetDevfileObj(devObj)
 		context.LocalConfigProvider = context.EnvSpecificInfo
-	} else if options.IsNow {
-		context = NewContextCreatingAppIfNeeded(options.Cmd)
-		context.ComponentContext = options.ComponentContext
+	} else if parameters.IsNow {
+		context = NewContextCreatingAppIfNeeded(parameters.Cmd)
+		context.ComponentContext = parameters.ComponentContext
 
-		err := context.InitConfigFromContext()
+		err = context.InitConfigFromContext()
 		if err != nil {
 			return nil, err
 		}
 		context.LocalConfigProvider = context.LocalConfigInfo
 	} else {
-		context = NewContext(options.Cmd)
-		context.ComponentContext = options.ComponentContext
+		context = NewContext(parameters.Cmd)
+		context.ComponentContext = parameters.ComponentContext
 
-		err := context.InitConfigFromContext()
+		err = context.InitConfigFromContext()
 		if err != nil {
 			return nil, err
 		}
@@ -123,7 +125,7 @@ func (o *Context) InitEnvInfoFromContext() (err error) {
 	return nil
 }
 
-// CompleteDevfilePath completes the devfile path from context
+// completeDevfilePath completes the devfile path from context
 func completeDevfilePath(componentContext, devfilePath string) string {
 	if len(devfilePath) > 0 {
 		return filepath.Join(componentContext, devfilePath)