Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handling Missing Ingress Domain for Registry Operator #89

Merged
merged 9 commits into from
May 21, 2024
53 changes: 31 additions & 22 deletions controllers/devfileregistry_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,36 +152,45 @@ func (r *DevfileRegistryReconciler) Reconcile(ctx context.Context, req ctrl.Requ
} else {
// Create/update the ingress for the devfile registry
hostname = registry.GetDevfileRegistryIngress(devfileRegistry)
result, err = r.ensure(ctx, devfileRegistry, &networkingv1.Ingress{}, labels, hostname)
if result != nil {
return *result, err

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.")
}
}

var devfileRegistryServer string

if registry.IsTLSEnabled(devfileRegistry) {
devfileRegistryServer = "https://" + hostname
} else {
// 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 {
devfileRegistryServer = "https://" + hostname
}

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

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
3 changes: 3 additions & 0 deletions pkg/registry/ingress.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,9 @@ 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
}

Expand Down
10 changes: 10 additions & 0 deletions pkg/registry/ingress_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,16 @@ 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) {
Expand Down
Loading