Skip to content
36 changes: 36 additions & 0 deletions spec/common/encoding.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import { expect } from 'chai';
import { convertInvoker } from '../../src/common/encoding';

describe('convertInvoker', () => {
it('should raise an error on mixing public and service accounts', () => {
expect(() => convertInvoker(['public', 'service-account@'])).to.throw;
});

it('should raise an error on mixing private and service accounts', () => {
expect(() => convertInvoker(['private', 'service-account@'])).to.throw;
});

it('should return the correct public invoker', () => {
const invoker = convertInvoker('public');

expect(invoker).to.deep.equal(['public']);
});

it('should return the correct private invoker', () => {
const invoker = convertInvoker('private');

expect(invoker).to.deep.equal(['private']);
});

it('should return the correct scalar invoker', () => {
const invoker = convertInvoker('service-account@');

expect(invoker).to.deep.equal(['service-account@']);
});

it('should return the correct array invoker', () => {
const invoker = convertInvoker(['service-account1@', 'service-account2@']);

expect(invoker).to.deep.equal(['service-account1@', 'service-account2@']);
});
});
5 changes: 5 additions & 0 deletions spec/v2/providers/https.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ describe('onRequest', () => {
{
...FULL_OPTIONS,
region: ['us-west1', 'us-central1'],
invoker: ['service-account1@', 'service-account2@'],
},
(req, res) => {
res.send(200);
Expand All @@ -101,6 +102,7 @@ describe('onRequest', () => {
allowInsecure: false,
},
regions: ['us-west1', 'us-central1'],
invoker: ['service-account1@', 'service-account2@'],
});
});

Expand All @@ -109,12 +111,14 @@ describe('onRequest', () => {
concurrency: 20,
region: 'europe-west1',
minInstances: 1,
invoker: 'public',
});

const result = https.onRequest(
{
region: ['us-west1', 'us-central1'],
minInstances: 3,
invoker: 'private',
},
(req, res) => {
res.send(200);
Expand All @@ -131,6 +135,7 @@ describe('onRequest', () => {
minInstances: 3,
regions: ['us-west1', 'us-central1'],
labels: {},
invoker: ['private'],
});
});

Expand Down
17 changes: 17 additions & 0 deletions src/common/encoding.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,3 +64,20 @@ export function serviceAccountFromShorthand(
);
}
}

export function convertInvoker(invoker: string | string[]): string[] {
if (typeof invoker === 'string') {
invoker = [invoker];
}

if (
invoker.length > 1 &&
invoker.find((inv) => inv === 'public' || inv === 'private')
) {
throw new Error(
"Invalid option for invoker. Cannot have 'public' or 'private' in an array of service accounts"
);
}

return invoker;
}
7 changes: 4 additions & 3 deletions src/v1/cloud-functions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import {
Duration,
durationFromSeconds,
serviceAccountFromShorthand,
convertInvoker,
} from '../common/encoding';

/** @hidden */
Expand Down Expand Up @@ -279,7 +280,7 @@ export interface TriggerAnnotated {
vpcConnectorEgressSettings?: string;
serviceAccountEmail?: string;
ingressSettings?: string;
invoker?: Invoker | Invoker[];
invoker?: Invoker[];
};
}

Expand Down Expand Up @@ -499,8 +500,7 @@ export function optionsToTrigger(options: DeploymentOptions) {
'ingressSettings',
'vpcConnectorEgressSettings',
'vpcConnector',
'labels',
'invoker'
'labels'
);
convertIfPresent(
trigger,
Expand Down Expand Up @@ -543,6 +543,7 @@ export function optionsToTrigger(options: DeploymentOptions) {
'serviceAccount',
serviceAccountFromShorthand
);
convertIfPresent(trigger, options, 'invoker', 'invoker', convertInvoker);

return trigger;
}
1 change: 1 addition & 0 deletions src/v2/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export interface TriggerAnnotation {
vpcConnectorEgressSettings?: string;
serviceAccountEmail?: string;
ingressSettings?: string;
invoker?: string[];

// TODO: schedule
}
Expand Down
12 changes: 12 additions & 0 deletions src/v2/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import {
durationFromSeconds,
serviceAccountFromShorthand,
convertInvoker,
} from '../common/encoding';
import { convertIfPresent, copyIfPresent } from '../common/encoding';
import * as logger from '../logger';
Expand Down Expand Up @@ -110,6 +111,11 @@ export const SUPPORTED_INGRESS_SETTINGS = [

export type IngressSetting = typeof SUPPORTED_INGRESS_SETTINGS[number];

/**
* Invoker access control type for https functions.
*/
export type Invoker = 'public' | 'private' | string;

/**
* GlobalOptions are options that can be set across an entire project.
* These options are common to HTTPS and Event handling functions.
Expand Down Expand Up @@ -182,6 +188,11 @@ export interface GlobalOptions {
* User labels to set on the function.
*/
labels?: Record<string, string>;

/**
* Invoker to set access control on https functions.
*/
invoker?: Invoker | Invoker[];
Copy link
Member

Choose a reason for hiding this comment

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

Given your new defensive programming stance I agree with Daniel now that we can/should change our typing.

invoker?: 'public' | 'private' | string | string[]; 

This will make sure code complete doesn't suggest "public" or "private" when the customer enters an array.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed the type definition

}

let globalOptions: GlobalOptions | undefined;
Expand Down Expand Up @@ -270,6 +281,7 @@ export function optionsToTriggerAnnotations(
return retry ? { retry: true } : null;
}
);
convertIfPresent(annotation, opts, 'invoker', 'invoker', convertInvoker);

return annotation;
}
Expand Down