Meshery: [mesheryctl] Bug: version in config.yaml file is not validated

[navendu-pottekkat]

Description

You can have invalid versions as shown below(version: bob) in the config.yaml file.

user@computer:~/.meshery$ cat config.yaml
contexts:
  local:
    endpoint: http://localhost:9081
    token:
      name: Default
      location: auth.json
    platform: docker
    adapters:
    - meshery-istio
    - meshery-linkerd
    - meshery-consul
    - meshery-octarine
    - meshery-nsm
    - meshery-kuma
    - meshery-cpx
    - meshery-osm
    - meshery-nginx-sm
    channel: stable
    version: bob

current-context: local
tokens: {}

This can break commands like mesheryctl system reset which will try to pull the docker-compose.yaml file from the specified version in GitHub-

user@computer:$ mesheryctl system reset -c local
Meshery config file will be reset to system defaults. Are you sure you want to continue [y/n]? y
Meshery resetting...

Current Context: local
Channel: stable
Version: bob

Fetching default docker-compose file at version: bob...

...Meshery config (/home/user/.meshery/meshery.yaml) now reset to default settings.
user@computer:$ 

That version does not exist and GitHub throws a 404 error. Which is missed by our current error handling function and the meshery.yaml file will be like this-

This is because it actually tries to pull the file https://raw.githubusercontent.com/layer5io/meshery/bob/docker-compose.yaml and does not check if the version is invalid.

This error needs to be handled. For ex, here it should throw an error saying the version is invalid and should not try to reset with an invalid version.

Possible fixes

A quick fix would be to check if the response body is 404: Not Found.

But, a better and robust fix would be to have a central validator with the MesheryCtlConfig struct in the config.go file. This would be helpful if we require version validation for other commands.

The validation could be done using Regex.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[navendu-pottekkat]

@metonymic-smokey This might be a good issue to take up

[metonymic-smokey]

Thanks @navendu-pottekkat , I shall look into this.

[metonymic-smokey]

@navendu-pottekkat
I had a couple of queries:

1. I thought of an alternative way of validating the version in config.go by creating a function similar to CheckIfCurrentContextIsValid for version and using that in all command requiring a version check.
   Does this sound viable?

1. The version can be either latest or a string conforming to the regex v[0-9]+.[0-9]+.[0-9]+[-a-z0-9]* right? This is what I observed from the Release tags.

Thanks!

[navendu-pottekkat]

@metonymic-smokey Yes that would be good idea.

I think there is already a regex comparison for the version somewhere in the mesheryctl code.

You can create and function as you mentioned and use it for checking...

[metonymic-smokey]

@navendu-pottekkat ,

Since each context has a separate version, I am calling the function to check valid version from within CheckIfCurrentContextIsValid in config.go.
This is the output I get for an invalid context as shown in the issue:

Is this alright or should I check for the version of each context?

Thanks!

[navendu-pottekkat]

That was fast! Could you make a draft PR so that I can take a look?

In the output above, the context passed in the flag is wrong right? So it checks current-context?

[metonymic-smokey]

I am just using the default current context(since the -c flag is optional). Should I do it differently?

[navendu-pottekkat]

I might be missing something. I will check the draft PR so we are on the same page.