search

aws-quickstart / quickstart-cisco-meraki-sd-wan-vmx

AWS Quick Start Team
Apache License 2.0
11 stars 16 forks source link

Develop #1

Closed simarbir closed 2 years ago

simarbir commented 3 years ago

Description of changes: Still a work in progress, creating a PR for an initial review and feedback around the template structure.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

mufaddalq commented 3 years ago

Some comments on the PR:

  1. Address cfn-lint errors: AWS Quick Start uses cfn-lint (OSS) project for linting checks. Here is the link to it https://github.com/aws-cloudformation/cfn-lint. To add QS specific lint rules, you can clone https://github.com/aws-quickstart/qs-cfn-lint-rules repo and in .cfnlintrc file add qs lint rules using the --append-rules command.
  2. In the meraki_vmx_deployment.yaml, how does a user obtain the token? Is this the only information needed by vMX to register itself with the Meraki dashboard?
  3. In meraki_tgw_deployment.yaml, I didn’t see any definitions for route table, associations and propagations. Is the intention to use the default route table? As configured currently, DefaultRouteTablePropagation is enabled (default) but DefaultRouteTableAssociation is disabled which indicates a faulty configuration. As a best practice, it may make sense to have a dedicated route table for SD-WAN that is separate from default. Perhaps the intention is for all routing to be configured via the lambda. If so, please ignore this comment.
simarbir commented 3 years ago

Some comments on the PR:

  1. Address cfn-lint errors: AWS Quick Start uses cfn-lint (OSS) project for linting checks. Here is the link to it https://github.com/aws-cloudformation/cfn-lint. To add QS specific lint rules, you can clone https://github.com/aws-quickstart/qs-cfn-lint-rules repo and in .cfnlintrc file add qs lint rules using the --append-rules command.

Thanks, for sharing the links. Addressed all the previous lint errors. Working on addressing the new ones related to the copy lambda template and the lambda scripts.

  1. In the meraki_vmx_deployment.yaml, how does a user obtain the token? Is this the only information needed by vMX to register itself with the Meraki dashboard?

The user obtains the token from the Meraki Dashboard. Also, yes that's the only information needed by the vMX to onboard to the Meraki Dashboard.

  1. In meraki_tgw_deployment.yaml, I didn’t see any definitions for route table, associations and propagations. Is the intention to use the default route table? As configured currently, DefaultRouteTablePropagation is enabled (default) but DefaultRouteTableAssociation is disabled which indicates a faulty configuration. As a best practice, it may make sense to have a dedicated route table for SD-WAN that is separate from default. Perhaps the intention is for all routing to be configured via the lambda. If so, please ignore this comment.

Agreed on having a dedicated rt for SD-WAN, I've added the rout table definitions. The updates to the route table would be done by the lambda.