From 4fb2521678952218c229acc4fdfb08b2d8389279 Mon Sep 17 00:00:00 2001 From: Anatolii Bazko Date: Wed, 10 Feb 2021 12:16:38 +0200 Subject: [PATCH 1/4] Allow to configure domain to create routes Signed-off-by: Anatolii Bazko --- deploy/crds/org_v1_che_crd.yaml | 28 +++ .../manifests/org_v1_che_crd.yaml | 28 +++ .../manifests/org_v1_che_crd.yaml | 28 +++ pkg/apis/org/v1/che_types.go | 5 + pkg/controller/che/che_controller.go | 25 ++- pkg/controller/che/che_controller_test.go | 28 +-- .../devfile-registry/devfile_registry.go | 9 +- pkg/deploy/expose/expose.go | 8 +- .../identity-provider/identity_provider.go | 7 +- pkg/deploy/ingres_test.go | 150 ++++++++++++++ pkg/deploy/ingress.go | 9 +- pkg/deploy/plugin-registry/plugin_registry.go | 9 +- pkg/deploy/route.go | 21 +- pkg/deploy/route_test.go | 190 ++++++++++++++++++ pkg/deploy/tls.go | 20 +- 15 files changed, 519 insertions(+), 46 deletions(-) create mode 100644 pkg/deploy/ingres_test.go create mode 100644 pkg/deploy/route_test.go diff --git a/deploy/crds/org_v1_che_crd.yaml b/deploy/crds/org_v1_che_crd.yaml index be17424afb..f7daa9d3b8 100644 --- a/deploy/crds/org_v1_che_crd.yaml +++ b/deploy/crds/org_v1_che_crd.yaml @@ -153,6 +153,13 @@ spec: identityProviderRoute: description: Route custom settings. properties: + domain: + description: 'Operator uses the domain to generate a hostname + for a route. Operator uses domain in a conjunction with labels + to create a route, witch is served by a non-default Ingress + controller. The generated host name will follow this pattern: + `-.`.' + type: string labels: description: Comma separated list of labels that can be used to organize and categorize objects by scoping and selecting. @@ -475,6 +482,13 @@ spec: cheServerRoute: description: The Che server route custom settings. properties: + domain: + description: 'Operator uses the domain to generate a hostname + for a route. Operator uses domain in a conjunction with labels + to create a route, witch is served by a non-default Ingress + controller. The generated host name will follow this pattern: + `-.`.' + type: string labels: description: Comma separated list of labels that can be used to organize and categorize objects by scoping and selecting. @@ -532,6 +546,13 @@ spec: devfileRegistryRoute: description: The devfile registry route custom settings. properties: + domain: + description: 'Operator uses the domain to generate a hostname + for a route. Operator uses domain in a conjunction with labels + to create a route, witch is served by a non-default Ingress + controller. The generated host name will follow this pattern: + `-.`.' + type: string labels: description: Comma separated list of labels that can be used to organize and categorize objects by scoping and selecting. @@ -612,6 +633,13 @@ spec: pluginRegistryRoute: description: Plugin registry route custom settings. properties: + domain: + description: 'Operator uses the domain to generate a hostname + for a route. Operator uses domain in a conjunction with labels + to create a route, witch is served by a non-default Ingress + controller. The generated host name will follow this pattern: + `-.`.' + type: string labels: description: Comma separated list of labels that can be used to organize and categorize objects by scoping and selecting. diff --git a/deploy/olm-catalog/eclipse-che-preview-kubernetes/manifests/org_v1_che_crd.yaml b/deploy/olm-catalog/eclipse-che-preview-kubernetes/manifests/org_v1_che_crd.yaml index be17424afb..f7daa9d3b8 100644 --- a/deploy/olm-catalog/eclipse-che-preview-kubernetes/manifests/org_v1_che_crd.yaml +++ b/deploy/olm-catalog/eclipse-che-preview-kubernetes/manifests/org_v1_che_crd.yaml @@ -153,6 +153,13 @@ spec: identityProviderRoute: description: Route custom settings. properties: + domain: + description: 'Operator uses the domain to generate a hostname + for a route. Operator uses domain in a conjunction with labels + to create a route, witch is served by a non-default Ingress + controller. The generated host name will follow this pattern: + `-.`.' + type: string labels: description: Comma separated list of labels that can be used to organize and categorize objects by scoping and selecting. @@ -475,6 +482,13 @@ spec: cheServerRoute: description: The Che server route custom settings. properties: + domain: + description: 'Operator uses the domain to generate a hostname + for a route. Operator uses domain in a conjunction with labels + to create a route, witch is served by a non-default Ingress + controller. The generated host name will follow this pattern: + `-.`.' + type: string labels: description: Comma separated list of labels that can be used to organize and categorize objects by scoping and selecting. @@ -532,6 +546,13 @@ spec: devfileRegistryRoute: description: The devfile registry route custom settings. properties: + domain: + description: 'Operator uses the domain to generate a hostname + for a route. Operator uses domain in a conjunction with labels + to create a route, witch is served by a non-default Ingress + controller. The generated host name will follow this pattern: + `-.`.' + type: string labels: description: Comma separated list of labels that can be used to organize and categorize objects by scoping and selecting. @@ -612,6 +633,13 @@ spec: pluginRegistryRoute: description: Plugin registry route custom settings. properties: + domain: + description: 'Operator uses the domain to generate a hostname + for a route. Operator uses domain in a conjunction with labels + to create a route, witch is served by a non-default Ingress + controller. The generated host name will follow this pattern: + `-.`.' + type: string labels: description: Comma separated list of labels that can be used to organize and categorize objects by scoping and selecting. diff --git a/deploy/olm-catalog/eclipse-che-preview-openshift/manifests/org_v1_che_crd.yaml b/deploy/olm-catalog/eclipse-che-preview-openshift/manifests/org_v1_che_crd.yaml index 491412b42a..3f26d9b8e2 100644 --- a/deploy/olm-catalog/eclipse-che-preview-openshift/manifests/org_v1_che_crd.yaml +++ b/deploy/olm-catalog/eclipse-che-preview-openshift/manifests/org_v1_che_crd.yaml @@ -154,6 +154,13 @@ spec: identityProviderRoute: description: Route custom settings. properties: + domain: + description: 'Operator uses the domain to generate a hostname + for a route. Operator uses domain in a conjunction with labels + to create a route, witch is served by a non-default Ingress + controller. The generated host name will follow this pattern: + `-.`.' + type: string labels: description: Comma separated list of labels that can be used to organize and categorize objects by scoping and selecting. @@ -476,6 +483,13 @@ spec: cheServerRoute: description: The Che server route custom settings. properties: + domain: + description: 'Operator uses the domain to generate a hostname + for a route. Operator uses domain in a conjunction with labels + to create a route, witch is served by a non-default Ingress + controller. The generated host name will follow this pattern: + `-.`.' + type: string labels: description: Comma separated list of labels that can be used to organize and categorize objects by scoping and selecting. @@ -533,6 +547,13 @@ spec: devfileRegistryRoute: description: The devfile registry route custom settings. properties: + domain: + description: 'Operator uses the domain to generate a hostname + for a route. Operator uses domain in a conjunction with labels + to create a route, witch is served by a non-default Ingress + controller. The generated host name will follow this pattern: + `-.`.' + type: string labels: description: Comma separated list of labels that can be used to organize and categorize objects by scoping and selecting. @@ -613,6 +634,13 @@ spec: pluginRegistryRoute: description: Plugin registry route custom settings. properties: + domain: + description: 'Operator uses the domain to generate a hostname + for a route. Operator uses domain in a conjunction with labels + to create a route, witch is served by a non-default Ingress + controller. The generated host name will follow this pattern: + `-.`.' + type: string labels: description: Comma separated list of labels that can be used to organize and categorize objects by scoping and selecting. diff --git a/pkg/apis/org/v1/che_types.go b/pkg/apis/org/v1/che_types.go index a9ebd10049..a803db1385 100644 --- a/pkg/apis/org/v1/che_types.go +++ b/pkg/apis/org/v1/che_types.go @@ -437,6 +437,11 @@ type RouteCustomSettings struct { // Comma separated list of labels that can be used to organize and categorize objects by scoping and selecting. // +optional Labels string `json:"labels,omitempty"` + // Operator uses the domain to generate a hostname for a route. + // Operator uses domain in a conjunction with labels to create a route, witch is served by a non-default Ingress controller. + // The generated host name will follow this pattern: `-.`. + // +optional + Domain string `json:"domain,omitempty"` } // ResourceRequirements describes the compute resource requirements. diff --git a/pkg/controller/che/che_controller.go b/pkg/controller/che/che_controller.go index dbb53df285..50dc483700 100644 --- a/pkg/controller/che/che_controller.go +++ b/pkg/controller/che/che_controller.go @@ -397,7 +397,7 @@ func (r *ReconcileChe) Reconcile(request reconcile.Request) (reconcile.Result, e "openShiftoAuth": "nil", "initialOpenShiftOAuthUser": "nil", } - + if err := r.UpdateCheCRSpecByFields(instance, updateFields); err != nil { return reconcile.Result{}, err } @@ -822,14 +822,13 @@ func (r *ReconcileChe) Reconcile(request reconcile.Request) (reconcile.Result, e exposedServiceName := getServerExposingServiceName(instance) cheHost := "" if !isOpenShift { - additionalLabels := deployContext.CheCluster.Spec.Server.CheServerIngress.Labels ingress, err := deploy.SyncIngressToCluster( deployContext, cheFlavor, instance.Spec.Server.CheHost, exposedServiceName, 8080, - additionalLabels, + deployContext.CheCluster.Spec.Server.CheServerIngress, cheFlavor) if !tests { if ingress == nil { @@ -849,8 +848,14 @@ func (r *ReconcileChe) Reconcile(request reconcile.Request) (reconcile.Result, e customHost = "" } - additionalLabels := deployContext.CheCluster.Spec.Server.CheServerRoute.Labels - route, err := deploy.SyncRouteToCluster(deployContext, cheFlavor, customHost, exposedServiceName, 8080, additionalLabels, cheFlavor) + route, err := deploy.SyncRouteToCluster( + deployContext, + cheFlavor, + customHost, + exposedServiceName, + 8080, + deployContext.CheCluster.Spec.Server.CheServerRoute, + cheFlavor) if route == nil { logrus.Infof("Waiting on route '%s' to be ready", cheFlavor) if err != nil { @@ -1087,8 +1092,14 @@ func EvaluateCheServerVersion(cr *orgv1.CheCluster) string { func getDefaultCheHost(deployContext *deploy.DeployContext) (string, error) { cheFlavor := deploy.DefaultCheFlavor(deployContext.CheCluster) - additionalLabels := deployContext.CheCluster.Spec.Server.CheServerRoute.Labels - route, err := deploy.SyncRouteToCluster(deployContext, cheFlavor, "", getServerExposingServiceName(deployContext.CheCluster), 8080, additionalLabels, cheFlavor) + route, err := deploy.SyncRouteToCluster( + deployContext, + cheFlavor, + "", + getServerExposingServiceName(deployContext.CheCluster), + 8080, + deployContext.CheCluster.Spec.Server.CheServerRoute, + cheFlavor) if route == nil { logrus.Infof("Waiting on route '%s' to be ready", cheFlavor) if err != nil { diff --git a/pkg/controller/che/che_controller_test.go b/pkg/controller/che/che_controller_test.go index cf83eeb94d..0ace0fd2a5 100644 --- a/pkg/controller/che/che_controller_test.go +++ b/pkg/controller/che/che_controller_test.go @@ -1124,10 +1124,10 @@ func TestShouldSetUpCorrectlyInternalIdentityProviderServiceURL(t *testing.T) { } // Set up che host for route - cheRoute, _ := deploy.GetSpecRoute(deployContext, deploy.DefaultCheFlavor(testCase.cheCR), "che-host", "che-host", 8080, "", "che") + cheRoute, _ := deploy.GetSpecRoute(deployContext, deploy.DefaultCheFlavor(testCase.cheCR), "che-host", "che-host", 8080, orgv1.RouteCustomSettings{}, "che") r.client.Create(context.TODO(), cheRoute) // Set up keycloak host for route - keycloakRoute, _ := deploy.GetSpecRoute(deployContext, deploy.IdentityProviderName, "keycloak", deploy.IdentityProviderName, 8080, "", deploy.IdentityProviderName) + keycloakRoute, _ := deploy.GetSpecRoute(deployContext, deploy.IdentityProviderName, "keycloak", deploy.IdentityProviderName, 8080, orgv1.RouteCustomSettings{}, deploy.IdentityProviderName) r.client.Create(context.TODO(), keycloakRoute) _, err := r.Reconcile(req) @@ -1308,16 +1308,16 @@ func TestShouldSetUpCorrectlyInternalPluginRegistryServiceURL(t *testing.T) { } // Set up che host for route - cheRoute, _ := deploy.GetSpecRoute(deployContext, deploy.DefaultCheFlavor(testCase.cheCR), "che-host", "che-host", 8080, "", "che") + cheRoute, _ := deploy.GetSpecRoute(deployContext, deploy.DefaultCheFlavor(testCase.cheCR), "che-host", "che-host", 8080, orgv1.RouteCustomSettings{}, "che") r.client.Create(context.TODO(), cheRoute) // Set up keycloak host for route - keycloakRoute, _ := deploy.GetSpecRoute(deployContext, deploy.IdentityProviderName, "keycloak", deploy.IdentityProviderName, 8080, "", deploy.IdentityProviderName) + keycloakRoute, _ := deploy.GetSpecRoute(deployContext, deploy.IdentityProviderName, "keycloak", deploy.IdentityProviderName, 8080, orgv1.RouteCustomSettings{}, deploy.IdentityProviderName) r.client.Create(context.TODO(), keycloakRoute) // Set up plugin registry host for route - pluginRegistryRoute, _ := deploy.GetSpecRoute(deployContext, deploy.PluginRegistryName, "plugin-registry", deploy.PluginRegistryName, 8080, "", deploy.PluginRegistryName) + pluginRegistryRoute, _ := deploy.GetSpecRoute(deployContext, deploy.PluginRegistryName, "plugin-registry", deploy.PluginRegistryName, 8080, orgv1.RouteCustomSettings{}, deploy.PluginRegistryName) r.client.Create(context.TODO(), pluginRegistryRoute) // Set up devfile registry host for route - devfileRegistryRoute, _ := deploy.GetSpecRoute(deployContext, deploy.DevfileRegistryName, "devfile-registry", deploy.DevfileRegistryName, 8080, "", deploy.DevfileRegistryName) + devfileRegistryRoute, _ := deploy.GetSpecRoute(deployContext, deploy.DevfileRegistryName, "devfile-registry", deploy.DevfileRegistryName, 8080, orgv1.RouteCustomSettings{}, deploy.DevfileRegistryName) r.client.Create(context.TODO(), devfileRegistryRoute) _, err := r.Reconcile(req) @@ -1494,16 +1494,16 @@ func TestShouldSetUpCorrectlyInternalDevfileRegistryServiceURL(t *testing.T) { } // Set up che host for route - cheRoute, _ := deploy.GetSpecRoute(deployContext, deploy.DefaultCheFlavor(testCase.cheCR), "che-host", "che-host", 8080, "", "che") + cheRoute, _ := deploy.GetSpecRoute(deployContext, deploy.DefaultCheFlavor(testCase.cheCR), "che-host", "che-host", 8080, orgv1.RouteCustomSettings{}, "che") r.client.Create(context.TODO(), cheRoute) // Set up keycloak host for route - keycloakRoute, _ := deploy.GetSpecRoute(deployContext, deploy.IdentityProviderName, "keycloak", deploy.IdentityProviderName, 8080, "", deploy.IdentityProviderName) + keycloakRoute, _ := deploy.GetSpecRoute(deployContext, deploy.IdentityProviderName, "keycloak", deploy.IdentityProviderName, 8080, orgv1.RouteCustomSettings{}, deploy.IdentityProviderName) r.client.Create(context.TODO(), keycloakRoute) // Set up plugin registry host for route - pluginRegistryRoute, _ := deploy.GetSpecRoute(deployContext, deploy.PluginRegistryName, "plugin-registry", deploy.PluginRegistryName, 8080, "", deploy.PluginRegistryName) + pluginRegistryRoute, _ := deploy.GetSpecRoute(deployContext, deploy.PluginRegistryName, "plugin-registry", deploy.PluginRegistryName, 8080, orgv1.RouteCustomSettings{}, deploy.PluginRegistryName) r.client.Create(context.TODO(), pluginRegistryRoute) // Set up devfile registry host for route - devfileRegistryRoute, _ := deploy.GetSpecRoute(deployContext, deploy.DevfileRegistryName, "devfile-registry", deploy.DevfileRegistryName, 8080, "", deploy.DevfileRegistryName) + devfileRegistryRoute, _ := deploy.GetSpecRoute(deployContext, deploy.DevfileRegistryName, "devfile-registry", deploy.DevfileRegistryName, 8080, orgv1.RouteCustomSettings{}, deploy.DevfileRegistryName) r.client.Create(context.TODO(), devfileRegistryRoute) _, err := r.Reconcile(req) @@ -1629,16 +1629,16 @@ func TestShouldSetUpCorrectlyInternalCheServerURL(t *testing.T) { } // Set up che host for route - cheRoute, _ := deploy.GetSpecRoute(deployContext, deploy.DefaultCheFlavor(testCase.cheCR), "che-host", "che-host", 8080, "", "che") + cheRoute, _ := deploy.GetSpecRoute(deployContext, deploy.DefaultCheFlavor(testCase.cheCR), "che-host", "che-host", 8080, orgv1.RouteCustomSettings{}, "che") r.client.Create(context.TODO(), cheRoute) // Set up keycloak host for route - keycloakRoute, _ := deploy.GetSpecRoute(deployContext, deploy.IdentityProviderName, "keycloak", deploy.IdentityProviderName, 8080, "", deploy.IdentityProviderName) + keycloakRoute, _ := deploy.GetSpecRoute(deployContext, deploy.IdentityProviderName, "keycloak", deploy.IdentityProviderName, 8080, orgv1.RouteCustomSettings{}, deploy.IdentityProviderName) r.client.Create(context.TODO(), keycloakRoute) // Set up plugin registry host for route - pluginRegistryRoute, _ := deploy.GetSpecRoute(deployContext, deploy.PluginRegistryName, "plugin-registry", deploy.PluginRegistryName, 8080, "", deploy.PluginRegistryName) + pluginRegistryRoute, _ := deploy.GetSpecRoute(deployContext, deploy.PluginRegistryName, "plugin-registry", deploy.PluginRegistryName, 8080, orgv1.RouteCustomSettings{}, deploy.PluginRegistryName) r.client.Create(context.TODO(), pluginRegistryRoute) // Set up devfile registry host for route - devfileRegistryRoute, _ := deploy.GetSpecRoute(deployContext, deploy.DevfileRegistryName, "devfile-registry", deploy.DevfileRegistryName, 8080, "", deploy.DevfileRegistryName) + devfileRegistryRoute, _ := deploy.GetSpecRoute(deployContext, deploy.DevfileRegistryName, "devfile-registry", deploy.DevfileRegistryName, 8080, orgv1.RouteCustomSettings{}, deploy.DevfileRegistryName) r.client.Create(context.TODO(), devfileRegistryRoute) _, err := r.Reconcile(req) diff --git a/pkg/deploy/devfile-registry/devfile_registry.go b/pkg/deploy/devfile-registry/devfile_registry.go index cb1f8ba3ac..109bc76eaa 100644 --- a/pkg/deploy/devfile-registry/devfile_registry.go +++ b/pkg/deploy/devfile-registry/devfile_registry.go @@ -34,8 +34,13 @@ type DevFileRegistryConfigMap struct { func SyncDevfileRegistryToCluster(deployContext *deploy.DeployContext, cheHost string) (bool, error) { devfileRegistryURL := deployContext.CheCluster.Spec.Server.DevfileRegistryUrl if !deployContext.CheCluster.Spec.Server.ExternalDevfileRegistry { - additionalLabels := (map[bool]string{true: deployContext.CheCluster.Spec.Server.DevfileRegistryRoute.Labels, false: deployContext.CheCluster.Spec.Server.DevfileRegistryIngress.Labels})[util.IsOpenShift] - endpoint, done, err := expose.Expose(deployContext, cheHost, deploy.DevfileRegistryName, additionalLabels, deploy.DevfileRegistryName) + endpoint, done, err := expose.Expose( + deployContext, + cheHost, + deploy.DevfileRegistryName, + deployContext.CheCluster.Spec.Server.PluginRegistryRoute, + deployContext.CheCluster.Spec.Server.PluginRegistryIngress, + deploy.DevfileRegistryName) if !done { return false, err } diff --git a/pkg/deploy/expose/expose.go b/pkg/deploy/expose/expose.go index 83a92571e1..12738b7d8f 100644 --- a/pkg/deploy/expose/expose.go +++ b/pkg/deploy/expose/expose.go @@ -12,6 +12,7 @@ package expose import ( + orgv1 "github.com/eclipse/che-operator/pkg/apis/org/v1" "github.com/eclipse/che-operator/pkg/deploy" "github.com/eclipse/che-operator/pkg/deploy/gateway" "github.com/eclipse/che-operator/pkg/util" @@ -22,7 +23,8 @@ func Expose( deployContext *deploy.DeployContext, cheHost string, endpointName string, - additionalLabels string, + routeCustomSettings orgv1.RouteCustomSettings, + ingressCustomSettings orgv1.IngressCustomSettings, component string) (endpont string, done bool, err error) { exposureStrategy := util.GetServerExposureStrategy(deployContext.CheCluster, deploy.DefaultServerExposureStrategy) var domain string @@ -71,7 +73,7 @@ func Expose( logrus.Error(err) } } else { - ingress, err := deploy.SyncIngressToCluster(deployContext, endpointName, domain, endpointName, 8080, additionalLabels, component) + ingress, err := deploy.SyncIngressToCluster(deployContext, endpointName, domain, endpointName, 8080, ingressCustomSettings, component) if !util.IsTestMode() { if ingress == nil { logrus.Infof("Waiting on ingress '%s' to be ready", endpointName) @@ -102,7 +104,7 @@ func Expose( } } else { // the empty string for a host is intentional here - we let OpenShift decide on the hostname - route, err := deploy.SyncRouteToCluster(deployContext, endpointName, "", endpointName, 8080, additionalLabels, component) + route, err := deploy.SyncRouteToCluster(deployContext, endpointName, "", endpointName, 8080, routeCustomSettings, component) if route == nil { logrus.Infof("Waiting on route '%s' to be ready", endpointName) if err != nil { diff --git a/pkg/deploy/identity-provider/identity_provider.go b/pkg/deploy/identity-provider/identity_provider.go index bb7d0e49fc..9668547288 100644 --- a/pkg/deploy/identity-provider/identity_provider.go +++ b/pkg/deploy/identity-provider/identity_provider.go @@ -88,15 +88,12 @@ func syncExposure(deployContext *deploy.DeployContext) (bool, error) { protocol := (map[bool]string{ true: "https", false: "http"})[cr.Spec.Server.TlsSupport] - additionalLabels := (map[bool]string{ - true: cr.Spec.Auth.IdentityProviderRoute.Labels, - false: cr.Spec.Auth.IdentityProviderIngress.Labels})[util.IsOpenShift] - endpoint, done, err := expose.Expose( deployContext, cr.Spec.Server.CheHost, deploy.IdentityProviderName, - additionalLabels, + cr.Spec.Auth.IdentityProviderRoute, + cr.Spec.Auth.IdentityProviderIngress, deploy.IdentityProviderName) if !done { return false, err diff --git a/pkg/deploy/ingres_test.go b/pkg/deploy/ingres_test.go new file mode 100644 index 0000000000..eec693353e --- /dev/null +++ b/pkg/deploy/ingres_test.go @@ -0,0 +1,150 @@ +// +// Copyright (c) 2021 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 ( + "os" + "reflect" + + "github.com/google/go-cmp/cmp" + + orgv1 "github.com/eclipse/che-operator/pkg/apis/org/v1" + "k8s.io/api/extensions/v1beta1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/log/zap" + logf "sigs.k8s.io/controller-runtime/pkg/runtime/log" + + "testing" +) + +func TestIngressSpec(t *testing.T) { + _true := true + + type testCase struct { + name string + ingressName string + ingressHost string + ingressComponent string + serviceName string + servicePort int + initObjects []runtime.Object + ingressCustomSettings orgv1.IngressCustomSettings + expectedIngress *v1beta1.Ingress + } + + testCases := []testCase{ + { + name: "Test custom host", + ingressName: "test", + ingressComponent: "test-component", + ingressHost: "test-host", + serviceName: "che", + servicePort: 8080, + ingressCustomSettings: orgv1.IngressCustomSettings{ + Labels: "type=default", + }, + initObjects: []runtime.Object{}, + expectedIngress: &v1beta1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "eclipse-che", + Labels: map[string]string{ + "type": "default", + "app.kubernetes.io/component": "test-component", + "app.kubernetes.io/instance": "che", + "app.kubernetes.io/managed-by": "che-operator", + "app.kubernetes.io/name": "che", + }, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "org.eclipse.che/v1", + Kind: "CheCluster", + Controller: &_true, + BlockOwnerDeletion: &_true, + }, + }, + Annotations: map[string]string{ + "kubernetes.io/ingress.class": "nginx", + "nginx.ingress.kubernetes.io/proxy-connect-timeout": "3600", + "nginx.ingress.kubernetes.io/proxy-read-timeout": "3600", + "nginx.ingress.kubernetes.io/ssl-redirect": "false", + }, + }, + TypeMeta: metav1.TypeMeta{ + Kind: "Ingress", + APIVersion: v1beta1.SchemeGroupVersion.String(), + }, + Spec: v1beta1.IngressSpec{ + Rules: []v1beta1.IngressRule{ + { + Host: "test-host", + IngressRuleValue: v1beta1.IngressRuleValue{ + HTTP: &v1beta1.HTTPIngressRuleValue{ + Paths: []v1beta1.HTTPIngressPath{ + { + Backend: v1beta1.IngressBackend{ + ServiceName: "che", + ServicePort: intstr.FromInt(8080), + }, + Path: "/", + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + logf.SetLogger(zap.LoggerTo(os.Stdout, true)) + orgv1.SchemeBuilder.AddToScheme(scheme.Scheme) + testCase.initObjects = append(testCase.initObjects) + cli := fake.NewFakeClientWithScheme(scheme.Scheme, testCase.initObjects...) + + deployContext := &DeployContext{ + CheCluster: &orgv1.CheCluster{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "eclipse-che", + }, + }, + ClusterAPI: ClusterAPI{ + Client: cli, + Scheme: scheme.Scheme, + }, + } + + actualIngress, err := GetSpecIngress(deployContext, + testCase.ingressName, + testCase.ingressHost, + testCase.serviceName, + testCase.servicePort, + testCase.ingressCustomSettings, + testCase.ingressComponent, + ) + if err != nil { + t.Fatalf("Error creating ingress: %v", err) + } + + if !reflect.DeepEqual(testCase.expectedIngress, actualIngress) { + t.Errorf("Expected ingress and ingress returned from API server differ (-want, +got): %v", cmp.Diff(testCase.expectedIngress, actualIngress)) + } + }) + } +} diff --git a/pkg/deploy/ingress.go b/pkg/deploy/ingress.go index 73fc05fae8..0ee9e40e3a 100644 --- a/pkg/deploy/ingress.go +++ b/pkg/deploy/ingress.go @@ -17,6 +17,7 @@ import ( "reflect" "strconv" + orgv1 "github.com/eclipse/che-operator/pkg/apis/org/v1" "github.com/eclipse/che-operator/pkg/util" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" @@ -43,10 +44,10 @@ func SyncIngressToCluster( host string, serviceName string, servicePort int, - additionalLabels string, + ingressCustomSettings orgv1.IngressCustomSettings, component string) (*v1beta1.Ingress, error) { - specIngress, err := GetSpecIngress(deployContext, name, host, serviceName, servicePort, additionalLabels, component) + specIngress, err := GetSpecIngress(deployContext, name, host, serviceName, servicePort, ingressCustomSettings, component) if err != nil { return nil, err } @@ -120,7 +121,7 @@ func GetSpecIngress( host string, serviceName string, servicePort int, - additionalLabels string, + ingressCustomSettings orgv1.IngressCustomSettings, component string) (*v1beta1.Ingress, error) { tlsSupport := deployContext.CheCluster.Spec.Server.TlsSupport @@ -128,7 +129,7 @@ func GetSpecIngress( ingressDomain := deployContext.CheCluster.Spec.K8s.IngressDomain ingressClass := util.GetValue(deployContext.CheCluster.Spec.K8s.IngressClass, DefaultIngressClass) labels := GetLabels(deployContext.CheCluster, component) - MergeLabels(labels, additionalLabels) + MergeLabels(labels, ingressCustomSettings.Labels) if host == "" { if ingressStrategy == "multi-host" { diff --git a/pkg/deploy/plugin-registry/plugin_registry.go b/pkg/deploy/plugin-registry/plugin_registry.go index c8fd92185a..9590e09107 100644 --- a/pkg/deploy/plugin-registry/plugin_registry.go +++ b/pkg/deploy/plugin-registry/plugin_registry.go @@ -34,8 +34,13 @@ type PluginRegistryConfigMap struct { func SyncPluginRegistryToCluster(deployContext *deploy.DeployContext, cheHost string) (bool, error) { pluginRegistryURL := deployContext.CheCluster.Spec.Server.PluginRegistryUrl if !deployContext.CheCluster.Spec.Server.ExternalPluginRegistry { - additionalLabels := (map[bool]string{true: deployContext.CheCluster.Spec.Server.PluginRegistryRoute.Labels, false: deployContext.CheCluster.Spec.Server.PluginRegistryIngress.Labels})[util.IsOpenShift] - endpoint, done, err := expose.Expose(deployContext, cheHost, deploy.PluginRegistryName, additionalLabels, deploy.PluginRegistryName) + endpoint, done, err := expose.Expose( + deployContext, + cheHost, + deploy.PluginRegistryName, + deployContext.CheCluster.Spec.Server.PluginRegistryRoute, + deployContext.CheCluster.Spec.Server.PluginRegistryIngress, + deploy.PluginRegistryName) if !done { return false, err } diff --git a/pkg/deploy/route.go b/pkg/deploy/route.go index 43bc8882b4..0b53cbe1e2 100644 --- a/pkg/deploy/route.go +++ b/pkg/deploy/route.go @@ -16,6 +16,7 @@ import ( "fmt" "reflect" + orgv1 "github.com/eclipse/che-operator/pkg/apis/org/v1" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" routev1 "github.com/openshift/api/route/v1" @@ -29,6 +30,11 @@ import ( "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" ) +const ( + // host name template: `-.` + HostNameTemplate = "%s-%s.%s" +) + var routeDiffOpts = cmp.Options{ cmpopts.IgnoreFields(routev1.Route{}, "TypeMeta", "Status"), cmpopts.IgnoreFields(routev1.RouteSpec{}, "Host", "WildcardPolicy"), @@ -50,10 +56,10 @@ func SyncRouteToCluster( host string, serviceName string, servicePort int32, - additionalLabels string, + routeCustomSettings orgv1.RouteCustomSettings, component string) (*routev1.Route, error) { - specRoute, err := GetSpecRoute(deployContext, name, host, serviceName, servicePort, additionalLabels, component) + specRoute, err := GetSpecRoute(deployContext, name, host, serviceName, servicePort, routeCustomSettings, component) if err != nil { return nil, err } @@ -132,12 +138,12 @@ func GetSpecRoute( host string, serviceName string, servicePort int32, - additionalLabels string, + routeCustomSettings orgv1.RouteCustomSettings, component string) (*routev1.Route, error) { tlsSupport := deployContext.CheCluster.Spec.Server.TlsSupport labels := GetLabels(deployContext.CheCluster, component) - MergeLabels(labels, additionalLabels) + MergeLabels(labels, routeCustomSettings.Labels) weight := int32(100) @@ -158,7 +164,6 @@ func GetSpecRoute( } route.Spec = routev1.RouteSpec{ - Host: host, To: routev1.RouteTargetReference{ Kind: "Service", Name: serviceName, @@ -169,6 +174,12 @@ func GetSpecRoute( }, } + if host != "" { + route.Spec.Host = host + } else if routeCustomSettings.Domain != "" { + route.Spec.Host = fmt.Sprintf(HostNameTemplate, route.ObjectMeta.Name, route.ObjectMeta.Namespace, routeCustomSettings.Domain) + } + if tlsSupport { route.Spec.TLS = &routev1.TLSConfig{ InsecureEdgeTerminationPolicy: routev1.InsecureEdgeTerminationPolicyRedirect, diff --git a/pkg/deploy/route_test.go b/pkg/deploy/route_test.go new file mode 100644 index 0000000000..dc9baddc1a --- /dev/null +++ b/pkg/deploy/route_test.go @@ -0,0 +1,190 @@ +// +// Copyright (c) 2021 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 ( + "os" + "reflect" + + "github.com/google/go-cmp/cmp" + routev1 "github.com/openshift/api/route/v1" + + orgv1 "github.com/eclipse/che-operator/pkg/apis/org/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/log/zap" + logf "sigs.k8s.io/controller-runtime/pkg/runtime/log" + + "testing" +) + +func TestRouteSpec(t *testing.T) { + weight := int32(100) + _true := true + + type testCase struct { + name string + routeName string + routeHost string + routeComponent string + serviceName string + servicePort int32 + initObjects []runtime.Object + routeCustomSettings orgv1.RouteCustomSettings + expectedRoute *routev1.Route + } + + testCases := []testCase{ + { + name: "Test domain", + routeName: "test", + routeComponent: "test-component", + serviceName: "che", + servicePort: 8080, + routeCustomSettings: orgv1.RouteCustomSettings{ + Labels: "type=default", + Domain: "route-domain", + }, + initObjects: []runtime.Object{}, + expectedRoute: &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "eclipse-che", + Labels: map[string]string{ + "type": "default", + "app.kubernetes.io/component": "test-component", + "app.kubernetes.io/instance": "che", + "app.kubernetes.io/managed-by": "che-operator", + "app.kubernetes.io/name": "che", + }, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "org.eclipse.che/v1", + Kind: "CheCluster", + Controller: &_true, + BlockOwnerDeletion: &_true, + }, + }, + }, + TypeMeta: metav1.TypeMeta{ + Kind: "Route", + APIVersion: routev1.SchemeGroupVersion.String(), + }, + Spec: routev1.RouteSpec{ + Host: "test-eclipse-che.route-domain", + To: routev1.RouteTargetReference{ + Kind: "Service", + Name: "che", + Weight: &weight, + }, + Port: &routev1.RoutePort{ + TargetPort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: int32(8080), + }, + }, + }, + }, + }, + { + name: "Test custom host", + routeName: "test", + routeComponent: "test-component", + routeHost: "test-host", + serviceName: "che", + servicePort: 8080, + routeCustomSettings: orgv1.RouteCustomSettings{ + Labels: "type=default", + }, + initObjects: []runtime.Object{}, + expectedRoute: &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "eclipse-che", + Labels: map[string]string{ + "type": "default", + "app.kubernetes.io/component": "test-component", + "app.kubernetes.io/instance": "che", + "app.kubernetes.io/managed-by": "che-operator", + "app.kubernetes.io/name": "che", + }, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "org.eclipse.che/v1", + Kind: "CheCluster", + Controller: &_true, + BlockOwnerDeletion: &_true, + }, + }, + }, + TypeMeta: metav1.TypeMeta{ + Kind: "Route", + APIVersion: routev1.SchemeGroupVersion.String(), + }, + Spec: routev1.RouteSpec{ + Host: "test-host", + To: routev1.RouteTargetReference{ + Kind: "Service", + Name: "che", + Weight: &weight, + }, + Port: &routev1.RoutePort{ + TargetPort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: int32(8080), + }, + }, + }, + }, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + logf.SetLogger(zap.LoggerTo(os.Stdout, true)) + orgv1.SchemeBuilder.AddToScheme(scheme.Scheme) + testCase.initObjects = append(testCase.initObjects) + cli := fake.NewFakeClientWithScheme(scheme.Scheme, testCase.initObjects...) + + deployContext := &DeployContext{ + CheCluster: &orgv1.CheCluster{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "eclipse-che", + }, + }, + ClusterAPI: ClusterAPI{ + Client: cli, + Scheme: scheme.Scheme, + }, + } + + actualRoute, err := GetSpecRoute(deployContext, + testCase.routeName, + testCase.routeHost, + testCase.serviceName, + testCase.servicePort, + testCase.routeCustomSettings, + testCase.routeComponent, + ) + if err != nil { + t.Fatalf("Error creating route: %v", err) + } + + if !reflect.DeepEqual(testCase.expectedRoute, actualRoute) { + t.Errorf("Expected route and route returned from API server differ (-want, +got): %v", cmp.Diff(testCase.expectedRoute, actualRoute)) + } + }) + } +} diff --git a/pkg/deploy/tls.go b/pkg/deploy/tls.go index c15106fe3f..1d4c750b70 100644 --- a/pkg/deploy/tls.go +++ b/pkg/deploy/tls.go @@ -135,8 +135,14 @@ func GetEndpointTLSCrtChain(deployContext *DeployContext, endpointURL string) ([ if util.IsOpenShift { // Create test route to get certificates chain. // Note, it is not possible to use SyncRouteToCluster here as it may cause infinite reconcile loop. - additionalLabels := deployContext.CheCluster.Spec.Server.CheServerRoute.Labels - routeSpec, err := GetSpecRoute(deployContext, "test", "", "test", 8080, additionalLabels, cheFlavor) + routeSpec, err := GetSpecRoute( + deployContext, + "test", + "", + "test", + 8080, + deployContext.CheCluster.Spec.Server.CheServerRoute, + cheFlavor) if err != nil { return nil, err } @@ -174,8 +180,14 @@ func GetEndpointTLSCrtChain(deployContext *DeployContext, endpointURL string) ([ // Create test ingress to get certificates chain. // Note, it is not possible to use SyncIngressToCluster here as it may cause infinite reconcile loop. - additionalLabels := deployContext.CheCluster.Spec.Server.CheServerIngress.Labels - ingressSpec, err := GetSpecIngress(deployContext, "test", "", "test", 8080, additionalLabels, cheFlavor) + ingressSpec, err := GetSpecIngress( + deployContext, + "test", + "", + "test", + 8080, + deployContext.CheCluster.Spec.Server.CheServerIngress, + cheFlavor) if err != nil { return nil, err } From ece67616063da58bbb044bf63092eb1d8c76acab Mon Sep 17 00:00:00 2001 From: Anatolii Bazko Date: Wed, 10 Feb 2021 12:43:15 +0200 Subject: [PATCH 2/4] Update nightly bundle Signed-off-by: Anatolii Bazko --- deploy/crds/org_v1_che_crd.yaml | 28 ++++++++----------- .../manifests/org_v1_che_crd.yaml | 28 ++++++++----------- .../manifests/org_v1_che_crd.yaml | 28 ++++++++----------- pkg/apis/org/v1/che_types.go | 2 +- 4 files changed, 37 insertions(+), 49 deletions(-) diff --git a/deploy/crds/org_v1_che_crd.yaml b/deploy/crds/org_v1_che_crd.yaml index f7daa9d3b8..a0dc247246 100644 --- a/deploy/crds/org_v1_che_crd.yaml +++ b/deploy/crds/org_v1_che_crd.yaml @@ -155,10 +155,9 @@ spec: properties: domain: description: 'Operator uses the domain to generate a hostname - for a route. Operator uses domain in a conjunction with labels - to create a route, witch is served by a non-default Ingress - controller. The generated host name will follow this pattern: - `-.`.' + for a route. In a conjunction with labels it creates a route, + which is served by a non-default Ingress controller. The generated + host name will follow this pattern: `-.`.' type: string labels: description: Comma separated list of labels that can be used @@ -484,10 +483,9 @@ spec: properties: domain: description: 'Operator uses the domain to generate a hostname - for a route. Operator uses domain in a conjunction with labels - to create a route, witch is served by a non-default Ingress - controller. The generated host name will follow this pattern: - `-.`.' + for a route. In a conjunction with labels it creates a route, + which is served by a non-default Ingress controller. The generated + host name will follow this pattern: `-.`.' type: string labels: description: Comma separated list of labels that can be used @@ -548,10 +546,9 @@ spec: properties: domain: description: 'Operator uses the domain to generate a hostname - for a route. Operator uses domain in a conjunction with labels - to create a route, witch is served by a non-default Ingress - controller. The generated host name will follow this pattern: - `-.`.' + for a route. In a conjunction with labels it creates a route, + which is served by a non-default Ingress controller. The generated + host name will follow this pattern: `-.`.' type: string labels: description: Comma separated list of labels that can be used @@ -635,10 +632,9 @@ spec: properties: domain: description: 'Operator uses the domain to generate a hostname - for a route. Operator uses domain in a conjunction with labels - to create a route, witch is served by a non-default Ingress - controller. The generated host name will follow this pattern: - `-.`.' + for a route. In a conjunction with labels it creates a route, + which is served by a non-default Ingress controller. The generated + host name will follow this pattern: `-.`.' type: string labels: description: Comma separated list of labels that can be used diff --git a/deploy/olm-catalog/eclipse-che-preview-kubernetes/manifests/org_v1_che_crd.yaml b/deploy/olm-catalog/eclipse-che-preview-kubernetes/manifests/org_v1_che_crd.yaml index f7daa9d3b8..a0dc247246 100644 --- a/deploy/olm-catalog/eclipse-che-preview-kubernetes/manifests/org_v1_che_crd.yaml +++ b/deploy/olm-catalog/eclipse-che-preview-kubernetes/manifests/org_v1_che_crd.yaml @@ -155,10 +155,9 @@ spec: properties: domain: description: 'Operator uses the domain to generate a hostname - for a route. Operator uses domain in a conjunction with labels - to create a route, witch is served by a non-default Ingress - controller. The generated host name will follow this pattern: - `-.`.' + for a route. In a conjunction with labels it creates a route, + which is served by a non-default Ingress controller. The generated + host name will follow this pattern: `-.`.' type: string labels: description: Comma separated list of labels that can be used @@ -484,10 +483,9 @@ spec: properties: domain: description: 'Operator uses the domain to generate a hostname - for a route. Operator uses domain in a conjunction with labels - to create a route, witch is served by a non-default Ingress - controller. The generated host name will follow this pattern: - `-.`.' + for a route. In a conjunction with labels it creates a route, + which is served by a non-default Ingress controller. The generated + host name will follow this pattern: `-.`.' type: string labels: description: Comma separated list of labels that can be used @@ -548,10 +546,9 @@ spec: properties: domain: description: 'Operator uses the domain to generate a hostname - for a route. Operator uses domain in a conjunction with labels - to create a route, witch is served by a non-default Ingress - controller. The generated host name will follow this pattern: - `-.`.' + for a route. In a conjunction with labels it creates a route, + which is served by a non-default Ingress controller. The generated + host name will follow this pattern: `-.`.' type: string labels: description: Comma separated list of labels that can be used @@ -635,10 +632,9 @@ spec: properties: domain: description: 'Operator uses the domain to generate a hostname - for a route. Operator uses domain in a conjunction with labels - to create a route, witch is served by a non-default Ingress - controller. The generated host name will follow this pattern: - `-.`.' + for a route. In a conjunction with labels it creates a route, + which is served by a non-default Ingress controller. The generated + host name will follow this pattern: `-.`.' type: string labels: description: Comma separated list of labels that can be used diff --git a/deploy/olm-catalog/eclipse-che-preview-openshift/manifests/org_v1_che_crd.yaml b/deploy/olm-catalog/eclipse-che-preview-openshift/manifests/org_v1_che_crd.yaml index 3f26d9b8e2..6593ae7293 100644 --- a/deploy/olm-catalog/eclipse-che-preview-openshift/manifests/org_v1_che_crd.yaml +++ b/deploy/olm-catalog/eclipse-che-preview-openshift/manifests/org_v1_che_crd.yaml @@ -156,10 +156,9 @@ spec: properties: domain: description: 'Operator uses the domain to generate a hostname - for a route. Operator uses domain in a conjunction with labels - to create a route, witch is served by a non-default Ingress - controller. The generated host name will follow this pattern: - `-.`.' + for a route. In a conjunction with labels it creates a route, + which is served by a non-default Ingress controller. The generated + host name will follow this pattern: `-.`.' type: string labels: description: Comma separated list of labels that can be used @@ -485,10 +484,9 @@ spec: properties: domain: description: 'Operator uses the domain to generate a hostname - for a route. Operator uses domain in a conjunction with labels - to create a route, witch is served by a non-default Ingress - controller. The generated host name will follow this pattern: - `-.`.' + for a route. In a conjunction with labels it creates a route, + which is served by a non-default Ingress controller. The generated + host name will follow this pattern: `-.`.' type: string labels: description: Comma separated list of labels that can be used @@ -549,10 +547,9 @@ spec: properties: domain: description: 'Operator uses the domain to generate a hostname - for a route. Operator uses domain in a conjunction with labels - to create a route, witch is served by a non-default Ingress - controller. The generated host name will follow this pattern: - `-.`.' + for a route. In a conjunction with labels it creates a route, + which is served by a non-default Ingress controller. The generated + host name will follow this pattern: `-.`.' type: string labels: description: Comma separated list of labels that can be used @@ -636,10 +633,9 @@ spec: properties: domain: description: 'Operator uses the domain to generate a hostname - for a route. Operator uses domain in a conjunction with labels - to create a route, witch is served by a non-default Ingress - controller. The generated host name will follow this pattern: - `-.`.' + for a route. In a conjunction with labels it creates a route, + which is served by a non-default Ingress controller. The generated + host name will follow this pattern: `-.`.' type: string labels: description: Comma separated list of labels that can be used diff --git a/pkg/apis/org/v1/che_types.go b/pkg/apis/org/v1/che_types.go index a803db1385..f6f9f65f28 100644 --- a/pkg/apis/org/v1/che_types.go +++ b/pkg/apis/org/v1/che_types.go @@ -438,7 +438,7 @@ type RouteCustomSettings struct { // +optional Labels string `json:"labels,omitempty"` // Operator uses the domain to generate a hostname for a route. - // Operator uses domain in a conjunction with labels to create a route, witch is served by a non-default Ingress controller. + // In a conjunction with labels it creates a route, which is served by a non-default Ingress controller. // The generated host name will follow this pattern: `-.`. // +optional Domain string `json:"domain,omitempty"` From 7165804ecb476fe9b1c9808178aa7590e2a49b69 Mon Sep 17 00:00:00 2001 From: Anatolii Bazko Date: Mon, 15 Feb 2021 12:28:10 +0200 Subject: [PATCH 3/4] Remase on master, fix tests Signed-off-by: Anatolii Bazko --- pkg/controller/che/che_controller.go | 4 +-- pkg/deploy/deployment.go | 2 +- .../devfile-registry/devfile_registry.go | 4 +-- pkg/deploy/secret.go | 13 +++++++-- pkg/deploy/server/che_configmap.go | 28 ++++++++++--------- pkg/deploy/server/che_configmap_test.go | 16 ++++++----- 6 files changed, 39 insertions(+), 28 deletions(-) diff --git a/pkg/controller/che/che_controller.go b/pkg/controller/che/che_controller.go index 50dc483700..d4cf699c30 100644 --- a/pkg/controller/che/che_controller.go +++ b/pkg/controller/che/che_controller.go @@ -394,7 +394,7 @@ func (r *ReconcileChe) Reconcile(request reconcile.Request) (reconcile.Result, e instance.Spec.Auth.OpenShiftoAuth = nil instance.Spec.Auth.InitialOpenShiftOAuthUser = nil updateFields := map[string]string{ - "openShiftoAuth": "nil", + "openShiftoAuth": "nil", "initialOpenShiftOAuthUser": "nil", } @@ -406,7 +406,7 @@ func (r *ReconcileChe) Reconcile(request reconcile.Request) (reconcile.Result, e } if instance.Spec.Auth.InitialOpenShiftOAuthUser == nil && instance.Status.OpenShiftOAuthUserCredentialsSecret != "" { - secret, err := deploy.GetSecret(openShiftOAuthUserCredentialsSecret, instance.Namespace, deployContext.ClusterAPI) + secret, err := deploy.GetSecret(deployContext, instance.Status.OpenShiftOAuthUserCredentialsSecret, instance.Namespace) if secret == nil { if err == nil { instance.Status.OpenShiftOAuthUserCredentialsSecret = "" diff --git a/pkg/deploy/deployment.go b/pkg/deploy/deployment.go index 0fc7c69c1d..705015ea49 100644 --- a/pkg/deploy/deployment.go +++ b/pkg/deploy/deployment.go @@ -160,7 +160,7 @@ func MountSecrets(specDeployment *appsv1.Deployment, deployContext *DeployContex specDeployment.Spec.Template.Spec.Containers[0].VolumeMounts = append(specDeployment.Spec.Template.Spec.Containers[0].VolumeMounts, volumeMount) case "env": - secret, err := GetSecret(secretObj.Name, deployContext.CheCluster.Namespace, deployContext.ClusterAPI) + secret, err := GetSecret(deployContext, secretObj.Name, deployContext.CheCluster.Namespace) if err != nil { return err } else if secret == nil { diff --git a/pkg/deploy/devfile-registry/devfile_registry.go b/pkg/deploy/devfile-registry/devfile_registry.go index 109bc76eaa..cbfe342dfb 100644 --- a/pkg/deploy/devfile-registry/devfile_registry.go +++ b/pkg/deploy/devfile-registry/devfile_registry.go @@ -38,8 +38,8 @@ func SyncDevfileRegistryToCluster(deployContext *deploy.DeployContext, cheHost s deployContext, cheHost, deploy.DevfileRegistryName, - deployContext.CheCluster.Spec.Server.PluginRegistryRoute, - deployContext.CheCluster.Spec.Server.PluginRegistryIngress, + deployContext.CheCluster.Spec.Server.DevfileRegistryRoute, + deployContext.CheCluster.Spec.Server.DevfileRegistryIngress, deploy.DevfileRegistryName) if !done { return false, err diff --git a/pkg/deploy/secret.go b/pkg/deploy/secret.go index 3378252266..535f96f92d 100644 --- a/pkg/deploy/secret.go +++ b/pkg/deploy/secret.go @@ -43,7 +43,7 @@ func SyncSecret( return nil, err } - clusterSecret, err := GetSecret(specSecret.Name, specSecret.Namespace, deployContext.ClusterAPI) + clusterSecret, err := GetSecret(deployContext, specSecret.Name, specSecret.Namespace) if err != nil { return nil, err } @@ -74,13 +74,20 @@ func SyncSecret( } // GetSecret retrieves given secret from cluster -func GetSecret(name string, namespace string, clusterAPI ClusterAPI) (*corev1.Secret, error) { +func GetSecret(deployContext *DeployContext, name string, namespace string) (*corev1.Secret, error) { secret := &corev1.Secret{} namespacedName := types.NamespacedName{ Namespace: namespace, Name: name, } - err := clusterAPI.NonCachedClient.Get(context.TODO(), namespacedName, secret) + + var err error + if namespace == deployContext.CheCluster.ObjectMeta.Namespace { + err = deployContext.ClusterAPI.Client.Get(context.TODO(), namespacedName, secret) + } else { + err = deployContext.ClusterAPI.NonCachedClient.Get(context.TODO(), namespacedName, secret) + } + if err != nil { if errors.IsNotFound(err) { return nil, nil diff --git a/pkg/deploy/server/che_configmap.go b/pkg/deploy/server/che_configmap.go index 7fefc44ef1..e6b5673b58 100644 --- a/pkg/deploy/server/che_configmap.go +++ b/pkg/deploy/server/che_configmap.go @@ -280,21 +280,23 @@ func GetCheConfigMapData(deployContext *deploy.DeployContext) (cheEnv map[string // Add TLS key and server certificate to properties when user workspaces should be created in another // than Che server namespace, from where the Che TLS secret is not accessable if !util.IsWorkspaceInSameNamespaceWithChe(deployContext.CheCluster) { - cheTLSSecret, err := deploy.GetSecret(deployContext.CheCluster.Spec.K8s.TlsSecretName, deployContext.CheCluster.ObjectMeta.Namespace, deployContext.ClusterAPI) - if err != nil { - return nil, err - } - if cheTLSSecret == nil { - // default k8s secret is used - } else { - if _, exists := cheTLSSecret.Data["tls.key"]; !exists { - return nil, fmt.Errorf("%s secret has no 'tls.key' key in data", deployContext.CheCluster.Spec.K8s.TlsSecretName) + if deployContext.CheCluster.Spec.K8s.TlsSecretName != "" { + cheTLSSecret, err := deploy.GetSecret(deployContext, deployContext.CheCluster.Spec.K8s.TlsSecretName, deployContext.CheCluster.ObjectMeta.Namespace) + if err != nil { + return nil, err } - if _, exists := cheTLSSecret.Data["tls.crt"]; !exists { - return nil, fmt.Errorf("%s secret has no 'tls.crt' key in data", deployContext.CheCluster.Spec.K8s.TlsSecretName) + if cheTLSSecret == nil { + return nil, fmt.Errorf("%s secret not found", deployContext.CheCluster.Spec.K8s.TlsSecretName) + } else { + if _, exists := cheTLSSecret.Data["tls.key"]; !exists { + return nil, fmt.Errorf("%s secret has no 'tls.key' key in data", deployContext.CheCluster.Spec.K8s.TlsSecretName) + } + if _, exists := cheTLSSecret.Data["tls.crt"]; !exists { + return nil, fmt.Errorf("%s secret has no 'tls.crt' key in data", deployContext.CheCluster.Spec.K8s.TlsSecretName) + } + k8sCheEnv["CHE_INFRA_KUBERNETES_TLS__KEY"] = string(cheTLSSecret.Data["tls.key"]) + k8sCheEnv["CHE_INFRA_KUBERNETES_TLS__CERT"] = string(cheTLSSecret.Data["tls.crt"]) } - k8sCheEnv["CHE_INFRA_KUBERNETES_TLS__KEY"] = string(cheTLSSecret.Data["tls.key"]) - k8sCheEnv["CHE_INFRA_KUBERNETES_TLS__CERT"] = string(cheTLSSecret.Data["tls.crt"]) } } diff --git a/pkg/deploy/server/che_configmap_test.go b/pkg/deploy/server/che_configmap_test.go index 55011894dc..32a0e58fa0 100644 --- a/pkg/deploy/server/che_configmap_test.go +++ b/pkg/deploy/server/che_configmap_test.go @@ -43,15 +43,15 @@ func TestNewCheConfigMap(t *testing.T) { Proxy: &deploy.Proxy{}, ClusterAPI: deploy.ClusterAPI{}, } + expectedIdentityProvider := "openshift-v4" + util.IsOpenShift = true + util.IsOpenShift4 = true + cheEnv, _ := GetCheConfigMapData(deployContext) testCm, _ := deploy.GetSpecConfigMap(deployContext, CheConfigMapName, cheEnv, CheConfigMapName) + identityProvider := testCm.Data["CHE_INFRA_OPENSHIFT_OAUTH__IDENTITY__PROVIDER"] - _, isOpenshiftv4, _ := util.DetectOpenShift() protocol := strings.Split(testCm.Data["CHE_API"], "://")[0] - expectedIdentityProvider := "openshift-v3" - if isOpenshiftv4 { - expectedIdentityProvider = "openshift-v4" - } if identityProvider != expectedIdentityProvider { t.Errorf("Test failed. Expecting identity provider to be '%s' while got '%s'", expectedIdentityProvider, identityProvider) } @@ -153,12 +153,14 @@ func TestConfigMap(t *testing.T) { orgv1.SchemeBuilder.AddToScheme(scheme.Scheme) testCase.initObjects = append(testCase.initObjects) cli := fake.NewFakeClientWithScheme(scheme.Scheme, testCase.initObjects...) + nonCachedClient := fake.NewFakeClientWithScheme(scheme.Scheme, testCase.initObjects...) deployContext := &deploy.DeployContext{ CheCluster: testCase.cheCluster, ClusterAPI: deploy.ClusterAPI{ - Client: cli, - Scheme: scheme.Scheme, + Client: cli, + NonCachedClient: nonCachedClient, + Scheme: scheme.Scheme, }, Proxy: &deploy.Proxy{}, } From 46d795700f93daa1619a80976a01c65c56a674bf Mon Sep 17 00:00:00 2001 From: Anatolii Bazko Date: Mon, 15 Feb 2021 13:44:08 +0200 Subject: [PATCH 4/4] Fixes Signed-off-by: Anatolii Bazko --- pkg/controller/che/che_controller.go | 2 +- pkg/deploy/ingres_test.go | 7 +++---- pkg/deploy/route_test.go | 10 +++++----- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/pkg/controller/che/che_controller.go b/pkg/controller/che/che_controller.go index d4cf699c30..78aa4a4c69 100644 --- a/pkg/controller/che/che_controller.go +++ b/pkg/controller/che/che_controller.go @@ -406,7 +406,7 @@ func (r *ReconcileChe) Reconcile(request reconcile.Request) (reconcile.Result, e } if instance.Spec.Auth.InitialOpenShiftOAuthUser == nil && instance.Status.OpenShiftOAuthUserCredentialsSecret != "" { - secret, err := deploy.GetSecret(deployContext, instance.Status.OpenShiftOAuthUserCredentialsSecret, instance.Namespace) + secret, err := deploy.GetSecret(deployContext, openShiftOAuthUserCredentialsSecret, instance.Namespace) if secret == nil { if err == nil { instance.Status.OpenShiftOAuthUserCredentialsSecret = "" diff --git a/pkg/deploy/ingres_test.go b/pkg/deploy/ingres_test.go index eec693353e..f8b544ac05 100644 --- a/pkg/deploy/ingres_test.go +++ b/pkg/deploy/ingres_test.go @@ -18,6 +18,7 @@ import ( "github.com/google/go-cmp/cmp" orgv1 "github.com/eclipse/che-operator/pkg/apis/org/v1" + "github.com/eclipse/che-operator/pkg/util" "k8s.io/api/extensions/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -31,8 +32,6 @@ import ( ) func TestIngressSpec(t *testing.T) { - _true := true - type testCase struct { name string ingressName string @@ -72,8 +71,8 @@ func TestIngressSpec(t *testing.T) { { APIVersion: "org.eclipse.che/v1", Kind: "CheCluster", - Controller: &_true, - BlockOwnerDeletion: &_true, + Controller: util.NewBoolPointer(true), + BlockOwnerDeletion: util.NewBoolPointer(true), }, }, Annotations: map[string]string{ diff --git a/pkg/deploy/route_test.go b/pkg/deploy/route_test.go index dc9baddc1a..93ed2d97d4 100644 --- a/pkg/deploy/route_test.go +++ b/pkg/deploy/route_test.go @@ -19,6 +19,7 @@ import ( routev1 "github.com/openshift/api/route/v1" orgv1 "github.com/eclipse/che-operator/pkg/apis/org/v1" + "github.com/eclipse/che-operator/pkg/util" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/intstr" @@ -32,7 +33,6 @@ import ( func TestRouteSpec(t *testing.T) { weight := int32(100) - _true := true type testCase struct { name string @@ -73,8 +73,8 @@ func TestRouteSpec(t *testing.T) { { APIVersion: "org.eclipse.che/v1", Kind: "CheCluster", - Controller: &_true, - BlockOwnerDeletion: &_true, + Controller: util.NewBoolPointer(true), + BlockOwnerDeletion: util.NewBoolPointer(true), }, }, }, @@ -124,8 +124,8 @@ func TestRouteSpec(t *testing.T) { { APIVersion: "org.eclipse.che/v1", Kind: "CheCluster", - Controller: &_true, - BlockOwnerDeletion: &_true, + Controller: util.NewBoolPointer(true), + BlockOwnerDeletion: util.NewBoolPointer(true), }, }, },