From 618ad1f1a9bd9d545391e9ed24105d89cd867884 Mon Sep 17 00:00:00 2001 From: Anatolii Bazko Date: Tue, 4 Jun 2024 10:42:53 +0200 Subject: [PATCH] feat: Disable plugin registry if openVSX registry is configured (#1843) * feat: Disable plugin registry if openVSX registry is configured Signed-off-by: Anatolii Bazko --- ...lease-next-catalog-and-operator-image.yaml | 2 +- build/scripts/minikube-tests/common.sh | 104 +++++++++- .../test-upgrade-from-stable-to-next.sh | 1 - .../test-upgrade-from-stable-to-stable.sh | 1 - build/scripts/release/editors-definitions.sh | 8 +- build/scripts/release/make-release.sh | 4 +- .../che-operator.clusterserviceversion.yaml | 8 +- config/samples/org_v1_checluster.yaml | 1 + config/samples/org_v2_checluster.yaml | 2 + controllers/che/checluster_controller.go | 3 + .../kubernetes/org_v2_checluster.yaml | 2 + .../openshift/org_v2_checluster.yaml | 2 + .../next/templates/org_v2_checluster.yaml | 2 + pkg/deploy/container-build/container_build.go | 4 +- .../editors_definitions.go} | 44 +++- .../editors_definitions_test.go} | 3 +- pkg/deploy/editors-definitions/init_test.go | 28 +++ .../test-editors-definitions/devfile.yaml | 1 + pkg/deploy/image-puller/imagepuller.go | 30 ++- pkg/deploy/pluginregistry/init_test.go | 2 - pkg/deploy/pluginregistry/pluginregistry.go | 18 +- .../pluginregistry/pluginregistry_test.go | 2 - pkg/deploy/postgres/postgres.go | 32 +-- pkg/deploy/service_account.go | 13 +- pkg/deploy/service_account_test.go | 29 +++ pkg/deploy/sync.go | 191 ++++++++---------- pkg/deploy/sync_test.go | 66 ++---- pkg/deploy/tls/certificates.go | 3 +- 28 files changed, 349 insertions(+), 257 deletions(-) rename pkg/deploy/{pluginregistry/pluginregistry_editors.go => editors-definitions/editors_definitions.go} (76%) rename pkg/deploy/{pluginregistry/pluginregistry_editors_test.go => editors-definitions/editors_definitions_test.go} (96%) create mode 100644 pkg/deploy/editors-definitions/init_test.go rename pkg/deploy/{pluginregistry => editors-definitions}/test-editors-definitions/devfile.yaml (96%) create mode 100644 pkg/deploy/service_account_test.go diff --git a/.github/workflows/release-next-catalog-and-operator-image.yaml b/.github/workflows/release-next-catalog-and-operator-image.yaml index aba80409d..73c9c2d46 100644 --- a/.github/workflows/release-next-catalog-and-operator-image.yaml +++ b/.github/workflows/release-next-catalog-and-operator-image.yaml @@ -78,7 +78,7 @@ jobs: registry: quay.io - name: Build catalog source run: | - ./build/scripts/release/editors-definitions.sh add-env-vars + ./build/scripts/release/editors-definitions.sh update-manager-yaml make update-dev-resources ./build/scripts/release/addDigests.sh -s $(make csv-path CHANNEL=next) -t next ./build/scripts/olm/release-catalog.sh \ diff --git a/build/scripts/minikube-tests/common.sh b/build/scripts/minikube-tests/common.sh index ccfa51445..e38801451 100755 --- a/build/scripts/minikube-tests/common.sh +++ b/build/scripts/minikube-tests/common.sh @@ -202,6 +202,102 @@ metadata: kubernetes.io/metadata.name: ${USER_NAMESPACE} EOF +kubectl apply -f - < /checode/entrypoint-logs.txt + 2>&1 & + component: che-code-runtime-description + id: init-che-code-command + components: + - container: + command: + - /entrypoint-init-container.sh + cpuLimit: 500m + cpuRequest: 30m + env: + - name: CHE_DASHBOARD_URL + value: https://$(minikube ip).nip.io + - name: OPENVSX_REGISTRY_URL + value: https://open-vsx.org + image: quay.io/che-incubator/che-code:latest + memoryLimit: 256Mi + memoryRequest: 32Mi + sourceMapping: /projects + volumeMounts: + - name: checode + path: /checode + name: che-code-injector + - attributes: + app.kubernetes.io/component: che-code-runtime + app.kubernetes.io/part-of: che-code.eclipse.org + controller.devfile.io/container-contribution: true + container: + cpuLimit: 500m + cpuRequest: 30m + endpoints: + - attributes: + cookiesAuthEnabled: true + discoverable: false + type: main + urlRewriteSupported: true + exposure: public + name: che-code + protocol: https + secure: true + targetPort: 3100 + - attributes: + discoverable: false + urlRewriteSupported: false + exposure: public + name: code-redirect-1 + protocol: https + targetPort: 13131 + - attributes: + discoverable: false + urlRewriteSupported: false + exposure: public + name: code-redirect-2 + protocol: https + targetPort: 13132 + - attributes: + discoverable: false + urlRewriteSupported: false + exposure: public + name: code-redirect-3 + protocol: https + targetPort: 13133 + env: + - name: CHE_DASHBOARD_URL + value: https://$(minikube ip).nip.io + - name: OPENVSX_REGISTRY_URL + value: https://open-vsx.org + image: quay.io/devfile/universal-developer-image:ubi8-latest + memoryLimit: 1024Mi + memoryRequest: 256Mi + sourceMapping: /projects + volumeMounts: + - name: checode + path: /checode + name: che-code-runtime-description + - name: checode + volume: {} + events: + postStart: + - init-che-code-command + preStart: + - init-container-command +EOF + kubectl apply -f - </dev/null case $COMMAND in 'release') release;; - 'add-env-vars') addEnvVars;; + 'update-manager-yaml'|'add-env-vars') updateManagerYaml;; *) usage; exit 1;; esac popd >/dev/null diff --git a/build/scripts/release/make-release.sh b/build/scripts/release/make-release.sh index d5258e3f7..f96ae3a15 100755 --- a/build/scripts/release/make-release.sh +++ b/build/scripts/release/make-release.sh @@ -183,10 +183,12 @@ releaseEditorsDefinitions() { echo "[INFO] Releasing editor definitions" . "${OPERATOR_REPO}/build/scripts/release/editors-definitions.sh" release --version "${RELEASE}" - . "${OPERATOR_REPO}/build/scripts/release/editors-definitions.sh" add-env-vars + . "${OPERATOR_REPO}/build/scripts/release/editors-definitions.sh" update-manager-yaml + make bundle CHANNEL=stable git add editors-definitions git add "${OPERATOR_REPO}/config/manager/manager.yaml" + git add "${OPERATOR_REPO}/bundle/stable" git commit -m "ci: Release editors definitions to $RELEASE" --signoff } diff --git a/bundle/next/eclipse-che/manifests/che-operator.clusterserviceversion.yaml b/bundle/next/eclipse-che/manifests/che-operator.clusterserviceversion.yaml index f6833e687..7c56affa6 100644 --- a/bundle/next/eclipse-che/manifests/che-operator.clusterserviceversion.yaml +++ b/bundle/next/eclipse-che/manifests/che-operator.clusterserviceversion.yaml @@ -46,6 +46,7 @@ metadata: } ], "externalDevfileRegistry": true, + "externalPluginRegistry": true, "workspaceNamespaceDefault": "-che" }, "storage": { @@ -69,6 +70,9 @@ metadata: "url": "https://registry.devfile.io" } ] + }, + "pluginRegistry": { + "disableInternalRegistry": true } }, "containerRegistry": {}, @@ -100,7 +104,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.87.0-870.next + name: eclipse-che.v7.87.0-871.next namespace: placeholder spec: apiservicedefinitions: {} @@ -1032,7 +1036,7 @@ spec: minKubeVersion: 1.19.0 provider: name: Eclipse Foundation - version: 7.87.0-870.next + version: 7.87.0-871.next webhookdefinitions: - admissionReviewVersions: - v1 diff --git a/config/samples/org_v1_checluster.yaml b/config/samples/org_v1_checluster.yaml index 9dd8567a1..a5a2bed7d 100644 --- a/config/samples/org_v1_checluster.yaml +++ b/config/samples/org_v1_checluster.yaml @@ -18,6 +18,7 @@ metadata: spec: server: workspaceNamespaceDefault: '-che' + externalPluginRegistry: true externalDevfileRegistry: true externalDevfileRegistries: - url: 'https://registry.devfile.io' diff --git a/config/samples/org_v2_checluster.yaml b/config/samples/org_v2_checluster.yaml index bd29d2788..f5e97c4b6 100644 --- a/config/samples/org_v2_checluster.yaml +++ b/config/samples/org_v2_checluster.yaml @@ -17,6 +17,8 @@ metadata: namespace: eclipse-che spec: components: + pluginRegistry: + disableInternalRegistry: true devfileRegistry: disableInternalRegistry: true externalDevfileRegistries: diff --git a/controllers/che/checluster_controller.go b/controllers/che/checluster_controller.go index 9374a460c..b86f2ac5c 100644 --- a/controllers/che/checluster_controller.go +++ b/controllers/che/checluster_controller.go @@ -15,6 +15,8 @@ package che import ( "context" + editorsdefinitions "github.com/eclipse-che/che-operator/pkg/deploy/editors-definitions" + "github.com/eclipse-che/che-operator/pkg/common/test" containerbuild "github.com/eclipse-che/che-operator/pkg/deploy/container-build" @@ -110,6 +112,7 @@ func NewReconciler( } reconcileManager.RegisterReconciler(devfileregistry.NewDevfileRegistryReconciler()) reconcileManager.RegisterReconciler(pluginregistry.NewPluginRegistryReconciler()) + reconcileManager.RegisterReconciler(editorsdefinitions.NewEditorsDefinitionsReconciler()) reconcileManager.RegisterReconciler(dashboard.NewDashboardReconciler()) reconcileManager.RegisterReconciler(gateway.NewGatewayReconciler()) reconcileManager.RegisterReconciler(server.NewCheServerReconciler()) diff --git a/deploy/deployment/kubernetes/org_v2_checluster.yaml b/deploy/deployment/kubernetes/org_v2_checluster.yaml index bd29d2788..f5e97c4b6 100644 --- a/deploy/deployment/kubernetes/org_v2_checluster.yaml +++ b/deploy/deployment/kubernetes/org_v2_checluster.yaml @@ -17,6 +17,8 @@ metadata: namespace: eclipse-che spec: components: + pluginRegistry: + disableInternalRegistry: true devfileRegistry: disableInternalRegistry: true externalDevfileRegistries: diff --git a/deploy/deployment/openshift/org_v2_checluster.yaml b/deploy/deployment/openshift/org_v2_checluster.yaml index bd29d2788..f5e97c4b6 100644 --- a/deploy/deployment/openshift/org_v2_checluster.yaml +++ b/deploy/deployment/openshift/org_v2_checluster.yaml @@ -17,6 +17,8 @@ metadata: namespace: eclipse-che spec: components: + pluginRegistry: + disableInternalRegistry: true devfileRegistry: disableInternalRegistry: true externalDevfileRegistries: diff --git a/helmcharts/next/templates/org_v2_checluster.yaml b/helmcharts/next/templates/org_v2_checluster.yaml index e351474fe..d13138e1a 100644 --- a/helmcharts/next/templates/org_v2_checluster.yaml +++ b/helmcharts/next/templates/org_v2_checluster.yaml @@ -17,6 +17,8 @@ metadata: namespace: eclipse-che spec: components: + pluginRegistry: + disableInternalRegistry: true devfileRegistry: disableInternalRegistry: true externalDevfileRegistries: diff --git a/pkg/deploy/container-build/container_build.go b/pkg/deploy/container-build/container_build.go index 1c5628b1e..30889e315 100644 --- a/pkg/deploy/container-build/container_build.go +++ b/pkg/deploy/container-build/container_build.go @@ -122,8 +122,8 @@ func (cb *ContainerBuildReconciler) syncSCC(ctx *chetypes.DeployContext) (bool, }) } } else { - // Create a new SCC. If custom SCC exists then it won't be touched. - return deploy.Create(ctx, cb.getSccSpec(ctx)) + // Create a new SCC. + return deploy.CreateIgnoreIfExists(ctx, cb.getSccSpec(ctx)) } return true, nil diff --git a/pkg/deploy/pluginregistry/pluginregistry_editors.go b/pkg/deploy/editors-definitions/editors_definitions.go similarity index 76% rename from pkg/deploy/pluginregistry/pluginregistry_editors.go rename to pkg/deploy/editors-definitions/editors_definitions.go index 0c66453ee..a49039eb3 100644 --- a/pkg/deploy/pluginregistry/pluginregistry_editors.go +++ b/pkg/deploy/editors-definitions/editors_definitions.go @@ -10,7 +10,7 @@ // Red Hat, Inc. - initial API and implementation // -package pluginregistry +package editorsdefinitions import ( "fmt" @@ -18,21 +18,47 @@ import ( "path/filepath" "regexp" - "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" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/yaml" + + "github.com/eclipse-che/che-operator/pkg/common/chetypes" + "github.com/eclipse-che/che-operator/pkg/common/constants" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/eclipse-che/che-operator/pkg/deploy" ) var ( editorsDefinitionsDir = "/tmp/editors-definitions" editorsDefinitionsConfigMapName = "editors-definitions" + logger = ctrl.Log.WithName("editorsdefinitions") ) -func (p *PluginRegistryReconciler) syncEditors(ctx *chetypes.DeployContext) (bool, error) { +type EditorsDefinitionsReconciler struct { + deploy.Reconcilable +} + +func NewEditorsDefinitionsReconciler() *EditorsDefinitionsReconciler { + return &EditorsDefinitionsReconciler{} +} + +func (p *EditorsDefinitionsReconciler) Reconcile(ctx *chetypes.DeployContext) (reconcile.Result, bool, error) { + done, err := p.syncEditors(ctx) + if !done { + return reconcile.Result{}, false, err + } + + return reconcile.Result{}, true, nil +} + +func (p *EditorsDefinitionsReconciler) Finalize(ctx *chetypes.DeployContext) bool { + return true +} + +func (p *EditorsDefinitionsReconciler) syncEditors(ctx *chetypes.DeployContext) (bool, error) { editorDefinitions, err := readEditorDefinitions() if err != nil { return false, err @@ -66,11 +92,11 @@ func readEditorDefinitions() (map[string][]byte, error) { var devfile map[string]interface{} err = yaml.Unmarshal(editorContent, &devfile) if err != nil { - return editorDefinitions, err + logger.Error(err, "Failed to unmarshal editor definition", "file", fileName) + continue } - updateEditorDefinitionImageFromEnv(devfile) - + updateEditorDefinitionImages(devfile) editorContent, err = yaml.Marshal(devfile) if err != nil { return editorDefinitions, err @@ -83,7 +109,7 @@ func readEditorDefinitions() (map[string][]byte, error) { return editorDefinitions, nil } -func updateEditorDefinitionImageFromEnv(devfile map[string]interface{}) { +func updateEditorDefinitionImages(devfile map[string]interface{}) { notAllowedCharsReg, _ := regexp.Compile("[^a-zA-Z0-9]+") metadata := devfile["metadata"].(map[string]interface{}) diff --git a/pkg/deploy/pluginregistry/pluginregistry_editors_test.go b/pkg/deploy/editors-definitions/editors_definitions_test.go similarity index 96% rename from pkg/deploy/pluginregistry/pluginregistry_editors_test.go rename to pkg/deploy/editors-definitions/editors_definitions_test.go index a31f257cb..f946785ac 100644 --- a/pkg/deploy/pluginregistry/pluginregistry_editors_test.go +++ b/pkg/deploy/editors-definitions/editors_definitions_test.go @@ -10,7 +10,7 @@ // Red Hat, Inc. - initial API and implementation // -package pluginregistry +package editorsdefinitions import ( "os" @@ -61,6 +61,7 @@ func TestSyncEditorDefinitions(t *testing.T) { editorDefinitions, err := readEditorDefinitions() assert.NoError(t, err) assert.NotEmpty(t, editorDefinitions) + assert.Len(t, editorDefinitions, 1) done, err := syncEditorDefinitions(ctx, editorDefinitions) assert.NoError(t, err) diff --git a/pkg/deploy/editors-definitions/init_test.go b/pkg/deploy/editors-definitions/init_test.go new file mode 100644 index 000000000..94526d0f0 --- /dev/null +++ b/pkg/deploy/editors-definitions/init_test.go @@ -0,0 +1,28 @@ +// +// Copyright (c) 2019-2024 Red Hat, Inc. +// This program and the accompanying materials are made +// available under the terms of the Eclipse Public License 2.0 +// which is available at https://www.eclipse.org/legal/epl-2.0/ +// +// SPDX-License-Identifier: EPL-2.0 +// +// Contributors: +// Red Hat, Inc. - initial API and implementation +// + +package editorsdefinitions + +import ( + "github.com/devfile/devworkspace-operator/pkg/infrastructure" + defaults "github.com/eclipse-che/che-operator/pkg/common/operator-defaults" + "github.com/eclipse-che/che-operator/pkg/common/test" +) + +func init() { + test.EnableTestMode() + + infrastructure.InitializeForTesting(infrastructure.OpenShiftv4) + defaults.InitializeForTesting("../../../config/manager/manager.yaml") + + editorsDefinitionsDir = "./test-editors-definitions" +} diff --git a/pkg/deploy/pluginregistry/test-editors-definitions/devfile.yaml b/pkg/deploy/editors-definitions/test-editors-definitions/devfile.yaml similarity index 96% rename from pkg/deploy/pluginregistry/test-editors-definitions/devfile.yaml rename to pkg/deploy/editors-definitions/test-editors-definitions/devfile.yaml index c2a766892..795e1b2e6 100644 --- a/pkg/deploy/pluginregistry/test-editors-definitions/devfile.yaml +++ b/pkg/deploy/editors-definitions/test-editors-definitions/devfile.yaml @@ -15,6 +15,7 @@ metadata: name: che-code attributes: version: 1.2.3 + publisher: test components: - name: component-a container: diff --git a/pkg/deploy/image-puller/imagepuller.go b/pkg/deploy/image-puller/imagepuller.go index 9b5548432..62a10063e 100644 --- a/pkg/deploy/image-puller/imagepuller.go +++ b/pkg/deploy/image-puller/imagepuller.go @@ -143,8 +143,10 @@ func getImagePullerCustomResourceName(ctx *chetypes.DeployContext) string { } func getDefaultImages(ctx *chetypes.DeployContext) string { - urls := collectRegistriesUrls(ctx) - allImages := fetchImagesFromRegistries(urls, ctx) + allImages := make(map[string]bool) + + addImagesFromRegistries(ctx, allImages) + addImagesFromEditorsDefinitions(allImages) // having them sorted, prevents from constant changing CR spec sortedImages := sortImages(allImages) @@ -154,18 +156,6 @@ func getDefaultImages(ctx *chetypes.DeployContext) string { 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, @@ -181,9 +171,8 @@ func collectRegistriesUrls(ctx *chetypes.DeployContext) []string { 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) +func addImagesFromRegistries(ctx *chetypes.DeployContext, allImages map[string]bool) { + urls := collectRegistriesUrls(ctx) for _, url := range urls { images, err := fetchImagesFromUrl(url, ctx) @@ -195,8 +184,13 @@ func fetchImagesFromRegistries(urls []string, ctx *chetypes.DeployContext) map[s } } } +} - return allImages +func addImagesFromEditorsDefinitions(allImages map[string]bool) { + envs := utils.GetEnvsByRegExp("RELATED_IMAGE_editor_definition_.*") + for _, env := range envs { + allImages[env.Value] = true + } } func sortImages(images map[string]bool) []string { diff --git a/pkg/deploy/pluginregistry/init_test.go b/pkg/deploy/pluginregistry/init_test.go index d8d7e5f0b..1af13a41f 100644 --- a/pkg/deploy/pluginregistry/init_test.go +++ b/pkg/deploy/pluginregistry/init_test.go @@ -23,6 +23,4 @@ func init() { infrastructure.InitializeForTesting(infrastructure.OpenShiftv4) defaults.InitializeForTesting("../../../config/manager/manager.yaml") - - editorsDefinitionsDir = "./test-editors-definitions" } diff --git a/pkg/deploy/pluginregistry/pluginregistry.go b/pkg/deploy/pluginregistry/pluginregistry.go index 81893555c..c6d6af441 100644 --- a/pkg/deploy/pluginregistry/pluginregistry.go +++ b/pkg/deploy/pluginregistry/pluginregistry.go @@ -16,9 +16,6 @@ import ( "fmt" "strings" - appsv1 "k8s.io/api/apps/v1" - corev1 "k8s.io/api/core/v1" - "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/deploy/gateway" @@ -38,25 +35,16 @@ func NewPluginRegistryReconciler() *PluginRegistryReconciler { func (p *PluginRegistryReconciler) Reconcile(ctx *chetypes.DeployContext) (reconcile.Result, bool, error) { if ctx.CheCluster.Spec.Components.PluginRegistry.DisableInternalRegistry { - _, _ = deploy.DeleteNamespacedObject(ctx, constants.PluginRegistryName, &corev1.Service{}) - _, _ = deploy.DeleteNamespacedObject(ctx, constants.PluginRegistryName, &corev1.ConfigMap{}) - _, _ = deploy.DeleteNamespacedObject(ctx, editorsDefinitionsConfigMapName, &corev1.ConfigMap{}) - _, _ = deploy.DeleteNamespacedObject(ctx, gateway.GatewayConfigMapNamePrefix+constants.PluginRegistryName, &corev1.ConfigMap{}) - _, _ = deploy.DeleteNamespacedObject(ctx, constants.PluginRegistryName, &appsv1.Deployment{}) - if ctx.CheCluster.Status.PluginRegistryURL != "" { ctx.CheCluster.Status.PluginRegistryURL = "" err := deploy.UpdateCheCRStatus(ctx, "PluginRegistryURL", "") return reconcile.Result{}, err == nil, err } + + return reconcile.Result{}, true, nil } - done, err := p.syncEditors(ctx) - if !done { - return reconcile.Result{}, false, err - } - - done, err = p.syncService(ctx) + done, err := p.syncService(ctx) if !done { return reconcile.Result{}, false, err } diff --git a/pkg/deploy/pluginregistry/pluginregistry_test.go b/pkg/deploy/pluginregistry/pluginregistry_test.go index 266539bc9..71ead00f1 100644 --- a/pkg/deploy/pluginregistry/pluginregistry_test.go +++ b/pkg/deploy/pluginregistry/pluginregistry_test.go @@ -16,7 +16,6 @@ import ( "github.com/devfile/devworkspace-operator/pkg/infrastructure" "github.com/eclipse-che/che-operator/pkg/common/test" "github.com/stretchr/testify/assert" - appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" @@ -37,7 +36,6 @@ func TestPluginRegistryReconcile(t *testing.T) { assert.True(t, test.IsObjectExists(ctx.ClusterAPI.Client, types.NamespacedName{Name: "plugin-registry", Namespace: "eclipse-che"}, &corev1.Service{})) assert.True(t, test.IsObjectExists(ctx.ClusterAPI.Client, types.NamespacedName{Name: "plugin-registry", Namespace: "eclipse-che"}, &corev1.ConfigMap{})) - assert.True(t, test.IsObjectExists(ctx.ClusterAPI.Client, types.NamespacedName{Name: "editors-definitions", Namespace: "eclipse-che"}, &corev1.ConfigMap{})) assert.True(t, test.IsObjectExists(ctx.ClusterAPI.Client, types.NamespacedName{Name: "plugin-registry", Namespace: "eclipse-che"}, &appsv1.Deployment{})) assert.NotEmpty(t, ctx.CheCluster.Status.PluginRegistryURL) } diff --git a/pkg/deploy/postgres/postgres.go b/pkg/deploy/postgres/postgres.go index a4228df8d..31f52d51b 100644 --- a/pkg/deploy/postgres/postgres.go +++ b/pkg/deploy/postgres/postgres.go @@ -37,13 +37,11 @@ func NewPostgresReconciler() *PostgresReconciler { func (p *PostgresReconciler) Reconcile(ctx *chetypes.DeployContext) (reconcile.Result, bool, error) { // PostgreSQL component is not used anymore - _, _ = p.syncDeployment(ctx) - _, _ = p.syncPVC(ctx) - _, _ = p.syncCredentials(ctx) - _, _ = p.syncService(ctx) - - // Backup server component is not used anymore - _, _ = p.syncBackupDeployment(ctx) + _, _ = deploy.DeleteNamespacedObject(ctx, postgresComponentName, &appsv1.Deployment{}) + _, _ = deploy.DeleteNamespacedObject(ctx, backupServerComponentName, &appsv1.Deployment{}) + _, _ = deploy.DeleteNamespacedObject(ctx, defaultPostgresVolumeClaimName, &corev1.PersistentVolumeClaim{}) + _, _ = deploy.DeleteNamespacedObject(ctx, defaultPostgresCredentialsSecret, &corev1.Secret{}) + _, _ = deploy.DeleteNamespacedObject(ctx, postgresComponentName, &corev1.Service{}) return reconcile.Result{}, true, nil } @@ -51,23 +49,3 @@ func (p *PostgresReconciler) Reconcile(ctx *chetypes.DeployContext) (reconcile.R func (p *PostgresReconciler) Finalize(ctx *chetypes.DeployContext) bool { return true } - -func (p *PostgresReconciler) syncService(ctx *chetypes.DeployContext) (bool, error) { - return deploy.DeleteNamespacedObject(ctx, postgresComponentName, &corev1.Service{}) -} - -func (p *PostgresReconciler) syncPVC(ctx *chetypes.DeployContext) (bool, error) { - return deploy.DeleteNamespacedObject(ctx, defaultPostgresVolumeClaimName, &corev1.PersistentVolumeClaim{}) -} - -func (p *PostgresReconciler) syncDeployment(ctx *chetypes.DeployContext) (bool, error) { - return deploy.DeleteNamespacedObject(ctx, postgresComponentName, &appsv1.Deployment{}) -} - -func (p *PostgresReconciler) syncCredentials(ctx *chetypes.DeployContext) (bool, error) { - return deploy.DeleteNamespacedObject(ctx, defaultPostgresCredentialsSecret, &corev1.Secret{}) -} - -func (p *PostgresReconciler) syncBackupDeployment(ctx *chetypes.DeployContext) (bool, error) { - return deploy.DeleteNamespacedObject(ctx, backupServerComponentName, &appsv1.Deployment{}) -} diff --git a/pkg/deploy/service_account.go b/pkg/deploy/service_account.go index f7feca95d..d95db5263 100644 --- a/pkg/deploy/service_account.go +++ b/pkg/deploy/service_account.go @@ -20,14 +20,7 @@ import ( ) func SyncServiceAccountToCluster(deployContext *chetypes.DeployContext, name string) (bool, error) { - saSpec := getServiceAccountSpec(deployContext, name) - _, err := CreateIfNotExists(deployContext, saSpec) - return err == nil, err -} - -func getServiceAccountSpec(deployContext *chetypes.DeployContext, name string) *corev1.ServiceAccount { - labels := GetLabels(defaults.GetCheFlavor()) - serviceAccount := &corev1.ServiceAccount{ + sa := &corev1.ServiceAccount{ TypeMeta: metav1.TypeMeta{ Kind: "ServiceAccount", APIVersion: "v1", @@ -35,9 +28,9 @@ func getServiceAccountSpec(deployContext *chetypes.DeployContext, name string) * ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: deployContext.CheCluster.Namespace, - Labels: labels, + Labels: GetLabels(defaults.GetCheFlavor()), }, } - return serviceAccount + return CreateIgnoreIfExists(deployContext, sa) } diff --git a/pkg/deploy/service_account_test.go b/pkg/deploy/service_account_test.go new file mode 100644 index 000000000..888b9d4e3 --- /dev/null +++ b/pkg/deploy/service_account_test.go @@ -0,0 +1,29 @@ +// +// Copyright (c) 2019-2023 Red Hat, Inc. +// This program and the accompanying materials are made +// available under the terms of the Eclipse Public License 2.0 +// which is available at https://www.eclipse.org/legal/epl-2.0/ +// +// SPDX-License-Identifier: EPL-2.0 +// +// Contributors: +// Red Hat, Inc. - initial API and implementation +// + +package deploy + +import ( + "github.com/eclipse-che/che-operator/pkg/common/test" + "github.com/stretchr/testify/assert" + "k8s.io/apimachinery/pkg/runtime" + + "testing" +) + +func TestSyncServiceAccountToCluster(t *testing.T) { + ctx := test.GetDeployContext(nil, []runtime.Object{}) + + done, err := SyncServiceAccountToCluster(ctx, "test") + assert.NoError(t, err) + assert.True(t, done) +} diff --git a/pkg/deploy/sync.go b/pkg/deploy/sync.go index 2be298259..3a8ef51f8 100644 --- a/pkg/deploy/sync.go +++ b/pkg/deploy/sync.go @@ -54,7 +54,7 @@ func SyncWithClient(cli client.Client, deployContext *chetypes.DeployContext, bl } key := types.NamespacedName{Name: blueprint.GetName(), Namespace: blueprint.GetNamespace()} - exists, err := GetWithClient(cli, key, actual.(client.Object)) + exists, err := doGet(context.TODO(), cli, key, actual.(client.Object)) if err != nil { return false, err } @@ -62,17 +62,18 @@ func SyncWithClient(cli client.Client, deployContext *chetypes.DeployContext, bl // set GroupVersionKind (it might be empty) actual.GetObjectKind().SetGroupVersionKind(runtimeObject.GetObjectKind().GroupVersionKind()) if !exists { - return CreateWithClient(cli, deployContext, blueprint, false) + return doCreate(context.TODO(), cli, deployContext, blueprint, false) } - return UpdateWithClient(cli, deployContext, actual.(client.Object), blueprint, diffOpts...) + return doUpdate(cli, deployContext, actual.(client.Object), blueprint, diffOpts...) } -// Gets object by key. +// Get gets object. // Returns true if object exists otherwise returns false. +// Returns error if object cannot be retrieved otherwise returns nil. func Get(deployContext *chetypes.DeployContext, key client.ObjectKey, actual client.Object) (bool, error) { cli := getClientForObject(key.Namespace, deployContext) - return GetWithClient(cli, key, actual) + return doGet(context.TODO(), cli, key, actual) } // Gets namespaced scope object by name @@ -80,7 +81,7 @@ func Get(deployContext *chetypes.DeployContext, key client.ObjectKey, actual cli func GetNamespacedObject(deployContext *chetypes.DeployContext, name string, actual client.Object) (bool, error) { client := deployContext.ClusterAPI.Client key := types.NamespacedName{Name: name, Namespace: deployContext.CheCluster.Namespace} - return GetWithClient(client, key, actual) + return doGet(context.TODO(), client, key, actual) } // Gets cluster scope object by name @@ -88,34 +89,15 @@ func GetNamespacedObject(deployContext *chetypes.DeployContext, name string, act func GetClusterObject(deployContext *chetypes.DeployContext, name string, actual client.Object) (bool, error) { client := deployContext.ClusterAPI.NonCachingClient key := types.NamespacedName{Name: name} - return GetWithClient(client, key, actual) + return doGet(context.TODO(), client, key, actual) } -// Creates object. -// Return true if a new object is created, false if it has been already created or error occurred. -func CreateIfNotExists(deployContext *chetypes.DeployContext, blueprint client.Object) (isCreated bool, err error) { +// CreateIgnoreIfExists creates object. +// Return true if a new object is created or object already exists, otherwise returns false. +// Throws error if object cannot be created otherwise returns nil. +func CreateIgnoreIfExists(deployContext *chetypes.DeployContext, blueprint client.Object) (bool, error) { cli := getClientForObject(blueprint.GetNamespace(), deployContext) - return CreateIfNotExistsWithClient(cli, deployContext, blueprint) -} - -func CreateIfNotExistsWithClient(cli client.Client, deployContext *chetypes.DeployContext, blueprint client.Object) (isCreated bool, err error) { - key := types.NamespacedName{Name: blueprint.GetName(), Namespace: blueprint.GetNamespace()} - actual := blueprint.DeepCopyObject().(client.Object) - exists, err := GetWithClient(cli, key, actual) - if exists { - return false, nil - } else if err != nil { - return false, err - } - - return CreateWithClient(cli, deployContext, blueprint, false) -} - -// Creates object. -// Return true if a new object is created otherwise returns false. -func Create(deployContext *chetypes.DeployContext, blueprint client.Object) (bool, error) { - client := getClientForObject(blueprint.GetNamespace(), deployContext) - return CreateWithClient(client, deployContext, blueprint, false) + return doCreate(context.TODO(), cli, deployContext, blueprint, true) } // Deletes object. @@ -137,60 +119,6 @@ func DeleteClusterObject(deployContext *chetypes.DeployContext, name string, obj return DeleteByKeyWithClient(client, key, objectMeta) } -// Updates object. -// Returns true if object is up to date otherwiser return false -func UpdateWithClient(client client.Client, deployContext *chetypes.DeployContext, actual client.Object, blueprint client.Object, diffOpts ...cmp.Option) (bool, error) { - actualMeta, ok := actual.(metav1.Object) - if !ok { - return false, fmt.Errorf("object %T is not a metav1.Object. Cannot sync it", actualMeta) - } - - diff := cmp.Diff(actual, blueprint, diffOpts...) - if len(diff) > 0 { - // don't print difference if there are no diffOpts mainly to avoid huge output - if len(diffOpts) != 0 { - fmt.Printf("Difference:\n%s", diff) - } - - if isUpdateUsingDeleteCreate(actual.GetObjectKind().GroupVersionKind().Kind) { - done, err := doDeleteIgnoreIfNotFound(context.TODO(), client, actual) - if !done { - return false, err - } - return CreateWithClient(client, deployContext, blueprint, false) - } else { - err := setOwnerReferenceIfNeeded(deployContext, blueprint) - if err != nil { - return false, err - } - - // to be able to update, we need to set the resource version of the object that we know of - blueprint.(metav1.Object).SetResourceVersion(actualMeta.GetResourceVersion()) - err = client.Update(context.TODO(), blueprint) - syncLog.Info("Object updated", "namespace", actual.GetNamespace(), "kind", GetObjectType(actual), "name", actual.GetName()) - return false, err - } - } - return true, nil -} - -func CreateWithClient(client client.Client, deployContext *chetypes.DeployContext, blueprint client.Object, returnTrueIfAlreadyExists bool) (bool, error) { - err := setOwnerReferenceIfNeeded(deployContext, blueprint) - if err != nil { - return false, err - } - - err = client.Create(context.TODO(), blueprint) - if err == nil { - syncLog.Info("Object created", "namespace", blueprint.GetNamespace(), "kind", GetObjectType(blueprint), "name", blueprint.GetName()) - return true, nil - } else if errors.IsAlreadyExists(err) { - return returnTrueIfAlreadyExists, nil - } else { - return false, err - } -} - func DeleteByKeyWithClient(cli client.Client, key client.ObjectKey, objectMeta client.Object) (bool, error) { runtimeObject, ok := objectMeta.(runtime.Object) if !ok { @@ -198,7 +126,7 @@ func DeleteByKeyWithClient(cli client.Client, key client.ObjectKey, objectMeta c } actual := runtimeObject.DeepCopyObject().(client.Object) - exists, err := GetWithClient(cli, key, actual) + exists, err := doGet(context.TODO(), cli, key, actual) if !exists { return true, nil } else if err != nil { @@ -208,17 +136,6 @@ func DeleteByKeyWithClient(cli client.Client, key client.ObjectKey, objectMeta c return doDeleteIgnoreIfNotFound(context.TODO(), cli, actual) } -func GetWithClient(client client.Client, key client.ObjectKey, object client.Object) (bool, error) { - err := client.Get(context.TODO(), key, object) - if err == nil { - return true, nil - } else if errors.IsNotFound(err) { - return false, nil - } else { - return false, err - } -} - func isUpdateUsingDeleteCreate(kind string) bool { return "Service" == kind || "Ingress" == kind || "Route" == kind || "Job" == kind || "Secret" == kind } @@ -255,7 +172,13 @@ func GetObjectType(obj interface{}) string { // DeleteIgnoreIfNotFound deletes object. // Returns nil if object deleted or not found otherwise returns error. -func DeleteIgnoreIfNotFound(context context.Context, cli client.Client, key client.ObjectKey, blueprint client.Object) error { +// Return error if object cannot be deleted otherwise returns nil. +func DeleteIgnoreIfNotFound( + context context.Context, + cli client.Client, + key client.ObjectKey, + blueprint client.Object, +) error { runtimeObj, ok := blueprint.(runtime.Object) if !ok { return fmt.Errorf("object %T is not a runtime.Object. Cannot sync it", runtimeObj) @@ -273,9 +196,14 @@ func DeleteIgnoreIfNotFound(context context.Context, cli client.Client, key clie } // doCreate creates object. -// Returns true if object created otherwise returns false. -// Throws error if object cannot be created or already exists otherwise returns nil. -func doCreate(context context.Context, client client.Client, deployContext *chetypes.DeployContext, blueprint client.Object) (bool, error) { +// Return error if object cannot be created otherwise returns nil. +func doCreate( + context context.Context, + client client.Client, + deployContext *chetypes.DeployContext, + blueprint client.Object, + returnTrueIfAlreadyExists bool, +) (bool, error) { err := setOwnerReferenceIfNeeded(deployContext, blueprint) if err != nil { return false, err @@ -285,6 +213,8 @@ func doCreate(context context.Context, client client.Client, deployContext *chet if err == nil { syncLog.Info("Object created", "namespace", blueprint.GetNamespace(), "kind", GetObjectType(blueprint), "name", blueprint.GetName()) return true, nil + } else if errors.IsAlreadyExists(err) { + return returnTrueIfAlreadyExists, nil } else { return false, err } @@ -293,7 +223,11 @@ func doCreate(context context.Context, client client.Client, deployContext *chet // doDeleteIgnoreIfNotFound deletes object. // Returns true if object deleted or not found otherwise returns false. // Returns error if object cannot be deleted otherwise returns nil. -func doDeleteIgnoreIfNotFound(context context.Context, cli client.Client, actual client.Object) (bool, error) { +func doDeleteIgnoreIfNotFound( + context context.Context, + cli client.Client, + actual client.Object, +) (bool, error) { err := cli.Delete(context, actual) if err == nil { if errors.IsNotFound(err) { @@ -310,7 +244,12 @@ func doDeleteIgnoreIfNotFound(context context.Context, cli client.Client, actual // doGet gets object. // Returns true if object exists otherwise returns false. // Returns error if object cannot be retrieved otherwise returns nil. -func doGet(context context.Context, cli client.Client, key client.ObjectKey, object client.Object) (bool, error) { +func doGet( + context context.Context, + cli client.Client, + key client.ObjectKey, + object client.Object, +) (bool, error) { err := cli.Get(context, key, object) if err == nil { return true, nil @@ -320,3 +259,49 @@ func doGet(context context.Context, cli client.Client, key client.ObjectKey, obj return false, err } } + +// doUpdate updates object. +// Returns true if object is up-to-date otherwise return false +func doUpdate( + cli client.Client, + deployContext *chetypes.DeployContext, + actual client.Object, + blueprint client.Object, + diffOpts ...cmp.Option, +) (bool, error) { + actualMeta, ok := actual.(metav1.Object) + if !ok { + return false, fmt.Errorf("object %T is not a metav1.Object. Cannot sync it", actualMeta) + } + + diff := cmp.Diff(actual, blueprint, diffOpts...) + if len(diff) > 0 { + // don't print difference if there are no diffOpts mainly to avoid huge output + if len(diffOpts) != 0 { + fmt.Printf("Difference:\n%s", diff) + } + + if isUpdateUsingDeleteCreate(actual.GetObjectKind().GroupVersionKind().Kind) { + done, err := doDeleteIgnoreIfNotFound(context.TODO(), cli, actual) + if !done { + return false, err + } + return doCreate(context.TODO(), cli, deployContext, blueprint, false) + } else { + err := setOwnerReferenceIfNeeded(deployContext, blueprint) + if err != nil { + return false, err + } + + // to be able to update, we need to set the resource version of the object that we know of + blueprint.(metav1.Object).SetResourceVersion(actualMeta.GetResourceVersion()) + err = cli.Update(context.TODO(), blueprint) + if err == nil { + syncLog.Info("Object updated", "namespace", actual.GetNamespace(), "kind", GetObjectType(actual), "name", actual.GetName()) + } + return false, err + } + } + + return true, nil +} diff --git a/pkg/deploy/sync_test.go b/pkg/deploy/sync_test.go index e35e7bf9a..43aafcce4 100644 --- a/pkg/deploy/sync_test.go +++ b/pkg/deploy/sync_test.go @@ -15,6 +15,8 @@ package deploy import ( "context" + "github.com/stretchr/testify/assert" + chev2 "github.com/eclipse-che/che-operator/api/v2" "github.com/eclipse-che/che-operator/pkg/common/chetypes" "github.com/google/go-cmp/cmp" @@ -74,68 +76,28 @@ func TestGet(t *testing.T) { } } -func TestCreate(t *testing.T) { +func TestCreateIgnoreIfExistsShouldReturnTrueIfObjectCreated(t *testing.T) { cli, deployContext := initDeployContext() - done, err := Create(deployContext, testObj.DeepCopy()) - if err != nil { - t.Fatalf("Failed to create object: %v", err) - } - - if !done { - t.Fatalf("Object has not been created") - } + done, err := CreateIgnoreIfExists(deployContext, testObj.DeepCopy()) + assert.NoError(t, err) + assert.True(t, done) actual := &corev1.Secret{} err = cli.Get(context.TODO(), testKey, actual) - if err != nil && !errors.IsNotFound(err) { - t.Fatalf("Failed to get object: %v", err) - } - - if actual == nil { - t.Fatalf("Object not found") - } + assert.NoError(t, err) + assert.NotNil(t, actual) } -func TestCreateIfNotExistsShouldReturnTrueIfObjectCreated(t *testing.T) { - cli, deployContext := initDeployContext() - - done, err := CreateIfNotExists(deployContext, testObj.DeepCopy()) - if err != nil { - t.Fatalf("Failed to create object: %v", err) - } - - if !done { - t.Fatalf("Object has not been created") - } - - actual := &corev1.Secret{} - err = cli.Get(context.TODO(), testKey, actual) - if err != nil && !errors.IsNotFound(err) { - t.Fatalf("Failed to get object: %v", err) - } - - if actual == nil { - t.Fatalf("Object not found") - } -} - -func TestCreateIfNotExistsShouldReturnFalseIfObjectExist(t *testing.T) { +func TestCreateIgnoreIfExistsShouldReturnTrueIfObjectExist(t *testing.T) { cli, deployContext := initDeployContext() err := cli.Create(context.TODO(), testObj.DeepCopy()) - if err != nil { - t.Fatalf("Failed to create object: %v", err) - } + assert.NoError(t, err) - isCreated, err := CreateIfNotExists(deployContext, testObj.DeepCopy()) - if err != nil { - t.Fatalf("Failed to create object: %v", err) - } - - if isCreated { - t.Fatalf("Object has been created") - } + done, err := CreateIgnoreIfExists(deployContext, testObj.DeepCopy()) + assert.NoError(t, err) + assert.True(t, done) } func TestUpdate(t *testing.T) { @@ -152,7 +114,7 @@ func TestUpdate(t *testing.T) { t.Fatalf("Failed to get object: %v", err) } - _, err = UpdateWithClient(cli, deployContext, actual, testObjLabeled.DeepCopy(), cmp.Options{}) + _, err = doUpdate(cli, deployContext, actual, testObjLabeled.DeepCopy(), cmp.Options{}) if err != nil { t.Fatalf("Failed to update object: %v", err) } diff --git a/pkg/deploy/tls/certificates.go b/pkg/deploy/tls/certificates.go index 9a4e6fe86..cc098ee63 100644 --- a/pkg/deploy/tls/certificates.go +++ b/pkg/deploy/tls/certificates.go @@ -94,8 +94,7 @@ func (c *CertificatesReconciler) syncTrustStoreConfigMapToCluster(ctx *chetypes. if !exists { // We have to create an empty config map with the specific labels - done, err := deploy.Create(ctx, configMapSpec) - return done, err + return deploy.CreateIgnoreIfExists(ctx, configMapSpec) } if actual.ObjectMeta.Labels[injector] != "true" ||