Skip to content

Commit

Permalink
Cleanup extraneous console messsages and config parsing fix (#23)
Browse files Browse the repository at this point in the history
- Cleanup extraneous console messsages
- Fix for parser seeing VPC Based Providers as invalid vpcNames: in routing
- Added GitHub action test for each configuration file to assure it continues to parse
- Updating Tests to run against Node 18 and 20 instead of 16 and 18.  16 is now considered deprecated for the CDK
  • Loading branch information
apmclean committed Sep 19, 2023
1 parent f2303dd commit eba5533
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 14 deletions.
14 changes: 12 additions & 2 deletions .github/workflows/node.js.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ jobs:

strategy:
matrix:
node-version: [16.x, 18.x]
node-version: [18.x, 20.x]
# See supported Node.js release schedule at https://nodejs.org/en/about/releases/

steps:
Expand All @@ -29,5 +29,15 @@ jobs:
- run: npx prettier --check **/**/*.ts
- run: npm ci
- run: npm run build --if-present
# Run our test cases
- run: npm test

# Synthesize each of our sample configuration files to assure they continue to build templates
- run: npm install -g cdk
- run: cdk ls -c config=sample-central-egress.vpcBuilder.yaml
- run: cdk ls -c config=sample-central-egress-inspected.vpcBuilder.yaml
- run: cdk ls -c config=sample-central-ingress.vpcBuilder.yaml
- run: cdk ls -c config=sample-central-ingress-inspected.vpcBuilder.yaml
- run: cdk ls -c config=sample-complex.vpcBuilder.yaml
- run: cdk ls -c config=sample-firewall-blog.vpcBuilder.yaml
- run: cdk ls -c config=sample-vpc-endpoints.vpcBuilder.yaml
- run: cdk ls -c config=sample-vpn-onprem.vpcBuilder.yaml
1 change: 1 addition & 0 deletions config/sample-firewall-blog.vpcBuilder.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ vpcs:
SpokeVpc:
style: workloadIsolated
vpcCidr: 10.3.0.0/16
providerInternet: centralEgress
subnets:
workloadSubnet:
cidrMask: 24
Expand Down
44 changes: 36 additions & 8 deletions lib/config/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -219,20 +219,14 @@ export class ConfigParser {
)) {
const configStanza = this.configRaw.transitGateways[transitGatewayName];
if (configStanza.blackholeRoutes) {
console.log(`Black Hole Routes found, evaluating`);
for (const route of configStanza.blackholeRoutes) {
route.blackholeCidrs.forEach(
(blackholeCidr: string, index: number) => {
if (this.blackholeIsCidr(blackholeCidr)) {
console.log(`${blackholeCidr} is considered a blackholecidr`);
this.verifyCidr(blackholeCidr, false);
} else {
console.log(`${blackholeCidr} is not a valid cidr`);
// Value provided is not CIDR formatted, see if it matches a VPC
if (this.vpcNameExists(blackholeCidr)) {
console.log(
`${blackholeCidr} is considered a valid VPC Name`,
);
// We will substitute the value of our VPCs CIDR address here since the rest of our code
// Expects our value to be a CIDR format
route.blackholeCidrs[index] =
Expand Down Expand Up @@ -456,6 +450,37 @@ export class ConfigParser {
return false;
}

// We allow routing configuration for providers that are VPC Based (currently all of them are).
// Let's not assume that will always be the case. This is used by our route sanity checks to assure routes
// under `vpcName:` are actually VPCs so our routing logic will function
providerNameIsVpcStyle(providerCheckName: string): boolean {
const vpcBasedProviderStyles = [
"natEgress",
"serviceInterfaceEndpoint",
"route53ResolverEndpoint",
"awsNetworkFirewall",
];
for (const providerType of ["endpoints", "internet", "firewall"]) {
if (this.configRaw.hasOwnProperty("providers")) {
if (this.configRaw.providers.hasOwnProperty(providerType)) {
for (const providerName of Object.keys(
this.configRaw.providers[providerType],
)) {
const configStanza =
this.configRaw.providers[providerType][providerName];
if (providerName == providerCheckName) {
// Assure our type matches what we consider VPC types
if (vpcBasedProviderStyles.includes(configStanza.style)) {
return true;
}
}
}
}
}
}
return false;
}

allProviderNames(): Array<string> {
const providerNames: Array<string> = [];
for (const providerType of ["endpoints", "internet", "firewall"]) {
Expand Down Expand Up @@ -741,8 +766,11 @@ export class ConfigParser {
routeTypes.forEach((routeType) => {
if (configStanza[routeType]) {
for (const route of configStanza[routeType]) {
// vpcName points to a vpc
if (!this.vpcNameExists(route.vpcName)) {
// Assure vpcName points to a vpc: or provider: that is implemented in a VPC Style
if (
!this.vpcNameExists(route.vpcName) &&
!this.providerNameIsVpcStyle(route.vpcName)
) {
if (allNames.includes(route.vpcName)) {
// If vpcName points to a non-vpc provide a more useful message
throw new Error(
Expand Down
1 change: 0 additions & 1 deletion test/config-parser.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -561,7 +561,6 @@ test("RouteNamingSanity", () => {
]
}
// vpcName is not present for any resource
console.log(JSON.stringify(configContents, null, 2))
let config = new ConfigParser({ configContents: configContents });
expect(() => config.parse()).toThrow(
`A ${routeHuman[routeType]} was specified for testing2 - vpc with that name could not be found`
Expand Down
4 changes: 1 addition & 3 deletions test/transit-gateway-routes-stack-tgw-routes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,7 @@ test("TgwRouteDynamic", () => {

const template = Template.fromStack(routeStack);
const templateJson = template.toJSON();
// console.log(templateJson)
// Confirm we get a an association both ways
// Confirm we get an association both ways
const firstRouteId =
`TGWPropRoute${firstVpc.name}to${secondVpc.name}`.replace(/-/g, "");
expect(templateJson.Resources).toMatchObject({
Expand Down Expand Up @@ -128,7 +127,6 @@ test("TgwRouteDynamicToDxGw", () => {

const template = Template.fromStack(routeStack);
const templateJson = template.toJSON();
// console.log(JSON.stringify(templateJson, null, 2))
// Confirm we get an association both ways
const firstRouteId =
`TGWPropRoute${firstVpc.name}to${dxgw.name}`.replace(/-/g, "");
Expand Down

0 comments on commit eba5533

Please sign in to comment.