Fallback to trying kic image without SHA when using image registry

[ghosind]

Like #11068, minikube with --image-repository option may fail to download kic image because it only tries to download the image with SHA. This patch will add the kic image without SHA to the fallback images list to avoid its failure.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: ghosind
Once this PR has been reviewed and has the lgtm label, please assign spowelljr for approval. For more information see the Code Review Process.

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

Needs approval from an approver in each of these files:

• OWNERS

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

[k8s-ci-robot]

Hi @ghosind. 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-sigs/prow repository.

[minikube-bot]

Can one of the admins verify this patch?

[medyagh]

hi @ghosind do you mind explaining the logic, how this doing it, some of the code is not clear for me.
and also I think if we have to do this, we might wanna output a warnning to user that we chose the image without SHA, since this could be a security issue

[ghosind]

hi @ghosind do you mind explaining the logic, how this doing it, some of the code is not clear for me. and also I think if we have to do this, we might wanna output a warnning to user that we chose the image without SHA, since this could be a security issue

Hi @medyagh,

minikube will try to download kic image with sha, and try the image without sha as fallback:

minikube/pkg/drivers/kic/types.go

Line 42 in 210b148

FallbackImages = []string{

minikube/pkg/minikube/node/cache.go

Line 142 in 7a8a1c9

for _, img := range append([]string{baseImg}, kic.FallbackImages...) {

However, it with --image-registry option only updates the base image's repo, but not the fallback image. So, if the image is unavailable, it never tries the repo updated image without sha.

minikube/pkg/minikube/node/cache.go

Line 127 in 7a8a1c9

baseImg = updateKicImageRepo(baseImg, cc.KubernetesConfig.ImageRepository)

For example, start minikube v1.34.0 with --image-registry=registry.cn-hangzhou.aliyuncs.com/google_containers option, it will try to download the kic base image by the following sequence:

# update from gcr.io/k8s-minikube/kicbase:v0.0.45@sha256:81df288595202a317b1a4dc2506ca2e4ed5f22373c19a441b88cfbf4b9867c85
registry.cn-hangzhou.aliyuncs.com/google_containers/kicbase:v0.0.45@sha256:81df288595202a317b1a4dc2506ca2e4ed5f22373c19a441b88cfbf4b9867c85
docker.io/kicbase/stable:v0.0.45@sha256:81df288595202a317b1a4dc2506ca2e4ed5f22373c19a441b88cfbf4b9867c85
gcr.io/k8s-minikube/kicbase:v0.0.45
docker.io/kicbase/stable:v0.0.45

The image registry.cn-.../kicbase:v0.0.45@sha256:81df... is unavailable, and it will never try to download registry.cn-.../kicbase:v0.0.45 like gcr.io one does. So, this patch will add registry.cn-.../kicbase:v0.0.45 to the fallback list.

[ghosind]

if minikube does not start with --image-registry option, or specify a different kic image that without sha (with --base-image option), it will not add the image without sha into the fallback list

[ghosind]

pkg/driver/kic/types.go did not export the image's sha, so I removed the sha part from the repo updated image.

[ghosind]

As my last reply said, minikube is already trying to download no sha image as the fallback, is it a better idea to make another patch to add warning output? I think it is not for this patch only.