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.

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

fix(thrift-server-hapi): Allow passing auth strategy to hapi plugin #55

Merged
merged 3 commits into from
Mar 13, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions packages/thrift-client/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ Zipkin tracing is added to your client through middleware.
```typescript
import {
createHttpClient,
zipkinClientMiddleware,
ZipkinTracingThriftClient,
} from '@creditkaram/thrift-client'

import { Calculator } from './codegen/calculator'
Expand All @@ -333,7 +333,7 @@ const thriftClient: Calculator.Client<ThriftContext<CoreOptions>> =
createHttpClient(Calculator.Client, {
hostName: 'localhost',
port: 8080,
register: [ zipkinClientMiddleware({
register: [ ZipkinTracingThriftClient({
localServiceName: 'calculator-client',
remoteServiceName: 'calculator-service',
endpoint: 'http://localhost:9411/api/v1/spans',
Expand Down
2 changes: 1 addition & 1 deletion packages/thrift-client/src/main/observability/index.ts
Original file line number Diff line number Diff line change
@@ -1 +1 @@
export { zipkinClientMiddleware } from './zipkin'
export { ZipkinTracingThriftClient } from './zipkin'
2 changes: 1 addition & 1 deletion packages/thrift-client/src/main/observability/zipkin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ function readRequestContext(context: ThriftContext<CoreOptions>, tracer: Tracer)
}
}

export function zipkinClientMiddleware({
export function ZipkinTracingThriftClient({
localServiceName,
remoteServiceName,
debug = false,
Expand Down
4 changes: 2 additions & 2 deletions packages/thrift-client/src/tests/add-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { Int64 } from '@creditkarma/thrift-server-core'

import {
createThriftServer,
zipkinPlugin,
ZipkinTracingHapi,
} from '@creditkarma/thrift-server-hapi'

import * as Hapi from 'hapi'
Expand Down Expand Up @@ -47,7 +47,7 @@ export function createServer(sampleRate: number = 0): Hapi.Server {

if (sampleRate > 0) {
server.register(
zipkinPlugin({
ZipkinTracingHapi({
localServiceName: 'add-service',
endpoint: 'http://localhost:9411/api/v1/spans',
sampleRate,
Expand Down
8 changes: 4 additions & 4 deletions packages/thrift-client/src/tests/calculator-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { Int64 } from '@creditkarma/thrift-server-core'

import {
createThriftServer,
zipkinPlugin,
ZipkinTracingHapi,
} from '@creditkarma/thrift-server-hapi'

import * as Hapi from 'hapi'
Expand All @@ -29,7 +29,7 @@ import {
import {
createHttpClient,
ThriftContext,
zipkinClientMiddleware,
ZipkinTracingThriftClient,
} from '../main/index'

export function createServer(sampleRate: number = 0): Hapi.Server {
Expand All @@ -40,7 +40,7 @@ export function createServer(sampleRate: number = 0): Hapi.Server {
port: ADD_SERVER_CONFIG.port,
register: (
(sampleRate > 0) ?
[ zipkinClientMiddleware({
[ ZipkinTracingThriftClient({
localServiceName: 'calculator-service',
remoteServiceName: 'add-service',
endpoint: 'http://localhost:9411/api/v1/spans',
Expand Down Expand Up @@ -158,7 +158,7 @@ export function createServer(sampleRate: number = 0): Hapi.Server {

if (sampleRate > 0) {
server.register(
zipkinPlugin({
ZipkinTracingHapi({
localServiceName: 'calculator-service',
endpoint: 'http://localhost:9411/api/v1/spans',
sampleRate,
Expand Down
8 changes: 4 additions & 4 deletions packages/thrift-client/src/tests/client.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { zipkinMiddleware } from '@creditkarma/thrift-server-express'
import { ZipkinTracingExpress } from '@creditkarma/thrift-server-express'

import {
createHttpClient,
ThriftContext,
zipkinClientMiddleware,
ZipkinTracingThriftClient,
} from '../main/'

import * as express from 'express'
Expand All @@ -26,7 +26,7 @@ export function createClientServer(sampleRate: number = 0): Promise<net.Server>
const app = express()

if (sampleRate > 0) {
app.use(zipkinMiddleware({
app.use(ZipkinTracingExpress({
localServiceName: 'calculator-client',
endpoint: 'http://localhost:9411/api/v1/spans',
sampleRate,
Expand All @@ -40,7 +40,7 @@ export function createClientServer(sampleRate: number = 0): Promise<net.Server>
port: CALC_SERVER_CONFIG.port,
register: (
(sampleRate > 0) ?
[ zipkinClientMiddleware({
[ ZipkinTracingThriftClient({
localServiceName: 'calculator-client',
remoteServiceName: 'calculator-service',
endpoint: 'http://localhost:9411/api/v1/spans',
Expand Down
8 changes: 4 additions & 4 deletions packages/thrift-server-express/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ To get things working you need to register the middleware as you would any other
```typescript
import * as bodyParser from 'body-parser'
import * as express from 'express'
import { thriftServerExpress } from '../main'
import { ThriftServerExpress } from '../main'

import {
Calculator,
Expand Down Expand Up @@ -77,7 +77,7 @@ const serviceHandlers: Calculator.IHandler<express.Request> = {
app.use(
'/thrift',
bodyParser.raw(),
thriftServerExpress<Calculator.Processor>({
ThriftServerExpress<Calculator.Processor>({
serviceName: 'calculator-service',
handler: new Calculator.Processor(serviceHandlers),
}),
Expand Down Expand Up @@ -145,7 +145,7 @@ import * as express from 'express'

import {
createThriftServer,
zipkinMiddleware,
ZipkinTracingExpress,
} from '@creditkarma/thrift-server-express'

import { Calculator } from './codegen/calculator'
Expand All @@ -168,7 +168,7 @@ const app: express.Application = createThriftServer<Calculator.Processor>({
}
})

app.use(zipkinMiddleware({
app.use(ZipkinTracingExpress({
localServiceName: SERVICE_NAME,
endpoint: 'http://localhost:9411/api/v1/spans',
sampleRate: 0.1,
Expand Down
28 changes: 6 additions & 22 deletions packages/thrift-server-express/src/main/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,6 @@ import * as express from 'express'

export * from './observability'

export type ExpressOptionsFunction<TProcessor> =
(req?: express.Request, res?: express.Response) => IThriftServerOptions<TProcessor>

export interface ICreateExpressServerOptions<TProcessor> {
path?: string
thriftOptions: IThriftServerOptions<TProcessor>
Expand All @@ -29,35 +26,22 @@ export function createThriftServer<TProcessor extends IThriftProcessor<express.R
app.use(
options.path || '/thrift',
bodyParser.raw(),
thriftServerExpress<TProcessor>(options.thriftOptions),
ThriftServerExpress<TProcessor>(options.thriftOptions),
)

return app
}

function getPluginOptions<TProcessor>(
request: express.Request,
response: express.Response,
options: IThriftServerOptions<TProcessor> | ExpressOptionsFunction<TProcessor>,
): IThriftServerOptions<TProcessor> {
if (typeof options === 'function') {
return options(request, response)
} else {
return options
}
}

export function thriftServerExpress<TProcessor extends IThriftProcessor<express.Request>>(
pluginOptions: IThriftServerOptions<TProcessor> | ExpressOptionsFunction<TProcessor>,
export function ThriftServerExpress<TProcessor extends IThriftProcessor<express.Request>>(
pluginOptions: IThriftServerOptions<TProcessor>,
): express.RequestHandler {
return (request: express.Request, response: express.Response, next: express.NextFunction): void => {
const options = getPluginOptions(request, response, pluginOptions)
const Transport: ITransportConstructor = getTransport(options.transport)
const Protocol: IProtocolConstructor = getProtocol(options.protocol)
const Transport: ITransportConstructor = getTransport(pluginOptions.transport)
const Protocol: IProtocolConstructor = getProtocol(pluginOptions.protocol)

try {
process({
processor: options.handler,
processor: pluginOptions.handler,
buffer: request.body,
Transport,
Protocol,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ function formatRequestUrl(req: express.Request): string {
})
}

export function zipkinMiddleware({
export function ZipkinTracingExpress({
localServiceName,
port = 0,
debug = false,
Expand Down
11 changes: 6 additions & 5 deletions packages/thrift-server-hapi/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,11 @@ npm install --save @creditkarma/thrift-server-hapi

To get things working you need to register the Thrift plugin and define handlers for your service methods.

The `thriftServerHapi` function creates a Hapi route at the given path on which to serve this Thrift service.
The `ThriftServerHapi` function creates a Hapi route at the given path on which to serve this Thrift service.

```typescript
import * as Hapi from 'hapi'
import { thriftServerHapi } from '@creditkarma/thrift-server-hapi'
import { ThriftServerHapi } from '@creditkarma/thrift-server-hapi'
import { Calculator } from './codegen/calculator'

const PORT: number = 8080
Expand Down Expand Up @@ -87,7 +87,7 @@ const processor: Calculator.Processor<Hapi.Request> = new Calculator.Processor(s
* option is the path to attache the route handler to and the handler is the
* Thrift service processor instance.
*/
server.register(thriftServerHapi<Calculator.Processor>({
server.register(ThriftServerHapi<Calculator.Processor>({
path: '/thrift',
thriftOptions: {
serviceName: 'calculator-service',
Expand All @@ -113,6 +113,7 @@ server.start((err) => {
#### Options

* path (required): The path on which to server your Thrift service. Defaults to '/thrift'.
* auth (optional): Authentication strategy for Thrift route as defined by [Hapi](https://hapijs.com/api/16.6.3#serverauthstrategyname-scheme-mode-options).
* thriftOptions.serviceName (required): The name of your service. Used for logging and tracing.
* thriftOptions.handler (required): The service Processor instance to handle service method calls.
* thriftOptions.transport (optional): The kind of Thrift transport to use. Only 'buffered' is currently supported.
Expand Down Expand Up @@ -169,7 +170,7 @@ import * as hapi from 'hapi'

import {
createThriftServer,
zipkinPlugin,
ZipkinTracingHapi,
} from '@creditkarma/thrift-server-hapi'

import { Calculator } from './codegen/calculator'
Expand All @@ -195,7 +196,7 @@ const server: Hapi.Server = createThriftServer({
})

server.register(
zipkinPlugin({
ZipkinTracingHapi({
localServiceName: SERVICE_NAME,
endpoint: 'http://localhost:9411/api/v1/spans',
sampleRate: 0.1
Expand Down
37 changes: 7 additions & 30 deletions packages/thrift-server-hapi/src/main/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,24 +13,14 @@ import {

export * from './observability'

export type HapiOptionsFunction<TProcessor> = (
req?: Hapi.Request,
) => IThriftServerOptions<TProcessor>

export interface IHandlerOptions<TProcessor> {
service: TProcessor
}

export type HapiThriftOptionsFunction<TProcessor> = (
request: Hapi.Request,
reply: Hapi.ReplyNoContinue,
) => IThriftServerOptions<TProcessor>

export interface IHapiPluginOptions<TProcessor> {
path?: string
thriftOptions:
| IThriftServerOptions<TProcessor>
| HapiOptionsFunction<TProcessor>
auth?: false | string | Hapi.AuthOptions
thriftOptions: IThriftServerOptions<TProcessor>
}

export interface ICreateHapiServerOptions<TProcessor>
Expand Down Expand Up @@ -66,7 +56,7 @@ export function createThriftServer<TProcessor extends IThriftProcessor<Hapi.Requ
}),
(err: any) => {
if (err) {
console.log('error 2: ', err)
console.log('error: ', err)
throw err
}
},
Expand All @@ -75,17 +65,6 @@ export function createThriftServer<TProcessor extends IThriftProcessor<Hapi.Requ
return server
}

function getPluginOptions<TProcessor>(
request: Hapi.Request,
options: IHapiPluginOptions<TProcessor>,
): IThriftServerOptions<TProcessor> {
if (typeof options.thriftOptions === 'function') {
return options.thriftOptions(request)
} else {
return options.thriftOptions
}
}

/**
* Create the thrift plugin for Hapi
*
Expand All @@ -103,8 +82,7 @@ export function thirftServerHapi<TProcessor extends IThriftProcessor<Hapi.Reques
request: Hapi.Request,
reply: Hapi.ReplyNoContinue,
) => {
const options: IThriftServerOptions<TProcessor> =
getPluginOptions(request, pluginOptions)
const options: IThriftServerOptions<TProcessor> = pluginOptions.thriftOptions

const Transport: ITransportConstructor = getTransport(
options.transport,
Expand Down Expand Up @@ -142,6 +120,7 @@ export function thirftServerHapi<TProcessor extends IThriftProcessor<Hapi.Reques
payload: {
parse: false,
},
auth: pluginOptions.auth,
},
})

Expand All @@ -150,10 +129,8 @@ export function thirftServerHapi<TProcessor extends IThriftProcessor<Hapi.Reques
}

hapiThriftPlugin.register.attributes = {
pkg: {
name: 'thrift-server-hapi',
version: require('../../package.json').version,
},
name: require('../../package.json').name,
version: require('../../package.json').version,
}

return hapiThriftPlugin
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ function readStatusCode({ response }: Hapi.Request): number {
}
}

export function zipkinPlugin({
export function ZipkinTracingHapi({
localServiceName,
port = 0,
debug = false,
Expand Down