diff --git a/frontend/src/__mocks__/mockModelRegistryService.ts b/frontend/src/__mocks__/mockModelRegistryService.ts new file mode 100644 index 0000000000..c90b17e6dc --- /dev/null +++ b/frontend/src/__mocks__/mockModelRegistryService.ts @@ -0,0 +1,38 @@ +import { ServiceKind } from '~/k8sTypes'; + +type MockServiceType = { + name?: string; + namespace?: string; +}; + +export const mockModelRegistryService = ({ + name = 'modelregistry-sample', + namespace = 'odh-model-registries', +}: MockServiceType): ServiceKind => ({ + kind: 'Service', + apiVersion: 'v1', + metadata: { + name, + namespace, + }, + spec: { + selector: { + app: name, + component: 'model-registry', + }, + ports: [ + { + name: 'grpc-api', + protocol: 'TCP', + port: 9090, + targetPort: 9090, + }, + { + name: 'http-api', + protocol: 'TCP', + port: 8080, + targetPort: 8080, + }, + ], + }, +}); diff --git a/frontend/src/__mocks__/mockSelfSubjectRulesReview.ts b/frontend/src/__mocks__/mockSelfSubjectRulesReview.ts new file mode 100644 index 0000000000..16dc9dc07f --- /dev/null +++ b/frontend/src/__mocks__/mockSelfSubjectRulesReview.ts @@ -0,0 +1,238 @@ +import { SelfSubjectRulesReviewKind } from '~/k8sTypes'; + +export const mockSelfSubjectRulesReview = (): SelfSubjectRulesReviewKind => ({ + kind: 'SelfSubjectRulesReview', + apiVersion: 'authorization.k8s.io/v1', + spec: { + namespace: 'odh-model-registries', + }, + status: { + resourceRules: [ + { + verbs: ['create'], + apiGroups: ['', 'project.openshift.io'], + resources: ['projectrequests'], + }, + { + verbs: ['impersonate'], + apiGroups: ['authentication.k8s.io'], + resources: ['userextras/scopes.authorization.openshift.io'], + }, + { + verbs: ['get', 'list', 'watch'], + apiGroups: ['helm.openshift.io'], + resources: ['helmchartrepositories'], + }, + { + verbs: ['create'], + apiGroups: ['authorization.k8s.io'], + resources: ['selfsubjectaccessreviews', 'selfsubjectrulesreviews'], + }, + { + verbs: ['create'], + apiGroups: ['authentication.k8s.io'], + resources: ['selfsubjectreviews'], + }, + { + verbs: ['create'], + apiGroups: ['', 'project.openshift.io'], + resources: ['projectrequests'], + }, + { + verbs: ['get', 'list', 'watch', 'delete'], + apiGroups: ['oauth.openshift.io'], + resources: ['useroauthaccesstokens'], + }, + { + verbs: ['get', 'list', 'watch'], + apiGroups: ['snapshot.storage.k8s.io'], + resources: ['volumesnapshotclasses'], + }, + { + verbs: ['get'], + apiGroups: ['', 'user.openshift.io'], + resources: ['users'], + resourceNames: ['~'], + }, + { + verbs: ['list'], + apiGroups: ['', 'project.openshift.io'], + resources: ['projectrequests'], + }, + { + verbs: ['get', 'list'], + apiGroups: ['', 'authorization.openshift.io'], + resources: ['clusterroles'], + }, + { + verbs: ['get', 'list', 'watch'], + apiGroups: ['rbac.authorization.k8s.io'], + resources: ['clusterroles'], + }, + { + verbs: ['get', 'list'], + apiGroups: ['storage.k8s.io'], + resources: ['storageclasses'], + }, + { + verbs: ['list', 'watch'], + apiGroups: ['', 'project.openshift.io'], + resources: ['projects'], + }, + { + verbs: ['create'], + apiGroups: ['', 'authorization.openshift.io'], + resources: ['selfsubjectrulesreviews'], + }, + { + verbs: ['create'], + apiGroups: ['authorization.k8s.io'], + resources: ['selfsubjectaccessreviews'], + }, + { + verbs: ['delete'], + apiGroups: ['', 'oauth.openshift.io'], + resources: ['oauthaccesstokens', 'oauthauthorizetokens'], + }, + { + verbs: ['create'], + apiGroups: ['', 'build.openshift.io'], + resources: ['builds/source'], + }, + { + verbs: ['create'], + apiGroups: ['', 'authorization.openshift.io'], + resources: ['selfsubjectrulesreviews'], + }, + { + verbs: ['create'], + apiGroups: ['authorization.k8s.io'], + resources: ['selfsubjectaccessreviews'], + }, + { + verbs: ['create', 'get'], + apiGroups: ['', 'build.openshift.io'], + resources: ['buildconfigs/webhooks'], + }, + { + verbs: ['create'], + apiGroups: ['', 'build.openshift.io'], + resources: ['builds/jenkinspipeline'], + }, + { + verbs: ['use'], + apiGroups: ['security.openshift.io'], + resources: ['securitycontextconstraints'], + resourceNames: ['restricted-v2'], + }, + { + verbs: ['get', 'list', 'watch'], + apiGroups: ['console.openshift.io'], + resources: [ + 'consoleclidownloads', + 'consoleexternalloglinks', + 'consolelinks', + 'consolenotifications', + 'consoleplugins', + 'consolequickstarts', + 'consolesamples', + 'consoleyamlsamples', + ], + }, + { + verbs: ['create'], + apiGroups: ['', 'build.openshift.io'], + resources: ['builds/docker', 'builds/optimizeddocker'], + }, + { + verbs: ['get'], + apiGroups: [''], + resources: ['services'], + resourceNames: ['modelregistry-sample'], + }, + { + verbs: ['get'], + apiGroups: [''], + resources: ['services'], + resourceNames: ['dallas-mr'], + }, + ], + nonResourceRules: [ + { + verbs: ['get'], + nonResourceURLs: ['/healthz', '/healthz/'], + }, + { + verbs: ['get'], + nonResourceURLs: [ + '/version', + '/version/*', + '/api', + '/api/*', + '/apis', + '/apis/*', + '/oapi', + '/oapi/*', + '/openapi/v2', + '/swaggerapi', + '/swaggerapi/*', + '/swagger.json', + '/swagger-2.0.0.pb-v1', + '/osapi', + '/osapi/', + '/.well-known', + '/.well-known/oauth-authorization-server', + '/', + ], + }, + { + verbs: ['get'], + nonResourceURLs: [ + '/version', + '/version/*', + '/api', + '/api/*', + '/apis', + '/apis/*', + '/oapi', + '/oapi/*', + '/openapi/v2', + '/swaggerapi', + '/swaggerapi/*', + '/swagger.json', + '/swagger-2.0.0.pb-v1', + '/osapi', + '/osapi/', + '/.well-known', + '/.well-known/oauth-authorization-server', + '/', + ], + }, + { + verbs: ['get'], + nonResourceURLs: [ + '/api', + '/api/*', + '/apis', + '/apis/*', + '/healthz', + '/livez', + '/openapi', + '/openapi/*', + '/readyz', + '/version', + '/version/', + ], + }, + { + verbs: ['get'], + nonResourceURLs: ['/.well-known', '/.well-known/*'], + }, + { + verbs: ['get'], + nonResourceURLs: ['/healthz', '/livez', '/readyz', '/version', '/version/'], + }, + ], + incomplete: false, + }, +}); diff --git a/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelRegistry.cy.ts b/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelRegistry.cy.ts index 1e110dff25..e59b33e37c 100644 --- a/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelRegistry.cy.ts +++ b/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelRegistry.cy.ts @@ -1,16 +1,23 @@ /* eslint-disable camelcase */ -import { mockK8sResourceList, mockRouteK8sResourceModelRegistry } from '~/__mocks__'; +import { mockK8sResourceList } from '~/__mocks__'; import { mockComponents } from '~/__mocks__/mockComponents'; import { mockDashboardConfig } from '~/__mocks__/mockDashboardConfig'; -import { mockModelRegistry } from '~/__mocks__/mockModelRegistry'; import { mockRegisteredModelList } from '~/__mocks__/mockRegisteredModelsList'; import { labelModal, modelRegistry } from '~/__tests__/cypress/cypress/pages/modelRegistry'; import { be } from '~/__tests__/cypress/cypress/utils/should'; -import { ModelRegistryModel, RouteModel } from '~/__tests__/cypress/cypress/utils/models'; +import { + SelfSubjectAccessReviewModel, + SelfSubjectRulesReviewModel, + ServiceModel, +} from '~/__tests__/cypress/cypress/utils/models'; import { mockModelVersionList } from '~/__mocks__/mockModelVersionList'; import { mockModelVersion } from '~/__mocks__/mockModelVersion'; import type { ModelVersion, RegisteredModel } from '~/concepts/modelRegistry/types'; import { mockRegisteredModel } from '~/__mocks__/mockRegisteredModel'; +import { mockModelRegistryService } from '~/__mocks__/mockModelRegistryService'; +import { asProjectEditUser } from '~/__tests__/cypress/cypress/utils/mockUsers'; +import { mockSelfSubjectRulesReview } from '~/__mocks__/mockSelfSubjectRulesReview'; +import { mockSelfSubjectAccessReview } from '~/__mocks__/mockSelfSubjectAccessReview'; const MODEL_REGISTRY_API_VERSION = 'v1alpha3'; @@ -18,6 +25,7 @@ type HandlersProps = { disableModelRegistryFeature?: boolean; registeredModels?: RegisteredModel[]; modelVersions?: ModelVersion[]; + allowed?: boolean; }; const initIntercepts = ({ @@ -56,6 +64,7 @@ const initIntercepts = ({ mockModelVersion({ author: 'Author 1' }), mockModelVersion({ name: 'model version' }), ], + allowed = true, }: HandlersProps) => { cy.interceptOdh( 'GET /api/config', @@ -65,18 +74,28 @@ const initIntercepts = ({ ); cy.interceptOdh('GET /api/components', { query: { installed: 'true' } }, mockComponents()); + cy.interceptK8s('POST', SelfSubjectRulesReviewModel, mockSelfSubjectRulesReview()); + cy.interceptK8sList( - ModelRegistryModel, - mockK8sResourceList([mockModelRegistry({}), mockModelRegistry({ name: 'test-registry' })]), + ServiceModel, + mockK8sResourceList([ + mockModelRegistryService({ name: 'modelregistry-sample' }), + mockModelRegistryService({ name: 'modelregistry-sample-2' }), + ]), ); - cy.interceptK8s(ModelRegistryModel, mockModelRegistry({})); + cy.interceptK8s(ServiceModel, mockModelRegistryService({ name: 'modelregistry-sample' })); + + cy.interceptK8s(ServiceModel, mockModelRegistryService({ name: 'dallas-mr' })); cy.interceptK8s( - RouteModel, - mockRouteK8sResourceModelRegistry({ - name: 'modelregistry-sample-http', - namespace: 'odh-model-registries', + 'POST', + SelfSubjectAccessReviewModel, + mockSelfSubjectAccessReview({ + verb: 'list', + resource: 'services', + group: 'user.openshift.io', + allowed, }), ); @@ -187,4 +206,16 @@ describe('Model Registry', () => { modelRegistry.findTableRows().should('have.length', 1); modelRegistry.findTableRows().contains('Fraud detection model'); }); + + it('should be accessible for non-admin users', () => { + asProjectEditUser(); + initIntercepts({ + disableModelRegistryFeature: false, + allowed: false, + }); + + modelRegistry.visit(); + modelRegistry.navigate(); + modelRegistry.shouldModelRegistrySelectorExist(); + }); }); diff --git a/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelVersionArchive.cy.ts b/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelVersionArchive.cy.ts index bc768236a5..ce1fc616a2 100644 --- a/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelVersionArchive.cy.ts +++ b/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelVersionArchive.cy.ts @@ -1,9 +1,8 @@ /* eslint-disable camelcase */ import { mockK8sResourceList } from '~/__mocks__'; import { mockDashboardConfig } from '~/__mocks__/mockDashboardConfig'; -import { mockModelRegistry } from '~/__mocks__/mockModelRegistry'; import { mockRegisteredModelList } from '~/__mocks__/mockRegisteredModelsList'; -import { ModelRegistryModel } from '~/__tests__/cypress/cypress/utils/models'; +import { ServiceModel } from '~/__tests__/cypress/cypress/utils/models'; import { mockModelVersionList } from '~/__mocks__/mockModelVersionList'; import { mockModelVersion } from '~/__mocks__/mockModelVersion'; import type { ModelVersion } from '~/concepts/modelRegistry/types'; @@ -16,6 +15,7 @@ import { restoreVersionModal, } from '~/__tests__/cypress/cypress/pages/modelRegistry/modelVersionArchive'; import { labelModal, modelRegistry } from '~/__tests__/cypress/cypress/pages/modelRegistry'; +import { mockModelRegistryService } from '~/__mocks__/mockModelRegistryService'; const MODEL_REGISTRY_API_VERSION = 'v1alpha3'; @@ -55,12 +55,13 @@ const initIntercepts = ({ ); cy.interceptK8sList( - ModelRegistryModel, - mockK8sResourceList([mockModelRegistry({}), mockModelRegistry({ name: 'test-registry' })]), + ServiceModel, + mockK8sResourceList([ + mockModelRegistryService({ name: 'modelregistry-sample' }), + mockModelRegistryService({ name: 'modelregistry-sample-2' }), + ]), ); - cy.interceptK8s(ModelRegistryModel, mockModelRegistry({})); - cy.interceptOdh( 'GET /api/service/modelregistry/:serviceName/api/model_registry/:apiVersion/registered_models', { path: { serviceName: 'modelregistry-sample', apiVersion: MODEL_REGISTRY_API_VERSION } }, diff --git a/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelVersionDetails.cy.ts b/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelVersionDetails.cy.ts index cc1084ecf6..ac956b5520 100644 --- a/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelVersionDetails.cy.ts +++ b/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelVersionDetails.cy.ts @@ -1,18 +1,14 @@ /* eslint-disable camelcase */ -import { - mockDashboardConfig, - mockK8sResourceList, - mockRouteK8sResourceModelRegistry, -} from '~/__mocks__'; +import { mockDashboardConfig, mockK8sResourceList } from '~/__mocks__'; import { mockComponents } from '~/__mocks__/mockComponents'; -import { mockModelRegistry } from '~/__mocks__/mockModelRegistry'; -import { ModelRegistryModel, RouteModel } from '~/__tests__/cypress/cypress/utils/models'; +import { ServiceModel } from '~/__tests__/cypress/cypress/utils/models'; import { modelVersionDetails } from '~/__tests__/cypress/cypress/pages/modelRegistry/modelVersionDetails'; import { mockRegisteredModel } from '~/__mocks__/mockRegisteredModel'; import { mockModelVersion } from '~/__mocks__/mockModelVersion'; import { mockModelVersionList } from '~/__mocks__/mockModelVersionList'; import { mockModelArtifactList } from '~/__mocks__/mockModelArtifactList'; import { verifyRelativeURL } from '~/__tests__/cypress/cypress/utils/url'; +import { mockModelRegistryService } from '~/__mocks__/mockModelRegistryService'; const MODEL_REGISTRY_API_VERSION = 'v1alpha3'; @@ -25,16 +21,12 @@ const initIntercepts = () => { ); cy.interceptOdh('GET /api/components', { query: { installed: 'true' } }, mockComponents()); - cy.interceptK8sList(ModelRegistryModel, mockK8sResourceList([mockModelRegistry({})])); - - cy.interceptK8s(ModelRegistryModel, mockModelRegistry({})); - - cy.interceptK8s( - RouteModel, - mockRouteK8sResourceModelRegistry({ - name: 'modelregistry-sample-http', - namespace: 'odh-model-registries', - }), + cy.interceptK8sList( + ServiceModel, + mockK8sResourceList([ + mockModelRegistryService({ name: 'modelregistry-sample' }), + mockModelRegistryService({ name: 'modelregistry-sample-2' }), + ]), ); cy.interceptOdh( diff --git a/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelVersions.cy.ts b/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelVersions.cy.ts index 89e5c30eef..543d5051fb 100644 --- a/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelVersions.cy.ts +++ b/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelVersions.cy.ts @@ -1,16 +1,16 @@ /* eslint-disable camelcase */ import { mockK8sResourceList } from '~/__mocks__'; import { mockDashboardConfig } from '~/__mocks__/mockDashboardConfig'; -import { mockModelRegistry } from '~/__mocks__/mockModelRegistry'; import { mockModelVersionList } from '~/__mocks__/mockModelVersionList'; import { mockRegisteredModelList } from '~/__mocks__/mockRegisteredModelsList'; import { labelModal, modelRegistry } from '~/__tests__/cypress/cypress/pages/modelRegistry'; import { be } from '~/__tests__/cypress/cypress/utils/should'; -import { ModelRegistryModel } from '~/__tests__/cypress/cypress/utils/models'; +import { ServiceModel } from '~/__tests__/cypress/cypress/utils/models'; import { verifyRelativeURL } from '~/__tests__/cypress/cypress/utils/url'; import { mockRegisteredModel } from '~/__mocks__/mockRegisteredModel'; import type { ModelVersion } from '~/concepts/modelRegistry/types'; import { mockModelVersion } from '~/__mocks__/mockModelVersion'; +import { mockModelRegistryService } from '~/__mocks__/mockModelRegistryService'; const MODEL_REGISTRY_API_VERSION = 'v1alpha3'; @@ -49,12 +49,13 @@ const initIntercepts = ({ ); cy.interceptK8sList( - ModelRegistryModel, - mockK8sResourceList([mockModelRegistry({}), mockModelRegistry({ name: 'test-registry' })]), + ServiceModel, + mockK8sResourceList([ + mockModelRegistryService({ name: 'modelregistry-sample' }), + mockModelRegistryService({ name: 'modelregistry-sample-2' }), + ]), ); - cy.interceptK8s(ModelRegistryModel, mockModelRegistry({})); - cy.interceptOdh( `GET /api/service/modelregistry/:serviceName/api/model_registry/:apiVersion/registered_models`, { path: { serviceName: 'modelregistry-sample', apiVersion: MODEL_REGISTRY_API_VERSION } }, diff --git a/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/registeredModelArchive.cy.ts b/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/registeredModelArchive.cy.ts index 97456039e2..8feea1aab0 100644 --- a/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/registeredModelArchive.cy.ts +++ b/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/registeredModelArchive.cy.ts @@ -1,9 +1,8 @@ /* eslint-disable camelcase */ import { mockK8sResourceList } from '~/__mocks__'; import { mockDashboardConfig } from '~/__mocks__/mockDashboardConfig'; -import { mockModelRegistry } from '~/__mocks__/mockModelRegistry'; import { mockRegisteredModelList } from '~/__mocks__/mockRegisteredModelsList'; -import { ModelRegistryModel } from '~/__tests__/cypress/cypress/utils/models'; +import { ServiceModel } from '~/__tests__/cypress/cypress/utils/models'; import { mockModelVersion } from '~/__mocks__/mockModelVersion'; import type { ModelVersion, RegisteredModel } from '~/concepts/modelRegistry/types'; import { ModelState } from '~/concepts/modelRegistry/types'; @@ -16,6 +15,7 @@ import { restoreModelModal, } from '~/__tests__/cypress/cypress/pages/modelRegistry/registeredModelArchive'; import { mockModelVersionList } from '~/__mocks__/mockModelVersionList'; +import { mockModelRegistryService } from '~/__mocks__/mockModelRegistryService'; const MODEL_REGISTRY_API_VERSION = 'v1alpha3'; @@ -58,12 +58,13 @@ const initIntercepts = ({ ); cy.interceptK8sList( - ModelRegistryModel, - mockK8sResourceList([mockModelRegistry({}), mockModelRegistry({ name: 'test-registry' })]), + ServiceModel, + mockK8sResourceList([ + mockModelRegistryService({ name: 'modelregistry-sample' }), + mockModelRegistryService({ name: 'modelregistry-sample-2' }), + ]), ); - cy.interceptK8s(ModelRegistryModel, mockModelRegistry({})); - cy.interceptOdh( `GET /api/service/modelregistry/:serviceName/api/model_registry/:apiVersion/registered_models`, { diff --git a/frontend/src/api/index.ts b/frontend/src/api/index.ts index bb9c85fe40..1e1e791e57 100644 --- a/frontend/src/api/index.ts +++ b/frontend/src/api/index.ts @@ -45,6 +45,9 @@ export * from './models'; // User access review hook export * from './useAccessReview'; +// Rules access review hook +export * from './useRulesReview'; + // Explainability export * from './trustyai/custom'; export * from './trustyai/rawTypes'; diff --git a/frontend/src/api/modelRegistry/k8s.ts b/frontend/src/api/modelRegistry/k8s.ts index abcaf05c4a..5f4d8b2b29 100644 --- a/frontend/src/api/modelRegistry/k8s.ts +++ b/frontend/src/api/modelRegistry/k8s.ts @@ -1,8 +1,8 @@ import { k8sGetResource, k8sListResource } from '@openshift/dynamic-plugin-sdk-utils'; -import { K8sAPIOptions, ModelRegistryKind } from '~/k8sTypes'; +import { K8sAPIOptions, KnownLabels, ModelRegistryKind, ServiceKind } from '~/k8sTypes'; import { applyK8sAPIOptions } from '~/api/apiMergeUtils'; import { ModelRegistryModel } from '~/api/models/modelRegistry'; -import { MODEL_REGISTRY_DEFAULT_NAMESPACE } from '~/concepts/modelRegistry/const'; +import { ServiceModel } from '~/api/models'; export const getModelRegistryCR = async ( namespace: string, @@ -22,10 +22,11 @@ export const getModelRegistryCR = async ( ), ); -export const listModelRegistries = async (): Promise => - k8sListResource({ - model: ModelRegistryModel, +export const listServices = async (namespace: string): Promise => + k8sListResource({ + model: ServiceModel, queryOptions: { - ns: MODEL_REGISTRY_DEFAULT_NAMESPACE, + ns: namespace, + queryParams: { labelSelector: KnownLabels.LABEL_SELECTOR_MODEL_REGISTRY }, }, }).then((listResource) => listResource.items); diff --git a/frontend/src/api/models/k8s.ts b/frontend/src/api/models/k8s.ts index 3565a1c4ea..ba47097926 100644 --- a/frontend/src/api/models/k8s.ts +++ b/frontend/src/api/models/k8s.ts @@ -64,6 +64,19 @@ export const SelfSubjectAccessReviewModel: K8sModelCommon = { plural: 'selfsubjectaccessreviews', }; +export const SelfSubjectRulesReviewModel: K8sModelCommon = { + apiVersion: 'v1', + apiGroup: 'authorization.k8s.io', + kind: 'SelfSubjectRulesReview', + plural: 'selfsubjectrulesreviews', +}; + +export const ServiceModel: K8sModelCommon = { + apiVersion: 'v1', + kind: 'Service', + plural: 'services', +}; + export const ServiceAccountModel: K8sModelCommon = { apiVersion: 'v1', kind: 'ServiceAccount', diff --git a/frontend/src/api/useRulesReview.ts b/frontend/src/api/useRulesReview.ts new file mode 100644 index 0000000000..8b738d0e28 --- /dev/null +++ b/frontend/src/api/useRulesReview.ts @@ -0,0 +1,42 @@ +import { k8sCreateResource } from '@openshift/dynamic-plugin-sdk-utils'; +import * as React from 'react'; +import { SelfSubjectRulesReviewModel } from '~/api/models'; +import { SelfSubjectRulesReviewKind } from '~/k8sTypes'; + +const checkAccess = (ns: string): Promise => { + const selfSubjectRulesReview: SelfSubjectRulesReviewKind = { + apiVersion: 'authorization.k8s.io/v1', + kind: 'SelfSubjectRulesReview', + spec: { + namespace: ns, + }, + }; + return k8sCreateResource({ + model: SelfSubjectRulesReviewModel, + resource: selfSubjectRulesReview, + }); +}; + +export const useRulesReview = ( + namespace: string, +): [SelfSubjectRulesReviewKind['status'], boolean] => { + const [loaded, setLoaded] = React.useState(false); + const [status, setStatus] = React.useState(undefined); + + React.useEffect(() => { + checkAccess(namespace) + .then((result) => { + if (!result.status?.incomplete && !result.status?.evaluationError) { + setStatus(result.status); + } + setLoaded(true); + }) + .catch((e) => { + // eslint-disable-next-line no-console + console.warn('SelfSubjectRulesReview failed', e); + setLoaded(true); + }); + }, [namespace]); + + return [status, loaded]; +}; diff --git a/frontend/src/concepts/modelRegistry/apiHooks/__tests__/useModelRegistryServices.spec.ts b/frontend/src/concepts/modelRegistry/apiHooks/__tests__/useModelRegistryServices.spec.ts new file mode 100644 index 0000000000..e07936c3a5 --- /dev/null +++ b/frontend/src/concepts/modelRegistry/apiHooks/__tests__/useModelRegistryServices.spec.ts @@ -0,0 +1,147 @@ +import { k8sGetResource } from '@openshift/dynamic-plugin-sdk-utils'; +import { act } from 'react-dom/test-utils'; +import { useAccessReview, useRulesReview, listServices } from '~/api'; +import { ServiceKind } from '~/k8sTypes'; +import { useModelRegistryServices } from '~/concepts/modelRegistry/apiHooks/useModelRegistryServices'; +import { standardUseFetchState, testHook } from '~/__tests__/unit/testUtils/hooks'; +import { mockModelRegistryService } from '~/__mocks__/mockModelRegistryService'; + +jest.mock('@openshift/dynamic-plugin-sdk-utils', () => ({ + k8sListResource: jest.fn(), + k8sGetResource: jest.fn(), +})); + +jest.mock('~/api', () => ({ + useAccessReview: jest.fn(), + useRulesReview: jest.fn(), + listServices: jest.fn(), +})); + +jest.mock('~/concepts/modelRegistry/apiHooks/useModelRegistryServices', () => ({ + ...jest.requireActual('~/concepts/modelRegistry/apiHooks/useModelRegistryServices'), + fetchServices: jest.fn(), +})); + +const mockGetResource = jest.mocked(k8sGetResource); +const mockListServices = jest.mocked(listServices); + +describe('useModelRegistryServices', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + test('returns loading state initially', () => { + (useAccessReview as jest.Mock).mockReturnValue([false, false]); + (useRulesReview as jest.Mock).mockReturnValue([{}, false]); + const renderResult = testHook(useModelRegistryServices)(); + + expect(renderResult.result.current[1]).toBe(false); + expect(renderResult.result.current[0]).toEqual([]); + }); + + it('returns services when allowList is true', async () => { + (useAccessReview as jest.Mock).mockReturnValue([true, true]); + (useRulesReview as jest.Mock).mockReturnValue([{}, false]); + (useRulesReview as jest.Mock).mockReturnValue([ + { incomplete: false, nonResourceRules: [], resourceRules: [] }, + true, + ] satisfies ReturnType); + mockListServices.mockResolvedValue([ + mockModelRegistryService({ name: 'service-1', namespace: 'test-namespace' }), + ]); + + const renderResult = testHook(useModelRegistryServices)(); + expect(mockListServices).toHaveBeenCalledTimes(1); + expect(renderResult).hookToStrictEqual(standardUseFetchState([])); + expect(renderResult).hookToHaveUpdateCount(1); + + // wait for update + await renderResult.waitForNextUpdate(); + expect(mockListServices).toHaveBeenCalledTimes(1); + expect(renderResult).hookToStrictEqual( + standardUseFetchState( + [mockModelRegistryService({ name: 'service-1', namespace: 'test-namespace' })], + true, + ), + ); + expect(renderResult).hookToHaveUpdateCount(2); + expect(renderResult).hookToBeStable([false, false, true, true]); + + // refresh + mockListServices.mockResolvedValue([ + mockModelRegistryService({ name: 'service-1', namespace: 'test-namespace' }), + ]); + await act(() => renderResult.result.current[3]()); + expect(mockListServices).toHaveBeenCalledTimes(2); + expect(renderResult).hookToHaveUpdateCount(3); + expect(renderResult).hookToBeStable([false, true, true, true]); + }); + + it('returns services fetched by names when allowList is false', async () => { + (useAccessReview as jest.Mock).mockReturnValue([false, true]); + (useRulesReview as jest.Mock).mockReturnValue([ + { + resourceRules: [ + { + resources: ['services'], + verbs: ['get'], + resourceNames: ['service-1'], + }, + ], + }, + true, + ]); + mockGetResource.mockResolvedValue( + mockModelRegistryService({ name: 'service-1', namespace: 'test-namespace' }), + ); + + const renderResult = testHook(useModelRegistryServices)(); + expect(mockGetResource).toHaveBeenCalledTimes(1); + expect(renderResult).hookToStrictEqual(standardUseFetchState([])); + expect(renderResult).hookToHaveUpdateCount(1); + + // wait for update + await renderResult.waitForNextUpdate(); + expect(mockGetResource).toHaveBeenCalledTimes(1); + expect(renderResult).hookToStrictEqual( + standardUseFetchState( + [mockModelRegistryService({ name: 'service-1', namespace: 'test-namespace' })], + true, + ), + ); + expect(renderResult).hookToHaveUpdateCount(2); + expect(renderResult).hookToBeStable([false, false, true, true]); + + // refresh + mockGetResource.mockResolvedValue(mockModelRegistryService({})); + await act(() => renderResult.result.current[3]()); + expect(mockGetResource).toHaveBeenCalledTimes(2); + expect(renderResult).hookToHaveUpdateCount(3); + expect(renderResult).hookToBeStable([false, true, true, true]); + }); + + test('returns empty array if no service names are provided', async () => { + (useAccessReview as jest.Mock).mockReturnValue([false, true]); + (useRulesReview as jest.Mock).mockReturnValue([ + { + resourceRules: [ + { + resources: ['services'], + verbs: ['use'], + resourceNames: ['service-1'], + }, + ], + }, + true, + ]); + mockGetResource.mockResolvedValue( + mockModelRegistryService({ name: 'service-1', namespace: 'test-namespace' }), + ); + const renderResult = testHook(useModelRegistryServices)(); + await renderResult.waitForNextUpdate(); + + expect(renderResult.result.current[0]).toEqual([]); + expect(renderResult.result.current[1]).toBe(true); + expect(mockGetResource).toHaveBeenCalledTimes(0); + }); +}); diff --git a/frontend/src/concepts/modelRegistry/apiHooks/useModelRegistries.ts b/frontend/src/concepts/modelRegistry/apiHooks/useModelRegistries.ts deleted file mode 100644 index 18d94e6f75..0000000000 --- a/frontend/src/concepts/modelRegistry/apiHooks/useModelRegistries.ts +++ /dev/null @@ -1,11 +0,0 @@ -import React from 'react'; -import useFetchState, { FetchState } from '~/utilities/useFetchState'; -import { ModelRegistryKind } from '~/k8sTypes'; -import { listModelRegistries } from '~/api'; - -const useModelRegistries = (): FetchState => { - const getModelRegistries = React.useCallback(() => listModelRegistries(), []); - return useFetchState(getModelRegistries, []); -}; - -export default useModelRegistries; diff --git a/frontend/src/concepts/modelRegistry/apiHooks/useModelRegistryServices.ts b/frontend/src/concepts/modelRegistry/apiHooks/useModelRegistryServices.ts new file mode 100644 index 0000000000..592a9ffb51 --- /dev/null +++ b/frontend/src/concepts/modelRegistry/apiHooks/useModelRegistryServices.ts @@ -0,0 +1,87 @@ +import React from 'react'; +import { k8sGetResource } from '@openshift/dynamic-plugin-sdk-utils'; +import useFetchState, { + FetchState, + FetchStateCallbackPromise, + NotReadyError, +} from '~/utilities/useFetchState'; +import { AccessReviewResourceAttributes, ServiceKind } from '~/k8sTypes'; +import { ServiceModel, useAccessReview, useRulesReview, listServices } from '~/api'; +import { MODEL_REGISTRY_DEFAULT_NAMESPACE } from '~/concepts/modelRegistry/const'; + +const accessReviewResource: AccessReviewResourceAttributes = { + group: 'user.openshift.io', + resource: 'services', + verb: 'list', + namespace: MODEL_REGISTRY_DEFAULT_NAMESPACE, +}; + +const getServiceByName = (name: string, namespace: string): Promise => + k8sGetResource({ + model: ServiceModel, + queryOptions: { name, ns: namespace }, + }); + +const fetchServices = async (names: string[], namespace: string): Promise => { + if (!namespace) { + throw new NotReadyError('No namespace'); + } + + if (names.length === 0) { + return []; + } + + const servicePromises = names.map((name) => getServiceByName(name, namespace)); + const services = await Promise.all(servicePromises); + + return services; +}; + +const listServicesOrFetchThemByNames = async ( + allowList: boolean, + accessReviewLoaded: boolean, + rulesReviewLoaded: boolean, + serviceNames?: string[], +): Promise => { + if (!accessReviewLoaded || !rulesReviewLoaded) { + throw new NotReadyError('Access review or Rules review not loaded'); + } + + const services = allowList + ? await listServices(MODEL_REGISTRY_DEFAULT_NAMESPACE) + : await fetchServices(serviceNames || [], MODEL_REGISTRY_DEFAULT_NAMESPACE); + + return services; +}; + +export const useModelRegistryServices = (): FetchState => { + const [allowList, accessReviewLoaded] = useAccessReview(accessReviewResource); + const [statuses, rulesReviewLoaded] = useRulesReview(MODEL_REGISTRY_DEFAULT_NAMESPACE); + + const serviceNames = React.useMemo(() => { + if (!rulesReviewLoaded) { + return []; + } + return statuses?.resourceRules + .filter( + ({ resources, verbs }) => + resources?.includes('services') && verbs.some((verb) => verb === 'get'), + ) + .flatMap((rule) => rule.resourceNames || []); + }, [rulesReviewLoaded, statuses]); + + const callback = React.useCallback>( + () => + listServicesOrFetchThemByNames( + allowList, + accessReviewLoaded, + rulesReviewLoaded, + serviceNames, + ), + [allowList, accessReviewLoaded, rulesReviewLoaded, serviceNames], + ); + + return useFetchState(callback, [], { + initialPromisePurity: true, + }); +}; diff --git a/frontend/src/concepts/modelRegistry/context/ModelRegistryContext.tsx b/frontend/src/concepts/modelRegistry/context/ModelRegistryContext.tsx index dfde7ebcc8..92d045a96b 100644 --- a/frontend/src/concepts/modelRegistry/context/ModelRegistryContext.tsx +++ b/frontend/src/concepts/modelRegistry/context/ModelRegistryContext.tsx @@ -1,21 +1,9 @@ import * as React from 'react'; -import { Alert, Bullseye } from '@patternfly/react-core'; import { SupportedArea, conditionalArea } from '~/concepts/areas'; -import { MODEL_REGISTRY_DEFAULT_NAMESPACE } from '~/concepts/modelRegistry/const'; import useModelRegistryAPIState, { ModelRegistryAPIState } from './useModelRegistryAPIState'; -import { - hasServerTimedOut, - isModelRegistryAvailable, - useModelRegistryNamespaceCR, -} from './useModelRegistryNamespaceCR'; export type ModelRegistryContextType = { - hasCR: boolean; - crInitializing: boolean; - serverTimedOut: boolean; apiState: ModelRegistryAPIState; - ignoreTimedOut: () => void; - refreshState: () => Promise; refreshAPIState: () => void; }; @@ -25,13 +13,8 @@ type ModelRegistryContextProviderProps = { }; export const ModelRegistryContext = React.createContext({ - hasCR: false, - crInitializing: false, - serverTimedOut: false, // eslint-disable-next-line @typescript-eslint/consistent-type-assertions apiState: { apiAvailable: false, api: null as unknown as ModelRegistryAPIState['api'] }, - ignoreTimedOut: () => undefined, - refreshState: async () => undefined, refreshAPIState: () => undefined, }); @@ -39,44 +22,14 @@ export const ModelRegistryContextProvider = conditionalArea { - const state = useModelRegistryNamespaceCR(MODEL_REGISTRY_DEFAULT_NAMESPACE, modelRegistryName); - const [modelRegistryCR, crLoaded, crLoadError, refreshCR] = state; - const isCRReady = isModelRegistryAvailable(state); - - const [disableTimeout, setDisableTimeout] = React.useState(false); - const serverTimedOut = !disableTimeout && hasServerTimedOut(state, isCRReady); - const ignoreTimedOut = React.useCallback(() => { - setDisableTimeout(true); - }, []); - const hostPath = modelRegistryName ? `/api/service/modelregistry/${modelRegistryName}` : null; const [apiState, refreshAPIState] = useModelRegistryAPIState(hostPath); - const refreshState = React.useCallback( - () => Promise.all([refreshCR()]).then(() => undefined), - [refreshCR], - ); - - if (crLoadError) { - return ( - - - {crLoadError.message} - - - ); - } - return ( diff --git a/frontend/src/concepts/modelRegistry/context/ModelRegistrySelectorContext.tsx b/frontend/src/concepts/modelRegistry/context/ModelRegistrySelectorContext.tsx index ae4863f77e..1ac9f2b7f5 100644 --- a/frontend/src/concepts/modelRegistry/context/ModelRegistrySelectorContext.tsx +++ b/frontend/src/concepts/modelRegistry/context/ModelRegistrySelectorContext.tsx @@ -1,14 +1,14 @@ import * as React from 'react'; -import { ModelRegistryKind } from '~/k8sTypes'; -import useModelRegistries from '~/concepts/modelRegistry/apiHooks/useModelRegistries'; +import { ServiceKind } from '~/k8sTypes'; import useModelRegistryEnabled from '~/concepts/modelRegistry/useModelRegistryEnabled'; +import { useModelRegistryServices } from '~/concepts/modelRegistry/apiHooks/useModelRegistryServices'; export type ModelRegistrySelectorContextType = { - modelRegistriesLoaded: boolean; - modelRegistriesLoadError?: Error; - modelRegistries: ModelRegistryKind[]; - preferredModelRegistry: ModelRegistryKind | undefined; - updatePreferredModelRegistry: (modelRegistry: ModelRegistryKind | undefined) => void; + modelRegistryServicesLoaded: boolean; + modelRegistryServicesLoadError?: Error; + modelRegistryServices: ServiceKind[]; + preferredModelRegistry: ServiceKind | undefined; + updatePreferredModelRegistry: (modelRegistry: ServiceKind | undefined) => void; }; type ModelRegistrySelectorContextProviderProps = { @@ -16,9 +16,9 @@ type ModelRegistrySelectorContextProviderProps = { }; export const ModelRegistrySelectorContext = React.createContext({ - modelRegistriesLoaded: false, - modelRegistriesLoadError: undefined, - modelRegistries: [], + modelRegistryServicesLoaded: false, + modelRegistryServicesLoadError: undefined, + modelRegistryServices: [], preferredModelRegistry: undefined, updatePreferredModelRegistry: () => undefined, }); @@ -39,23 +39,23 @@ export const ModelRegistrySelectorContextProvider: React.FC< const EnabledModelRegistrySelectorContextProvider: React.FC< ModelRegistrySelectorContextProviderProps > = ({ children }) => { - const [modelRegistries, isLoaded, error] = useModelRegistries(); + const [modelRegistryServices, isLoaded, error] = useModelRegistryServices(); const [preferredModelRegistry, setPreferredModelRegistry] = React.useState(undefined); - const firstModelRegistry = modelRegistries.length > 0 ? modelRegistries[0] : null; + const firstModelRegistry = modelRegistryServices.length > 0 ? modelRegistryServices[0] : null; return ( ({ - modelRegistriesLoaded: isLoaded, - modelRegistriesLoadError: error, - modelRegistries, + modelRegistryServicesLoaded: isLoaded, + modelRegistryServicesLoadError: error, + modelRegistryServices, preferredModelRegistry: preferredModelRegistry ?? firstModelRegistry ?? undefined, updatePreferredModelRegistry: setPreferredModelRegistry, }), - [isLoaded, error, modelRegistries, preferredModelRegistry, firstModelRegistry], + [isLoaded, error, modelRegistryServices, preferredModelRegistry, firstModelRegistry], )} > {children} diff --git a/frontend/src/k8sTypes.ts b/frontend/src/k8sTypes.ts index 4b806c18c1..226d3012df 100644 --- a/frontend/src/k8sTypes.ts +++ b/frontend/src/k8sTypes.ts @@ -1028,6 +1028,51 @@ export type SelfSubjectAccessReviewKind = K8sResourceCommon & { }; }; +export type SelfSubjectRulesReviewKind = K8sResourceCommon & { + spec: { + namespace: string; + }; + status?: { + incomplete: boolean; + nonResourceRules: { + verbs: string[]; + nonResourceURLs?: string[]; + }[]; + resourceRules: { + verbs: string[]; + apiGroups?: string[]; + resourceNames?: string[]; + resources?: string[]; + }[]; + evaluationError?: string; + }; +}; + +export type ServiceKind = K8sResourceCommon & { + metadata: { + annotations?: DisplayNameAnnotations; + name: string; + namespace: string; + labels?: Partial<{ + 'opendatahub.io/user': string; + component: string; + }>; + }; + spec: { + selector: { + app: string; + component: string; + }; + ports: { + name?: string; + protocol?: string; + appProtocol?: string; + port?: number; + targetPort?: number | string; + }[]; + }; +}; + /** @deprecated - Tekton is no longer used */ export type PipelineRunTaskSpecDigest = { name: string; diff --git a/frontend/src/pages/modelRegistry/ModelRegistryCoreLoader.tsx b/frontend/src/pages/modelRegistry/ModelRegistryCoreLoader.tsx index f4e1d721fd..4a8c3a2828 100644 --- a/frontend/src/pages/modelRegistry/ModelRegistryCoreLoader.tsx +++ b/frontend/src/pages/modelRegistry/ModelRegistryCoreLoader.tsx @@ -28,28 +28,27 @@ const ModelRegistryCoreLoader: React.FC = )(({ getInvalidRedirectPath }) => { const { modelRegistry } = useParams<{ modelRegistry: string }>(); const { - modelRegistriesLoaded, - modelRegistriesLoadError, - modelRegistries, + modelRegistryServicesLoaded, + modelRegistryServicesLoadError, + modelRegistryServices, preferredModelRegistry, } = React.useContext(ModelRegistrySelectorContext); - if (modelRegistriesLoadError) { + if (modelRegistryServicesLoadError) { return ( - {modelRegistriesLoadError.message} + {modelRegistryServicesLoadError.message} ); } - - if (!modelRegistriesLoaded) { + if (!modelRegistryServicesLoaded) { return Loading model registries...; } let renderStateProps: ApplicationPageRenderState & { children?: React.ReactNode }; - if (modelRegistries.length === 0) { + if (modelRegistryServices.length === 0) { renderStateProps = { empty: true, emptyStatePage: ( @@ -65,7 +64,9 @@ const ModelRegistryCoreLoader: React.FC = ), }; } else if (modelRegistry) { - const foundModelRegistry = modelRegistries.find((mr) => mr.metadata.name === modelRegistry); + const foundModelRegistry = modelRegistryServices.find( + (mr) => mr.metadata.name === modelRegistry, + ); if (foundModelRegistry) { // Render the content return ( @@ -82,7 +83,7 @@ const ModelRegistryCoreLoader: React.FC = }; } else { // Redirect the namespace suffix into the URL - const redirectModelRegistry = preferredModelRegistry ?? modelRegistries[0]; + const redirectModelRegistry = preferredModelRegistry ?? modelRegistryServices[0]; return ; } diff --git a/frontend/src/pages/modelRegistry/screens/ModelRegistrySelector.tsx b/frontend/src/pages/modelRegistry/screens/ModelRegistrySelector.tsx index 89b14afd4f..3591b97cff 100644 --- a/frontend/src/pages/modelRegistry/screens/ModelRegistrySelector.tsx +++ b/frontend/src/pages/modelRegistry/screens/ModelRegistrySelector.tsx @@ -27,7 +27,7 @@ import { getDisplayNameFromK8sResource, getResourceNameFromK8sResource, } from '~/concepts/k8s/utils'; -import { ModelRegistryKind } from '~/k8sTypes'; +import { ServiceKind } from '~/k8sTypes'; const MODEL_REGISTRY_FAVORITE_STORAGE_KEY = 'odh.dashboard.model.registry.favorite'; @@ -42,10 +42,10 @@ const ModelRegistrySelector: React.FC = ({ onSelection, primary, }) => { - const { modelRegistries, updatePreferredModelRegistry } = React.useContext( + const { modelRegistryServices, updatePreferredModelRegistry } = React.useContext( ModelRegistrySelectorContext, ); - const selection = modelRegistries.find((mr) => mr.metadata.name === modelRegistry); + const selection = modelRegistryServices.find((mr) => mr.metadata.name === modelRegistry); const [isOpen, setIsOpen] = React.useState(false); const [favorites, setFavorites] = useBrowserStorage( MODEL_REGISTRY_FAVORITE_STORAGE_KEY, @@ -54,9 +54,10 @@ const ModelRegistrySelector: React.FC = ({ const selectionDisplayName = selection ? getDisplayNameFromK8sResource(selection) : modelRegistry; - const toggleLabel = modelRegistries.length === 0 ? 'No model registries' : selectionDisplayName; + const toggleLabel = + modelRegistryServices.length === 0 ? 'No model registries' : selectionDisplayName; - const getMRSelectDescription = (mr: ModelRegistryKind) => { + const getMRSelectDescription = (mr: ServiceKind) => { const desc = getDescriptionFromK8sResource(mr); if (!desc) { return; @@ -82,7 +83,7 @@ const ModelRegistrySelector: React.FC = ({ const options = [ - {modelRegistries.map((mr) => ( + {modelRegistryServices.map((mr) => ( = ({ toggleId="model-registry-selector-dropdown" variant={SelectVariant.single} onToggle={() => setIsOpen(!isOpen)} - isDisabled={modelRegistries.length === 0} + isDisabled={modelRegistryServices.length === 0} onSelect={(_e, value) => { setIsOpen(false); - updatePreferredModelRegistry(modelRegistries.find((obj) => obj.metadata.name === value)); + updatePreferredModelRegistry( + modelRegistryServices.find((obj) => obj.metadata.name === value), + ); if (typeof value === 'string') { onSelection(value); }