Merge branch 'opendatahub-io:main' into main

backend/src/routes/api/storage-class/index.ts

@@ -0,0 +1,27 @@
+import { FastifyReply, FastifyRequest } from 'fastify';
+import { StorageClassConfig, KubeFastifyInstance } from '../../../types';
+import { secureAdminRoute } from '../../../utils/route-security';
+import { updateStorageClassConfig } from './utils';
+
+export default async (fastify: KubeFastifyInstance): Promise<void> => {
+  fastify.put(
+    '/:storageClassName/config',
+    secureAdminRoute(fastify)(
+      async (
+        request: FastifyRequest<{
+          Body: StorageClassConfig;
+          Params: { storageClassName: string };
+        }>,
+        reply: FastifyReply,
+      ) => {
+        return updateStorageClassConfig(fastify, request)
+          .then((res) => {
+            return res;
+          })
+          .catch((res) => {
+            reply.send(res);
+          });
+      },
+    ),
+  );
+};

backend/src/routes/api/storage-class/utils.ts

@@ -0,0 +1,63 @@
+import { FastifyRequest } from 'fastify';
+import {
+  K8sResourceCommon,
+  KubeFastifyInstance,
+  ResponseStatus,
+  StorageClassConfig,
+} from '../../../types';
+import { isHttpError } from '../../../utils';
+import { errorHandler } from '../../../utils';
+
+export async function updateStorageClassConfig(
+  fastify: KubeFastifyInstance,
+  request: FastifyRequest<{ Body: StorageClassConfig; Params: { storageClassName: string } }>,
+): Promise<ResponseStatus> {
+  const params = request.params;
+  const body = request.body;
+
+  try {
+    // Fetch the existing custom object for the StorageClass
+    const storageClass = await fastify.kube.customObjectsApi.getClusterCustomObject(
+      'storage.k8s.io',
+      'v1',
+      'storageclasses',
+      params.storageClassName,
+    );
+
+    // Extract and update the annotations
+    const annotations = (storageClass.body as K8sResourceCommon).metadata.annotations || {};
+    annotations['opendatahub.io/sc-config'] = JSON.stringify(body);
+
+    // Patch the StorageClass with the new annotations
+    await fastify.kube.customObjectsApi.patchClusterCustomObject(
+      'storage.k8s.io',
+      'v1',
+      'storageclasses',
+      params.storageClassName,
+      {
+        metadata: {
+          annotations,
+        },
+      },
+      undefined,
+      undefined,
+      undefined,
+      {
+        headers: {
+          'Content-Type': 'application/merge-patch+json',
+        },
+      },
+    );
+
+    return { success: true, error: '' };
+  } catch (e) {
+    if (!isHttpError(e) || e.statusCode !== 404) {
+      fastify.log.error(e, 'Unable to update storage class metadata.');
+      return {
+        success: false,
+        error: `Unable to update storage class metadata: ${errorHandler(e)}`,
+      };
+    }
+    throw e;
+  }
+}

backend/src/types.ts

@@ -624,6 +624,14 @@ export type ImageTagInfo = {
 
 export type ImageType = 'byon' | 'jupyter' | 'other';
 
+export type StorageClassConfig = {
+  displayName: string;
+  isEnabled: boolean;
+  isDefault: boolean;
+  lastModified: string;
+  description?: string;
+};
+
 export type PersistentVolumeClaimKind = {
   apiVersion?: string;
   kind?: string;
@@ -1191,3 +1199,8 @@ export type ModelRegistryKind = K8sResourceCommon & {
     conditions?: K8sCondition[];
   };
 };
+
+export type ResponseStatus = {
+  success: boolean;
+  error: string;
+};

frontend/src/__mocks__/mlmd/mockGetArtifacts.ts

@@ -344,6 +344,17 @@ export const mockedArtifactsResponse: GetArtifactsResponse = {
       createTimeSinceEpoch: 1611399342384,
       lastUpdateTimeSinceEpoch: 1611399342384,
     },
+    {
+      id: 7,
+      typeId: 14,
+      type: 'system.Metrics',
+      uri: 's3://scalar-metrics-uri-scalar-metrics-uri',
+      properties: {},
+      customProperties: {},
+      state: 2,
+      createTimeSinceEpoch: 1611399342384,
+      lastUpdateTimeSinceEpoch: 1611399342384,
+    },
   ],
 };
 

frontend/src/__tests__/cypress/cypress/pages/pipelines/artifacts.ts

@@ -106,6 +106,14 @@ class ArtifactDetails {
     return cy.findByTestId(`custom-props-description-list-${label}`);
   }
 
+  findPropSection() {
+    return cy.findByTestId('artifact-properties-section');
+  }
+
+  findCustomPropSection() {
+    return cy.findByTestId('artifact-custom-properties-section');
+  }
+
   mockGetArtifactById(projectName: string, response: GrpcResponse) {
     return cy.interceptOdh(
       'POST /api/service/mlmd/:namespace/:serviceName/ml_metadata.MetadataStoreService/GetArtifactsByID',

frontend/src/__tests__/cypress/cypress/tests/mocked/pipelines/artifacts.cy.ts

@@ -90,16 +90,18 @@ describe('Artifacts', () => {
           3,
         );
         artifactsGlobal.visit(projectName);
-        artifactsTable.findRows().should('have.length', 5);
+        artifactsTable.findRows().should('have.length', 6);
       });
 
       it('name', () => {
         artifactsGlobal.selectFilterByName('Artifact');
         artifactsTable.mockGetArtifacts(
           projectName,
           mockGetArtifactsResponse({
-            artifacts: mockedArtifactsResponse.artifacts.filter((mockArtifact) =>
-              mockArtifact.customProperties.display_name.stringValue?.includes('metrics'),
+            artifacts: mockedArtifactsResponse.artifacts.filter(
+              (mockArtifact) =>
+                Object.entries(mockArtifact.customProperties).length !== 0 &&
+                mockArtifact.customProperties.display_name.stringValue?.includes('metrics'),
             ),
           }),
           1,
@@ -139,13 +141,25 @@ describe('Artifacts', () => {
         );
         artifactsGlobal.findFilterField().click();
         artifactsGlobal.selectFilterType('system.Metrics');
-        artifactsTable.findRows().should('have.length', 1);
+        artifactsTable.findRows().should('have.length', 2);
         artifactsTable.getRowByName('scalar metrics').find().should('be.visible');
       });
     });
   });
 
   describe('details', () => {
+    it('shows empty state for properties and custom properties', () => {
+      artifactDetails.mockGetArtifactById(
+        projectName,
+        mockGetArtifactsById({
+          artifacts: [mockedArtifactsResponse.artifacts[5]],
+          artifactTypes: [],
+        }),
+      );
+      artifactDetails.visit(projectName, '(No name)', '7');
+      artifactDetails.findPropSection().should('contain.text', 'No properties');
+      artifactDetails.findCustomPropSection().should('contain.text', 'No custom properties');
+    });
     it('shows Overview tab content', () => {
       artifactDetails.mockGetArtifactById(
         projectName,

frontend/src/__tests__/cypress/cypress/tests/mocked/pipelines/pipelineCreateRuns.cy.ts

@@ -164,11 +164,12 @@ describe('Pipeline create runs', () => {
       );
       createRunPage.find();
 
+      const veryLongDesc = 'Test description'.repeat(30); // A string over 255 characters
       // Fill out the form without a schedule and submit
       createRunPage.fillName(initialMockRuns[0].display_name);
       cy.findByTestId('duplicate-name-help-text').should('be.visible');
       createRunPage.fillName('New run');
-      createRunPage.fillDescription('New run description');
+      createRunPage.fillDescription(veryLongDesc);
       createRunPage.findExperimentSelect().should('contain.text', 'Select an experiment');
       createRunPage.findExperimentSelect().should('not.be.disabled').click();
       createRunPage.selectExperimentByName('Test experiment 1');
@@ -189,7 +190,7 @@ describe('Pipeline create runs', () => {
       cy.wait('@createRun').then((interception) => {
         expect(interception.request.body).to.eql({
           display_name: 'New run',
-          description: 'New run description',
+          description: veryLongDesc.substring(0, 255), // Verify the description in truncated
           pipeline_version_reference: {
             pipeline_id: 'test-pipeline',
             pipeline_version_id: 'test-pipeline-version',

frontend/src/components/ExpandableFormSection.tsx

@@ -0,0 +1,28 @@
+import * as React from 'react';
+import { ExpandableSection, ExpandableSectionProps, FormSection } from '@patternfly/react-core';
+
+type ExpandableFormSectionProps = Omit<ExpandableSectionProps, 'ref'> & {
+  initExpanded?: boolean;
+};
+
+const ExpandableFormSection: React.FC<ExpandableFormSectionProps> = ({
+  children,
+  initExpanded = false,
+  isIndented = true,
+  ...props
+}) => {
+  const [expanded, setExpanded] = React.useState<boolean>(initExpanded);
+
+  return (
+    <ExpandableSection
+      isIndented={isIndented}
+      isExpanded={expanded}
+      onToggle={(_ev, isExpanded) => setExpanded(isExpanded)}
+      {...props}
+    >
+      <FormSection>{children}</FormSection>
+    </ExpandableSection>
+  );
+};
+
+export default ExpandableFormSection;

frontend/src/components/NumberInputWrapper.scss

@@ -0,0 +1,15 @@
+.odh-number-input-wrapper {
+  &.m-full-width {
+    width: 100%;
+    display: flex;
+    .pf-v5-c-input-group__item:where(:not(:first-child)):where(:not(:last-child)) {
+      flex: 1;
+      .pf-v5-c-form-control {
+        width: 100%;
+      }
+    }
+    .pf-v5-c-number-input__unit {
+      white-space: nowrap;
+    }
+  }
+}

frontend/src/components/NumberInputWrapper.tsx

@@ -1,21 +1,29 @@
 import * as React from 'react';
 import { NumberInput } from '@patternfly/react-core';
 
+import './NumberInputWrapper.scss';
+
 type NumberInputWrapperProps = {
-  onBlur?: (blurValue: number) => void;
-  onChange?: (newValue: number) => void;
+  onBlur?: (blurValue: number | undefined) => void;
+  onChange?: (newValue: number | undefined) => void;
+  intOnly?: boolean;
+  fullWidth?: boolean;
 } & Omit<React.ComponentProps<typeof NumberInput>, 'onChange' | 'onPlus' | 'onMinus'>;
 
 const NumberInputWrapper: React.FC<NumberInputWrapperProps> = ({
   onBlur,
   onChange,
+  intOnly = true,
+  fullWidth = false,
   value,
   validated,
   min,
   max,
   ...otherProps
 }) => (
   <NumberInput
+    className={fullWidth ? 'odh-number-input-wrapper m-full-width' : undefined}
+    inputProps={{ placeholder: '' }}
     {...otherProps}
     min={min}
     max={max}
@@ -24,26 +32,49 @@ const NumberInputWrapper: React.FC<NumberInputWrapperProps> = ({
     onChange={
       onChange
         ? (e) => {
-            let v = parseInt(e.currentTarget.value);
-            if (min) {
-              v = Math.max(v, min);
-            }
-            if (max) {
-              v = Math.min(v, max);
+            if (e.type === 'change') {
+              let v: number = intOnly
+                ? parseInt(e.currentTarget.value)
+                : parseFloat(e.currentTarget.value);
+              if (Number.isNaN(v)) {
+                e.currentTarget.value = '';
+                onChange(undefined);
+                return;
+              }
+              if (min) {
+                v = Math.max(v, min);
+              }
+              if (max) {
+                v = Math.min(v, max);
+              }
+              onChange(v);
             }
-            onChange(v);
           }
         : undefined
     }
-    onBlur={
-      onBlur &&
-      ((e) => {
-        onBlur(parseInt(e.currentTarget.value));
-      })
+    onBlur={(e) => {
+      // eslint-disable-next-line no-param-reassign
+      e.target.value = value ?? '';
+      if (onBlur) {
+        onBlur(value);
+      }
+    }}
+    onPlus={
+      onChange
+        ? () => {
+            const newVal = (value || 0) + 1;
+            onChange(min ? Math.max(newVal, min) : newVal);
+          }
+        : undefined
+    }
+    onMinus={
+      onChange
+        ? () => {
+            const newVal = (value || 0) - 1;
+            onChange(max ? Math.min(newVal, max) : newVal);
+          }
+        : undefined
     }
-    onPlus={onChange ? () => onChange((value || 0) + 1) : undefined}
-    onMinus={onChange ? () => onChange((value || 0) - 1) : undefined}
   />
 );
-
 export default NumberInputWrapper;