Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,25 @@ type BackupCronJobConfig struct {
BackoffLimit *int32 `json:"backoffLimit,omitempty"`
}

// GatewayReference defines a reference to a Gateway API Gateway resource
// that HTTPRoutes should attach to via parentRefs.
type GatewayReference struct {
// Name is the name of the Gateway resource
// +kubebuilder:validation:Required

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Required only checks presence; maybe add length bounds?

Suggested change
// +kubebuilder:validation:Required
// +kubebuilder:validation:Required
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=253

Name string `json:"name"`
// Namespace is the namespace of the Gateway resource.
// If not specified, HTTPRoutes will reference a Gateway in the same namespace
// as the DevWorkspace.
// +kubebuilder:validation:Optional

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit, Perhaps add DNS-1123 label bounds here too

Suggested change
// +kubebuilder:validation:Optional
// +kubebuilder:validation:Optional
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=63

Namespace *string `json:"namespace,omitempty"`
// GatewayClassName is the name of the GatewayClass to use.
// This is used for validation and informational purposes.
// Defaults to "nginx" if not specified.
// +kubebuilder:validation:Optional
// +kubebuilder:default:="nginx"
GatewayClassName string `json:"gatewayClassName,omitempty"`

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+kubebuilder:default always populates this, would omitempty be needed?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On second thought, I am not sure why this field is needed. It is not referenced in gateway solver. It only consumes Name and Namespace. So generated HTTPRoutes are identical regardless of what this is set to.

}

type RoutingConfig struct {
// DefaultRoutingClass specifies the routingClass to be used when a DevWorkspace
// specifies an empty `.spec.routingClass`. Supported routingClasses can be defined
Expand All @@ -144,6 +163,12 @@ type RoutingConfig struct {
// TLSCertificateConfigmapRef defines the name and namespace of the configmap with a certificate to inject into the
// HTTP client.
TLSCertificateConfigmapRef *ConfigmapReference `json:"tlsCertificateConfigmapRef,omitempty"`
// GatewayRef defines a reference to a Gateway API Gateway resource that HTTPRoutes
// should attach to when using the 'gateway-api' routing class. This field is required
// when routingClass is set to 'gateway-api'. The referenced Gateway must be provisioned
// by the cluster administrator or Che Operator before workspaces can use Gateway API routing.
// +kubebuilder:validation:Optional
GatewayRef *GatewayReference `json:"gatewayRef,omitempty"`
}

// OverrideConfig defines configuration options for controlling which fields are restricted
Expand Down
1 change: 1 addition & 0 deletions apis/controller/v1alpha1/devworkspacerouting_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ const (
DevWorkspaceRoutingCluster DevWorkspaceRoutingClass = "cluster"
DevWorkspaceRoutingClusterTLS DevWorkspaceRoutingClass = "cluster-tls"
DevWorkspaceRoutingWebTerminal DevWorkspaceRoutingClass = "web-terminal"
DevWorkspaceRoutingGatewayAPI DevWorkspaceRoutingClass = "gateway-api"
)

// DevWorkspaceRoutingStatus defines the observed state of DevWorkspaceRouting
Expand Down
25 changes: 25 additions & 0 deletions apis/controller/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/controller"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
gwapiv1 "sigs.k8s.io/gateway-api/apis/v1"

controllerv1alpha1 "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1"
)
Expand Down Expand Up @@ -69,6 +70,7 @@ type DevWorkspaceRoutingReconciler struct {
// +kubebuilder:rbac:groups=route.openshift.io,resources=routes,verbs=*
// +kubebuidler:rbac:groups=route.openshift.io,resources=routes/status,verbs=get,list,watch
// +kubebuilder:rbac:groups=route.openshift.io,resources=routes/custom-host,verbs=create
// +kubebuilder:rbac:groups=gateway.networking.k8s.io,resources=httproutes,verbs=*

func (r *DevWorkspaceRoutingReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
reqLogger := r.Log.WithValues("Request.Namespace", req.Namespace, "Request.Name", req.Name)
Expand Down Expand Up @@ -184,6 +186,16 @@ func (r *DevWorkspaceRoutingReconciler) Reconcile(ctx context.Context, req ctrl.
routes[idx].Annotations = maputils.Append(routes[idx].Annotations, constants.DevWorkspaceRestrictedAccessAnnotation, restrictedAccess)
}
}
httpRoutes := routingObjects.HTTPRoutes
for idx := range httpRoutes {
err := controllerutil.SetControllerReference(instance, &httpRoutes[idx], r.Scheme)
if err != nil {
return reconcile.Result{}, err
}
if setRestrictedAccess {
httpRoutes[idx].Annotations = maputils.Append(httpRoutes[idx].Annotations, constants.DevWorkspaceRestrictedAccessAnnotation, restrictedAccess)
}
}

servicesInSync, clusterServices, err := r.syncServices(instance, services)
if err != nil {
Expand Down Expand Up @@ -232,12 +244,34 @@ func (r *DevWorkspaceRoutingReconciler) Reconcile(ctx context.Context, req ctrl.
clusterRoutingObj.Ingresses = clusterIngresses
}

// Sync HTTPRoutes if using gateway-api routing class
if len(httpRoutes) > 0 {
httpRoutesInSync, clusterHTTPRoutes, err := r.syncHTTPRoutes(instance, httpRoutes)
if err != nil {
failError := &sync.UnrecoverableSyncError{}
if errors.As(err, &failError) {
return reconcile.Result{}, r.markRoutingFailed(instance, err.Error())
}
reqLogger.Error(err, "Error syncing HTTPRoutes")
return reconcile.Result{Requeue: true}, r.reconcileStatus(instance, nil, nil, false, "Preparing HTTPRoutes")
} else if !httpRoutesInSync {
reqLogger.Info("HTTPRoutes not in sync")
return reconcile.Result{Requeue: true}, r.reconcileStatus(instance, nil, nil, false, "Preparing HTTPRoutes")
}
clusterRoutingObj.HTTPRoutes = clusterHTTPRoutes
}

Comment thread
coderabbitai[bot] marked this conversation as resolved.
exposedEndpoints, endpointsAreReady, err := solver.GetExposedEndpoints(instance.Spec.Endpoints, clusterRoutingObj)
if err != nil {
reqLogger.Error(err, "Could not get exposed endpoints for devworkspace")
return reconcile.Result{}, r.markRoutingFailed(instance, fmt.Sprintf("Could not get exposed endpoints for DevWorkspace: %s", err))
}

if !endpointsAreReady {
reqLogger.Info("Endpoints not ready, requeuing to check HTTPRoute status")
return reconcile.Result{RequeueAfter: 3 * time.Second}, r.reconcileStatus(instance, nil, nil, false, "Waiting for HTTPRoute endpoints to be ready")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This message always says “HTTPRoute…” for all routing classes, perhaps we should keep it generic?

}

return reconcile.Result{}, r.reconcileStatus(instance, &routingObjects, exposedEndpoints, endpointsAreReady, "")
}

Expand Down Expand Up @@ -347,7 +381,8 @@ func (r *DevWorkspaceRoutingReconciler) SetupWithManager(mgr ctrl.Manager) error
}).
For(&controllerv1alpha1.DevWorkspaceRouting{}).
Owns(&corev1.Service{}).
Owns(&networkingv1.Ingress{})
Owns(&networkingv1.Ingress{}).
Owns(&gwapiv1.HTTPRoute{})

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We currently register it unconditionally. Gateway API CRDs aren't installed by default on Kubernetes or OpenShift. Could we gate this the same way the Route watch is gated ?

if infrastructure.IsOpenShift() {
bld.Owns(&routeV1.Route{})
}
Expand Down
Loading
Loading