From ac01524f5e4b12e7cbfdd0a42038465ce1169249 Mon Sep 17 00:00:00 2001
From: ppadti <ppadti@redhat.com>
Date: Thu, 3 Oct 2024 18:33:48 +0530
Subject: [PATCH] Display internal addresses even when models are external

---
 .../cypress/cypress/pages/modelServing.ts     |   8 ++
 .../modelServing/servingRuntimeList.cy.ts     | 105 ++++++++++++++++++
 .../global/InferenceServiceEndpoint.tsx       |  46 +++++++-
 .../global/InternalServicePopoverContent.tsx  |  36 +++---
 4 files changed, 176 insertions(+), 19 deletions(-)

diff --git a/frontend/src/__tests__/cypress/cypress/pages/modelServing.ts b/frontend/src/__tests__/cypress/cypress/pages/modelServing.ts
index 50cc4f2fd8..1d202f8818 100644
--- a/frontend/src/__tests__/cypress/cypress/pages/modelServing.ts
+++ b/frontend/src/__tests__/cypress/cypress/pages/modelServing.ts
@@ -311,6 +311,14 @@ class InferenceServiceRow extends TableRow {
   findInternalServicePopover() {
     return cy.findByTestId('internal-service-popover');
   }
+
+  findExternalServiceButton() {
+    return this.find().findByTestId('internal-external-service-button');
+  }
+
+  findExternalServicePopover() {
+    return cy.findByTestId('external-service-popover');
+  }
 }
 class ServingPlatformCard extends Contextual<HTMLElement> {
   findDeployModelButton() {
diff --git a/frontend/src/__tests__/cypress/cypress/tests/mocked/modelServing/servingRuntimeList.cy.ts b/frontend/src/__tests__/cypress/cypress/tests/mocked/modelServing/servingRuntimeList.cy.ts
index a7691f7ef4..4b44614a33 100644
--- a/frontend/src/__tests__/cypress/cypress/tests/mocked/modelServing/servingRuntimeList.cy.ts
+++ b/frontend/src/__tests__/cypress/cypress/tests/mocked/modelServing/servingRuntimeList.cy.ts
@@ -481,6 +481,10 @@ describe('Serving Runtime List', () => {
           mockInferenceServiceK8sResource({
             name: 'another-inference-service',
             displayName: 'Another Inference Service',
+            statusPredictor: {
+              grpcUrl: 'grpc://modelmesh-serving.app:8033',
+              restUrl: 'http:///modelmesh-serving.app:8000',
+            },
             deleted: true,
             isModelMesh: true,
           }),
@@ -548,6 +552,107 @@ describe('Serving Runtime List', () => {
         .findInferenceServiceTableHeaderButton('Model name')
         .should(be.sortDescending);
     });
+
+    it('modelmesh inference endpoints when external route is enabled', () => {
+      initIntercepts({
+        projectEnableModelMesh: true,
+        disableKServeConfig: false,
+        disableModelMeshConfig: true,
+        inferenceServices: [
+          mockInferenceServiceK8sResource({
+            name: 'another-inference-service',
+            displayName: 'Another Inference Service',
+            statusPredictor: {
+              grpcUrl: 'grpc://modelmesh-serving.app:8033',
+              restUtl: 'http:///modelmesh-serving.app:8000',
+            },
+            deleted: true,
+            isModelMesh: true,
+          }),
+        ],
+      });
+
+      projectDetails.visitSection('test-project', 'model-server');
+      modelServingSection
+        .getModelMeshRow('OVMS Model Serving')
+        .findDeployedModelExpansionButton()
+        .click();
+      const inferenceServiceRow = modelServingSection.getInferenceServiceRow(
+        'Another Inference Service',
+      );
+      inferenceServiceRow.findExternalServiceButton().click();
+      inferenceServiceRow
+        .findExternalServicePopover()
+        .findByText('Internal (can only be accessed from inside the cluster)')
+        .should('exist');
+      inferenceServiceRow
+        .findExternalServicePopover()
+        .findByText('grpc://modelmesh-serving.app:8033')
+        .should('exist');
+      inferenceServiceRow
+        .findExternalServicePopover()
+        .findByText('http:///modelmesh-serving.app:8000')
+        .should('exist');
+      inferenceServiceRow
+        .findExternalServicePopover()
+        .findByText('External (can be accessed from inside or outside the cluster)')
+        .should('exist');
+      inferenceServiceRow
+        .findExternalServicePopover()
+        .findByText('https://another-inference-service-test-project.apps.user.com/infer')
+        .should('exist');
+    });
+
+    it('modelmesh inference endpoints when external route is not enabled', () => {
+      initIntercepts({
+        projectEnableModelMesh: true,
+        disableKServeConfig: false,
+        disableModelMeshConfig: true,
+        inferenceServices: [
+          mockInferenceServiceK8sResource({
+            name: 'another-inference-service',
+            displayName: 'Another Inference Service',
+            statusPredictor: {
+              grpcUrl: 'grpc://modelmesh-serving.app:8033',
+              restUtl: 'http:///modelmesh-serving.app:8000',
+            },
+            deleted: true,
+            isModelMesh: true,
+          }),
+        ],
+        servingRuntimes: [
+          mockServingRuntimeK8sResourceLegacy({}),
+          mockServingRuntimeK8sResource({
+            name: 'test-model',
+            namespace: 'test-project',
+            auth: true,
+            route: false,
+          }),
+        ],
+      });
+
+      projectDetails.visitSection('test-project', 'model-server');
+      modelServingSection
+        .getModelMeshRow('OVMS Model Serving')
+        .findDeployedModelExpansionButton()
+        .click();
+      const inferenceServiceRow = modelServingSection.getInferenceServiceRow(
+        'Another Inference Service',
+      );
+      inferenceServiceRow.findInternalServiceButton().click();
+      inferenceServiceRow
+        .findInternalServicePopover()
+        .findByText('Internal (can only be accessed from inside the cluster)')
+        .should('exist');
+      inferenceServiceRow
+        .findInternalServicePopover()
+        .findByText('grpc://modelmesh-serving.app:8033')
+        .should('exist');
+      inferenceServiceRow
+        .findInternalServicePopover()
+        .findByText('http:///modelmesh-serving.app:8000')
+        .should('exist');
+    });
   });
 
   describe('KServe', () => {
diff --git a/frontend/src/pages/modelServing/screens/global/InferenceServiceEndpoint.tsx b/frontend/src/pages/modelServing/screens/global/InferenceServiceEndpoint.tsx
index 1ab73f040a..4323957362 100644
--- a/frontend/src/pages/modelServing/screens/global/InferenceServiceEndpoint.tsx
+++ b/frontend/src/pages/modelServing/screens/global/InferenceServiceEndpoint.tsx
@@ -2,6 +2,12 @@ import * as React from 'react';
 import {
   Button,
   ClipboardCopy,
+  ClipboardCopyVariant,
+  DescriptionList,
+  DescriptionListDescription,
+  DescriptionListGroup,
+  DescriptionListTerm,
+  Divider,
   HelperText,
   HelperTextItem,
   Popover,
@@ -40,14 +46,15 @@ const InferenceServiceEndpoint: React.FC<InferenceServiceEndpointProps> = ({
     return (
       <Popover
         data-testid="internal-service-popover"
-        headerContent="Internal Service can be accessed inside the cluster"
+        headerContent="Inference endpoints"
         aria-label="Internal Service Info"
+        hasAutoWidth
         bodyContent={
           <InternalServicePopoverContent inferenceService={inferenceService} isKserve={isKserve} />
         }
       >
         <Button data-testid="internal-service-button" isInline variant="link">
-          Internal Service
+          Endpoint details (Internal)
         </Button>
       </Popover>
     );
@@ -68,9 +75,38 @@ const InferenceServiceEndpoint: React.FC<InferenceServiceEndpointProps> = ({
   }
 
   return (
-    <ClipboardCopy hoverTip="Copy" clickTip="Copied" isReadOnly>
-      {isKserve ? routeLink : `${routeLink}/infer`}
-    </ClipboardCopy>
+    <Popover
+      data-testid="external-service-popover"
+      headerContent="Inference endpoints"
+      aria-label="External Service Info"
+      hasAutoWidth
+      bodyContent={
+        <InternalServicePopoverContent inferenceService={inferenceService} isKserve={isKserve} />
+      }
+      footerContent={
+        <DescriptionList>
+          <DescriptionListGroup>
+            <Divider />
+            <DescriptionListTerm>
+              External (can be accessed from inside or outside the cluster)
+            </DescriptionListTerm>
+            <DescriptionListDescription style={{ paddingLeft: 'var(--pf-v5-global--spacer--md)' }}>
+              <ClipboardCopy
+                hoverTip="Copy"
+                clickTip="Copied"
+                variant={ClipboardCopyVariant.inlineCompact}
+              >
+                {isKserve ? routeLink : `${routeLink}/infer`}
+              </ClipboardCopy>
+            </DescriptionListDescription>
+          </DescriptionListGroup>
+        </DescriptionList>
+      }
+    >
+      <Button data-testid="internal-external-service-button" isInline variant="link">
+        Endpoint details (Internal and external)
+      </Button>
+    </Popover>
   );
 };
 
diff --git a/frontend/src/pages/modelServing/screens/global/InternalServicePopoverContent.tsx b/frontend/src/pages/modelServing/screens/global/InternalServicePopoverContent.tsx
index 82cce44ab3..67aaa3187a 100644
--- a/frontend/src/pages/modelServing/screens/global/InternalServicePopoverContent.tsx
+++ b/frontend/src/pages/modelServing/screens/global/InternalServicePopoverContent.tsx
@@ -6,6 +6,7 @@ import {
   DescriptionListDescription,
   DescriptionListGroup,
   DescriptionListTerm,
+  ListItem,
 } from '@patternfly/react-core';
 import { InferenceServiceKind } from '~/k8sTypes';
 
@@ -47,20 +48,27 @@ const InternalServicePopoverContent: React.FC<InternalServicePopoverContentProps
 
   return (
     <DescriptionList isCompact>
-      {Object.entries(isInternalServiceEnabled).map(([route, value]) => (
-        <DescriptionListGroup key={route}>
-          <DescriptionListTerm>{route}</DescriptionListTerm>
-          <DescriptionListDescription>
-            <ClipboardCopy
-              hoverTip="Copy"
-              clickTip="Copied"
-              variant={ClipboardCopyVariant.inlineCompact}
-            >
-              {value}
-            </ClipboardCopy>
-          </DescriptionListDescription>
-        </DescriptionListGroup>
-      ))}
+      <DescriptionListTerm>
+        Internal (can only be accessed from inside the cluster)
+      </DescriptionListTerm>
+      {Object.entries(isInternalServiceEnabled)
+        .slice(0, 2)
+        .map(([route, value]) => (
+          <DescriptionListGroup key={route}>
+            <DescriptionListTerm>
+              <ListItem>{route}</ListItem>
+            </DescriptionListTerm>
+            <DescriptionListDescription style={{ paddingLeft: 'var(--pf-v5-global--spacer--md)' }}>
+              <ClipboardCopy
+                hoverTip="Copy"
+                clickTip="Copied"
+                variant={ClipboardCopyVariant.inlineCompact}
+              >
+                {value}
+              </ClipboardCopy>
+            </DescriptionListDescription>
+          </DescriptionListGroup>
+        ))}
     </DescriptionList>
   );
 };