Skip to content

Commit

Permalink
Allow all users to list model registries
Browse files Browse the repository at this point in the history
  • Loading branch information
manaswinidas committed Jul 25, 2024
1 parent 13b5562 commit f6b3d2c
Show file tree
Hide file tree
Showing 12 changed files with 262 additions and 27 deletions.
9 changes: 9 additions & 0 deletions frontend/src/api/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,15 @@ export * from './models';
// User access review hook
export * from './useAccessReview';

// Rules access review hook
export * from './useRulesReview';

// Services by Names hook for regular users
export * from './useServicesByNames';

// Services hook for admin users
export * from './useServices';

// Explainability
export * from './trustyai/custom';
export * from './trustyai/rawTypes';
Expand Down
9 changes: 8 additions & 1 deletion frontend/src/api/modelRegistry/k8s.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { k8sGetResource, k8sListResource } from '@openshift/dynamic-plugin-sdk-utils';
import { K8sAPIOptions, ModelRegistryKind } from '~/k8sTypes';
import { K8sAPIOptions, 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,
Expand All @@ -29,3 +30,9 @@ export const listModelRegistries = async (): Promise<ModelRegistryKind[]> =>
ns: MODEL_REGISTRY_DEFAULT_NAMESPACE,
},
}).then((listResource) => listResource.items);

export const listServices = async (namespace: string): Promise<ServiceKind[]> =>
k8sListResource<ServiceKind>({
model: ServiceModel,
queryOptions: { ns: namespace, queryParams: { labelSelector: 'component=model-registry' } },
}).then((listResource) => listResource.items);
13 changes: 13 additions & 0 deletions frontend/src/api/models/k8s.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
42 changes: 42 additions & 0 deletions frontend/src/api/useRulesReview.ts
Original file line number Diff line number Diff line change
@@ -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<SelfSubjectRulesReviewKind> => {
const selfSubjectRulesReview: SelfSubjectRulesReviewKind = {
apiVersion: 'authorization.k8s.io/v1',
kind: 'SelfSubjectRulesReview',
spec: {
namespace: ns,
},
};
return k8sCreateResource<SelfSubjectRulesReviewKind>({
model: SelfSubjectRulesReviewModel,
resource: selfSubjectRulesReview,
});
};

export const useRulesReview = (
namespace: string,
): [boolean, SelfSubjectRulesReviewKind['status']] => {
const [loaded, setLoaded] = React.useState(false);
const [status, setStatus] = React.useState<SelfSubjectRulesReviewKind['status']>(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 [loaded, status];
};
15 changes: 15 additions & 0 deletions frontend/src/api/useServices.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import React from 'react';
import useFetchState, { FetchState, FetchStateCallbackPromise } from '~/utilities/useFetchState';
import { ServiceKind } from '~/k8sTypes';
import { listServices } from './modelRegistry/k8s';

export const useServices = (namespace: string): FetchState<ServiceKind[]> => {
const callback = React.useCallback<FetchStateCallbackPromise<ServiceKind[]>>(
() => listServices(namespace),
[namespace],
);

return useFetchState(callback, [], {
initialPromisePurity: true,
});
};
44 changes: 44 additions & 0 deletions frontend/src/api/useServicesByNames.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import { k8sGetResource } from '@openshift/dynamic-plugin-sdk-utils';
import React from 'react';
import { ServiceKind } from '~/k8sTypes';
import useFetchState, {
FetchState,
FetchStateCallbackPromise,
NotReadyError,
} from '~/utilities/useFetchState';
import { ServiceModel } from './models';

export const getServiceByName = (name: string, namespace: string): Promise<ServiceKind> =>
k8sGetResource<ServiceKind>({
model: ServiceModel,
queryOptions: { name, ns: namespace },
});

const fetchServices = async (names: string[], namespace: string): Promise<ServiceKind[]> => {
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;
};

export const useServicesByNames = (
names: string[],
namespace: string,
): FetchState<ServiceKind[]> => {
const callback = React.useCallback<FetchStateCallbackPromise<ServiceKind[]>>(
() => fetchServices(names, namespace),
[names, namespace],
);

return useFetchState(callback, [], {
initialPromisePurity: true,
});
};
68 changes: 61 additions & 7 deletions frontend/src/concepts/modelRegistry/apiHooks/useModelRegistries.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,65 @@
import React from 'react';
import useFetchState, { FetchState } from '~/utilities/useFetchState';
import { ModelRegistryKind } from '~/k8sTypes';
import { listModelRegistries } from '~/api';
import useFetchState, { FetchState, FetchStateCallbackPromise } from '~/utilities/useFetchState';
import { AccessReviewResourceAttributes, ServiceKind } from '~/k8sTypes';
import {
ServiceModel,
useAccessReview,
useRulesReview,
useServicesByNames,
useServices,
} from '~/api';
import { MODEL_REGISTRY_DEFAULT_NAMESPACE } from '~/concepts/modelRegistry/const';

const useModelRegistries = (): FetchState<ModelRegistryKind[]> => {
const getModelRegistries = React.useCallback(() => listModelRegistries(), []);
return useFetchState<ModelRegistryKind[]>(getModelRegistries, []);
const accessReviewResource: AccessReviewResourceAttributes = {
group: 'user.openshift.io',
resource: 'services',
verb: 'list',
namespace: MODEL_REGISTRY_DEFAULT_NAMESPACE,
};

export default useModelRegistries;
const useModelRegistryServices = (): FetchState<ServiceKind[]> => {
const [allowList, accessReviewLoaded] = useAccessReview(accessReviewResource);
const [loaded, statuses] = useRulesReview(MODEL_REGISTRY_DEFAULT_NAMESPACE);
const serviceNames = React.useMemo(() => {
if (!loaded) {
return [];
}
return (
statuses?.resourceRules.find(
({ resources, verbs }) =>
resources?.includes(ServiceModel.plural) && verbs.some((verb) => verb === 'get'),
)?.resourceNames || []
);
}, [loaded, statuses]);

const [nonAdminServices, nonAdminServicesLoaded] = useServicesByNames(
serviceNames,
MODEL_REGISTRY_DEFAULT_NAMESPACE,
);

const [adminServices, adminServicesLoaded] = useServices(MODEL_REGISTRY_DEFAULT_NAMESPACE);

const getModelRegistryServices = React.useCallback<
FetchStateCallbackPromise<ServiceKind[]>
>(async () => {
if (!accessReviewLoaded || (!nonAdminServicesLoaded && !adminServicesLoaded)) {
return [];
}

const services = allowList ? adminServices : nonAdminServices;
return Promise.resolve(services);
}, [
accessReviewLoaded,
nonAdminServicesLoaded,
adminServicesLoaded,
allowList,
adminServices,
nonAdminServices,
]);

return useFetchState<ServiceKind[]>(getModelRegistryServices, [], {
initialPromisePurity: true,
});
};

export default useModelRegistryServices;
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import * as React from 'react';
import { ModelRegistryKind } from '~/k8sTypes';
import { ServiceKind } from '~/k8sTypes';
import useModelRegistries from '~/concepts/modelRegistry/apiHooks/useModelRegistries';
import useModelRegistryEnabled from '~/concepts/modelRegistry/useModelRegistryEnabled';

export type ModelRegistrySelectorContextType = {
modelRegistriesLoaded: boolean;
modelRegistriesLoadError?: Error;
modelRegistries: ModelRegistryKind[];
preferredModelRegistry: ModelRegistryKind | undefined;
updatePreferredModelRegistry: (modelRegistry: ModelRegistryKind | undefined) => void;
modelRegistryServices: ServiceKind[];
preferredModelRegistry: ServiceKind | undefined;
updatePreferredModelRegistry: (modelRegistry: ServiceKind | undefined) => void;
};

type ModelRegistrySelectorContextProviderProps = {
Expand All @@ -18,7 +18,7 @@ type ModelRegistrySelectorContextProviderProps = {
export const ModelRegistrySelectorContext = React.createContext<ModelRegistrySelectorContextType>({
modelRegistriesLoaded: false,
modelRegistriesLoadError: undefined,
modelRegistries: [],
modelRegistryServices: [],
preferredModelRegistry: undefined,
updatePreferredModelRegistry: () => undefined,
});
Expand All @@ -39,23 +39,23 @@ export const ModelRegistrySelectorContextProvider: React.FC<
const EnabledModelRegistrySelectorContextProvider: React.FC<
ModelRegistrySelectorContextProviderProps
> = ({ children }) => {
const [modelRegistries, isLoaded, error] = useModelRegistries();
const [modelRegistryServices, isLoaded, error] = useModelRegistries();
const [preferredModelRegistry, setPreferredModelRegistry] =
React.useState<ModelRegistrySelectorContextType['preferredModelRegistry']>(undefined);

const firstModelRegistry = modelRegistries.length > 0 ? modelRegistries[0] : null;
const firstModelRegistry = modelRegistryServices.length > 0 ? modelRegistryServices[0] : null;

return (
<ModelRegistrySelectorContext.Provider
value={React.useMemo(
() => ({
modelRegistriesLoaded: isLoaded,
modelRegistriesLoadError: error,
modelRegistries,
modelRegistryServices,
preferredModelRegistry: preferredModelRegistry ?? firstModelRegistry ?? undefined,
updatePreferredModelRegistry: setPreferredModelRegistry,
}),
[isLoaded, error, modelRegistries, preferredModelRegistry, firstModelRegistry],
[isLoaded, error, modelRegistryServices, preferredModelRegistry, firstModelRegistry],
)}
>
{children}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ export const useModelRegistryNamespaceCR = (namespace: string, name: string): Fe
// Not finding is okay, not an error
return null;
}
if (e.statusObject?.code === 403) {
// Forbidden is okay, not an error
return null;
}
throw e;
});
},
Expand Down
42 changes: 42 additions & 0 deletions frontend/src/k8sTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1018,6 +1018,48 @@ 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.kubernetes.io/name': string;
};
ports: {
protocol?: string;
port?: number;
targetPort?: number | string;
}[];
};
};

/** @deprecated - Tekton is no longer used */
export type PipelineRunTaskSpecDigest = {
name: string;
Expand Down
10 changes: 6 additions & 4 deletions frontend/src/pages/modelRegistry/ModelRegistryCoreLoader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ const ModelRegistryCoreLoader: React.FC<ModelRegistryCoreLoaderProps> =
const {
modelRegistriesLoaded,
modelRegistriesLoadError,
modelRegistries,
modelRegistryServices,
preferredModelRegistry,
} = React.useContext(ModelRegistrySelectorContext);

Expand All @@ -49,7 +49,7 @@ const ModelRegistryCoreLoader: React.FC<ModelRegistryCoreLoaderProps> =
}

let renderStateProps: ApplicationPageRenderState & { children?: React.ReactNode };
if (modelRegistries.length === 0) {
if (modelRegistryServices.length === 0) {
renderStateProps = {
empty: true,
emptyStatePage: (
Expand All @@ -65,7 +65,9 @@ const ModelRegistryCoreLoader: React.FC<ModelRegistryCoreLoaderProps> =
),
};
} 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 (
Expand All @@ -82,7 +84,7 @@ const ModelRegistryCoreLoader: React.FC<ModelRegistryCoreLoaderProps> =
};
} else {
// Redirect the namespace suffix into the URL
const redirectModelRegistry = preferredModelRegistry ?? modelRegistries[0];
const redirectModelRegistry = preferredModelRegistry ?? modelRegistryServices[0];
return <Navigate to={getInvalidRedirectPath(redirectModelRegistry.metadata.name)} replace />;
}

Expand Down
Loading

0 comments on commit f6b3d2c

Please sign in to comment.