"odo registry add" adds registry for invalid url in devfileV2

[prietyc123]

/kind bug

What versions of software are you using?

Operating System:
All

Output of odo version:
master

How did you run odo exactly?

Steps to reproduce:

1. Enable experimental mode odo preference set Experimental true
2. odo registry add wRegistry "http://api.google.com/q?exp=a|b" with invalid url
3. odo registry list

Actual behavior

$ odo preference view
PARAMETER             CURRENT_VALUE
UpdateNotification    false
NamePrefix            
Timeout               
PushTimeout           
Experimental          true

$ odo registry add wRegistry "http://api.google.com/q?exp=a|b"
New registry successfully added

$ odo registry listNAME                       URL
CheDevfileRegistry         https://che-devfile-registry.openshift.io
DefaultDevfileRegistry     https://raw.githubusercontent.com/elsony/devfile-registry/master
demoRegistry               http://docker.io
wrongRegistry              http://www.xyz.com
wRegistry                  http://api.google.com/q?exp=a|b

Expected behavior

odo registry add should throw error for invalid url

Any logs, error output, etc?

Manually verified.

[prietyc123]

Even I tried with $ also but somehow it is converting the $ into some random number.

$ odo registry add wRegistry "http://$$$$,,,,,@hdjh.hcgcyd"
New registry successfully added
$ odo registry list
NAME                       URL
wRegistry                  http://1624716247,,,,,@hdjh.hcgcyd

This looks something weird to me.

[kadel]

This shouldn't be a high priority.

/remove-priority high
/priority low

We intentionally don't validate that the registry is accessible and in odo registry add command.

I think that http://api.google.com/q?exp=a|b is a valid URL.

http://$$$$,,,,,@hdjh.hcgcyd is another story, that is clearly invalid URL.

We definitely don't want to check if URL is available or not in odo registry add.

But it might make sense to add format validation that would check for proper URL format.

[mik-dass]

We can maybe do some basic URL validation using ParseRequestURI() from the url package.

[GeekArthur]

Regarding arguments/flags validation, the design is that we divide the validation into two stages:
Early validation stage: Only validate URL basic format, happens when user adds registry URL
Validation stage: Validate URL accessibility, happens when user uses registry URL

That design is based on yum-config-manager, design proposal with details under this thread: #2940 (comment)

@prietyc123 In short, all the inaccessible/invalid URL will be detected/validated when users use the URL by running odo catalog, odo create, etc

[prietyc123]

I think that http://api.google.com/q?exp=a|b is a valid URL.

I don't find it a valid url as it contains | which already has some special meaning. IMO if it is encoded as http://api.google.com/q?exp=a%7Cb then we can consider this a valid.

http://$$$$,,,,,@hdjh.hcgcyd is another story, that is clearly invalid URL.

Yes, for such kind of urls we should add some format validation. Also we can consider to exclude those url which has some special meaning like | and $

But it might make sense to add format validation that would check for proper URL format.

+1

[GeekArthur]

@mik-dass We do have the URL validation function here: https://github.com/openshift/odo/blob/master/pkg/util/util.go#L1139, it uses url.Parse package to implement the basic URL format validation which is similar to use url.ParseRequestURI as both of the packages are used to parse and get the essential components of the URL for analysis. The original design of the basic URL format validation is that the URL is valid if it has both scheme and host.

Regarding the discussion above, based on the current RFC https://www.ietf.org/rfc/rfc3986.txt, I agree with @kadel, this URL http://api.google.com/q?exp=a|b is valid as | is not reserved character, it may throw URISyntaxException error in Java code but that doesn't apply to our case because odo uses Golang.

My suggestion is that we can add the code that excluding the reserved character for the essential components to our existing basic URL format validation.