From a3ca1f122f15a5b53aaebe52f02b4c5be26c6684 Mon Sep 17 00:00:00 2001 From: Sergii Leshchenko Date: Tue, 6 Apr 2021 12:51:36 +0300 Subject: [PATCH] Refactor exposure to have an ability share host among components --- pkg/controller/che/che_controller.go | 9 +- .../devfile-registry/devfile_registry.go | 6 +- pkg/deploy/expose/expose.go | 123 ++++++++++-------- pkg/deploy/gateway/gateway.go | 8 +- .../identity-provider/identity_provider.go | 4 +- pkg/deploy/ingres_test.go | 9 +- pkg/deploy/ingress.go | 39 +++--- pkg/deploy/plugin-registry/plugin_registry.go | 6 +- pkg/deploy/route.go | 11 +- pkg/deploy/route_test.go | 2 + pkg/deploy/tls.go | 4 +- 11 files changed, 123 insertions(+), 98 deletions(-) diff --git a/pkg/controller/che/che_controller.go b/pkg/controller/che/che_controller.go index ea3d10ca97..b3168cc1e6 100644 --- a/pkg/controller/che/che_controller.go +++ b/pkg/controller/che/che_controller.go @@ -845,10 +845,11 @@ func (r *ReconcileChe) Reconcile(request reconcile.Request) (reconcile.Result, e exposedServiceName := getServerExposingServiceName(instance) cheHost := "" if !isOpenShift { - done, err := deploy.SyncIngressToCluster( + _, done, err := deploy.SyncIngressToCluster( deployContext, cheFlavor, instance.Spec.Server.CheHost, + "", exposedServiceName, 8080, deployContext.CheCluster.Spec.Server.CheServerIngress, @@ -882,6 +883,7 @@ func (r *ReconcileChe) Reconcile(request reconcile.Request) (reconcile.Result, e deployContext, cheFlavor, customHost, + "", exposedServiceName, 8080, deployContext.CheCluster.Spec.Server.CheServerRoute, @@ -918,7 +920,7 @@ func (r *ReconcileChe) Reconcile(request reconcile.Request) (reconcile.Result, e } } - provisioned, err = devfile_registry.SyncDevfileRegistryToCluster(deployContext, cheHost) + provisioned, err = devfile_registry.SyncDevfileRegistryToCluster(deployContext) if !tests { if !provisioned { if err != nil { @@ -928,7 +930,7 @@ func (r *ReconcileChe) Reconcile(request reconcile.Request) (reconcile.Result, e } } - provisioned, err = plugin_registry.SyncPluginRegistryToCluster(deployContext, cheHost) + provisioned, err = plugin_registry.SyncPluginRegistryToCluster(deployContext) if !tests { if !provisioned { if err != nil { @@ -1059,6 +1061,7 @@ func getDefaultCheHost(deployContext *deploy.DeployContext) (string, error) { deployContext, cheFlavor, "", + "", getServerExposingServiceName(deployContext.CheCluster), 8080, deployContext.CheCluster.Spec.Server.CheServerRoute, diff --git a/pkg/deploy/devfile-registry/devfile_registry.go b/pkg/deploy/devfile-registry/devfile_registry.go index f5706f7890..e8e457ae22 100644 --- a/pkg/deploy/devfile-registry/devfile_registry.go +++ b/pkg/deploy/devfile-registry/devfile_registry.go @@ -31,16 +31,14 @@ type DevFileRegistryConfigMap struct { /** * Create devfile registry resources unless an external registry is used. */ -func SyncDevfileRegistryToCluster(deployContext *deploy.DeployContext, cheHost string) (bool, error) { +func SyncDevfileRegistryToCluster(deployContext *deploy.DeployContext) (bool, error) { devfileRegistryURL := deployContext.CheCluster.Spec.Server.DevfileRegistryUrl if !deployContext.CheCluster.Spec.Server.ExternalDevfileRegistry { endpoint, done, err := expose.Expose( deployContext, - cheHost, deploy.DevfileRegistryName, deployContext.CheCluster.Spec.Server.DevfileRegistryRoute, - deployContext.CheCluster.Spec.Server.DevfileRegistryIngress, - deploy.DevfileRegistryName) + deployContext.CheCluster.Spec.Server.DevfileRegistryIngress) if !done { return false, err } diff --git a/pkg/deploy/expose/expose.go b/pkg/deploy/expose/expose.go index 06c2a8d2eb..c40030bb0e 100644 --- a/pkg/deploy/expose/expose.go +++ b/pkg/deploy/expose/expose.go @@ -12,6 +12,8 @@ package expose import ( + "strings" + orgv1 "github.com/eclipse-che/che-operator/pkg/apis/org/v1" "github.com/eclipse-che/che-operator/pkg/deploy" "github.com/eclipse-che/che-operator/pkg/deploy/gateway" @@ -20,63 +22,47 @@ import ( extentionsv1beta1 "k8s.io/api/extensions/v1beta1" ) +//Expose exposes the specified component according to the configured exposure strategy rules func Expose( deployContext *deploy.DeployContext, - cheHost string, - endpointName string, + componentName string, + routeCustomSettings orgv1.RouteCustomSettings, + ingressCustomSettings orgv1.IngressCustomSettings) (endpointUrl string, done bool, err error) { + //the host and path are empty and will be evaluated for the specified component + return ExposeWithHostPath(deployContext, componentName, "", "", routeCustomSettings, ingressCustomSettings) +} + +//Expose exposes the specified component on the specified host and domain. +//Empty host or path will be evaluated according to the configured strategy rules. +//Note: path may be prefixed according to the configured strategy rules. +func ExposeWithHostPath( + deployContext *deploy.DeployContext, + component string, + host string, + path string, routeCustomSettings orgv1.RouteCustomSettings, - ingressCustomSettings orgv1.IngressCustomSettings, - component string) (endpont string, done bool, err error) { + ingressCustomSettings orgv1.IngressCustomSettings) (endpointUrl string, done bool, err error) { + exposureStrategy := util.GetServerExposureStrategy(deployContext.CheCluster, deploy.DefaultServerExposureStrategy) - var domain string - var endpoint string - var pathPrefix string - var stripPrefix bool - if endpointName == deploy.IdentityProviderName { - pathPrefix = "auth" - stripPrefix = false - } else { - pathPrefix = endpointName - stripPrefix = true - } - if exposureStrategy == "multi-host" { - // this won't get used on openshift, because there we're intentionally let Openshift decide on the domain name - domain = endpointName + "-" + deployContext.CheCluster.Namespace + "." + deployContext.CheCluster.Spec.K8s.IngressDomain - endpoint = domain - } else { - domain = cheHost - if endpointName == deploy.IdentityProviderName { - // legacy - endpoint = domain - } else { - endpoint = domain + "/" + pathPrefix - } + if path != "" && !strings.HasPrefix(path, "/") { + path = "/" + path } - gatewayConfig := "che-gateway-route-" + endpointName singleHostExposureType := deploy.GetSingleHostExposureType(deployContext.CheCluster) useGateway := exposureStrategy == "single-host" && (util.IsOpenShift || singleHostExposureType == "gateway") - + gatewayConfig := "che-gateway-route-" + component if !util.IsOpenShift { if useGateway { - cfg := gateway.GetGatewayRouteConfig(deployContext, gatewayConfig, "/"+pathPrefix, 10, "http://"+endpointName+":8080", stripPrefix) - done, err := deploy.SyncConfigMapSpecToCluster(deployContext, &cfg) - if !util.IsTestMode() { - if !done { - if err != nil { - logrus.Error(err) - } - return "", false, err + return exposeWithGateway(deployContext, gatewayConfig, component, path, func() { + if _, err = deploy.DeleteNamespacedObject(deployContext, component, &extentionsv1beta1.Ingress{}); err != nil { + logrus.Error(err) } - } - if _, err = deploy.DeleteNamespacedObject(deployContext, endpointName, &extentionsv1beta1.Ingress{}); err != nil { - logrus.Error(err) - } + }) } else { - done, err := deploy.SyncIngressToCluster(deployContext, endpointName, domain, endpointName, 8080, ingressCustomSettings, component) + endpointUrl, done, err = deploy.SyncIngressToCluster(deployContext, component, host, path, component, 8080, ingressCustomSettings, component) if !done { - logrus.Infof("Waiting on ingress '%s' to be ready", endpointName) + logrus.Infof("Waiting on ingress '%s' to be ready", component) if err != nil { logrus.Error(err) } @@ -85,25 +71,21 @@ func Expose( if err := gateway.DeleteGatewayRouteConfig(gatewayConfig, deployContext); !util.IsTestMode() && err != nil { logrus.Error(err) } + + return endpointUrl, true, nil } } else { if useGateway { - cfg := gateway.GetGatewayRouteConfig(deployContext, gatewayConfig, "/"+pathPrefix, 10, "http://"+endpointName+":8080", stripPrefix) - done, err := deploy.SyncConfigMapSpecToCluster(deployContext, &cfg) - if !done { - if err != nil { + return exposeWithGateway(deployContext, gatewayConfig, component, path, func() { + if err := deploy.DeleteRouteIfExists(component, deployContext); !util.IsTestMode() && err != nil { logrus.Error(err) } - return "", false, err - } - if err := deploy.DeleteRouteIfExists(endpointName, deployContext); !util.IsTestMode() && err != nil { - logrus.Error(err) - } + }) } 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, routeCustomSettings, component) + route, err := deploy.SyncRouteToCluster(deployContext, component, host, path, component, 8080, routeCustomSettings, component) if route == nil { - logrus.Infof("Waiting on route '%s' to be ready", endpointName) + logrus.Infof("Waiting on route '%s' to be ready", component) if err != nil { logrus.Error(err) } @@ -114,8 +96,37 @@ func Expose( logrus.Error(err) } - endpoint = route.Spec.Host + return route.Spec.Host + route.Spec.Path, true, nil + } + } +} + +func exposeWithGateway(deployContext *deploy.DeployContext, + gatewayConfig string, + component string, + path string, + cleanUpRouting func()) (endpointUrl string, done bool, err error) { + + var pathPrefix string + var stripPrefix bool + if component == deploy.IdentityProviderName { + pathPrefix = "auth" + path + stripPrefix = false + } else { + pathPrefix = component + path + stripPrefix = true + } + + cfg := gateway.GetGatewayRouteConfig(deployContext, component, gatewayConfig, "/"+pathPrefix, 10, "http://"+component+":8080", stripPrefix) + done, err = deploy.SyncConfigMapSpecToCluster(deployContext, &cfg) + if !done { + if err != nil { + logrus.Error(err) } + return "", false, err } - return endpoint, true, nil + + cleanUpRouting() + + return "", true, err } diff --git a/pkg/deploy/gateway/gateway.go b/pkg/deploy/gateway/gateway.go index c53a662257..5f93041410 100644 --- a/pkg/deploy/gateway/gateway.go +++ b/pkg/deploy/gateway/gateway.go @@ -186,7 +186,7 @@ func delete(clusterAPI deploy.ClusterAPI, obj metav1.Object) error { // GetGatewayRouteConfig creates a config map with traefik configuration for a single new route. // `serviceName` is an arbitrary name identifying the configuration. This should be unique within operator. Che server only creates // new configuration for workspaces, so the name should not resemble any of the names created by the Che server. -func GetGatewayRouteConfig(deployContext *deploy.DeployContext, serviceName string, pathPrefix string, priority int, internalUrl string, stripPrefix bool) corev1.ConfigMap { +func GetGatewayRouteConfig(deployContext *deploy.DeployContext, component string, serviceName string, pathPrefix string, priority int, internalUrl string, stripPrefix bool) corev1.ConfigMap { pathRewrite := pathPrefix != "/" && stripPrefix data := `--- @@ -225,14 +225,14 @@ http: Kind: "ConfigMap", }, ObjectMeta: metav1.ObjectMeta{ - Name: serviceName, + Name: component, Namespace: deployContext.CheCluster.Namespace, Labels: util.MergeMaps( deploy.GetLabels(deployContext.CheCluster, gatewayConfigComponentName), util.GetMapValue(deployContext.CheCluster.Spec.Server.SingleHostGatewayConfigMapLabels, deploy.DefaultSingleHostGatewayConfigMapLabels)), }, Data: map[string]string{ - serviceName + ".yml": data, + component + ".yml": data, }, } @@ -255,7 +255,7 @@ func DeleteGatewayRouteConfig(serviceName string, deployContext *deploy.DeployCo // below functions declare the desired states of the various objects required for the gateway func getGatewayServerConfigSpec(deployContext *deploy.DeployContext) corev1.ConfigMap { - return GetGatewayRouteConfig(deployContext, gatewayServerConfigName, "/", 1, "http://"+deploy.CheServiceName+":8080", false) + return GetGatewayRouteConfig(deployContext, gatewayServerConfigName, gatewayServerConfigName, "/", 1, "http://"+deploy.CheServiceName+":8080", false) } func getGatewayServiceAccountSpec(instance *orgv1.CheCluster) corev1.ServiceAccount { diff --git a/pkg/deploy/identity-provider/identity_provider.go b/pkg/deploy/identity-provider/identity_provider.go index 4576a09be7..07ea585cc3 100644 --- a/pkg/deploy/identity-provider/identity_provider.go +++ b/pkg/deploy/identity-provider/identity_provider.go @@ -82,11 +82,9 @@ func syncExposure(deployContext *deploy.DeployContext) (bool, error) { false: "http"})[cr.Spec.Server.TlsSupport] endpoint, done, err := expose.Expose( deployContext, - cr.Spec.Server.CheHost, deploy.IdentityProviderName, cr.Spec.Auth.IdentityProviderRoute, - cr.Spec.Auth.IdentityProviderIngress, - deploy.IdentityProviderName) + cr.Spec.Auth.IdentityProviderIngress) if !done { return false, err } diff --git a/pkg/deploy/ingres_test.go b/pkg/deploy/ingres_test.go index f4021caf78..99d5713652 100644 --- a/pkg/deploy/ingres_test.go +++ b/pkg/deploy/ingres_test.go @@ -37,6 +37,7 @@ func TestIngressSpec(t *testing.T) { name string ingressName string ingressHost string + ingressPath string ingressComponent string serviceName string servicePort int @@ -57,6 +58,7 @@ func TestIngressSpec(t *testing.T) { ingressName: "test", ingressComponent: "test-component", ingressHost: "test-host", + ingressPath: "", serviceName: "che", servicePort: 8080, ingressCustomSettings: orgv1.IngressCustomSettings{ @@ -124,9 +126,10 @@ func TestIngressSpec(t *testing.T) { }, } - actualIngress := GetIngressSpec(deployContext, + actualIngress, _ := GetIngressSpec(deployContext, testCase.ingressName, testCase.ingressHost, + testCase.ingressPath, testCase.serviceName, testCase.servicePort, testCase.ingressCustomSettings, @@ -157,12 +160,12 @@ func TestSyncIngressToCluster(t *testing.T) { }, } - done, err := SyncIngressToCluster(deployContext, "test", "host-1", "service-1", 8080, orgv1.IngressCustomSettings{}, "component") + _, done, err := SyncIngressToCluster(deployContext, "test", "host-1", "", "service-1", 8080, orgv1.IngressCustomSettings{}, "component") if !done || err != nil { t.Fatalf("Failed to sync ingress: %v", err) } - done, err = SyncIngressToCluster(deployContext, "test", "host-2", "service-2", 8080, orgv1.IngressCustomSettings{}, "component") + _, done, err = SyncIngressToCluster(deployContext, "test", "host-2", "", "service-2", 8080, orgv1.IngressCustomSettings{}, "component") if !done || err != nil { t.Fatalf("Failed to sync ingress: %v", err) } diff --git a/pkg/deploy/ingress.go b/pkg/deploy/ingress.go index 419e50cf76..8a3d3014d9 100644 --- a/pkg/deploy/ingress.go +++ b/pkg/deploy/ingress.go @@ -35,13 +35,15 @@ func SyncIngressToCluster( deployContext *DeployContext, name string, host string, + path string, serviceName string, servicePort int, ingressCustomSettings orgv1.IngressCustomSettings, - component string) (bool, error) { + component string) (endpointUrl string, done bool, err error) { - ingressSpec := GetIngressSpec(deployContext, name, host, serviceName, servicePort, ingressCustomSettings, component) - return Sync(deployContext, ingressSpec, ingressDiffOpts) + ingressSpec, ingressUrl := GetIngressSpec(deployContext, name, host, path, serviceName, servicePort, ingressCustomSettings, component) + sync, err := Sync(deployContext, ingressSpec, ingressDiffOpts) + return ingressUrl, sync, err } // GetIngressSpec returns expected ingress config for given parameters @@ -49,10 +51,11 @@ func GetIngressSpec( deployContext *DeployContext, name string, host string, + path string, serviceName string, servicePort int, ingressCustomSettings orgv1.IngressCustomSettings, - component string) *v1beta1.Ingress { + component string) (i *v1beta1.Ingress, ingressUrl string) { tlsSupport := deployContext.CheCluster.Spec.Server.TlsSupport ingressStrategy := util.GetServerExposureStrategy(deployContext.CheCluster, DefaultServerExposureStrategy) @@ -63,7 +66,7 @@ func GetIngressSpec( if host == "" { if ingressStrategy == "multi-host" { - host = name + "-" + deployContext.CheCluster.Namespace + "." + ingressDomain + host = component + "-" + deployContext.CheCluster.Namespace + "." + ingressDomain } else if ingressStrategy == "single-host" { host = ingressDomain } @@ -71,20 +74,22 @@ func GetIngressSpec( tlsSecretName := util.GetValue(deployContext.CheCluster.Spec.K8s.TlsSecretName, "") if tlsSupport { - if name == DefaultCheFlavor(deployContext.CheCluster) && deployContext.CheCluster.Spec.Server.CheHostTLSSecret != "" { + if component == DefaultCheFlavor(deployContext.CheCluster) && deployContext.CheCluster.Spec.Server.CheHostTLSSecret != "" { tlsSecretName = deployContext.CheCluster.Spec.Server.CheHostTLSSecret } } - path := "/" - if ingressStrategy != "multi-host" { - switch name { - case IdentityProviderName: - path = "/auth" - case DevfileRegistryName: - path = "/" + DevfileRegistryName + "/(.*)" - case PluginRegistryName: - path = "/" + PluginRegistryName + "/(.*)" + if path == "" { + path = "/" + if ingressStrategy != "multi-host" { + switch component { + case IdentityProviderName: + path = "/auth" + case DevfileRegistryName: + path = "/" + DevfileRegistryName + "/(.*)" + case PluginRegistryName: + path = "/" + PluginRegistryName + "/(.*)" + } } } @@ -94,7 +99,7 @@ func GetIngressSpec( "nginx.ingress.kubernetes.io/proxy-connect-timeout": "3600", "nginx.ingress.kubernetes.io/ssl-redirect": strconv.FormatBool(tlsSupport), } - if ingressStrategy != "multi-host" && (name == DevfileRegistryName || name == PluginRegistryName) { + if ingressStrategy != "multi-host" && (component == DevfileRegistryName || component == PluginRegistryName) { annotations["nginx.ingress.kubernetes.io/rewrite-target"] = "/$1" } @@ -142,5 +147,5 @@ func GetIngressSpec( } } - return ingress + return ingress, host + path } diff --git a/pkg/deploy/plugin-registry/plugin_registry.go b/pkg/deploy/plugin-registry/plugin_registry.go index 9339629a44..c2765306d6 100644 --- a/pkg/deploy/plugin-registry/plugin_registry.go +++ b/pkg/deploy/plugin-registry/plugin_registry.go @@ -31,16 +31,14 @@ type PluginRegistryConfigMap struct { /** * Create plugin registry resources unless an external registry is used. */ -func SyncPluginRegistryToCluster(deployContext *deploy.DeployContext, cheHost string) (bool, error) { +func SyncPluginRegistryToCluster(deployContext *deploy.DeployContext) (bool, error) { pluginRegistryURL := deployContext.CheCluster.Spec.Server.PluginRegistryUrl if !deployContext.CheCluster.Spec.Server.ExternalPluginRegistry { endpoint, done, err := expose.Expose( deployContext, - cheHost, deploy.PluginRegistryName, deployContext.CheCluster.Spec.Server.PluginRegistryRoute, - deployContext.CheCluster.Spec.Server.PluginRegistryIngress, - deploy.PluginRegistryName) + deployContext.CheCluster.Spec.Server.PluginRegistryIngress) if !done { return false, err } diff --git a/pkg/deploy/route.go b/pkg/deploy/route.go index 1533abcd5c..64e1cb6511 100644 --- a/pkg/deploy/route.go +++ b/pkg/deploy/route.go @@ -54,12 +54,14 @@ func SyncRouteToCluster( deployContext *DeployContext, name string, host string, + path string, serviceName string, servicePort int32, routeCustomSettings orgv1.RouteCustomSettings, - component string) (*routev1.Route, error) { + component string) (r *routev1.Route, err error) { + + specRoute, err := GetSpecRoute(deployContext, name, host, path, serviceName, servicePort, routeCustomSettings, component) - specRoute, err := GetSpecRoute(deployContext, name, host, serviceName, servicePort, routeCustomSettings, component) if err != nil { return nil, err } @@ -136,6 +138,7 @@ func GetSpecRoute( deployContext *DeployContext, name string, host string, + path string, serviceName string, servicePort int32, routeCustomSettings orgv1.RouteCustomSettings, @@ -164,6 +167,8 @@ func GetSpecRoute( } route.Spec = routev1.RouteSpec{ + Host: host, + Path: path, To: routev1.RouteTargetReference{ Kind: "Service", Name: serviceName, @@ -186,7 +191,7 @@ func GetSpecRoute( Termination: routev1.TLSTerminationEdge, } - if name == DefaultCheFlavor(deployContext.CheCluster) && deployContext.CheCluster.Spec.Server.CheHostTLSSecret != "" { + if component == DefaultCheFlavor(deployContext.CheCluster) && deployContext.CheCluster.Spec.Server.CheHostTLSSecret != "" { secret := &corev1.Secret{} namespacedName := types.NamespacedName{ Namespace: deployContext.CheCluster.Namespace, diff --git a/pkg/deploy/route_test.go b/pkg/deploy/route_test.go index 3c89987649..7a9d9c5efb 100644 --- a/pkg/deploy/route_test.go +++ b/pkg/deploy/route_test.go @@ -38,6 +38,7 @@ func TestRouteSpec(t *testing.T) { name string routeName string routeHost string + routePath string routeComponent string serviceName string servicePort int32 @@ -175,6 +176,7 @@ func TestRouteSpec(t *testing.T) { actualRoute, err := GetSpecRoute(deployContext, testCase.routeName, testCase.routeHost, + testCase.routePath, testCase.serviceName, testCase.servicePort, testCase.routeCustomSettings, diff --git a/pkg/deploy/tls.go b/pkg/deploy/tls.go index 6405cbf963..1584a76c55 100644 --- a/pkg/deploy/tls.go +++ b/pkg/deploy/tls.go @@ -139,6 +139,7 @@ func GetEndpointTLSCrtChain(deployContext *DeployContext, endpointURL string) ([ deployContext, "test", "", + "", "test", 8080, deployContext.CheCluster.Spec.Server.CheServerRoute, @@ -180,10 +181,11 @@ 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. - ingressSpec := GetIngressSpec( + ingressSpec, _ := GetIngressSpec( deployContext, "test", "", + "", "test", 8080, deployContext.CheCluster.Spec.Server.CheServerIngress,