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
23 changes: 23 additions & 0 deletions apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,23 @@ 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

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Good catch, I will add the length bounds.

// +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

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Agreed, adding bounds.

// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=63
Namespace *string `json:"namespace,omitempty"`
}

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 +161,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.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// by the cluster administrator or Che Operator before workspaces can use Gateway API routing.
// by the cluster administrator before workspaces can use Gateway API routing.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

To provide context, in DWO we try to minimize any mention of Eclipse Che, since technically, Che is not a dependency of DWO

// +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=get;list;watch;create;update;patch;delete

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")
return reconcile.Result{RequeueAfter: 3 * time.Second}, r.reconcileStatus(instance, nil, nil, false, "Waiting for endpoints to be ready")
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm not sure if a requeue is necessary here, since there nothing in the reconciliation loop that should happen after endpointsAreReady goes from false to true?

I could be missing something but is there a reason to requeue?


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

Expand Down Expand Up @@ -351,6 +385,9 @@ func (r *DevWorkspaceRoutingReconciler) SetupWithManager(mgr ctrl.Manager) error
if infrastructure.IsOpenShift() {
bld.Owns(&routeV1.Route{})
}
if infrastructure.IsGatewayAPIInstalled() {
bld.Owns(&gwapiv1.HTTPRoute{})
}
if r.SolverGetter == nil {
return NoSolversEnabled
}
Expand Down
Loading
Loading