From c7c8d2459ea5f6d765d7cfb42c7b077a3ba5091a Mon Sep 17 00:00:00 2001 From: David Kwon Date: Fri, 21 Apr 2023 15:38:21 -0400 Subject: [PATCH 1/3] Make workspace routes more user-friendly Signed-off-by: David Kwon --- .../devworkspace/solver/che_routing.go | 85 ++++++++--- .../devworkspace/solver/che_routing_test.go | 136 +++++++++++++----- .../devworkspace/solver/endpoint_exposer.go | 12 +- 3 files changed, 174 insertions(+), 59 deletions(-) diff --git a/controllers/devworkspace/solver/che_routing.go b/controllers/devworkspace/solver/che_routing.go index f1a2a49dc9..97cc06e9eb 100644 --- a/controllers/devworkspace/solver/che_routing.go +++ b/controllers/devworkspace/solver/che_routing.go @@ -16,6 +16,7 @@ import ( "context" "fmt" "path" + "regexp" "strconv" "strings" @@ -281,7 +282,17 @@ func (c *CheRoutingSolver) cheExposedEndpoints(cheCluster *chev2.CheCluster, wor return map[string]dwo.ExposedEndpointList{}, false, nil } - publicURLPrefix := getPublicURLPrefixForEndpoint(workspaceID, component, endpoint) + username, err := getNormalizedUsername(c.client, routingObj.Services[0].Namespace) + if err != nil { + return map[string]dwo.ExposedEndpointList{}, false, err + } + + dwName, err := getNormalizedWkspName(c.client, routingObj.Services[0].Namespace, routingObj.Services[0].ObjectMeta.OwnerReferences[0].Name) + if err != nil { + return map[string]dwo.ExposedEndpointList{}, false, err + } + + publicURLPrefix := getPublicURLPrefixForEndpoint(dwName, username, endpoint) endpointURL = path.Join(gatewayHost, publicURLPrefix, endpoint.Path) } @@ -331,16 +342,25 @@ func (c *CheRoutingSolver) getGatewayConfigsAndFillRoutingObjects(cheCluster *ch } configs := make([]corev1.ConfigMap, 0) + username, err := getNormalizedUsername(c.client, routing.Namespace) + if err != nil { + return nil, err + } + + dwName, err := getNormalizedWkspName(c.client, routing.Namespace, routing.Name) + if err != nil { + return nil, err + } // first do routing from main che-gateway into workspace service - if mainWsRouteConfig, err := provisionMainWorkspaceRoute(cheCluster, routing, cmLabels); err != nil { + if mainWsRouteConfig, err := provisionMainWorkspaceRoute(cheCluster, routing, username, dwName, cmLabels); err != nil { return nil, err } else { configs = append(configs, *mainWsRouteConfig) } // then expose the endpoints - if infraExposer, err := c.getInfraSpecificExposer(cheCluster, routing, objs); err != nil { + if infraExposer, err := c.getInfraSpecificExposer(cheCluster, routing, objs, username, dwName); err != nil { return nil, err } else { if workspaceConfig := exposeAllEndpoints(cheCluster, routing, objs, infraExposer); workspaceConfig != nil { @@ -351,14 +371,43 @@ func (c *CheRoutingSolver) getGatewayConfigsAndFillRoutingObjects(cheCluster *ch return configs, nil } -func (c *CheRoutingSolver) getInfraSpecificExposer(cheCluster *chev2.CheCluster, routing *dwo.DevWorkspaceRouting, objs *solvers.RoutingObjects) (func(info *EndpointInfo), error) { +func getNormalizedUsername(c client.Client, namespace string) (string, error) { + secret := &corev1.Secret{} + err := c.Get(context.TODO(), client.ObjectKey{Name: "user-profile", Namespace: namespace}, secret) + if err != nil { + return "", err + } + username := string(secret.Data["name"]) + return normalize(username), nil +} + +func getNormalizedWkspName(c client.Client, namespace string, routingName string) (string, error) { + routing := &dwo.DevWorkspaceRouting{} + err := c.Get(context.TODO(), client.ObjectKey{Name: routingName, Namespace: namespace}, routing) + if err != nil { + return "", err + } + return normalize(routing.ObjectMeta.OwnerReferences[0].Name), nil +} + +func normalize(username string) string { + r1 := regexp.MustCompile(`[^-a-zA-Z0-9]`) + r2 := regexp.MustCompile(`-+`) + r3 := regexp.MustCompile(`^-|-$`) + + result := r1.ReplaceAllString(username, "-") // replace invalid chars with '-' + result = r2.ReplaceAllString(result, "-") // replace multiple '-' with single ones + return r3.ReplaceAllString(result, "") // trim dashes at beginning/end +} + +func (c *CheRoutingSolver) getInfraSpecificExposer(cheCluster *chev2.CheCluster, routing *dwo.DevWorkspaceRouting, objs *solvers.RoutingObjects, username string, dwName string) (func(info *EndpointInfo), error) { if infrastructure.IsOpenShift() { exposer := &RouteExposer{} if err := exposer.initFrom(context.TODO(), c.client, cheCluster, routing); err != nil { return nil, err } return func(info *EndpointInfo) { - route := exposer.getRouteForService(info) + route := exposer.getRouteForService(info, username, dwName) objs.Routes = append(objs.Routes, route) }, nil } else { @@ -367,7 +416,7 @@ func (c *CheRoutingSolver) getInfraSpecificExposer(cheCluster *chev2.CheCluster, return nil, err } return func(info *EndpointInfo) { - ingress := exposer.getIngressForService(info) + ingress := exposer.getIngressForService(info, username, dwName) objs.Ingresses = append(objs.Ingresses, ingress) }, nil } @@ -475,16 +524,16 @@ func containPort(service *corev1.Service, port int32) bool { return false } -func provisionMainWorkspaceRoute(cheCluster *chev2.CheCluster, routing *dwo.DevWorkspaceRouting, cmLabels map[string]string) (*corev1.ConfigMap, error) { +func provisionMainWorkspaceRoute(cheCluster *chev2.CheCluster, routing *dwo.DevWorkspaceRouting, username string, dwName string, cmLabels map[string]string) (*corev1.ConfigMap, error) { dwId := routing.Spec.DevWorkspaceId dwNamespace := routing.Namespace cfg := gateway.CreateCommonTraefikConfig( dwId, - fmt.Sprintf("PathPrefix(`/%s`)", dwId), + fmt.Sprintf("PathPrefix(`/%s/%s`)", username, dwName), 100, getServiceURL(wsGatewayPort, dwId, dwNamespace), - []string{"/" + dwId}) + []string{fmt.Sprintf("/%s/%s", username, dwName)}) if cheCluster.IsAccessTokenConfigured() { cfg.AddAuthHeaderRewrite(dwId) @@ -496,7 +545,7 @@ func provisionMainWorkspaceRoute(cheCluster *chev2.CheCluster, routing *dwo.DevW add5XXErrorHandling(cfg, dwId) // make '/healthz' path of main endpoints reachable from outside - routeForHealthzEndpoint(cfg, dwId, routing.Spec.Endpoints) + routeForHealthzEndpoint(cfg, username, dwName, dwId, routing.Spec.Endpoints) if contents, err := yaml.Marshal(cfg); err != nil { return nil, err @@ -542,7 +591,7 @@ func add5XXErrorHandling(cfg *gateway.TraefikConfig, dwId string) { } // makes '/healthz' path of main endpoints reachable from the outside -func routeForHealthzEndpoint(cfg *gateway.TraefikConfig, dwId string, endpoints map[string]dwo.EndpointList) { +func routeForHealthzEndpoint(cfg *gateway.TraefikConfig, username string, dwName string, 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) { @@ -553,7 +602,7 @@ func routeForHealthzEndpoint(cfg *gateway.TraefikConfig, dwId string, endpoints routeName, endpointPath := createEndpointPath(&e, componentName) routerName := fmt.Sprintf("%s-%s-healthz", dwId, routeName) cfg.HTTP.Routers[routerName] = &gateway.TraefikConfigRouter{ - Rule: fmt.Sprintf("Path(`/%s%s/healthz`)", dwId, endpointPath), + Rule: fmt.Sprintf("Path(`/%s/%s%s/healthz`)", username, dwName, endpointPath), Service: dwId, Middlewares: middlewares, Priority: 101, @@ -604,7 +653,7 @@ func createEndpointPath(e *dwo.Endpoint, componentName string) (routeName string // if endpoint is NOT unique, we're exposing on /componentName/ routeName = strconv.Itoa(e.TargetPort) } - path = fmt.Sprintf("/%s/%s", componentName, routeName) + path = fmt.Sprintf("/%s", routeName) return routeName, path } @@ -715,20 +764,20 @@ func getServiceURL(port int32, workspaceID string, workspaceNamespace string) st return fmt.Sprintf("http://%s.%s.svc:%d", common.ServiceName(workspaceID), workspaceNamespace, port) } -func getPublicURLPrefixForEndpoint(workspaceID string, machineName string, endpoint dwo.Endpoint) string { +func getPublicURLPrefixForEndpoint(dwName string, username string, endpoint dwo.Endpoint) string { endpointName := "" if endpoint.Attributes.GetString(uniqueEndpointAttributeName, nil) == "true" { endpointName = endpoint.Name } - return getPublicURLPrefix(workspaceID, machineName, int32(endpoint.TargetPort), endpointName) + return getPublicURLPrefix(dwName, username, int32(endpoint.TargetPort), endpointName) } -func getPublicURLPrefix(workspaceID string, machineName string, port int32, uniqueEndpointName string) string { +func getPublicURLPrefix(dwName string, username string, port int32, uniqueEndpointName string) string { if uniqueEndpointName == "" { - return fmt.Sprintf(endpointURLPrefixPattern, workspaceID, machineName, port) + return fmt.Sprintf(endpointURLPrefixPattern, username, dwName, port) } - return fmt.Sprintf(uniqueEndpointURLPrefixPattern, workspaceID, machineName, uniqueEndpointName) + return fmt.Sprintf(uniqueEndpointURLPrefixPattern, username, dwName, uniqueEndpointName) } func determineEndpointScheme(e dwo.Endpoint) string { diff --git a/controllers/devworkspace/solver/che_routing_test.go b/controllers/devworkspace/solver/che_routing_test.go index 4aa9ab9831..252a15749d 100644 --- a/controllers/devworkspace/solver/che_routing_test.go +++ b/controllers/devworkspace/solver/che_routing_test.go @@ -47,6 +47,7 @@ import ( utilruntime "k8s.io/apimachinery/pkg/util/runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/reconcile" "sigs.k8s.io/yaml" ) @@ -68,7 +69,7 @@ func createTestScheme() *runtime.Scheme { func getSpecObjectsForManager(t *testing.T, mgr *chev2.CheCluster, routing *dwo.DevWorkspaceRouting, additionalInitialObjects ...runtime.Object) (client.Client, solvers.RoutingSolver, solvers.RoutingObjects) { scheme := createTestScheme() - allObjs := []runtime.Object{mgr} + allObjs := []runtime.Object{mgr, routing} for i := range additionalInitialObjects { allObjs = append(allObjs, additionalInitialObjects[i]) } @@ -97,6 +98,26 @@ func getSpecObjectsForManager(t *testing.T, mgr *chev2.CheCluster, routing *dwo. t.Fatal(err) } + // set owner references for the routing objects + for idx := range objs.Services { + err := controllerutil.SetControllerReference(routing, &objs.Services[idx], scheme) + if err != nil { + t.Fatal(err) + } + } + for idx := range objs.Ingresses { + err := controllerutil.SetControllerReference(routing, &objs.Ingresses[idx], scheme) + if err != nil { + t.Fatal(err) + } + } + for idx := range objs.Routes { + err := controllerutil.SetControllerReference(routing, &objs.Routes[idx], scheme) + if err != nil { + t.Fatal(err) + } + } + // now we need a second round of che manager reconciliation so that it proclaims the che gateway as established cheRecon.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "che", Namespace: "ns"}}) @@ -116,7 +137,7 @@ func getSpecObjects(t *testing.T, routing *dwo.DevWorkspaceRouting) (client.Clie Hostname: "over.the.rainbow", }, }, - }, routing) + }, routing, userProfileSecret()) } func subdomainDevWorkspaceRouting() *dwo.DevWorkspaceRouting { @@ -124,6 +145,14 @@ func subdomainDevWorkspaceRouting() *dwo.DevWorkspaceRouting { ObjectMeta: metav1.ObjectMeta{ Name: "routing", Namespace: "ws", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "workspace.devfile.io/v1alpha2", + Kind: "DevWorkspace", + Name: "my-workspace", + UID: "uid", + }, + }, }, Spec: dwo.DevWorkspaceRoutingSpec{ DevWorkspaceId: "wsid", @@ -161,6 +190,14 @@ func relocatableDevWorkspaceRouting() *dwo.DevWorkspaceRouting { ObjectMeta: metav1.ObjectMeta{ Name: "routing", Namespace: "ws", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "workspace.devfile.io/v1alpha2", + Kind: "DevWorkspace", + Name: "my-workspace", + UID: "uid", + }, + }, }, Spec: dwo.DevWorkspaceRoutingSpec{ DevWorkspaceId: "wsid", @@ -203,6 +240,19 @@ func relocatableDevWorkspaceRouting() *dwo.DevWorkspaceRouting { } } +func userProfileSecret() *corev1.Secret { + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "user-profile", + Namespace: "ws", + Finalizers: []string{controller.FinalizerName}, + }, + Data: map[string][]byte{ + "name": []byte("username"), + }, + } +} + func TestCreateRelocatedObjectsK8S(t *testing.T) { infrastructure.InitializeForTesting(infrastructure.Kubernetes) cl, _, objs := getSpecObjects(t, relocatableDevWorkspaceRouting()) @@ -344,7 +394,7 @@ func TestCreateRelocatedObjectsK8S(t *testing.T) { healthzName := "wsid-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.Equal(t, workspaceMainConfig.HTTP.Routers[healthzName].Rule, "Path(`/username/my-workspace/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.NotContains(t, workspaceMainConfig.HTTP.Routers[healthzName].Middlewares, "wsid"+gateway.HeaderRewriteMiddlewareSuffix) @@ -354,7 +404,7 @@ func TestCreateRelocatedObjectsK8S(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.Equal(t, workspaceConfig.HTTP.Routers[healthzName].Rule, "Path(`/9999/healthz`)") assert.NotContains(t, workspaceConfig.HTTP.Routers[healthzName].Middlewares, healthzName+gateway.AuthMiddlewareSuffix) assert.Contains(t, workspaceConfig.HTTP.Routers[healthzName].Middlewares, healthzName+gateway.StripPrefixMiddlewareSuffix) }) @@ -449,7 +499,7 @@ func TestCreateRelocatedObjectsOpenshift(t *testing.T) { healthzName := "wsid-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.Equal(t, workspaceMainConfig.HTTP.Routers[healthzName].Rule, "Path(`/username/my-workspace/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) @@ -459,7 +509,7 @@ func TestCreateRelocatedObjectsOpenshift(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.Equal(t, workspaceConfig.HTTP.Routers[healthzName].Rule, "Path(`/9999/healthz`)") assert.NotContains(t, workspaceConfig.HTTP.Routers[healthzName].Middlewares, healthzName+gateway.AuthMiddlewareSuffix) assert.Contains(t, workspaceConfig.HTTP.Routers[healthzName].Middlewares, healthzName+gateway.StripPrefixMiddlewareSuffix) }) @@ -474,6 +524,14 @@ func TestUniqueMainEndpoint(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: "routing", Namespace: "ws", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "workspace.devfile.io/v1alpha2", + Kind: "DevWorkspace", + Name: "my-workspace", + UID: "uid", + }, + }, }, Spec: dwo.DevWorkspaceRoutingSpec{ DevWorkspaceId: wsid, @@ -525,7 +583,7 @@ func TestUniqueMainEndpoint(t *testing.T) { healthzName := wsid + "-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.Equal(t, workspaceMainConfig.HTTP.Routers[healthzName].Rule, "Path(`/username/my-workspace/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) @@ -535,7 +593,7 @@ func TestUniqueMainEndpoint(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.Equal(t, workspaceConfig.HTTP.Routers[healthzName].Rule, "Path(`/e1/healthz`)") assert.NotContains(t, workspaceConfig.HTTP.Routers[healthzName].Middlewares, healthzName+gateway.AuthMiddlewareSuffix) assert.Contains(t, workspaceConfig.HTTP.Routers[healthzName].Middlewares, healthzName+gateway.StripPrefixMiddlewareSuffix) }) @@ -590,14 +648,14 @@ func TestCreateSubDomainObjects(t *testing.T) { if len(objs.Ingresses) != 3 { t.Error("Expected 3 ingress, found ", len(objs.Ingresses)) } - if objs.Ingresses[0].Spec.Rules[0].Host != "wsid-1.down.on.earth" { - t.Error("Expected Ingress host 'wsid-1.down.on.earth', but got ", objs.Ingresses[0].Spec.Rules[0].Host) + if objs.Ingresses[0].Spec.Rules[0].Host != "username.my-workspace.e1.down.on.earth" { + t.Error("Expected Ingress host 'username.my-workspace.e1.down.on.earth', but got ", objs.Ingresses[0].Spec.Rules[0].Host) } - if objs.Ingresses[1].Spec.Rules[0].Host != "wsid-2.down.on.earth" { - t.Error("Expected Ingress host 'wsid-2.down.on.earth', but got ", objs.Ingresses[1].Spec.Rules[0].Host) + if objs.Ingresses[1].Spec.Rules[0].Host != "username.my-workspace.e2.down.on.earth" { + t.Error("Expected Ingress host 'username.my-workspace.e2.down.on.earth', but got ", objs.Ingresses[1].Spec.Rules[0].Host) } - if objs.Ingresses[2].Spec.Rules[0].Host != "wsid-3.down.on.earth" { - t.Error("Expected Ingress host 'wsid-3.down.on.earth', but got ", objs.Ingresses[2].Spec.Rules[0].Host) + if objs.Ingresses[2].Spec.Rules[0].Host != "username.my-workspace.e3.down.on.earth" { + t.Error("Expected Ingress host 'username.my-workspace.e3.down.on.earth', but got ", objs.Ingresses[2].Spec.Rules[0].Host) } }) @@ -606,8 +664,8 @@ func TestCreateSubDomainObjects(t *testing.T) { if len(objs.Routes) != 3 { t.Error("Expected 3 Routes, found ", len(objs.Routes)) } - if objs.Routes[0].Spec.Host != "wsid-1.down.on.earth" { - t.Error("Expected Route host 'wsid-1.down.on.earth', but got ", objs.Routes[0].Spec.Host) + if objs.Routes[0].Spec.Host != "username.my-workspace.e1.down.on.earth" { + t.Error("Expected Route host 'username.my-workspace.e1.down.on.earth', but got ", objs.Routes[0].Spec.Host) } }) } @@ -645,24 +703,24 @@ func TestReportRelocatableExposedEndpoints(t *testing.T) { if e1.Name != "e1" { t.Errorf("The first endpoint should have been e1 but is %s", e1.Name) } - if e1.Url != "https://over.the.rainbow/wsid/m1/9999/1/" { - t.Errorf("The e1 endpoint should have the following URL: '%s' but has '%s'.", "https://over.the.rainbow/wsid/m1/9999/1/", e1.Url) + if e1.Url != "https://over.the.rainbow/username/my-workspace/9999/1/" { + t.Errorf("The e1 endpoint should have the following URL: '%s' but has '%s'.", "https://over.the.rainbow/username/my-workspace/9999/1/", e1.Url) } e2 := m1[1] if e2.Name != "e2" { t.Errorf("The second endpoint should have been e2 but is %s", e1.Name) } - if e2.Url != "https://over.the.rainbow/wsid/m1/9999/2.js" { - t.Errorf("The e2 endpoint should have the following URL: '%s' but has '%s'.", "https://over.the.rainbow/wsid/m1/9999/2.js", e2.Url) + if e2.Url != "https://over.the.rainbow/username/my-workspace/9999/2.js" { + t.Errorf("The e2 endpoint should have the following URL: '%s' but has '%s'.", "https://over.the.rainbow/username/my-workspace/9999/2.js", e2.Url) } e3 := m1[2] if e3.Name != "e3" { t.Errorf("The third endpoint should have been e3 but is %s", e1.Name) } - if e3.Url != "https://over.the.rainbow/wsid/m1/9999/" { - t.Errorf("The e3 endpoint should have the following URL: '%s' but has '%s'.", "https://over.the.rainbow/wsid/m1/9999/", e3.Url) + if e3.Url != "https://over.the.rainbow/username/my-workspace/9999/" { + t.Errorf("The e3 endpoint should have the following URL: '%s' but has '%s'.", "https://over.the.rainbow/username/my-workspace/9999/", e3.Url) } } @@ -673,6 +731,14 @@ func TestExposeEndpoints(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: "routing", Namespace: "ws", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "workspace.devfile.io/v1alpha2", + Kind: "DevWorkspace", + Name: "my-workspace", + UID: "uid", + }, + }, }, Spec: dwo.DevWorkspaceRoutingSpec{ DevWorkspaceId: "wsid", @@ -767,13 +833,13 @@ func TestExposeEndpoints(t *testing.T) { assert.True(t, ok) assert.Equal(t, 1, len(sp)) assert.Equal(t, "server-pub", sp[0].Name) - assert.Equal(t, "https://over.the.rainbow/wsid/server-public/8082/", sp[0].Url) + assert.Equal(t, "https://over.the.rainbow/username/my-workspace/8082/", sp[0].Url) spnr, ok := exposed["server-public-no-rewrite"] assert.True(t, ok) assert.Equal(t, 1, len(spnr)) assert.Equal(t, "server-pub-nr", spnr[0].Name) - assert.Equal(t, "http://wsid-1.down.on.earth/", spnr[0].Url) + assert.Equal(t, "http://username.my-workspace.server-pub-nr.down.on.earth/", spnr[0].Url) } func TestReportSubdomainExposedEndpoints(t *testing.T) { @@ -807,24 +873,24 @@ func TestReportSubdomainExposedEndpoints(t *testing.T) { if e1.Name != "e1" { t.Errorf("The first endpoint should have been e1 but is %s", e1.Name) } - if e1.Url != "https://wsid-1.down.on.earth/1/" { - t.Errorf("The e1 endpoint should have the following URL: '%s' but has '%s'.", "https://wsid-1.down.on.earth/1/", e1.Url) + if e1.Url != "https://username.my-workspace.e1.down.on.earth/1/" { + t.Errorf("The e1 endpoint should have the following URL: '%s' but has '%s'.", "https://username.my-workspace.e1.down.on.earth/1/", e1.Url) } e2 := m1[1] if e2.Name != "e2" { t.Errorf("The second endpoint should have been e2 but is %s", e1.Name) } - if e2.Url != "https://wsid-2.down.on.earth/2.js" { - t.Errorf("The e2 endpoint should have the following URL: '%s' but has '%s'.", "https://wsid-2.down.on.earth/2.js", e2.Url) + if e2.Url != "https://username.my-workspace.e2.down.on.earth/2.js" { + t.Errorf("The e2 endpoint should have the following URL: '%s' but has '%s'.", "https://username.my-workspace.e2.down.on.earth/2.js", e2.Url) } e3 := m1[2] if e3.Name != "e3" { t.Errorf("The third endpoint should have been e3 but is %s", e1.Name) } - if e3.Url != "http://wsid-3.down.on.earth/" { - t.Errorf("The e3 endpoint should have the following URL: '%s' but has '%s'.", "https://wsid-3.down.on.earth/", e3.Url) + if e3.Url != "http://username.my-workspace.e3.down.on.earth/" { + t.Errorf("The e3 endpoint should have the following URL: '%s' but has '%s'.", "http://username.my-workspace.e3.down.on.earth/", e3.Url) } } @@ -892,7 +958,7 @@ func TestUsesIngressAnnotationsForWorkspaceEndpointIngresses(t *testing.T) { }, } - _, _, objs := getSpecObjectsForManager(t, mgr, subdomainDevWorkspaceRouting()) + _, _, objs := getSpecObjectsForManager(t, mgr, subdomainDevWorkspaceRouting(), userProfileSecret()) if len(objs.Ingresses) != 3 { t.Fatalf("Unexpected number of generated ingresses: %d", len(objs.Ingresses)) @@ -927,7 +993,7 @@ func TestUsesCustomCertificateForWorkspaceEndpointIngresses(t *testing.T) { }, } - _, _, objs := getSpecObjectsForManager(t, mgr, subdomainDevWorkspaceRouting(), &corev1.Secret{ + _, _, objs := getSpecObjectsForManager(t, mgr, subdomainDevWorkspaceRouting(), userProfileSecret(), &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "tlsSecret", Namespace: "ns", @@ -956,7 +1022,7 @@ func TestUsesCustomCertificateForWorkspaceEndpointIngresses(t *testing.T) { t.Fatalf("Unexpected number of host records on the TLS spec: %d", len(ingress.Spec.TLS[0].Hosts)) } - if ingress.Spec.TLS[0].Hosts[0] != "wsid-1.almost.trivial" { + if ingress.Spec.TLS[0].Hosts[0] != "username.my-workspace.e1.almost.trivial" { t.Errorf("Unexpected host name of the TLS spec: %s", ingress.Spec.TLS[0].Hosts[0]) } @@ -974,7 +1040,7 @@ func TestUsesCustomCertificateForWorkspaceEndpointIngresses(t *testing.T) { t.Fatalf("Unexpected number of host records on the TLS spec: %d", len(ingress.Spec.TLS[0].Hosts)) } - if ingress.Spec.TLS[0].Hosts[0] != "wsid-2.almost.trivial" { + if ingress.Spec.TLS[0].Hosts[0] != "username.my-workspace.e2.almost.trivial" { t.Errorf("Unexpected host name of the TLS spec: %s", ingress.Spec.TLS[0].Hosts[0]) } @@ -1003,7 +1069,7 @@ func TestUsesCustomCertificateForWorkspaceEndpointRoutes(t *testing.T) { }, } - _, _, objs := getSpecObjectsForManager(t, mgr, subdomainDevWorkspaceRouting(), &corev1.Secret{ + _, _, objs := getSpecObjectsForManager(t, mgr, subdomainDevWorkspaceRouting(), userProfileSecret(), &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "tlsSecret", Namespace: "ns", diff --git a/controllers/devworkspace/solver/endpoint_exposer.go b/controllers/devworkspace/solver/endpoint_exposer.go index 6ee93062a8..dd8ce06aa6 100644 --- a/controllers/devworkspace/solver/endpoint_exposer.go +++ b/controllers/devworkspace/solver/endpoint_exposer.go @@ -142,7 +142,7 @@ func (e *IngressExposer) initFrom(ctx context.Context, cl client.Client, cluster return nil } -func (e *RouteExposer) getRouteForService(endpoint *EndpointInfo) routev1.Route { +func (e *RouteExposer) getRouteForService(endpoint *EndpointInfo, username string, dwName string) routev1.Route { targetEndpoint := intstr.FromInt(int(endpoint.port)) labels := labels.Merge( e.labels, @@ -159,7 +159,7 @@ func (e *RouteExposer) getRouteForService(endpoint *EndpointInfo) routev1.Route OwnerReferences: endpoint.service.OwnerReferences, }, Spec: routev1.RouteSpec{ - Host: hostName(endpoint.order, e.devWorkspaceID, e.baseDomain), + Host: hostName(username, dwName, endpoint.endpointName, e.baseDomain), To: routev1.RouteTargetReference{ Kind: "Service", Name: endpoint.service.Name, @@ -185,8 +185,8 @@ func (e *RouteExposer) getRouteForService(endpoint *EndpointInfo) routev1.Route return route } -func (e *IngressExposer) getIngressForService(endpoint *EndpointInfo) networkingv1.Ingress { - hostname := hostName(endpoint.order, e.devWorkspaceID, e.baseDomain) +func (e *IngressExposer) getIngressForService(endpoint *EndpointInfo, username string, dwName string) networkingv1.Ingress { + hostname := hostName(username, dwName, endpoint.endpointName, e.baseDomain) ingressPathType := networkingv1.PathTypeImplementationSpecific ingress := networkingv1.Ingress{ @@ -239,8 +239,8 @@ func (e *IngressExposer) getIngressForService(endpoint *EndpointInfo) networking return ingress } -func hostName(order int, workspaceID string, baseDomain string) string { - return fmt.Sprintf("%s-%d.%s", workspaceID, order, baseDomain) +func hostName(username string, workspaceName string, endpointName string, baseDomain string) string { + return fmt.Sprintf("%s.%s.%s.%s", username, workspaceName, endpointName, baseDomain) } func routeAnnotations(machineName string, endpointName string) map[string]string { From c25db9ca5b626698249f90cafae7e7e03ab37ccc Mon Sep 17 00:00:00 2001 From: David Kwon Date: Mon, 8 May 2023 12:16:06 -0400 Subject: [PATCH 2/3] Add update verb for ingress in che-operator CR Signed-off-by: David Kwon --- .../manifests/che-operator.clusterserviceversion.yaml | 5 +++-- config/rbac/cluster_role.yaml | 1 + deploy/deployment/kubernetes/combined.yaml | 1 + .../kubernetes/objects/che-operator.ClusterRole.yaml | 1 + deploy/deployment/openshift/combined.yaml | 1 + .../openshift/objects/che-operator.ClusterRole.yaml | 1 + helmcharts/next/templates/che-operator.ClusterRole.yaml | 1 + 7 files changed, 9 insertions(+), 2 deletions(-) diff --git a/bundle/next/eclipse-che/manifests/che-operator.clusterserviceversion.yaml b/bundle/next/eclipse-che/manifests/che-operator.clusterserviceversion.yaml index d1fbf39682..2b84a88603 100644 --- a/bundle/next/eclipse-che/manifests/che-operator.clusterserviceversion.yaml +++ b/bundle/next/eclipse-che/manifests/che-operator.clusterserviceversion.yaml @@ -77,7 +77,7 @@ metadata: operators.operatorframework.io/project_layout: go.kubebuilder.io/v3 repository: https://github.com/eclipse-che/che-operator support: Eclipse Foundation - name: eclipse-che.v7.66.0-791.next + name: eclipse-che.v7.66.0-792.next namespace: placeholder spec: apiservicedefinitions: {} @@ -713,6 +713,7 @@ spec: - create - watch - get + - update - delete - apiGroups: - apiextensions.k8s.io @@ -1243,7 +1244,7 @@ spec: minKubeVersion: 1.19.0 provider: name: Eclipse Foundation - version: 7.66.0-791.next + version: 7.66.0-792.next webhookdefinitions: - admissionReviewVersions: - v1 diff --git a/config/rbac/cluster_role.yaml b/config/rbac/cluster_role.yaml index 26f49aa752..6b0780979e 100644 --- a/config/rbac/cluster_role.yaml +++ b/config/rbac/cluster_role.yaml @@ -236,6 +236,7 @@ rules: - create - watch - get + - update - delete - apiGroups: - apiextensions.k8s.io diff --git a/deploy/deployment/kubernetes/combined.yaml b/deploy/deployment/kubernetes/combined.yaml index aee563e248..1c9ed7b2f5 100644 --- a/deploy/deployment/kubernetes/combined.yaml +++ b/deploy/deployment/kubernetes/combined.yaml @@ -8233,6 +8233,7 @@ rules: - create - watch - get + - update - delete - apiGroups: - apiextensions.k8s.io diff --git a/deploy/deployment/kubernetes/objects/che-operator.ClusterRole.yaml b/deploy/deployment/kubernetes/objects/che-operator.ClusterRole.yaml index 1820f247e9..71f3031429 100644 --- a/deploy/deployment/kubernetes/objects/che-operator.ClusterRole.yaml +++ b/deploy/deployment/kubernetes/objects/che-operator.ClusterRole.yaml @@ -236,6 +236,7 @@ rules: - create - watch - get + - update - delete - apiGroups: - apiextensions.k8s.io diff --git a/deploy/deployment/openshift/combined.yaml b/deploy/deployment/openshift/combined.yaml index 9295fe33ef..be196f080e 100644 --- a/deploy/deployment/openshift/combined.yaml +++ b/deploy/deployment/openshift/combined.yaml @@ -8233,6 +8233,7 @@ rules: - create - watch - get + - update - delete - apiGroups: - apiextensions.k8s.io diff --git a/deploy/deployment/openshift/objects/che-operator.ClusterRole.yaml b/deploy/deployment/openshift/objects/che-operator.ClusterRole.yaml index 1820f247e9..71f3031429 100644 --- a/deploy/deployment/openshift/objects/che-operator.ClusterRole.yaml +++ b/deploy/deployment/openshift/objects/che-operator.ClusterRole.yaml @@ -236,6 +236,7 @@ rules: - create - watch - get + - update - delete - apiGroups: - apiextensions.k8s.io diff --git a/helmcharts/next/templates/che-operator.ClusterRole.yaml b/helmcharts/next/templates/che-operator.ClusterRole.yaml index 1820f247e9..71f3031429 100644 --- a/helmcharts/next/templates/che-operator.ClusterRole.yaml +++ b/helmcharts/next/templates/che-operator.ClusterRole.yaml @@ -236,6 +236,7 @@ rules: - create - watch - get + - update - delete - apiGroups: - apiextensions.k8s.io From 30db97c71c9e40e0ca016baed5079041df33586f Mon Sep 17 00:00:00 2001 From: David Kwon Date: Mon, 8 May 2023 16:58:58 -0400 Subject: [PATCH 3/3] Update test-operator.sh Signed-off-by: David Kwon --- build/scripts/minikube-tests/test-operator.sh | 71 ++++++++++++++++++- 1 file changed, 70 insertions(+), 1 deletion(-) diff --git a/build/scripts/minikube-tests/test-operator.sh b/build/scripts/minikube-tests/test-operator.sh index 9a892fb4f9..4b52db9d39 100755 --- a/build/scripts/minikube-tests/test-operator.sh +++ b/build/scripts/minikube-tests/test-operator.sh @@ -30,6 +30,75 @@ runTest() { yq -riSY '.spec.template.spec.containers[0].image = "'${OPERATOR_IMAGE}'"' "${CURRENT_OPERATOR_VERSION_TEMPLATE_PATH}/che-operator/kubernetes/operator.yaml" yq -riSY '.spec.template.spec.containers[0].imagePullPolicy = "IfNotPresent"' "${CURRENT_OPERATOR_VERSION_TEMPLATE_PATH}/che-operator/kubernetes/operator.yaml" + cat > /tmp/patch.yaml <