minor refactorings after code review

Signed-off-by: Morgan Martinet <morgan@mmm-experts.com>

plugins/kubernetes/src/components/KubernetesDrawer/KubernetesDrawer.tsx

@@ -34,6 +34,7 @@ import jsYaml from 'js-yaml';
 import {
   CodeSnippet,
   StructuredMetadataTable,
+  Link,
 } from '@backstage/core-components';
 import { ClusterContext } from '../../hooks';
 import { formatClusterLink } from '../../utils/clusterLinks';
@@ -108,11 +109,11 @@ const KubernetesDrawerContent = <T extends KubernetesDrawerable>({
 
   const classes = useDrawerContentStyles();
   const cluster = useContext(ClusterContext);
-  const clusterLink = formatClusterLink(
-    cluster.dashboardUrl ?? '',
+  const clusterLink = formatClusterLink({
+    dashboardUrl: cluster.dashboardUrl,
     object,
     kind,
-  );
+  });
 
   return (
     <>
@@ -150,8 +151,8 @@ const KubernetesDrawerContent = <T extends KubernetesDrawerable>({
               variant="contained"
               color="primary"
               size="small"
-              href={clusterLink}
-              target="_blank"
+              component={Link}
+              to={clusterLink}
             >
               Open Kubernetes Dashboard...
             </Button>

plugins/kubernetes/src/utils/clusterLinks.test.ts

@@ -19,94 +19,98 @@ import { formatClusterLink } from './clusterLinks';
 describe('clusterLinks', () => {
   describe('formatClusterLink', () => {
     it('should not return an url when there is no dashboard url', () => {
-      const url = formatClusterLink('', {}, 'foo');
+      const url = formatClusterLink({ object: {}, kind: 'foo' });
       expect(url).toBeUndefined();
     });
     it('should return an url even when there is no object', () => {
-      const url = formatClusterLink('https://k8s.foo.com', undefined, 'foo');
+      const url = formatClusterLink({
+        dashboardUrl: 'https://k8s.foo.com',
+        object: undefined,
+        kind: 'foo',
+      });
       expect(url).toBe('https://k8s.foo.com');
     });
     it('should return an url on the workloads when there is a namespace only', () => {
-      const url = formatClusterLink(
-        'https://k8s.foo.com',
-        {
+      const url = formatClusterLink({
+        dashboardUrl: 'https://k8s.foo.com',
+        object: {
           metadata: {
             namespace: 'bar',
           },
         },
-        'foo',
-      );
+        kind: 'foo',
+      });
       expect(url).toBe('https://k8s.foo.com/#/workloads?namespace=bar');
     });
     it('should return an url on the workloads when the kind is not recognizeed', () => {
-      const url = formatClusterLink(
-        'https://k8s.foo.com',
-        {
+      const url = formatClusterLink({
+        dashboardUrl: 'https://k8s.foo.com',
+        object: {
           metadata: {
             name: 'foobar',
             namespace: 'bar',
           },
         },
-        'UnknownKind',
-      );
+        kind: 'UnknownKind',
+      });
       expect(url).toBe('https://k8s.foo.com/#/workloads?namespace=bar');
     });
     it('should return an url on the deployment', () => {
-      const url = formatClusterLink(
-        'https://k8s.foo.com/',
-        {
+      const url = formatClusterLink({
+        dashboardUrl: 'https://k8s.foo.com/',
+        object: {
           metadata: {
             name: 'foobar',
             namespace: 'bar',
           },
         },
-        'Deployment',
-      );
+        kind: 'Deployment',
+      });
       expect(url).toBe(
         'https://k8s.foo.com/#/deployment/bar/foobar?namespace=bar',
       );
     });
     it('should return an url on the service', () => {
-      const url = formatClusterLink(
-        'https://k8s.foo.com/',
-        {
+      const url = formatClusterLink({
+        dashboardUrl: 'https://k8s.foo.com/',
+        object: {
           metadata: {
             name: 'foobar',
             namespace: 'bar',
           },
         },
-        'Service',
-      );
+        kind: 'Service',
+      });
       expect(url).toBe(
         'https://k8s.foo.com/#/service/bar/foobar?namespace=bar',
       );
     });
     it('should return an url on the ingress', () => {
-      const url = formatClusterLink(
-        'https://k8s.foo.com/',
-        {
+      const url = formatClusterLink({
+        dashboardUrl: 'https://k8s.foo.com/',
+        object: {
           metadata: {
             name: 'foobar',
             namespace: 'bar',
           },
         },
-        'Ingress',
-      );
+        kind: 'Ingress',
+      });
       expect(url).toBe(
         'https://k8s.foo.com/#/ingress/bar/foobar?namespace=bar',
       );
     });
     it('should return an url on the deployment for a hpa', () => {
-      const url = formatClusterLink(
-        'https://k8s.foo.com/',
-        {
+      const url = formatClusterLink({
+        dashboardUrl: 'https://k8s.foo.com/',
+        object: {
           metadata: {
             name: 'foobar',
             namespace: 'bar',
           },
         },
-        'HorizontalPodAutoscaler',
-      );
+        kind: 'HorizontalPodAutoscaler',
+      });
       expect(url).toBe(
         'https://k8s.foo.com/#/deployment/bar/foobar?namespace=bar',
       );

plugins/kubernetes/src/utils/clusterLinks.ts

@@ -14,33 +14,37 @@
  * limitations under the License.
  */
 
-const KindMappings: any = {
+const KindMappings: Record<string, string> = {
   deployment: 'deployment',
   ingress: 'ingress',
   service: 'service',
   horizontalpodautoscaler: 'deployment',
 };
 
-export function formatClusterLink(
-  dashboardUrl: string,
-  object: any,
-  kind: string,
-) {
-  if (!dashboardUrl) {
+export function formatClusterLink(options: {
+  dashboardUrl?: string;
+  object: any;
+  kind: string;
+}) {
+  if (!options.dashboardUrl) {
     return undefined;
   }
-  if (!object) {
-    return dashboardUrl;
+  if (!options.object) {
+    return options.dashboardUrl;
   }
-  const host = dashboardUrl.endsWith('/') ? dashboardUrl : `${dashboardUrl}/`;
-  const name = object.metadata?.name;
-  const namespace = object.metadata?.namespace;
-  const validKind = KindMappings[kind.toLocaleLowerCase()];
+  const host = options.dashboardUrl.endsWith('/')
+    ? options.dashboardUrl
+    : `${options.dashboardUrl}/`;
+  const name = options.object.metadata?.name;
+  const namespace = options.object.metadata?.namespace;
+  const validKind = KindMappings[options.kind.toLocaleLowerCase()];
   if (validKind && name && namespace) {
-    return `${host}#/${validKind}/${namespace}/${name}?namespace=${namespace}`;
+    return `${host}#/${encodeURIComponent(validKind)}/${encodeURIComponent(
+      namespace,
+    )}/${encodeURIComponent(name)}?namespace=${encodeURIComponent(namespace)}`;
   }
   if (namespace) {
-    return `${host}#/workloads?namespace=${namespace}`;
+    return `${host}#/workloads?namespace=${encodeURIComponent(namespace)}`;
   }
-  return dashboardUrl;
+  return options.dashboardUrl;
 }