Skip to content

Improve validation for extra-config. #10886

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.

Sign up for GitHub

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

Merged
merged 9 commits into from
Apr 1, 2021
Merged

Improve validation for extra-config. #10886

Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c402edd
Added error for quotation.
vishjain Mar 21, 2021
e916793
Add check for opening and closing quote.
vishjain Mar 21, 2021
e1f9fb1
Check suffix ends with quotation.
vishjain Mar 24, 2021
28f2697
Fixed end quote.
vishjain Mar 24, 2021
198fa8d
Improve error message.
vishjain Mar 26, 2021
b565130
Added unit test.
vishjain Mar 28, 2021
bdd4aa1
Added comment.
vishjain Mar 28, 2021
21477e3
Ran make lint
vishjain Mar 30, 2021
f125300
Reset test.
vishjain Mar 30, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions pkg/minikube/config/extra_options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,14 @@ func (es *ExtraOptionSlice) Exists(value string) bool {

// Set parses the string value into a slice
func (es *ExtraOptionSlice) Set(value string) error {

// Check we don't end with suffix quotation.
prefixExists := strings.HasPrefix(value, "”") || strings.HasPrefix(value, "“")
suffixExists := strings.HasSuffix(value, "”") || strings.HasSuffix(value, "“")
if !prefixExists && suffixExists {
return fmt.Errorf("invalid value: cannot contain end quotation: %q", value)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happens when error happens ? could u paste in the PR description

Before and After this PR ?
how about adding Unit test for Set Function ? with example of things that should Error and example of things that should NOT error

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also improve the error wording, -extra-config

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a test and in the PR description above I've included what happens when we do the error checking. We are stopping he hanging occurring when a extra-config value ends with a open or close quote. It's important to note if the extra-config value has a prefix with an open or close quote then there's a separate error check in start.go file (cli) - I have left that alone.

}

// The component is the value before the first dot.
componentSplit := strings.SplitN(value, ".", 2)
if len(componentSplit) < 2 {
Expand Down