Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"odo config set" doesn't validate port values and thus "odo describe" panics #3036

Closed
dharmit opened this issue Apr 28, 2020 · 2 comments · Fixed by #3259
Closed

"odo config set" doesn't validate port values and thus "odo describe" panics #3036

dharmit opened this issue Apr 28, 2020 · 2 comments · Fixed by #3259
Assignees
Labels
good first issue Denotes an issue ready for a new contributor. Maintainers will help mentor new contributors. kind/bug Categorizes issue or PR as related to a bug.
Projects

Comments

@dharmit
Copy link
Member

dharmit commented Apr 28, 2020

/kind bug

What versions of software are you using?

Operating System: all

Output of odo version: master

How did you run odo exactly?

#3008 (comment)

$ odo config set ports 909,808 -f
 ✓  Local config successfully updated

Run `odo push --config` command to apply changes to the cluster

$ odo describe 
panic: runtime error: index out of range [1] with length 1

goroutine 1 [running]:
github.com/openshift/odo/pkg/component.getPortsProtocolMapping(0xc0006136c0, 0x2, 0x2, 0x0)
        /home/mrinaldas/go/src/github.com/openshift/odo/pkg/component/component.go:991 +0x177
github.com/openshift/odo/pkg/component.GetComponentFromConfig(0xc000ba7500, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
        /home/mrinaldas/go/src/github.com/openshift/odo/pkg/component/component.go:962 +0x6d6
github.

Actual behavior

odo describe panics because odo config set doesn't perform a check for ports provided by the user

Expected behavior

odo should validate that provided value for ports follow <port>/<protocol> format and the values are valid.

Any logs, error output, etc?

@openshift-ci-robot openshift-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Apr 28, 2020
@girishramnani girishramnani added the good first issue Denotes an issue ready for a new contributor. Maintainers will help mentor new contributors. label May 11, 2020
@dharmit dharmit added this to For consideration in Sprint 184 via automation May 11, 2020
@kadel
Copy link
Member

kadel commented May 26, 2020

/approve @dev-gaur

@devang-gaur
Copy link
Contributor

I tried reproducing this issue but could not.
After a discussion with @dharmit , I found out that this has been partially fixed by #3224 and the rest of the problem that still persists is depicted in #3229.

Nevertheless, I feel that there should be proper validation for config parameters provided by user via odo config set param value command, which is currently absent right now : https://github.com/openshift/odo/blob/master/pkg/odo/cli/config/set.go#L62 .

Therefore, to add validations, I've created #3259 .

@girishramnani girishramnani removed this from To do in Sprint 184 Jun 1, 2020
@girishramnani girishramnani added this to For consideration in Sprint 185 via automation Jun 1, 2020
@girishramnani girishramnani moved this from For consideration to To do in Sprint 185 Jun 1, 2020
@girishramnani girishramnani added this to For consideration in Sprint 186 via automation Jun 22, 2020
@girishramnani girishramnani removed this from To do in Sprint 185 Jun 22, 2020
@girishramnani girishramnani moved this from For consideration to To do in Sprint 186 Jun 22, 2020
@dharmit dharmit moved this from To do to For review in Sprint 186 Jun 22, 2020
Sprint 186 automation moved this from For review to Done Jul 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Denotes an issue ready for a new contributor. Maintainers will help mentor new contributors. kind/bug Categorizes issue or PR as related to a bug.
Projects
No open projects
Sprint 186
  
Done
Development

Successfully merging a pull request may close this issue.

5 participants