From a8249260c8b48ff201ef70c010886bc0f373bbfc Mon Sep 17 00:00:00 2001 From: Anatolii Bazko Date: Fri, 29 Mar 2024 11:49:58 +0100 Subject: [PATCH] feat: Automation of the expected images to be used by ImagePuller (#1822) * feat: Retrieve default images from plugin & devfile registries Signed-off-by: Anatolii Bazko --- .../che-operator.clusterserviceversion.yaml | 4 +- pkg/deploy/image-puller/imagepuller.go | 192 +++++++++++++----- pkg/deploy/image-puller/imagepuller_test.go | 82 +------- 3 files changed, 144 insertions(+), 134 deletions(-) diff --git a/bundle/next/eclipse-che/manifests/che-operator.clusterserviceversion.yaml b/bundle/next/eclipse-che/manifests/che-operator.clusterserviceversion.yaml index 00887a170..c40872474 100644 --- a/bundle/next/eclipse-che/manifests/che-operator.clusterserviceversion.yaml +++ b/bundle/next/eclipse-che/manifests/che-operator.clusterserviceversion.yaml @@ -100,7 +100,7 @@ metadata: operators.operatorframework.io/project_layout: go.kubebuilder.io/v3 repository: https://github.com/eclipse-che/che-operator support: Eclipse Foundation - name: eclipse-che.v7.84.0-861.next + name: eclipse-che.v7.84.0-862.next namespace: placeholder spec: apiservicedefinitions: {} @@ -1032,7 +1032,7 @@ spec: minKubeVersion: 1.19.0 provider: name: Eclipse Foundation - version: 7.84.0-861.next + version: 7.84.0-862.next webhookdefinitions: - admissionReviewVersions: - v1 diff --git a/pkg/deploy/image-puller/imagepuller.go b/pkg/deploy/image-puller/imagepuller.go index 11fca929a..9b5548432 100644 --- a/pkg/deploy/image-puller/imagepuller.go +++ b/pkg/deploy/image-puller/imagepuller.go @@ -13,38 +13,32 @@ package imagepuller import ( - goerror "errors" "fmt" + "io" + "net/http" "sort" "strings" + "time" - "github.com/google/go-cmp/cmp/cmpopts" - - "github.com/google/go-cmp/cmp" - ctrl "sigs.k8s.io/controller-runtime" + "k8s.io/apimachinery/pkg/util/validation" - chev1alpha1 "github.com/che-incubator/kubernetes-image-puller-operator/api/v1alpha1" "github.com/eclipse-che/che-operator/pkg/common/chetypes" "github.com/eclipse-che/che-operator/pkg/common/constants" "github.com/eclipse-che/che-operator/pkg/common/utils" - "github.com/eclipse-che/che-operator/pkg/deploy" + "github.com/google/go-cmp/cmp/cmpopts" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" - "k8s.io/apimachinery/pkg/util/validation" "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/google/go-cmp/cmp" + ctrl "sigs.k8s.io/controller-runtime" + + chev1alpha1 "github.com/che-incubator/kubernetes-image-puller-operator/api/v1alpha1" + "github.com/eclipse-che/che-operator/pkg/deploy" ) var ( - log = ctrl.Log.WithName("image-puller") - defaultImagePatterns = [...]string{ - "^RELATED_IMAGE_.*_theia.*", - "^RELATED_IMAGE_.*_code.*", - "^RELATED_IMAGE_.*_idea.*", - "^RELATED_IMAGE_.*_machine(_)?exec(_.*)?_plugin_registry_image.*", - "^RELATED_IMAGE_.*_kubernetes(_.*)?_plugin_registry_image.*", - "^RELATED_IMAGE_.*_openshift(_.*)?_plugin_registry_image.*", - "^RELATED_IMAGE_universal(_)?developer(_)?image(_.*)?_devfile_registry_image.*", - } + log = ctrl.Log.WithName("image-puller") kubernetesImagePullerDiffOpts = cmp.Options{ cmpopts.IgnoreFields(chev1alpha1.KubernetesImagePuller{}, "TypeMeta", "ObjectMeta", "Status"), } @@ -59,8 +53,6 @@ const ( defaultImagePullerImage = "quay.io/eclipse/kubernetes-image-puller:next" ) -type Images2Pull = map[string]string - type ImagePuller struct { deploy.Reconcilable } @@ -77,11 +69,11 @@ func (ip *ImagePuller) Reconcile(ctx *chetypes.DeployContext) (reconcile.Result, } if done, err := ip.syncKubernetesImagePuller(ctx); !done { - return reconcile.Result{}, false, err + return reconcile.Result{Requeue: true}, false, err } } else { if done, err := ip.uninstallImagePuller(ctx); !done { - return reconcile.Result{}, false, err + return reconcile.Result{Requeue: true}, false, err } } return reconcile.Result{}, true, nil @@ -139,7 +131,9 @@ func (ip *ImagePuller) syncKubernetesImagePuller(ctx *chetypes.DeployContext) (b imagePuller.Spec.ConfigMapName = utils.GetValue(imagePuller.Spec.ConfigMapName, defaultConfigMapName) imagePuller.Spec.DeploymentName = utils.GetValue(imagePuller.Spec.DeploymentName, defaultDeploymentName) imagePuller.Spec.ImagePullerImage = utils.GetValue(imagePuller.Spec.ImagePullerImage, defaultImagePullerImage) - imagePuller.Spec.Images = utils.GetValue(imagePuller.Spec.Images, getDefaultImages()) + if strings.TrimSpace(imagePuller.Spec.Images) == "" { + imagePuller.Spec.Images = getDefaultImages(ctx) + } return deploy.SyncWithClient(ctx.ClusterAPI.NonCachingClient, ctx, imagePuller, kubernetesImagePullerDiffOpts) } @@ -148,22 +142,132 @@ func getImagePullerCustomResourceName(ctx *chetypes.DeployContext) string { return ctx.CheCluster.Name + "-image-puller" } -// imagesToString returns a string representation of the provided image slice, -// suitable for the imagePuller.spec.images field -func imagesToString(images Images2Pull) string { - imageNames := make([]string, 0, len(images)) - for k := range images { - imageNames = append(imageNames, k) +func getDefaultImages(ctx *chetypes.DeployContext) string { + urls := collectRegistriesUrls(ctx) + allImages := fetchImagesFromRegistries(urls, ctx) + + // having them sorted, prevents from constant changing CR spec + sortedImages := sortImages(allImages) + return convertToSpecField(sortedImages) +} + +func collectRegistriesUrls(ctx *chetypes.DeployContext) []string { + urls := make([]string, 0) + + if ctx.CheCluster.Status.PluginRegistryURL != "" { + urls = append( + urls, + fmt.Sprintf( + "http://%s.%s.svc:8080/v3/%s", + constants.PluginRegistryName, + ctx.CheCluster.Namespace, + "external_images.txt", + ), + ) + } + + if ctx.CheCluster.Status.DevfileRegistryURL != "" { + urls = append( + urls, + fmt.Sprintf( + "http://%s.%s.svc:8080/%s", + constants.DevfileRegistryName, + ctx.CheCluster.Namespace, + "devfiles/external_images.txt", + ), + ) } - sort.Strings(imageNames) - imagesAsString := "" - for _, imageName := range imageNames { - if name, err := convertToRFC1123(imageName); err == nil { - imagesAsString += name + "=" + images[imageName] + ";" + return urls +} + +func fetchImagesFromRegistries(urls []string, ctx *chetypes.DeployContext) map[string]bool { + // return as map to make the list unique + allImages := make(map[string]bool) + + for _, url := range urls { + images, err := fetchImagesFromUrl(url, ctx) + if err != nil { + log.Error(err, fmt.Sprintf("Failed to fetch images from %s", url)) + } else { + for image := range images { + allImages[image] = true + } } } - return imagesAsString + + return allImages +} + +func sortImages(images map[string]bool) []string { + sortedImages := make([]string, len(images)) + + i := 0 + for image := range images { + sortedImages[i] = image + i++ + } + + sort.Strings(sortedImages) + return sortedImages +} + +func convertToSpecField(images []string) string { + specField := "" + for index, image := range images { + imageName, _ := utils.GetImageNameAndTag(image) + imageNameEntries := strings.Split(imageName, "/") + name, err := convertToRFC1123(imageNameEntries[len(imageNameEntries)-1]) + if err != nil { + name = "image" + } + + // Adding index make the name unique + specField += fmt.Sprintf("%s-%d=%s;", name, index, image) + } + + return specField +} + +func fetchImagesFromUrl(url string, ctx *chetypes.DeployContext) (map[string]bool, error) { + transport := &http.Transport{} + if ctx.Proxy.HttpProxy != "" { + deploy.ConfigureProxy(ctx, transport) + } + + client := &http.Client{ + Transport: transport, + Timeout: time.Second * 3, + } + + req, err := http.NewRequest("GET", url, nil) + if err != nil { + return map[string]bool{}, err + } + + resp, err := client.Do(req) + if err != nil { + return map[string]bool{}, err + } + + data, err := io.ReadAll(resp.Body) + if err != nil { + return map[string]bool{}, err + } + + images := make(map[string]bool) + for _, image := range strings.Split(string(data), "\n") { + image = strings.TrimSpace(image) + if image != "" { + images[image] = true + } + } + + if err = resp.Body.Close(); err != nil { + log.Error(err, "Failed to close a body response") + } + + return images, nil } // convertToRFC1123 converts input string to RFC 1123 format ([a-z0-9]([-a-z0-9]*[a-z0-9])?) max 63 characters, if possible @@ -183,7 +287,7 @@ func convertToRFC1123(str string) (string, error) { result = strings.ReplaceAll(result, "_", "-") if errs := validation.IsDNS1123Label(result); len(errs) > 0 { - return "", goerror.New("Cannot convert the following string to RFC 1123 format: " + str) + return "", fmt.Errorf("cannot convert the following string to RFC 1123 format: %s", str) } return result, nil } @@ -192,19 +296,3 @@ func isRFC1123Char(ch byte) bool { errs := validation.IsDNS1123Label(string(ch)) return len(errs) == 0 } - -// GetDefaultImages returns the current default images from the environment variables -func getDefaultImages() string { - images := map[string]string{} - for _, pattern := range defaultImagePatterns { - matches := utils.GetGetArchitectureDependentEnvsByRegExp(pattern) - sort.SliceStable(matches, func(i, j int) bool { - return strings.Compare(matches[i].Name, matches[j].Name) < 0 - }) - - for _, match := range matches { - images[match.Name[len("RELATED_IMAGE_"):]] = match.Value - } - } - return imagesToString(images) -} diff --git a/pkg/deploy/image-puller/imagepuller_test.go b/pkg/deploy/image-puller/imagepuller_test.go index 56c5abe62..a7a6e6033 100644 --- a/pkg/deploy/image-puller/imagepuller_test.go +++ b/pkg/deploy/image-puller/imagepuller_test.go @@ -14,7 +14,6 @@ package imagepuller import ( "context" - "os" chev1alpha1 "github.com/che-incubator/kubernetes-image-puller-operator/api/v1alpha1" "github.com/eclipse-che/che-operator/pkg/deploy" @@ -23,20 +22,14 @@ import ( "github.com/stretchr/testify/assert" "k8s.io/apimachinery/pkg/api/errors" + chev2 "github.com/eclipse-che/che-operator/api/v2" "github.com/eclipse-che/che-operator/pkg/common/constants" "github.com/eclipse-che/che-operator/pkg/common/test" - "github.com/eclipse-che/che-operator/pkg/common/utils" - - chev2 "github.com/eclipse-che/che-operator/api/v2" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" fakeDiscovery "k8s.io/client-go/discovery/fake" - "sigs.k8s.io/controller-runtime/pkg/client" - logf "sigs.k8s.io/controller-runtime/pkg/log" - "sigs.k8s.io/controller-runtime/pkg/log/zap" - "testing" ) @@ -48,15 +41,6 @@ func TestImagePullerConfiguration(t *testing.T) { expectedImagePuller *chev1alpha1.KubernetesImagePuller } - // unset RELATED_IMAGE environment variables, set them back after tests complete - matches := utils.GetGetArchitectureDependentEnvsByRegExp("^RELATED_IMAGE_.*") - for _, match := range matches { - if originalValue, exists := os.LookupEnv(match.Name); exists { - os.Unsetenv(match.Name) - defer os.Setenv(match.Name, originalValue) - } - } - testCases := []testCase{ { name: "case #1: KubernetesImagePuller with defaults", @@ -67,7 +51,6 @@ func TestImagePullerConfiguration(t *testing.T) { DeploymentName: defaultDeploymentName, ConfigMapName: defaultConfigMapName, ImagePullerImage: defaultImagePullerImage, - Images: getDefaultImages(), }), }, { @@ -97,14 +80,12 @@ func TestImagePullerConfiguration(t *testing.T) { DeploymentName: defaultDeploymentName, ConfigMapName: defaultConfigMapName, ImagePullerImage: defaultImagePullerImage, - Images: getDefaultImages(), }), }, expectedImagePuller: InitImagePuller(chev1alpha1.KubernetesImagePullerSpec{ DeploymentName: defaultDeploymentName, ConfigMapName: defaultConfigMapName, ImagePullerImage: defaultImagePullerImage, - Images: getDefaultImages(), }), }, { @@ -122,7 +103,6 @@ func TestImagePullerConfiguration(t *testing.T) { DeploymentName: defaultDeploymentName, ConfigMapName: defaultConfigMapName, ImagePullerImage: defaultImagePullerImage, - Images: getDefaultImages(), }), }, expectedImagePuller: InitImagePuller(chev1alpha1.KubernetesImagePullerSpec{ @@ -142,7 +122,6 @@ func TestImagePullerConfiguration(t *testing.T) { DeploymentName: defaultDeploymentName, ConfigMapName: defaultConfigMapName, ImagePullerImage: defaultImagePullerImage, - Images: getDefaultImages(), }), }, }, @@ -150,9 +129,7 @@ func TestImagePullerConfiguration(t *testing.T) { for _, testCase := range testCases { t.Run(testCase.name, func(t *testing.T) { - logf.SetLogger(zap.New(zap.WriteTo(os.Stdout), zap.UseDevMode(true))) - - ctx := test.GetDeployContext(testCase.cheCluster, []runtime.Object{}) + ctx := test.GetDeployContext(testCase.cheCluster, testCase.initObjects) ctx.ClusterAPI.DiscoveryClient.(*fakeDiscovery.FakeDiscovery).Fake.Resources = []*metav1.APIResourceList{ { GroupVersion: "che.eclipse.org/v1alpha1", @@ -164,11 +141,6 @@ func TestImagePullerConfiguration(t *testing.T) { }, } - for _, obj := range testCase.initObjects { - err := ctx.ClusterAPI.Client.Create(context.TODO(), obj.(client.Object)) - assert.NoError(t, err) - } - ip := NewImagePuller() _, _, err := ip.Reconcile(ctx) assert.NoError(t, err) @@ -192,56 +164,6 @@ func TestImagePullerConfiguration(t *testing.T) { } } -func TestDefaultImages(t *testing.T) { - type testcase struct { - name string - env map[string]string - expected string - expectedImagesAsString string - } - - // unset RELATED_IMAGE environment variables, set them back after tests complete - matches := utils.GetGetArchitectureDependentEnvsByRegExp("^RELATED_IMAGE_.*") - for _, match := range matches { - if originalValue, exists := os.LookupEnv(match.Name); exists { - os.Unsetenv(match.Name) - defer os.Setenv(match.Name, originalValue) - } - } - - cases := []testcase{ - { - name: "case #1", - env: map[string]string{ - "RELATED_IMAGE_che_theia_plugin_registry_image_IBZWQYJ": "quay.io/eclipse/che-theia", - "RELATED_IMAGE_che_theia_endpoint_runtime_binary_plugin_registry_image_IBZWQYJ": "quay.io/eclipse/che-theia-endpoint-runtime-binary", - }, - expected: "che-theia-endpoint-runtime-binary-plugin-registry-image-ibzwqyj=quay.io/eclipse/che-theia-endpoint-runtime-binary;che-theia-plugin-registry-image-ibzwqyj=quay.io/eclipse/che-theia;", - }, - { - name: "case #2", - env: map[string]string{ - "RELATED_IMAGE_che_machine_exec_plugin_registry_image_IBZWQYJ": "quay.io/eclipse/che-machine-exec", - "RELATED_IMAGE_codeready_workspaces_machineexec_plugin_registry_image_GIXDCMQK": "registry.redhat.io/codeready-workspaces/machineexec-rhel8", - }, - expected: "che-machine-exec-plugin-registry-image-ibzwqyj=quay.io/eclipse/che-machine-exec;codeready-workspaces-machineexec-plugin-registry-image-gixdcmqk=registry.redhat.io/codeready-workspaces/machineexec-rhel8;", - }, - } - - for _, c := range cases { - t.Run(c.name, func(t *testing.T) { - for k, v := range c.env { - os.Setenv(k, v) - defer os.Unsetenv(k) - } - actual := getDefaultImages() - if d := cmp.Diff(c.expected, actual); d != "" { - t.Errorf("Error, collected images differ (-want, +got): %v", d) - } - }) - } -} - func InitCheCluster(imagePuller chev2.ImagePuller) *chev2.CheCluster { return &chev2.CheCluster{ ObjectMeta: metav1.ObjectMeta{