fix: Remove plugin registry pod if openVSX configured (#1913)

* fix: Remove plugin registry pod if openVSX configured Signed-off-by: Anatolii Bazko <[email protected]>
eclipse-che · Oct 8, 2024 · 891a727 · 891a727
1 parent 6960b81
commit 891a727
Show file tree

Hide file tree

Showing 9 changed files with 144 additions and 16 deletions.
diff --git a/api/v2/checluster_types.go b/api/v2/checluster_types.go
@@ -1034,11 +1034,14 @@ func (c *CheCluster) IsCheFlavor() bool {
 // IsEmbeddedOpenVSXRegistryConfigured returns true if the Open VSX Registry is configured to be embedded
 // only if only the `Spec.Components.PluginRegistry.OpenVSXURL` is empty.
 func (c *CheCluster) IsEmbeddedOpenVSXRegistryConfigured() bool {
-	openVSXURL := defaults.GetPluginRegistryOpenVSXURL()
 	if c.Spec.Components.PluginRegistry.OpenVSXURL != nil {
-		openVSXURL = *c.Spec.Components.PluginRegistry.OpenVSXURL
+		return *c.Spec.Components.PluginRegistry.OpenVSXURL == ""
 	}
-	return openVSXURL == ""
+	return defaults.GetPluginRegistryOpenVSXURL() == ""
+}
+
+func (c *CheCluster) IsInternalPluginRegistryDisabled() bool {
+	return c.Spec.Components.PluginRegistry.DisableInternalRegistry || !c.IsEmbeddedOpenVSXRegistryConfigured()
 }
 
 // IsCheBeingInstalled returns true if the Che version is not set in the status.

diff --git a/pkg/deploy/dashboard/dashboard_deployment_test.go b/pkg/deploy/dashboard/dashboard_deployment_test.go
@@ -224,13 +224,6 @@ func TestDashboardDeploymentEnvVars(t *testing.T) {
 			Name:  "CHE_DASHBOARD_INTERNAL_URL",
 			Value: fmt.Sprintf("http://%s-dashboard.eclipse-che.svc:8080", defaults.GetCheFlavor()),
 		})
-	assert.Contains(
-		t,
-		deployment.Spec.Template.Spec.Containers[0].Env,
-		corev1.EnvVar{
-			Name:  "CHE_WORKSPACE_PLUGIN__REGISTRY__INTERNAL__URL",
-			Value: "http://plugin-registry.eclipse-che.svc:8080/v3",
-		})
 	assert.Contains(
 		t,
 		deployment.Spec.Template.Spec.Containers[0].Env,

diff --git a/pkg/deploy/dashboard/deployment_dashboard.go b/pkg/deploy/dashboard/deployment_dashboard.go
@@ -83,7 +83,7 @@ func (d *DashboardReconciler) getDashboardDeploymentSpec(ctx *chetypes.DeployCon
 			Value: fmt.Sprintf("http://%s.%s.svc:8080/api", deploy.CheServiceName, ctx.CheCluster.Namespace)},
 	)
 
-	if !ctx.CheCluster.Spec.Components.PluginRegistry.DisableInternalRegistry {
+	if !ctx.CheCluster.IsInternalPluginRegistryDisabled() {
 		envVars = append(envVars,
 			corev1.EnvVar{
 				Name:  "CHE_WORKSPACE_PLUGIN__REGISTRY__INTERNAL__URL",

diff --git a/pkg/deploy/gateway/gateway_test.go b/pkg/deploy/gateway/gateway_test.go
@@ -18,6 +18,8 @@ import (
 	"strings"
 	"testing"
 
+	"k8s.io/utils/pointer"
+
 	"github.com/devfile/devworkspace-operator/pkg/infrastructure"
 	chev2 "github.com/eclipse-che/che-operator/api/v2"
 	"github.com/eclipse-che/che-operator/pkg/common/chetypes"
@@ -170,6 +172,7 @@ func TestOauthProxyConfigUnauthorizedPaths(t *testing.T) {
 					Components: chev2.CheClusterComponents{
 						PluginRegistry: chev2.PluginRegistry{
 							DisableInternalRegistry: false,
+							OpenVSXURL:              pointer.String(""),
 						},
 					}},
 			}, nil)

diff --git a/pkg/deploy/gateway/oauth_proxy.go b/pkg/deploy/gateway/oauth_proxy.go
@@ -160,7 +160,7 @@ cookie_domains = "%s"
 
 func skipAuthConfig(instance *chev2.CheCluster) string {
 	var skipAuthPaths []string
-	if !instance.Spec.Components.PluginRegistry.DisableInternalRegistry {
+	if !instance.IsInternalPluginRegistryDisabled() {
 		skipAuthPaths = append(skipAuthPaths, "^/"+constants.PluginRegistryName)
 	}
 	skipAuthPaths = append(skipAuthPaths, "^/$")

diff --git a/pkg/deploy/pluginregistry/pluginregistry.go b/pkg/deploy/pluginregistry/pluginregistry.go
@@ -16,6 +16,9 @@ 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"
@@ -34,7 +37,12 @@ func NewPluginRegistryReconciler() *PluginRegistryReconciler {
 }
 
 func (p *PluginRegistryReconciler) Reconcile(ctx *chetypes.DeployContext) (reconcile.Result, bool, error) {
-	if ctx.CheCluster.Spec.Components.PluginRegistry.DisableInternalRegistry {
+	if ctx.CheCluster.IsInternalPluginRegistryDisabled() {
+		_, _ = deploy.DeleteNamespacedObject(ctx, constants.PluginRegistryName, &corev1.Service{})
+		_, _ = deploy.DeleteNamespacedObject(ctx, constants.PluginRegistryName, &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", "")

diff --git a/pkg/deploy/pluginregistry/pluginregistry_test.go b/pkg/deploy/pluginregistry/pluginregistry_test.go
@@ -13,21 +13,39 @@
 package pluginregistry
 
 import (
+	"os"
+
 	"github.com/devfile/devworkspace-operator/pkg/infrastructure"
+	chev2 "github.com/eclipse-che/che-operator/api/v2"
+	defaults "github.com/eclipse-che/che-operator/pkg/common/operator-defaults"
 	"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"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/utils/pointer"
 
 	"testing"
 )
 
-func TestPluginRegistryReconcile(t *testing.T) {
+func TestShouldDeployPluginRegistryIfOpenVSXIsEmpty(t *testing.T) {
 	infrastructure.InitializeForTesting(infrastructure.OpenShiftv4)
 
-	ctx := test.GetDeployContext(nil, []runtime.Object{})
+	ctx := test.GetDeployContext(&chev2.CheCluster{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "eclipse-che",
+			Namespace: "eclipse-che",
+		},
+		Spec: chev2.CheClusterSpec{
+			Components: chev2.CheClusterComponents{
+				PluginRegistry: chev2.PluginRegistry{
+					OpenVSXURL: pointer.String(""),
+				},
+			},
+		},
+	}, []runtime.Object{})
 
 	pluginregistry := NewPluginRegistryReconciler()
 	_, done, err := pluginregistry.Reconcile(ctx)
@@ -39,3 +57,96 @@ func TestPluginRegistryReconcile(t *testing.T) {
 	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)
 }
+
+func TestShouldDeployPluginRegistryIfOpenVSXIsEmptyByDefault(t *testing.T) {
+	defaultOpenVSXURL := os.Getenv("CHE_DEFAULT_SPEC_COMPONENTS_PLUGINREGISTRY_OPENVSXURL")
+
+	err := os.Unsetenv("CHE_DEFAULT_SPEC_COMPONENTS_PLUGINREGISTRY_OPENVSXURL")
+	assert.NoError(t, err)
+
+	defer func() {
+		_ = os.Setenv("CHE_DEFAULT_SPEC_COMPONENTS_PLUGINREGISTRY_OPENVSXURL", defaultOpenVSXURL)
+	}()
+
+	// re initialize defaults with new env var
+	defaults.Initialize()
+
+	infrastructure.InitializeForTesting(infrastructure.OpenShiftv4)
+
+	ctx := test.GetDeployContext(&chev2.CheCluster{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "eclipse-che",
+			Namespace: "eclipse-che",
+		},
+	}, []runtime.Object{})
+
+	pluginregistry := NewPluginRegistryReconciler()
+	_, done, err := pluginregistry.Reconcile(ctx)
+	assert.True(t, done)
+	assert.Nil(t, err)
+
+	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: "plugin-registry", Namespace: "eclipse-che"}, &appsv1.Deployment{}))
+	assert.NotEmpty(t, ctx.CheCluster.Status.PluginRegistryURL)
+}
+
+func TestShouldNotDeployPluginRegistryIfOpenVSXConfigured(t *testing.T) {
+	infrastructure.InitializeForTesting(infrastructure.OpenShiftv4)
+
+	ctx := test.GetDeployContext(&chev2.CheCluster{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "eclipse-che",
+			Namespace: "eclipse-che",
+		},
+		Spec: chev2.CheClusterSpec{
+			Components: chev2.CheClusterComponents{
+				PluginRegistry: chev2.PluginRegistry{
+					OpenVSXURL: pointer.String("https://openvsx.org"),
+				},
+			},
+		},
+	}, []runtime.Object{})
+
+	pluginregistry := NewPluginRegistryReconciler()
+	_, done, err := pluginregistry.Reconcile(ctx)
+	assert.True(t, done)
+	assert.Nil(t, err)
+
+	assert.False(t, test.IsObjectExists(ctx.ClusterAPI.Client, types.NamespacedName{Name: "plugin-registry", Namespace: "eclipse-che"}, &corev1.Service{}))
+	assert.False(t, test.IsObjectExists(ctx.ClusterAPI.Client, types.NamespacedName{Name: "plugin-registry", Namespace: "eclipse-che"}, &corev1.ConfigMap{}))
+	assert.False(t, test.IsObjectExists(ctx.ClusterAPI.Client, types.NamespacedName{Name: "plugin-registry", Namespace: "eclipse-che"}, &appsv1.Deployment{}))
+	assert.Empty(t, ctx.CheCluster.Status.PluginRegistryURL)
+}
+
+func TestShouldNotDeployPluginRegistryIfOpenVSXConfiguredByDefault(t *testing.T) {
+	defaultOpenVSXURL := os.Getenv("CHE_DEFAULT_SPEC_COMPONENTS_PLUGINREGISTRY_OPENVSXURL")
+	err := os.Setenv("CHE_DEFAULT_SPEC_COMPONENTS_PLUGINREGISTRY_OPENVSXURL", "https://openvsx.org")
+	assert.NoError(t, err)
+
+	defer func() {
+		_ = os.Setenv("CHE_DEFAULT_SPEC_COMPONENTS_PLUGINREGISTRY_OPENVSXURL", defaultOpenVSXURL)
+	}()
+
+	// re initialize defaults with new env var
+	defaults.Initialize()
+
+	infrastructure.InitializeForTesting(infrastructure.OpenShiftv4)
+
+	ctx := test.GetDeployContext(&chev2.CheCluster{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "eclipse-che",
+			Namespace: "eclipse-che",
+		},
+	}, []runtime.Object{})
+
+	pluginregistry := NewPluginRegistryReconciler()
+	_, done, err := pluginregistry.Reconcile(ctx)
+	assert.True(t, done)
+	assert.Nil(t, err)
+
+	assert.False(t, test.IsObjectExists(ctx.ClusterAPI.Client, types.NamespacedName{Name: "plugin-registry", Namespace: "eclipse-che"}, &corev1.Service{}))
+	assert.False(t, test.IsObjectExists(ctx.ClusterAPI.Client, types.NamespacedName{Name: "plugin-registry", Namespace: "eclipse-che"}, &corev1.ConfigMap{}))
+	assert.False(t, test.IsObjectExists(ctx.ClusterAPI.Client, types.NamespacedName{Name: "plugin-registry", Namespace: "eclipse-che"}, &appsv1.Deployment{}))
+	assert.Empty(t, ctx.CheCluster.Status.PluginRegistryURL)
+}
diff --git a/pkg/deploy/server/server_configmap.go b/pkg/deploy/server/server_configmap.go
@@ -142,7 +142,7 @@ func (s *CheServerReconciler) getCheConfigMapData(ctx *chetypes.DeployContext) (
 	cheAPI := "https://" + ctx.CheHost + "/api"
 	var pluginRegistryInternalURL string
 
-	if !ctx.CheCluster.Spec.Components.PluginRegistry.DisableInternalRegistry {
+	if !ctx.CheCluster.IsInternalPluginRegistryDisabled() {
 		pluginRegistryInternalURL = fmt.Sprintf("http://%s.%s.svc:8080/v3", constants.PluginRegistryName, ctx.CheCluster.Namespace)
 	}
 

diff --git a/pkg/deploy/server/server_configmap_test.go b/pkg/deploy/server/server_configmap_test.go
@@ -15,6 +15,8 @@ package server
 import (
 	"testing"
 
+	"k8s.io/utils/pointer"
+
 	"github.com/devfile/devworkspace-operator/pkg/infrastructure"
 	"github.com/eclipse-che/che-operator/pkg/common/test"
 	"github.com/stretchr/testify/assert"
@@ -349,6 +351,14 @@ func TestShouldSetUpCorrectlyPluginRegistryURL(t *testing.T) {
 				ObjectMeta: metav1.ObjectMeta{
 					Namespace: "eclipse-che",
 				},
+				Spec: chev2.CheClusterSpec{
+					Components: chev2.CheClusterComponents{
+						PluginRegistry: chev2.PluginRegistry{
+							DisableInternalRegistry: false,
+							OpenVSXURL:              pointer.String(""),
+						},
+					},
+				},
 				Status: chev2.CheClusterStatus{
 					PluginRegistryURL: "internal-plugin-registry",
 				},