Skip to content

Commit

Permalink
Add support for external routes to proxyService, use for model registry
Browse files Browse the repository at this point in the history
Signed-off-by: Mike Turley <[email protected]>
  • Loading branch information
mturley committed Sep 17, 2024
1 parent 5613032 commit 97e0d6b
Show file tree
Hide file tree
Showing 9 changed files with 121 additions and 67 deletions.
2 changes: 1 addition & 1 deletion backend/src/routes/api/service/mlmd/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export default proxyService<DSPipelineKind>(
plural: 'datasciencepipelinesapplications',
},
{
constructUrl: (resource) => resource.status?.components.mlmdProxy.url,
constructUrl: (resource: DSPipelineKind) => resource.status?.components.mlmdProxy.url,
},
{
// Use port forwarding for local development:
Expand Down
5 changes: 3 additions & 2 deletions backend/src/routes/api/service/modelregistry/index.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import { ServiceAddressAnnotation } from '../../../../types';
import { MODEL_REGISTRY_NAMESPACE } from '../../../../utils/constants';
import { proxyService } from '../../../../utils/proxy';

export default proxyService(
null,
{
port: 8080,
addressAnnotation: ServiceAddressAnnotation.EXTERNAL_REST,
internalPort: 8080,
namespace: MODEL_REGISTRY_NAMESPACE,
},
{
Expand All @@ -14,5 +16,4 @@ export default proxyService(
port: process.env.MODEL_REGISTRY_SERVICE_PORT,
},
null,
false,
);
2 changes: 1 addition & 1 deletion backend/src/routes/api/service/pipelines/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export default proxyService<DSPipelineKind>(
plural: 'datasciencepipelinesapplications',
},
{
constructUrl: (resource) => resource.status?.components.apiServer.url,
constructUrl: (resource: DSPipelineKind) => resource.status?.components.apiServer.url,
},
{
// Use port forwarding for local development:
Expand Down
2 changes: 1 addition & 1 deletion backend/src/routes/api/service/trustyai/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export default proxyService<TrustyAIKind>(
plural: 'trustyaiservices',
},
{
port: 443,
internalPort: 443,
suffix: '-tls',
},
{
Expand Down
5 changes: 5 additions & 0 deletions backend/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1215,3 +1215,8 @@ export type ResponseStatus = {
success: boolean;
error: string;
};

export enum ServiceAddressAnnotation {
EXTERNAL_REST = 'routing.opendatahub.io/external-address-rest',
EXTERNAL_GRPC = 'routing.opendatahub.io/external-address-grpc',
}
128 changes: 84 additions & 44 deletions backend/src/utils/proxy.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import { FastifyRequest } from 'fastify';
import httpProxy from '@fastify/http-proxy';
import { K8sResourceCommon, KubeFastifyInstance } from '../types';
import { K8sResourceCommon, KubeFastifyInstance, ServiceAddressAnnotation } from '../types';
import { isK8sStatus, passThroughResource } from '../routes/api/k8s/pass-through';
import { DEV_MODE } from './constants';
import { createCustomError } from './requestUtils';
import { getAccessToken, getDirectCallOptions } from './directCallUtils';
import { EitherNotBoth } from '../typeHelpers';
import { V1Service } from '@kubernetes/client-node';

export const getParam = <F extends FastifyRequest<any, any>>(req: F, name: string): string =>
(req.params as { [key: string]: string })[name];
Expand All @@ -26,12 +27,14 @@ const notFoundError = (kind: string, name: string, e?: any, overrideMessage?: st
404,
);
};

export const proxyService =
<K extends K8sResourceCommon = never>(
model: { apiGroup: string; apiVersion: string; plural: string; kind: string } | null,
service: EitherNotBoth<
{
port: number | string;
addressAnnotation?: ServiceAddressAnnotation;
internalPort: number | string;
prefix?: string;
suffix?: string;
namespace?: string;
Expand Down Expand Up @@ -74,56 +77,93 @@ export const proxyService =
// see `prefix` for named params
const namespace = service.namespace ?? getParam(request, 'namespace');
const name = getParam(request, 'name');
const serviceName = `${service.prefix ?? ''}${name}${service.suffix ?? ''}`;
const scheme = tls ? 'https' : 'http';
const kc = fastify.kube.config;
const cluster = kc.getCurrentCluster();

const doServiceRequest = (resource?: K) => {
const scheme = tls ? 'https' : 'http';

const upstream = DEV_MODE
? // Use port forwarding for local development:
// kubectl port-forward -n <namespace> svc/<service-name> <local.port>:<service.port>
`${scheme}://${local.host}:${local.port}`
: // Construct service URL
service.constructUrl
? service.constructUrl(resource)
: `${scheme}://${service.prefix || ''}${name}${
service.suffix ?? ''
}.${namespace}.svc.cluster.local:${service.port}`;
const getServiceAddress = async (
serviceName: string,
resource?: K,
): Promise<string | null> => {
if (DEV_MODE) {
// Use port forwarding for local development:
// kubectl port-forward -n <namespace> svc/<service-name> <local.port>:<service.port>
return `${scheme}://${local.host}:${local.port}`;
}
if (service.constructUrl) {
return service.constructUrl(resource);
}
if (service.addressAnnotation) {
try {
const k8sService = await passThroughResource<V1Service>(fastify, request, {
url: `${cluster.server}/api/v1/namespaces/${namespace}/services/${serviceName}`,
method: 'GET',
});
if (isK8sStatus(k8sService)) {
fastify.log.error(
`Proxy failed to read k8s service ${serviceName} in namespace ${namespace}.`,
);
return null;
}
const address = k8sService.metadata?.annotations?.[service.addressAnnotation];
if (address) {
return `${scheme}://${address}`;
}
fastify.log.error(
`Proxy could not find address annotation on k8s service ${serviceName} in namespace ${namespace}, falling back to internal address. Annotation expected: ${service.addressAnnotation}`,
);
} catch (e) {
fastify.log.error(
e,
`Proxy failed to read k8s service ${serviceName} in namespace ${namespace}.`,
);
return null;
}
}
// For services configured for internal addresses (no annotation), construct the URL
// or if annotation is expected but missing, fall back to internal address for compatibility
return `${scheme}://${serviceName}.${namespace}.svc.cluster.local:${service.internalPort}`;
};

// assign the `upstream` param so we can dynamically set the upstream URL for http-proxy
const doServiceRequest = async (resource?: K) => {
const upstream = await getServiceAddress(serviceName, resource);
if (!upstream) {
done(notFoundError('Service', serviceName, undefined, 'service unavailable'));
return;
}
// Assign the `upstream` param so we can dynamically set the upstream URL for http-proxy
setParam(request, 'upstream', upstream);

if (tls) {
const requestOptions = await getDirectCallOptions(fastify, request, '');
const token = getAccessToken(requestOptions);
request.headers.authorization = `Bearer ${token}`;
}
fastify.log.info(`Proxy ${request.method} request ${request.url} to ${upstream}`);
done();
};

if (model) {
const kc = fastify.kube.config;
const cluster = kc.getCurrentCluster();

// retreive the gating resource by name and namespace
passThroughResource<K>(fastify, request, {
url: `${cluster.server}/apis/${model.apiGroup}/${model.apiVersion}/namespaces/${namespace}/${model.plural}/${name}`,
method: 'GET',
})
.then((resource) => {
return getDirectCallOptions(fastify, request, request.url).then((requestOptions) => {
if (isK8sStatus(resource)) {
done(notFoundError(model.kind, name));
} else if (!statusCheck || statusCheck(resource)) {
if (tls) {
const token = getAccessToken(requestOptions);
request.headers.authorization = `Bearer ${token}`;
}

doServiceRequest(resource);
} else {
done(notFoundError(model.kind, name, undefined, 'service unavailable'));
}
});
})
.catch((e) => {
done(notFoundError(model.kind, name, e));
const doServiceRequestWithGatingResource = async () => {
try {
// Retreive the gating resource by name and namespace
const resource = await passThroughResource<K>(fastify, request, {
url: `${cluster.server}/apis/${model.apiGroup}/${model.apiVersion}/namespaces/${namespace}/${model.plural}/${name}`,
method: 'GET',
});
if (isK8sStatus(resource)) {
done(notFoundError(model.kind, name));
} else if (!statusCheck || statusCheck(resource)) {
doServiceRequest(resource);
} else {
done(notFoundError(model.kind, name, undefined, 'service unavailable'));
}
} catch (e) {
done(notFoundError(model.kind, name, e));
}
};

if (model) {
doServiceRequestWithGatingResource();
} else {
doServiceRequest();
}
Expand Down
13 changes: 7 additions & 6 deletions frontend/src/__mocks__/mockModelRegistry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,13 @@ export const mockModelRegistry = ({
namespace,
},
spec: {
grpc: {
port: 9090,
grpc: {},
rest: {},
istio: {
gateway: {
grpc: { tls: {} },
rest: { tls: {} },
},
},
postgres: {
database: 'model-registry',
Expand All @@ -33,10 +38,6 @@ export const mockModelRegistry = ({
sslMode: 'disable',
username: 'mlmduser',
},
rest: {
port: 8080,
serviceRoute: 'disabled',
},
},
status: {
conditions: [
Expand Down
17 changes: 11 additions & 6 deletions frontend/src/k8sTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1379,12 +1379,17 @@ export type ModelRegistryKind = K8sResourceCommon & {
annotations?: DisplayNameAnnotations;
};
spec: {
grpc: {
port: number;
};
rest: {
port: number;
serviceRoute: string;
grpc: Record<string, never>; // Empty object at create time, properties here aren't used by the UI
rest: Record<string, never>; // Empty object at create time, properties here aren't used by the UI
istio: {
gateway: {
grpc: {
tls: Record<string, never>; // Empty object at create time, properties here aren't used by the UI
};
rest: {
tls: Record<string, never>; // Empty object at create time, properties here aren't used by the UI
};
};
};
} & EitherNotBoth<
{
Expand Down
14 changes: 8 additions & 6 deletions frontend/src/pages/modelRegistrySettings/CreateModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -82,18 +82,20 @@ const CreateModal: React.FC<CreateModalProps> = ({ isOpen, onClose, refresh }) =
},
},
spec: {
grpc: {
port: 9090,
},
rest: {
port: 8080,
serviceRoute: 'disabled',
grpc: {},
rest: {},
istio: {
gateway: {
grpc: { tls: {} },
rest: { tls: {} },
},
},
mysql: {
host,
port: Number(port),
database,
username,
skipDBCreation: false,
},
},
};
Expand Down

0 comments on commit 97e0d6b

Please sign in to comment.