Skip to content

Commit

Permalink
Handling Missing Ingress Domain for Registry Operator (#89)
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
Jdubrick committed May 21, 2024
1 parent eff2b64 commit 91b7165
Show file tree
Hide file tree
Showing 6 changed files with 74 additions and 2 deletions.
1 change: 1 addition & 0 deletions controllers/condition_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,5 @@ const (
typeValidateDevfileRegistries = "ValidateDevfileRegistries"
typeUpdateDevfileRegistries = "UpdateDevfileRegistries"
typeUpdateDevfileRegistry = "UpdateDevfileRegistry"
typeNoDeployDevfileRegistry = "NoDeployDevfileRegistry"
)
16 changes: 16 additions & 0 deletions controllers/devfileregistry_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
2 changes: 2 additions & 0 deletions pkg/registry/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,5 @@
package registry

const maxTruncLength = 63

const localHostname = "localhost:8080"
9 changes: 9 additions & 0 deletions pkg/registry/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
44 changes: 44 additions & 0 deletions pkg/registry/defaults_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
}

}
4 changes: 2 additions & 2 deletions pkg/registry/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
},
},
})
Expand Down

0 comments on commit 91b7165

Please sign in to comment.