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.

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

Jump to bottom

Improve validation for extra-config. #10886

Merged
merged 9 commits into from
Apr 1, 2021
Merged

Improve validation for extra-config. #10886

Changes from 2 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
11 changes: 11 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,17 @@ func (es *ExtraOptionSlice) Exists(value string) bool {

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

// Check doesn't start or end with an opening quotation.
if strings.HasPrefix(value, "“") || strings.HasSuffix(value, "“") {
return fmt.Errorf("invalid value: canot contain 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.

Can you verify if this is actually true ? Could it be it hangs only if the there is One Qutation without closing ?

Do u mind posting the output of minikube fore and after this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My mistake: There is only hanging currently if at the end there is a quotation mark:
minikube vishal$ ./out/minikube start --driver=docker --extra-config=etcd.client-cert-auth=false“
😄 minikube v1.18.1 on Darwin 10.14.6
✨ Using the docker driver based on existing profile
👍 Starting control plane node minikube in cluster minikube
🏃 Updating the running docker "minikube" container ...
🐳 Preparing Kubernetes v1.20.2 on Docker 20.10.3 ...
▪ etcd.client-cert-auth=false“

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is already an error that shows up when there is a quote at the beginning too:
./out/minikube start --driver=docker --extra-config=“etcd.client-cert-auth=false”
😄 minikube v1.18.1 on Darwin 10.14.6
✨ Using the docker driver based on existing profile
❗ These --extra-config parameters are invalid: [“etcd]

❌ Exiting due to MK_USAGE: Valid components are: [apiserver controller-manager scheduler etcd kubeadm kubelet kube-proxy]

So we need to just check the end of the string for a quote.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

./out/minikube start --driver=docker --extra-config=“etcd.client-cert-auth=false
😄 minikube v1.18.1 on Darwin 10.14.6
✨ Using the docker driver based on existing profile
❗ These --extra-config parameters are invalid: [“etcd]

❌ Exiting due to MK_USAGE: Valid components are: [apiserver controller-manager scheduler etcd kubeadm kubelet kube-proxy]

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 have preserved the initial errors and also added the error when there's a suffix quote at the end:
./out/minikube start --driver=docker --extra-config=etcd.client-cert-auth=false”
Error: invalid argument "etcd.client-cert-auth=false”" for "--extra-config" flag: invalid value: cannot contain end quotation: "etcd.client-cert-auth=false”"

}

// Check doesn't start or end with a closing quotation.
if strings.HasPrefix(value, "”") || strings.HasSuffix(value, "”") {
return fmt.Errorf("invalid value: canot contain quotation: %q", value)
}

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