Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DO NOT MERGE] Gen1 Request Forwarding to Gen2 #10535

Closed
wants to merge 13 commits into from
Closed

[DO NOT MERGE] Gen1 Request Forwarding to Gen2 #10535

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
64115a7
add request forwarding to Gen2
soloseng Aug 31, 2023
9abfdf1
using http-proxy lib
soloseng Aug 31, 2023
5e5e300
updated error logging
soloseng Aug 31, 2023
12a2ccc
++ timeout
soloseng Aug 31, 2023
126dea2
Merge branch 'master' into soloseng/create-combiner-proxy
soloseng Sep 1, 2023
4442d9a
typo
soloseng Sep 1, 2023
8686699
Merge branch 'master' into soloseng/create-combiner-proxy
soloseng Sep 1, 2023
9f7a498
mod proxy config
soloseng Sep 1, 2023
222b0fe
troubleshooting combiner proxy
soloseng Sep 5, 2023
951c364
clean up
soloseng Sep 14, 2023
1ce7723
Merge commit '2cc7296f1d66d1511099ab6562ac5e37ceeb8bc3' into soloseng…
soloseng Sep 14, 2023
943902f
Merge branch 'master' into soloseng/create-combiner-proxy
soloseng Sep 14, 2023
449c176
Merge branch 'master' into soloseng/create-combiner-proxy
soloseng Sep 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/phone-number-privacy/combiner/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
"express": "^4.17.1",
"firebase-admin": "^11.10.1",
"firebase-functions": "^4.4.1",
"http-proxy": "^1.18.1",
"knex": "^2.1.0",
"node-fetch": "^2.6.9",
"pg": "^8.2.1",
Expand All @@ -57,6 +58,7 @@
"@celo/utils": "^4.1.2-dev",
"@celo/phone-utils": "^4.1.2-dev",
"@types/express": "^4.17.6",
"@types/http-proxy":"^1.17.11",
"@types/supertest": "^2.0.12",
"@types/uuid": "^7.0.3",
"firebase-functions-test": "^3.1.0",
Expand Down
22 changes: 18 additions & 4 deletions packages/phone-number-privacy/combiner/src/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,16 @@ export interface OdisConfig {
fullNodeRetryCount: number
fullNodeRetryDelayMs: number
}

export interface ProxyConfig {
deploymentEnv: string
forwardToGen2: boolean
}
export interface CombinerConfig {
serviceName: string
blockchain: BlockchainConfig
phoneNumberPrivacy: OdisConfig
domains: OdisConfig
proxy: ProxyConfig
}

let config: CombinerConfig
Expand Down Expand Up @@ -141,6 +145,10 @@ if (DEV_MODE) {
fullNodeRetryCount: RETRY_COUNT,
fullNodeRetryDelayMs: RETRY_DELAY_IN_MS,
},
proxy: {
forwardToGen2: false,
deploymentEnv: 'local',
},
}
} else {
const functionConfig = functions.config()
Expand Down Expand Up @@ -182,12 +190,18 @@ if (DEV_MODE) {
currentVersion: Number(functionConfig.domains_keys.current_version),
versions: functionConfig.domains_keys.versions,
},
fullNodeTimeoutMs: Number(functionConfig.pnp.full_node_timeout_ms ?? FULL_NODE_TIMEOUT_IN_MS),
fullNodeRetryCount: Number(functionConfig.pnp.full_node_retry_count ?? RETRY_COUNT),
fullNodeTimeoutMs: Number(
functionConfig.domains.full_node_timeout_ms ?? FULL_NODE_TIMEOUT_IN_MS
soloseng marked this conversation as resolved.
Show resolved Hide resolved
),
fullNodeRetryCount: Number(functionConfig.domains.full_node_retry_count ?? RETRY_COUNT),
fullNodeRetryDelayMs: Number(
functionConfig.pnp.full_node_retry_delay_ms ?? RETRY_DELAY_IN_MS
functionConfig.domains.full_node_retry_delay_ms ?? RETRY_DELAY_IN_MS
),
},
proxy: {
forwardToGen2: toBool(functionConfig.proxy.forward_to_gen2, false),
deploymentEnv: functionConfig.proxy.deployment_env,
},
}
}
export default config
12 changes: 10 additions & 2 deletions packages/phone-number-privacy/combiner/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { getContractKitWithAgent } from '@celo/phone-number-privacy-common'
import * as functions from 'firebase-functions'

import config from './config'
import { startCombiner } from './server'
import { startCombiner, startProxy } from './server'

require('dotenv').config()

Expand All @@ -12,5 +13,12 @@ export const combiner = functions
// Defined check required for running tests vs. deployment
minInstances: functions.config().service ? Number(functions.config().service.min_instances) : 0,
})
.https.onRequest(startCombiner(config, getContractKitWithAgent(config.blockchain)))
.https.onRequest((req, res) => {
if (config.proxy.forwardToGen2) {
startProxy(req, res, config)
} else {
const app = startCombiner(config, getContractKitWithAgent(config.blockchain))
app(req, res)
}
})
export * from './config'
49 changes: 47 additions & 2 deletions packages/phone-number-privacy/combiner/src/server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ import {
OdisRequest,
rootLogger,
} from '@celo/phone-number-privacy-common'
import express, { RequestHandler } from 'express'
import express, { Express, RequestHandler } from 'express'
import httpProxy from 'http-proxy'
import { Signer } from './common/combine'
import {
catchErrorHandler,
Expand All @@ -24,7 +25,7 @@ import { createPnpSignHandler } from './pnp/endpoints/sign/action'

require('events').EventEmitter.defaultMaxListeners = 15

export function startCombiner(config: CombinerConfig, kit: ContractKit) {
export function startCombiner(config: CombinerConfig, kit: ContractKit): Express {
const logger = rootLogger(config.serviceName)

logger.info('Creating combiner express server')
Expand Down Expand Up @@ -87,3 +88,47 @@ export function createHandler<R extends OdisRequest>(
): PromiseHandler<R> {
return meteringHandler(catchErrorHandler(enabled ? handler : disabledHandler<R>))
}

export function startProxy(req: any, res: any, config: CombinerConfig) {
let destinationUrl

const logger = rootLogger(config.serviceName)
const proxy = httpProxy.createProxyServer({
proxyTimeout: config.phoneNumberPrivacy.odisServices.timeoutMilliSeconds,
})

const originalPath = req.path
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are you just trying to get the endpoint? You should be able to just use req.url for that

const strippedPath = originalPath.replace(/\/combiner/, '')

switch (config.proxy.deploymentEnv) {
soloseng marked this conversation as resolved.
Show resolved Hide resolved
case 'mainnet':
// XXX (soloseng):URL may need to be updated after gen2 function is created on mainnet
destinationUrl =
'https://us-central1-celo-pgpnp-mainnet.cloudfunctions.net/combinerGen2' +
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like you forgot to add strippedPath here

proxy.web(req, res, { target: destinationUrl })
break
case 'alfajores':
// XXX (soloseng):URL may need to be updated after gen2 function is created on alfajores
destinationUrl =
'https://us-central1-celo-phone-number-privacy.cloudfunctions.net/combinerGen2' +
strippedPath

proxy.web(req, res, { target: destinationUrl })
break
case 'staging':
destinationUrl =
'https://us-central1-celo-phone-number-privacy-stg.cloudfunctions.net/combinerGen2' +
strippedPath

proxy.web(req, res, { target: destinationUrl })
break
}
proxy.on('error', (_) => {
logger.error('Error in Proxying request to Combiner.')
res.status(500).json({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this override the actual error returned by the gen2 combiner? I'd rather just pass that along to the end user if possible

success: false,
error:
'Error handling your request. Please make sure you are running the latest SDK version.',
})
})
}
20 changes: 18 additions & 2 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6158,6 +6158,13 @@
resolved "https://registry.yarnpkg.com/@types/http-errors/-/http-errors-2.0.1.tgz#20172f9578b225f6c7da63446f56d4ce108d5a65"
integrity sha512-/K3ds8TRAfBvi5vfjuz8y6+GiAYBZ0x4tXv1Av6CWBWn0IlADc+ZX9pMq7oU0fNQPnBwIZl3rmeLp6SBApbxSQ==

"@types/http-proxy@^1.17.11":
version "1.17.11"
resolved "https://registry.yarnpkg.com/@types/http-proxy/-/http-proxy-1.17.11.tgz#0ca21949a5588d55ac2b659b69035c84bd5da293"
integrity sha512-HC8G7c1WmaF2ekqpnFq626xd3Zz0uvaqFmBJNRZCGEZCXkvSdJoNFn/8Ygbd9fKNQj8UzLdCETaI0UWPAjK7IA==
dependencies:
"@types/node" "*"

"@types/humanize-duration@^3.18.0":
version "3.27.1"
resolved "https://registry.yarnpkg.com/@types/humanize-duration/-/humanize-duration-3.27.1.tgz#f14740d1f585a0a8e3f46359b62fda8b0eaa31e7"
Expand Down Expand Up @@ -12077,7 +12084,7 @@ [email protected]:
resolved "https://registry.yarnpkg.com/eventemitter3/-/eventemitter3-4.0.4.tgz#b5463ace635a083d018bdc7c917b4c5f10a85384"
integrity sha512-rlaVLnVxtxvoyLsQQFBx53YmXHDxRIzzTLbdfxqi4yocpSjAxXwkU0cScM5JgSKMqEhrZpnvQ2D9gjylR0AimQ==

eventemitter3@^4.0.4:
eventemitter3@^4.0.0, eventemitter3@^4.0.4:
version "4.0.7"
resolved "https://registry.yarnpkg.com/eventemitter3/-/eventemitter3-4.0.7.tgz#2de9b68f6528d5644ef5c59526a1b4a07306169f"
integrity sha512-8guHBZCwKnFhYdHr2ysuRWErTwhoN2X8XELRlrRwpmfeY2jjuUN4taQMsULKUVo1K4DvZl+0pgfyoysHxvmvEw==
Expand Down Expand Up @@ -13062,7 +13069,7 @@ [email protected]:
dependencies:
debug "=3.1.0"

follow-redirects@^1.12.1, follow-redirects@^1.14.0, follow-redirects@^1.14.7, follow-redirects@^1.15.0:
follow-redirects@^1.0.0, follow-redirects@^1.12.1, follow-redirects@^1.14.0, follow-redirects@^1.14.7, follow-redirects@^1.15.0:
version "1.15.2"
resolved "https://registry.yarnpkg.com/follow-redirects/-/follow-redirects-1.15.2.tgz#b460864144ba63f2681096f274c4e57026da2c13"
integrity sha512-VQLG33o04KaQ8uYi2tVNbdrWp1QWxNNea+nmIB4EVM28v0hmP17z7aG1+wAkNzVq4KeXTq3221ye5qTJP91JwA==
Expand Down Expand Up @@ -14822,6 +14829,15 @@ http-proxy-agent@^7.0.0:
agent-base "^7.1.0"
debug "^4.3.4"

http-proxy@^1.18.1:
version "1.18.1"
resolved "https://registry.yarnpkg.com/http-proxy/-/http-proxy-1.18.1.tgz#401541f0534884bbf95260334e72f88ee3976549"
integrity sha512-7mz/721AbnJwIVbnaSv1Cz3Am0ZLT/UBwkC92VlxhXv/k/BBQfM2fXElQNC27BVGr0uwUpplYPQM9LnaBMR5NQ==
dependencies:
eventemitter3 "^4.0.0"
follow-redirects "^1.0.0"
requires-port "^1.0.0"

http-response-object@^3.0.1:
version "3.0.2"
resolved "https://registry.yarnpkg.com/http-response-object/-/http-response-object-3.0.2.tgz#7f435bb210454e4360d074ef1f989d5ea8aa9810"
Expand Down
Loading