amplify-education / serverless-vpc-discovery

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

Support wildcard lookup #29

Closed jalaziz closed 3 years ago

jalaziz commented 4 years ago

When looking up subnet IDs, there is a step to "[Compare] the valid subnets with ones given to find invalid subnet names". This validation prevents using wildcards in the subnet names.

For example, instead of listing all subnets explicitly, I'd like to have a configuration that looks like:

vpc:
  vpcName: dev
  subnetNames:
    - 'dev-private-*'
RLRabinowitz commented 3 years ago

We're having the same issue. We use our own fork from a way earlier stage of this repo. For us the wildcard lookup accidentally works. When we tried to go back to the base, instead of our fork, we noticed that wildcards are not supported

RLRabinowitz commented 3 years ago

@rddimon Is there a reason why subnetNames and securityGroupNames are filtered out manually? They should be already filtered out correctly by the Filter when using AWS API

https://github.com/amplify-education/serverless-vpc-discovery/blob/ddc4a624abc9d77c3340c010d1a6cda27eba0bca/src/aws/ec2-wrapper.ts#L86-L93

https://github.com/amplify-education/serverless-vpc-discovery/blob/ddc4a624abc9d77c3340c010d1a6cda27eba0bca/src/aws/ec2-wrapper.ts#L134-L140

RLRabinowitz commented 3 years ago

If the reason for this is that we want to make sure we have subnets matching each line in the configuration, then this logic needs to be smarter and support wildcards

rddimon commented 3 years ago

Hi @RLRabinowitz Thank you for your question.

This logic for excluding the case when you specify a subnet name or security group name but there are no items found So we need to check if names are correct or remove the not existing name

RLRabinowitz commented 3 years ago

Got it.

So in order to support wildcards, the pieces of code I referred to above should lookup with a regex matching AWS wildcard logic, instead of matching of exact name.

I might create a PR for this

RLRabinowitz commented 3 years ago

I've started working on a PR, and when I worked on the tests I noticed that there are issues with them. I've opened a PR to fix the tests as a prerequisite to this fix