From 2c937e7a8d8c5b88f53a4d42da038dd9197a2eff Mon Sep 17 00:00:00 2001 From: Wen Zhou Date: Mon, 3 Nov 2025 18:38:46 +0100 Subject: [PATCH] feat: add support for Ingress .spec.appsDomain - if user set appsDomain in default Ingress CR, we will use it as domain for ODH - otherwise fall back to the default domain value - we could consider even to have componentRoutes in the future Signed-off-by: Wen Zhou (cherry picked from commit 757912c2c05a2c0e278fae2bde2f774d440d61ff) --- pkg/cluster/cluster_config.go | 18 ++- pkg/cluster/cluster_config_test.go | 172 ++++++++++++++++++++++++++++- 2 files changed, 185 insertions(+), 5 deletions(-) diff --git a/pkg/cluster/cluster_config.go b/pkg/cluster/cluster_config.go index 12411671bab4..118f84d2de85 100644 --- a/pkg/cluster/cluster_config.go +++ b/pkg/cluster/cluster_config.go @@ -154,12 +154,24 @@ func GetDomain(ctx context.Context, c client.Client) (string, error) { return "", fmt.Errorf("failed fetching cluster's ingress details: %w", err) } + // add support for appsDomain overwrite default domain + appsDomain, found, err := unstructured.NestedString(ingress.Object, "spec", "appsDomain") + if err != nil { + return "", fmt.Errorf("failed to read spec.appsDomain: %w", err) + } + if found && len(appsDomain) > 0 { + return appsDomain, nil + } + domain, found, err := unstructured.NestedString(ingress.Object, "spec", "domain") - if !found { - return "", errors.New("spec.domain not found") + if err != nil { + return "", fmt.Errorf("failed to read spec.domain: %w", err) + } + if !found || len(domain) == 0 { + return "", errors.New("spec.domain not found or empty") } - return domain, err + return domain, nil } // This is an Openshift specific implementation. diff --git a/pkg/cluster/cluster_config_test.go b/pkg/cluster/cluster_config_test.go index fdcee2ca0a7b..1b6ca8bcefe1 100644 --- a/pkg/cluster/cluster_config_test.go +++ b/pkg/cluster/cluster_config_test.go @@ -3,12 +3,14 @@ package cluster_test import ( "context" "errors" + "strings" "testing" configv1 "github.com/openshift/api/config/v1" corev1 "k8s.io/api/core/v1" k8serr "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" @@ -16,6 +18,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster/gvk" ) // erroringClient is a wrapper around a client.Client that allows us to inject errors. @@ -26,7 +29,7 @@ type erroringClient struct { } func (c *erroringClient) Get(ctx context.Context, key types.NamespacedName, obj client.Object, opts ...client.GetOption) error { - if key.Name == "cluster-config-v1" { + if key.Name == "cluster-config-v1" || key.Name == "cluster" { return c.err } return c.Client.Get(ctx, key, obj, opts...) @@ -406,7 +409,6 @@ func TestIsIntegratedOAuth(t *testing.T) { expectedError: true, }, } - for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { ctx := context.Background() @@ -436,3 +438,169 @@ func TestIsIntegratedOAuth(t *testing.T) { }) } } + +func TestGetDomain(t *testing.T) { + testCases := []struct { + name string + ingress *unstructured.Unstructured + clientErr error + expectedDomain string + expectError bool + errorContains string + }{ + { + name: "appsDomain takes precedence over domain", + ingress: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "config.openshift.io/v1", + "kind": "Ingress", + "metadata": map[string]interface{}{ + "name": "cluster", + }, + "spec": map[string]interface{}{ + "appsDomain": "apps.custom.example.com", + "domain": "example.com", + }, + }, + }, + expectedDomain: "apps.custom.example.com", + expectError: false, + }, + { + name: "appsDomain empty string falls back to domain", + ingress: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "config.openshift.io/v1", + "kind": "Ingress", + "metadata": map[string]interface{}{ + "name": "cluster", + }, + "spec": map[string]interface{}{ + "appsDomain": "", + "domain": "example.com", + }, + }, + }, + expectedDomain: "example.com", + expectError: false, + }, + { + name: "appsDomain not set falls back to domain", + ingress: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "config.openshift.io/v1", + "kind": "Ingress", + "metadata": map[string]interface{}{ + "name": "cluster", + }, + "spec": map[string]interface{}{ + "domain": "example.com", + }, + }, + }, + expectedDomain: "example.com", + expectError: false, + }, + { + name: "only domain field present", + ingress: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "config.openshift.io/v1", + "kind": "Ingress", + "metadata": map[string]interface{}{ + "name": "cluster", + }, + "spec": map[string]interface{}{ + "domain": "example.com", + }, + }, + }, + expectedDomain: "example.com", + expectError: false, + }, + { + name: "domain field missing returns error", + ingress: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "config.openshift.io/v1", + "kind": "Ingress", + "metadata": map[string]interface{}{ + "name": "cluster", + }, + "spec": map[string]interface{}{}, + }, + }, + expectError: true, + errorContains: "spec.domain not found or empty", + }, + { + name: "domain field empty string returns error", + ingress: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "config.openshift.io/v1", + "kind": "Ingress", + "metadata": map[string]interface{}{ + "name": "cluster", + }, + "spec": map[string]interface{}{ + "domain": "", + }, + }, + }, + expectError: true, + errorContains: "spec.domain not found or empty", + }, + { + name: "ingress not found returns error", + clientErr: k8serr.NewNotFound(schema.GroupResource{Group: "config.openshift.io", Resource: "ingresses"}, "cluster"), + expectError: true, + errorContains: "failed fetching cluster's ingress details", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + // Create a fake client + var fakeClient client.Client + objs := []runtime.Object{} + + if tc.ingress != nil { + tc.ingress.SetGroupVersionKind(gvk.OpenshiftIngress) + objs = append(objs, tc.ingress) + } + + fakeClient = fake.NewClientBuilder().WithRuntimeObjects(objs...).Build() + + if tc.clientErr != nil { + fakeClient = &erroringClient{ + Client: fakeClient, + err: tc.clientErr, + } + } + + // Call the function under test + ctx := context.Background() + domain, err := cluster.GetDomain(ctx, fakeClient) + + // Check the error + if tc.expectError { + if err == nil { + t.Errorf("GetDomain() expected error but got nil") + } else if tc.errorContains != "" { + if !strings.Contains(err.Error(), tc.errorContains) { + t.Errorf("GetDomain() error = %v, want error containing %q", err, tc.errorContains) + } + } + } else { + if err != nil { + t.Errorf("GetDomain() unexpected error = %v", err) + } + } + + // Check the domain result + if !tc.expectError && domain != tc.expectedDomain { + t.Errorf("GetDomain() = %q, want %q", domain, tc.expectedDomain) + } + }) + } +}