Skip to content

Commit

Permalink
Use registryProxyUrl config setting; not env vars
Browse files Browse the repository at this point in the history
  • Loading branch information
John Schulz committed Oct 5, 2020
1 parent 2124232 commit d91456d
Show file tree
Hide file tree
Showing 5 changed files with 11 additions and 54 deletions.
1 change: 1 addition & 0 deletions x-pack/plugins/ingest_manager/common/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export * from './rest_spec';
export interface IngestManagerConfigType {
enabled: boolean;
registryUrl?: string;
registryProxyUrl?: string;
fleet: {
enabled: boolean;
tlsCheckDisabled: boolean;
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/ingest_manager/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export const config = {
schema: schema.object({
enabled: schema.boolean({ defaultValue: true }),
registryUrl: schema.maybe(schema.uri()),
registryProxyUrl: schema.maybe(schema.uri()),
fleet: schema.object({
enabled: schema.boolean({ defaultValue: true }),
tlsCheckDisabled: schema.boolean({ defaultValue: false }),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

import HttpProxyAgent from 'http-proxy-agent';
import { HttpsProxyAgent } from 'https-proxy-agent';
import { getProxyAgent, getProxyAgentOptions, getProxyForUrl } from './proxy';
import { getProxyAgent, getProxyAgentOptions } from './proxy';

describe('getProxyAgent', () => {
test('return HttpsProxyAgent for https proxy url', () => {
Expand All @@ -26,49 +26,6 @@ describe('getProxyAgent', () => {
});
});

// getProxyForUrl is currently an alias for https://github.com/Rob--W/proxy-from-env
// see those docs & tests for all the environment variables it supports
// these tests set some different ones to confirm they work,
// but aren't meant to be convey all that's supported or possible
describe('getProxyForUrl', () => {
test('if https_proxy env is set - return url only for https', () => {
const proxyA = 'https://12.34.56.78:910';
process.env.https_proxy = proxyA;

const urlA = getProxyForUrl('https://example.com/?a=b&c=d');
expect(urlA).toBe(proxyA);

// given http value and https proxy
const urlB = getProxyForUrl('http://example.com/?a=b&c=d');
expect(urlB).toBe('');

delete process.env.https_proxy;
});

test('if ALL_PROXY env is set - return url for https & http', () => {
const proxyA = 'https://some.tld';
process.env.ALL_PROXY = proxyA;

const urlA = getProxyForUrl('https://example.com/?a=b&c=d');
expect(urlA).toBe(proxyA);

// given http value and https proxy
const urlB = getProxyForUrl('http://example.com/?a=b&c=d');
expect(urlB).toBe(proxyA);

delete process.env.ALL_PROXY;
});

// putting these at the end to help catch if we don't clean up process.env mutations above
test('if no env variables are set - return empty string for all', () => {
const urlA = getProxyForUrl('https://example.com/?a=b&c=d');
expect(urlA).toBe('');

const urlB = getProxyForUrl('http://example.com/?a=b&c=d');
expect(urlB).toBe('');
});
});

describe('getProxyAgentOptions', () => {
test('return url only for https', () => {
const httpsProxy = 'https://12.34.56.78:910';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,20 +9,18 @@ import HttpsProxyAgent, {
HttpsProxyAgent as IHttpsProxyAgent,
HttpsProxyAgentOptions,
} from 'https-proxy-agent';
import { getProxyForUrl as getProxyFromEnvForUrl } from 'proxy-from-env';

export interface ProxySettings {
import { appContextService } from '../../index';
export interface RegistryProxySettings {
proxyUrl: string;
proxyHeaders?: Record<string, string>;
proxyRejectUnauthorizedCertificates?: boolean;
}

type ProxyAgent = IHttpsProxyAgent | HttpProxyAgent;
type GetProxyAgentParams = ProxySettings & { targetUrl: string };
type GetProxyAgentParams = RegistryProxySettings & { targetUrl: string };

export function getProxyForUrl(url: string): string {
// start with env values and can add from flags, defaults, etc later
const proxyUrl = getProxyFromEnvForUrl(url);
export function getRegistryProxyUrl(): string | undefined {
const proxyUrl = appContextService.getConfig()?.registryProxyUrl;
return proxyUrl;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import pRetry from 'p-retry';
import { streamToString } from './streams';
import { appContextService } from '../../app_context';
import { RegistryError, RegistryConnectionError, RegistryResponseError } from '../../../errors';
import { getProxyAgent, getProxyForUrl } from './proxy';
import { getProxyAgent, getRegistryProxyUrl } from './proxy';

type FailedAttemptErrors = pRetry.FailedAttemptError | FetchError | Error;

Expand Down Expand Up @@ -84,13 +84,13 @@ function isSystemError(error: FailedAttemptErrors): boolean {
}

export function getFetchOptions(targetUrl: string): RequestInit | undefined {
const proxyUrl = getProxyForUrl(targetUrl);
const proxyUrl = getRegistryProxyUrl();
if (!proxyUrl) {
return undefined;
}

const logger = appContextService.getLogger();
logger.debug(`Using ${proxyUrl} from environment variable as proxy for ${targetUrl}`);
logger.debug(`Using ${proxyUrl} as proxy for ${targetUrl}`);

return {
agent: getProxyAgent({ proxyUrl, targetUrl }),
Expand Down

0 comments on commit d91456d

Please sign in to comment.