cherry pick remove items & block deployment if ingress unset

Signed-off-by: Jordan Dubrick <jdubrick@redhat.com>
devfile · May 14, 2024 · 78f0c44 · 78f0c44
1 parent 94467ca
commit 78f0c44
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 44 deletions.
diff --git a/controllers/condition_types.go b/controllers/condition_types.go
@@ -20,4 +20,5 @@ const (
 	typeValidateDevfileRegistries = "ValidateDevfileRegistries"
 	typeUpdateDevfileRegistries   = "UpdateDevfileRegistries"
 	typeUpdateDevfileRegistry     = "UpdateDevfileRegistry"
+	typeNoDeployDevfileRegistry   = "NoDeployDevfileRegistry"
 )
diff --git a/controllers/devfileregistry_controller.go b/controllers/devfileregistry_controller.go
@@ -75,6 +75,22 @@ func (r *DevfileRegistryReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 		return ctrl.Result{}, err
 	}
 
+	// Block the Devfile Registry deployment if an Ingress domain is missing for Kubernetes
+	if !config.ControllerCfg.IsOpenShift() && registry.IsIngressSkipped(devfileRegistry) {
+		meta.SetStatusCondition(&devfileRegistry.Status.Conditions, metav1.Condition{
+			Type:    typeNoDeployDevfileRegistry,
+			Status:  metav1.ConditionUnknown,
+			Reason:  "DeploymentBlocked",
+			Message: "No Ingress domain set for Devfile Registry - Deployment Blocked",
+		})
+
+		log.Info("Blocked deployment due to unset Ingress domain")
+
+		err = r.Status().Update(ctx, devfileRegistry)
+
+		return ctrl.Result{}, err
+	}
+
 	if devfileRegistry.Status.Conditions == nil || len(devfileRegistry.Status.Conditions) == 0 {
 		err = retry.RetryOnConflict(retry.DefaultRetry, func() error {
 			meta.SetStatusCondition(&devfileRegistry.Status.Conditions, metav1.Condition{
@@ -152,45 +168,36 @@ func (r *DevfileRegistryReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 	} else {
 		// Create/update the ingress for the devfile registry
 		hostname = registry.GetDevfileRegistryIngress(devfileRegistry)
-
-		if !registry.IsIngressSkipped(devfileRegistry) {
-			result, err = r.ensure(ctx, devfileRegistry, &networkingv1.Ingress{}, labels, hostname)
-			if result != nil {
-				return *result, err
-			}
-		} else {
-			log.Info("Ingress creation skipped due to missing IngressDomain field.")
+		result, err = r.ensure(ctx, devfileRegistry, &networkingv1.Ingress{}, labels, hostname)
+		if result != nil {
+			return *result, err
 		}
 	}
 
 	var devfileRegistryServer string
 
-	// When running on K8s and no ingress set we default to http
-	if (!config.ControllerCfg.IsOpenShift() && registry.IsIngressSkipped(devfileRegistry)) || !registry.IsTLSEnabled(devfileRegistry) {
-		devfileRegistryServer = "http://" + hostname
-	} else {
+	if registry.IsTLSEnabled(devfileRegistry) {
 		devfileRegistryServer = "https://" + hostname
+	} else {
+		devfileRegistryServer = "http://" + hostname
 	}
 
-	// Setting the status should be skipped in instances where running on K8s and no ingress is set
-	if !(!config.ControllerCfg.IsOpenShift() && registry.IsIngressSkipped(devfileRegistry)) {
-		if devfileRegistry.Status.URL != devfileRegistryServer {
-			// Check to see if the registry is active, and if so, update the status to reflect the URL
-			// when deploying a new devfile registry, it may not have a signed cert installed yet, so we will skip TLS checking.  We just want to make sure
-			// server is up and running
-			err = util.WaitForServer(devfileRegistryServer, 30*time.Second, false)
-			if err != nil {
-				log.Error(err, "Devfile registry server failed to start after 30 seconds, re-queueing...")
-				return ctrl.Result{Requeue: true}, err
-			}
-
-			// Update the status
-			devfileRegistry.Status.URL = devfileRegistryServer
-			err := r.Status().Update(ctx, devfileRegistry)
-			if err != nil {
-				log.Error(err, "Failed to update DevfileRegistry status")
-				return ctrl.Result{Requeue: true}, err
-			}
+	if devfileRegistry.Status.URL != devfileRegistryServer {
+		// Check to see if the registry is active, and if so, update the status to reflect the URL
+		// when deploying a new devfile registry, it may not have a signed cert installed yet, so we will skip TLS checking.  We just want to make sure
+		// server is up and running
+		err = util.WaitForServer(devfileRegistryServer, 30*time.Second, false)
+		if err != nil {
+			log.Error(err, "Devfile registry server failed to start after 30 seconds, re-queueing...")
+			return ctrl.Result{Requeue: true}, err
+		}
+
+		// Update the status
+		devfileRegistry.Status.URL = devfileRegistryServer
+		err := r.Status().Update(ctx, devfileRegistry)
+		if err != nil {
+			log.Error(err, "Failed to update DevfileRegistry status")
+			return ctrl.Result{Requeue: true}, err
 		}
 	}
 

diff --git a/pkg/registry/ingress.go b/pkg/registry/ingress.go
@@ -72,9 +72,6 @@ func GenerateIngress(cr *registryv1alpha1.DevfileRegistry, host string, scheme *
 }
 
 func GetDevfileRegistryIngress(cr *registryv1alpha1.DevfileRegistry) string {
-	if IsIngressSkipped(cr) {
-		return localHostname
-	}
 	return GetHostname(cr) + "." + cr.Spec.K8s.IngressDomain
 }
 

diff --git a/pkg/registry/ingress_test.go b/pkg/registry/ingress_test.go
@@ -44,16 +44,6 @@ func TestGetDevfileRegistryIngress(t *testing.T) {
 				}},
 			want: "test-name-devfile-registry-test-namespace.my-domain",
 		},
-		{
-			name: "Case 2: Unset Ingress",
-			cr: registryv1alpha1.DevfileRegistry{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "test-name",
-					Namespace: "test-namespace",
-				},
-			},
-			want: localHostname,
-		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {