Browse Source

fix: use cache when retrieving generators (#4153)

* fix: use cache when retrieving generators

Signed-off-by: Mathew Wicks <5735406+thesuperzapper@users.noreply.github.com>

* fix longstanding schema issues

Signed-off-by: Mathew Wicks <5735406+thesuperzapper@users.noreply.github.com>

---------

Signed-off-by: Mathew Wicks <5735406+thesuperzapper@users.noreply.github.com>
Co-authored-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
Mathew Wicks 1 year ago
parent
commit
73bff05bf2

+ 0 - 25
apis/generators/v1alpha1/generator_schema.go

@@ -17,10 +17,6 @@ package v1alpha1
 import (
 import (
 	"fmt"
 	"fmt"
 	"sync"
 	"sync"
-
-	apiextensions "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/util/json"
 )
 )
 
 
 var builder map[string]Generator
 var builder map[string]Generator
@@ -58,24 +54,3 @@ func GetGeneratorByName(kind string) (Generator, bool) {
 	buildlock.RUnlock()
 	buildlock.RUnlock()
 	return f, ok
 	return f, ok
 }
 }
-
-// GetGenerator returns an implementation from a generator
-// defined as json.
-func GetGenerator(obj *apiextensions.JSON) (Generator, error) {
-	type unknownGenerator struct {
-		metav1.TypeMeta   `json:",inline"`
-		metav1.ObjectMeta `json:"metadata,omitempty"`
-	}
-	var res unknownGenerator
-	err := json.Unmarshal(obj.Raw, &res)
-	if err != nil {
-		return nil, err
-	}
-	buildlock.RLock()
-	defer buildlock.RUnlock()
-	gen, ok := builder[res.Kind]
-	if !ok {
-		return nil, fmt.Errorf("failed to find registered generator for: %s with kind: %s", string(obj.Raw), res.Kind)
-	}
-	return gen, nil
-}

+ 4 - 0
apis/generators/v1alpha1/generator_types.go

@@ -32,6 +32,10 @@ type ControllerClassResource struct {
 }
 }
 
 
 type GeneratorSpec struct {
 type GeneratorSpec struct {
+	// NOTE: when adding new supported generators, make sure to also update
+	//       clusterGeneratorToVirtual() function in pkg/utils/resolvers/generator.go
+	//       so they can be unpacked correctly.
+
 	ACRAccessTokenSpec        *ACRAccessTokenSpec        `json:"acrAccessTokenSpec,omitempty"`
 	ACRAccessTokenSpec        *ACRAccessTokenSpec        `json:"acrAccessTokenSpec,omitempty"`
 	ECRAuthorizationTokenSpec *ECRAuthorizationTokenSpec `json:"ecrRAuthorizationTokenSpec,omitempty"`
 	ECRAuthorizationTokenSpec *ECRAuthorizationTokenSpec `json:"ecrRAuthorizationTokenSpec,omitempty"`
 	FakeSpec                  *FakeSpec                  `json:"fakeSpec,omitempty"`
 	FakeSpec                  *FakeSpec                  `json:"fakeSpec,omitempty"`

+ 1 - 1
apis/generators/v1alpha1/register.go

@@ -125,7 +125,7 @@ var (
 )
 )
 
 
 func init() {
 func init() {
-	SchemeBuilder.Register(&ECRAuthorizationToken{}, &ECRAuthorizationToken{})
+	SchemeBuilder.Register(&ECRAuthorizationToken{}, &ECRAuthorizationTokenList{})
 	SchemeBuilder.Register(&GCRAccessToken{}, &GCRAccessTokenList{})
 	SchemeBuilder.Register(&GCRAccessToken{}, &GCRAccessTokenList{})
 	SchemeBuilder.Register(&GithubAccessToken{}, &GithubAccessTokenList{})
 	SchemeBuilder.Register(&GithubAccessToken{}, &GithubAccessTokenList{})
 	SchemeBuilder.Register(&ACRAccessToken{}, &ACRAccessTokenList{})
 	SchemeBuilder.Register(&ACRAccessToken{}, &ACRAccessTokenList{})

+ 9 - 5
cmd/root.go

@@ -25,6 +25,7 @@ import (
 	v1 "k8s.io/api/core/v1"
 	v1 "k8s.io/api/core/v1"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime"
+	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
 	ctrl "sigs.k8s.io/controller-runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/cache"
 	"sigs.k8s.io/controller-runtime/pkg/cache"
@@ -94,11 +95,14 @@ const (
 )
 )
 
 
 func init() {
 func init() {
-	_ = clientgoscheme.AddToScheme(scheme)
-	_ = esv1beta1.AddToScheme(scheme)
-	_ = esv1alpha1.AddToScheme(scheme)
-	_ = genv1alpha1.AddToScheme(scheme)
-	_ = apiextensionsv1.AddToScheme(scheme)
+	// kubernetes schemes
+	utilruntime.Must(clientgoscheme.AddToScheme(scheme))
+	utilruntime.Must(apiextensionsv1.AddToScheme(scheme))
+
+	// external-secrets schemes
+	utilruntime.Must(esv1beta1.AddToScheme(scheme))
+	utilruntime.Must(esv1alpha1.AddToScheme(scheme))
+	utilruntime.Must(genv1alpha1.AddToScheme(scheme))
 }
 }
 
 
 var rootCmd = &cobra.Command{
 var rootCmd = &cobra.Command{

+ 7 - 3
cmd/webhook.go

@@ -29,6 +29,7 @@ import (
 
 
 	"github.com/spf13/cobra"
 	"github.com/spf13/cobra"
 	"go.uber.org/zap/zapcore"
 	"go.uber.org/zap/zapcore"
+	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
 	ctrl "sigs.k8s.io/controller-runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
@@ -45,9 +46,12 @@ const (
 )
 )
 
 
 func init() {
 func init() {
-	_ = clientgoscheme.AddToScheme(scheme)
-	_ = esv1beta1.AddToScheme(scheme)
-	_ = esv1alpha1.AddToScheme(scheme)
+	// kubernetes schemes
+	utilruntime.Must(clientgoscheme.AddToScheme(scheme))
+
+	// external-secrets schemes
+	utilruntime.Must(esv1beta1.AddToScheme(scheme))
+	utilruntime.Must(esv1alpha1.AddToScheme(scheme))
 }
 }
 
 
 var webhookCmd = &cobra.Command{
 var webhookCmd = &cobra.Command{

+ 20 - 14
e2e/framework/util/util.go

@@ -24,37 +24,43 @@ import (
 
 
 	fluxhelm "github.com/fluxcd/helm-controller/api/v2beta1"
 	fluxhelm "github.com/fluxcd/helm-controller/api/v2beta1"
 	fluxsrc "github.com/fluxcd/source-controller/api/v1beta2"
 	fluxsrc "github.com/fluxcd/source-controller/api/v1beta2"
-
-	// nolint
-	. "github.com/onsi/ginkgo/v2"
 	v1 "k8s.io/api/core/v1"
 	v1 "k8s.io/api/core/v1"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime"
+	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/kubernetes"
-	"k8s.io/client-go/kubernetes/scheme"
+	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
 	restclient "k8s.io/client-go/rest"
 	restclient "k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/clientcmd"
 	"k8s.io/client-go/tools/clientcmd"
 	"k8s.io/client-go/tools/remotecommand"
 	"k8s.io/client-go/tools/remotecommand"
 	crclient "sigs.k8s.io/controller-runtime/pkg/client"
 	crclient "sigs.k8s.io/controller-runtime/pkg/client"
 
 
+	// nolint
+	. "github.com/onsi/ginkgo/v2"
+
 	esv1alpha1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1alpha1"
 	esv1alpha1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1alpha1"
 	esv1beta1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1beta1"
 	esv1beta1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1beta1"
 	genv1alpha1 "github.com/external-secrets/external-secrets/apis/generators/v1alpha1"
 	genv1alpha1 "github.com/external-secrets/external-secrets/apis/generators/v1alpha1"
 )
 )
 
 
-var Scheme = runtime.NewScheme()
+var scheme = runtime.NewScheme()
 
 
 func init() {
 func init() {
-	_ = scheme.AddToScheme(Scheme)
-	_ = esv1beta1.AddToScheme(Scheme)
-	_ = esv1alpha1.AddToScheme(Scheme)
-	_ = genv1alpha1.AddToScheme(Scheme)
-	_ = fluxhelm.AddToScheme(Scheme)
-	_ = fluxsrc.AddToScheme(Scheme)
-	_ = apiextensionsv1.AddToScheme(Scheme)
+	// kubernetes schemes
+	utilruntime.Must(clientgoscheme.AddToScheme(scheme))
+	utilruntime.Must(apiextensionsv1.AddToScheme(scheme))
+
+	// external-secrets schemes
+	utilruntime.Must(esv1beta1.AddToScheme(scheme))
+	utilruntime.Must(esv1alpha1.AddToScheme(scheme))
+	utilruntime.Must(genv1alpha1.AddToScheme(scheme))
+
+	// other schemes
+	utilruntime.Must(fluxhelm.AddToScheme(scheme))
+	utilruntime.Must(fluxsrc.AddToScheme(scheme))
 }
 }
 
 
 const (
 const (
@@ -129,7 +135,7 @@ func execCmd(client kubernetes.Interface, config *restclient.Config, podName, co
 	}
 	}
 	req.VersionedParams(
 	req.VersionedParams(
 		option,
 		option,
-		scheme.ParameterCodec,
+		clientgoscheme.ParameterCodec,
 	)
 	)
 	exec, err := remotecommand.NewSPDYExecutor(config, "POST", req.URL())
 	exec, err := remotecommand.NewSPDYExecutor(config, "POST", req.URL())
 	if err != nil {
 	if err != nil {
@@ -290,7 +296,7 @@ func NewConfig() (*restclient.Config, *kubernetes.Clientset, crclient.Client) {
 		Fail(err.Error())
 		Fail(err.Error())
 	}
 	}
 
 
-	CRClient, err := crclient.New(kubeConfig, crclient.Options{Scheme: Scheme})
+	CRClient, err := crclient.New(kubeConfig, crclient.Options{Scheme: scheme})
 	if err != nil {
 	if err != nil {
 		Fail(err.Error())
 		Fail(err.Error())
 	}
 	}

+ 5 - 1
pkg/controllers/externalsecret/externalsecret_controller.go

@@ -799,12 +799,16 @@ func shouldSkipUnmanagedStore(ctx context.Context, namespace string, r *Reconcil
 
 
 		// verify that generator's controllerClass matches
 		// verify that generator's controllerClass matches
 		if ref.SourceRef != nil && ref.SourceRef.GeneratorRef != nil {
 		if ref.SourceRef != nil && ref.SourceRef.GeneratorRef != nil {
-			_, obj, err := resolvers.GeneratorRef(ctx, r.RestConfig, namespace, ref.SourceRef.GeneratorRef)
+			_, obj, err := resolvers.GeneratorRef(ctx, r.Client, r.Scheme, namespace, ref.SourceRef.GeneratorRef)
 			if err != nil {
 			if err != nil {
 				if apierrors.IsNotFound(err) {
 				if apierrors.IsNotFound(err) {
 					// skip non-existent generators
 					// skip non-existent generators
 					continue
 					continue
 				}
 				}
+				if errors.Is(err, resolvers.ErrUnableToGetGenerator) {
+					// skip generators that we can't get (e.g. due to being invalid)
+					continue
+				}
 				return false, err
 				return false, err
 			}
 			}
 			skipGenerator, err := shouldSkipGenerator(r, obj)
 			skipGenerator, err := shouldSkipGenerator(r, obj)

+ 1 - 1
pkg/controllers/externalsecret/externalsecret_controller_secret.go

@@ -111,7 +111,7 @@ func toStoreGenSourceRef(ref *esv1beta1.StoreSourceRef) *esv1beta1.StoreGenerato
 }
 }
 
 
 func (r *Reconciler) handleGenerateSecrets(ctx context.Context, namespace string, remoteRef esv1beta1.ExternalSecretDataFromRemoteRef, i int) (map[string][]byte, error) {
 func (r *Reconciler) handleGenerateSecrets(ctx context.Context, namespace string, remoteRef esv1beta1.ExternalSecretDataFromRemoteRef, i int) (map[string][]byte, error) {
-	gen, obj, err := resolvers.GeneratorRef(ctx, r.RestConfig, namespace, remoteRef.SourceRef.GeneratorRef)
+	gen, obj, err := resolvers.GeneratorRef(ctx, r.Client, r.Scheme, namespace, remoteRef.SourceRef.GeneratorRef)
 	if err != nil {
 	if err != nil {
 		return nil, fmt.Errorf("unable to resolve generator: %w", err)
 		return nil, fmt.Errorf("unable to resolve generator: %w", err)
 	}
 	}

+ 1 - 1
pkg/controllers/pushsecret/pushsecret_controller.go

@@ -372,7 +372,7 @@ func (r *Reconciler) resolveSecret(ctx context.Context, ps esapi.PushSecret) (*v
 }
 }
 
 
 func (r *Reconciler) resolveSecretFromGenerator(ctx context.Context, namespace string, generatorRef *v1beta1.GeneratorRef) (*v1.Secret, error) {
 func (r *Reconciler) resolveSecretFromGenerator(ctx context.Context, namespace string, generatorRef *v1beta1.GeneratorRef) (*v1.Secret, error) {
-	gen, obj, err := resolvers.GeneratorRef(ctx, r.RestConfig, namespace, generatorRef)
+	gen, obj, err := resolvers.GeneratorRef(ctx, r.Client, r.Scheme, namespace, generatorRef)
 	if err != nil {
 	if err != nil {
 		return nil, fmt.Errorf("unable to resolve generator: %w", err)
 		return nil, fmt.Errorf("unable to resolve generator: %w", err)
 	}
 	}

+ 15 - 6
pkg/controllers/secretstore/client_manager_test.go

@@ -25,6 +25,7 @@ import (
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime"
+	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake"
 	fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake"
@@ -35,9 +36,13 @@ import (
 
 
 func TestManagerGet(t *testing.T) {
 func TestManagerGet(t *testing.T) {
 	scheme := runtime.NewScheme()
 	scheme := runtime.NewScheme()
-	_ = clientgoscheme.AddToScheme(scheme)
-	_ = esv1beta1.AddToScheme(scheme)
-	_ = apiextensionsv1.AddToScheme(scheme)
+
+	// add kubernetes schemes
+	utilruntime.Must(clientgoscheme.AddToScheme(scheme))
+	utilruntime.Must(apiextensionsv1.AddToScheme(scheme))
+
+	// add external-secrets schemes
+	utilruntime.Must(esv1beta1.AddToScheme(scheme))
 
 
 	// We have a test provider to control
 	// We have a test provider to control
 	// the behavior of the NewClient func.
 	// the behavior of the NewClient func.
@@ -312,9 +317,13 @@ func TestManagerGet(t *testing.T) {
 
 
 func TestShouldProcessSecret(t *testing.T) {
 func TestShouldProcessSecret(t *testing.T) {
 	scheme := runtime.NewScheme()
 	scheme := runtime.NewScheme()
-	_ = clientgoscheme.AddToScheme(scheme)
-	_ = esv1beta1.AddToScheme(scheme)
-	_ = apiextensionsv1.AddToScheme(scheme)
+
+	// add kubernetes schemes
+	utilruntime.Must(clientgoscheme.AddToScheme(scheme))
+	utilruntime.Must(apiextensionsv1.AddToScheme(scheme))
+
+	// add external-secrets schemes
+	utilruntime.Must(esv1beta1.AddToScheme(scheme))
 
 
 	testNamespace := "test-a"
 	testNamespace := "test-a"
 	testCases := []struct {
 	testCases := []struct {

+ 156 - 104
pkg/utils/resolvers/generator.go

@@ -17,146 +17,198 @@ package resolvers
 import (
 import (
 	"context"
 	"context"
 	"fmt"
 	"fmt"
+	"reflect"
 
 
 	apiextensions "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	apiextensions "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
-	"k8s.io/apimachinery/pkg/api/meta"
-	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/runtime/schema"
-	"k8s.io/apimachinery/pkg/util/json"
-	"k8s.io/client-go/discovery"
-	"k8s.io/client-go/dynamic"
-	"k8s.io/client-go/rest"
-	"k8s.io/client-go/restmapper"
+	"k8s.io/apimachinery/pkg/types"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 
 	esv1beta1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1beta1"
 	esv1beta1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1beta1"
 	genv1alpha1 "github.com/external-secrets/external-secrets/apis/generators/v1alpha1"
 	genv1alpha1 "github.com/external-secrets/external-secrets/apis/generators/v1alpha1"
 )
 )
 
 
+// these errors are explicitly defined so we can detect them with `errors.Is()`.
+var (
+	// ErrUnableToGetGenerator is returned when a generator reference cannot be resolved.
+	ErrUnableToGetGenerator = fmt.Errorf("unable to get generator")
+)
+
 // GeneratorRef resolves a generator reference to a generator implementation.
 // GeneratorRef resolves a generator reference to a generator implementation.
-func GeneratorRef(ctx context.Context, restConfig *rest.Config, namespace string, generatorRef *esv1beta1.GeneratorRef) (genv1alpha1.Generator, *apiextensions.JSON, error) {
-	obj, err := getGeneratorDefinition(ctx, restConfig, namespace, generatorRef)
-	if err != nil {
-		return nil, nil, fmt.Errorf("unable to get generator definition: %w", err)
-	}
-	generator, err := genv1alpha1.GetGenerator(obj)
+func GeneratorRef(ctx context.Context, cl client.Client, scheme *runtime.Scheme, namespace string, generatorRef *esv1beta1.GeneratorRef) (genv1alpha1.Generator, *apiextensions.JSON, error) {
+	generator, jsonObj, err := getGenerator(ctx, cl, scheme, namespace, generatorRef)
 	if err != nil {
 	if err != nil {
-		return nil, nil, fmt.Errorf("unable to get generator: %w", err)
+		return nil, nil, fmt.Errorf("%w: %w", ErrUnableToGetGenerator, err)
 	}
 	}
-	return generator, obj, nil
+	return generator, jsonObj, nil
 }
 }
 
 
-func getGeneratorDefinition(ctx context.Context, restConfig *rest.Config, namespace string, generatorRef *esv1beta1.GeneratorRef) (*apiextensions.JSON, error) {
-	// client-go dynamic client needs a GVR to fetch the resource
-	// But we only have the GVK in our generatorRef.
-	//
-	// TODO: there is no need to discover the GroupVersionResource
-	//       this should be cached.
-	c := discovery.NewDiscoveryClientForConfigOrDie(restConfig)
-	groupResources, err := restmapper.GetAPIGroupResources(c)
-	if err != nil {
-		return nil, err
-	}
-
+func getGenerator(ctx context.Context, cl client.Client, scheme *runtime.Scheme, namespace string, generatorRef *esv1beta1.GeneratorRef) (genv1alpha1.Generator, *apiextensions.JSON, error) {
+	// get a GVK from the generatorRef
 	gv, err := schema.ParseGroupVersion(generatorRef.APIVersion)
 	gv, err := schema.ParseGroupVersion(generatorRef.APIVersion)
 	if err != nil {
 	if err != nil {
-		return nil, err
-	}
-	mapper := restmapper.NewDiscoveryRESTMapper(groupResources)
-	mapping, err := mapper.RESTMapping(schema.GroupKind{
-		Group: gv.Group,
-		Kind:  generatorRef.Kind,
-	})
-	if err != nil {
-		return nil, err
+		return nil, nil, reconcile.TerminalError(fmt.Errorf("generatorRef has invalid APIVersion: %w", err))
 	}
 	}
-	d, err := dynamic.NewForConfig(restConfig)
-	if err != nil {
-		return nil, err
-	}
-
-	if generatorRef.Kind == "ClusterGenerator" {
-		return extractGeneratorFromClusterGenerator(ctx, d, mapping, generatorRef)
+	gvk := schema.GroupVersionKind{
+		Group:   gv.Group,
+		Version: gv.Version,
+		Kind:    generatorRef.Kind,
 	}
 	}
 
 
-	res, err := d.Resource(mapping.Resource).Namespace(namespace).Get(ctx, generatorRef.Name, metav1.GetOptions{})
-	if err != nil {
-		return nil, err
+	// fail if the GVK does not use the generator group
+	if gvk.Group != genv1alpha1.Group {
+		return nil, nil, reconcile.TerminalError(fmt.Errorf("generatorRef may only reference the generators group, but got %s", gvk.Group))
 	}
 	}
 
 
-	jsonRes, err := res.MarshalJSON()
-	if err != nil {
-		return nil, err
+	// get a client Object from the GVK
+	t, exists := scheme.AllKnownTypes()[gvk]
+	if !exists {
+		return nil, nil, reconcile.TerminalError(fmt.Errorf("generatorRef references unknown GVK %s", gvk))
 	}
 	}
-	return &apiextensions.JSON{Raw: jsonRes}, nil
-}
+	obj := reflect.New(t).Interface().(client.Object)
 
 
-func extractGeneratorFromClusterGenerator(
-	ctx context.Context,
-	d *dynamic.DynamicClient,
-	mapping *meta.RESTMapping,
-	generatorRef *esv1beta1.GeneratorRef,
-) (*apiextensions.JSON, error) {
-	res, err := d.Resource(mapping.Resource).Get(ctx, generatorRef.Name, metav1.GetOptions{})
-	if err != nil {
-		return nil, err
-	}
+	// this interface provides the Generate() method used by the controller
+	// NOTE: all instances of a generator kind use the same instance of this interface
+	var generator genv1alpha1.Generator
 
 
-	spec, err := extractValue[map[string]any](res.Object, genv1alpha1.GeneratorSpecKey)
-	if err != nil {
-		return nil, err
-	}
+	// ClusterGenerator is a special case because it's a cluster-scoped resource
+	// to use it, we create a "virtual" namespaced generator for the current namespace, as if one existed in the API
+	if gvk.Kind == genv1alpha1.ClusterGeneratorKind {
+		clusterGenerator := obj.(*genv1alpha1.ClusterGenerator)
 
 
-	generator, err := extractValue[map[string]any](spec, genv1alpha1.GeneratorGeneratorKey)
-	if err != nil {
-		return nil, err
-	}
+		// get the cluster generator resource from the API
+		// NOTE: it's important that we use the structured client so we use the cache
+		err = cl.Get(ctx, client.ObjectKey{Name: generatorRef.Name}, clusterGenerator)
+		if err != nil {
+			return nil, nil, err
+		}
 
 
-	kind, err := extractValue[string](spec, genv1alpha1.GeneratorKindKey)
-	if err != nil {
-		return nil, err
-	}
+		// convert the cluster generator to a virtual namespaced generator object
+		obj, err = clusterGeneratorToVirtual(clusterGenerator)
+		if err != nil {
+			return nil, nil, reconcile.TerminalError(fmt.Errorf("invalid ClusterGenerator: %w", err))
+		}
 
 
-	// find the first value and that's what we are going to take
-	// this will be the generator that has been set by the user
-	var result []byte
-	for _, v := range generator {
-		vMap, ok := v.(map[string]interface{})
+		// get the generator interface
+		var ok bool
+		generator, ok = genv1alpha1.GetGeneratorByName(clusterGenerator.Spec.Kind)
 		if !ok {
 		if !ok {
-			return nil, fmt.Errorf("kind was not of object type for cluster generator %T", v)
+			return nil, nil, reconcile.TerminalError(fmt.Errorf("ClusterGenerator has unknown kind %s", clusterGenerator.Spec.Kind))
 		}
 		}
-
-		// Construct our generator object so it can be later unmarshalled into a valid Generator Spec.
-		object := map[string]interface{}{}
-		object["kind"] = kind
-		object["spec"] = vMap
-		result, err = json.Marshal(object)
+	} else {
+		// get the generator resource from the API
+		// NOTE: it's important that we use the structured client so we use the cache
+		err = cl.Get(ctx, types.NamespacedName{
+			Name:      generatorRef.Name,
+			Namespace: namespace,
+		}, obj)
 		if err != nil {
 		if err != nil {
-			return nil, err
+			return nil, nil, err
 		}
 		}
 
 
-		return &apiextensions.JSON{Raw: result}, nil
+		// get the generator interface
+		var ok bool
+		generator, ok = genv1alpha1.GetGeneratorByName(gvk.Kind)
+		if !ok {
+			return nil, nil, reconcile.TerminalError(fmt.Errorf("generatorRef has unknown kind %s", gvk.Kind))
+		}
 	}
 	}
 
 
-	return nil, fmt.Errorf("no defined generators found for cluster generator spec: %v", spec)
-}
-
-// extractValue fetches a specific key value that we are looking for in a map.
-func extractValue[T any](m any, k string) (T, error) {
-	var result T
-	v, ok := m.(map[string]any)
-	if !ok {
-		return result, fmt.Errorf("value was not of type map[string]any but: %T", m)
+	// convert the generator to unstructured object
+	u := &unstructured.Unstructured{}
+	u.Object, err = runtime.DefaultUnstructuredConverter.ToUnstructured(obj)
+	if err != nil {
+		return nil, nil, err
 	}
 	}
 
 
-	vv, ok := v[k]
-	if !ok {
-		return result, fmt.Errorf("key %s was not found in map", k)
+	// convert the unstructured object to JSON
+	// NOTE: we do this for backwards compatibility with how this API works, not because it's a good idea
+	//       we should refactor the generator API to use the normal typed objects
+	jsonObj, err := u.MarshalJSON()
+	if err != nil {
+		return nil, nil, err
 	}
 	}
 
 
-	vvv, ok := vv.(T)
-	if !ok {
-		return result, fmt.Errorf("value was not of type T but: %T", vvv)
-	}
+	return generator, &apiextensions.JSON{Raw: jsonObj}, nil
+}
 
 
-	return vvv, nil
+// clusterGeneratorToVirtual converts a ClusterGenerator to a "virtual" namespaced generator that doesn't actually exist in the API.
+func clusterGeneratorToVirtual(gen *genv1alpha1.ClusterGenerator) (client.Object, error) {
+	switch gen.Spec.Kind {
+	case genv1alpha1.ACRAccessTokenKind:
+		if gen.Spec.Generator.ACRAccessTokenSpec == nil {
+			return nil, fmt.Errorf("when kind is %s, ACRAccessTokenSpec must be set", gen.Spec.Kind)
+		}
+		return &genv1alpha1.ACRAccessToken{
+			Spec: *gen.Spec.Generator.ACRAccessTokenSpec,
+		}, nil
+	case genv1alpha1.ECRAuthorizationTokenKind:
+		if gen.Spec.Generator.ECRAuthorizationTokenSpec == nil {
+			return nil, fmt.Errorf("when kind is %s, ECRAuthorizationTokenSpec must be set", gen.Spec.Kind)
+		}
+		return &genv1alpha1.ECRAuthorizationToken{
+			Spec: *gen.Spec.Generator.ECRAuthorizationTokenSpec,
+		}, nil
+	case genv1alpha1.FakeKind:
+		if gen.Spec.Generator.FakeSpec == nil {
+			return nil, fmt.Errorf("when kind is %s, FakeSpec must be set", gen.Spec.Kind)
+		}
+		return &genv1alpha1.Fake{
+			Spec: *gen.Spec.Generator.FakeSpec,
+		}, nil
+	case genv1alpha1.GCRAccessTokenKind:
+		if gen.Spec.Generator.GCRAccessTokenSpec == nil {
+			return nil, fmt.Errorf("when kind is %s, GCRAccessTokenSpec must be set", gen.Spec.Kind)
+		}
+		return &genv1alpha1.GCRAccessToken{
+			Spec: *gen.Spec.Generator.GCRAccessTokenSpec,
+		}, nil
+	case genv1alpha1.GithubAccessTokenKind:
+		if gen.Spec.Generator.GithubAccessTokenSpec == nil {
+			return nil, fmt.Errorf("when kind is %s, GithubAccessTokenSpec must be set", gen.Spec.Kind)
+		}
+		return &genv1alpha1.GithubAccessToken{
+			Spec: *gen.Spec.Generator.GithubAccessTokenSpec,
+		}, nil
+	case genv1alpha1.PasswordKind:
+		if gen.Spec.Generator.PasswordSpec == nil {
+			return nil, fmt.Errorf("when kind is %s, PasswordSpec must be set", gen.Spec.Kind)
+		}
+		return &genv1alpha1.Password{
+			Spec: *gen.Spec.Generator.PasswordSpec,
+		}, nil
+	case genv1alpha1.STSSessionTokenKind:
+		if gen.Spec.Generator.STSSessionTokenSpec == nil {
+			return nil, fmt.Errorf("when kind is %s, STSSessionTokenSpec must be set", gen.Spec.Kind)
+		}
+		return &genv1alpha1.STSSessionToken{
+			Spec: *gen.Spec.Generator.STSSessionTokenSpec,
+		}, nil
+	case genv1alpha1.UUIDKind:
+		if gen.Spec.Generator.UUIDSpec == nil {
+			return nil, fmt.Errorf("when kind is %s, UUIDSpec must be set", gen.Spec.Kind)
+		}
+		return &genv1alpha1.UUID{
+			Spec: *gen.Spec.Generator.UUIDSpec,
+		}, nil
+	case genv1alpha1.VaultDynamicSecretKind:
+		if gen.Spec.Generator.VaultDynamicSecretSpec == nil {
+			return nil, fmt.Errorf("when kind is %s, VaultDynamicSecretSpec must be set", gen.Spec.Kind)
+		}
+		return &genv1alpha1.VaultDynamicSecret{
+			Spec: *gen.Spec.Generator.VaultDynamicSecretSpec,
+		}, nil
+	case genv1alpha1.WebhookKind:
+		if gen.Spec.Generator.WebhookSpec == nil {
+			return nil, fmt.Errorf("when kind is %s, WebhookSpec must be set", gen.Spec.Kind)
+		}
+		return &genv1alpha1.Webhook{
+			Spec: *gen.Spec.Generator.WebhookSpec,
+		}, nil
+	default:
+		return nil, fmt.Errorf("unknown kind %s", gen.Spec.Kind)
+	}
 }
 }