From d104845f8987d6b77bf4b2bb1563bb51540e78e8 Mon Sep 17 00:00:00 2001 From: Michal Vala Date: Mon, 11 Oct 2021 19:22:36 +0200 Subject: [PATCH] feat: accessible workspace healthz endpoint (#1119) Signed-off-by: Michal Vala --- .../devworkspace/solver/che_routing.go | 195 +++++++----------- .../devworkspace/solver/che_routing_test.go | 183 ++++++++++------ pkg/deploy/dashboard/dashboard.go | 3 +- pkg/deploy/devfileregistry/devfileregistry.go | 4 +- pkg/deploy/gateway/gateway.go | 6 +- pkg/deploy/gateway/gateway_test.go | 12 +- pkg/deploy/gateway/traefik_config_util.go | 70 +++++-- .../gateway/traefik_config_util_test.go | 66 ++++-- .../identity-provider/identity_provider.go | 3 +- pkg/deploy/pluginregistry/pluginregistry.go | 4 +- 10 files changed, 324 insertions(+), 222 deletions(-) diff --git a/controllers/devworkspace/solver/che_routing.go b/controllers/devworkspace/solver/che_routing.go index b9fd4ca85..42621e4ef 100644 --- a/controllers/devworkspace/solver/che_routing.go +++ b/controllers/devworkspace/solver/che_routing.go @@ -297,8 +297,10 @@ func (c *CheRoutingSolver) getGatewayConfigsAndFillRoutingObjects(cheCluster *v2 configs := make([]corev1.ConfigMap, 0) // first do routing from main che-gateway into workspace service - if masterConfig := provisionMasterRouting(cheCluster, routing, cmLabels); masterConfig != nil { - configs = append(configs, *masterConfig) + if mainWsRouteConfig, err := provisionMainWorkspaceRoute(cheCluster, routing, cmLabels); err != nil { + return nil, err + } else { + configs = append(configs, *mainWsRouteConfig) } // then expose the endpoints @@ -346,13 +348,7 @@ func getCommonService(objs *solvers.RoutingObjects, dwId string) *corev1.Service } func exposeAllEndpoints(cheCluster *v2alpha1.CheCluster, routing *dwo.DevWorkspaceRouting, objs *solvers.RoutingObjects, ingressExpose func(*EndpointInfo)) *corev1.ConfigMap { - wsRouteConfig := gateway.TraefikConfig{ - HTTP: gateway.TraefikConfigHTTP{ - Routers: map[string]*gateway.TraefikConfigRouter{}, - Services: map[string]*gateway.TraefikConfigService{}, - Middlewares: map[string]*gateway.TraefikConfigMiddleware{}, - }, - } + wsRouteConfig := gateway.CreateEmptyTraefikConfig() commonService := getCommonService(objs, routing.Spec.DevWorkspaceId) if commonService == nil { @@ -367,7 +363,7 @@ func exposeAllEndpoints(cheCluster *v2alpha1.CheCluster, routing *dwo.DevWorkspa } if e.Attributes.GetString(urlRewriteSupportedEndpointAttributeName, nil) == "true" { - addEndpointToTraefikConfig(componentName, e, &wsRouteConfig, cheCluster, routing) + addEndpointToTraefikConfig(componentName, e, wsRouteConfig, cheCluster, routing) } else { if !containPort(commonService, int32(e.TargetPort)) { commonService.Spec.Ports = append(commonService.Spec.Ports, corev1.ServicePort{ @@ -436,71 +432,33 @@ func containPort(service *corev1.Service, port int32) bool { return false } -func provisionMasterRouting(cheCluster *v2alpha1.CheCluster, routing *dwo.DevWorkspaceRouting, cmLabels map[string]string) *corev1.ConfigMap { - cfg := &gateway.TraefikConfig{ - HTTP: gateway.TraefikConfigHTTP{ - Routers: map[string]*gateway.TraefikConfigRouter{}, - Services: map[string]*gateway.TraefikConfigService{}, - Middlewares: map[string]*gateway.TraefikConfigMiddleware{}, - }, - } - - rtrs := cfg.HTTP.Routers - srvcs := cfg.HTTP.Services - mdls := cfg.HTTP.Middlewares - +func provisionMainWorkspaceRoute(cheCluster *v2alpha1.CheCluster, routing *dwo.DevWorkspaceRouting, cmLabels map[string]string) (*corev1.ConfigMap, error) { dwId := routing.Spec.DevWorkspaceId dwNamespace := routing.Namespace - rtrs[dwId] = &gateway.TraefikConfigRouter{ - Rule: fmt.Sprintf("PathPrefix(`/%s`)", dwId), - Service: dwId, - Middlewares: calculateMiddlewares(dwId, true), - Priority: 100, - } - - srvcs[dwId] = &gateway.TraefikConfigService{ - LoadBalancer: gateway.TraefikConfigLoadbalancer{ - Servers: []gateway.TraefikConfigLoadbalancerServer{ - { - URL: getServiceURL(wsGatewayPort, dwId, dwNamespace), - }, - }, - }, - } - - mdls[dwId+"-prefix"] = &gateway.TraefikConfigMiddleware{ - StripPrefix: &gateway.TraefikConfigStripPrefix{ - Prefixes: []string{"/" + dwId}, - }, - } + cfg := gateway.CreateCommonTraefikConfig( + dwId, + fmt.Sprintf("PathPrefix(`/%s`)", dwId), + 100, + getServiceURL(wsGatewayPort, dwId, dwNamespace), + []string{"/" + dwId}) if util.IsOpenShift4 { - mdls[dwId+"-auth"] = &gateway.TraefikConfigMiddleware{ - ForwardAuth: &gateway.TraefikConfigForwardAuth{ - Address: "http://127.0.0.1:8089?namespace=" + dwNamespace, - }, - } + // on OpenShift, we need to set authorization header. + // This MUST come before Auth, because Auth needs Authorization header to be properly set. + cfg.AddAuthHeaderRewrite(dwId) - mdls[dwId+"-header"] = &gateway.TraefikConfigMiddleware{ - Plugin: &gateway.TraefikPlugin{ - HeaderRewrite: &gateway.TraefikPluginHeaderRewrite{ - From: "X-Forwarded-Access-Token", - To: "Authorization", - Prefix: "Bearer ", - }, - }, - } + // authorize against kube-rbac-proxy in che-gateway. This will be needed for k8s native auth as well. + cfg.AddAuth(dwId, "http://127.0.0.1:8089?namespace="+dwNamespace) + + // make '/healthz' path of main endpoints reachable from outside + routeForHealthzEndpoint(cfg, dwId, routing.Spec.Endpoints) } - if len(cfg.HTTP.Routers) > 0 { - contents, err := yaml.Marshal(cfg) - if err != nil { - logger.Error(err, "can't serialize traefik config") - return nil - } - - configMap := &corev1.ConfigMap{ + if contents, err := yaml.Marshal(cfg); err != nil { + return nil, err + } else { + return &corev1.ConfigMap{ ObjectMeta: v1.ObjectMeta{ Name: defaults.GetGatewayWorkspaceConfigMapName(dwId), Namespace: cheCluster.Namespace, @@ -510,24 +468,34 @@ func provisionMasterRouting(cheCluster *v2alpha1.CheCluster, routing *dwo.DevWor defaults.ConfigAnnotationDevWorkspaceRoutingNamespace: routing.Namespace, }, }, - Data: map[string]string{}, - } - configMap.Data[dwId+".yml"] = string(contents) - return configMap + Data: map[string]string{dwId + ".yml": string(contents)}, + }, nil + } +} + +// makes '/healthz' path of main endpoints reachable from the outside +func routeForHealthzEndpoint(cfg *gateway.TraefikConfig, dwId string, endpoints map[string]dwo.EndpointList) { + for componentName, endpoints := range endpoints { + for _, e := range endpoints { + if e.Attributes.GetString(string(dwo.TypeEndpointAttribute), nil) == string(dwo.MainEndpointType) { + middlewares := []string{dwId + gateway.StripPrefixMiddlewareSuffix} + if util.IsOpenShift4 { + middlewares = append(middlewares, dwId+gateway.HeaderRewriteMiddlewareSuffix) + } + routeName, endpointPath := createEndpointPath(&e, componentName) + cfg.HTTP.Routers[routeName+"-healthz"] = &gateway.TraefikConfigRouter{ + Rule: fmt.Sprintf("Path(`/%s%s/healthz`)", dwId, endpointPath), + Service: dwId, + Middlewares: middlewares, + Priority: 101, + } + } + } } - return nil } func addEndpointToTraefikConfig(componentName string, e dw.Endpoint, cfg *gateway.TraefikConfig, cheCluster *v2alpha1.CheCluster, routing *dwo.DevWorkspaceRouting) { - var routeName string - if e.Attributes.GetString(uniqueEndpointAttributeName, nil) == "true" { - // if endpoint is unique, we're exposing on /componentName/ - routeName = e.Name - } else { - // if endpoint is NOT unique, we're exposing on /componentName/ - routeName = strconv.Itoa(e.TargetPort) - } - prefix := fmt.Sprintf("/%s/%s", componentName, routeName) + routeName, prefix := createEndpointPath(&e, componentName) rulePrefix := fmt.Sprintf("PathPrefix(`%s`)", prefix) // skip if exact same route is already exposed @@ -538,48 +506,41 @@ func addEndpointToTraefikConfig(componentName string, e dw.Endpoint, cfg *gatewa } name := fmt.Sprintf("%s-%s-%s", routing.Spec.DevWorkspaceId, componentName, routeName) - cfg.HTTP.Routers[name] = &gateway.TraefikConfigRouter{ - Rule: rulePrefix, - Service: name, - Middlewares: calculateMiddlewares(name, false), - Priority: 100, - } - - cfg.HTTP.Services[name] = &gateway.TraefikConfigService{ - LoadBalancer: gateway.TraefikConfigLoadbalancer{ - Servers: []gateway.TraefikConfigLoadbalancerServer{ - { - URL: fmt.Sprintf("http://127.0.0.1:%d", e.TargetPort), - }, - }, - }, - } - - cfg.HTTP.Middlewares[name+"-prefix"] = &gateway.TraefikConfigMiddleware{ - StripPrefix: &gateway.TraefikConfigStripPrefix{ - Prefixes: []string{prefix}, - }, - } - + cfg.AddComponent( + name, + rulePrefix, + 100, + fmt.Sprintf("http://127.0.0.1:%d", e.TargetPort), + []string{prefix}) if util.IsOpenShift4 { - cfg.HTTP.Middlewares[name+"-auth"] = &gateway.TraefikConfigMiddleware{ - ForwardAuth: &gateway.TraefikConfigForwardAuth{ - Address: fmt.Sprintf("http://%s.%s:8089?namespace=%s", gateway.GatewayServiceName, cheCluster.Namespace, routing.Namespace), - }, - } + cfg.AddAuth(name, fmt.Sprintf("http://%s.%s:8089?namespace=%s", gateway.GatewayServiceName, cheCluster.Namespace, routing.Namespace)) + } + + // we need to disable auth for '/healthz' path in main endpoint, for now only on OpenShift + if util.IsOpenShift4 && + e.Attributes.GetString(string(dwo.TypeEndpointAttribute), nil) == string(dwo.MainEndpointType) { + healthzName := name + "-healthz" + healthzPath := prefix + "/healthz" + cfg.AddComponent( + healthzName, + fmt.Sprintf("Path(`%s`)", healthzPath), + 101, + fmt.Sprintf("http://127.0.0.1:%d", e.TargetPort), + []string{prefix}) } } -func calculateMiddlewares(name string, header bool) []string { - if util.IsOpenShift4 { - if header { - return []string{name + "-header", name + "-prefix", name + "-auth"} - } else { - return []string{name + "-prefix", name + "-auth"} - } +func createEndpointPath(e *dw.Endpoint, componentName string) (routeName string, path string) { + if e.Attributes.GetString(uniqueEndpointAttributeName, nil) == "true" { + // if endpoint is unique, we're exposing on /componentName/ + routeName = e.Name } else { - return []string{name + "-prefix"} + // if endpoint is NOT unique, we're exposing on /componentName/ + routeName = strconv.Itoa(e.TargetPort) } + path = fmt.Sprintf("/%s/%s", componentName, routeName) + + return routeName, path } func findServiceForPort(port int32, objs *solvers.RoutingObjects) *corev1.Service { diff --git a/controllers/devworkspace/solver/che_routing_test.go b/controllers/devworkspace/solver/che_routing_test.go index c79c4691f..5b06bd2ed 100644 --- a/controllers/devworkspace/solver/che_routing_test.go +++ b/controllers/devworkspace/solver/che_routing_test.go @@ -8,6 +8,8 @@ import ( "github.com/eclipse-che/che-operator/pkg/util" + "github.com/stretchr/testify/assert" + "github.com/eclipse-che/che-operator/pkg/deploy/gateway" dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" @@ -163,6 +165,7 @@ func relocatableDevWorkspaceRouting() *dwo.DevWorkspaceRouting { Path: "/1/", Attributes: attributes.Attributes{ urlRewriteSupportedEndpointAttributeName: apiext.JSON{Raw: []byte("\"true\"")}, + string(dwo.TypeEndpointAttribute): apiext.JSON{Raw: []byte("\"main\"")}, }, }, { @@ -304,7 +307,7 @@ func TestCreateRelocatedObjectsK8S(t *testing.T) { t.Fatalf("Expected 1 middlewares set but got '%d'", len(workspaceConfig.HTTP.Middlewares)) } - mwares := []string{wsid + "-prefix"} + mwares := []string{wsid + gateway.StripPrefixMiddlewareSuffix} for _, mware := range mwares { if _, ok := workspaceConfig.HTTP.Middlewares[mware]; !ok { t.Fatalf("traefik config doesn't set middleware '%s'", mware) @@ -329,34 +332,22 @@ func TestCreateRelocatedObjectsOpenshift(t *testing.T) { cl, _, objs := getSpecObjects(t, relocatableDevWorkspaceRouting()) - t.Run("noIngresses", func(t *testing.T) { - if len(objs.Ingresses) != 0 { - t.Error() - } - }) - - t.Run("noRoutes", func(t *testing.T) { - if len(objs.Routes) != 0 { - t.Error() - } - }) + assert.Empty(t, objs.Ingresses) + assert.Empty(t, objs.Routes) t.Run("testPodAdditions", func(t *testing.T) { - if len(objs.PodAdditions.Containers) != 1 || objs.PodAdditions.Containers[0].Name != wsGatewayName { - t.Error("expected Container pod addition with Workspace Gateway. Got ", objs.PodAdditions) - } - if len(objs.PodAdditions.Volumes) != 1 || objs.PodAdditions.Volumes[0].Name != wsGatewayName { - t.Error("expected Volume pod addition for workspace gateway. Got ", objs.PodAdditions) - } + assert.Len(t, objs.PodAdditions.Containers, 1) + assert.Equal(t, objs.PodAdditions.Containers[0].Name, wsGatewayName) + + assert.Len(t, objs.PodAdditions.Volumes, 1) + assert.Equal(t, objs.PodAdditions.Volumes[0].Name, wsGatewayName) }) t.Run("traefikConfig", func(t *testing.T) { cms := &corev1.ConfigMapList{} cl.List(context.TODO(), cms) - if len(cms.Items) != 2 { - t.Errorf("there should be 2 configmaps created for the gateway config of the workspace but there were: %d", len(cms.Items)) - } + assert.Len(t, cms.Items, 2) var workspaceMainCfg *corev1.ConfigMap var workspaceCfg *corev1.ConfigMap @@ -369,69 +360,135 @@ func TestCreateRelocatedObjectsOpenshift(t *testing.T) { } } - if workspaceMainCfg == nil { - t.Fatalf("traefik configuration for the workspace not found") - } + assert.NotNil(t, workspaceMainCfg, "traefik configuration for the workspace not found") traefikMainWorkspaceConfig := workspaceMainCfg.Data["wsid.yml"] - - if len(traefikMainWorkspaceConfig) == 0 { - t.Fatal("No traefik config file found in the main workspace config configmap") - } + assert.NotEmpty(t, traefikMainWorkspaceConfig, "No traefik config file found in the main workspace config configmap") traefikWorkspaceConfig := workspaceCfg.Data["workspace.yml"] - if len(traefikWorkspaceConfig) == 0 { - t.Fatal("No traefik config file found in the workspace config configmap") - } + assert.NotEmpty(t, traefikWorkspaceConfig, "No traefik config file found in the workspace config configmap") workspaceConfig := gateway.TraefikConfig{} - if err := yaml.Unmarshal([]byte(traefikWorkspaceConfig), &workspaceConfig); err != nil { - t.Fatal(err) - } - - if len(workspaceConfig.HTTP.Routers) != 1 { - t.Fatalf("Expected 1 traefik routers but got %d", len(workspaceConfig.HTTP.Routers)) - } + assert.NoError(t, yaml.Unmarshal([]byte(traefikWorkspaceConfig), &workspaceConfig)) wsid := "wsid-m1-9999" - if _, ok := workspaceConfig.HTTP.Routers[wsid]; !ok { - t.Fatal("traefik config doesn't contain expected workspace configuration") - } - - if len(workspaceConfig.HTTP.Routers[wsid].Middlewares) != 2 { - t.Fatalf("Expected 2 middlewares in router but got '%d'", len(workspaceConfig.HTTP.Routers[wsid].Middlewares)) - } + assert.Contains(t, workspaceConfig.HTTP.Routers, wsid) + assert.Len(t, workspaceConfig.HTTP.Routers[wsid].Middlewares, 2) workspaceMainConfig := gateway.TraefikConfig{} - if err := yaml.Unmarshal([]byte(traefikMainWorkspaceConfig), &workspaceMainConfig); err != nil { - t.Fatal(err) - } - - if len(workspaceMainConfig.HTTP.Routers) != 1 { - t.Fatalf("Expected one route in main route config but got '%d'", len(workspaceMainConfig.HTTP.Routers)) - } - - if len(workspaceMainConfig.HTTP.Middlewares) != 3 { - t.Fatalf("Expected 3 middlewares set but got '%d'", len(workspaceConfig.HTTP.Middlewares)) - } + assert.NoError(t, yaml.Unmarshal([]byte(traefikMainWorkspaceConfig), &workspaceMainConfig)) + assert.Len(t, workspaceMainConfig.HTTP.Middlewares, 3) wsid = "wsid" - mwares := []string{wsid + "-auth", wsid + "-prefix", wsid + "-header"} + mwares := []string{ + wsid + gateway.AuthMiddlewareSuffix, + wsid + gateway.StripPrefixMiddlewareSuffix, + wsid + gateway.HeaderRewriteMiddlewareSuffix} for _, mware := range mwares { - if _, ok := workspaceMainConfig.HTTP.Middlewares[mware]; !ok { - t.Fatalf("traefik config doesn't set middleware '%s'", mware) - } + assert.Contains(t, workspaceMainConfig.HTTP.Middlewares, mware) + found := false for _, r := range workspaceMainConfig.HTTP.Routers[wsid].Middlewares { if r == mware { found = true } } - if !found { - t.Fatalf("traefik config route doesn't set middleware '%s'", mware) - } + assert.Truef(t, found, "traefik config route doesn't set middleware '%s'", mware) } + t.Run("testHealthzEndpointInMainWorkspaceRoute", func(t *testing.T) { + healthzName := "9999-healthz" + assert.Contains(t, workspaceMainConfig.HTTP.Routers, healthzName) + assert.Equal(t, workspaceMainConfig.HTTP.Routers[healthzName].Service, wsid) + assert.Equal(t, workspaceMainConfig.HTTP.Routers[healthzName].Rule, "Path(`/wsid/m1/9999/healthz`)") + assert.NotContains(t, workspaceMainConfig.HTTP.Routers[healthzName].Middlewares, "wsid"+gateway.AuthMiddlewareSuffix) + assert.Contains(t, workspaceMainConfig.HTTP.Routers[healthzName].Middlewares, "wsid"+gateway.StripPrefixMiddlewareSuffix) + assert.Contains(t, workspaceMainConfig.HTTP.Routers[healthzName].Middlewares, "wsid"+gateway.HeaderRewriteMiddlewareSuffix) + }) + + t.Run("testHealthzEndpointInWorkspaceRoute", func(t *testing.T) { + healthzName := "wsid-m1-9999-healthz" + assert.Contains(t, workspaceConfig.HTTP.Routers, healthzName) + assert.Equal(t, workspaceConfig.HTTP.Routers[healthzName].Service, healthzName) + assert.Equal(t, workspaceConfig.HTTP.Routers[healthzName].Rule, "Path(`/m1/9999/healthz`)") + assert.NotContains(t, workspaceConfig.HTTP.Routers[healthzName].Middlewares, healthzName+gateway.AuthMiddlewareSuffix) + assert.Contains(t, workspaceConfig.HTTP.Routers[healthzName].Middlewares, healthzName+gateway.StripPrefixMiddlewareSuffix) + }) + }) +} + +func TestUniqueMainEndpoint(t *testing.T) { + wsid := "wsid123" + + infrastructure.InitializeForTesting(infrastructure.OpenShiftv4) + cl, _, _ := getSpecObjects(t, &dwo.DevWorkspaceRouting{ + ObjectMeta: metav1.ObjectMeta{ + Name: "routing", + Namespace: "ws", + }, + Spec: dwo.DevWorkspaceRoutingSpec{ + DevWorkspaceId: wsid, + RoutingClass: "che", + Endpoints: map[string]dwo.EndpointList{ + "m1": { + { + Name: "e1", + TargetPort: 9999, + Exposure: dw.PublicEndpointExposure, + Protocol: "https", + Path: "/1/", + Attributes: attributes.Attributes{ + urlRewriteSupportedEndpointAttributeName: apiext.JSON{Raw: []byte("\"true\"")}, + string(dwo.TypeEndpointAttribute): apiext.JSON{Raw: []byte("\"main\"")}, + uniqueEndpointAttributeName: apiext.JSON{Raw: []byte("\"true\"")}, + }, + }, + }, + }, + }, + }) + + cms := &corev1.ConfigMapList{} + cl.List(context.TODO(), cms) + + assert.Len(t, cms.Items, 2) + + var workspaceMainCfg *corev1.ConfigMap + var workspaceCfg *corev1.ConfigMap + for _, cfg := range cms.Items { + if cfg.Name == wsid+"-route" && cfg.Namespace == "ns" { + workspaceMainCfg = cfg.DeepCopy() + } + if cfg.Name == wsid+"-route" && cfg.Namespace == "ws" { + workspaceCfg = cfg.DeepCopy() + } + } + + traefikWorkspaceConfig := workspaceCfg.Data["workspace.yml"] + workspaceConfig := gateway.TraefikConfig{} + assert.NoError(t, yaml.Unmarshal([]byte(traefikWorkspaceConfig), &workspaceConfig)) + + traefikMainWorkspaceConfig := workspaceMainCfg.Data[wsid+".yml"] + workspaceMainConfig := gateway.TraefikConfig{} + assert.NoError(t, yaml.Unmarshal([]byte(traefikMainWorkspaceConfig), &workspaceMainConfig)) + + t.Run("testHealthzEndpointInMainWorkspaceRoute", func(t *testing.T) { + healthzName := "e1-healthz" + assert.Contains(t, workspaceMainConfig.HTTP.Routers, healthzName) + assert.Equal(t, workspaceMainConfig.HTTP.Routers[healthzName].Service, wsid) + assert.Equal(t, workspaceMainConfig.HTTP.Routers[healthzName].Rule, "Path(`/"+wsid+"/m1/e1/healthz`)") + assert.NotContains(t, workspaceMainConfig.HTTP.Routers[healthzName].Middlewares, wsid+gateway.AuthMiddlewareSuffix) + assert.Contains(t, workspaceMainConfig.HTTP.Routers[healthzName].Middlewares, wsid+gateway.StripPrefixMiddlewareSuffix) + assert.Contains(t, workspaceMainConfig.HTTP.Routers[healthzName].Middlewares, wsid+gateway.HeaderRewriteMiddlewareSuffix) + }) + + t.Run("testHealthzEndpointInWorkspaceRoute", func(t *testing.T) { + healthzName := wsid + "-m1-e1-healthz" + assert.Contains(t, workspaceConfig.HTTP.Routers, healthzName) + assert.Equal(t, workspaceConfig.HTTP.Routers[healthzName].Service, healthzName) + assert.Equal(t, workspaceConfig.HTTP.Routers[healthzName].Rule, "Path(`/m1/e1/healthz`)") + assert.NotContains(t, workspaceConfig.HTTP.Routers[healthzName].Middlewares, healthzName+gateway.AuthMiddlewareSuffix) + assert.Contains(t, workspaceConfig.HTTP.Routers[healthzName].Middlewares, healthzName+gateway.StripPrefixMiddlewareSuffix) }) } diff --git a/pkg/deploy/dashboard/dashboard.go b/pkg/deploy/dashboard/dashboard.go index 79a69d3a0..c4d5b89df 100644 --- a/pkg/deploy/dashboard/dashboard.go +++ b/pkg/deploy/dashboard/dashboard.go @@ -113,7 +113,8 @@ func (d *Dashboard) createGatewayConfig() *gateway.TraefikConfig { d.component, fmt.Sprintf("PathPrefix(`%s`)", exposePath), 10, - "http://"+d.component+":8080") + "http://"+d.component+":8080", + []string{}) if util.IsNativeUserModeEnabled(d.deployContext.CheCluster) { cfg.AddAuthHeaderRewrite(d.component) } diff --git a/pkg/deploy/devfileregistry/devfileregistry.go b/pkg/deploy/devfileregistry/devfileregistry.go index 950f7d277..aeef5db76 100644 --- a/pkg/deploy/devfileregistry/devfileregistry.go +++ b/pkg/deploy/devfileregistry/devfileregistry.go @@ -113,8 +113,8 @@ func (p *DevfileRegistry) createGatewayConfig() *gateway.TraefikConfig { deploy.DevfileRegistryName, fmt.Sprintf("PathPrefix(`%s`)", pathPrefix), 10, - "http://"+deploy.DevfileRegistryName+":8080") - cfg.AddStripPrefix(deploy.DevfileRegistryName, []string{pathPrefix}) + "http://"+deploy.DevfileRegistryName+":8080", + []string{pathPrefix}) return cfg } diff --git a/pkg/deploy/gateway/gateway.go b/pkg/deploy/gateway/gateway.go index 4e895d573..e4a1646c8 100644 --- a/pkg/deploy/gateway/gateway.go +++ b/pkg/deploy/gateway/gateway.go @@ -270,7 +270,8 @@ func getGatewayServerConfigSpec(deployContext *deploy.DeployContext) (corev1.Con serverComponentName, "Path(`/`, `/f`) || PathPrefix(`/api`, `/swagger`, `/_app`)", 1, - "http://"+deploy.CheServiceName+":8080") + "http://"+deploy.CheServiceName+":8080", + []string{}) if util.IsNativeUserModeEnabled(deployContext.CheCluster) { cfg.AddAuthHeaderRewrite(serverComponentName) @@ -427,6 +428,9 @@ func skipAuthConfig(instance *orgv1.CheCluster) string { if !instance.Spec.Server.ExternalDevfileRegistry { skipAuthPaths = append(skipAuthPaths, "^/"+deploy.DevfileRegistryName) } + if util.IsNativeUserModeEnabled(instance) { + skipAuthPaths = append(skipAuthPaths, "/healthz$") + } if len(skipAuthPaths) > 0 { return fmt.Sprintf("skip_auth_regex = \"%s\"", strings.Join(skipAuthPaths, "|")) } diff --git a/pkg/deploy/gateway/gateway_test.go b/pkg/deploy/gateway/gateway_test.go index 154885f67..02ffefa1a 100644 --- a/pkg/deploy/gateway/gateway_test.go +++ b/pkg/deploy/gateway/gateway_test.go @@ -190,7 +190,7 @@ func TestRandomCookieSecret(t *testing.T) { } } -func TestOauthProxyConfigUnauthorizedRegistries(t *testing.T) { +func TestOauthProxyConfigUnauthorizedPaths(t *testing.T) { t.Run("no skip auth", func(t *testing.T) { configmap := getGatewayOauthProxyConfigSpec(&orgv1.CheCluster{ Spec: orgv1.CheClusterSpec{ @@ -242,6 +242,16 @@ func TestOauthProxyConfigUnauthorizedRegistries(t *testing.T) { t.Error("oauth config should skip auth for plugin and devfile registry.", config) } }) + + t.Run("skip '/healthz' path", func(t *testing.T) { + configmap := getGatewayOauthProxyConfigSpec(&orgv1.CheCluster{ + Spec: orgv1.CheClusterSpec{ + Auth: orgv1.CheClusterSpecAuth{ + NativeUserMode: util.NewBoolPointer(true), + }}}, "blabol") + config := configmap.Data["oauth-proxy.cfg"] + assert.Contains(t, config, "/healthz$") + }) } func TestTokenValidityCheckOnOpenShiftNativeUser(t *testing.T) { diff --git a/pkg/deploy/gateway/traefik_config_util.go b/pkg/deploy/gateway/traefik_config_util.go index ebe924557..c660c29bd 100644 --- a/pkg/deploy/gateway/traefik_config_util.go +++ b/pkg/deploy/gateway/traefik_config_util.go @@ -11,35 +11,51 @@ // package gateway -func CreateCommonTraefikConfig(componentName string, rule string, priority int, serviceAddr string) *TraefikConfig { +const ( + StripPrefixMiddlewareSuffix = "-strip-prefix" + HeaderRewriteMiddlewareSuffix = "-header-rewrite" + AuthMiddlewareSuffix = "-auth" +) + +func CreateEmptyTraefikConfig() *TraefikConfig { return &TraefikConfig{ HTTP: TraefikConfigHTTP{ - Routers: map[string]*TraefikConfigRouter{ - componentName: { - Rule: rule, - Service: componentName, - Middlewares: []string{}, - Priority: priority, - }, - }, - Services: map[string]*TraefikConfigService{ - componentName: { - LoadBalancer: TraefikConfigLoadbalancer{ - Servers: []TraefikConfigLoadbalancerServer{ - { - URL: serviceAddr, - }, - }, - }, - }, - }, + Routers: map[string]*TraefikConfigRouter{}, + Services: map[string]*TraefikConfigService{}, Middlewares: map[string]*TraefikConfigMiddleware{}, }, } } +func CreateCommonTraefikConfig(componentName string, rule string, priority int, serviceAddr string, stripPrefixes []string) *TraefikConfig { + cfg := CreateEmptyTraefikConfig() + cfg.AddComponent(componentName, rule, priority, serviceAddr, stripPrefixes) + return cfg +} + +func (cfg *TraefikConfig) AddComponent(componentName string, rule string, priority int, serviceAddr string, stripPrefixes []string) { + cfg.HTTP.Routers[componentName] = &TraefikConfigRouter{ + Rule: rule, + Service: componentName, + Middlewares: []string{}, + Priority: priority, + } + cfg.HTTP.Services[componentName] = &TraefikConfigService{ + LoadBalancer: TraefikConfigLoadbalancer{ + Servers: []TraefikConfigLoadbalancerServer{ + { + URL: serviceAddr, + }, + }, + }, + } + if len(stripPrefixes) > 0 { + cfg.AddStripPrefix(componentName, stripPrefixes) + } +} + func (cfg *TraefikConfig) AddStripPrefix(componentName string, prefixes []string) { - middlewareName := componentName + "-strip-prefix" + middlewareName := componentName + StripPrefixMiddlewareSuffix cfg.HTTP.Routers[componentName].Middlewares = append(cfg.HTTP.Routers[componentName].Middlewares, middlewareName) cfg.HTTP.Middlewares[middlewareName] = &TraefikConfigMiddleware{ StripPrefix: &TraefikConfigStripPrefix{ @@ -49,7 +65,7 @@ func (cfg *TraefikConfig) AddStripPrefix(componentName string, prefixes []string } func (cfg *TraefikConfig) AddAuthHeaderRewrite(componentName string) { - middlewareName := componentName + "-header-rewrite" + middlewareName := componentName + HeaderRewriteMiddlewareSuffix cfg.HTTP.Routers[componentName].Middlewares = append(cfg.HTTP.Routers[componentName].Middlewares, middlewareName) cfg.HTTP.Middlewares[middlewareName] = &TraefikConfigMiddleware{ Plugin: &TraefikPlugin{ @@ -75,3 +91,13 @@ func (cfg *TraefikConfig) AddOpenShiftTokenCheck(componentName string) { }, } } + +func (cfg *TraefikConfig) AddAuth(componentName string, authAddress string) { + middlewareName := componentName + AuthMiddlewareSuffix + cfg.HTTP.Routers[componentName].Middlewares = append(cfg.HTTP.Routers[componentName].Middlewares, middlewareName) + cfg.HTTP.Middlewares[middlewareName] = &TraefikConfigMiddleware{ + ForwardAuth: &TraefikConfigForwardAuth{ + Address: authAddress, + }, + } +} diff --git a/pkg/deploy/gateway/traefik_config_util_test.go b/pkg/deploy/gateway/traefik_config_util_test.go index 3c7b762e9..23d8c9b3d 100644 --- a/pkg/deploy/gateway/traefik_config_util_test.go +++ b/pkg/deploy/gateway/traefik_config_util_test.go @@ -22,8 +22,48 @@ const ( testRule = "PathPrefix(`/test`)" ) +func TestCreateEmptyTraefikConfig(t *testing.T) { + cfg := CreateEmptyTraefikConfig() + + assert.Empty(t, cfg.HTTP.Routers) + assert.Empty(t, cfg.HTTP.Services) + assert.Empty(t, cfg.HTTP.Middlewares) +} + +func TestTraefikConfig_AddComponent(t *testing.T) { + cfg := CreateEmptyTraefikConfig() + cfg.AddComponent(testComponentName, testRule, 1, "http://svc", []string{}) + + assert.Contains(t, cfg.HTTP.Routers, testComponentName) + assert.Contains(t, cfg.HTTP.Services, testComponentName) + assert.Empty(t, cfg.HTTP.Middlewares) +} + +func TestStripPrefixesWhenCreating(t *testing.T) { + check := func(cfg *TraefikConfig) { + assert.Contains(t, cfg.HTTP.Routers[testComponentName].Middlewares, testComponentName+StripPrefixMiddlewareSuffix) + setPrefixes := cfg.HTTP.Middlewares[testComponentName+StripPrefixMiddlewareSuffix].StripPrefix.Prefixes + assert.Contains(t, setPrefixes, "a") + assert.Contains(t, setPrefixes, "b") + assert.Contains(t, setPrefixes, "c") + assert.Len(t, setPrefixes, 3) + } + t.Run("addComponent", func(t *testing.T) { + cfg := CreateEmptyTraefikConfig() + cfg.AddComponent(testComponentName, testRule, 1, "http://svc", []string{"a", "b", "c"}) + + check(cfg) + }) + + t.Run("createCommon", func(t *testing.T) { + cfg := CreateCommonTraefikConfig(testComponentName, testRule, 1, "http://svc", []string{"a", "b", "c"}) + + check(cfg) + }) +} + func TestAddStripPrefix(t *testing.T) { - cfg := CreateCommonTraefikConfig(testComponentName, testRule, 1, "http://svc:8080") + cfg := CreateCommonTraefikConfig(testComponentName, testRule, 1, "http://svc:8080", []string{}) cfg.AddStripPrefix(testComponentName, []string{"/test"}) assert.Len(t, cfg.HTTP.Routers[testComponentName].Middlewares, 1, *cfg) @@ -32,7 +72,7 @@ func TestAddStripPrefix(t *testing.T) { } func TestAddAuthHeaderRewrite(t *testing.T) { - cfg := CreateCommonTraefikConfig(testComponentName, testRule, 1, "http://svc:8080") + cfg := CreateCommonTraefikConfig(testComponentName, testRule, 1, "http://svc:8080", []string{}) cfg.AddAuthHeaderRewrite(testComponentName) assert.Len(t, cfg.HTTP.Routers[testComponentName].Middlewares, 1, *cfg) @@ -41,7 +81,7 @@ func TestAddAuthHeaderRewrite(t *testing.T) { } func TestAddOpenShiftTokenCheck(t *testing.T) { - cfg := CreateCommonTraefikConfig(testComponentName, testRule, 1, "http://svc:8080") + cfg := CreateCommonTraefikConfig(testComponentName, testRule, 1, "http://svc:8080", []string{}) cfg.AddOpenShiftTokenCheck(testComponentName) assert.Len(t, cfg.HTTP.Routers[testComponentName].Middlewares, 1, *cfg) @@ -54,31 +94,33 @@ func TestAddOpenShiftTokenCheck(t *testing.T) { func TestMiddlewaresPreserveOrder(t *testing.T) { t.Run("strip-header", func(t *testing.T) { - cfg := CreateCommonTraefikConfig(testComponentName, testRule, 1, "http://svc:8080") + cfg := CreateCommonTraefikConfig(testComponentName, testRule, 1, "http://svc:8080", []string{}) cfg.AddStripPrefix(testComponentName, []string{"/test"}) cfg.AddAuthHeaderRewrite(testComponentName) - assert.Equal(t, testComponentName+"-strip-prefix", cfg.HTTP.Routers[testComponentName].Middlewares[0], + assert.Equal(t, testComponentName+StripPrefixMiddlewareSuffix, cfg.HTTP.Routers[testComponentName].Middlewares[0], "first middleware should be strip-prefix") - assert.Equal(t, testComponentName+"-header-rewrite", cfg.HTTP.Routers[testComponentName].Middlewares[1], + assert.Equal(t, testComponentName+HeaderRewriteMiddlewareSuffix, cfg.HTTP.Routers[testComponentName].Middlewares[1], "second middleware should be header-rewrite") }) t.Run("header-strip", func(t *testing.T) { - cfg := CreateCommonTraefikConfig(testComponentName, testRule, 1, "http://svc:8080") + cfg := CreateCommonTraefikConfig(testComponentName, testRule, 1, "http://svc:8080", []string{}) cfg.AddAuthHeaderRewrite(testComponentName) cfg.AddStripPrefix(testComponentName, []string{"/test"}) - assert.Equal(t, testComponentName+"-header-rewrite", cfg.HTTP.Routers[testComponentName].Middlewares[0], + assert.Equal(t, testComponentName+HeaderRewriteMiddlewareSuffix, cfg.HTTP.Routers[testComponentName].Middlewares[0], "first middleware should be header-rewrite") - assert.Equal(t, testComponentName+"-strip-prefix", cfg.HTTP.Routers[testComponentName].Middlewares[1], + assert.Equal(t, testComponentName+StripPrefixMiddlewareSuffix, cfg.HTTP.Routers[testComponentName].Middlewares[1], "second middleware should be strip-prefix") }) } func TestCreateCommonTraefikConfig(t *testing.T) { - cfg := CreateCommonTraefikConfig(testComponentName, testRule, 1, "http://svc:8080") + cfg := CreateCommonTraefikConfig(testComponentName, testRule, 1, "http://svc:8080", []string{}) - assert.Len(t, cfg.HTTP.Routers[testComponentName].Middlewares, 0, *cfg) - assert.Len(t, cfg.HTTP.Middlewares, 0, *cfg) + assert.Contains(t, cfg.HTTP.Routers, testComponentName) + assert.Contains(t, cfg.HTTP.Services, testComponentName) + assert.Empty(t, cfg.HTTP.Routers[testComponentName].Middlewares, *cfg) + assert.Empty(t, cfg.HTTP.Middlewares, *cfg) } diff --git a/pkg/deploy/identity-provider/identity_provider.go b/pkg/deploy/identity-provider/identity_provider.go index b00d36ef1..99cd638bb 100644 --- a/pkg/deploy/identity-provider/identity_provider.go +++ b/pkg/deploy/identity-provider/identity_provider.go @@ -365,7 +365,8 @@ func createGatewayConfig(cheCluster *orgv1.CheCluster) *gateway.TraefikConfig { deploy.IdentityProviderName, "PathPrefix(`/auth`)", 10, - "http://"+deploy.IdentityProviderName+":8080") + "http://"+deploy.IdentityProviderName+":8080", + []string{}) if util.IsNativeUserModeEnabled(cheCluster) { cfg.AddAuthHeaderRewrite(deploy.IdentityProviderName) diff --git a/pkg/deploy/pluginregistry/pluginregistry.go b/pkg/deploy/pluginregistry/pluginregistry.go index 5447d9e01..096eb2ea6 100644 --- a/pkg/deploy/pluginregistry/pluginregistry.go +++ b/pkg/deploy/pluginregistry/pluginregistry.go @@ -122,8 +122,8 @@ func (p *PluginRegistry) createGatewayConfig() *gateway.TraefikConfig { deploy.PluginRegistryName, fmt.Sprintf("PathPrefix(`%s`)", pathPrefix), 10, - "http://"+deploy.PluginRegistryName+":8080") - cfg.AddStripPrefix(deploy.PluginRegistryName, []string{pathPrefix}) + "http://"+deploy.PluginRegistryName+":8080", + []string{pathPrefix}) return cfg }