awslabs / landing-zone-accelerator-on-aws

Deploy a multi-account cloud foundation to support highly-regulated workloads and complex compliance requirements.
https://aws.amazon.com/solutions/implementations/landing-zone-accelerator-on-aws/
Apache License 2.0
557 stars 443 forks source link

GlobalConfig: BudgetReportConfig + SNS Topics- inconsistent documentation / functionality #216

Open richardkeit opened 1 year ago

richardkeit commented 1 year ago

Describe the bug The BudgetReportConfig object has an attribute subscriptionType, which states :

The type of notification that AWS sends to a subscriber. An enum value that specifies the target subscription type either EMAIL or SNS

While the config validator says the subscription will fail validation: https://github.com/awslabs/landing-zone-accelerator-on-aws/blob/cdfa741d83d02967cf07191d809c55422f35855e/source/packages/@aws-accelerator/config/validator/global-config-validator.ts#L184-L196

To Reproduce global-config.yaml

snsTopics:
  deploymentTargets:
    organizationalUnits:
      - Root
  topics:
    - name: Security
      emailAddresses:
        - richard.keit+l@my-domain.com.au

  budgets:
    - deploymentTargets:
        accounts:
          - Management
      name: accel-budget-management-account
      timeUnit: MONTHLY
      .... # removed for brevity 
      notifications:
        - type: ACTUAL
          thresholdType: PERCENTAGE
          threshold: 100
          comparisonOperator: GREATER_THAN
          subscriptionType: SNS
          address: Security     

Expected behavior

reports.budgets clarification: Ability to publish budget alerts to SNS topics OR corrected documentation

snsTopics clarification Further clarity on how the global-config: snsTopics is leveraged. https://docs.aws.amazon.com/solutions/latest/landing-zone-accelerator-on-aws/amazon-sns-topics.html does not explain this attribute in detail:

Please complete the following information about the solution:

Screenshots

❯ yarn validate-config /Users/richardkeit/git/versent/my-lza

yarn run v1.22.19
$ ts-node $PWD/packages/@aws-accelerator/accelerator/lib/config-validator.ts /Users/richardkeit/git/my-lza
2023-07-21 16:37:36.530 | info | config-validator | Config source directory -  /Users/richardkeit/git/my-lza
2023-07-21 16:37:36.541 | info | accounts-config-validator | accounts-config.yaml file validation started
2023-07-21 16:37:36.541 | info | global-config-validator | global-config.yaml file validation started
2023-07-21 16:37:36.542 | info | global-config-validator | email count: 1
2023-07-21 16:37:36.542 | info | iam-config-validator | iam-config.yaml file validation started
2023-07-21 16:37:36.543 | info | network-config-validator | network-config.yaml file validation started
2023-07-21 16:37:36.545 | info | organization-config-validator | organization-config.yaml file validation started
2023-07-21 16:37:36.546 | info | security-config-validator | security-config.yaml file validation started
2023-07-21 16:37:36.546 | warn | config-validator | Config file validation failed !!!
2023-07-21 16:37:36.552 | warn | config-validator | global-config.yaml has 1 issues:
Invalid report notification email Security.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

Additional context Add any other context about the problem here.

padebnat commented 1 year ago

Hello @richardkeit,

Thank you for reaching out to the Landing Zone Accelerator team. I wanted to let you know that we have added an item in our backlog to look into this issue.

Thank you for your support and interest of the LZA solution! I will leave this issue open should you have any follow-ups for the team, and we will update you when the issue is fixed.

felipempda commented 1 year ago

I saw that from v1.5.0 it's now possible to put a SNS ARN on the configuration https://github.com/awslabs/landing-zone-accelerator-on-aws/commit/6a99ccbcc5e06f9390de56b7725ddb795c6d04d1. However, would it also be possible to use global SNS Topics by name as discussed here? Both of these options would be great functionality, referencing SNS topics by ARN or global ones by name. Thanks a lot.