amplify-education / serverless-vpc-discovery

Serverless plugin for discovering VPC / Subnet / Security Group configuration by name.
MIT License
38 stars 15 forks source link

subnetNames and securityGroupNames both required (documentation) #35

Closed kjudson closed 3 years ago

kjudson commented 3 years ago

In the documentation, there's this:

vpcDiscovery:
    vpcName: '${opt:env}'
    subnetNames: # optional if securityGroupNames are specified
      - '${opt:env}_NAME OF SUBNET'
    securityGroupNames: # optional if subnetNames are specified
      - '${opt:env}_NAME OF SECURITY GROUP'

which suggests that you don't need to provide both securityGroupNames and subnetNames.

However, this doesn't seem to work for AWS. There's this condition in the serverless aws package:

      if (
        !functionResource.Properties.VpcConfig.SecurityGroupIds ||
        !functionResource.Properties.VpcConfig.SubnetIds
      ) {
        delete functionResource.Properties.VpcConfig;
      }

(https://github.com/serverless/serverless/blob/306656e97d6782e1a13bed60ec0cec8b3e521e10/lib/plugins/aws/package/compile/functions/index.js#L323)

If you don't specify both properties, then the vpcConfig section isn't created.

I'm not sure if this is the case with other providers.

rddimon commented 3 years ago

Hi @kjudson Thank you for your issue

This is specific plugin behavior and it's optional for the plugin

You can specify a mixed config for example:

provider:
  name: aws
  vpc:
    securityGroupIds:
      - securityGroupId1

vpcDiscovery:
    vpcName: '${opt:env}'
    subnetNames:
      - '${opt:env}_NAME OF SUBNET'
kjudson commented 3 years ago

Thanks @rddimon, that makes sense. It's optional in the sense that the plugin doesn't enforce both. But AWS does still require both. I'll close the issue.