Skip to content

Commit

Permalink
Replace model registry hard-coded namespace with namespace from DSC
Browse files Browse the repository at this point in the history
  • Loading branch information
ppadti committed Sep 23, 2024
1 parent 5640b39 commit b899b25
Show file tree
Hide file tree
Showing 22 changed files with 120 additions and 45 deletions.
27 changes: 16 additions & 11 deletions backend/src/routes/api/modelRegistries/modelRegistryUtils.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
import { MODEL_REGISTRY_NAMESPACE } from '../../../utils/constants';
import { KubeFastifyInstance, ModelRegistryKind, RecursivePartial } from '../../../types';
import { PatchUtils, V1Secret, V1Status } from '@kubernetes/client-node';
import { getClusterStatus } from '../../../utils/dsc';

const MODEL_REGISTRY_API_GROUP = 'modelregistry.opendatahub.io';
const MODEL_REGISTRY_API_VERSION = 'v1alpha1';
const MODEL_REGISTRY_PLURAL = 'modelregistries';

export const getModelRegistryNamespace = async (fastify: KubeFastifyInstance): Promise<string> => {
const modelRegistryNamespace = await getClusterStatus(fastify);
return modelRegistryNamespace.components.modelregistry.registriesNamespace;
};

const base64encode = (value?: string): string => {
// This usage of toString is fine for encoding
// eslint-disable-next-line no-restricted-properties
Expand Down Expand Up @@ -33,7 +38,7 @@ export const listModelRegistries = async (
const response = await (fastify.kube.customObjectsApi.listNamespacedCustomObject(
MODEL_REGISTRY_API_GROUP,
MODEL_REGISTRY_API_VERSION,
MODEL_REGISTRY_NAMESPACE,
await getModelRegistryNamespace(fastify),
MODEL_REGISTRY_PLURAL,
undefined,
undefined,
Expand All @@ -59,7 +64,7 @@ const createDatabasePasswordSecret = async (
apiVersion: 'v1',
metadata: {
generateName: `${modelRegistry.metadata.name}-db-`,
namespace: MODEL_REGISTRY_NAMESPACE,
namespace: await getModelRegistryNamespace(fastify),
annotations: {
'template.openshift.io/expose-database_name': "{.data['database-name']}",
'template.openshift.io/expose-username': "{.data['database-user']}",
Expand All @@ -74,7 +79,7 @@ const createDatabasePasswordSecret = async (
type: 'Opaque',
};
const response = await fastify.kube.coreV1Api.createNamespacedSecret(
MODEL_REGISTRY_NAMESPACE,
await getModelRegistryNamespace(fastify),
secret,
undefined,
dryRun ? 'All' : undefined,
Expand Down Expand Up @@ -108,7 +113,7 @@ const createModelRegistry = async (
const response = await (fastify.kube.customObjectsApi.createNamespacedCustomObject(
MODEL_REGISTRY_API_GROUP,
MODEL_REGISTRY_API_VERSION,
MODEL_REGISTRY_NAMESPACE,
await getModelRegistryNamespace(fastify),
MODEL_REGISTRY_PLURAL,
modelRegistryWithSecretRef,
undefined,
Expand Down Expand Up @@ -147,7 +152,7 @@ export const getModelRegistry = async (
const response = await (fastify.kube.customObjectsApi.getNamespacedCustomObject(
MODEL_REGISTRY_API_GROUP,
MODEL_REGISTRY_API_VERSION,
MODEL_REGISTRY_NAMESPACE,
await getModelRegistryNamespace(fastify),
MODEL_REGISTRY_PLURAL,
modelRegistryName,
// getNamespacedCustomObject doesn't support TS generics and returns body as `object`, so we assert its real type
Expand All @@ -165,7 +170,7 @@ const getDatabasePasswordSecret = async (
}
const response = await fastify.kube.coreV1Api.readNamespacedSecret(
secretRef.name,
MODEL_REGISTRY_NAMESPACE,
await getModelRegistryNamespace(fastify),
);
return { secret: response.body, passwordDataKey: secretRef.key };
};
Expand Down Expand Up @@ -193,7 +198,7 @@ const deleteDatabasePasswordSecret = async (
}
const response = await fastify.kube.coreV1Api.deleteNamespacedSecret(
existingSecret.metadata.name,
MODEL_REGISTRY_NAMESPACE,
await getModelRegistryNamespace(fastify),
undefined,
dryRun ? 'All' : undefined,
);
Expand All @@ -209,7 +214,7 @@ const patchModelRegistry = async (
const response = await (fastify.kube.customObjectsApi.patchNamespacedCustomObject(
MODEL_REGISTRY_API_GROUP,
MODEL_REGISTRY_API_VERSION,
MODEL_REGISTRY_NAMESPACE,
await getModelRegistryNamespace(fastify),
MODEL_REGISTRY_PLURAL,
modelRegistryName,
patchBody,
Expand All @@ -235,7 +240,7 @@ const updateDatabasePassword = async (
if (databasePassword) {
await fastify.kube.coreV1Api.patchNamespacedSecret(
secret.metadata.name,
MODEL_REGISTRY_NAMESPACE,
await getModelRegistryNamespace(fastify),
{
data: {
...secret.data,
Expand Down Expand Up @@ -280,7 +285,7 @@ export const deleteModelRegistryAndSecret = async (
const response = await fastify.kube.customObjectsApi.deleteNamespacedCustomObject(
MODEL_REGISTRY_API_GROUP,
MODEL_REGISTRY_API_VERSION,
MODEL_REGISTRY_NAMESPACE,
await getModelRegistryNamespace(fastify),
MODEL_REGISTRY_PLURAL,
modelRegistryName,
undefined,
Expand Down
4 changes: 2 additions & 2 deletions backend/src/routes/api/service/modelregistry/index.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import { getModelRegistryNamespace } from '../../../api/modelRegistries/modelRegistryUtils';
import { ServiceAddressAnnotation } from '../../../../types';
import { MODEL_REGISTRY_NAMESPACE } from '../../../../utils/constants';
import { proxyService } from '../../../../utils/proxy';

export default proxyService(
null,
{
addressAnnotation: ServiceAddressAnnotation.EXTERNAL_REST,
internalPort: 8080,
namespace: MODEL_REGISTRY_NAMESPACE,
namespace: getModelRegistryNamespace,
},
{
// Use port forwarding for local development:
Expand Down
5 changes: 5 additions & 0 deletions backend/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1004,6 +1004,11 @@ type ComponentNames =
| 'workbenches';

export type DataScienceClusterKindStatus = {
components: {
modelregistry: {
registriesNamespace: string
}
}
conditions: K8sCondition[];
installedComponents: { [key in ComponentNames]?: boolean };
phase?: string;
Expand Down
2 changes: 0 additions & 2 deletions backend/src/utils/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,5 +142,3 @@ export const THANOS_RBAC_PORT = '9092';
export const THANOS_INSTANCE_NAME = 'thanos-querier';
export const THANOS_NAMESPACE = 'openshift-monitoring';
export const LABEL_SELECTOR_DASHBOARD_RESOURCE = `${KnownLabels.DASHBOARD_RESOURCE}=true`;

export const MODEL_REGISTRY_NAMESPACE = 'odh-model-registries';
2 changes: 1 addition & 1 deletion backend/src/utils/proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export const proxyService =
internalPort: number | string;
prefix?: string;
suffix?: string;
namespace?: string;
namespace?: string | ((fastify: KubeFastifyInstance) => Promise<string>);
},
{
constructUrl: (resource: K) => string;
Expand Down
9 changes: 7 additions & 2 deletions backend/src/utils/route-security.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,15 @@ import { createCustomError } from './requestUtils';
import { isUserAdmin } from './adminUtils';
import { getNamespaces } from './notebookUtils';
import { logRequestDetails } from './fileUtils';
import { DEV_MODE, MODEL_REGISTRY_NAMESPACE } from './constants';
import { DEV_MODE } from './constants';
import {
K8sNamespacedResourceCommon,
KubeFastifyInstance,
NotebookData,
NotebookState,
OauthFastifyRequest,
} from '../types';
import { getModelRegistryNamespace } from '../routes/api/modelRegistries/modelRegistryUtils';

const testAdmin = async (
fastify: KubeFastifyInstance,
Expand Down Expand Up @@ -73,7 +74,11 @@ const requestSecurityGuard = async (
const isReadRequest = request.method.toLowerCase() === 'get';

// Check to see if a request was made against one of our namespaces
if (![notebookNamespace, dashboardNamespace, MODEL_REGISTRY_NAMESPACE].includes(namespace)) {
if (
![notebookNamespace, dashboardNamespace, await getModelRegistryNamespace(fastify)].includes(
namespace,
)
) {
// Not a valid namespace -- cannot make direct calls to just any namespace no matter who you are
fastify.log.error(
`User requested a resource that was not in our namespaces. Namespace: ${namespace}`,
Expand Down
5 changes: 5 additions & 0 deletions frontend/src/__mocks__/mockDscStatus.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ export const mockDscStatus = ({
conditions = [],
phase = 'Ready',
}: MockDscStatus): DataScienceClusterKindStatus => ({
components: {
modelregistry: {
registriesNamespace: 'odh-model-registries',
},
},
conditions: [
...[
{
Expand Down
3 changes: 1 addition & 2 deletions frontend/src/__mocks__/mockModelRegistry.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { MODEL_REGISTRY_DEFAULT_NAMESPACE } from '~/concepts/modelRegistry/const';
import { ModelRegistryKind } from '~/k8sTypes';

type MockModelRegistryType = {
Expand All @@ -8,7 +7,7 @@ type MockModelRegistryType = {

export const mockModelRegistry = ({
name = 'modelregistry-sample',
namespace = MODEL_REGISTRY_DEFAULT_NAMESPACE,
namespace = 'odh-model-registries',
}: MockModelRegistryType): ModelRegistryKind => ({
apiVersion: 'modelregistry.opendatahub.io/v1alpha1',
kind: 'ModelRegistry',
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* eslint-disable camelcase */
import { mockK8sResourceList } from '~/__mocks__';
import { mockDscStatus, mockK8sResourceList } from '~/__mocks__';
import { mockComponents } from '~/__mocks__/mockComponents';
import { mockDashboardConfig } from '~/__mocks__/mockDashboardConfig';
import { mockRegisteredModelList } from '~/__mocks__/mockRegisteredModelsList';
Expand Down Expand Up @@ -72,6 +72,15 @@ const initIntercepts = ({
],
allowed = true,
}: HandlersProps) => {
cy.interceptOdh(
'GET /api/dsc/status',
mockDscStatus({
installedComponents: {
'model-registry-operator': true,
},
}),
);

cy.interceptOdh(
'GET /api/config',
mockDashboardConfig({
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* eslint-disable camelcase */
import { mockK8sResourceList } from '~/__mocks__';
import { mockDscStatus, mockK8sResourceList } from '~/__mocks__';
import { mockDashboardConfig } from '~/__mocks__/mockDashboardConfig';
import { mockRegisteredModelList } from '~/__mocks__/mockRegisteredModelsList';
import { ServiceModel } from '~/__tests__/cypress/cypress/utils/models';
Expand Down Expand Up @@ -47,6 +47,15 @@ const initIntercepts = ({
mockModelVersion({ id: '3', name: 'model version 3' }),
],
}: HandlersProps) => {
cy.interceptOdh(
'GET /api/dsc/status',
mockDscStatus({
installedComponents: {
'model-registry-operator': true,
},
}),
);

cy.interceptOdh(
'GET /api/config',
mockDashboardConfig({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
mockServingRuntimeK8sResource,
mockInferenceServiceK8sResource,
mockProjectK8sResource,
mockDscStatus,
} from '~/__mocks__';

import {
Expand All @@ -33,6 +34,16 @@ const initIntercepts = () => {
disableModelRegistry: false,
}),
);

cy.interceptOdh(
'GET /api/dsc/status',
mockDscStatus({
installedComponents: {
'model-registry-operator': true,
},
}),
);

cy.interceptOdh('GET /api/components', { query: { installed: 'true' } }, mockComponents());

cy.interceptK8sList(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* eslint-disable camelcase */
import { mockK8sResourceList } from '~/__mocks__';
import { mockDscStatus, mockK8sResourceList } from '~/__mocks__';
import { mockDashboardConfig } from '~/__mocks__/mockDashboardConfig';
import { mockModelVersionList } from '~/__mocks__/mockModelVersionList';
import { mockRegisteredModelList } from '~/__mocks__/mockRegisteredModelsList';
Expand Down Expand Up @@ -47,6 +47,15 @@ const initIntercepts = ({
mockModelVersion({ id: '2', name: 'model version' }),
],
}: HandlersProps) => {
cy.interceptOdh(
'GET /api/dsc/status',
mockDscStatus({
installedComponents: {
'model-registry-operator': true,
},
}),
);

cy.interceptOdh(
'GET /api/config',
mockDashboardConfig({
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* eslint-disable camelcase */
import { mockK8sResourceList } from '~/__mocks__';
import { mockDscStatus, mockK8sResourceList } from '~/__mocks__';
import { mockDashboardConfig } from '~/__mocks__/mockDashboardConfig';
import { mockRegisteredModelList } from '~/__mocks__/mockRegisteredModelsList';
import { ServiceModel } from '~/__tests__/cypress/cypress/utils/models';
Expand Down Expand Up @@ -58,6 +58,15 @@ const initIntercepts = ({
}),
);

cy.interceptOdh(
'GET /api/dsc/status',
mockDscStatus({
installedComponents: {
'model-registry-operator': true,
},
}),
);

cy.interceptK8sList(
ServiceModel,
mockK8sResourceList([
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { mockK8sResourceList, mockProjectK8sResource } from '~/__mocks__';
import { mockDscStatus, mockK8sResourceList, mockProjectK8sResource } from '~/__mocks__';
import { mock200Status } from '~/__mocks__/mockK8sStatus';
import { mockRoleBindingK8sResource } from '~/__mocks__/mockRoleBindingK8sResource';
import { be } from '~/__tests__/cypress/cypress/utils/should';
Expand Down Expand Up @@ -51,6 +51,7 @@ const initIntercepts = ({ isEmpty = false, hasPermission = true }: HandlersProps
} else {
asProductAdminUser();
}
cy.interceptOdh('GET /api/dsc/status', mockDscStatus({}));
cy.interceptK8sList(
ModelRegistryModel,
mockK8sResourceList([
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,10 @@ describe('useModelRegistryServices', () => {
expect(isLoaded).toBe(true);
expect(mockGetResource).toHaveBeenCalledTimes(2);
expect(mockGetResource).toHaveBeenCalledWith({
queryOptions: { name: 'service-1', ns: 'odh-model-registries' },
queryOptions: { name: 'service-1', ns: 'test-namespace' },
});
expect(mockGetResource).toHaveBeenCalledWith({
queryOptions: { name: 'service-2', ns: 'odh-model-registries' },
queryOptions: { name: 'service-2', ns: 'test-namespace' },
});
});

Expand Down
Loading

0 comments on commit b899b25

Please sign in to comment.