From 0a73ed0ecad8da1053e4a8136ecd6f9aba2c1409 Mon Sep 17 00:00:00 2001 From: Anatolii Bazko Date: Tue, 8 Oct 2024 16:29:40 +0200 Subject: [PATCH] fix: Remove plugin registry pod if openVSX configured (#1913) (#1914) * fix: Remove plugin registry pod if openVSX configured Signed-off-by: Anatolii Bazko --- api/v2/checluster_types.go | 9 +- .../dashboard/dashboard_deployment_test.go | 7 -- pkg/deploy/dashboard/deployment_dashboard.go | 2 +- pkg/deploy/gateway/gateway_test.go | 3 + pkg/deploy/gateway/oauth_proxy.go | 2 +- pkg/deploy/pluginregistry/pluginregistry.go | 10 +- .../pluginregistry/pluginregistry_test.go | 115 +++++++++++++++++- pkg/deploy/server/server_configmap.go | 2 +- pkg/deploy/server/server_configmap_test.go | 10 ++ 9 files changed, 144 insertions(+), 16 deletions(-) diff --git a/api/v2/checluster_types.go b/api/v2/checluster_types.go index 7c2ffd3b92..68a18769aa 100644 --- 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 index dd1dbc486d..03257beaf0 100644 --- 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 index 3d5b48c94d..7c0e5e2b0b 100644 --- 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 index e3ed9a69f6..0ec9ff56cd 100644 --- 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 index 771adf69b5..91e58fa525 100644 --- 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 index c6d6af4416..7e7189ca06 100644 --- 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 index 71ead00f17..fa6434bf64 100644 --- 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 index 14f7575ef0..fa6044df65 100644 --- 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 index ce27683e03..3469f38b31 100644 --- 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", },