diff --git a/pkg/assets/builder.go b/pkg/assets/builder.go index 7947ce969f039..0fd11d1a6f785 100644 --- a/pkg/assets/builder.go +++ b/pkg/assets/builder.go @@ -144,7 +144,7 @@ func (a *AssetBuilder) RemapManifest(data []byte) ([]byte, error) { } // RemapImage normalizes a containers location if a user sets the AssetsLocation ContainerRegistry location. -func (a *AssetBuilder) RemapImage(image string) (string, error) { +func (a *AssetBuilder) RemapImage(image string) string { asset := &ImageAsset{ DownloadLocation: image, CanonicalLocation: image, @@ -179,66 +179,27 @@ func (a *AssetBuilder) RemapImage(image string) (string, error) { } } - if a.AssetsLocation != nil && a.AssetsLocation.ContainerProxy != nil { - containerProxy := strings.TrimSuffix(*a.AssetsLocation.ContainerProxy, "/") - normalized := image - - // If the image name contains only a single / we need to determine if the image is located on docker-hub or if it's using a convenient URL, - // like registry.k8s.io/ or registry.k8s.io/ - // In case of a hub image it should be sufficient to just prepend the proxy url, producing eg docker-proxy.example.com/weaveworks/weave-kube - if strings.Count(normalized, "/") <= 1 && !strings.ContainsAny(strings.Split(normalized, "/")[0], ".:") { - normalized = containerProxy + "/" + normalized - } else { - re := regexp.MustCompile(`^[^/]+`) - normalized = re.ReplaceAllString(normalized, containerProxy) - } - - asset.DownloadLocation = normalized - - // Run the new image - image = asset.DownloadLocation - } - - if a.AssetsLocation != nil && a.AssetsLocation.ContainerRegistry != nil { - registryMirror := *a.AssetsLocation.ContainerRegistry - normalized := image - - // Remove the 'standard' kubernetes image prefixes, just for sanity - normalized = strings.TrimPrefix(normalized, "registry.k8s.io/") - - // When assembling the cluster spec, kops may call the option more then once until the config converges - // This means that this function may me called more than once on the same image - // It this is pass is the second one, the image will already have been normalized with the containerRegistry settings - // If this is the case, passing though the process again will re-prepend the container registry again - // and again, causing the spec to never converge and the config build to fail. - if !strings.HasPrefix(normalized, registryMirror+"/") { - // We can't nest arbitrarily - // Some risk of collisions, but also -- and __ in the names appear to be blocked by docker hub - normalized = strings.Replace(normalized, "/", "-", -1) - asset.DownloadLocation = registryMirror + "/" + normalized - } - - // Run the new image - image = asset.DownloadLocation - } + normalized := NormalizeImage(a, image) + image = normalized + asset.DownloadLocation = normalized a.ImageAssets = append(a.ImageAssets, asset) if !featureflag.ImageDigest.Enabled() || os.Getenv("KOPS_BASE_URL") != "" { - return image, nil + return image } if strings.Contains(image, "@") { - return image, nil + return image } digest, err := crane.Digest(image, crane.WithAuthFromKeychain(authn.DefaultKeychain)) if err != nil { klog.Warningf("failed to digest image %q: %s", image, err) - return image, nil + return image } - return image + "@" + digest, nil + return image + "@" + digest } // RemapFile returns a remapped URL for the file, if AssetsLocation is defined. @@ -378,3 +339,46 @@ func (a *AssetBuilder) remapURL(canonicalURL *url.URL) (*url.URL, error) { return fileRepo, nil } + +func NormalizeImage(a *AssetBuilder, image string) string { + if a.AssetsLocation != nil && a.AssetsLocation.ContainerProxy != nil { + containerProxy := strings.TrimSuffix(*a.AssetsLocation.ContainerProxy, "/") + normalized := image + + // If the image name contains only a single / we need to determine if the image is located on docker-hub or if it's using a convenient URL, + // like registry.k8s.io/ or registry.k8s.io/ + // In case of a hub image it should be sufficient to just prepend the proxy url, producing eg docker-proxy.example.com/weaveworks/weave-kube + if strings.Count(normalized, "/") <= 1 && !strings.ContainsAny(strings.Split(normalized, "/")[0], ".:") { + normalized = containerProxy + "/" + normalized + } else { + re := regexp.MustCompile(`^[^/]+`) + normalized = re.ReplaceAllString(normalized, containerProxy) + } + + // Run the new image + image = normalized + } + + if a.AssetsLocation != nil && a.AssetsLocation.ContainerRegistry != nil { + registryMirror := *a.AssetsLocation.ContainerRegistry + normalized := image + + // Remove the 'standard' kubernetes image prefixes, just for sanity + normalized = strings.TrimPrefix(normalized, "registry.k8s.io/") + + // When assembling the cluster spec, kops may call the option more then once until the config converges + // This means that this function may me called more than once on the same image + // It this is pass is the second one, the image will already have been normalized with the containerRegistry settings + // If this is the case, passing though the process again will re-prepend the container registry again + // and again, causing the spec to never converge and the config build to fail. + if !strings.HasPrefix(normalized, registryMirror+"/") { + // We can't nest arbitrarily + // Some risk of collisions, but also -- and __ in the names appear to be blocked by docker hub + normalized = strings.Replace(normalized, "/", "-", -1) + normalized = registryMirror + "/" + normalized + } + image = normalized + } + // Run the new image + return image +} diff --git a/pkg/assets/builder_test.go b/pkg/assets/builder_test.go index d791b95770702..585f78731093d 100644 --- a/pkg/assets/builder_test.go +++ b/pkg/assets/builder_test.go @@ -42,11 +42,7 @@ func TestValidate_RemapImage_ContainerProxy_AppliesToDockerHub(t *testing.T) { builder.AssetsLocation.ContainerProxy = &proxyURL - remapped, err := builder.RemapImage(image) - if err != nil { - t.Error("Error remapping image", err) - } - + remapped := builder.RemapImage(image) if remapped != expected { t.Errorf("Error remapping image (Expecting: %s, got %s)", expected, remapped) } @@ -61,11 +57,7 @@ func TestValidate_RemapImage_ContainerProxy_AppliesToSimplifiedDockerHub(t *test builder.AssetsLocation.ContainerProxy = &proxyURL - remapped, err := builder.RemapImage(image) - if err != nil { - t.Error("Error remapping image", err) - } - + remapped := builder.RemapImage(image) if remapped != expected { t.Errorf("Error remapping image (Expecting: %s, got %s)", expected, remapped) } @@ -80,11 +72,7 @@ func TestValidate_RemapImage_ContainerProxy_AppliesToSimplifiedKubernetesURL(t * builder.AssetsLocation.ContainerProxy = &proxyURL - remapped, err := builder.RemapImage(image) - if err != nil { - t.Error("Error remapping image", err) - } - + remapped := builder.RemapImage(image) if remapped != expected { t.Errorf("Error remapping image (Expecting: %s, got %s)", expected, remapped) } @@ -99,11 +87,7 @@ func TestValidate_RemapImage_ContainerProxy_AppliesToLegacyKubernetesURL(t *test builder.AssetsLocation.ContainerProxy = &proxyURL - remapped, err := builder.RemapImage(image) - if err != nil { - t.Error("Error remapping image", err) - } - + remapped := builder.RemapImage(image) if remapped != expected { t.Errorf("Error remapping image (Expecting: %s, got %s)", expected, remapped) } @@ -118,11 +102,7 @@ func TestValidate_RemapImage_ContainerProxy_AppliesToImagesWithTags(t *testing.T builder.AssetsLocation.ContainerProxy = &proxyURL - remapped, err := builder.RemapImage(image) - if err != nil { - t.Error("Error remapping image", err) - } - + remapped := builder.RemapImage(image) if remapped != expected { t.Errorf("Error remapping image (Expecting: %s, got %s)", expected, remapped) } @@ -140,11 +120,7 @@ func TestValidate_RemapImage_ContainerRegistry_MappingMultipleTimesConverges(t * remapped := image iterations := make([]map[int]int, 2) for i := range iterations { - remapped, err := builder.RemapImage(remapped) - if err != nil { - t.Errorf("Error remapping image (iteration %d): %s", i, err) - } - + remapped := builder.RemapImage(remapped) if remapped != expected { t.Errorf("Error remapping image (Expecting: %s, got %s, iteration: %d)", expected, remapped, i) } diff --git a/pkg/kubemanifest/images.go b/pkg/kubemanifest/images.go index 419ead1bfd516..4fc4e2f842828 100644 --- a/pkg/kubemanifest/images.go +++ b/pkg/kubemanifest/images.go @@ -17,13 +17,12 @@ limitations under the License. package kubemanifest import ( - "fmt" "strings" "k8s.io/klog/v2" ) -type ImageRemapFunction func(image string) (string, error) +type ImageRemapFunction func(image string) string func (m *Object) RemapImages(mapper ImageRemapFunction) error { visitor := &imageRemapVisitor{ @@ -57,10 +56,7 @@ func (m *imageRemapVisitor) VisitString(path []string, v string, mutator func(st image := v klog.V(4).Infof("Consider image for re-mapping: %q", image) - remapped, err := m.mapper(v) - if err != nil { - return fmt.Errorf("error remapping image %q: %v", image, err) - } + remapped := m.mapper(v) if remapped != image { mutator(remapped) } diff --git a/pkg/model/components/context.go b/pkg/model/components/context.go index 7c08704dd4ac7..4401ac25812ac 100644 --- a/pkg/model/components/context.go +++ b/pkg/model/components/context.go @@ -150,11 +150,7 @@ func Image(component string, clusterSpec *kops.ClusterSpec, assetsBuilder *asset if !kopsmodel.IsBaseURL(clusterSpec.KubernetesVersion) { image := "registry.k8s.io/" + imageName + ":" + "v" + kubernetesVersion.String() - image, err := assetsBuilder.RemapImage(image) - if err != nil { - return "", fmt.Errorf("unable to remap container %q: %v", image, err) - } - return image, nil + return assetsBuilder.RemapImage(image), nil } // The simple name is valid when pulling. But if we diff --git a/pkg/model/components/etcdmanager/model.go b/pkg/model/components/etcdmanager/model.go index 2c9eaf97069a4..10b178282c437 100644 --- a/pkg/model/components/etcdmanager/model.go +++ b/pkg/model/components/etcdmanager/model.go @@ -314,11 +314,7 @@ func (b *EtcdManagerBuilder) buildPod(etcdCluster kops.EtcdClusterSpec, instance // Remap image via AssetBuilder for i := range pod.Spec.InitContainers { initContainer := &pod.Spec.InitContainers[i] - remapped, err := b.AssetBuilder.RemapImage(initContainer.Image) - if err != nil { - return nil, fmt.Errorf("unable to remap init container image %q: %w", container.Image, err) - } - initContainer.Image = remapped + initContainer.Image = b.AssetBuilder.RemapImage(initContainer.Image) } } @@ -334,11 +330,7 @@ func (b *EtcdManagerBuilder) buildPod(etcdCluster kops.EtcdClusterSpec, instance } // Remap image via AssetBuilder - remapped, err := b.AssetBuilder.RemapImage(container.Image) - if err != nil { - return nil, fmt.Errorf("unable to remap container image %q: %w", container.Image, err) - } - container.Image = remapped + container.Image = b.AssetBuilder.RemapImage(container.Image) } var clientHost string diff --git a/pkg/model/components/kubeapiserver/model.go b/pkg/model/components/kubeapiserver/model.go index 71c661f86a3a7..c6dc5c98f6d5d 100644 --- a/pkg/model/components/kubeapiserver/model.go +++ b/pkg/model/components/kubeapiserver/model.go @@ -138,13 +138,7 @@ func (b *KubeApiserverBuilder) buildHealthcheckSidecar() (*corev1.Pod, error) { } // Remap image via AssetBuilder - { - remapped, err := b.AssetBuilder.RemapImage(container.Image) - if err != nil { - return nil, fmt.Errorf("unable to remap container image %q: %v", container.Image, err) - } - container.Image = remapped - } + container.Image = b.AssetBuilder.RemapImage(container.Image) return pod, nil } diff --git a/pkg/model/components/kubelet.go b/pkg/model/components/kubelet.go index ce0c0823ad70c..9ac1989773cb4 100644 --- a/pkg/model/components/kubelet.go +++ b/pkg/model/components/kubelet.go @@ -167,11 +167,7 @@ func (b *KubeletOptionsBuilder) configureKubelet(cluster *kops.Cluster, kubelet // Prevent image GC from pruning the pause image // https://github.com/kubernetes/enhancements/tree/master/keps/sig-node/2040-kubelet-cri#pinned-images image := "registry.k8s.io/pause:3.9" - var err error - if image, err = b.AssetBuilder.RemapImage(image); err != nil { - return err - } - kubelet.PodInfraContainerImage = image + kubelet.PodInfraContainerImage = b.AssetBuilder.RemapImage(image) if kubelet.FeatureGates == nil { kubelet.FeatureGates = make(map[string]string) diff --git a/pkg/nodemodel/nodeupconfigbuilder.go b/pkg/nodemodel/nodeupconfigbuilder.go index 5982a3615c490..13275c2206407 100644 --- a/pkg/nodemodel/nodeupconfigbuilder.go +++ b/pkg/nodemodel/nodeupconfigbuilder.go @@ -509,7 +509,8 @@ func (n *nodeUpConfigBuilder) buildWarmPoolImages(ig *kops.InstanceGroup) []stri if assetBuilder != nil { for _, image := range assetBuilder.ImageAssets { for _, prefix := range desiredImagePrefixes { - if strings.HasPrefix(image.DownloadLocation, prefix) { + remappedPrefix := assets.NormalizeImage(assetBuilder, prefix) + if strings.HasPrefix(image.DownloadLocation, remappedPrefix) { images[image.DownloadLocation] = true } }