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

feat: Support provider.vpcEgress configuration #271

Merged
merged 12 commits into from
Sep 3, 2021
Merged

feat: Support provider.vpcEgress configuration #271

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
6a792f3
Updates to allow VPC Connector configuration
fcr1193 Aug 26, 2021
db39f69
validation update
fcr1193 Aug 27, 2021
5971775
update
fcr1193 Aug 27, 2021
1909cf2
update
fcr1193 Aug 27, 2021
b82abd9
update
fcr1193 Aug 27, 2021
33bbf4d
variable update
fcr1193 Aug 30, 2021
9c4e18d
test added
fcr1193 Aug 30, 2021
b41aa13
logic changes to support vpcEgress on provider level
fcr1193 Aug 31, 2021
98e74a9
updates to override provider level setting
fcr1193 Sep 1, 2021
4282079
Test to cover provider level and setting override added
fcr1193 Sep 1, 2021
33bf794
variable addded
fcr1193 Sep 3, 2021
bc04e10
test removed
fcr1193 Sep 3, 2021
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
49 changes: 41 additions & 8 deletions package/lib/compileFunctions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ module.exports = {
validateHandlerProperty(funcObject, functionName);
validateEventsProperty(funcObject, functionName);
validateVpcConnectorProperty(funcObject, functionName);
validateVpcConnectorEgressProperty(funcObject, functionName);

const funcTemplate = getFunctionTemplate(
funcObject,
Expand Down Expand Up @@ -57,6 +58,17 @@ module.exports = {
});
}

if (funcObject.vpcEgress) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we applying this only if vpcEgress is set directly on function? What is the purpose of provider level setting then? It will never be effective as it will be always overriden by value set on function or not applied at all

Copy link
Contributor Author

@fcr1193 fcr1193 Aug 30, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Hey! sorry, I'm not following you here. What do you mean that the value will be overwritten?

I have the following configuration (https://i.imgur.com/7kENrcp.png) in my serverless.yml file. I think vpcEgress is configured at the function level.

I deployed using this fork and got the following result:

https://i.imgur.com/tb748wY.png

I wait your answer, thank you!

Copy link
Contributor

@pgrzesik pgrzesik Aug 31, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

What I mean here is that while we support vpcEgress setting on provider level, it will never take effect because the logic is applied only when the setting is applied directly on function. I would assume that if it's only set on provider level, then it should be applied to all functions in service, correct?

In the provided example you set the value on function level and that case is covered well, but if you'd like to apply it across your service and set it on provider.vpcEgress, then it just does not work at all.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey! i think i got it, i just applied the change to make it through the provider level.
do you think it's ok?
thanks!

let egress = _.get(funcObject, 'vpcEgress') || _.get(this, 'serverless.service.provider.vpcEgress');
fcr1193 marked this conversation as resolved.
Show resolved Hide resolved
egress = egress.toUpperCase();
if (egress === 'ALL') egress = 'ALL_TRAFFIC';
if (egress === 'PRIVATE') egress = 'PRIVATE_RANGES_ONLY';
_.assign(funcTemplate.properties, {
vpcConnectorEgressSettings: egress,
});
}


if (funcObject.maxInstances) {
funcTemplate.properties.maxInstances = funcObject.maxInstances;
}
Expand Down Expand Up @@ -116,18 +128,39 @@ const validateHandlerProperty = (funcObject, functionName) => {

const validateVpcConnectorProperty = (funcObject, functionName) => {
if (funcObject.vpc && typeof funcObject.vpc === 'string') {
const vpcNamePattern = /projects\/[\s\S]*\/locations\/[\s\S]*\/connectors\/[\s\S]*/i;
if (!vpcNamePattern.test(funcObject.vpc)) {
const errorMessage = [
`The function "${functionName}" has invalid vpc connection name`,
' VPC Connector name should follow projects/{project_id}/locations/{region}/connectors/{connector_name}',
' Please check the docs for more info.',
].join('');
throw new Error(errorMessage);
// vpcConnector argument can be one of two possible formats as described here:
// https://cloud.google.com/functions/docs/reference/rest/v1/projects.locations.functions#resource:-cloudfunction
if (funcObject.vpc.indexOf('/') > -1) {
const vpcNamePattern = /projects\/[\s\S]*\/locations\/[\s\S]*\/connectors\/[\s\S]*/i;
if (!vpcNamePattern.test(funcObject.vpc)) {
const errorMessage = [
`The function "${functionName}" has invalid vpc connection name`,
' VPC Connector name should follow projects/{project_id}/locations/{region}/connectors/{connector_name}',
' or just {connector_name} if within the same project.',
' Please check the docs for more info at ',
' https://cloud.google.com/functions/docs/reference/rest/v1/projects.locations.functions#resource:-cloudfunction',
].join('');
throw new Error(errorMessage);
}
}
}
};



const validateVpcConnectorEgressProperty = (funcObject, functionName) => {
if (funcObject.vpcEgress && typeof funcObject.vpcEgress !== 'string') {
const errorMessage = [
`The function "${functionName}" has invalid vpc connection name`,
' VPC Connector Egress Setting be either ALL_TRAFFIC or PRIVATE_RANGES_ONLY. ',
' You may shorten these to ALL or PRIVATE optionally.',
' Please check the docs for more info at',
' https://cloud.google.com/functions/docs/reference/rest/v1/projects.locations.functions#resource:-cloudfunction',
].join('');
throw new Error(errorMessage);
}
};

const getFunctionTemplate = (funcObject, projectName, region, sourceArchiveUrl) => {
//eslint-disable-line
return {
Expand Down
10 changes: 10 additions & 0 deletions provider/googleProvider.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,14 @@ class GoogleProvider {
},
additionalProperties: false,
},
cloudFunctionVpcEgress: {
enum: [
'ALL',
'ALL_TRAFFIC',
'PRIVATE',
'PRIVATE_RANGES_ONLY'
],
},
resourceManagerLabels: {
type: 'object',
propertyNames: {
Expand All @@ -111,6 +119,7 @@ class GoogleProvider {
timeout: { type: 'string' }, // Can be overridden by function configuration
environment: { $ref: '#/definitions/cloudFunctionEnvironmentVariables' }, // Can be overridden by function configuration
vpc: { type: 'string' }, // Can be overridden by function configuration
vpcEgress: { $ref: '#/definitions/cloudFunctionVpcEgress' }, // Can be overridden by function configuration
labels: { $ref: '#/definitions/resourceManagerLabels' }, // Can be overridden by function configuration
},
},
Expand All @@ -123,6 +132,7 @@ class GoogleProvider {
timeout: { type: 'string' }, // Override provider configuration
environment: { $ref: '#/definitions/cloudFunctionEnvironmentVariables' }, // Override provider configuration
vpc: { type: 'string' }, // Override provider configuration
vpcEgress: {$ref: '#/definitions/cloudFunctionVpcEgress' }, // Can be overridden by function configuration
labels: { $ref: '#/definitions/resourceManagerLabels' }, // Override provider configuration
},
},
Expand Down