diff --git a/cmd/listener.go b/cmd/listener.go index d1d0982..937484c 100644 --- a/cmd/listener.go +++ b/cmd/listener.go @@ -16,6 +16,7 @@ import ( clientgoscheme "k8s.io/client-go/kubernetes/scheme" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/healthz" "sigs.k8s.io/controller-runtime/pkg/log/zap" "sigs.k8s.io/controller-runtime/pkg/metrics/filters" @@ -97,9 +98,19 @@ var listenCmd = &cobra.Command{ LeaderElectionID: "72231e1f.openmfp.io", } - newMgrFunc := kcp.ManagerFactory(appCfg) + clt, err := client.New(cfg, client.Options{ + Scheme: scheme, + }) + if err != nil { + setupLog.Error(err, "failed to create client from config") + os.Exit(1) + } - mgr, err := newMgrFunc(cfg, mgrOpts) + mf := &kcp.ManagerFactory{ + IsKCPEnabled: appCfg.EnableKcp, + } + + mgr, err := mf.NewManager(cfg, mgrOpts, clt) if err != nil { setupLog.Error(err, "unable to start manager") os.Exit(1) @@ -107,13 +118,12 @@ var listenCmd = &cobra.Command{ reconcilerOpts := kcp.ReconcilerOpts{ Scheme: scheme, + Client: clt, Config: cfg, OpenAPIDefinitionsPath: appCfg.OpenApiDefinitionsPath, } - newReconcilerFunc := kcp.ReconcilerFactory(appCfg) - - reconciler, err := newReconcilerFunc(reconcilerOpts) + reconciler, err := kcp.NewReconcilerFactory(appCfg).NewReconciler(reconcilerOpts) if err != nil { setupLog.Error(err, "unable to instantiate reconciler") os.Exit(1) diff --git a/design_assets/Listener_High_Level.drawio.svg b/design_assets/Listener_High_Level.drawio.svg new file mode 100644 index 0000000..b718bf3 --- /dev/null +++ b/design_assets/Listener_High_Level.drawio.svg @@ -0,0 +1,4 @@ + + + +
APIServer
APIServer
apiVersion: apis.kcp.io/v1alpha1
kind: APIBinding
metadata:
name: core.openmfp.io
spec:
reference:
export:
name: core.openmfp.io
path: root
apiVersion: apis.kcp.io/v1alpha1...
Listener
Listener
APISchemaResolver
APISchemaResolver
Uses
Uses
Uses
Uses
APIBindingReconciler
APIBindingReconciler
IOHandler
IOHandler
Uses
Uses
CRDReconciler
CRDReconciler
Uses
Uses
Reconciles
Reconciles
Resolves
Resolves
{
  "definitions": {
    "io.k8s.apimachinery.pkg.apis.meta.v1.FieldsV1": {
      "description": "...",
      "type": "object"
    },
{...
JSON File
JSON File
Queries
Queries
Writes
Writes
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
name: issuers.cert-manager.io
spec:
group: cert-manager.io
names:
kind: Issuer
listKind: IssuerList
plural: issuers
singular: issuer
categories:
- cert-manager
scope: Namespaced
versions:
- name: v1
......
apiVersion: apiextensions.k8s.io/v1...
Gateway
Gateway
Loads
LoadsText is not SVG - cannot display \ No newline at end of file diff --git a/listener/apischema/crd_resolver.go b/listener/apischema/crd_resolver.go index 1a4f63d..0e13acf 100644 --- a/listener/apischema/crd_resolver.go +++ b/listener/apischema/crd_resolver.go @@ -29,12 +29,12 @@ type GroupKindVersions struct { } type CRDResolver struct { - *discovery.DiscoveryClient + discovery.DiscoveryInterface meta.RESTMapper } func (cr *CRDResolver) Resolve() ([]byte, error) { - return resolveSchema(cr.DiscoveryClient, cr.RESTMapper) + return resolveSchema(cr.DiscoveryInterface, cr.RESTMapper) } func (cr *CRDResolver) ResolveApiSchema(crd *apiextensionsv1.CustomResourceDefinition) ([]byte, error) { diff --git a/listener/clusterpath/resolver.go b/listener/clusterpath/resolver.go index 5366b1c..2191a1c 100644 --- a/listener/clusterpath/resolver.go +++ b/listener/clusterpath/resolver.go @@ -12,36 +12,44 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) +var ( + errNilConfig = errors.New("config should not be nil") + errNilScheme = errors.New("scheme should not be nil") +) + +type clientFactory func(config *rest.Config, options client.Options) (client.Client, error) + type Resolver struct { *runtime.Scheme *rest.Config - ResolverFunc + clientFactory } -type ResolverFunc func(name string, cfg *rest.Config, scheme *runtime.Scheme) (string, error) - -func Resolve(name string, cfg *rest.Config, scheme *runtime.Scheme) (string, error) { - if name == "root" { - return name, nil - } +func NewResolver(cfg *rest.Config, scheme *runtime.Scheme) (*Resolver, error) { if cfg == nil { - return "", errors.New("config should not be nil") + return nil, errNilConfig } if scheme == nil { - return "", errors.New("scheme should not be nil") + return nil, errNilScheme } - clusterCfg := rest.CopyConfig(cfg) - clusterCfgURL, err := url.Parse(clusterCfg.Host) + return &Resolver{ + Scheme: scheme, + Config: cfg, + clientFactory: client.New, + }, nil +} + +func (rf *Resolver) ClientForCluster(name string) (client.Client, error) { + clusterConfig, err := getClusterConfig(name, rf.Config) if err != nil { - return "", fmt.Errorf("failed to parse rest config Host URL: %w", err) + return nil, fmt.Errorf("failed to get cluster config: %w", err) } - clusterCfgURL.Path = fmt.Sprintf("/clusters/%s", name) - clusterCfg.Host = clusterCfgURL.String() - clt, err := client.New(clusterCfg, client.Options{ - Scheme: scheme, - }) - if err != nil { - return "", fmt.Errorf("failed to create client for cluster: %w", err) + return rf.clientFactory(clusterConfig, client.Options{Scheme: rf.Scheme}) +} + +func PathForCluster(name string, clt client.Client) (string, error) { + if name == "root" { + return name, nil } lc := &kcpcore.LogicalCluster{} if err := clt.Get(context.TODO(), client.ObjectKey{Name: "cluster"}, lc); err != nil { @@ -53,3 +61,17 @@ func Resolve(name string, cfg *rest.Config, scheme *runtime.Scheme) (string, err } return path, nil } + +func getClusterConfig(name string, cfg *rest.Config) (*rest.Config, error) { + if cfg == nil { + return nil, errors.New("config should not be nil") + } + clusterCfg := rest.CopyConfig(cfg) + clusterCfgURL, err := url.Parse(clusterCfg.Host) + if err != nil { + return nil, fmt.Errorf("failed to parse rest config's Host URL: %w", err) + } + clusterCfgURL.Path = fmt.Sprintf("/clusters/%s", name) + clusterCfg.Host = clusterCfgURL.String() + return clusterCfg, nil +} diff --git a/listener/clusterpath/resolver_test.go b/listener/clusterpath/resolver_test.go new file mode 100644 index 0000000..1887b9f --- /dev/null +++ b/listener/clusterpath/resolver_test.go @@ -0,0 +1,157 @@ +package clusterpath + +import ( + "net/url" + "testing" + + kcpcore "github.com/kcp-dev/kcp/sdk/apis/core/v1alpha1" + "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" +) + +func TestResolver(t *testing.T) { + tests := map[string]struct { + baseConfig *rest.Config + clusterName string + expectErr bool + }{ + "valid_cluster": {baseConfig: &rest.Config{}, clusterName: "test-cluster", expectErr: false}, + "nil_base_config": {baseConfig: nil, clusterName: "test-cluster", expectErr: true}, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + resolver := &Resolver{ + Scheme: runtime.NewScheme(), + Config: tc.baseConfig, + clientFactory: func(config *rest.Config, options client.Options) (client.Client, error) { + return fake.NewClientBuilder().WithScheme(options.Scheme).Build(), nil + }, + } + + client, err := resolver.ClientForCluster(tc.clusterName) + if tc.expectErr { + assert.Error(t, err) + return + } + assert.NoError(t, err) + assert.NotNil(t, client) + + }) + } +} + +func TestPathForCluster(t *testing.T) { + scheme := runtime.NewScheme() + err := kcpcore.AddToScheme(scheme) + assert.NoError(t, err) + tests := map[string]struct { + clusterName string + annotations map[string]string + expectErr bool + expectedPath string + }{ + "root_cluster": { + clusterName: "root", + annotations: nil, + expectErr: false, + expectedPath: "root", + }, + "valid_cluster_with_1st_level_path": { + clusterName: "sap", + annotations: map[string]string{"kcp.io/path": "root:sap"}, + expectErr: false, + expectedPath: "root:sap", + }, + "valid_cluster_with_2nd_level_path": { + clusterName: "openmfp", + annotations: map[string]string{"kcp.io/path": "root:sap:openmfp"}, + expectErr: false, + expectedPath: "root:sap:openmfp", + }, + "missing_annotation": { + clusterName: "test-cluster", + annotations: map[string]string{}, + expectErr: true, + }, + "nil_annotation": { + clusterName: "test-cluster", + annotations: nil, + expectErr: true, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + builder := fake.NewClientBuilder().WithScheme(scheme) + if tc.annotations != nil { + lc := &kcpcore.LogicalCluster{} + lc.SetName("cluster") + lc.SetAnnotations(tc.annotations) + builder = builder.WithObjects(lc) + } + clt := builder.Build() + + path, err := PathForCluster(tc.clusterName, clt) + if tc.expectErr { + assert.Error(t, err) + assert.Empty(t, path) + return + } + assert.NoError(t, err) + assert.Equal(t, tc.expectedPath, path) + + }) + } +} + +func TestGetClusterConfig(t *testing.T) { + tests := map[string]struct { + cfg *rest.Config + cluster string + expect *rest.Config + expectErr bool + }{ + "nil_config": { + cfg: nil, + cluster: "openmfp", + expect: nil, + expectErr: true, + }, + "valid_config": { + cfg: &rest.Config{Host: "https://127.0.0.1:56120/clusters/root"}, + cluster: "openmfp", + expect: &rest.Config{Host: "https://127.0.0.1:56120/clusters/openmfp"}, + expectErr: false, + }, + "invalid_URL": { + cfg: &rest.Config{Host: ":://bad-url"}, + cluster: "openmfp", + expect: nil, + expectErr: true, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + got, err := getClusterConfig(tc.cluster, tc.cfg) + if tc.expectErr { + assert.Error(t, err) + return + } + assert.NoError(t, err) + assert.NotNil(t, got) + assert.Equal(t, tc.expect.Host, got.Host) + parsedURL, err1 := url.Parse(got.Host) + assert.NoError(t, err1) + assert.NotEmpty(t, parsedURL) + expectedURL, err2 := url.Parse(tc.expect.Host) + assert.NoError(t, err2) + assert.NotEmpty(t, expectedURL) + assert.Equal(t, expectedURL, parsedURL) + }) + } +} diff --git a/listener/controller/apibinding_controller.go b/listener/controller/apibinding_controller.go index 786987e..f62f2e9 100644 --- a/listener/controller/apibinding_controller.go +++ b/listener/controller/apibinding_controller.go @@ -21,15 +21,15 @@ import ( // APIBindingReconciler reconciles an APIBinding object type APIBindingReconciler struct { - io workspacefile.IOHandler - df discoveryclient.Factory + io *workspacefile.IOHandler + df *discoveryclient.Factory sc apischema.Resolver pr *clusterpath.Resolver } func NewAPIBindingReconciler( - io workspacefile.IOHandler, - df discoveryclient.Factory, + io *workspacefile.IOHandler, + df *discoveryclient.Factory, sc apischema.Resolver, pr *clusterpath.Resolver, ) *APIBindingReconciler { @@ -48,7 +48,12 @@ func (r *APIBindingReconciler) Reconcile(ctx context.Context, req ctrl.Request) } logger := log.FromContext(ctx) - clusterPath, err := r.pr.ResolverFunc(req.ClusterName, r.pr.Config, r.pr.Scheme) + clusterClt, err := r.pr.ClientForCluster(req.ClusterName) + if err != nil { + logger.Error(err, "failed to get cluster client", "cluster", req.ClusterName) + return ctrl.Result{}, err + } + clusterPath, err := clusterpath.PathForCluster(req.ClusterName, clusterClt) if err != nil { logger.Error(err, "failed to get cluster path", "cluster", req.ClusterName) return ctrl.Result{}, err diff --git a/listener/controller/crd_controller.go b/listener/controller/crd_controller.go index 9da2ee9..a12b2cb 100644 --- a/listener/controller/crd_controller.go +++ b/listener/controller/crd_controller.go @@ -20,13 +20,13 @@ type CRDReconciler struct { ClusterName string client.Client *apischema.CRDResolver - io workspacefile.IOHandler + io *workspacefile.IOHandler } func NewCRDReconciler(name string, clt client.Client, cr *apischema.CRDResolver, - io workspacefile.IOHandler, + io *workspacefile.IOHandler, ) *CRDReconciler { return &CRDReconciler{ ClusterName: name, diff --git a/listener/discoveryclient/factory.go b/listener/discoveryclient/factory.go index 1c3c47f..6cce14e 100644 --- a/listener/discoveryclient/factory.go +++ b/listener/discoveryclient/factory.go @@ -11,32 +11,37 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/apiutil" ) -type Factory interface { - ClientForCluster(name string) (*discovery.DiscoveryClient, error) - RestMapperForCluster(name string) (meta.RESTMapper, error) +type NewDiscoveryIFFunc func(cfg *rest.Config) (discovery.DiscoveryInterface, error) + +func discoveryCltFactory(cfg *rest.Config) (discovery.DiscoveryInterface, error) { + return discovery.NewDiscoveryClientForConfig(cfg) } -type FactoryImpl struct { - restCfg *rest.Config +type Factory struct { + *rest.Config + NewDiscoveryIFFunc } -func NewFactory(cfg *rest.Config) (*FactoryImpl, error) { +func NewFactory(cfg *rest.Config) (*Factory, error) { if cfg == nil { return nil, errors.New("config should not be nil") } - return &FactoryImpl{restCfg: cfg}, nil + return &Factory{ + Config: cfg, + NewDiscoveryIFFunc: discoveryCltFactory, + }, nil } -func (f *FactoryImpl) ClientForCluster(name string) (*discovery.DiscoveryClient, error) { - clusterCfg, err := configForCluster(name, f.restCfg) +func (f *Factory) ClientForCluster(name string) (discovery.DiscoveryInterface, error) { + clusterCfg, err := configForCluster(name, f.Config) if err != nil { return nil, fmt.Errorf("failed to get rest config for cluster: %w", err) } - return discovery.NewDiscoveryClientForConfig(clusterCfg) + return f.NewDiscoveryIFFunc(clusterCfg) } -func (f *FactoryImpl) RestMapperForCluster(name string) (meta.RESTMapper, error) { - clusterCfg, err := configForCluster(name, f.restCfg) +func (f *Factory) RestMapperForCluster(name string) (meta.RESTMapper, error) { + clusterCfg, err := configForCluster(name, f.Config) if err != nil { return nil, fmt.Errorf("failed to get rest config for cluster: %w", err) } @@ -51,7 +56,7 @@ func configForCluster(name string, cfg *rest.Config) (*rest.Config, error) { clusterCfg := rest.CopyConfig(cfg) clusterCfgURL, err := url.Parse(clusterCfg.Host) if err != nil { - return nil, fmt.Errorf("failed to parse rest config Host URL: %w", err) + return nil, fmt.Errorf("failed to parse rest config's Host URL: %w", err) } clusterCfgURL.Path = fmt.Sprintf("/clusters/%s", name) clusterCfg.Host = clusterCfgURL.String() diff --git a/listener/discoveryclient/factory_test.go b/listener/discoveryclient/factory_test.go new file mode 100644 index 0000000..a2d8764 --- /dev/null +++ b/listener/discoveryclient/factory_test.go @@ -0,0 +1,98 @@ +package discoveryclient + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "k8s.io/client-go/discovery" + fakediscovery "k8s.io/client-go/discovery/fake" + fakeclientset "k8s.io/client-go/kubernetes/fake" + "k8s.io/client-go/rest" +) + +func TestNewFactory(t *testing.T) { + tests := map[string]struct { + inputCfg *rest.Config + expectErr bool + }{ + "valid_config": {inputCfg: &rest.Config{}, expectErr: false}, + "nil_config": {inputCfg: nil, expectErr: true}, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + factory, err := NewFactory(tc.inputCfg) + if tc.expectErr { + assert.Error(t, err) + return + } + assert.NoError(t, err) + assert.NotNil(t, factory) + assert.Equal(t, factory.Config, tc.inputCfg) + }) + } +} + +func TestClientForCluster(t *testing.T) { + tests := map[string]struct { + clusterName string + restCfg *rest.Config + expectErr bool + }{ + "invalid_config": {clusterName: "test-cluster", restCfg: &rest.Config{Host: "://192.168.1.13:6443"}, expectErr: true}, + "valid_config": {clusterName: "test-cluster", restCfg: &rest.Config{Host: "https://192.168.1.13:6443"}, expectErr: false}, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + factory := &Factory{ + Config: tc.restCfg, + NewDiscoveryIFFunc: fakeClientFactory, + } + dc, err := factory.ClientForCluster(tc.clusterName) + if tc.expectErr { + assert.Error(t, err) + return + } + assert.NoError(t, err) + assert.NotNil(t, dc) + }) + } +} + +func TestRestMapperForCluster(t *testing.T) { + tests := map[string]struct { + clusterName string + restCfg *rest.Config + expectErr bool + }{ + "invalid_config": {clusterName: "test-cluster", restCfg: &rest.Config{Host: "://192.168.1.13:6443"}, expectErr: true}, + "valid_config": {clusterName: "test-cluster", restCfg: &rest.Config{Host: "https://192.168.1.13:6443"}, expectErr: false}, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + factory := &Factory{ + Config: tc.restCfg, + NewDiscoveryIFFunc: fakeClientFactory, + } + rm, err := factory.RestMapperForCluster(tc.clusterName) + if tc.expectErr { + assert.Error(t, err) + return + } + assert.NoError(t, err) + assert.NotNil(t, rm) + }) + } +} + +func fakeClientFactory(_ *rest.Config) (discovery.DiscoveryInterface, error) { + client := fakeclientset.NewClientset() + fakeDiscovery, ok := client.Discovery().(*fakediscovery.FakeDiscovery) + if !ok { + return nil, errors.New("failed to get fake discovery client") + } + return fakeDiscovery, nil +} diff --git a/listener/kcp/manager_factory.go b/listener/kcp/manager_factory.go index 8778431..f81d561 100644 --- a/listener/kcp/manager_factory.go +++ b/listener/kcp/manager_factory.go @@ -5,29 +5,22 @@ import ( "k8s.io/client-go/rest" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" kcpctrl "sigs.k8s.io/controller-runtime/pkg/kcp" "sigs.k8s.io/controller-runtime/pkg/manager" - - "github.com/openmfp/kubernetes-graphql-gateway/common/config" ) -type NewManagerFunc func(cfg *rest.Config, opts ctrl.Options) (manager.Manager, error) - -func ManagerFactory(opFlags *config.Config) NewManagerFunc { - if opFlags.EnableKcp { - return NewKcpManager - } - return ctrl.NewManager +type ManagerFactory struct { + IsKCPEnabled bool } -func NewKcpManager(cfg *rest.Config, opts ctrl.Options) (manager.Manager, error) { - virtualWorkspaceCfg, err := virtualWorkspaceConfigFromCfg(cfg, opts.Scheme) - if err != nil { - return nil, fmt.Errorf("unable to get virtual workspace config: %w", err) +func (f *ManagerFactory) NewManager(cfg *rest.Config, opts ctrl.Options, clt client.Client) (manager.Manager, error) { + if !f.IsKCPEnabled { + return ctrl.NewManager(cfg, opts) } - mgr, err := kcpctrl.NewClusterAwareManager(virtualWorkspaceCfg, opts) + virtualWorkspaceCfg, err := virtualWorkspaceConfigFromCfg(cfg, clt) if err != nil { - return nil, fmt.Errorf("unable to instantiate manager: %w", err) + return nil, fmt.Errorf("unable to get virtual workspace config: %w", err) } - return mgr, nil + return kcpctrl.NewClusterAwareManager(virtualWorkspaceCfg, opts) } diff --git a/listener/kcp/manager_factory_test.go b/listener/kcp/manager_factory_test.go new file mode 100644 index 0000000..7398dd6 --- /dev/null +++ b/listener/kcp/manager_factory_test.go @@ -0,0 +1,62 @@ +package kcp + +import ( + "testing" + + kcpapis "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha1" + "github.com/stretchr/testify/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/rest" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +func TestNewManager(t *testing.T) { + + tests := map[string]struct { + cfg *rest.Config + isKCPEnabled bool + expectErr bool + }{ + "successful_KCP_manager_creation": {cfg: &rest.Config{Host: validAPIServerHost}, isKCPEnabled: true, expectErr: false}, + "error_from_virtualWorkspaceConfigFromCfg": {cfg: &rest.Config{Host: schemelessAPIServerHost}, isKCPEnabled: true, expectErr: true}, + "error_from_NewClusterAwareManager": {cfg: &rest.Config{}, isKCPEnabled: true, expectErr: true}, + "successful_manager_creation": {cfg: &rest.Config{Host: validAPIServerHost}, isKCPEnabled: false, expectErr: false}, + } + + for name, tc := range tests { + scheme := runtime.NewScheme() + err := kcpapis.AddToScheme(scheme) + assert.NoError(t, err) + t.Run(name, func(t *testing.T) { + + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects([]client.Object{ + &kcpapis.APIExport{ + ObjectMeta: metav1.ObjectMeta{Name: tenancyAPIExportName}, + Status: kcpapis.APIExportStatus{ + VirtualWorkspaces: []kcpapis.VirtualWorkspace{ + {URL: validAPIServerHost}, + }, + }, + }, + }...).Build() + f := &ManagerFactory{ + IsKCPEnabled: tc.isKCPEnabled, + } + mgr, err := f.NewManager(tc.cfg, ctrl.Options{ + Scheme: scheme, + }, fakeClient) + + if tc.expectErr { + assert.Error(t, err) + assert.Nil(t, mgr) + return + } + + assert.NoError(t, err) + assert.NotNil(t, mgr) + }) + } +} diff --git a/listener/kcp/reconciler_factory.go b/listener/kcp/reconciler_factory.go index 66da62f..f7d52ea 100644 --- a/listener/kcp/reconciler_factory.go +++ b/listener/kcp/reconciler_factory.go @@ -30,26 +30,45 @@ type CustomReconciler interface { type ReconcilerOpts struct { *rest.Config *runtime.Scheme + client.Client OpenAPIDefinitionsPath string } -type NewReconcilerFunc func(opts ReconcilerOpts) (CustomReconciler, error) +type newDiscoveryFactoryFunc func(cfg *rest.Config) (*discoveryclient.Factory, error) -func ReconcilerFactory(opFlags *config.Config) NewReconcilerFunc { - if opFlags.EnableKcp { - return NewKcpReconciler +type preReconcileFunc func(cr *apischema.CRDResolver, io *workspacefile.IOHandler) error + +type newDiscoveryIFFunc func(cfg *rest.Config) (discovery.DiscoveryInterface, error) + +func discoveryCltFactory(cfg *rest.Config) (discovery.DiscoveryInterface, error) { + return discovery.NewDiscoveryClientForConfig(cfg) +} + +type ReconcilerFactory struct { + IsKCPEnabled bool + newDiscoveryIFFunc + preReconcileFunc + newDiscoveryFactoryFunc +} + +func NewReconcilerFactory(opFlags *config.Config) *ReconcilerFactory { + return &ReconcilerFactory{ + IsKCPEnabled: opFlags.EnableKcp, + newDiscoveryIFFunc: discoveryCltFactory, + preReconcileFunc: preReconcile, + newDiscoveryFactoryFunc: discoveryclient.NewFactory, } - return NewReconciler } -func NewReconciler(opts ReconcilerOpts) (CustomReconciler, error) { - clt, err := client.New(opts.Config, client.Options{ - Scheme: opts.Scheme, - }) - if err != nil { - return nil, fmt.Errorf("failed to create client: %w", err) +func (f *ReconcilerFactory) NewReconciler(opts ReconcilerOpts) (CustomReconciler, error) { + if !f.IsKCPEnabled { + return f.newStdReconciler(opts) } - dc, err := discovery.NewDiscoveryClientForConfig(opts.Config) + return f.newKcpReconciler(opts) +} + +func (f *ReconcilerFactory) newStdReconciler(opts ReconcilerOpts) (CustomReconciler, error) { + dc, err := f.newDiscoveryIFFunc(opts.Config) if err != nil { return nil, fmt.Errorf("failed to create discovery client: %w", err) } @@ -65,15 +84,16 @@ func NewReconciler(opts ReconcilerOpts) (CustomReconciler, error) { } schemaResolver := &apischema.CRDResolver{ - DiscoveryClient: dc, - RESTMapper: rm, + DiscoveryInterface: dc, + RESTMapper: rm, } - if err := preReconcile(schemaResolver, ioHandler); err != nil { + if err := f.preReconcileFunc(schemaResolver, ioHandler); err != nil { return nil, fmt.Errorf("failed to generate OpenAPI Schema for cluster: %w", err) } - return controller.NewCRDReconciler(kubernetesClusterName, clt, schemaResolver, ioHandler), nil + return controller.NewCRDReconciler(kubernetesClusterName, opts.Client, schemaResolver, ioHandler), nil + } func restMapperFromConfig(cfg *rest.Config) (meta.RESTMapper, error) { @@ -90,7 +110,7 @@ func restMapperFromConfig(cfg *rest.Config) (meta.RESTMapper, error) { func preReconcile( cr *apischema.CRDResolver, - io workspacefile.IOHandler, + io *workspacefile.IOHandler, ) error { JSON, err := cr.Resolve() if err != nil { @@ -102,26 +122,24 @@ func preReconcile( return nil } -func NewKcpReconciler(opts ReconcilerOpts) (CustomReconciler, error) { - virtualWorkspaceCfg, err := virtualWorkspaceConfigFromCfg(opts.Config, opts.Scheme) - if err != nil { - return nil, fmt.Errorf("unable to get virtual workspace config: %w", err) - } +func (f *ReconcilerFactory) newKcpReconciler(opts ReconcilerOpts) (CustomReconciler, error) { ioHandler, err := workspacefile.NewIOHandler(opts.OpenAPIDefinitionsPath) if err != nil { return nil, fmt.Errorf("failed to create IO Handler: %w", err) } - - df, err := discoveryclient.NewFactory(virtualWorkspaceCfg) + pr, err := clusterpath.NewResolver(opts.Config, opts.Scheme) + if err != nil { + return nil, fmt.Errorf("failed to create cluster path resolver: %w", err) + } + virtualWorkspaceCfg, err := virtualWorkspaceConfigFromCfg(opts.Config, opts.Client) + if err != nil { + return nil, fmt.Errorf("unable to get virtual workspace config: %w", err) + } + df, err := f.newDiscoveryFactoryFunc(virtualWorkspaceCfg) if err != nil { return nil, fmt.Errorf("failed to create Discovery client factory: %w", err) } - return controller.NewAPIBindingReconciler( - ioHandler, df, apischema.NewResolver(), &clusterpath.Resolver{ - Scheme: opts.Scheme, - Config: opts.Config, - ResolverFunc: clusterpath.Resolve, - }, + ioHandler, df, apischema.NewResolver(), pr, ), nil } diff --git a/listener/kcp/reconciler_factory_test.go b/listener/kcp/reconciler_factory_test.go new file mode 100644 index 0000000..a6d1fa4 --- /dev/null +++ b/listener/kcp/reconciler_factory_test.go @@ -0,0 +1,141 @@ +package kcp + +import ( + "errors" + "path" + "testing" + + kcpapis "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha1" + "github.com/stretchr/testify/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/discovery" + fakediscovery "k8s.io/client-go/discovery/fake" + fakeclientset "k8s.io/client-go/kubernetes/fake" + "k8s.io/client-go/rest" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + "github.com/openmfp/kubernetes-graphql-gateway/listener/apischema" + "github.com/openmfp/kubernetes-graphql-gateway/listener/discoveryclient" + "github.com/openmfp/kubernetes-graphql-gateway/listener/workspacefile" +) + +func TestNewReconciler(t *testing.T) { + tempDir := t.TempDir() + + tests := map[string]struct { + cfg *rest.Config + definitionsPath string + isKCPEnabled bool + expectErr bool + }{ + "standard_reconciler_creation": { + cfg: &rest.Config{Host: validAPIServerHost}, + definitionsPath: tempDir, + isKCPEnabled: false, + expectErr: false, + }, + "kcp_reconciler_creation": { + cfg: &rest.Config{Host: validAPIServerHost}, + definitionsPath: tempDir, + isKCPEnabled: true, + expectErr: false, + }, + "failure_in_discovery_client_creation": { + cfg: nil, + definitionsPath: tempDir, + isKCPEnabled: false, + expectErr: true, + }, + "success_in_non-existent-dir": { + cfg: &rest.Config{Host: validAPIServerHost}, + definitionsPath: path.Join(tempDir, "non-existent"), + isKCPEnabled: false, + expectErr: false, + }, + "failure_in_rest_mapper_creation": { + cfg: &rest.Config{Host: schemelessAPIServerHost}, + definitionsPath: tempDir, + isKCPEnabled: false, + expectErr: true, + }, + "failure_in_virtual_workspace_config_retrieval_(kcp)": { + cfg: &rest.Config{Host: schemelessAPIServerHost}, + definitionsPath: tempDir, + isKCPEnabled: true, + expectErr: true, + }, + "failure_in_kcp_discovery_client_factory_creation": { + cfg: nil, + definitionsPath: tempDir, + isKCPEnabled: true, + expectErr: true, + }, + "failure_in_cluster_path_resolver_creation": { + cfg: &rest.Config{Host: schemelessAPIServerHost}, + definitionsPath: tempDir, + isKCPEnabled: true, + expectErr: true, + }, + } + + for name, tc := range tests { + scheme := runtime.NewScheme() + err := kcpapis.AddToScheme(scheme) + assert.NoError(t, err) + t.Run(name, func(t *testing.T) { + + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects([]client.Object{ + &kcpapis.APIExport{ + ObjectMeta: metav1.ObjectMeta{Name: tenancyAPIExportName}, + Status: kcpapis.APIExportStatus{ + VirtualWorkspaces: []kcpapis.VirtualWorkspace{ + {URL: validAPIServerHost}, + }, + }, + }, + }...).Build() + f := &ReconcilerFactory{ + IsKCPEnabled: tc.isKCPEnabled, + newDiscoveryIFFunc: fakeClientFactory, + preReconcileFunc: func(cr *apischema.CRDResolver, io *workspacefile.IOHandler) error { + return nil + }, + newDiscoveryFactoryFunc: func(cfg *rest.Config) (*discoveryclient.Factory, error) { + return &discoveryclient.Factory{ + Config: cfg, + NewDiscoveryIFFunc: fakeClientFactory, + }, nil + }, + } + reconciler, err := f.NewReconciler(ReconcilerOpts{ + Config: tc.cfg, + Scheme: scheme, + Client: fakeClient, + OpenAPIDefinitionsPath: tc.definitionsPath, + }) + + if tc.expectErr { + assert.Error(t, err) + assert.Nil(t, reconciler) + return + } + + assert.NoError(t, err) + assert.NotNil(t, reconciler) + }) + } +} + +func fakeClientFactory(cfg *rest.Config) (discovery.DiscoveryInterface, error) { + if cfg == nil { + return nil, errors.New("config cannot be nil") + } + client := fakeclientset.NewClientset() + fakeDiscovery, ok := client.Discovery().(*fakediscovery.FakeDiscovery) + if !ok { + return nil, errors.New("failed to get fake discovery client") + } + return fakeDiscovery, nil +} diff --git a/listener/kcp/workspace_config.go b/listener/kcp/workspace_config.go index b68f133..fdd1724 100644 --- a/listener/kcp/workspace_config.go +++ b/listener/kcp/workspace_config.go @@ -9,28 +9,19 @@ import ( kcpapis "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha1" kcptenancy "github.com/kcp-dev/kcp/sdk/apis/tenancy/v1alpha1" - "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/rest" "sigs.k8s.io/controller-runtime/pkg/client" ) -func virtualWorkspaceConfigFromCfg(cfg *rest.Config, scheme *runtime.Scheme) (*rest.Config, error) { +func virtualWorkspaceConfigFromCfg(cfg *rest.Config, clt client.Client) (*rest.Config, error) { cfgURL, err := url.Parse(cfg.Host) if err != nil { return nil, fmt.Errorf("failed to parse config Host: %w", err) } - clt, err := client.New(cfg, client.Options{ - Scheme: scheme, - }) - if err != nil { - return nil, fmt.Errorf("failed to create client from config: %w", err) - } ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() tenancyAPIExport := &kcpapis.APIExport{} - if err := clt.Get(ctx, client.ObjectKey{ - Name: kcptenancy.SchemeGroupVersion.Group, - }, tenancyAPIExport); err != nil { + if err := clt.Get(ctx, client.ObjectKey{Name: kcptenancy.SchemeGroupVersion.Group}, tenancyAPIExport); err != nil { return nil, fmt.Errorf("failed to get tenancy APIExport: %w", err) } virtualWorkspaces := tenancyAPIExport.Status.VirtualWorkspaces // nolint: staticcheck diff --git a/listener/kcp/workspace_config_test.go b/listener/kcp/workspace_config_test.go new file mode 100644 index 0000000..cfa9058 --- /dev/null +++ b/listener/kcp/workspace_config_test.go @@ -0,0 +1,94 @@ +package kcp + +import ( + "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" +) + +const ( + tenancyAPIExportName = "tenancy.kcp.io" + validAPIServerHost = "https://192.168.1.13:6443" + schemelessAPIServerHost = "://192.168.1.13:6443" +) + +func TestVirtualWorkspaceConfigFromCfg(t *testing.T) { + scheme := runtime.NewScheme() + err := kcpapis.AddToScheme(scheme) + assert.NoError(t, err) + tests := map[string]struct { + cfg *rest.Config + clientObjects []client.Object + expectErr bool + }{ + "successful_configuration_update": { + cfg: &rest.Config{Host: validAPIServerHost}, + clientObjects: []client.Object{ + &kcpapis.APIExport{ + ObjectMeta: metav1.ObjectMeta{Name: tenancyAPIExportName}, + Status: kcpapis.APIExportStatus{ + VirtualWorkspaces: []kcpapis.VirtualWorkspace{ + {URL: "https://192.168.1.13:6443/services/apiexport/root/tenancy.kcp.io"}, + }, + }, + }, + }, + expectErr: false, + }, + "invalid_config_host_url": { + cfg: &rest.Config{Host: schemelessAPIServerHost}, + expectErr: true, + }, + "error_retrieving_APIExport": { + cfg: &rest.Config{Host: validAPIServerHost}, + expectErr: true, + }, + "empty_virtual_workspace_list": { + cfg: &rest.Config{Host: validAPIServerHost}, + clientObjects: []client.Object{ + &kcpapis.APIExport{ + ObjectMeta: metav1.ObjectMeta{Name: tenancyAPIExportName}, + }, + }, + expectErr: true, + }, + "invalid_virtual_workspace_url": { + cfg: &rest.Config{Host: validAPIServerHost}, + clientObjects: []client.Object{ + &kcpapis.APIExport{ + ObjectMeta: metav1.ObjectMeta{Name: tenancyAPIExportName}, + Status: kcpapis.APIExportStatus{ + VirtualWorkspaces: []kcpapis.VirtualWorkspace{ + {URL: schemelessAPIServerHost}, + }, + }, + }, + }, + expectErr: true, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(tc.clientObjects...).Build() + + resultCfg, err := virtualWorkspaceConfigFromCfg(tc.cfg, fakeClient) + + if tc.expectErr { + assert.Error(t, err) + assert.Nil(t, resultCfg) + return + } + + assert.NoError(t, err) + assert.NotNil(t, resultCfg) + }) + } +} diff --git a/listener/workspacefile/io_handler.go b/listener/workspacefile/io_handler.go index 7e4a844..41bfcc5 100644 --- a/listener/workspacefile/io_handler.go +++ b/listener/workspacefile/io_handler.go @@ -6,30 +6,22 @@ import ( "path" ) -type IOHandler interface { - Reader - Writer +type IOHandler struct { + schemasDir string } -type IOHandlerImpl struct { - SchemasDir string -} - -func NewIOHandler(schemasDir string) (*IOHandlerImpl, error) { - _, err := os.Stat(schemasDir) - if os.IsNotExist(err) { - err := os.Mkdir(schemasDir, os.ModePerm) - if err != nil { - return nil, fmt.Errorf("failed to create openAPI definitions dir: %w", err) - } +func NewIOHandler(schemasDir string) (*IOHandler, error) { + if err := os.MkdirAll(schemasDir, os.ModePerm); err != nil { + return nil, fmt.Errorf("failed to create or access schemas directory: %w", err) } - return &IOHandlerImpl{ - SchemasDir: schemasDir, + + return &IOHandler{ + schemasDir: schemasDir, }, nil } -func (h *IOHandlerImpl) Read(clusterName string) ([]byte, error) { - fileName := path.Join(h.SchemasDir, clusterName) +func (h *IOHandler) Read(clusterName string) ([]byte, error) { + fileName := path.Join(h.schemasDir, clusterName) JSON, err := os.ReadFile(fileName) if err != nil { return nil, fmt.Errorf("failed to read JSON file: %w", err) @@ -37,10 +29,9 @@ func (h *IOHandlerImpl) Read(clusterName string) ([]byte, error) { return JSON, nil } -func (h *IOHandlerImpl) Write(JSON []byte, clusterName string) error { - fileName := path.Join(h.SchemasDir, clusterName) - err := os.WriteFile(fileName, JSON, os.ModePerm) - if err != nil { +func (h *IOHandler) Write(JSON []byte, clusterName string) error { + fileName := path.Join(h.schemasDir, clusterName) + if err := os.WriteFile(fileName, JSON, os.ModePerm); err != nil { return fmt.Errorf("failed to write JSON to file: %w", err) } return nil diff --git a/listener/workspacefile/io_handler_test.go b/listener/workspacefile/io_handler_test.go new file mode 100644 index 0000000..6cafd8a --- /dev/null +++ b/listener/workspacefile/io_handler_test.go @@ -0,0 +1,97 @@ +package workspacefile + +import ( + "os" + "path" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" +) + +var testJSON = []byte("{\"key\":\"value\"}") + +func TestNewIOHandler(t *testing.T) { + tempDir := t.TempDir() + + tests := map[string]struct { + schemasDir string + expectErr bool + }{ + "valid_directory": {schemasDir: tempDir, expectErr: false}, + "non_existent_directory": {schemasDir: path.Join(tempDir, "non-existent"), expectErr: false}, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + _, err := NewIOHandler(tc.schemasDir) + if tc.expectErr { + assert.Error(t, err) + return + } + assert.NoError(t, err) + }) + } +} + +func TestRead(t *testing.T) { + tempDir := t.TempDir() + + validClusterName := "root:sap:openmfp" + + validFile := filepath.Join(tempDir, validClusterName) + + err := os.WriteFile(validFile, testJSON, 0644) + assert.NoError(t, err) + + handler := &IOHandler{ + schemasDir: tempDir, + } + + tests := map[string]struct { + clusterName string + expectErr bool + }{ + "valid_file": {clusterName: validClusterName, expectErr: false}, + "non_existent_file": {clusterName: "root:non-existent", expectErr: true}, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + _, err := handler.Read(tc.clusterName) + if tc.expectErr { + assert.Error(t, err) + return + } + assert.NoError(t, err) + }) + } +} + +func TestWrite(t *testing.T) { + tempDir := t.TempDir() + handler := &IOHandler{ + schemasDir: tempDir, + } + + tests := map[string]struct { + clusterName string + expectErr bool + }{ + "valid_write": {clusterName: "root:sap:openmfp", expectErr: false}, + "invalid_path": {clusterName: "invalid/root:invalid", expectErr: true}, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + if err := handler.Write(testJSON, tc.clusterName); tc.expectErr { + assert.Error(t, err) + return + } + + writtenData, err := os.ReadFile(filepath.Join(tempDir, tc.clusterName)) + assert.NoError(t, err) + assert.Equal(t, string(writtenData), string(testJSON)) + }) + } +} diff --git a/listener/workspacefile/mocks/mock_IOHandler.go b/listener/workspacefile/mocks/mock_IOHandler.go deleted file mode 100644 index c95d0fa..0000000 --- a/listener/workspacefile/mocks/mock_IOHandler.go +++ /dev/null @@ -1,137 +0,0 @@ -// Code generated by mockery v2.43.2. DO NOT EDIT. - -package mocks - -import mock "github.com/stretchr/testify/mock" - -// MockIOHandler is an autogenerated mock type for the IOHandler type -type MockIOHandler struct { - mock.Mock -} - -type MockIOHandler_Expecter struct { - mock *mock.Mock -} - -func (_m *MockIOHandler) EXPECT() *MockIOHandler_Expecter { - return &MockIOHandler_Expecter{mock: &_m.Mock} -} - -// Read provides a mock function with given fields: clusterName -func (_m *MockIOHandler) Read(clusterName string) ([]byte, error) { - ret := _m.Called(clusterName) - - if len(ret) == 0 { - panic("no return value specified for Read") - } - - var r0 []byte - var r1 error - if rf, ok := ret.Get(0).(func(string) ([]byte, error)); ok { - return rf(clusterName) - } - if rf, ok := ret.Get(0).(func(string) []byte); ok { - r0 = rf(clusterName) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).([]byte) - } - } - - if rf, ok := ret.Get(1).(func(string) error); ok { - r1 = rf(clusterName) - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - -// MockIOHandler_Read_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Read' -type MockIOHandler_Read_Call struct { - *mock.Call -} - -// Read is a helper method to define mock.On call -// - clusterName string -func (_e *MockIOHandler_Expecter) Read(clusterName interface{}) *MockIOHandler_Read_Call { - return &MockIOHandler_Read_Call{Call: _e.mock.On("Read", clusterName)} -} - -func (_c *MockIOHandler_Read_Call) Run(run func(clusterName string)) *MockIOHandler_Read_Call { - _c.Call.Run(func(args mock.Arguments) { - run(args[0].(string)) - }) - return _c -} - -func (_c *MockIOHandler_Read_Call) Return(_a0 []byte, _a1 error) *MockIOHandler_Read_Call { - _c.Call.Return(_a0, _a1) - return _c -} - -func (_c *MockIOHandler_Read_Call) RunAndReturn(run func(string) ([]byte, error)) *MockIOHandler_Read_Call { - _c.Call.Return(run) - return _c -} - -// Write provides a mock function with given fields: JSON, clusterName -func (_m *MockIOHandler) Write(JSON []byte, clusterName string) error { - ret := _m.Called(JSON, clusterName) - - if len(ret) == 0 { - panic("no return value specified for Write") - } - - var r0 error - if rf, ok := ret.Get(0).(func([]byte, string) error); ok { - r0 = rf(JSON, clusterName) - } else { - r0 = ret.Error(0) - } - - return r0 -} - -// MockIOHandler_Write_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Write' -type MockIOHandler_Write_Call struct { - *mock.Call -} - -// Write is a helper method to define mock.On call -// - JSON []byte -// - clusterName string -func (_e *MockIOHandler_Expecter) Write(JSON interface{}, clusterName interface{}) *MockIOHandler_Write_Call { - return &MockIOHandler_Write_Call{Call: _e.mock.On("Write", JSON, clusterName)} -} - -func (_c *MockIOHandler_Write_Call) Run(run func(JSON []byte, clusterName string)) *MockIOHandler_Write_Call { - _c.Call.Run(func(args mock.Arguments) { - run(args[0].([]byte), args[1].(string)) - }) - return _c -} - -func (_c *MockIOHandler_Write_Call) Return(_a0 error) *MockIOHandler_Write_Call { - _c.Call.Return(_a0) - return _c -} - -func (_c *MockIOHandler_Write_Call) RunAndReturn(run func([]byte, string) error) *MockIOHandler_Write_Call { - _c.Call.Return(run) - return _c -} - -// NewMockIOHandler creates a new instance of MockIOHandler. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. -// The first argument is typically a *testing.T value. -func NewMockIOHandler(t interface { - mock.TestingT - Cleanup(func()) -}) *MockIOHandler { - mock := &MockIOHandler{} - mock.Mock.Test(t) - - t.Cleanup(func() { mock.AssertExpectations(t) }) - - return mock -} diff --git a/listener/workspacefile/reader.go b/listener/workspacefile/reader.go deleted file mode 100644 index d67d19d..0000000 --- a/listener/workspacefile/reader.go +++ /dev/null @@ -1,5 +0,0 @@ -package workspacefile - -type Reader interface { - Read(clusterName string) ([]byte, error) -} diff --git a/listener/workspacefile/writer.go b/listener/workspacefile/writer.go deleted file mode 100644 index abcd05f..0000000 --- a/listener/workspacefile/writer.go +++ /dev/null @@ -1,5 +0,0 @@ -package workspacefile - -type Writer interface { - Write(JSON []byte, clusterName string) error -}