Added validation for --insecure-registry values #9977

kadern0 · 2020-12-17T00:16:04Z

Fixes 8790

Original reported behavior (slightly summarized):

minikube start --driver=docker --profile=testing3 --insecure-registry=:
😄  [testing3] minikube v1.13.0 on Ubuntu 20.04
✨  Using the docker driver based on user configuration
👍  Starting control plane node testing3 in cluster testing3
🔥  Creating docker container (CPUs=2, Memory=3900MB) ...
✋  Stopping node "testing3"  ...
🛑  Powering off "testing3" via SSH ...
🔥  Deleting "testing3" in docker ...
🤦  StartHost failed, but will try again: creating host: create: provisioning: ssh command error:

...

❌  Exiting due to RT_DOCKER_RESTART: Failed to start host: creating host: create: provisioning: ssh command error:
command : sudo diff -u /lib/systemd/system/docker.service /lib/systemd/system/docker.service.new || { sudo mv /lib/systemd/system/docker.service.new /lib/systemd/system/docker.service; sudo systemctl -f daemon-reload && sudo systemctl -f enable docker && sudo systemctl -f restart docker; }
err     : Process exited with status 1


...

💡  Suggestion: Remove the invalid --docker-opt or --insecure-registry flag if one was provided
🍿  Related issue: https://github.com/kubernetes/minikube/issues/7070

New behavior:

minikube start --driver=docker --profile=testingt3 --insecure-registry=:
😄  [testingt3] minikube v1.13.0 on Ubuntu 20.04
✨  Using the docker driver based on user configuration

❌  Exiting due to MK_USAGE: Sorry, the address provided with the --insecure-registry flag is invalid: :

Additional validations:

minikube start --driver=docker --profile=testingt --insecure-registry 172.16.23/24
😄  [testingt] minikube v1.13.0 on Ubuntu 20.04
✨  Using the docker driver based on existing profile

❌  Exiting due to MK_USAGE: Sorry, the address provided with the --insecure-registry flag is invalid: 172.16.23/24

minikube start --driver=docker --profile=testingt --insecure-registry 172.16.23/24:asdf
😄  [testingt] minikube v1.13.0 on Ubuntu 20.04
✨  Using the docker driver based on existing profile

❌  Exiting due to MK_USAGE: Sorry, the address provided with the --insecure-registry flag is invalid: 172.16.23/24:asdf

minikube start --driver=docker --profile=testingt --insecure-registry 172.16.23/24:
😄  [testingt] minikube v1.13.0 on Ubuntu 20.04
✨  Using the docker driver based on existing profile

❌  Exiting due to MK_USAGE: Sorry, the address provided with the --insecure-registry flag is invalid: 172.16.23/24:

minikube start --driver=docker --profile=testingt3 --insecure-registry=192.168.100.123
😄  [testingt3] minikube v1.13.0 on Ubuntu 20.04
✨  Using the docker driver based on existing profile

❌  Exiting due to MK_USAGE: Sorry, the address provided with the --insecure-registry flag is invalid: 192.168.100.123

minikube start --driver=docker --profile=testingt3 --insecure-registry=localhost
😄  [testingt3] minikube v1.13.0 on Ubuntu 20.04
✨  Using the docker driver based on existing profile

❌  Exiting due to MK_USAGE: Sorry, the address provided with the --insecure-registry flag is invalid: localhost

k8s-ci-robot · 2020-12-17T00:16:12Z

Hi @kadern0. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

minikube-bot · 2020-12-17T00:21:03Z

Can one of the admins verify this patch?

tstromberg · 2020-12-17T16:12:07Z

cmd/minikube/cmd/start.go

@@ -1060,6 +1064,35 @@ func validateRegistryMirror() {
 	}
 }

+// This function validates that the --insecure-registry follows one of the following formats:
+// "<ip>:<port>" "<hostname>:<port>" "<network>/<netmask>"
+func ValidateInsecureRegistry() {


this should be a private function -- and probably moved to start_flags.go.

Agree with the private function, about the location I've placed it under the "validateRegistryMirror" function since they do pretty much the same. To me it makes sense having both together, what do you think?

cmd/minikube/cmd/start.go

Fixes 8790

medyagh · 2020-12-22T00:10:35Z

cmd/minikube/cmd/start.go

@@ -71,6 +72,7 @@ var (
 	insecureRegistry []string
 	apiServerNames   []string
 	apiServerIPs     []net.IP
+	hostRe           = regexp.MustCompile(`[\w\.-]+`)


add a comment here

Do you still want me to add the comment?

k8s-ci-robot · 2020-12-22T00:17:11Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kadern0, medyagh

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [medyagh]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

crossdot · 2021-02-03T12:34:44Z

Why hostname without port is not allowed? It works fine in previous versions.
Using JFrog Artifactory as image registry and for example localregistry:443/nginx:1.18 in deployment looks terrible.

kadern0 · 2021-02-03T21:42:53Z

@crossdot we are discussing about this on the issue's URL.

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Dec 17, 2020

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Dec 17, 2020

k8s-ci-robot requested review from medyagh and tstromberg December 17, 2020 00:16

k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Dec 17, 2020

tstromberg suggested changes Dec 17, 2020

View reviewed changes

Added validation for --insecure-registry values

dbbe066

Fixes 8790

kadern0 force-pushed the issue-8790 branch from 3806dbf to dbbe066 Compare December 19, 2020 23:00

medyagh approved these changes Dec 22, 2020

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 22, 2020

medyagh merged commit c300a05 into kubernetes:master Dec 22, 2020

kadern0 mentioned this pull request Jan 6, 2021

bad --insecure-registry values cause: Job for docker.service failed because the control process exited with error code. #8790

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added validation for --insecure-registry values #9977

Added validation for --insecure-registry values #9977

kadern0 commented Dec 17, 2020

k8s-ci-robot commented Dec 17, 2020

minikube-bot commented Dec 17, 2020

tstromberg Dec 17, 2020

kadern0 Dec 17, 2020

medyagh Dec 22, 2020

kadern0 Dec 22, 2020

k8s-ci-robot commented Dec 22, 2020

crossdot commented Feb 3, 2021 •

edited

Loading

kadern0 commented Feb 3, 2021

Added validation for --insecure-registry values #9977

Added validation for --insecure-registry values #9977

Conversation

kadern0 commented Dec 17, 2020

k8s-ci-robot commented Dec 17, 2020

minikube-bot commented Dec 17, 2020

tstromberg Dec 17, 2020

Choose a reason for hiding this comment

kadern0 Dec 17, 2020

Choose a reason for hiding this comment

medyagh Dec 22, 2020

Choose a reason for hiding this comment

kadern0 Dec 22, 2020

Choose a reason for hiding this comment

k8s-ci-robot commented Dec 22, 2020

crossdot commented Feb 3, 2021 • edited Loading

kadern0 commented Feb 3, 2021

crossdot commented Feb 3, 2021 •

edited

Loading