v1.7.0 regression when pulling images with --cache-repo and --mirror combined

[BarthV]

Actual behavior
Since v1.7.0 release my builds are broken, reverting the build image to v1.6.0 make them work again.

Error is :

Error while retrieving image from cache: ubuntu:bionic strict validation requires the registry to be explicitly defined

executor cmd (running in gitlab runner context, using gcr mirror + private registry used for layer caching) :

    - /kaniko/executor
      --registry-mirror mirror.gcr.io
      --cache=true
      --cache-repo=${GCR_CACHE_REGISTRY}/${CI_PROJECT_ID}
      --context ${CI_PROJECT_DIR}
      --reproducible
      --destination ${SHA_NAME}
      --destination ${IMG_NAME}:latest${CI_COMMIT_REF_SLUG:+-$CI_COMMIT_REF_SLUG}

Dockerfile is starting with a really simple & basic :

FROM ubuntu:bionic as base-img
ENV LANG=C.UTF-8 \
    LC_ALL=C.UTF-8

RUN apt-get update && apt-get install -q -y \
        curl \
        cmake \
        ...

Expected behavior
Build is running exactly like in v1.6.0

Additional Information

• gcr.io/kaniko-project/executor:debug was used
• feel free to ask for more info

Triage Notes for the Maintainers

Description	Yes/No
Please check if this a new feature you are proposing
Please check if the build works in docker but not in kaniko
Please check if this error is seen when you use --cache flag
Please check if your dockerfile is a multistage dockerfile

[maximegaillard]

Same issue on our side 👍

[brogon]

Sorry - that's actually my analysis for #1789, it may apply or not apply here, too. (Posted it on the wrong thread).

---

The problem is, with #1707, the "registry mirror" is no longer interpreted as "registry", but as "repository". It essentially expects a path after the servername/port (i.e. something with points and/or colon in it). Otherwise it's interpreted as path within a repository, which must not contain colons.

I.e. by using name.NewRepository(...) instead of name.NewRegistry(...) in pkg/image/remote/remote.go, it's no longer correctly interpreted. As this functionality comes from a third-party library, the "pure registry" case should be already handled in either the remote.go file, or even before (e.g. adapting a "pure" registry from the --registry-mirror option so it's handled correctly afterwards - like appending a "/" if it doesn't contain a separator already).

[tejal29]

Thanks @brogon Do you have a fix in mind?

[BarthV]

Thanks @brogon Do you have a fix in mind?

IMO pulling from a mirror repository should have been a complete new flag arg.
Now we're supposed to mix in the same arg "registry" mirror mode together with "repository" mirror mode ... and it's not perfectly clean :/

A repository is validated with the following sanity check :

	parts := strings.SplitN(name, regRepoDelimiter, 2)
	if len(parts) == 2 && (strings.ContainsRune(parts[0], '.') || strings.ContainsRune(parts[0], ':')) {
		// The first part of the repository is treated as the registry domain
		// iff it contains a '.' or ':' character, otherwise it is all repository
		// and the domain defaults to Docker Hub.
		registry = parts[0]
		repo = parts[1]
	}
	
	if err := checkRepository(repo); err != nil {
		return Repository{}, err
	}

Btw a simple hostname doesn't pass this test.

As a quick workaround it's really simple to make RetrieveRemoteImage func detecting the syntaxt of the --mirror arg and build the image ref using both a registry, or a repository (depending what's detected). But this time we should really make it testable !

[aaron-prindle]

Can someone in the thread here confirm if this is still an issue with the latest release/later-releases of kaniko? The PR here (merged Oct 21st 2021) - #1794 mentions it fixes this issue but I would like additional confirmation as a related issue - issue #1791 appears to be ongoing even after PR #1794 was merged event though it mentioned if fixed #1791 as well