feat: allowing the configuration of the virtual workspace url directly (#204)

* feat: allowing the configuration of the virtual workspace url directly

* test: fixing tests

Author: nexus49

cmd/listener.go

@@ -2,10 +2,12 @@ package cmd
 
 import (
 	"crypto/tls"
-	"github.com/openmfp/kubernetes-graphql-gateway/listener/discoveryclient"
-	"k8s.io/client-go/discovery"
 	"os"
 
+	"k8s.io/client-go/discovery"
+
+	"github.com/openmfp/kubernetes-graphql-gateway/listener/discoveryclient"
+
 	kcpapis "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha1"
 	kcpcore "github.com/kcp-dev/kcp/sdk/apis/core/v1alpha1"
 	kcptenancy "github.com/kcp-dev/kcp/sdk/apis/tenancy/v1alpha1"
@@ -135,15 +137,7 @@ var listenCmd = &cobra.Command{
 			OpenAPIDefinitionsPath: appCfg.OpenApiDefinitionsPath,
 		}
 
-		reconciler, err := kcp.NewReconciler(
-			ctx,
-			log,
-			appCfg,
-			reconcilerOpts,
-			discoveryInterface,
-			kcp.PreReconcile,
-			discoveryclient.NewFactory,
-		)
+		reconciler, err := kcp.NewReconciler(appCfg, reconcilerOpts, restCfg, discoveryInterface, kcp.PreReconcile, discoveryclient.NewFactory)
 
 		if err != nil {
 			setupLog.Error(err, "unable to instantiate reconciler")

listener/kcp/reconciler_factory.go

@@ -3,7 +3,6 @@ package kcp
 import (
 	"context"
 	"errors"
-	"github.com/openmfp/golang-commons/logger"
 
 	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -49,20 +48,15 @@ type ReconcilerOpts struct {
 	OpenAPIDefinitionsPath string
 }
 
-func NewReconciler(
-	ctx context.Context,
-	log *logger.Logger,
-	appCfg config.Config,
-	opts ReconcilerOpts,
+func NewReconciler(appCfg config.Config, opts ReconcilerOpts, restcfg *rest.Config,
 	discoveryInterface discovery.DiscoveryInterface,
 	preReconcileFunc func(cr *apischema.CRDResolver, io workspacefile.IOHandler) error,
-	discoverFactory func(cfg *rest.Config) (*discoveryclient.FactoryProvider, error),
-) (CustomReconciler, error) {
+	discoverFactory func(cfg *rest.Config) (*discoveryclient.FactoryProvider, error)) (CustomReconciler, error) {
 	if !appCfg.EnableKcp {
 		return newStandardReconciler(opts, discoveryInterface, preReconcileFunc)
 	}
 
-	return newKcpReconciler(ctx, log, appCfg, opts, discoverFactory)
+	return newKcpReconciler(opts, restcfg, discoverFactory)
 }
 
 func newStandardReconciler(
@@ -120,13 +114,7 @@ func PreReconcile(
 	return nil
 }
 
-func newKcpReconciler(
-	ctx context.Context,
-	log *logger.Logger,
-	appCfg config.Config,
-	opts ReconcilerOpts,
-	newDiscoveryFactoryFunc func(cfg *rest.Config) (*discoveryclient.FactoryProvider, error),
-) (CustomReconciler, error) {
+func newKcpReconciler(opts ReconcilerOpts, restcfg *rest.Config, newDiscoveryFactoryFunc func(cfg *rest.Config) (*discoveryclient.FactoryProvider, error)) (CustomReconciler, error) {
 	ioHandler, err := workspacefile.NewIOHandler(opts.OpenAPIDefinitionsPath)
 	if err != nil {
 		return nil, errors.Join(ErrCreateIOHandler, err)
@@ -137,12 +125,7 @@ func newKcpReconciler(
 		return nil, errors.Join(ErrCreatePathResolver, err)
 	}
 
-	virtualWorkspaceCfg, err := virtualWorkspaceConfigFromCfg(ctx, log, appCfg, opts.Config, opts.Client)
-	if err != nil {
-		return nil, errors.Join(ErrGetVWConfig, err)
-	}
-
-	df, err := newDiscoveryFactoryFunc(virtualWorkspaceCfg)
+	df, err := newDiscoveryFactoryFunc(restcfg)
 	if err != nil {
 		return nil, errors.Join(ErrCreateDiscoveryClient, err)
 	}

listener/kcp/reconciler_factory_test.go

@@ -1,15 +1,13 @@
 package kcp
 
 import (
-	"context"
 	"errors"
-	"github.com/openmfp/golang-commons/logger"
+	"path"
+	"testing"
+
 	"github.com/openmfp/kubernetes-graphql-gateway/common/config"
 	"github.com/openmfp/kubernetes-graphql-gateway/listener/clusterpath"
 	"github.com/openmfp/kubernetes-graphql-gateway/listener/kcp/mocks"
-	"github.com/stretchr/testify/require"
-	"path"
-	"testing"
 
 	kcpapis "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha1"
 	"github.com/stretchr/testify/assert"
@@ -73,9 +71,6 @@ func TestNewReconciler(t *testing.T) {
 		},
 	}
 
-	log, err := logger.New(logger.DefaultConfig())
-	require.NoError(t, err)
-
 	for name, tc := range tests {
 		scheme := runtime.NewScheme()
 		assert.NoError(t, kcpapis.AddToScheme(scheme))
@@ -99,25 +94,18 @@ func TestNewReconciler(t *testing.T) {
 				},
 			}...).Build()
 
-			reconciler, err := NewReconciler(
-				context.Background(),
-				log,
-				appCfg,
-				ReconcilerOpts{
-					Config:                 tc.cfg,
-					Scheme:                 scheme,
-					Client:                 fakeClient,
-					OpenAPIDefinitionsPath: tc.definitionsPath,
-				},
-				&mocks.MockDiscoveryInterface{},
-				func(cr *apischema.CRDResolver, io workspacefile.IOHandler) error {
-					return nil
-				},
-				func(cfg *rest.Config) (*discoveryclient.FactoryProvider, error) {
-					return &discoveryclient.FactoryProvider{
-						Config: cfg,
-					}, nil
-				})
+			reconciler, err := NewReconciler(appCfg, ReconcilerOpts{
+				Config:                 tc.cfg,
+				Scheme:                 scheme,
+				Client:                 fakeClient,
+				OpenAPIDefinitionsPath: tc.definitionsPath,
+			}, tc.cfg, &mocks.MockDiscoveryInterface{}, func(cr *apischema.CRDResolver, io workspacefile.IOHandler) error {
+				return nil
+			}, func(cfg *rest.Config) (*discoveryclient.FactoryProvider, error) {
+				return &discoveryclient.FactoryProvider{
+					Config: cfg,
+				}, nil
+			})
 
 			if tc.err != nil {
 				assert.EqualError(t, err, tc.err.Error())

listener/kcp/workspace_config.go

@@ -1,81 +1,15 @@
 package kcp
 
 import (
-	"context"
 	"errors"
-	kcptenancy "github.com/kcp-dev/kcp/sdk/apis/tenancy/v1alpha1"
-	"github.com/openmfp/golang-commons/logger"
 	"net/url"
 	"strings"
-	"time"
-
-	kcpapis "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha1"
-	"k8s.io/client-go/rest"
-	"sigs.k8s.io/controller-runtime/pkg/client"
-
-	"github.com/openmfp/kubernetes-graphql-gateway/common/config"
 )
 
 var (
-	ErrTimeoutFetchingAPIExport = errors.New("timeout fetching APIExport")
-	ErrFailedToGetAPIExport     = errors.New("failed to get APIExport")
-	ErrNoVirtualURLsFound       = errors.New("no virtual URLs found for APIExport")
-	ErrEmptyVirtualWorkspaceURL = errors.New("empty URL in virtual workspace for APIExport")
-	ErrInvalidURL               = errors.New("invalid URL format")
+	ErrInvalidURL = errors.New("invalid URL format")
 )
 
-func virtualWorkspaceConfigFromCfg(
-	ctx context.Context,
-	log *logger.Logger,
-	appCfg config.Config,
-	restCfg *rest.Config,
-	clt client.Client,
-) (*rest.Config, error) {
-	timeOutDuration := 10 * time.Second
-	ctx, cancelFn := context.WithTimeout(ctx, timeOutDuration)
-	defer cancelFn()
-
-	var apiExport kcpapis.APIExport
-	key := client.ObjectKey{
-		Namespace: appCfg.ApiExportWorkspace,
-		Name:      appCfg.ApiExportName,
-	}
-	if err := clt.Get(ctx, key, &apiExport); err != nil {
-		// if this is not a local development, we must have kubernetes.graphql.gateway apiexport
-		if !appCfg.LocalDevelopment {
-			if errors.Is(ctx.Err(), context.DeadlineExceeded) {
-				return nil, errors.Join(ErrTimeoutFetchingAPIExport, err)
-			}
-			return nil, errors.Join(ErrFailedToGetAPIExport, err)
-		}
-
-		// otherwise fallback to the default APIExport, but live ApiBinding watching will not work
-		if err = clt.Get(ctx, client.ObjectKey{Name: kcptenancy.SchemeGroupVersion.Group}, &apiExport); err != nil {
-			return nil, errors.Join(ErrFailedToGetAPIExport, err)
-		}
-
-		log.Warn().Str("apiexport", appCfg.ApiExportName).Msg("failed to find ApiExport, listener will not watch ApiBinding changes in realtime")
-	}
-
-	if len(apiExport.Status.VirtualWorkspaces) == 0 { // nolint: staticcheck
-		return nil, ErrNoVirtualURLsFound
-	}
-
-	virtualWorkspaceURL := apiExport.Status.VirtualWorkspaces[0].URL // nolint: staticcheck
-	if virtualWorkspaceURL == "" {
-		return nil, ErrEmptyVirtualWorkspaceURL
-	}
-
-	internalVirtualWorkspaceURL, err := combineBaseURLAndPath(restCfg.Host, virtualWorkspaceURL)
-	if err != nil {
-		return nil, err
-	}
-
-	restCfg.Host = internalVirtualWorkspaceURL
-
-	return restCfg, nil
-}
-
 func combineBaseURLAndPath(baseURLStr, pathURLStr string) (string, error) {
 	baseURL, err := url.Parse(baseURLStr)
 	if err != nil {

listener/kcp/workspace_config_test.go

@@ -1,131 +1,12 @@
 package kcp
 
 import (
-	"context"
 	"errors"
-	"github.com/openmfp/golang-commons/logger"
-	"github.com/openmfp/kubernetes-graphql-gateway/common/config"
-	"github.com/stretchr/testify/require"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
-	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/client-go/rest"
-	"sigs.k8s.io/controller-runtime/pkg/client"
-	"sigs.k8s.io/controller-runtime/pkg/client/fake"
-
-	kcpapis "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-func TestVirtualWorkspaceConfigFromCfg(t *testing.T) {
-	scheme := runtime.NewScheme()
-	assert.NoError(t, kcpapis.AddToScheme(scheme))
-
-	tests := map[string]struct {
-		clientObjects func(appCfg *config.Config) []client.Object
-		err           error
-	}{
-		"successful_configuration_update": {
-			clientObjects: func(appCfg *config.Config) []client.Object {
-				return []client.Object{
-					&kcpapis.APIExport{
-						ObjectMeta: metav1.ObjectMeta{
-							Namespace: appCfg.ApiExportWorkspace,
-							Name:      appCfg.ApiExportName,
-						},
-						Status: kcpapis.APIExportStatus{
-							VirtualWorkspaces: []kcpapis.VirtualWorkspace{
-								{URL: "https://192.168.1.13:6443/services/apiexport/root/tenancy.kcp.io"},
-							},
-						},
-					},
-				}
-			},
-		},
-		"error_retrieving_APIExport": {
-			err: errors.Join(ErrFailedToGetAPIExport, errors.New("apiexports.apis.kcp.io \"tenancy.kcp.io\" not found")),
-		},
-		"empty_virtual_workspace_list": {
-			clientObjects: func(appCfg *config.Config) []client.Object {
-				return []client.Object{
-					&kcpapis.APIExport{
-						ObjectMeta: metav1.ObjectMeta{
-							Namespace: appCfg.ApiExportWorkspace,
-							Name:      appCfg.ApiExportName,
-						},
-					},
-				}
-			},
-			err: ErrNoVirtualURLsFound,
-		},
-		"empty_virtual_workspace_url": {
-			clientObjects: func(appCfg *config.Config) []client.Object {
-				return []client.Object{
-					&kcpapis.APIExport{
-						ObjectMeta: metav1.ObjectMeta{
-							Namespace: appCfg.ApiExportWorkspace,
-							Name:      appCfg.ApiExportName,
-						},
-						Status: kcpapis.APIExportStatus{
-							VirtualWorkspaces: []kcpapis.VirtualWorkspace{
-								{URL: ""},
-							},
-						},
-					},
-				}
-			},
-			err: ErrEmptyVirtualWorkspaceURL,
-		},
-		"wrong_url_in_virtual_ws": {
-			clientObjects: func(appCfg *config.Config) []client.Object {
-				return []client.Object{
-					&kcpapis.APIExport{
-						ObjectMeta: metav1.ObjectMeta{
-							Namespace: appCfg.ApiExportWorkspace,
-							Name:      appCfg.ApiExportName,
-						},
-						Status: kcpapis.APIExportStatus{
-							VirtualWorkspaces: []kcpapis.VirtualWorkspace{
-								{URL: "ht@tp://bad_url"},
-							},
-						},
-					},
-				}
-			},
-			err: errors.Join(ErrInvalidURL, errors.New("parse \"ht@tp://bad_url\": first path segment in URL cannot contain colon")),
-		},
-	}
-
-	log, err := logger.New(logger.DefaultConfig())
-	require.NoError(t, err)
-
-	for name, tc := range tests {
-		t.Run(name, func(t *testing.T) {
-			appCfg, err := config.NewFromEnv()
-			assert.NoError(t, err)
-			appCfg.LocalDevelopment = true
-
-			fakeClientBuilder := fake.NewClientBuilder().WithScheme(scheme)
-			if tc.clientObjects != nil {
-				fakeClientBuilder.WithObjects(tc.clientObjects(&appCfg)...)
-			}
-			fakeClient := fakeClientBuilder.Build()
-
-			resultCfg, err := virtualWorkspaceConfigFromCfg(context.Background(), log, appCfg, &rest.Config{Host: validAPIServerHost}, fakeClient)
-
-			if tc.err != nil {
-				// here it fails
-				assert.EqualError(t, err, tc.err.Error())
-				assert.Nil(t, resultCfg)
-			} else {
-				assert.NoError(t, err)
-				assert.Equal(t, tc.clientObjects(&appCfg)[0].(*kcpapis.APIExport).Status.VirtualWorkspaces[0].URL, resultCfg.Host) // nolint: staticcheck
-			}
-		})
-	}
-}
-
 func TestCombineBaseURLAndPath(t *testing.T) {
 	tests := []struct {
 		name     string