Make workspace routes more user-friendly #1672
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,7 +16,7 @@ import ( | |
| "context" | ||
| "fmt" | ||
| "path" | ||
| "regexp" | ||
| "strings" | ||
|
|
||
| dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" | ||
|
|
@@ -232,6 +232,8 @@ func (c *CheRoutingSolver) cheExposedEndpoints(cheCluster *chev2.CheCluster, wor | |
|
|
||
| gatewayHost := cheCluster.GetCheHost() | ||
|
|
||
| endpointStrategy := getEndpointPathStrategy(c.client, workspaceID, routingObj.Services[0].Namespace, routingObj.Services[0].ObjectMeta.OwnerReferences[0].Name) | ||
|
|
||
| for component, endpoints := range componentEndpoints { | ||
| for _, endpoint := range endpoints { | ||
| if dw.EndpointExposure(endpoint.Exposure) == dw.NoneEndpointExposure { | ||
|
|
@@ -279,7 +281,7 @@ func (c *CheRoutingSolver) cheExposedEndpoints(cheCluster *chev2.CheCluster, wor | |
| return map[string]dwo.ExposedEndpointList{}, false, nil | ||
| } | ||
|
|
||
| publicURLPrefix := getPublicURLPrefixForEndpoint(component, endpoint, endpointStrategy) | ||
| endpointURL = path.Join(gatewayHost, publicURLPrefix, endpoint.Path) | ||
| } | ||
|
|
||
|
|
@@ -329,34 +331,124 @@ func (c *CheRoutingSolver) getGatewayConfigsAndFillRoutingObjects(cheCluster *ch | |
| } | ||
|
|
||
| configs := make([]corev1.ConfigMap, 0) | ||
| endpointStrategy := getEndpointPathStrategy(c.client, workspaceID, routing.Namespace, routing.Name) | ||
|
|
||
| // first do routing from main che-gateway into workspace service | ||
| if mainWsRouteConfig, err := provisionMainWorkspaceRoute(cheCluster, routing, cmLabels, endpointStrategy); err != nil { | ||
| return nil, err | ||
| } else { | ||
| configs = append(configs, *mainWsRouteConfig) | ||
| } | ||
|
|
||
| // then expose the endpoints | ||
| if infraExposer, err := c.getInfraSpecificExposer(cheCluster, routing, objs, endpointStrategy); err != nil { | ||
| return nil, err | ||
| } else { | ||
| if workspaceConfig := exposeAllEndpoints(cheCluster, routing, objs, infraExposer, endpointStrategy); workspaceConfig != nil { | ||
| configs = append(configs, *workspaceConfig) | ||
| } | ||
| } | ||
|
|
||
| return configs, nil | ||
| } | ||
|
|
||
| func getEndpointPathStrategy(c client.Client, workspaceId string, namespace string, dwRoutingName string) EndpointStrategy { | ||
| useLegacyPaths := false | ||
| username, err := getNormalizedUsername(c, namespace) | ||
| if err != nil { | ||
| useLegacyPaths = true | ||
| } | ||
|
|
||
| dwName, err := getNormalizedWkspName(c, namespace, dwRoutingName) | ||
| if err != nil { | ||
| useLegacyPaths = true | ||
| } | ||
|
|
||
| if useLegacyPaths { | ||
| strategy := new(Legacy) | ||
| strategy.workspaceID = workspaceId | ||
| return strategy | ||
| } else { | ||
| strategy := new(UsernameWkspName) | ||
| strategy.username = username | ||
| strategy.workspaceName = dwName | ||
| return strategy | ||
| } | ||
| } | ||
|
|
||
| func getNormalizedUsername(c client.Client, namespace string) (string, error) { | ||
| username, err := getUsernameFromNamespace(c, namespace) | ||
|
|
||
| if err != nil { | ||
| username, err = getUsernameFromSecret(c, namespace) | ||
| } | ||
|
|
||
| if err != nil { | ||
| return "", err | ||
| } | ||
| return normalize(username), nil | ||
| } | ||
|
|
||
| func getUsernameFromSecret(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 | ||
|
There was a problem hiding this comment. The username is being read from the Another way to read the username could be to read the There was a problem hiding this comment. BTW, There was a problem hiding this comment. Do we have a backup plan instead of throwing an error if Secret doesn't exist ? There was a problem hiding this comment. I'm afraid there is no backup plan right now if that secret does not exist, are there other ways to detect the username? There was a problem hiding this comment. What about using the legacy route name if username is unknown? There was a problem hiding this comment. whereas a backup plan could be nice to have, using There was a problem hiding this comment. After more investigation, I think the minikube tests are failing because the As a result, workspaces cannot start There was a problem hiding this comment. @dkwon17 thanks for the investigation, so in this case the flow when devworkspace is created using There was a problem hiding this comment. @ibuziuk yes, that's right There was a problem hiding this comment. I am currently working on the legacy routes in this case, as suggested by @tolusha |
||
| } | ||
|
|
||
| func getUsernameFromNamespace(c client.Client, namespace string) (string, error) { | ||
| nsInfo := &corev1.Namespace{} | ||
| err := c.Get(context.TODO(), client.ObjectKey{Name: namespace}, nsInfo) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
|
|
||
| notFoundError := fmt.Errorf("username not found in namespace %s", namespace) | ||
|
|
||
| if nsInfo.GetAnnotations() == nil { | ||
| return "", notFoundError | ||
| } | ||
|
|
||
| username, exists := nsInfo.GetAnnotations()["che.eclipse.org/username"] | ||
|
|
||
| if exists { | ||
| return username, nil | ||
| } | ||
|
|
||
| return "", notFoundError | ||
| } | ||
|
|
||
| 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 { | ||
|
There was a problem hiding this comment. |
||
| 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 | ||
| result = r3.ReplaceAllString(result, "") // trim dashes at beginning/end | ||
| return strings.ToLower(result) | ||
| } | ||
|
|
||
| func (c *CheRoutingSolver) getInfraSpecificExposer(cheCluster *chev2.CheCluster, routing *dwo.DevWorkspaceRouting, objs *solvers.RoutingObjects, endpointStrategy EndpointStrategy) (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, endpointStrategy) | ||
| objs.Routes = append(objs.Routes, route) | ||
| }, nil | ||
| } else { | ||
|
|
@@ -365,7 +457,7 @@ func (c *CheRoutingSolver) getInfraSpecificExposer(cheCluster *chev2.CheCluster, | |
| return nil, err | ||
| } | ||
| return func(info *EndpointInfo) { | ||
| ingress := exposer.getIngressForService(info, endpointStrategy) | ||
| objs.Ingresses = append(objs.Ingresses, ingress) | ||
| }, nil | ||
| } | ||
|
|
@@ -381,7 +473,7 @@ func getCommonService(objs *solvers.RoutingObjects, dwId string) *corev1.Service | |
| return nil | ||
| } | ||
|
|
||
| func exposeAllEndpoints(cheCluster *chev2.CheCluster, routing *dwo.DevWorkspaceRouting, objs *solvers.RoutingObjects, ingressExpose func(*EndpointInfo), endpointStrategy EndpointStrategy) *corev1.ConfigMap { | ||
| wsRouteConfig := gateway.CreateEmptyTraefikConfig() | ||
|
|
||
| commonService := getCommonService(objs, routing.Spec.DevWorkspaceId) | ||
|
|
@@ -412,7 +504,7 @@ func exposeAllEndpoints(cheCluster *chev2.CheCluster, routing *dwo.DevWorkspaceR | |
| } | ||
|
|
||
| if e.Attributes.GetString(urlRewriteSupportedEndpointAttributeName, nil) == "true" { | ||
| addEndpointToTraefikConfig(componentName, e, wsRouteConfig, cheCluster, routing, endpointStrategy) | ||
| } else { | ||
| ingressExpose(&EndpointInfo{ | ||
| order: order, | ||
|
|
@@ -473,16 +565,16 @@ func containPort(service *corev1.Service, port int32) bool { | |
| return false | ||
| } | ||
|
|
||
| func provisionMainWorkspaceRoute(cheCluster *chev2.CheCluster, routing *dwo.DevWorkspaceRouting, cmLabels map[string]string, endpointStrategy EndpointStrategy) (*corev1.ConfigMap, error) { | ||
| dwId := routing.Spec.DevWorkspaceId | ||
| dwNamespace := routing.Namespace | ||
|
|
||
| cfg := gateway.CreateCommonTraefikConfig( | ||
| dwId, | ||
| fmt.Sprintf("PathPrefix(`%s`)", endpointStrategy.getMainWorkspacePathPrefix()), | ||
| 100, | ||
| getServiceURL(wsGatewayPort, dwId, dwNamespace), | ||
| []string{endpointStrategy.getMainWorkspacePathPrefix()}) | ||
|
|
||
| if cheCluster.IsAccessTokenConfigured() { | ||
| cfg.AddAuthHeaderRewrite(dwId) | ||
|
|
@@ -494,7 +586,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, endpointStrategy) | ||
|
|
||
| if contents, err := yaml.Marshal(cfg); err != nil { | ||
| return nil, err | ||
|
|
@@ -540,18 +632,18 @@ 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, endpointStrategy EndpointStrategy) { | ||
| 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 infrastructure.IsOpenShift() { | ||
| middlewares = append(middlewares, dwId+gateway.HeaderRewriteMiddlewareSuffix) | ||
| } | ||
| routeName, endpointPath := endpointStrategy.getEndpointPath(&e, componentName) | ||
| routerName := fmt.Sprintf("%s-%s-healthz", dwId, routeName) | ||
| cfg.HTTP.Routers[routerName] = &gateway.TraefikConfigRouter{ | ||
| Rule: fmt.Sprintf("Path(`%s/healthz`)", endpointStrategy.getEndpointPathPrefix(endpointPath)), | ||
| Service: dwId, | ||
| Middlewares: middlewares, | ||
| Priority: 101, | ||
|
|
@@ -561,8 +653,8 @@ func routeForHealthzEndpoint(cfg *gateway.TraefikConfig, dwId string, endpoints | |
| } | ||
| } | ||
|
|
||
| func addEndpointToTraefikConfig(componentName string, e dwo.Endpoint, cfg *gateway.TraefikConfig, cheCluster *chev2.CheCluster, routing *dwo.DevWorkspaceRouting, endpointStrategy EndpointStrategy) { | ||
| routeName, prefix := endpointStrategy.getEndpointPath(&e, componentName) | ||
| rulePrefix := fmt.Sprintf("PathPrefix(`%s`)", prefix) | ||
|
|
||
| // skip if exact same route is already exposed | ||
|
|
@@ -594,19 +686,6 @@ func addEndpointToTraefikConfig(componentName string, e dwo.Endpoint, cfg *gatew | |
| } | ||
| } | ||
|
|
||
| func findServiceForPort(port int32, objs *solvers.RoutingObjects) *corev1.Service { | ||
| for i := range objs.Services { | ||
| svc := &objs.Services[i] | ||
|
|
@@ -713,20 +792,12 @@ func getServiceURL(port int32, workspaceID string, workspaceNamespace string) st | |
| return fmt.Sprintf("http://%s.%s.svc:%d", common.ServiceName(workspaceID), workspaceNamespace, port) | ||
| } | ||
|
|
||
| func getPublicURLPrefixForEndpoint(componentName string, endpoint dwo.Endpoint, endpointStrategy EndpointStrategy) string { | ||
| endpointName := "" | ||
| if endpoint.Attributes.GetString(uniqueEndpointAttributeName, nil) == "true" { | ||
| endpointName = endpoint.Name | ||
| } | ||
| return endpointStrategy.getPublicURLPrefix(int32(endpoint.TargetPort), endpointName, componentName) | ||
| } | ||
|
|
||
| func determineEndpointScheme(e dwo.Endpoint) string { | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The username is also being read from the namespace as well, in the case that the admin has pre-provisioned the namespaces following the doc, since in this case, the
user-profilesecret would not be created