Skip to content

feat: add support for Ingress .spec.appsDomain #2798

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.

Sign up for GitHub

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

Open
zdtsw wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: add support for Ingress .spec.appsDomain #2798

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions pkg/cluster/cluster_config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
172 changes: 170 additions & 2 deletions pkg/cluster/cluster_config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,22 @@ 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"
"sigs.k8s.io/controller-runtime/pkg/client"
"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.
Expand All @@ -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...)
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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)
}
})
}
}