Skip to content

Commit

Permalink
Improve sources configuration typings (#1944)
Browse files Browse the repository at this point in the history
  • Loading branch information
melikhov-dev authored Dec 13, 2024
1 parent 34aad28 commit e5c259a
Show file tree
Hide file tree
Showing 6 changed files with 108 additions and 50 deletions.
1 change: 1 addition & 0 deletions src/@types/nodekit.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import type {Request, Response} from '@gravity-ui/expresskit';

import type {RedisConfig} from '../server/components/cache-client';
import type {ChartTemplates} from '../server/components/charts-engine/components/chart-generator';
import type {SourceConfig} from '../server/components/charts-engine/types';
import type {AppEnvironment} from '../shared';
import type {FeatureConfig} from '../shared/types';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import querystring from 'querystring';
import url from 'url';

import type {Request} from '@gravity-ui/expresskit';
import type {AppContext} from '@gravity-ui/nodekit';
import {REQUEST_ID_PARAM_NAME} from '@gravity-ui/nodekit';
import {isObject, isString} from 'lodash';
import sizeof from 'object-sizeof';
import PQueue from 'p-queue';
Expand All @@ -21,7 +23,7 @@ import {
import {registry} from '../../../../registry';
import {config} from '../../constants';
import type {ChartsEngine} from '../../index';
import type {Source} from '../../types';
import type {Source, SourceConfig} from '../../types';
import {Request as RequestPromise} from '../request';
import {hideSensitiveData} from '../utils';

Expand Down Expand Up @@ -59,6 +61,7 @@ type DataFetcherOptions = {
chartsEngine: ChartsEngine;
sources: Record<string, Source | string>;
req: Request;
ctx?: AppContext;
postprocess?:
| ((
data: Record<string, DataFetcherResult>,
Expand Down Expand Up @@ -151,13 +154,18 @@ export class DataFetcher {
chartsEngine,
sources,
req,
ctx,
postprocess = null,
subrequestHeaders,
userId,
userLogin,
iamToken,
workbookId,
}: DataFetcherOptions): Promise<Record<string, DataFetcherResult>> {
// TODO: remove aftex extension will be migrated
if (ctx === undefined) {
ctx = req.ctx;
}
const fetchingTimeout = chartsEngine.config.fetchingTimeout || DEFAULT_FETCHING_TIMEOUT;

const fetchingStartTime = Date.now();
Expand Down Expand Up @@ -185,6 +193,7 @@ export class DataFetcher {
source
? DataFetcher.fetchSource({
req,
ctx,
sourceName,
source: isString(source) ? {url: source} : source,
chartsEngine,
Expand Down Expand Up @@ -401,6 +410,7 @@ export class DataFetcher {
sourceName,
source,
req,
ctx,
chartsEngine,
fetchingStartTime,
subrequestHeaders,
Expand All @@ -414,6 +424,7 @@ export class DataFetcher {
sourceName: string;
source: Source;
req: Request;
ctx: AppContext;
chartsEngine: ChartsEngine;
fetchingStartTime: number;
subrequestHeaders: Record<string, string>;
Expand All @@ -424,7 +435,6 @@ export class DataFetcher {
iamToken?: string | null;
workbookId?: WorkbookId;
}) {
const ctx = req.ctx;
const singleFetchingTimeout =
chartsEngine.config.singleFetchingTimeout || DEFAULT_SINGLE_FETCHING_TIMEOUT;

Expand Down Expand Up @@ -533,7 +543,12 @@ export class DataFetcher {

const {passedCredentials, extraHeaders, sourceType} = sourceConfig;

if (sourceConfig.allowedMethods && !sourceConfig.allowedMethods.includes(sourceMethod)) {
if (
sourceConfig.allowedMethods &&
!sourceConfig.allowedMethods.includes(
sourceMethod as 'GET' | 'POST' | 'PUT' | 'PATCH' | 'DELETE',
)
) {
const message = `This HTTP method (${sourceMethod}) is not allowed for this source: ${sourceName}`;

ctx.logError(message);
Expand All @@ -546,35 +561,15 @@ export class DataFetcher {
}

if (sourceConfig.check) {
try {
const checkResult = await sourceConfig.check(req, targetUri, req.body.params);
if (checkResult === true) {
ctx.log('Access to source allowed');
} else if (checkResult === false) {
ctx.log('Access to source forbidden');

return {
sourceId: sourceName,
sourceType,
message: 'Access to source forbidden',
};
} else {
ctx.logError('Source access check failed');

return {
sourceId: sourceName,
sourceType,
message: 'Source access check failed',
};
}
} catch (error) {
ctx.logError('Failed to run source check', error);

return {
sourceId: sourceName,
sourceType,
message: 'Failed to run source check',
};
const result = await sourceConfigCheck({
ctx,
targetUri,
sourceConfig,
sourceName,
sourceType,
});
if (result.valid === false) {
return result;
}
}

Expand All @@ -593,9 +588,8 @@ export class DataFetcher {
return sourceConfig.adapter({
targetUri: croppedTargetUri,
sourceName,
source,
fetchingStartTime,
req,
ctx,
});
}

Expand Down Expand Up @@ -804,7 +798,7 @@ export class DataFetcher {
onDataFetchingFailed(error, {
sourceName: dataSourceName,
statusCode,
requestId: req.id,
requestId: ctx.get(REQUEST_ID_PARAM_NAME) || '',
latency,
traceId,
tenantId,
Expand Down Expand Up @@ -911,7 +905,7 @@ export class DataFetcher {
onDataFetched({
sourceName: dataSourceName,
statusCode: response.statusCode,
requestId: req.id,
requestId: ctx.get(REQUEST_ID_PARAM_NAME) || '',
latency,
url: publicTargetUri,
traceId,
Expand Down Expand Up @@ -1016,3 +1010,70 @@ export class DataFetcher {
});
}
}

type SourceCheckResult = {
valid: boolean;
meta?: {
sourceId: string;
sourceType?: string;
message: string;
};
};

async function sourceConfigCheck({
ctx,
sourceName,
sourceType,
sourceConfig,
targetUri,
}: {
ctx: AppContext;
sourceName: string;
sourceType?: string;
sourceConfig: SourceConfig;
targetUri: string;
}): Promise<SourceCheckResult> {
if (!sourceConfig.check) {
return {valid: true};
}
try {
const checkResult = await sourceConfig.check(targetUri);
if (checkResult === true) {
ctx.log('Access to source allowed');
return {valid: true};
} else if (checkResult === false) {
ctx.log('Access to source forbidden');

return {
valid: false,
meta: {
sourceId: sourceName,
sourceType,
message: 'Access to source forbidden',
},
};
} else {
ctx.logError('Source access check failed');

return {
valid: false,
meta: {
sourceId: sourceName,
sourceType,
message: 'Source access check failed',
},
};
}
} catch (error) {
ctx.logError('Failed to run source check', error);

return {
valid: false,
meta: {
sourceId: sourceName,
sourceType,
message: 'Failed to run source check',
},
};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,6 @@ export class Processor {
hrStart = process.hrtime();
try {
processedModules = await builder.buildModules({
req,
ctx,
subrequestHeaders,
onModuleBuild: ({executionTiming, filename}) => {
Expand Down Expand Up @@ -558,6 +557,7 @@ export class Processor {
chartsEngine,
sources,
req,
ctx,
iamToken,
subrequestHeaders,
userId,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import type {Request} from '@gravity-ui/expresskit';
import type {AppContext} from '@gravity-ui/nodekit';

import type {
Expand Down Expand Up @@ -147,7 +146,6 @@ export type ChartBuilder = {
buildShared: () => Promise<void>;
buildModules: (args: {
subrequestHeaders: Record<string, string>;
req: Request;
ctx: AppContext;
onModuleBuild: (args: {executionTiming: [number, number]; filename: string}) => void;
}) => Promise<Record<string, ChartBuilderResult>>;
Expand Down
17 changes: 7 additions & 10 deletions src/server/components/charts-engine/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import type {OutgoingHttpHeaders} from 'http';

import type {AppMiddleware, AppRouteDescription, Request, Response} from '@gravity-ui/expresskit';
import type {HttpMethod} from '@gravity-ui/expresskit/dist/types';
import type {AppContext} from '@gravity-ui/nodekit';

import type {EDITOR_TYPE_CONFIG_TABS} from '../../../shared';
import type {SourcesArgs} from '../../modes/charts/plugins/datalens/url/build-sources/types';
Expand Down Expand Up @@ -138,30 +139,26 @@ export type SourceConfig = {
uiEndpointFormatter?: (url: string, sourceData?: Source['data']) => string | null;
uiEndpoint?: string;
passedCredentials?: Record<string, boolean>;
extraHeaders?: Record<string, string> | ((req: Request) => Record<string, string>);
extraHeaders?: Record<string, string | undefined> | ((req: Request) => Record<string, string>);
sourceType?: string;
dataEndpoint?: string;
preprocess?: (url: string) => string;
allowedMethods?: 'GET' | 'POST' | 'PUT' | 'PATCH' | 'DELETE';
allowedMethods?: ('GET' | 'POST' | 'PUT' | 'PATCH' | 'DELETE')[];

adapter?: ({
targetUri,
sourceName,
req,
ctx,
}: {
targetUri: string;
sourceName: string;
source: Source;
req: Request;
fetchingStartTime: number;
}) => Promise<unknown>;
ctx: AppContext;
}) => unknown;

middlewareAdapter?: (args: MiddlewareSourceAdapterArgs) => Promise<any>;
check?: (
req: Request,
targetUri: string,
params: Request['body']['params'],
) => Promise<boolean>;
check?: (targetUri: string, params?: Request['body']['params']) => Promise<boolean>;

args?: Record<string, string | number | (string | number)[]>;
maxRedirects?: number;
Expand Down
3 changes: 2 additions & 1 deletion src/server/configs/opensource/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
} from '../../../shared';
import {resolveSource} from '../../../shared/endpoints/sources';
import {nativeModules} from '../../components/charts-engine/components/processor/native-modules';
import type {SourceConfig} from '../../components/charts-engine/types';
import {SERVICE_NAME_DATALENS} from '../../constants';
import controlDashChartTemplate from '../shared/control-dash-chart-template';
import datalensChartTemplate from '../shared/datalens-chart-template';
Expand Down Expand Up @@ -59,7 +60,7 @@ export default {
control_dash: controlDashChartTemplate,
},

getSourcesByEnv: (env: AppEnvironment) => {
getSourcesByEnv: (env: AppEnvironment): Record<string, SourceConfig> => {
const sources = resolveSource(AppInstallation.Opensource, env);

return {
Expand Down

0 comments on commit e5c259a

Please sign in to comment.