Fixup: double protocol; no dockerimage workspaces

- Avoid errors when workspace doesn't contain any dockerimage (or plugin/editor) components - Fix double protocol for URLs in runtime annotation - Fix import alias naming Signed-off-by: Angel Misevski <amisevsk@redhat.com>
devfile · Apr 6, 2020 · d3df36b · d3df36b
1 parent 0e7bbba
commit d3df36b
Show file tree

Hide file tree

Showing 8 changed files with 95 additions and 45 deletions.
diff --git a/pkg/controller/workspace/provision/components.go b/pkg/controller/workspace/provision/components.go
@@ -128,44 +128,50 @@ func getSpecComponents(workspace *v1alpha1.Workspace, scheme *runtime.Scheme) ([
 		return nil, err
 	}
 
-	dockerResolver := v1alpha1.Component{
-		ObjectMeta: v1.ObjectMeta{
-			Name:      fmt.Sprintf("components-%s-%s", workspace.Status.WorkspaceId, "docker"),
-			Namespace: workspace.Namespace,
-			Labels: map[string]string{
-				config.WorkspaceIDLabel: workspace.Status.WorkspaceId,
+	var components []v1alpha1.Component
+	if len(dockerComponents) > 0 {
+		dockerResolver := v1alpha1.Component{
+			ObjectMeta: v1.ObjectMeta{
+				Name:      fmt.Sprintf("components-%s-%s", workspace.Status.WorkspaceId, "docker"),
+				Namespace: workspace.Namespace,
+				Labels: map[string]string{
+					config.WorkspaceIDLabel: workspace.Status.WorkspaceId,
+				},
 			},
-		},
-		Spec: v1alpha1.WorkspaceComponentSpec{
-			WorkspaceId: workspace.Status.WorkspaceId,
-			Components:  dockerComponents,
-			Commands:    workspace.Spec.Devfile.Commands,
-		},
-	}
-	pluginResolver := v1alpha1.Component{
-		ObjectMeta: v1.ObjectMeta{
-			Name:      fmt.Sprintf("components-%s-%s", workspace.Status.WorkspaceId, "plugins"),
-			Namespace: workspace.Namespace,
-			Labels: map[string]string{
-				config.WorkspaceIDLabel: workspace.Status.WorkspaceId,
+			Spec: v1alpha1.WorkspaceComponentSpec{
+				WorkspaceId: workspace.Status.WorkspaceId,
+				Components:  dockerComponents,
+				Commands:    workspace.Spec.Devfile.Commands,
 			},
-		},
-		Spec: v1alpha1.WorkspaceComponentSpec{
-			WorkspaceId: workspace.Status.WorkspaceId,
-			Components:  pluginComponents,
-			Commands:    workspace.Spec.Devfile.Commands,
-		},
-	}
-	err = controllerutil.SetControllerReference(workspace, &dockerResolver, scheme)
-	if err != nil {
-		return nil, err
-	}
-	err = controllerutil.SetControllerReference(workspace, &pluginResolver, scheme)
-	if err != nil {
-		return nil, err
+		}
+		err = controllerutil.SetControllerReference(workspace, &dockerResolver, scheme)
+		if err != nil {
+			return nil, err
+		}
+		components = append(components, dockerResolver)
+	}
+	if len(pluginComponents) > 0 {
+		pluginResolver := v1alpha1.Component{
+			ObjectMeta: v1.ObjectMeta{
+				Name:      fmt.Sprintf("components-%s-%s", workspace.Status.WorkspaceId, "plugins"),
+				Namespace: workspace.Namespace,
+				Labels: map[string]string{
+					config.WorkspaceIDLabel: workspace.Status.WorkspaceId,
+				},
+			},
+			Spec: v1alpha1.WorkspaceComponentSpec{
+				WorkspaceId: workspace.Status.WorkspaceId,
+				Components:  pluginComponents,
+				Commands:    workspace.Spec.Devfile.Commands,
+			},
+		}
+		err = controllerutil.SetControllerReference(workspace, &pluginResolver, scheme)
+		if err != nil {
+			return nil, err
+		}
+		components = append(components, pluginResolver)
 	}
-
-	return []v1alpha1.Component{pluginResolver, dockerResolver}, nil
+	return components, nil
 }
 
 func getClusterComponents(workspace *v1alpha1.Workspace, client runtimeClient.Client) ([]v1alpha1.Component, error) {

diff --git a/pkg/controller/workspace/runtime/runtime_annotation.go b/pkg/controller/workspace/runtime/runtime_annotation.go
@@ -14,7 +14,6 @@ package runtime
 
 import (
 	"encoding/json"
-	"fmt"
 	"github.com/che-incubator/che-workspace-operator/pkg/apis/workspace/v1alpha1"
 )
 
@@ -45,12 +44,10 @@ func getMachinesAnnotation(components []v1alpha1.ComponentDescription, endpoints
 			servers := map[string]v1alpha1.CheWorkspaceServer{}
 			// TODO: This is likely not a good choice for matching, since it'll fail if container name does not match an endpoint key
 			for _, endpoint := range endpoints[containerName] {
-				protocol := endpoint.Attributes[v1alpha1.PROTOCOL_ENDPOINT_ATTRIBUTE]
-
 				servers[endpoint.Name] = v1alpha1.CheWorkspaceServer{
 					Attributes: endpoint.Attributes,
 					Status:     v1alpha1.RunningServerStatus, // TODO: This is just set so the circles are green
-					URL:        fmt.Sprintf("%s://%s", protocol, endpoint.Url),
+					URL:        endpoint.Url,
 				}
 			}
 			machines[containerName] = v1alpha1.CheWorkspaceMachine{

diff --git a/pkg/controller/workspacerouting/solvers/common.go b/pkg/controller/workspacerouting/solvers/common.go
@@ -113,7 +113,7 @@ func getIngressesForSpec(endpoints map[string][]v1alpha1.Endpoint, workspaceMeta
 
 			endpointName := common.EndpointName(endpoint.Name)
 			ingressHostname := common.EndpointHostname(workspaceMeta.WorkspaceId, endpointName, endpoint.Port, workspaceMeta.IngressGlobalDomain)
-			ingressURL := fmt.Sprintf("http://%s", ingressHostname)
+			ingressURL := fmt.Sprintf("%s://%s", endpoint.Attributes[v1alpha1.PROTOCOL_ENDPOINT_ATTRIBUTE], ingressHostname)
 			ingresses = append(ingresses, v1beta1.Ingress{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      common.RouteName(workspaceMeta.WorkspaceId, endpointName),
@@ -152,3 +152,16 @@ func getIngressesForSpec(endpoints map[string][]v1alpha1.Endpoint, workspaceMeta
 	}
 	return ingresses, exposedEndpoints
 }
+
+// getSecureProtocol takes a (potentially unsecure protocol e.g. http) and returns the secure version (e.g. https).
+// If protocol isn't recognized, it is returned unmodified.
+func getSecureProtocol(protocol string) string {
+	switch protocol {
+	case "ws":
+		return "wss"
+	case "http":
+		return "https"
+	default:
+		return protocol
+	}
+}
diff --git a/pkg/controller/workspacerouting/solvers/oauth_proxy_container.go b/pkg/controller/workspacerouting/solvers/oauth_proxy_container.go
@@ -25,7 +25,7 @@ import (
 const proxyServiceAcctAnnotationKeyFmt string = "serviceaccounts.openshift.io/oauth-redirectreference.%s-%s"
 const proxyServiceAcctAnnotationValueFmt string = `{"kind":"OAuthRedirectReference","apiVersion":"v1","reference":{"kind":"Route","name":"%s"}}`
 
-func (s *OpenShiftOAuthSolver) getProxyPodAdditions(proxyEndpoints map[string]proxyEndpoint, meta WorkspaceMetadata) *v1alpha1.PodAdditions {
+func getProxyPodAdditions(proxyEndpoints map[string]proxyEndpoint, meta WorkspaceMetadata) *v1alpha1.PodAdditions {
 	tlsSecretVolume := buildSecretVolume(common.OAuthProxySecretName(meta.WorkspaceId))
 	var proxyContainers []corev1.Container
 	for _, proxyEndpoint := range proxyEndpoints {

diff --git a/pkg/controller/workspacerouting/solvers/openshift_oauth_solver.go b/pkg/controller/workspacerouting/solvers/openshift_oauth_solver.go
@@ -90,7 +90,7 @@ func (s *OpenShiftOAuthSolver) getProxyRoutes(
 			targetEndpoint := intstr.FromInt(int(endpoint.Port))
 			endpointName := common.EndpointName(endpoint.Name)
 			hostname := common.EndpointHostname(workspaceMeta.WorkspaceId, endpointName, endpoint.Port, workspaceMeta.IngressGlobalDomain)
-			url := fmt.Sprintf("https://%s", hostname)
+			url := fmt.Sprintf("%s://%s", getSecureProtocol(endpoint.Attributes[v1alpha1.PROTOCOL_ENDPOINT_ATTRIBUTE]), hostname)
 
 			// NOTE: openshift oauth-proxy only supports listening on a single port; as a result, we can't proxy more than
 			// one endpoint or we run into cookie issues (each proxied port gets a container and sets a cookie in the browser
@@ -136,7 +136,7 @@ func (s *OpenShiftOAuthSolver) getProxyRoutes(
 			})
 		}
 	}
-	podAdditions = s.getProxyPodAdditions(portMappings, workspaceMeta)
+	podAdditions = getProxyPodAdditions(portMappings, workspaceMeta)
 	return routes, exposedEndpoints, podAdditions
 }
 

diff --git a/pkg/controller/workspacerouting/solvers/solver.go b/pkg/controller/workspacerouting/solvers/solver.go
@@ -14,15 +14,15 @@ package solvers
 
 import (
 	"github.com/che-incubator/che-workspace-operator/pkg/apis/workspace/v1alpha1"
-	v12 "github.com/openshift/api/route/v1"
+	routeV1 "github.com/openshift/api/route/v1"
 	"k8s.io/api/core/v1"
 	"k8s.io/api/extensions/v1beta1"
 )
 
 type RoutingObjects struct {
 	Services         []v1.Service
 	Ingresses        []v1beta1.Ingress
-	Routes           []v12.Route
+	Routes           []routeV1.Route
 	PodAdditions     *v1alpha1.PodAdditions
 	ExposedEndpoints map[string][]v1alpha1.ExposedEndpoint
 }

diff --git a/samples/no-dockerimage.yaml b/samples/no-dockerimage.yaml
@@ -0,0 +1,15 @@
+apiVersion: workspace.che.eclipse.org/v1alpha1
+kind: Workspace
+metadata:
+  name: cloud-shell
+spec:
+  started: true
+  devfile:
+    apiVersion: 0.0.1
+    metadata:
+      name: cloud-shell
+    components:
+      - alias: cloud-shell
+        type: cheEditor
+        id: eclipse/cloud-shell/nightly
+
diff --git a/samples/no-editor.yaml b/samples/no-editor.yaml
@@ -0,0 +1,19 @@
+apiVersion: workspace.che.eclipse.org/v1alpha1
+kind: Workspace
+metadata:
+  name: cloud-shell
+spec:
+  started: true
+  devfile:
+    apiVersion: 0.0.1
+    metadata:
+      name: cloud-shell
+    components:
+      - type: dockerimage
+        memoryLimit: 256Mi
+        alias: dev
+        image: 'quay.io/eclipse/che-sidecar-openshift-connector:0.1.2-2601509'
+        args: ["tail", "-f", "/dev/null"]
+        env:
+          - value: '\[\e[34m\]>\[\e[m\]\[\e[33m\]>\[\e[m\]'
+            name: PS1