chore: fix up code review comments

Signed-off-by: benjdlambert <ben@blam.sh>
2025-08-18 15:30:53 +02:00
parent 79e342ec36
commit 908420bf3b
4 changed files with 116 additions and 150 deletions
@@ -39,7 +39,7 @@ export const buildDefaultAuthStrategyMap = ({
    ['aws', new AwsIamStrategy({ config })],
    ['azure', new AzureIdentityStrategy(logger)],
    ['google', new GoogleStrategy()],
-    ['googleServiceAccount', new GoogleServiceAccountStrategy()],
+    ['googleServiceAccount', new GoogleServiceAccountStrategy({ config })],
    ['localKubectlProxy', new AnonymousStrategy()],
    ['oidc', new OidcStrategy()],
    ['serviceAccount', new ServiceAccountStrategy()],
@@ -28,16 +28,20 @@ import {
  type KubernetesClustersSupplier,
  kubernetesClusterSupplierExtensionPoint,
  type KubernetesClusterSupplierExtensionPoint,
+  KubernetesClusterSupplierFactory,
  type KubernetesFetcher,
  kubernetesFetcherExtensionPoint,
  type KubernetesFetcherExtensionPoint,
+  KubernetesFetcherFactory,
  type KubernetesObjectsProvider,
  kubernetesObjectsProviderExtensionPoint,
  type KubernetesObjectsProviderExtensionPoint,
+  KubernetesObjectsProviderFactory,
  KubernetesObjectTypes,
  type KubernetesServiceLocator,
  kubernetesServiceLocatorExtensionPoint,
  type KubernetesServiceLocatorExtensionPoint,
+  KubernetesServiceLocatorFactory,
  ObjectToFetch,
 } from '@backstage/plugin-kubernetes-node';
 import { KubernetesBuilder } from './service/KubernetesBuilder';
@@ -54,36 +58,22 @@ import {
 } from './service/KubernetesFanOutHandler';

 class ObjectsProvider implements KubernetesObjectsProviderExtensionPoint {
-  private objectsProvider:
-    | (({
-        getDefault,
-        clusterSupplier,
-        serviceLocator,
-        customResources,
-        objectTypesToFetch,
-        authStrategy,
-      }: {
-        getDefault: () => KubernetesObjectsProvider;
-        clusterSupplier: KubernetesClustersSupplier;
-        serviceLocator: KubernetesServiceLocator;
-        customResources: CustomResource[];
-        objectTypesToFetch?: ObjectToFetch[];
-        authStrategy: AuthenticationStrategy;
-      }) => KubernetesObjectsProvider)
-    | undefined;
+  private objectsProvider: KubernetesObjectsProviderFactory | undefined;

  getObjectsProvider() {
    return this.objectsProvider;
  }

-  addObjectsProvider(provider: KubernetesObjectsProvider) {
+  addObjectsProvider(
+    provider: KubernetesObjectsProvider | KubernetesObjectsProviderFactory,
+  ) {
    if (this.objectsProvider) {
      throw new Error(
        'Multiple Kubernetes objects provider is not supported at this time',
      );
    }
    if (typeof provider !== 'function') {
-      this.objectsProvider = () => provider;
+      this.objectsProvider = async () => provider;
    } else {
      this.objectsProvider = provider;
    }
@@ -91,26 +81,24 @@ class ObjectsProvider implements KubernetesObjectsProviderExtensionPoint {
 }

 class ClusterSuplier implements KubernetesClusterSupplierExtensionPoint {
-  private clusterSupplier:
-    | (({
-        getDefault,
-      }: {
-        getDefault: () => KubernetesClustersSupplier;
-      }) => KubernetesClustersSupplier)
-    | undefined;
+  private clusterSupplier: KubernetesClusterSupplierFactory | undefined;

  getClusterSupplier() {
    return this.clusterSupplier;
  }

-  addClusterSupplier(clusterSupplier: KubernetesClustersSupplier) {
+  addClusterSupplier(
+    clusterSupplier:
+      | KubernetesClustersSupplier
+      | KubernetesClusterSupplierFactory,
+  ) {
    if (this.clusterSupplier) {
      throw new Error(
        'Multiple Kubernetes Cluster Suppliers is not supported at this time',
      );
    }
    if (typeof clusterSupplier !== 'function') {
-      this.clusterSupplier = () => clusterSupplier;
+      this.clusterSupplier = async () => clusterSupplier;
    } else {
      this.clusterSupplier = clusterSupplier;
    }
@@ -118,34 +106,20 @@ class ClusterSuplier implements KubernetesClusterSupplierExtensionPoint {
 }

 class Fetcher implements KubernetesFetcherExtensionPoint {
-  private fetcher:
-    | (({
-        getDefault,
-      }: {
-        getDefault: () => KubernetesFetcher;
-      }) => KubernetesFetcher)
-    | undefined;
+  private fetcher: KubernetesFetcherFactory | undefined;

  getFetcher() {
    return this.fetcher;
  }

-  addFetcher(
-    fetcher:
-      | KubernetesFetcher
-      | (({
-          getDefault,
-        }: {
-          getDefault: () => KubernetesFetcher;
-        }) => KubernetesFetcher),
-  ) {
+  addFetcher(fetcher: KubernetesFetcher | KubernetesFetcherFactory) {
    if (this.fetcher) {
      throw new Error(
        'Multiple Kubernetes Fetchers is not supported at this time',
      );
    }
    if (typeof fetcher !== 'function') {
-      this.fetcher = () => fetcher;
+      this.fetcher = async () => fetcher;
    } else {
      this.fetcher = fetcher;
    }
@@ -153,21 +127,15 @@ class Fetcher implements KubernetesFetcherExtensionPoint {
 }

 class ServiceLocator implements KubernetesServiceLocatorExtensionPoint {
-  private serviceLocator:
-    | (({
-        getDefault,
-        clusterSupplier,
-      }: {
-        getDefault: () => KubernetesServiceLocator;
-        clusterSupplier: KubernetesClustersSupplier;
-      }) => KubernetesServiceLocator)
-    | undefined;
+  private serviceLocator: KubernetesServiceLocatorFactory | undefined;

  getServiceLocator() {
    return this.serviceLocator;
  }

-  addServiceLocator(serviceLocator: KubernetesServiceLocator) {
+  addServiceLocator(
+    serviceLocator: KubernetesServiceLocator | KubernetesServiceLocatorFactory,
+  ) {
    if (this.serviceLocator) {
      throw new Error(
        'Multiple Kubernetes Service Locators is not supported at this time',
@@ -175,7 +143,7 @@ class ServiceLocator implements KubernetesServiceLocatorExtensionPoint {
    }

    if (typeof serviceLocator !== 'function') {
-      this.serviceLocator = () => serviceLocator;
+      this.serviceLocator = async () => serviceLocator;
    } else {
      this.serviceLocator = serviceLocator;
    }
@@ -256,9 +224,9 @@ export const kubernetesPlugin = createBackendPlugin({
        auth,
        httpAuth,
      }) {
+        // TODO: this could do with a cleanup and push some of this initalization somewhere else
        if (config.has('kubernetes')) {
-          // TODO: expose all of the customization & extension points of the builder here
-          const defaultFetcherFactory = () =>
+          const defaultFetcherFactory = async () =>
            new KubernetesClientBasedFetcher({
              logger,
            });
@@ -280,7 +248,7 @@ export const kubernetesPlugin = createBackendPlugin({
            minutes: 60,
          });

-          const defaultClusterSupplierFactory = () =>
+          const defaultClusterSupplierFactory = async () =>
            getCombinedClusterSupplier(
              config,
              catalog,
@@ -297,16 +265,16 @@ export const kubernetesPlugin = createBackendPlugin({
              getDefault: defaultClusterSupplierFactory,
            }) ?? defaultClusterSupplierFactory();

-          const defaultServiceLocatorFactory = () =>
+          const defaultServiceLocatorFactory = async () =>
            buildDefaultServiceLocator({
              config,
-              clusterSupplier,
+              clusterSupplier: await clusterSupplier,
            });

          const serviceLocator =
            extPointServiceLocator.getServiceLocator()?.({
              getDefault: defaultServiceLocatorFactory,
-              clusterSupplier: clusterSupplier,
+              clusterSupplier: await clusterSupplier,
            }) ?? defaultServiceLocatorFactory();

          const objectTypesToFetchStrings = config.getOptionalStringArray(
@@ -347,12 +315,12 @@ export const kubernetesPlugin = createBackendPlugin({
              } as CustomResource),
          );

-          const defaultObjectsProviderFactory = () =>
+          const defaultObjectsProviderFactory = async () =>
            new KubernetesFanOutHandler({
              logger,
              config,
-              fetcher,
-              serviceLocator,
+              fetcher: await fetcher,
+              serviceLocator: await serviceLocator,
              customResources,
              objectTypesToFetch,
              authStrategy: new DispatchStrategy({
@@ -362,9 +330,9 @@ export const kubernetesPlugin = createBackendPlugin({

          const objectsProvider =
            extPointObjectsProvider.getObjectsProvider()?.({
-              clusterSupplier,
+              clusterSupplier: await clusterSupplier,
              getDefault: defaultObjectsProviderFactory,
-              serviceLocator,
+              serviceLocator: await serviceLocator,
              customResources,
              objectTypesToFetch,
              authStrategy: new DispatchStrategy({
@@ -381,10 +349,10 @@ export const kubernetesPlugin = createBackendPlugin({
            auth,
            httpAuth,
            authStrategyMap: Object.fromEntries(authStrategyMap.entries()),
-            fetcher,
-            clusterSupplier,
-            serviceLocator,
-            objectsProvider,
+            fetcher: await fetcher,
+            clusterSupplier: await clusterSupplier,
+            serviceLocator: await serviceLocator,
+            objectsProvider: await objectsProvider,
          });

          const { router } = await builder.build();
@@ -100,17 +100,18 @@ export interface KubernetesClusterSupplierExtensionPoint {
  addClusterSupplier(
    clusterSupplier:
      | KubernetesClustersSupplier_2
-      | (({
-          getDefault,
-        }: {
-          getDefault: () => KubernetesClustersSupplier_2;
-        }) => KubernetesClustersSupplier_2),
+      | KubernetesClusterSupplierFactory,
  ): void;
 }

 // @public
 export const kubernetesClusterSupplierExtensionPoint: ExtensionPoint<KubernetesClusterSupplierExtensionPoint>;

+// @public
+export type KubernetesClusterSupplierFactory = (opts: {
+  getDefault: () => Promise<KubernetesClustersSupplier_2>;
+}) => Promise<KubernetesClustersSupplier_2>;
+
 // @public
 export type KubernetesCredential =
  | {
@@ -144,20 +145,17 @@ export interface KubernetesFetcher {
 // @public
 export interface KubernetesFetcherExtensionPoint {
  // (undocumented)
-  addFetcher(
-    fetcher:
-      | KubernetesFetcher_2
-      | (({
-          getDefault,
-        }: {
-          getDefault: () => KubernetesFetcher_2;
-        }) => KubernetesFetcher_2),
-  ): void;
+  addFetcher(fetcher: KubernetesFetcher_2 | KubernetesFetcherFactory): void;
 }

 // @public
 export const kubernetesFetcherExtensionPoint: ExtensionPoint<KubernetesFetcherExtensionPoint>;

+// @public
+export type KubernetesFetcherFactory = (opts: {
+  getDefault: () => Promise<KubernetesFetcher_2>;
+}) => Promise<KubernetesFetcher_2>;
+
 // @public (undocumented)
 export interface KubernetesObjectsByEntity {
  // (undocumented)
@@ -188,29 +186,23 @@ export interface KubernetesObjectsProvider {
 export interface KubernetesObjectsProviderExtensionPoint {
  // (undocumented)
  addObjectsProvider(
-    provider:
-      | KubernetesObjectsProvider_2
-      | (({
-          getDefault,
-          clusterSupplier,
-          serviceLocator,
-          customResources,
-          objectTypesToFetch,
-          authStrategy,
-        }: {
-          getDefault: () => KubernetesObjectsProvider_2;
-          clusterSupplier: KubernetesClustersSupplier_2;
-          serviceLocator: KubernetesServiceLocator_2;
-          customResources: CustomResource_2[];
-          objectTypesToFetch?: ObjectToFetch_2[];
-          authStrategy: AuthenticationStrategy_2;
-        }) => KubernetesObjectsProvider_2),
+    provider: KubernetesObjectsProvider_2 | KubernetesObjectsProviderFactory,
  ): void;
 }

 // @public
 export const kubernetesObjectsProviderExtensionPoint: ExtensionPoint<KubernetesObjectsProviderExtensionPoint>;

+// @public
+export type KubernetesObjectsProviderFactory = (opts: {
+  getDefault: () => Promise<KubernetesObjectsProvider_2>;
+  clusterSupplier: KubernetesClustersSupplier_2;
+  serviceLocator: KubernetesServiceLocator_2;
+  customResources: CustomResource_2[];
+  objectTypesToFetch?: ObjectToFetch_2[];
+  authStrategy: AuthenticationStrategy_2;
+}) => Promise<KubernetesObjectsProvider_2>;
+
 // @public (undocumented)
 export type KubernetesObjectTypes =
  | 'pods'
@@ -246,19 +238,19 @@ export interface KubernetesServiceLocatorExtensionPoint {
  addServiceLocator(
    serviceLocator:
      | KubernetesServiceLocator_2
-      | (({
-          getDefault,
-          clusterSupplier,
-        }: {
-          getDefault: () => KubernetesServiceLocator_2;
-          clusterSupplier: KubernetesClustersSupplier_2;
-        }) => KubernetesServiceLocator_2),
+      | KubernetesServiceLocatorFactory,
  ): void;
 }

 // @public
 export const kubernetesServiceLocatorExtensionPoint: ExtensionPoint<KubernetesServiceLocatorExtensionPoint>;

+// @public
+export type KubernetesServiceLocatorFactory = (opts: {
+  getDefault: () => Promise<KubernetesServiceLocator_2>;
+  clusterSupplier: KubernetesClustersSupplier_2;
+}) => Promise<KubernetesServiceLocator_2>;
+
 // @public (undocumented)
 export interface ObjectFetchParams {
  // (undocumented)
@@ -24,6 +24,20 @@ import {
  KubernetesServiceLocator,
 } from '@backstage/plugin-kubernetes-node';

+/**
+ * A factory function for creating a KubernetesObjectsProvider.
+ *
+ * @public
+ */
+export type KubernetesObjectsProviderFactory = (opts: {
+  getDefault: () => Promise<KubernetesObjectsProvider>;
+  clusterSupplier: KubernetesClustersSupplier;
+  serviceLocator: KubernetesServiceLocator;
+  customResources: CustomResource[];
+  objectTypesToFetch?: ObjectToFetch[];
+  authStrategy: AuthenticationStrategy;
+}) => Promise<KubernetesObjectsProvider>;
+
 /**
 * The interface for {@link kubernetesObjectsProviderExtensionPoint}.
 *
@@ -31,23 +45,7 @@ import {
 */
 export interface KubernetesObjectsProviderExtensionPoint {
  addObjectsProvider(
-    provider:
-      | KubernetesObjectsProvider
-      | (({
-          getDefault,
-          clusterSupplier,
-          serviceLocator,
-          customResources,
-          objectTypesToFetch,
-          authStrategy,
-        }: {
-          getDefault: () => KubernetesObjectsProvider;
-          clusterSupplier: KubernetesClustersSupplier;
-          serviceLocator: KubernetesServiceLocator;
-          customResources: CustomResource[];
-          objectTypesToFetch?: ObjectToFetch[];
-          authStrategy: AuthenticationStrategy;
-        }) => KubernetesObjectsProvider),
+    provider: KubernetesObjectsProvider | KubernetesObjectsProviderFactory,
  ): void;
 }

@@ -61,6 +59,15 @@ export const kubernetesObjectsProviderExtensionPoint =
    id: 'kubernetes.objects-provider',
  });

+/**
+ * A factory function for creating a KubernetesClustersSupplier.
+ *
+ * @public
+ */
+export type KubernetesClusterSupplierFactory = (opts: {
+  getDefault: () => Promise<KubernetesClustersSupplier>;
+}) => Promise<KubernetesClustersSupplier>;
+
 /**
 * The interface for {@link kubernetesClusterSupplierExtensionPoint}.
 *
@@ -70,11 +77,7 @@ export interface KubernetesClusterSupplierExtensionPoint {
  addClusterSupplier(
    clusterSupplier:
      | KubernetesClustersSupplier
-      | (({
-          getDefault,
-        }: {
-          getDefault: () => KubernetesClustersSupplier;
-        }) => KubernetesClustersSupplier),
+      | KubernetesClusterSupplierFactory,
  ): void;
 }

@@ -107,21 +110,22 @@ export const kubernetesAuthStrategyExtensionPoint =
    id: 'kubernetes.auth-strategy',
  });

+/**
+ * A factory function for creating a KubernetesFetcher.
+ *
+ * @public
+ */
+export type KubernetesFetcherFactory = (opts: {
+  getDefault: () => Promise<KubernetesFetcher>;
+}) => Promise<KubernetesFetcher>;
+
 /**
 * The interface for {@link kubernetesFetcherExtensionPoint}.
 *
 * @public
 */
 export interface KubernetesFetcherExtensionPoint {
-  addFetcher(
-    fetcher:
-      | KubernetesFetcher
-      | (({
-          getDefault,
-        }: {
-          getDefault: () => KubernetesFetcher;
-        }) => KubernetesFetcher),
-  ): void;
+  addFetcher(fetcher: KubernetesFetcher | KubernetesFetcherFactory): void;
 }

 /**
@@ -134,6 +138,16 @@ export const kubernetesFetcherExtensionPoint =
    id: 'kubernetes.fetcher',
  });

+/**
+ * A factory function for creating a KubernetesServiceLocator.
+ *
+ * @public
+ */
+export type KubernetesServiceLocatorFactory = (opts: {
+  getDefault: () => Promise<KubernetesServiceLocator>;
+  clusterSupplier: KubernetesClustersSupplier;
+}) => Promise<KubernetesServiceLocator>;
+
 /**
 * The interface for {@link kubernetesServiceLocatorExtensionPoint}.
 *
@@ -141,15 +155,7 @@ export const kubernetesFetcherExtensionPoint =
 */
 export interface KubernetesServiceLocatorExtensionPoint {
  addServiceLocator(
-    serviceLocator:
-      | KubernetesServiceLocator
-      | (({
-          getDefault,
-          clusterSupplier,
-        }: {
-          getDefault: () => KubernetesServiceLocator;
-          clusterSupplier: KubernetesClustersSupplier;
-        }) => KubernetesServiceLocator),
+    serviceLocator: KubernetesServiceLocator | KubernetesServiceLocatorFactory,
  ): void;
 }