From 91b716565a0da956c0be5699268b60a40632e197 Mon Sep 17 00:00:00 2001
From: Jordan Dubrick <jdubrick@redhat.com>
Date: Tue, 21 May 2024 09:29:34 -0400
Subject: [PATCH] Handling Missing Ingress Domain for Registry Operator (#89)

* add logic for handling missing ingress domain

Signed-off-by: Jordan Dubrick <jdubrick@redhat.com>

* use local hostname if ingress skipped

Signed-off-by: Jordan Dubrick <jdubrick@redhat.com>

* update localhost values to use global var

Signed-off-by: Jordan Dubrick <jdubrick@redhat.com>

* fix typo for using localhost constant

Signed-off-by: Jordan Dubrick <jdubrick@redhat.com>

* remove protocol from local hostname constant

Signed-off-by: Jordan Dubrick <jdubrick@redhat.com>

* create helper func for skipping ingress

Signed-off-by: Jordan Dubrick <jdubrick@redhat.com>

* prefix http protocol to url

Signed-off-by: Jordan Dubrick <jdubrick@redhat.com>

* add logic for skipping checks for missing ingress k8s cases

Signed-off-by: Jordan Dubrick <jdubrick@redhat.com>

* cherry pick remove items & block deployment if ingress unset

Signed-off-by: Jordan Dubrick <jdubrick@redhat.com>

---------

Signed-off-by: Jordan Dubrick <jdubrick@redhat.com>
---
 controllers/condition_types.go            |  1 +
 controllers/devfileregistry_controller.go | 16 +++++++++
 pkg/registry/constants.go                 |  2 ++
 pkg/registry/defaults.go                  |  9 +++++
 pkg/registry/defaults_test.go             | 44 +++++++++++++++++++++++
 pkg/registry/deployment.go                |  4 +--
 6 files changed, 74 insertions(+), 2 deletions(-)

diff --git a/controllers/condition_types.go b/controllers/condition_types.go
index 3e4321f..6682032 100644
--- 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
index a8de52e..ee1b62c 100644
--- 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{
diff --git a/pkg/registry/constants.go b/pkg/registry/constants.go
index 9fdff4c..71fe6ae 100644
--- a/pkg/registry/constants.go
+++ b/pkg/registry/constants.go
@@ -17,3 +17,5 @@
 package registry
 
 const maxTruncLength = 63
+
+const localHostname = "localhost:8080"
diff --git a/pkg/registry/defaults.go b/pkg/registry/defaults.go
index 37b29d7..1ed8908 100644
--- a/pkg/registry/defaults.go
+++ b/pkg/registry/defaults.go
@@ -298,3 +298,12 @@ func getAppFullName(cr *registryv1alpha1.DevfileRegistry) string {
 
 	return truncateName(DefaultAppName)
 }
+
+// IsIngressSkipped returns true if no Ingress is set in the DevfileRegistry CR.
+// If cr does not exist return true by default as no Ingress resource should be created
+func IsIngressSkipped(cr *registryv1alpha1.DevfileRegistry) bool {
+	if cr != nil {
+		return cr.Spec.K8s.IngressDomain == ""
+	}
+	return true
+}
diff --git a/pkg/registry/defaults_test.go b/pkg/registry/defaults_test.go
index 68b98b6..df61de0 100644
--- a/pkg/registry/defaults_test.go
+++ b/pkg/registry/defaults_test.go
@@ -714,3 +714,47 @@ func Test_getAppFullName(t *testing.T) {
 		})
 	}
 }
+
+func TestIsIngressSkipped(t *testing.T) {
+
+	tests := []struct {
+		name string
+		cr   *registryv1alpha1.DevfileRegistry
+		want bool
+	}{
+		{
+			name: "Case 1: Ingress skipped",
+			cr: &registryv1alpha1.DevfileRegistry{
+				Spec: registryv1alpha1.DevfileRegistrySpec{
+					K8s: registryv1alpha1.DevfileRegistrySpecK8sOnly{},
+				},
+			},
+			want: true,
+		},
+		{
+			name: "Case 2: Ingress set",
+			cr: &registryv1alpha1.DevfileRegistry{
+				Spec: registryv1alpha1.DevfileRegistrySpec{
+					K8s: registryv1alpha1.DevfileRegistrySpecK8sOnly{
+						IngressDomain: "test",
+					},
+				},
+			},
+			want: false,
+		},
+		{
+			name: "Case 3: CR is nil",
+			cr:   nil,
+			want: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ingressSkipped := IsIngressSkipped(tt.cr)
+			if ingressSkipped != tt.want {
+				t.Errorf("TestIsIngressSkipped error: value mismatch, expected: %v got: %v", tt.want, ingressSkipped)
+			}
+		})
+	}
+
+}
diff --git a/pkg/registry/deployment.go b/pkg/registry/deployment.go
index 0d12dab..39aebd9 100644
--- a/pkg/registry/deployment.go
+++ b/pkg/registry/deployment.go
@@ -274,10 +274,10 @@ func GenerateDeployment(cr *registryv1alpha1.DevfileRegistry, scheme *runtime.Sc
 					[
 						{
 							"name": "%s",
-							"url": "http://localhost:8080",
+							"url": "http://%s",
 							"fqdn": "%s"
 						}
-					]`, cr.ObjectMeta.Name, cr.Status.URL),
+					]`, cr.ObjectMeta.Name, localHostname, cr.Status.URL),
 				},
 			},
 		})