minor changes after code review

Signed-off-by: Morgan Martinet <morgan.martinet@montreal.ca>
2021-09-05 20:02:07 -04:00
parent 991356f3af
commit a65206d5a4
17 changed files with 83 additions and 28 deletions
@@ -104,9 +104,9 @@ kubectl -n <NAMESPACE> get secret $(kubectl -n <NAMESPACE> get sa <SERVICE_ACCOU

 Specifies the link to the Kubernetes dashboard managing this cluster.

-Note that you need to specify the app used for the dashboard using the
+Note that you should specify the app used for the dashboard using the
 **dashboardApp property**, in order to properly format links to kubernetes
-resources.
+resources, otherwise it will assume that you're running the standard one.

 ##### `clusters.\*.dashboardApp` (optional)

@@ -116,7 +116,7 @@ This will be used for formatting links to kubernetes objects inside the
 dashboard.

 The supported dashboards are: `standard`, `rancher`, `openshift`, `gke`, `aks`,
-`eks`
+`eks` However, not all of them are implemented yet, so please contribute!

 Note that it will default to the regular dashboard provided by the Kubernetes
 project (`standard`), that can run in any Kubernetes cluster.
@@ -86,9 +86,9 @@ export interface ClusterDetails {
  /**
   * Specifies the link to the Kubernetes dashboard managing this cluster.
   * @remarks
-   * Note that you need to specify the app used for the dashboard
+   * Note that you should specify the app used for the dashboard
   * using the dashboardApp property, in order to properly format
-   * links to kubernetes resources.
+   * links to kubernetes resources,  otherwise it will assume that you're running the standard one.
   * @see dashboardApp
   */
  dashboardUrl?: string;
@@ -40,9 +40,9 @@ export interface ClusterAttributes {
  /**
   * Specifies the link to the Kubernetes dashboard managing this cluster.
   * @remarks
-   * Note that you need to specify the app used for the dashboard
+   * Note that you should specify the app used for the dashboard
   * using the dashboardApp property, in order to properly format
-   * links to kubernetes resources.
+   * links to kubernetes resources,  otherwise it will assume that you're running the standard one.
   * @see dashboardApp
   */
  dashboardUrl?: string;
@@ -25,15 +25,13 @@ export const EntityKubernetesContent: (_props: {
  entity?: Entity | undefined;
 }) => JSX.Element;

+// Warning: (ae-forgotten-export) The symbol "FormatClusterLinkOptions" needs to be exported by the entry point index.d.ts
 // Warning: (ae-missing-release-tag) "formatClusterLink" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
 //
 // @public (undocumented)
-export function formatClusterLink(options: {
-  dashboardUrl?: string;
-  dashboardApp?: string;
-  object: any;
-  kind: string;
-}): string | undefined;
+export function formatClusterLink(
+  options: FormatClusterLinkOptions,
+): string | undefined;

 // Warning: (ae-forgotten-export) The symbol "KubernetesAuthProvidersApi" needs to be exported by the entry point index.d.ts
 // Warning: (ae-missing-release-tag) "KubernetesAuthProviders" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
@@ -36,9 +36,12 @@ import {
  CodeSnippet,
  StructuredMetadataTable,
  Link,
+  WarningPanel,
 } from '@backstage/core-components';
 import { ClusterContext } from '../../hooks';
 import { formatClusterLink } from '../../utils/clusterLinks';
+import { ClusterAttributes } from '@backstage/plugin-kubernetes-common';
+import { FormatClusterLinkOptions } from '../../utils/clusterLinks/formatClusterLink';

 const useDrawerStyles = makeStyles((theme: Theme) =>
  createStyles({
@@ -57,6 +60,10 @@ const useDrawerContentStyles = makeStyles((_: Theme) =>
      flexDirection: 'row',
      justifyContent: 'space-between',
    },
+    errorMessage: {
+      marginTop: '1em',
+      marginBottom: '1em',
+    },
    options: {
      display: 'flex',
      flexDirection: 'row',
@@ -80,6 +87,27 @@ const PodDrawerButton = withStyles({
  },
 })(Button);

+type ErrorPanelProps = {
+  cluster: ClusterAttributes;
+  errorMessage?: string;
+  children?: React.ReactNode;
+};
+
+export const ErrorPanel = ({ cluster, errorMessage }: ErrorPanelProps) => (
+  <WarningPanel
+    title="There was a problem formatting the link to the Kubernetes dashboard"
+    message={`Could not format the link to the dashboard of your cluster named '${
+      cluster.name
+    }'. Its dashboardApp property has been set to '${
+      cluster.dashboardApp || 'standard'
+    }.'`}
+  >
+    {errorMessage && (
+      <Typography variant="body2">Errors: {errorMessage}</Typography>
+    )}
+  </WarningPanel>
+);
+
 interface KubernetesDrawerable {
  metadata?: V1ObjectMeta;
 }
@@ -100,6 +128,20 @@ function replaceNullsWithUndefined(someObj: any) {
  return JSON.parse(JSON.stringify(someObj, replacer));
 }

+function tryFormatClusterLink(options: FormatClusterLinkOptions) {
+  try {
+    return {
+      clusterLink: formatClusterLink(options),
+      errorMessage: '',
+    };
+  } catch (err) {
+    return {
+      clusterLink: '',
+      errorMessage: err.message || err.toString(),
+    };
+  }
+}
+
 const KubernetesDrawerContent = <T extends KubernetesDrawerable>({
  toggleDrawer,
  object,
@@ -110,7 +152,7 @@ const KubernetesDrawerContent = <T extends KubernetesDrawerable>({

  const classes = useDrawerContentStyles();
  const cluster = useContext(ClusterContext);
-  const clusterLink = formatClusterLink({
+  const { clusterLink, errorMessage } = tryFormatClusterLink({
    dashboardUrl: cluster.dashboardUrl,
    dashboardApp: cluster.dashboardApp,
    object,
@@ -146,6 +188,11 @@ const KubernetesDrawerContent = <T extends KubernetesDrawerable>({
          <Close className={classes.icon} />
        </IconButton>
      </div>
+      {errorMessage && (
+        <div className={classes.errorMessage}>
+          <ErrorPanel cluster={cluster} errorMessage={errorMessage} />
+        </div>
+      )}
      <div className={classes.options}>
        <div>
          {clusterLink && (
@@ -16,12 +16,14 @@

 import { defaultFormatterName, clusterLinksFormatters } from './formatters';

-export function formatClusterLink(options: {
+export type FormatClusterLinkOptions = {
  dashboardUrl?: string;
  dashboardApp?: string;
  object: any;
  kind: string;
-}) {
+};
+
+export function formatClusterLink(options: FormatClusterLinkOptions) {
  if (!options.dashboardUrl) {
    return undefined;
  }
@@ -28,6 +28,6 @@ describe('clusterLinks - AKS formatter', () => {
        },
        kind: 'Deployment',
      }),
-    ).toThrowError('AKS formatter is not yet implemented');
+    ).toThrowError('AKS formatter is not yet implemented. Please, contribute!');
  });
 });
@@ -16,5 +16,5 @@
 import { ClusterLinksFormatterOptions } from '../../../types/types';

 export function aksFormatter(_options: ClusterLinksFormatterOptions): URL {
-  throw new Error('AKS formatter is not yet implemented');
+  throw new Error('AKS formatter is not yet implemented. Please, contribute!');
 }
@@ -28,6 +28,6 @@ describe('clusterLinks - EKS formatter', () => {
        },
        kind: 'Deployment',
      }),
-    ).toThrowError('EKS formatter is not yet implemented');
+    ).toThrowError('EKS formatter is not yet implemented. Please, contribute!');
  });
 });
@@ -16,5 +16,5 @@
 import { ClusterLinksFormatterOptions } from '../../../types/types';

 export function eksFormatter(_options: ClusterLinksFormatterOptions): URL {
-  throw new Error('EKS formatter is not yet implemented');
+  throw new Error('EKS formatter is not yet implemented. Please, contribute!');
 }
@@ -28,6 +28,6 @@ describe('clusterLinks - GKE formatter', () => {
        },
        kind: 'Deployment',
      }),
-    ).toThrowError('GKE formatter is not yet implemented');
+    ).toThrowError('GKE formatter is not yet implemented. Please, contribute!');
  });
 });
@@ -16,5 +16,5 @@
 import { ClusterLinksFormatterOptions } from '../../../types/types';

 export function gkeFormatter(_options: ClusterLinksFormatterOptions): URL {
-  throw new Error('GKE formatter is not yet implemented');
+  throw new Error('GKE formatter is not yet implemented. Please, contribute!');
 }
@@ -28,6 +28,8 @@ describe('clusterLinks - OpenShift formatter', () => {
        },
        kind: 'Deployment',
      }),
-    ).toThrowError('OpenShift formatter is not yet implemented');
+    ).toThrowError(
+      'OpenShift formatter is not yet implemented. Please, contribute!',
+    );
  });
 });
@@ -18,5 +18,7 @@ import { ClusterLinksFormatterOptions } from '../../../types/types';
 export function openshiftFormatter(
  _options: ClusterLinksFormatterOptions,
 ): URL {
-  throw new Error('OpenShift formatter is not yet implemented');
+  throw new Error(
+    'OpenShift formatter is not yet implemented. Please, contribute!',
+  );
 }
@@ -28,6 +28,8 @@ describe('clusterLinks - Rancher formatter', () => {
        },
        kind: 'Deployment',
      }),
-    ).toThrowError('Rancher formatter is not yet implemented');
+    ).toThrowError(
+      'Rancher formatter is not yet implemented. Please, contribute!',
+    );
  });
 });
@@ -16,5 +16,7 @@
 import { ClusterLinksFormatterOptions } from '../../../types/types';

 export function rancherFormatter(_options: ClusterLinksFormatterOptions): URL {
-  throw new Error('Rancher formatter is not yet implemented');
+  throw new Error(
+    'Rancher formatter is not yet implemented. Please, contribute!',
+  );
 }
@@ -15,7 +15,7 @@
 */
 import { ClusterLinksFormatterOptions } from '../../../types/types';

-const KindMappings: Record<string, string> = {
+const kindMappings: Record<string, string> = {
  deployment: 'deployment',
  ingress: 'ingress',
  service: 'service',
@@ -26,7 +26,7 @@ export function standardFormatter(options: ClusterLinksFormatterOptions) {
  const result = new URL(options.dashboardUrl.href);
  const name = options.object.metadata?.name;
  const namespace = options.object.metadata?.namespace;
-  const validKind = KindMappings[options.kind.toLocaleLowerCase()];
+  const validKind = kindMappings[options.kind.toLocaleLowerCase('en-US')];
  if (namespace) {
    result.searchParams.set('namespace', namespace);
  }