From 30ac467c745d4a70842bb6009bcd90f20602a466 Mon Sep 17 00:00:00 2001 From: Erik Peterson Date: Mon, 15 Jun 2026 15:06:26 -0400 Subject: [PATCH 1/6] Discover BCM Data Exports CUR 2.0 buckets for IAM policy Discovery now calls bcm-data-exports ListExports/GetExport to find the S3 destination buckets of CUR 2.0 exports and folds every export's bucket into MasterPayerBillingBucketArns. Bucket access is granted type-agnostically (only COST_AND_USAGE_REPORT is ingested), and the discovery Lambda role gains the two bcm-data-exports actions. Author: Erik Peterson --- docs/releases/1.0.101.md | 24 ++++ services/discovery/src/app.py | 47 +++++++- services/discovery/template.yaml | 2 + services/discovery/tests/unit/test_app.py | 140 ++++++++++++++++++++++ 4 files changed, 211 insertions(+), 2 deletions(-) create mode 100644 docs/releases/1.0.101.md diff --git a/docs/releases/1.0.101.md b/docs/releases/1.0.101.md new file mode 100644 index 0000000..7077160 --- /dev/null +++ b/docs/releases/1.0.101.md @@ -0,0 +1,24 @@ +# [1.0.101](https://github.com/Cloudzero/provision-account/compare/1.0.100...1.0.101) (2026-06-15) + +> Discover BCM Data Exports (CUR 2.0) S3 buckets and add them to the master-payer IAM policy + +## New Features + +### CUR 2.0 Bucket Discovery +* **Discovery now calls `bcm-data-exports:ListExports` + `bcm-data-exports:GetExport`** — AWS is migrating customers from the classic CUR to BCM Data Exports. `list_exports` only returns export ARNs, so each export is resolved with `get_export` to read its `DestinationConfigurations.S3Destination.S3Bucket`. +* **Type-agnostic bucket access, `COST_AND_USAGE_REPORT`-only ingest** — CloudZero only ingests exports of type `COST_AND_USAGE_REPORT` (CUR 2.0), but the IAM policy is intentionally granted access to the destination bucket of *every* export regardless of type. This mirrors the existing schema-agnostic behavior for classic CUR: a bucket referenced by any export is legitimate billing storage, and the customer can switch export configurations without redeploying this stack. +* **CUR 2.0 buckets are folded into `MasterPayerBillingBucketArns`** — `get_all_local_cur_bucket_names` now unions the classic CUR report buckets with the BCM Data Exports destination buckets, keeping only those owned locally. The master-payer role gains `s3:Get*`/`s3:List*` on the export buckets the same way it already covers classic CUR buckets. +* **Graceful degradation** — if the account cannot call BCM Data Exports (e.g. `AccessDeniedException`), discovery logs a warning and falls back to the classic CUR buckets rather than failing. + +### Template +* **`DiscoveryFunction` execution role gains `bcm-data-exports:ListExports` and `bcm-data-exports:GetExport`** — required for the discovery Lambda to enumerate CUR 2.0 export destinations. (The cross-account role policies already grant `bcm-data-exports:Get*/List*`.) + +## Files Modified + +- `services/discovery/src/app.py` +- `services/discovery/template.yaml` +- `services/discovery/tests/unit/test_app.py` + +## Deployment Notes + +* No customer action required. On the next stack update, the discovery Lambda begins enumerating BCM Data Exports destinations and the master-payer IAM policy's billing-bucket statement gains `Resource:` entries for any local CUR 2.0 / FOCUS export buckets. diff --git a/services/discovery/src/app.py b/services/discovery/src/app.py index 6f0e4cd..be0995c 100644 --- a/services/discovery/src/app.py +++ b/services/discovery/src/app.py @@ -16,6 +16,7 @@ logger.setLevel(logging.INFO) ct = boto3.client('cloudtrail') cur = boto3.client('cur', region_name='us-east-1') # cur is only in us-east-1 +bcm = boto3.client('bcm-data-exports', region_name='us-east-1') # bcm-data-exports is only in us-east-1 orgs = boto3.client('organizations') s3 = boto3.client('s3') @@ -70,12 +71,14 @@ }, required=True, extra=ALLOW_EXTRA) DEFAULT_PAYER_REPORTS = {'is_master_payer': False, 'report_definitions': []} +DEFAULT_DATA_EXPORTS = {'s3_buckets': []} NOT_IN_ORGANIZATION_RESPONSE = {} event_account_id = get_in(['event', 'ResourceProperties', 'AccountId']) coeffects_traillist = get_in(['coeffects', 'cloudtrail', 'trailList'], default=[]) coeffects_buckets = get_in(['coeffects', 's3', 'Buckets'], default=[]) coeffects_payer_reports = get_in(['coeffects', 'cur'], default=DEFAULT_PAYER_REPORTS) +coeffects_data_export_buckets = get_in(['coeffects', 'bcm_data_exports', 's3_buckets'], default=[]) coeffects_master_account_id = get_in(['coeffects', 'organizations', 'Organization', 'MasterAccountId']) output_is_organization_master = get_in(['output', 'IsOrganizationMasterAccount']) output_is_account_outside_organization = get_in(['output', 'IsAccountOutsideOrganization']) @@ -91,6 +94,7 @@ def coeffects(world): coeffects_cloudtrail, coeffects_s3, coeffects_cur, + coeffects_bcm_data_exports, coeffects_organizations) @@ -130,6 +134,38 @@ def coeffects_cur(world): return DEFAULT_PAYER_REPORTS +@coeffect('bcm_data_exports') +def coeffects_bcm_data_exports(world): + # CUR 2.0 exports (and any other BCM Data Exports) live here rather than in the + # classic CUR. CloudZero only ingests COST_AND_USAGE_REPORT exports, but we collect + # the S3 destination bucket for *every* export regardless of type so the master-payer + # role's bucket access covers all of them -- a customer can then repoint CloudZero at + # a different export without redeploying this stack. list_exports only returns export + # ARNs, so each export is resolved with get_export to read its destination bucket; + # bucket selection against the account's local buckets happens later in + # get_all_local_cur_bucket_names. + try: + export_arns = [] + next_token = None + while True: + response = bcm.list_exports(**({'NextToken': next_token} if next_token else {})) + export_arns.extend(ref['ExportArn'] for ref in response.get('Exports', []) if ref.get('ExportArn')) + next_token = response.get('NextToken') + if not next_token: + break + + s3_buckets = [] + for export_arn in export_arns: + export = bcm.get_export(ExportArn=export_arn).get('Export', {}) + bucket = get_in(['DestinationConfigurations', 'S3Destination', 'S3Bucket'], export) + if bucket: + s3_buckets.append(bucket) + return {'s3_buckets': s3_buckets} + except ClientError: + logger.warning('Failed to access BCM Data Exports ListExports/GetExport', exc_info=True) + return DEFAULT_DATA_EXPORTS + + @coeffect('organizations') def coeffects_organizations(world): try: @@ -309,9 +345,16 @@ def get_all_local_cur_bucket_names(world, report_definitions): # CloudZero to a different report without redeploying this stack. A bucket referenced # by a CUR report is by definition a CUR bucket; the schema filter only governs which # report CloudZero currently ingests, not which buckets are legitimate CUR storage. + # + # CUR 2.0 data lives in BCM Data Exports rather than the classic CUR, so we also + # include the S3 destination buckets discovered via bcm-data-exports list/get. As + # with classic CUR above, this is deliberately type-agnostic: CloudZero only ingests + # COST_AND_USAGE_REPORT exports, but the role is granted bucket access to every + # export's destination so the customer can switch export types without redeploy. local_buckets = {x['Name'] for x in coeffects_buckets(world)} - return sorted({r['S3Bucket'] for r in report_definitions - if isinstance(r.get('S3Bucket'), str) and r['S3Bucket'] in local_buckets}) + report_buckets = {r['S3Bucket'] for r in report_definitions if isinstance(r.get('S3Bucket'), str)} + data_export_buckets = {b for b in coeffects_data_export_buckets(world) if isinstance(b, str)} + return sorted((report_buckets | data_export_buckets) & local_buckets) def format_bucket_arns(bucket_names): diff --git a/services/discovery/template.yaml b/services/discovery/template.yaml index a19ef06..c39f933 100644 --- a/services/discovery/template.yaml +++ b/services/discovery/template.yaml @@ -36,6 +36,8 @@ Resources: - cloudtrail:DescribeTrails - s3:ListAllMyBuckets - cur:DescribeReportDefinitions + - bcm-data-exports:ListExports + - bcm-data-exports:GetExport - organizations:DescribeOrganization Resource: '*' Environment: diff --git a/services/discovery/tests/unit/test_app.py b/services/discovery/tests/unit/test_app.py index daef975..9a7b5dc 100644 --- a/services/discovery/tests/unit/test_app.py +++ b/services/discovery/tests/unit/test_app.py @@ -20,6 +20,9 @@ LOCAL_BUCKET_NAME = 'local-bucket' SECOND_LOCAL_BUCKET_NAME = 'another-local-cur-bucket' REMOTE_BUCKET_NAME = 'remote-bucket' +DATA_EXPORT_BUCKET_NAME = 'cur2-data-export-bucket' + +EXPORT_ARN = f'arn:aws:bcm-data-exports:us-east-1:{LOCAL_ACCOUNT_ID}:export/cur2-export' LOCAL_TRAIL_ARN = f'arn:aws:cloudtrail:us-east-1:{LOCAL_ACCOUNT_ID}:trail/local-trail' REMOTE_TRAIL_ARN = f'arn:aws:cloudtrail:us-east-1:{REMOTE_ACCOUNT_ID}:trail/remote-trail' @@ -266,6 +269,8 @@ def context(mocker): context.mock_cfnresponse_send = mocker.patch(f'{context.prefix}.cfnresponse.send', autospec=True) context.mock_ct = mocker.patch(f'{context.prefix}.ct', autospec=True) context.mock_cur = mocker.patch(f'{context.prefix}.cur', autospec=True) + context.mock_bcm = mocker.patch(f'{context.prefix}.bcm', autospec=True) + context.mock_bcm.list_exports.return_value = {'Exports': []} context.mock_orgs = mocker.patch(f'{context.prefix}.orgs', autospec=True) context.mock_s3 = mocker.patch(f'{context.prefix}.s3', autospec=True) yield context @@ -632,3 +637,138 @@ def test_handler_master_payer_excludes_remote_cur_buckets( ]) assert output['MasterPayerBillingBucketArns'] == expected assert f'arn:aws:s3:::{REMOTE_BUCKET_NAME}' not in output['MasterPayerBillingBucketArns'] + + +def _list_exports_response(*export_arns): + return {'Exports': [{'ExportArn': arn, 'ExportName': arn.split('/')[-1]} for arn in export_arns]} + + +def _get_export_response(bucket_name): + return { + 'Export': { + 'ExportArn': EXPORT_ARN, + 'Name': 'cur2-export', + 'DestinationConfigurations': { + 'S3Destination': { + 'S3Bucket': bucket_name, + 'S3Prefix': 'cur2', + 'S3Region': 'us-east-1', + 'S3OutputConfigurations': {}, + }, + }, + }, + } + + +@pytest.fixture() +def list_buckets_response_local_and_data_export(): + return { + 'Buckets': [ + {'Name': LOCAL_BUCKET_NAME}, + {'Name': DATA_EXPORT_BUCKET_NAME}, + ] + } + + +@pytest.mark.unit +def test_handler_master_payer_includes_local_cur2_data_export_bucket( + context, cfn_event, describe_trails_response_local, + list_buckets_response_local_and_data_export, describe_report_definitions_response_local, + describe_organizations_local, +): + context.mock_ct.describe_trails.return_value = describe_trails_response_local + context.mock_cur.describe_report_definitions.return_value = describe_report_definitions_response_local + context.mock_bcm.list_exports.return_value = _list_exports_response(EXPORT_ARN) + context.mock_bcm.get_export.return_value = _get_export_response(DATA_EXPORT_BUCKET_NAME) + context.mock_orgs.describe_organization.return_value = describe_organizations_local + context.mock_s3.list_buckets.return_value = list_buckets_response_local_and_data_export + ret = app.handler(cfn_event, None) + assert ret is None + ((_, _, status, output, _), _) = context.mock_cfnresponse_send.call_args + assert status == cfnresponse.SUCCESS + expected = ','.join([ + f'arn:aws:s3:::{DATA_EXPORT_BUCKET_NAME}', + f'arn:aws:s3:::{DATA_EXPORT_BUCKET_NAME}/*', + f'arn:aws:s3:::{LOCAL_BUCKET_NAME}', + f'arn:aws:s3:::{LOCAL_BUCKET_NAME}/*', + ]) + assert output['MasterPayerBillingBucketArns'] == expected + context.mock_bcm.get_export.assert_called_once_with(ExportArn=EXPORT_ARN) + + +@pytest.mark.unit +def test_handler_master_payer_includes_all_export_buckets_regardless_of_type( + context, cfn_event, describe_trails_response_local, + describe_report_definitions_response_local, describe_organizations_local, +): + # CloudZero only ingests COST_AND_USAGE_REPORT exports, but the IAM policy must grant + # bucket access to every export's destination regardless of type. The discovery code + # never inspects the export type, so two exports pointing at two local buckets both + # get covered. + second_export_arn = f'arn:aws:bcm-data-exports:us-east-1:{LOCAL_ACCOUNT_ID}:export/focus-export' + export_buckets = {EXPORT_ARN: DATA_EXPORT_BUCKET_NAME, second_export_arn: SECOND_LOCAL_BUCKET_NAME} + context.mock_ct.describe_trails.return_value = describe_trails_response_local + context.mock_cur.describe_report_definitions.return_value = describe_report_definitions_response_local + context.mock_bcm.list_exports.return_value = _list_exports_response(EXPORT_ARN, second_export_arn) + context.mock_bcm.get_export.side_effect = lambda ExportArn: _get_export_response(export_buckets[ExportArn]) + context.mock_orgs.describe_organization.return_value = describe_organizations_local + context.mock_s3.list_buckets.return_value = { + 'Buckets': [ + {'Name': LOCAL_BUCKET_NAME}, + {'Name': DATA_EXPORT_BUCKET_NAME}, + {'Name': SECOND_LOCAL_BUCKET_NAME}, + ] + } + ret = app.handler(cfn_event, None) + assert ret is None + ((_, _, status, output, _), _) = context.mock_cfnresponse_send.call_args + assert status == cfnresponse.SUCCESS + arns = output['MasterPayerBillingBucketArns'] + for bucket in (LOCAL_BUCKET_NAME, DATA_EXPORT_BUCKET_NAME, SECOND_LOCAL_BUCKET_NAME): + assert f'arn:aws:s3:::{bucket}' in arns + assert f'arn:aws:s3:::{bucket}/*' in arns + + +@pytest.mark.unit +def test_handler_master_payer_excludes_remote_cur2_data_export_bucket( + context, cfn_event, describe_trails_response_local, + list_buckets_response, describe_report_definitions_response_local, + describe_organizations_local, +): + context.mock_ct.describe_trails.return_value = describe_trails_response_local + context.mock_cur.describe_report_definitions.return_value = describe_report_definitions_response_local + context.mock_bcm.list_exports.return_value = _list_exports_response(EXPORT_ARN) + context.mock_bcm.get_export.return_value = _get_export_response(REMOTE_BUCKET_NAME) + context.mock_orgs.describe_organization.return_value = describe_organizations_local + context.mock_s3.list_buckets.return_value = list_buckets_response + ret = app.handler(cfn_event, None) + assert ret is None + ((_, _, status, output, _), _) = context.mock_cfnresponse_send.call_args + assert status == cfnresponse.SUCCESS + assert output['MasterPayerBillingBucketArns'] == ( + f'arn:aws:s3:::{LOCAL_BUCKET_NAME},arn:aws:s3:::{LOCAL_BUCKET_NAME}/*' + ) + assert f'arn:aws:s3:::{REMOTE_BUCKET_NAME}' not in output['MasterPayerBillingBucketArns'] + + +@pytest.mark.unit +def test_handler_master_payer_survives_bcm_data_exports_access_denied( + context, cfn_event, describe_trails_response_local, + list_buckets_response, describe_report_definitions_response_local, + describe_organizations_local, +): + context.mock_ct.describe_trails.return_value = describe_trails_response_local + context.mock_cur.describe_report_definitions.return_value = describe_report_definitions_response_local + context.mock_bcm.list_exports.side_effect = ClientError( + {'Error': {'Code': 'AccessDeniedException', 'Message': 'not authorized to call ListExports'}}, + 'ListExports', + ) + context.mock_orgs.describe_organization.return_value = describe_organizations_local + context.mock_s3.list_buckets.return_value = list_buckets_response + ret = app.handler(cfn_event, None) + assert ret is None + ((_, _, status, output, _), _) = context.mock_cfnresponse_send.call_args + assert status == cfnresponse.SUCCESS + assert output['MasterPayerBillingBucketArns'] == ( + f'arn:aws:s3:::{LOCAL_BUCKET_NAME},arn:aws:s3:::{LOCAL_BUCKET_NAME}/*' + ) From 50c9b31136fad670b67bb61f17ed52e72aba0d0b Mon Sep 17 00:00:00 2001 From: khill2018 Date: Tue, 16 Jun 2026 10:14:24 -0400 Subject: [PATCH 2/6] Isolate per-export GetExport failures and cover pagination Address review feedback on BCM Data Exports discovery: - Wrap each get_export call in its own try/except so a transient failure on a single export no longer drops buckets already resolved from other exports; the failing export is logged and skipped. - Keep list_exports failure as graceful fallback to empty exports. - Add tests for the multi-page list_exports NextToken pagination path and for per-export get_export error isolation. Co-Authored-By: Claude Opus 4.8 (1M context) --- services/discovery/src/app.py | 25 +++++--- services/discovery/tests/unit/test_app.py | 73 +++++++++++++++++++++++ 2 files changed, 89 insertions(+), 9 deletions(-) diff --git a/services/discovery/src/app.py b/services/discovery/src/app.py index be0995c..d7f66fd 100644 --- a/services/discovery/src/app.py +++ b/services/discovery/src/app.py @@ -153,18 +153,25 @@ def coeffects_bcm_data_exports(world): next_token = response.get('NextToken') if not next_token: break - - s3_buckets = [] - for export_arn in export_arns: - export = bcm.get_export(ExportArn=export_arn).get('Export', {}) - bucket = get_in(['DestinationConfigurations', 'S3Destination', 'S3Bucket'], export) - if bucket: - s3_buckets.append(bucket) - return {'s3_buckets': s3_buckets} except ClientError: - logger.warning('Failed to access BCM Data Exports ListExports/GetExport', exc_info=True) + logger.warning('Failed to access BCM Data Exports ListExports', exc_info=True) return DEFAULT_DATA_EXPORTS + # Resolve each export independently: a transient failure on one export should not + # drop the buckets already resolved from the others, so isolate the get_export call + # per export rather than wrapping the whole loop in a single try/except. + s3_buckets = [] + for export_arn in export_arns: + try: + export = bcm.get_export(ExportArn=export_arn).get('Export', {}) + except ClientError: + logger.warning(f'Failed to access BCM Data Exports GetExport for {export_arn}', exc_info=True) + continue + bucket = get_in(['DestinationConfigurations', 'S3Destination', 'S3Bucket'], export) + if bucket: + s3_buckets.append(bucket) + return {'s3_buckets': s3_buckets} + @coeffect('organizations') def coeffects_organizations(world): diff --git a/services/discovery/tests/unit/test_app.py b/services/discovery/tests/unit/test_app.py index 9a7b5dc..a06bcab 100644 --- a/services/discovery/tests/unit/test_app.py +++ b/services/discovery/tests/unit/test_app.py @@ -772,3 +772,76 @@ def test_handler_master_payer_survives_bcm_data_exports_access_denied( assert output['MasterPayerBillingBucketArns'] == ( f'arn:aws:s3:::{LOCAL_BUCKET_NAME},arn:aws:s3:::{LOCAL_BUCKET_NAME}/*' ) + + +@pytest.mark.unit +def test_handler_master_payer_paginates_list_exports( + context, cfn_event, describe_trails_response_local, + describe_report_definitions_response_local, describe_organizations_local, +): + # list_exports is paginated: a NextToken on the first page must drive a second call, + # and exports from every page must be resolved. The two exports live on separate pages. + second_export_arn = f'arn:aws:bcm-data-exports:us-east-1:{LOCAL_ACCOUNT_ID}:export/focus-export' + export_buckets = {EXPORT_ARN: DATA_EXPORT_BUCKET_NAME, second_export_arn: SECOND_LOCAL_BUCKET_NAME} + context.mock_ct.describe_trails.return_value = describe_trails_response_local + context.mock_cur.describe_report_definitions.return_value = describe_report_definitions_response_local + first_page = _list_exports_response(EXPORT_ARN) + first_page['NextToken'] = 'page-2' + second_page = _list_exports_response(second_export_arn) + context.mock_bcm.list_exports.side_effect = [first_page, second_page] + context.mock_bcm.get_export.side_effect = lambda ExportArn: _get_export_response(export_buckets[ExportArn]) + context.mock_orgs.describe_organization.return_value = describe_organizations_local + context.mock_s3.list_buckets.return_value = { + 'Buckets': [ + {'Name': LOCAL_BUCKET_NAME}, + {'Name': DATA_EXPORT_BUCKET_NAME}, + {'Name': SECOND_LOCAL_BUCKET_NAME}, + ] + } + ret = app.handler(cfn_event, None) + assert ret is None + ((_, _, status, output, _), _) = context.mock_cfnresponse_send.call_args + assert status == cfnresponse.SUCCESS + assert context.mock_bcm.list_exports.call_count == 2 + context.mock_bcm.list_exports.assert_any_call(NextToken='page-2') + arns = output['MasterPayerBillingBucketArns'] + for bucket in (DATA_EXPORT_BUCKET_NAME, SECOND_LOCAL_BUCKET_NAME): + assert f'arn:aws:s3:::{bucket}' in arns + assert f'arn:aws:s3:::{bucket}/*' in arns + + +@pytest.mark.unit +def test_handler_master_payer_isolates_per_export_get_export_failure( + context, cfn_event, describe_trails_response_local, + describe_report_definitions_response_local, describe_organizations_local, +): + # A get_export failure on a single export must not drop buckets already resolved from + # the other exports: the failing export is skipped and the rest still get covered. + failing_export_arn = f'arn:aws:bcm-data-exports:us-east-1:{LOCAL_ACCOUNT_ID}:export/broken-export' + + def get_export(ExportArn): + if ExportArn == failing_export_arn: + raise ClientError( + {'Error': {'Code': 'InternalServerException', 'Message': 'transient failure'}}, + 'GetExport', + ) + return _get_export_response(DATA_EXPORT_BUCKET_NAME) + + context.mock_ct.describe_trails.return_value = describe_trails_response_local + context.mock_cur.describe_report_definitions.return_value = describe_report_definitions_response_local + context.mock_bcm.list_exports.return_value = _list_exports_response(failing_export_arn, EXPORT_ARN) + context.mock_bcm.get_export.side_effect = get_export + context.mock_orgs.describe_organization.return_value = describe_organizations_local + context.mock_s3.list_buckets.return_value = { + 'Buckets': [ + {'Name': LOCAL_BUCKET_NAME}, + {'Name': DATA_EXPORT_BUCKET_NAME}, + ] + } + ret = app.handler(cfn_event, None) + assert ret is None + ((_, _, status, output, _), _) = context.mock_cfnresponse_send.call_args + assert status == cfnresponse.SUCCESS + arns = output['MasterPayerBillingBucketArns'] + assert f'arn:aws:s3:::{DATA_EXPORT_BUCKET_NAME}' in arns + assert f'arn:aws:s3:::{DATA_EXPORT_BUCKET_NAME}/*' in arns From 5b0c9e0223c1d14f95e4377bb9a2e869a188ecec Mon Sep 17 00:00:00 2001 From: Erik Peterson Date: Tue, 16 Jun 2026 14:24:03 -0400 Subject: [PATCH 3/6] Flatten connected-account stack and modernize custom resources Collapse the 6 nested stacks into one: connected_account.yaml inlines the Discovery and Notification Lambdas and provisions account resources through a single nested AccountResources stack (merged resource_owner + master_payer). Replace the Custom::CostAndUsageReport Lambda with native AWS::S3::Bucket, AWS::S3::BucketPolicy and AWS::CUR::ReportDefinition, gated on us-east-1 with DeletionPolicy: Retain. Remove the deprecated audit and cloudtrail-owner account types (templates, discovery detection, cloudtrail:DescribeTrails); the reactor payload contract is unchanged, emitting null/false for the deprecated fields. Rewrite the discovery and notification Lambdas into clear procedural flows; notification now reads direct resource properties instead of scraping stack outputs. Add a Stage parameter so prod/dev parents differ by one line. Author: Erik Peterson --- README.md | 2 + docs/releases/1.0.102.md | 68 ++ services/account_type/account_resources.yaml | 606 +++++++++++++++++ services/account_type/audit.yaml | 78 --- services/account_type/cloudtrail_owner.yaml | 85 --- services/account_type/master_payer.yaml | 471 ------------- services/account_type/resource_owner.yaml | 238 ------- services/connected_account.yaml | 234 ++++--- services/connected_account_dev.yaml | 239 ++++--- services/discovery/src/app.py | 376 ++++------- services/discovery/template.yaml | 1 - services/discovery/tests/unit/test_app.py | 669 ++++--------------- services/notification/src/app.py | 369 +++------- services/notification/tests/unit/test_app.py | 307 ++++----- 14 files changed, 1472 insertions(+), 2271 deletions(-) create mode 100644 docs/releases/1.0.102.md create mode 100644 services/account_type/account_resources.yaml delete mode 100644 services/account_type/audit.yaml delete mode 100644 services/account_type/cloudtrail_owner.yaml delete mode 100644 services/account_type/master_payer.yaml delete mode 100644 services/account_type/resource_owner.yaml diff --git a/README.md b/README.md index 316fbac..75d37a1 100644 --- a/README.md +++ b/README.md @@ -82,6 +82,8 @@ The AWS account that contains your Cost and Usage Report (CUR) and is the payer **Important**: CloudZero requires an HOURLY Cost and Usage Report. Daily reports are not supported. +**Deploy payer accounts in `us-east-1`**: when the payer account has no existing CUR, this template creates one for you using the native `AWS::CUR::ReportDefinition` resource, which AWS only supports in `us-east-1` (the single region the Cost and Usage Report API runs in). Launch the stack in `us-east-1` on a payer account. If it is launched elsewhere on a payer account that needs a new CUR, the cross-account role is still created but the CUR is not — the stack's `MasterPayerCurStatus` output reports the skip. Payer accounts that already have a CUR are unaffected by region. + If the payer account has more than one CUR configured, the auto-generated IAM policy grants `s3:Get*`/`s3:List*` on every CUR bucket discovered in the account, not only the one CloudZero ingests as primary. This keeps the role aligned with the customer's actual CUR footprint and avoids surprises if the primary CUR is later swapped. #### Resource Owner Account diff --git a/docs/releases/1.0.102.md b/docs/releases/1.0.102.md new file mode 100644 index 0000000..f02d9fc --- /dev/null +++ b/docs/releases/1.0.102.md @@ -0,0 +1,68 @@ +# [1.0.102](https://github.com/Cloudzero/provision-account/compare/1.0.101...1.0.102) (2026-06-16) + +> Flatten the connected-account stack to a single nested stack, move CUR creation to native CloudFormation, and remove the deprecated audit / cloudtrail-owner account types + +## Breaking / Behavioral Changes + +### Master-payer CUR creation now requires `us-east-1` +* The Cost and Usage Report for a payer account is now created with the native + **`AWS::CUR::ReportDefinition`** resource (plus `AWS::S3::Bucket` + `AWS::S3::BucketPolicy`), + replacing the inline `Custom::CostAndUsageReport` Lambda. AWS only supports this resource in + `us-east-1`, so **payer accounts that need a new CUR must be onboarded from `us-east-1`**. + Launched elsewhere on such an account, the cross-account role is still created but the CUR is + skipped; the stack's new `MasterPayerCurStatus` output reports why. Payer accounts that + already have a CUR are unaffected by region. The CUR bucket uses `DeletionPolicy: Retain`, so + billing data is never deleted on stack removal (matching the prior Lambda behavior). + +### Audit and CloudTrail-owner account types removed +* The deprecated **audit** and **cloudtrail-owner** roles/features are gone: their templates, + the discovery Lambda's CloudTrail/audit detection, and the `cloudtrail:DescribeTrails` + discovery permission have all been removed. +* The reactor notification payload contract is **unchanged** — the `links.audit`, + `links.cloudtrail_owner`, and `discovery.*cloudtrail*` / `is_audit_account` / + `is_cloudtrail_owner_account` fields remain in the payload and are now always `null` / `false`. + +## New Features + +### Single nested stack (was 6) +* `connected_account.yaml` now **inlines** the Discovery and Notification Lambdas (as native + `AWS::Lambda::Function` + `AWS::IAM::Role`) and provisions account resources through one + nested `AccountResources` stack (`services/account_type/account_resources.yaml`, merging the + former resource_owner + master_payer templates). The discovery, notification, resource_owner, + cloudtrail_owner, audit, and master_payer nested templates are no longer used as nested stacks. +* The notification Lambda no longer scrapes sibling CloudFormation stack outputs at runtime; it + receives every value it needs as direct resource properties, and its execution role drops + `AWSCloudFormationReadOnlyAccess`. + +### `Stage` parameter +* Both `connected_account.yaml` and `connected_account_dev.yaml` now carry a `Stage` parameter + (`prod` / `dev`) selecting the `CallbackConfiguration` mapping, so the two files differ only + by the `Stage` default instead of diverging across every resource. + +### Code clarity +* The discovery and notification Lambda handlers were rewritten from the `toolz`-pipeline / + decorator-coeffect style into straightforward, auditable procedural flows. + +## Files Modified + +- `services/connected_account.yaml`, `services/connected_account_dev.yaml` +- `services/account_type/account_resources.yaml` (new; replaces the four account-type templates) +- `services/discovery/src/app.py`, `services/discovery/template.yaml`, discovery tests +- `services/notification/src/app.py`, notification tests +- `README.md` + +## Files Removed + +- `services/account_type/resource_owner.yaml` +- `services/account_type/master_payer.yaml` +- `services/account_type/audit.yaml` +- `services/account_type/cloudtrail_owner.yaml` + +## Deployment Notes + +* On the next stack update, existing customer stacks delete the old per-account-type nested + stacks and create the single `AccountResources` stack; the cross-account IAM roles are + recreated (trust is by ExternalId and role path/names are stable, so CloudZero access + continues). Validate on a stack *update* in a test account, including the master-payer paths. +* **Follow-up:** `policies/` and `terraform/` still carry audit / cloudtrail-owner definitions to + clean up in a separate change. diff --git a/services/account_type/account_resources.yaml b/services/account_type/account_resources.yaml new file mode 100644 index 0000000..ef4d946 --- /dev/null +++ b/services/account_type/account_resources.yaml @@ -0,0 +1,606 @@ +# -*- coding: utf-8 -*- +# Copyright (c) 2016-present, CloudZero, Inc. All rights reserved. +# Licensed under the BSD-style license. See LICENSE file in the project root for full license information. + +AWSTemplateFormatVersion: '2010-09-09' +Description: > + CloudZero Account Resources Template + + Single nested stack that provisions the cross-account resources for whichever + account type Discovery detected. Merges the former resource_owner and master_payer + templates. Resource creation is gated on the Discovery flags passed in as parameters + (CloudFormation Conditions cannot read custom-resource outputs directly, so the + parent passes them as parameters here). The audit and cloudtrail-owner account types + are deprecated and intentionally not provisioned. + +Parameters: + ExternalId: + Type: String + Description: | + Unique ExternalId for Customer Organization; for cross-account Role Access and + associating this template with a Customer Organization + ReactorAccountId: + Type: String + Description: The CloudZero reactor AWS account ID + Default: '061190967865' + ConnectorsAccountId: + Type: String + Description: Additional CloudZero AWS account ID that will assume the cross-account role. + Default: '559846027439' + IsResourceOwnerAccount: + Type: String + Description: Flag indicating whether to provision the resource-owner role. + AllowedValues: ['true', 'false'] + IsOrganizationMasterAccount: + Type: String + Description: Flag indicating whether this is an organization management account. + AllowedValues: ['true', 'false'] + IsMasterPayerAccount: + Type: String + Description: Flag indicating whether this is the master-payer account (existing valid CUR or standalone). + AllowedValues: ['true', 'false'] + MasterPayerBillingBucketName: + Type: String + Description: The name of the existing S3 bucket holding billing (CUR) data, or 'null'. + MasterPayerBillingBucketArns: + Type: String + Default: '' + Description: | + Comma-separated list of S3 ARNs (both bucket and bucket/* forms) for every + locally-owned S3 bucket referenced by any CUR report/export in this account. + Empty only when no CUR points to a locally-owned bucket. + +Mappings: + MasterPayerDefaults: + Cur: + BucketName: 'cz-cur-hourly-csv' + +Conditions: + # --- resource owner --- + ResourceOwnerCreateResources: !Equals [!Ref IsResourceOwnerAccount, 'true'] + + # --- master payer --- + IsUsEast1: !Equals [!Ref 'AWS::Region', 'us-east-1'] + MasterPayerValidExistingBucketName: !Not [!Equals [!Ref MasterPayerBillingBucketName, 'null']] + MasterPayerHasBillingBucketArns: !Not [!Equals [!Ref MasterPayerBillingBucketArns, '']] + # Master payer that has no existing CUR -> a new CUR should be created for it. + MasterPayerCreateCurAndBucketAndPolicy: !And + - !Not [Condition: MasterPayerValidExistingBucketName] + - !Equals [!Ref IsMasterPayerAccount, 'true'] + # Master payer that already has a CUR -> only attach the read policy. + MasterPayerCreatePolicyForExistingCur: !And + - Condition: MasterPayerValidExistingBucketName + - !Equals [!Ref IsMasterPayerAccount, 'true'] + # The cross-account role is created for any master payer (existing or new CUR). + MasterPayerCreateRoleAndPolicy: !Or + - Condition: MasterPayerCreatePolicyForExistingCur + - Condition: MasterPayerCreateCurAndBucketAndPolicy + # Native CUR resources (bucket + policy + report) can only be created in us-east-1, + # the single region in which AWS::CUR::ReportDefinition / the CUR API exist. A master + # payer onboarded from any other region therefore gets its role but no new CUR -- see + # the MasterPayerCurStatus output and README. Existing-CUR payers are unaffected. + MasterPayerCreateNativeCur: !And + - Condition: MasterPayerCreateCurAndBucketAndPolicy + - Condition: IsUsEast1 + +Resources: + + ##################### + # Resource Owner + ##################### + ResourceOwnerRole: + Type: AWS::IAM::Role + Condition: ResourceOwnerCreateResources + Properties: + Path: '/cloudzero/' + AssumeRolePolicyDocument: + Version: 2012-10-17 + Statement: + - Effect: Allow + Principal: + AWS: + - !Sub 'arn:aws:iam::${ReactorAccountId}:root' + - !Sub 'arn:aws:iam::${ConnectorsAccountId}:root' + Action: + - 'sts:AssumeRole' + Condition: + StringEquals: + 'sts:ExternalId': !Ref ExternalId + ManagedPolicyArns: + - arn:aws:iam::aws:policy/CloudWatchReadOnlyAccess + - arn:aws:iam::aws:policy/ComputeOptimizerReadOnlyAccess + - arn:aws:iam::aws:policy/job-function/ViewOnlyAccess + - arn:aws:iam::aws:policy/AWSBillingReadOnlyAccess + + ResourceOwnerRolePolicy: + Type: AWS::IAM::Policy + Condition: ResourceOwnerCreateResources + Properties: + PolicyName: !Sub 'cloudzero-resource-owner-policy-${ReactorAccountId}' + PolicyDocument: + Version: 2012-10-17 + Statement: + - Sid: CZTier0BillingDataAccess20260420 + Effect: Allow + Action: + - account:GetAccountInformation + - bcm-data-exports:Get* + - bcm-data-exports:List* + - billing:Get* + - ce:Describe* + - ce:Get* + - ce:List* + - consolidatedbilling:Get* + - consolidatedbilling:List* + - cur:Describe* + - cur:Get* + - cur:List* + - cur:Validate* + - freetier:Get* + - invoicing:Get* + - invoicing:List* + - mapcredits:List* + - payments:Get* + - payments:List* + - purchase-orders:Get* + - purchase-orders:List* + - purchase-orders:View* + - sustainability:Get* + - tax:Get* + - tax:List* + Resource: "*" + - Sid: CZTier0ReservedCapacity20260420 + Effect: Allow + Action: + - dynamodb:DescribeReserved* + - ec2:DescribeReserved* + - elasticache:DescribeReserved* + - es:DescribeReserved* + - rds:DescribeReserved* + - redshift:DescribeReserved* + Resource: "*" + - Sid: CZTier0SavingsPlans20260420 + Effect: Allow + Action: + - savingsplans:Describe* + - savingsplans:List* + Resource: "*" + - Sid: CZTier1Budgets20260420 + Effect: Allow + Action: + - budgets:Describe* + - budgets:View* + Resource: "*" + - Sid: CZTier1BillingConductor20260420 + Effect: Allow + Action: + - billingconductor:Get* + - billingconductor:List* + Resource: "*" + - Sid: CZTier1PricingAndForecasting20260420 + Effect: Allow + Action: + - bcm-pricing-calculator:Get* + - bcm-pricing-calculator:List* + - pricing:* + Resource: "*" + - Sid: CZTier1OrganizationsAndTags20260420 + Effect: Allow + Action: + - account:ListRegions + - organizations:Describe* + - organizations:List* + - resource-explorer:List* + - resource-groups:Get* + - resource-groups:List* + - resource-groups:Search* + - tag:Describe* + - tag:Get* + Resource: "*" + - Sid: CZTier1ComputeOptimizer20260420 + Effect: Allow + Action: + - compute-optimizer:Describe* + - compute-optimizer:Get* + Resource: "*" + - Sid: CZTier1CostOptimizationHub20260420 + Effect: Allow + Action: + - cost-optimization-hub:Get* + - cost-optimization-hub:List* + Resource: "*" + - Sid: CZTier1TrustedAdvisorAndHealth20260420 + Effect: Allow + Action: + - health:Describe* + - support:DescribeTrustedAdvisor* + Resource: "*" + - Sid: CZTier1ContainerOrchestration20260511 + Effect: Allow + Action: + - ecs:Describe* + - ecs:List* + - eks:Describe* + - eks:List* + Resource: "*" + - Sid: CZTier1ContainerInsightsLogs20260420 + Effect: Allow + Action: + - logs:Describe* + - logs:Filter* + - logs:Get* + - logs:List* + - logs:StartQuery + - logs:StopQuery + Resource: arn:aws:logs:*:*:log-group:/aws/containerinsights/* + - Sid: CZTier1ContainerInsightsLogQuery20260420 + Effect: Allow + Action: + - logs:DescribeLogGroups + - logs:GetQueryResults + Resource: arn:aws:logs:*:*:log-group:* + - Sid: CZTier2CloudTrail20260420 + Effect: Allow + Action: + - cloudtrail:Describe* + - cloudtrail:Get* + - cloudtrail:List* + - cloudtrail:LookupEvents + Resource: "*" + - Sid: CZTier2ServiceQuotas20260420 + Effect: Allow + Action: + - servicequotas:Get* + - servicequotas:List* + Resource: "*" + - Sid: CZTier2CloudWatchMetrics20260420 + Effect: Allow + Action: + - autoscaling:Describe* + - cloudwatch:Describe* + - cloudwatch:Get* + - cloudwatch:List* + Resource: "*" + - Sid: CZTier2CloudFormation20260420 + Effect: Allow + Action: + - cloudformation:Describe* + - cloudformation:Get* + - cloudformation:List* + Resource: "*" + - Sid: CZTier2SelfInspection20260420 + Effect: Allow + Action: + - iam:GetRole + - iam:GetRolePolicy + - iam:ListAttachedRolePolicies + - iam:ListRolePolicies + - iam:ListRoleTags + - iam:SimulatePrincipalPolicy + Resource: arn:aws:iam::*:role/cloudzero/* + - Sid: CZTier2SelfInspectionPolicies20260420 + Effect: Allow + Action: + - iam:GetPolicy + - iam:GetPolicyVersion + Resource: + - arn:aws:iam::aws:policy/AWSBillingReadOnlyAccess + - arn:aws:iam::aws:policy/CloudWatchReadOnlyAccess + - arn:aws:iam::aws:policy/ComputeOptimizerReadOnlyAccess + - arn:aws:iam::aws:policy/job-function/ViewOnlyAccess + Roles: + - !Ref ResourceOwnerRole + + ##################### + # Master Payer + ##################### + # Native Cost & Usage Report resources. Previously a Custom::CostAndUsageReport Lambda + # created these because CloudFormation had no CUR resource; AWS::CUR::ReportDefinition + # now exists, so we use it directly. Only created in us-east-1 (see MasterPayerCreateNativeCur). + # Billing data is never destroyed on stack delete -- hence DeletionPolicy: Retain. + MasterPayerCurBucket: + Type: AWS::S3::Bucket + Condition: MasterPayerCreateNativeCur + DeletionPolicy: Retain + UpdateReplacePolicy: Retain + Properties: + BucketName: !Sub + - '${BucketName}-${Id}' + - BucketName: !FindInMap [MasterPayerDefaults, Cur, BucketName] + Id: !Select [2, !Split ['/', !Ref 'AWS::StackId']] + + MasterPayerCurBucketPolicy: + Type: AWS::S3::BucketPolicy + Condition: MasterPayerCreateNativeCur + DeletionPolicy: Retain + UpdateReplacePolicy: Retain + Properties: + Bucket: !Ref MasterPayerCurBucket + PolicyDocument: + Version: '2012-10-17' + Statement: + - Sid: AddBillReportsGet + Effect: Allow + Principal: + Service: billingreports.amazonaws.com + Action: + - 's3:GetBucketAcl' + - 's3:GetBucketPolicy' + Resource: !GetAtt MasterPayerCurBucket.Arn + - Sid: AddBillReportsPut + Effect: Allow + Principal: + Service: billingreports.amazonaws.com + Action: 's3:PutObject' + Resource: !Sub '${MasterPayerCurBucket.Arn}/*' + + MasterPayerCurReport: + Type: AWS::CUR::ReportDefinition + Condition: MasterPayerCreateNativeCur + DependsOn: MasterPayerCurBucketPolicy + DeletionPolicy: Retain + UpdateReplacePolicy: Retain + Properties: + ReportName: cloudzero-cur-hourly-csv + TimeUnit: HOURLY + Format: textORcsv + Compression: GZIP + AdditionalSchemaElements: + - RESOURCES + S3Bucket: !Ref MasterPayerCurBucket + S3Prefix: cloudzero + S3Region: !Ref 'AWS::Region' + RefreshClosedReports: true + ReportVersioning: CREATE_NEW_REPORT + + MasterPayerRole: + Condition: MasterPayerCreateRoleAndPolicy + Type: 'AWS::IAM::Role' + Properties: + Path: '/cloudzero/' + AssumeRolePolicyDocument: + Version: '2012-10-17' + Statement: + - Sid: czServiceAccount01 + Effect: Allow + Principal: + AWS: + - !Sub 'arn:aws:iam::${ReactorAccountId}:root' + - !Sub 'arn:aws:iam::${ConnectorsAccountId}:root' + Action: + - 'sts:AssumeRole' + Condition: + StringEquals: + 'sts:ExternalId': !Ref ExternalId + ManagedPolicyArns: + - arn:aws:iam::aws:policy/CloudWatchReadOnlyAccess + - arn:aws:iam::aws:policy/ComputeOptimizerReadOnlyAccess + - arn:aws:iam::aws:policy/job-function/ViewOnlyAccess + - arn:aws:iam::aws:policy/AWSBillingReadOnlyAccess + + MasterPayerRolePolicy: + Condition: MasterPayerCreateRoleAndPolicy + Type: 'AWS::IAM::Policy' + Properties: + PolicyName: !Sub 'cloudzero-master-payer-policy-${ReactorAccountId}' + Roles: + - !Ref MasterPayerRole + PolicyDocument: + Version: '2012-10-17' + Statement: + - Sid: CZTier0BillingBucket20260420 + Effect: Allow + Action: + - s3:Get* + - s3:List* + # The discovery Lambda emits MasterPayerBillingBucketArns as a comma-separated + # ARN list covering every locally-owned S3 bucket referenced by any CUR report + # (both `bucket` and `bucket/*` forms). When ARNs is empty (e.g., the stack was + # invoked directly with only MasterPayerBillingBucketName), we fall back to the + # legacy single-bucket pair so !Split never sees an empty string (which would + # otherwise yield [''] and produce an invalid IAM Resource). When this stack is + # also creating a brand-new CUR bucket, discovery found no existing CURs and + # falls through to the new-bucket pair below. + Resource: !Split + - ',' + - !If + - MasterPayerCreatePolicyForExistingCur + - !If + - MasterPayerHasBillingBucketArns + - !Ref MasterPayerBillingBucketArns + - !Sub 'arn:aws:s3:::${MasterPayerBillingBucketName},arn:aws:s3:::${MasterPayerBillingBucketName}/*' + - !Sub + - 'arn:aws:s3:::${BucketName}-${Id},arn:aws:s3:::${BucketName}-${Id}/*' + - BucketName: !FindInMap [MasterPayerDefaults, Cur, BucketName] + Id: !Select [2, !Split ['/', !Ref 'AWS::StackId']] + - Sid: CZTier0BillingDataAccess20260420 + Effect: Allow + Action: + - account:GetAccountInformation + - bcm-data-exports:Get* + - bcm-data-exports:List* + - billing:Get* + - ce:Describe* + - ce:Get* + - ce:List* + - consolidatedbilling:Get* + - consolidatedbilling:List* + - cur:Describe* + - cur:Get* + - cur:List* + - cur:Validate* + - freetier:Get* + - invoicing:Get* + - invoicing:List* + - mapcredits:List* + - payments:Get* + - payments:List* + - purchase-orders:Get* + - purchase-orders:List* + - purchase-orders:View* + - sustainability:Get* + - tax:Get* + - tax:List* + Resource: "*" + - Sid: CZTier0ReservedCapacity20260420 + Effect: Allow + Action: + - dynamodb:DescribeReserved* + - ec2:DescribeReserved* + - elasticache:DescribeReserved* + - es:DescribeReserved* + - rds:DescribeReserved* + - redshift:DescribeReserved* + Resource: "*" + - Sid: CZTier0SavingsPlans20260420 + Effect: Allow + Action: + - savingsplans:Describe* + - savingsplans:List* + Resource: "*" + - Sid: CZTier1Budgets20260420 + Effect: Allow + Action: + - budgets:Describe* + - budgets:View* + Resource: "*" + - Sid: CZTier1BillingConductor20260420 + Effect: Allow + Action: + - billingconductor:Get* + - billingconductor:List* + Resource: "*" + - Sid: CZTier1PricingAndForecasting20260420 + Effect: Allow + Action: + - bcm-pricing-calculator:Get* + - bcm-pricing-calculator:List* + - pricing:* + Resource: "*" + - Sid: CZTier1OrganizationsAndTags20260420 + Effect: Allow + Action: + - account:ListRegions + - organizations:Describe* + - organizations:List* + - resource-explorer:List* + - resource-groups:Get* + - resource-groups:List* + - resource-groups:Search* + - tag:Describe* + - tag:Get* + Resource: "*" + - Sid: CZTier1ComputeOptimizer20260420 + Effect: Allow + Action: + - compute-optimizer:Describe* + - compute-optimizer:Get* + Resource: "*" + - Sid: CZTier1CostOptimizationHub20260420 + Effect: Allow + Action: + - cost-optimization-hub:Get* + - cost-optimization-hub:List* + Resource: "*" + - Sid: CZTier1TrustedAdvisorAndHealth20260420 + Effect: Allow + Action: + - health:Describe* + - support:DescribeTrustedAdvisor* + Resource: "*" + - Sid: CZTier1ContainerOrchestration20260511 + Effect: Allow + Action: + - ecs:Describe* + - ecs:List* + - eks:Describe* + - eks:List* + Resource: "*" + - Sid: CZTier1ContainerInsightsLogs20260420 + Effect: Allow + Action: + - logs:Describe* + - logs:Filter* + - logs:Get* + - logs:List* + - logs:StartQuery + - logs:StopQuery + Resource: arn:aws:logs:*:*:log-group:/aws/containerinsights/* + - Sid: CZTier1ContainerInsightsLogQuery20260420 + Effect: Allow + Action: + - logs:DescribeLogGroups + - logs:GetQueryResults + Resource: arn:aws:logs:*:*:log-group:* + - Sid: CZTier2CloudTrail20260420 + Effect: Allow + Action: + - cloudtrail:Describe* + - cloudtrail:Get* + - cloudtrail:List* + - cloudtrail:LookupEvents + Resource: "*" + - Sid: CZTier2ServiceQuotas20260420 + Effect: Allow + Action: + - servicequotas:Get* + - servicequotas:List* + Resource: "*" + - Sid: CZTier2CloudWatchMetrics20260420 + Effect: Allow + Action: + - autoscaling:Describe* + - cloudwatch:Describe* + - cloudwatch:Get* + - cloudwatch:List* + Resource: "*" + - Sid: CZTier2CloudFormation20260420 + Effect: Allow + Action: + - cloudformation:Describe* + - cloudformation:Get* + - cloudformation:List* + Resource: "*" + - Sid: CZTier2SelfInspection20260420 + Effect: Allow + Action: + - iam:GetRole + - iam:GetRolePolicy + - iam:ListAttachedRolePolicies + - iam:ListRolePolicies + - iam:ListRoleTags + - iam:SimulatePrincipalPolicy + Resource: arn:aws:iam::*:role/cloudzero/* + - Sid: CZTier2SelfInspectionPolicies20260420 + Effect: Allow + Action: + - iam:GetPolicy + - iam:GetPolicyVersion + Resource: + - arn:aws:iam::aws:policy/AWSBillingReadOnlyAccess + - arn:aws:iam::aws:policy/CloudWatchReadOnlyAccess + - arn:aws:iam::aws:policy/ComputeOptimizerReadOnlyAccess + - arn:aws:iam::aws:policy/job-function/ViewOnlyAccess + +Outputs: + ResourceOwnerRoleArn: + Description: Resource Owner Cross Account Role ARN + Value: !If [ResourceOwnerCreateResources, !GetAtt ResourceOwnerRole.Arn, 'null'] + MasterPayerRoleArn: + Description: Master Payer Cross Account Role ARN + Value: !If [MasterPayerCreateRoleAndPolicy, !GetAtt MasterPayerRole.Arn, 'null'] + MasterPayerReportS3Bucket: + Description: The CUR bucket if one was created + Value: !If [MasterPayerCreateNativeCur, !Ref MasterPayerCurBucket, 'null'] + MasterPayerReportS3Prefix: + Description: The CUR prefix if a report was created + Value: !If [MasterPayerCreateNativeCur, 'cloudzero/cloudzero-cur-hourly-csv', 'null'] + MasterPayerCurStatus: + Description: Whether a new CUR was created, and why not when skipped. + Value: !If + - MasterPayerCreateNativeCur + - 'created' + - !If + - MasterPayerCreateCurAndBucketAndPolicy + - 'skipped: master-payer stack must be deployed in us-east-1 to create a CUR' + - 'not-applicable' diff --git a/services/account_type/audit.yaml b/services/account_type/audit.yaml deleted file mode 100644 index a99b878..0000000 --- a/services/account_type/audit.yaml +++ /dev/null @@ -1,78 +0,0 @@ -# -*- coding: utf-8 -*- -# Copyright (c) 2016-present, CloudZero, Inc. All rights reserved. -# Licensed under the BSD-style license. See LICENSE file in the project root for full license information. - -AWSTemplateFormatVersion: '2010-09-09' -Description: CloudZero Audit Account Template - - -Parameters: - IsAuditAccount: - Description: Flag to indicate if this is the master payer account - Type: String - AllowedValues: - - 'true' - - 'false' - AuditCloudTrailBucketName: - Description: The name of the S3 bucket responsible for CloudTrail event data - Type: String - ReactorAccountId: - Description: The CloudZero reactor AWS account ID - Type: String - Default: '061190967865' - ExternalId: - Description: | - Unique ExternalId for Customer Organization; for cross-account Role Access and - associating this template with a Customer Organization - Type: String - - -Conditions: - CreateResources: !And - - !Not - - !Equals [ !Ref AuditCloudTrailBucketName, 'null' ] - - !Equals [ !Ref IsAuditAccount, 'true' ] - - -Resources: - - Role: - Condition: CreateResources - Type: 'AWS::IAM::Role' - Properties: - Path: '/cloudzero/' - AssumeRolePolicyDocument: - Version: '2012-10-17' - Statement: - - Effect: Allow - Principal: - AWS: !Sub 'arn:aws:iam::${ReactorAccountId}:root' - Action: - - 'sts:AssumeRole' - Condition: - StringEquals: - 'sts:ExternalId': !Ref ExternalId - - RolePolicy: - Condition: CreateResources - Type: 'AWS::IAM::Policy' - Properties: - PolicyName: !Sub 'cloudzero-audit-policy-${ReactorAccountId}' - Roles: - - !Ref Role - PolicyDocument: - Version: '2012-10-17' - Statement: - - Sid: AccessAuditCloudTrailBucket - Effect: Allow - Action: - - 's3:Get*' - - 's3:List*' - Resource: - - !Sub 'arn:aws:s3:::${AuditCloudTrailBucketName}' - - !Sub 'arn:aws:s3:::${AuditCloudTrailBucketName}/*' - -Outputs: - RoleArn: - Description: The cloudzero cross account role ARN - Value: !If [ CreateResources, !GetAtt Role.Arn, 'null' ] diff --git a/services/account_type/cloudtrail_owner.yaml b/services/account_type/cloudtrail_owner.yaml deleted file mode 100644 index 41a32d4..0000000 --- a/services/account_type/cloudtrail_owner.yaml +++ /dev/null @@ -1,85 +0,0 @@ -AWSTemplateFormatVersion: '2010-09-09' - - -Description: CloudZero CloudTrail Owner Template - - -Parameters: - IsCloudTrailOwnerAccount: - Description: Flag that indicates if this account owns the cloudtrail - Type: String - AllowedValues: - - 'true' - - 'false' - CloudTrailSNSTopicArn: - Description: 'The CloudZero SNS Topic ARN responsible for receiving notifications from the cloudtrail' - Type: String - ReactorAccountId: - Description: The CloudZero reactor AWS account ID - Type: String - Default: '061190967865' - - -Conditions: - CreateResources: !And [ !Not [ !Equals [ !Ref CloudTrailSNSTopicArn, 'null' ] ], !Equals [ !Ref IsCloudTrailOwnerAccount, 'true' ] ] - - -Resources: - - SqsQueue: - Type: 'AWS::SQS::Queue' - Condition: CreateResources - Properties: - Tags: - - Key: cloudzero-stack - Value: !Ref AWS::StackName - - Key: cloudzero-reactor-account-id - Value: !Ref ReactorAccountId - - SnsSubscription: - Type: AWS::SNS::Subscription - Condition: CreateResources - Properties: - Protocol: sqs - Endpoint: !GetAtt SqsQueue.Arn - Region: !Select - - 3 - - !Split [ ':', !Ref CloudTrailSNSTopicArn ] - TopicArn: !Ref CloudTrailSNSTopicArn - - SqsPolicy: - Type: 'AWS::SQS::QueuePolicy' - Condition: CreateResources - Properties: - Queues: - - !Ref SqsQueue - PolicyDocument: - Id: 'CloudZeroReactorQueuePolicy20190816' - Version: '2012-10-17' - Statement: - - Sid: 'SNSTopicAccess20190816' - Effect: Allow - Action: - - 'sqs:SendMessage' - Principal: - AWS: "*" - Resource: "*" - Condition: - ArnEquals: - 'aws:SourceArn': !Ref CloudTrailSNSTopicArn - - Sid: 'ReactorAccess20190816' - Effect: Allow - Action: - - 'sqs:*' - Principal: - AWS: !Sub 'arn:aws:iam::${ReactorAccountId}:root' - Resource: !GetAtt SqsQueue.Arn - - -Outputs: - SQSQueueArn: - Description: The CloudZero SQS queue - Value: !If [ CreateResources, !GetAtt SqsQueue.Arn, 'null' ] - SQSQueuePolicyName: - Description: The CloudZero SQS queue policy - Value: !If [ CreateResources, !Ref SqsPolicy, 'null' ] diff --git a/services/account_type/master_payer.yaml b/services/account_type/master_payer.yaml deleted file mode 100644 index 00b5502..0000000 --- a/services/account_type/master_payer.yaml +++ /dev/null @@ -1,471 +0,0 @@ -# -*- coding: utf-8 -*- -# Copyright (c) 2016-present, CloudZero, Inc. All rights reserved. -# Licensed under the BSD-style license. See LICENSE file in the project root for full license information. - -AWSTemplateFormatVersion: '2010-09-09' - - -Description: CloudZero Master Payer Template - - -Parameters: - IsMasterPayerAccount: - Description: Flag to indicate if this is the master payer account, i.e. existing valid CUR - Type: String - AllowedValues: - - 'true' - - 'false' - IsOrganizationMasterAccount: - Description: Flag to indicate if this is an organization master account - Type: String - AllowedValues: - - 'true' - - 'false' - MasterPayerBillingBucketName: - Description: The name of the S3 bucket responsible for billing data - Type: String - MasterPayerBillingBucketArns: - Description: | - Comma-separated list of S3 ARNs (both bucket and bucket/* forms) for every - locally-owned S3 bucket referenced by any CUR report in this account (valid - or not). Empty only when no CUR report points to a locally-owned bucket. - Must be non-empty whenever MasterPayerBillingBucketName is provided. - Type: String - Default: '' - ReactorAccountId: - Description: The CloudZero reactor AWS account ID - Type: String - Default: '061190967865' - ConnectorsAccountId: - Description: Additional CloudZero AWS account ID that will assume the cross-account role. - Type: String - Default: '559846027439' - ExternalId: - Description: | - Unique ExternalId for Customer Organization; for cross-account Role Access and - associating this template with a Customer Organization - Type: String - -Conditions: - ValidExistingBucketName: !Not [ !Equals [ !Ref MasterPayerBillingBucketName, 'null' ] ] - HasMasterPayerBillingBucketArns: !Not [ !Equals [ !Ref MasterPayerBillingBucketArns, '' ] ] - CreateCurAndBucketAndPolicy: !And - - !Not - - Condition: ValidExistingBucketName - - !Equals [ !Ref IsMasterPayerAccount, 'true' ] - CreatePolicyForExistingCur: !And - - Condition: ValidExistingBucketName - - !Equals [ !Ref IsMasterPayerAccount, 'true' ] - CreateRoleAndPolicy: !Or - - Condition: CreatePolicyForExistingCur - - Condition: CreateCurAndBucketAndPolicy - -Mappings: - Defaults: - Cur: - BucketName: 'cz-cur-hourly-csv' - -Resources: - Role: - Condition: CreateRoleAndPolicy - Type: 'AWS::IAM::Role' - Properties: - Path: '/cloudzero/' - AssumeRolePolicyDocument: - Version: '2012-10-17' - Statement: - - Sid: czServiceAccount01 - Effect: Allow - Principal: - AWS: - - !Sub 'arn:aws:iam::${ReactorAccountId}:root' - - !Sub 'arn:aws:iam::${ConnectorsAccountId}:root' - Action: - - 'sts:AssumeRole' - Condition: - StringEquals: - 'sts:ExternalId': !Ref ExternalId - ManagedPolicyArns: - - arn:aws:iam::aws:policy/CloudWatchReadOnlyAccess - - arn:aws:iam::aws:policy/ComputeOptimizerReadOnlyAccess - - arn:aws:iam::aws:policy/job-function/ViewOnlyAccess - - arn:aws:iam::aws:policy/AWSBillingReadOnlyAccess - - CostAndUsageReportResource: - Condition: CreateCurAndBucketAndPolicy - Type: 'Custom::CostAndUsageReport' - Properties: - ServiceToken: !GetAtt CostAndUsageReportLambda.Arn - Region: !Ref AWS::Region - BucketName: !Sub - - '${BucketName}-${Id}' - - { BucketName: !FindInMap [ Defaults, Cur, BucketName ], - Id: !Select [ 2, !Split [ '/', !Ref 'AWS::StackId' ] ] } - - CostAndUsageReportRole: - Condition: CreateCurAndBucketAndPolicy - Type: 'AWS::IAM::Role' - Properties: - AssumeRolePolicyDocument: - Version: 2012-10-17 - Statement: - - Effect: Allow - Principal: - Service: lambda.amazonaws.com - Action: sts:AssumeRole - Policies: - - PolicyName: CurAccess - PolicyDocument: - Version: 2012-10-17 - Statement: - - Sid: AllowLogging - Effect: Allow - Action: - - 'logs:CreateLogGroup' - - 'logs:CreateLogStream' - - 'logs:PutLogEvents' - Resource: '*' - - Sid: AllowCreateCur - Effect: Allow - Action: - - 'cur:*' - Resource: '*' - - Sid: AllowVerifyCurBucket - Effect: Allow - Action: - - 's3:*' - Resource: - - !Sub - - 'arn:aws:s3:::${BucketName}-${Id}' - - { BucketName: !FindInMap [ Defaults, Cur, BucketName ], - Id: !Select [ 2, !Split [ '/', !Ref 'AWS::StackId' ] ] } - - !Sub - - 'arn:aws:s3:::${BucketName}-${Id}/*' - - { BucketName: !FindInMap [ Defaults, Cur, BucketName ], - Id: !Select [ 2, !Split [ '/', !Ref 'AWS::StackId' ] ] } - - # NOTE: This is an inline lambda function b/c we want it to be easy for you - # to see the resource we're creating, in this case a Cost and Usage report. - # We always prefer pure CFN to custom resources; however, CFN currently does - # not support creating a CUR. - CostAndUsageReportLambda: - Condition: CreateCurAndBucketAndPolicy - Type: 'AWS::Lambda::Function' - Properties: - Description: Create/Delete Cost and Usage Report - Handler: index.handler - Runtime: python3.12 - Role: !GetAtt CostAndUsageReportRole.Arn - Timeout: 120 - Code: - ZipFile: | - import boto3 - import cfnresponse - import json - import logging - import time - - cur = boto3.client('cur', region_name='us-east-1') # cur is only in us-east-1 - logger = logging.getLogger() - logger.setLevel(logging.INFO) - - def handler(event, context): - result = cfnresponse.SUCCESS - report_name = 'cloudzero-cur-hourly-csv' - s3_prefix = 'cloudzero' - report_s3_prefix = f'{s3_prefix}/{report_name}' - - try: - logger.info(f'Received event: {event}') - bucket_name = event['ResourceProperties']['BucketName'] - current_region = event['ResourceProperties']['Region'] - valid_regions = {'us-west-1','us-west-2','ap-northeast-1','ap-southeast-1', - 'eu-west-1','us-east-1','ap-southeast-2','eu-central-1'} - region = current_region if current_region in valid_regions else 'us-east-1' - s3 = boto3.resource('s3', region_name=region) - bucket = s3.Bucket(bucket_name) - - if event['RequestType'] == 'Create': - logger.info(f'Creating {bucket_name} in {region}') - bucket.create(**({} if region == 'us-east-1' else {'CreateBucketConfiguration': {'LocationConstraint': region}})) - bucket.wait_until_exists() - policy = { - 'Version': '2012-10-17', - 'Statement': [ - { - 'Sid': 'AddBillReportsGet', - 'Effect': 'Allow', - 'Principal': { - 'Service': 'billingreports.amazonaws.com' - }, - 'Action': [ - 's3:GetBucketAcl', - 's3:GetBucketPolicy' - ], - 'Resource': f'arn:aws:s3:::{bucket_name}' - }, - { - 'Sid': 'AddBillReportsPut', - 'Effect': 'Allow', - 'Principal': { - 'Service': 'billingreports.amazonaws.com' - }, - 'Action': 's3:PutObject', - 'Resource': f'arn:aws:s3:::{bucket_name}/*' - } - ] - } - for attempt in range(5): - try: - logger.info(f'Adding bucket policy to {bucket_name}') - bucket.Policy().put(Policy=json.dumps(policy)) - except: - time.sleep(1) - else: - break - - for attempt in range(5): - try: - logger.info(f'Creating {report_name}') - cur.put_report_definition( - ReportDefinition={ - 'AdditionalSchemaElements': ['RESOURCES'], - 'Compression': 'GZIP', 'Format': 'textORcsv', - 'RefreshClosedReports': True, - 'ReportName': report_name, - 'ReportVersioning': 'CREATE_NEW_REPORT', - 'S3Bucket': bucket_name, 'S3Prefix': s3_prefix, - 'S3Region': region, 'TimeUnit': 'HOURLY', - } - ) - except: - time.sleep(1) - else: - break - elif event['RequestType'] == 'Update': - logger.warning(f'Update is unsupported') - elif event['RequestType'] == 'Delete': - logger.warning(f'We do not want to delete your billing data.') - except Exception: - logger.error('Failed to Update Cur Resource', exc_info=True) - result = cfnresponse.FAILED - finally: - cfnresponse.send(event, context, result, {'ReportS3Bucket': bucket_name, 'ReportS3Prefix': report_s3_prefix}) - - - - - RolePolicy: - Condition: CreateRoleAndPolicy - Type: 'AWS::IAM::Policy' - Properties: - PolicyName: !Sub 'cloudzero-master-payer-policy-${ReactorAccountId}' - Roles: - - !Ref Role - PolicyDocument: - Version: '2012-10-17' - Statement: - - Sid: CZTier0BillingBucket20260420 - Effect: Allow - Action: - - s3:Get* - - s3:List* - # The discovery Lambda emits MasterPayerBillingBucketArns as a comma-separated - # ARN list covering every locally-owned S3 bucket referenced by any CUR report - # (both `bucket` and `bucket/*` forms). When ARNs is empty (e.g., the stack was - # invoked directly with only MasterPayerBillingBucketName), we fall back to the - # legacy single-bucket pair so !Split never sees an empty string (which would - # otherwise yield [''] and produce an invalid IAM Resource). When this stack is - # also creating a brand-new CUR bucket, discovery found no existing CURs and - # falls through to the new-bucket pair below. - Resource: !Split - - ',' - - !If - - CreatePolicyForExistingCur - - !If - - HasMasterPayerBillingBucketArns - - !Ref MasterPayerBillingBucketArns - - !Sub 'arn:aws:s3:::${MasterPayerBillingBucketName},arn:aws:s3:::${MasterPayerBillingBucketName}/*' - - !Sub - - 'arn:aws:s3:::${BucketName}-${Id},arn:aws:s3:::${BucketName}-${Id}/*' - - { BucketName: !FindInMap [ Defaults, Cur, BucketName ], - Id: !Select [ 2, !Split [ '/', !Ref 'AWS::StackId' ] ] } - - Sid: CZTier0BillingDataAccess20260420 - Effect: Allow - Action: - - account:GetAccountInformation - - bcm-data-exports:Get* - - bcm-data-exports:List* - - billing:Get* - - ce:Describe* - - ce:Get* - - ce:List* - - consolidatedbilling:Get* - - consolidatedbilling:List* - - cur:Describe* - - cur:Get* - - cur:List* - - cur:Validate* - - freetier:Get* - - invoicing:Get* - - invoicing:List* - - mapcredits:List* - - payments:Get* - - payments:List* - - purchase-orders:Get* - - purchase-orders:List* - - purchase-orders:View* - - sustainability:Get* - - tax:Get* - - tax:List* - Resource: "*" - - Sid: CZTier0ReservedCapacity20260420 - Effect: Allow - Action: - - dynamodb:DescribeReserved* - - ec2:DescribeReserved* - - elasticache:DescribeReserved* - - es:DescribeReserved* - - rds:DescribeReserved* - - redshift:DescribeReserved* - Resource: "*" - - Sid: CZTier0SavingsPlans20260420 - Effect: Allow - Action: - - savingsplans:Describe* - - savingsplans:List* - Resource: "*" - - Sid: CZTier1Budgets20260420 - Effect: Allow - Action: - - budgets:Describe* - - budgets:View* - Resource: "*" - - Sid: CZTier1BillingConductor20260420 - Effect: Allow - Action: - - billingconductor:Get* - - billingconductor:List* - Resource: "*" - - Sid: CZTier1PricingAndForecasting20260420 - Effect: Allow - Action: - - bcm-pricing-calculator:Get* - - bcm-pricing-calculator:List* - - pricing:* - Resource: "*" - - Sid: CZTier1OrganizationsAndTags20260420 - Effect: Allow - Action: - - account:ListRegions - - organizations:Describe* - - organizations:List* - - resource-explorer:List* - - resource-groups:Get* - - resource-groups:List* - - resource-groups:Search* - - tag:Describe* - - tag:Get* - Resource: "*" - - Sid: CZTier1ComputeOptimizer20260420 - Effect: Allow - Action: - - compute-optimizer:Describe* - - compute-optimizer:Get* - Resource: "*" - - Sid: CZTier1CostOptimizationHub20260420 - Effect: Allow - Action: - - cost-optimization-hub:Get* - - cost-optimization-hub:List* - Resource: "*" - - Sid: CZTier1TrustedAdvisorAndHealth20260420 - Effect: Allow - Action: - - health:Describe* - - support:DescribeTrustedAdvisor* - Resource: "*" - - Sid: CZTier1ContainerOrchestration20260511 - Effect: Allow - Action: - - ecs:Describe* - - ecs:List* - - eks:Describe* - - eks:List* - Resource: "*" - - Sid: CZTier1ContainerInsightsLogs20260420 - Effect: Allow - Action: - - logs:Describe* - - logs:Filter* - - logs:Get* - - logs:List* - - logs:StartQuery - - logs:StopQuery - Resource: arn:aws:logs:*:*:log-group:/aws/containerinsights/* - - Sid: CZTier1ContainerInsightsLogQuery20260420 - Effect: Allow - Action: - - logs:DescribeLogGroups - - logs:GetQueryResults - Resource: arn:aws:logs:*:*:log-group:* - - Sid: CZTier2CloudTrail20260420 - Effect: Allow - Action: - - cloudtrail:Describe* - - cloudtrail:Get* - - cloudtrail:List* - - cloudtrail:LookupEvents - Resource: "*" - - Sid: CZTier2ServiceQuotas20260420 - Effect: Allow - Action: - - servicequotas:Get* - - servicequotas:List* - Resource: "*" - - Sid: CZTier2CloudWatchMetrics20260420 - Effect: Allow - Action: - - autoscaling:Describe* - - cloudwatch:Describe* - - cloudwatch:Get* - - cloudwatch:List* - Resource: "*" - - Sid: CZTier2CloudFormation20260420 - Effect: Allow - Action: - - cloudformation:Describe* - - cloudformation:Get* - - cloudformation:List* - Resource: "*" - - Sid: CZTier2SelfInspection20260420 - Effect: Allow - Action: - - iam:GetRole - - iam:GetRolePolicy - - iam:ListAttachedRolePolicies - - iam:ListRolePolicies - - iam:ListRoleTags - - iam:SimulatePrincipalPolicy - Resource: arn:aws:iam::*:role/cloudzero/* - - Sid: CZTier2SelfInspectionPolicies20260420 - Effect: Allow - Action: - - iam:GetPolicy - - iam:GetPolicyVersion - Resource: - - arn:aws:iam::aws:policy/AWSBillingReadOnlyAccess - - arn:aws:iam::aws:policy/CloudWatchReadOnlyAccess - - arn:aws:iam::aws:policy/ComputeOptimizerReadOnlyAccess - - arn:aws:iam::aws:policy/job-function/ViewOnlyAccess - -Outputs: - RoleArn: - Description: The cloudzero cross account role ARN - Value: !If [ CreateRoleAndPolicy, !GetAtt Role.Arn, 'null' ] - ReportS3Bucket: - Description: The CUR bucket if it was created - Value: !If [ CreateCurAndBucketAndPolicy, !GetAtt CostAndUsageReportResource.ReportS3Bucket, 'null' ] - ReportS3Prefix: - Description: The CUR bucket prefix if it was created - Value: !If [ CreateCurAndBucketAndPolicy, !GetAtt CostAndUsageReportResource.ReportS3Prefix, 'null' ] diff --git a/services/account_type/resource_owner.yaml b/services/account_type/resource_owner.yaml deleted file mode 100644 index 462c983..0000000 --- a/services/account_type/resource_owner.yaml +++ /dev/null @@ -1,238 +0,0 @@ -# -*- coding: utf-8 -*- -# Copyright (c) 2016-present, CloudZero, Inc. All rights reserved. -# Licensed under the BSD-style license. See LICENSE file in the project root for full license information. - -AWSTemplateFormatVersion: '2010-09-09' -Description: CloudZero Resource Owner Account Template - -Parameters: - ExternalId: - Type: String - Description: | - Unique ExternalId for Customer Organization; for cross-account Role Access and - associating this template with a Customer Organization - ReactorAccountId: - Type: String - Description: | - CloudZero AWS AccountID; for cross-account Role Access - ConnectorsAccountId: - Description: Additional CloudZero AWS account ID that will assume the cross-account role. - Type: String - Default: '559846027439' - IsResourceOwnerAccount: - Type: String - Description: | - Should this template instantiate any resources? - -Conditions: - CreateResources: !Equals [ !Ref IsResourceOwnerAccount, 'true' ] - -Resources: - Role: - Type: AWS::IAM::Role - Condition: CreateResources - Properties: - Path: '/cloudzero/' - AssumeRolePolicyDocument: - Version: 2012-10-17 - Statement: - - Effect: Allow - Principal: - AWS: - - !Sub 'arn:aws:iam::${ReactorAccountId}:root' - - !Sub 'arn:aws:iam::${ConnectorsAccountId}:root' - Action: - - 'sts:AssumeRole' - Condition: - StringEquals: - 'sts:ExternalId': !Ref ExternalId - ManagedPolicyArns: - - arn:aws:iam::aws:policy/CloudWatchReadOnlyAccess - - arn:aws:iam::aws:policy/ComputeOptimizerReadOnlyAccess - - arn:aws:iam::aws:policy/job-function/ViewOnlyAccess - - arn:aws:iam::aws:policy/AWSBillingReadOnlyAccess - - RolePolicy: - Type: AWS::IAM::Policy - Condition: CreateResources - Properties: - PolicyName: !Sub 'cloudzero-resource-owner-policy-${ReactorAccountId}' - PolicyDocument: - Version: 2012-10-17 - Statement: - - Sid: CZTier0BillingDataAccess20260420 - Effect: Allow - Action: - - account:GetAccountInformation - - bcm-data-exports:Get* - - bcm-data-exports:List* - - billing:Get* - - ce:Describe* - - ce:Get* - - ce:List* - - consolidatedbilling:Get* - - consolidatedbilling:List* - - cur:Describe* - - cur:Get* - - cur:List* - - cur:Validate* - - freetier:Get* - - invoicing:Get* - - invoicing:List* - - mapcredits:List* - - payments:Get* - - payments:List* - - purchase-orders:Get* - - purchase-orders:List* - - purchase-orders:View* - - sustainability:Get* - - tax:Get* - - tax:List* - Resource: "*" - - Sid: CZTier0ReservedCapacity20260420 - Effect: Allow - Action: - - dynamodb:DescribeReserved* - - ec2:DescribeReserved* - - elasticache:DescribeReserved* - - es:DescribeReserved* - - rds:DescribeReserved* - - redshift:DescribeReserved* - Resource: "*" - - Sid: CZTier0SavingsPlans20260420 - Effect: Allow - Action: - - savingsplans:Describe* - - savingsplans:List* - Resource: "*" - - Sid: CZTier1Budgets20260420 - Effect: Allow - Action: - - budgets:Describe* - - budgets:View* - Resource: "*" - - Sid: CZTier1BillingConductor20260420 - Effect: Allow - Action: - - billingconductor:Get* - - billingconductor:List* - Resource: "*" - - Sid: CZTier1PricingAndForecasting20260420 - Effect: Allow - Action: - - bcm-pricing-calculator:Get* - - bcm-pricing-calculator:List* - - pricing:* - Resource: "*" - - Sid: CZTier1OrganizationsAndTags20260420 - Effect: Allow - Action: - - account:ListRegions - - organizations:Describe* - - organizations:List* - - resource-explorer:List* - - resource-groups:Get* - - resource-groups:List* - - resource-groups:Search* - - tag:Describe* - - tag:Get* - Resource: "*" - - Sid: CZTier1ComputeOptimizer20260420 - Effect: Allow - Action: - - compute-optimizer:Describe* - - compute-optimizer:Get* - Resource: "*" - - Sid: CZTier1CostOptimizationHub20260420 - Effect: Allow - Action: - - cost-optimization-hub:Get* - - cost-optimization-hub:List* - Resource: "*" - - Sid: CZTier1TrustedAdvisorAndHealth20260420 - Effect: Allow - Action: - - health:Describe* - - support:DescribeTrustedAdvisor* - Resource: "*" - - Sid: CZTier1ContainerOrchestration20260511 - Effect: Allow - Action: - - ecs:Describe* - - ecs:List* - - eks:Describe* - - eks:List* - Resource: "*" - - Sid: CZTier1ContainerInsightsLogs20260420 - Effect: Allow - Action: - - logs:Describe* - - logs:Filter* - - logs:Get* - - logs:List* - - logs:StartQuery - - logs:StopQuery - Resource: arn:aws:logs:*:*:log-group:/aws/containerinsights/* - - Sid: CZTier1ContainerInsightsLogQuery20260420 - Effect: Allow - Action: - - logs:DescribeLogGroups - - logs:GetQueryResults - Resource: arn:aws:logs:*:*:log-group:* - - Sid: CZTier2CloudTrail20260420 - Effect: Allow - Action: - - cloudtrail:Describe* - - cloudtrail:Get* - - cloudtrail:List* - - cloudtrail:LookupEvents - Resource: "*" - - Sid: CZTier2ServiceQuotas20260420 - Effect: Allow - Action: - - servicequotas:Get* - - servicequotas:List* - Resource: "*" - - Sid: CZTier2CloudWatchMetrics20260420 - Effect: Allow - Action: - - autoscaling:Describe* - - cloudwatch:Describe* - - cloudwatch:Get* - - cloudwatch:List* - Resource: "*" - - Sid: CZTier2CloudFormation20260420 - Effect: Allow - Action: - - cloudformation:Describe* - - cloudformation:Get* - - cloudformation:List* - Resource: "*" - - Sid: CZTier2SelfInspection20260420 - Effect: Allow - Action: - - iam:GetRole - - iam:GetRolePolicy - - iam:ListAttachedRolePolicies - - iam:ListRolePolicies - - iam:ListRoleTags - - iam:SimulatePrincipalPolicy - Resource: arn:aws:iam::*:role/cloudzero/* - - Sid: CZTier2SelfInspectionPolicies20260420 - Effect: Allow - Action: - - iam:GetPolicy - - iam:GetPolicyVersion - Resource: - - arn:aws:iam::aws:policy/AWSBillingReadOnlyAccess - - arn:aws:iam::aws:policy/CloudWatchReadOnlyAccess - - arn:aws:iam::aws:policy/ComputeOptimizerReadOnlyAccess - - arn:aws:iam::aws:policy/job-function/ViewOnlyAccess - Roles: - - !Ref Role - -Outputs: - RoleArn: - Condition: CreateResources - Value: !GetAtt Role.Arn - Description: Resource Owner Cross Account Role ARN diff --git a/services/connected_account.yaml b/services/connected_account.yaml index c566220..81a5fc7 100644 --- a/services/connected_account.yaml +++ b/services/connected_account.yaml @@ -42,6 +42,12 @@ Parameters: Default: 'latest' Description: | Version to target when deploying the stack. `latest` should be used by default. + Stage: + Type: String + Default: 'prod' + AllowedValues: ['prod', 'dev'] + Description: | + Which CloudZero environment to link to. Customers should leave this as `prod`. Mappings: CallbackConfiguration: @@ -51,148 +57,166 @@ Mappings: ConnectorsAccountId: '559846027439' ReactorId: 'f7a07d02-d509-4e8d-9fb9-69c7985a6bd8' ReactorCallbackUrl: 'https://api.cloudzero.com/accounts/v1/link' + dev: + ReactorAccountId: '998146006915' + ConnectorsAccountId: '483772923246' + ReactorId: '67ad3e83-1ec5-4f4f-b505-c27e2758b990' + ReactorCallbackUrl: 'https://qc8rbx9mcg.execute-api.us-east-1.amazonaws.com/alfa/v1/link' Conditions: ValidReactorCallbackUrl: !Not - - !Equals [!FindInMap [CallbackConfiguration, prod, ReactorCallbackUrl], 'null'] + - !Equals [!FindInMap [CallbackConfiguration, !Ref Stage, ReactorCallbackUrl], 'null'] Resources: - Discovery: - Type: AWS::CloudFormation::Stack + # --------------------------------------------------------------------------- + # Discovery: inspect this account at deploy time. Inlined here (previously a + # nested stack) so its outputs can be passed as parameters into the single + # AccountResources stack -- CloudFormation Conditions cannot read custom-resource + # outputs directly, so parameterizing them through a nested stack is required. + # --------------------------------------------------------------------------- + DiscoveryFunctionRole: + Type: AWS::IAM::Role Properties: - Tags: - - Key: cloudzero-stack - Value: !Ref AWS::StackName - - Key: cloudzero-reactor-account-id - Value: !FindInMap [CallbackConfiguration, prod, ReactorAccountId] - TemplateURL: !Sub https://s3.amazonaws.com/cz-provision-account/${Version}/services/discovery.yaml - Parameters: - Version: !Sub ${Version} + AssumeRolePolicyDocument: + Version: '2012-10-17' + Statement: + - Effect: Allow + Principal: + Service: lambda.amazonaws.com + Action: sts:AssumeRole + ManagedPolicyArns: + - arn:aws:iam::aws:policy/service-role/AWSLambdaBasicExecutionRole + Policies: + - PolicyName: CZDiscovery + PolicyDocument: + Version: '2012-10-17' + Statement: + - Sid: CZDiscovery20190912 + Effect: Allow + Action: + - s3:ListAllMyBuckets + - cur:DescribeReportDefinitions + - bcm-data-exports:ListExports + - bcm-data-exports:GetExport + - organizations:DescribeOrganization + Resource: '*' - ResourceOwnerAccount: - Type: AWS::CloudFormation::Stack + DiscoveryFunction: + Type: AWS::Lambda::Function Properties: - Tags: - - Key: cloudzero-stack - Value: !Ref AWS::StackName - - Key: cloudzero-reactor-account-id - Value: !FindInMap [CallbackConfiguration, prod, ReactorAccountId] - Parameters: - IsResourceOwnerAccount: !GetAtt Discovery.Outputs.IsResourceOwnerAccount - ExternalId: !Ref ExternalId - ReactorAccountId: !FindInMap [CallbackConfiguration, prod, ReactorAccountId] - ConnectorsAccountId: !FindInMap [CallbackConfiguration, prod, ConnectorsAccountId] - TemplateURL: !Sub https://s3.amazonaws.com/cz-provision-account/${Version}/services/account_type/resource_owner.yaml + Code: + S3Bucket: !Sub 'cz-provision-account-${AWS::Region}' + S3Key: !Sub ${Version}/services/discovery.zip + Handler: src.app.handler + Runtime: python3.12 + MemorySize: 512 + Timeout: 30 + Role: !GetAtt DiscoveryFunctionRole.Arn + Environment: + Variables: + VERSION: '20230523' - CloudTrailOwnerAccount: - Type: AWS::CloudFormation::Stack + DiscoveryResource: + Type: Custom::Discovery Properties: - Tags: - - Key: cloudzero-stack - Value: !Ref AWS::StackName - - Key: cloudzero-reactor-account-id - Value: !FindInMap [CallbackConfiguration, prod, ReactorAccountId] - Parameters: - IsCloudTrailOwnerAccount: !GetAtt Discovery.Outputs.IsCloudTrailOwnerAccount - CloudTrailSNSTopicArn: !GetAtt Discovery.Outputs.CloudTrailSNSTopicArn - ReactorAccountId: !FindInMap [CallbackConfiguration, prod, ReactorAccountId] - TemplateURL: !Sub https://s3.amazonaws.com/cz-provision-account/${Version}/services/account_type/cloudtrail_owner.yaml + ServiceToken: !GetAtt DiscoveryFunction.Arn + AccountId: !Ref AWS::AccountId + Version: !Sub ${Version} - AuditAccount: + # --------------------------------------------------------------------------- + # The single remaining nested stack: provisions resources for whichever account + # type Discovery detected (resource-owner and/or master-payer). Discovery outputs + # are passed in as parameters so the nested stack's Conditions can gate on them. + # --------------------------------------------------------------------------- + AccountResources: Type: AWS::CloudFormation::Stack Properties: Tags: - Key: cloudzero-stack Value: !Ref AWS::StackName - Key: cloudzero-reactor-account-id - Value: !FindInMap [CallbackConfiguration, prod, ReactorAccountId] + Value: !FindInMap [CallbackConfiguration, !Ref Stage, ReactorAccountId] + TemplateURL: !Sub https://s3.amazonaws.com/cz-provision-account/${Version}/services/account_type/account_resources.yaml Parameters: - IsAuditAccount: !GetAtt Discovery.Outputs.IsAuditAccount - AuditCloudTrailBucketName: !GetAtt Discovery.Outputs.AuditCloudTrailBucketName ExternalId: !Ref ExternalId - ReactorAccountId: !FindInMap [CallbackConfiguration, prod, ReactorAccountId] - TemplateURL: !Sub https://s3.amazonaws.com/cz-provision-account/${Version}/services/account_type/audit.yaml + ReactorAccountId: !FindInMap [CallbackConfiguration, !Ref Stage, ReactorAccountId] + ConnectorsAccountId: !FindInMap [CallbackConfiguration, !Ref Stage, ConnectorsAccountId] + IsResourceOwnerAccount: !GetAtt DiscoveryResource.IsResourceOwnerAccount + IsOrganizationMasterAccount: !GetAtt DiscoveryResource.IsOrganizationMasterAccount + IsMasterPayerAccount: !GetAtt DiscoveryResource.IsMasterPayerAccount + MasterPayerBillingBucketName: !GetAtt DiscoveryResource.MasterPayerBillingBucketName + MasterPayerBillingBucketArns: !GetAtt DiscoveryResource.MasterPayerBillingBucketArns - MasterPayerAccount: - Type: AWS::CloudFormation::Stack + # --------------------------------------------------------------------------- + # Notification: POST the provisioning result back to the CloudZero reactor. + # Inlined (previously a nested stack). Receives every value it needs as direct + # properties below, so it no longer reads sibling stack outputs at runtime. + # --------------------------------------------------------------------------- + NotificationFunctionRole: + Type: AWS::IAM::Role + Condition: ValidReactorCallbackUrl Properties: - Tags: - - Key: cloudzero-stack - Value: !Ref AWS::StackName - - Key: cloudzero-reactor-account-id - Value: !FindInMap [CallbackConfiguration, prod, ReactorAccountId] - Parameters: - IsOrganizationMasterAccount: !GetAtt Discovery.Outputs.IsOrganizationMasterAccount - IsMasterPayerAccount: !GetAtt Discovery.Outputs.IsMasterPayerAccount - MasterPayerBillingBucketName: !GetAtt Discovery.Outputs.MasterPayerBillingBucketName - MasterPayerBillingBucketArns: !GetAtt Discovery.Outputs.MasterPayerBillingBucketArns - ExternalId: !Ref ExternalId - ReactorAccountId: !FindInMap [CallbackConfiguration, prod, ReactorAccountId] - ConnectorsAccountId: !FindInMap [CallbackConfiguration, prod, ConnectorsAccountId] - TemplateURL: !Sub https://s3.amazonaws.com/cz-provision-account/${Version}/services/account_type/master_payer.yaml + AssumeRolePolicyDocument: + Version: '2012-10-17' + Statement: + - Effect: Allow + Principal: + Service: lambda.amazonaws.com + Action: sts:AssumeRole + ManagedPolicyArns: + - arn:aws:iam::aws:policy/service-role/AWSLambdaBasicExecutionRole - - NotificationFunctionStack: - Type: AWS::CloudFormation::Stack + NotificationFunction: + Type: AWS::Lambda::Function Condition: ValidReactorCallbackUrl Properties: - Tags: - - Key: cloudzero-stack - Value: !Ref AWS::StackName - - Key: cloudzero-reactor-account-id - Value: !FindInMap [CallbackConfiguration, prod, ReactorAccountId] - Parameters: - ReactorCallbackUrl: !FindInMap [CallbackConfiguration, prod, ReactorCallbackUrl] - Version: !Sub ${Version} - TemplateURL: !Sub https://s3.amazonaws.com/cz-provision-account/${Version}/services/notification.yaml - + Code: + S3Bucket: !Sub 'cz-provision-account-${AWS::Region}' + S3Key: !Sub ${Version}/services/notification.zip + Handler: src.app.handler + Runtime: python3.12 + MemorySize: 512 + Timeout: 30 + Role: !GetAtt NotificationFunctionRole.Arn + Environment: + Variables: + VERSION: '1' NotifyCloudZeroReactor: Type: Custom::NotifyCloudZero Condition: ValidReactorCallbackUrl Properties: - Tags: - - Key: cloudzero-stack - Value: !Ref AWS::StackName - - Key: cloudzero-reactor-account-id - Value: !FindInMap [CallbackConfiguration, prod, ReactorAccountId] - ServiceToken: !GetAtt NotificationFunctionStack.Outputs.Arn + ServiceToken: !GetAtt NotificationFunction.Arn + # Reactor / account metadata AccountId: !Ref AWS::AccountId - Version: !Sub ${Version} - # Parameters from Other Resources in this Stack + Region: !Ref AWS::Region ExternalId: !Ref ExternalId - ReactorCallbackUrl: !FindInMap [CallbackConfiguration, prod, ReactorCallbackUrl] AccountName: !Ref AccountName - Region: !Ref AWS::Region - ReactorId: !FindInMap [CallbackConfiguration, prod, ReactorId] - # References to other stacks; it's easier for _this_ Resource to simply grab the outputs - # programmatically b/c some of the outputs are conditional. - Stacks: - Discovery: !Ref Discovery - ResourceOwnerAccount: !Ref ResourceOwnerAccount - CloudTrailOwnerAccount: !Ref CloudTrailOwnerAccount - AuditAccount: !Ref AuditAccount - MasterPayerAccount: !Ref MasterPayerAccount - LegacyAccount: !Ref ResourceOwnerAccount + ReactorId: !FindInMap [CallbackConfiguration, !Ref Stage, ReactorId] + ReactorCallbackUrl: !FindInMap [CallbackConfiguration, !Ref Stage, ReactorCallbackUrl] + # Discovery flags + billing details (resolved from the Discovery custom resource) + IsResourceOwnerAccount: !GetAtt DiscoveryResource.IsResourceOwnerAccount + IsMasterPayerAccount: !GetAtt DiscoveryResource.IsMasterPayerAccount + IsOrganizationMasterAccount: !GetAtt DiscoveryResource.IsOrganizationMasterAccount + MasterPayerBillingBucketName: !GetAtt DiscoveryResource.MasterPayerBillingBucketName + MasterPayerBillingBucketPath: !GetAtt DiscoveryResource.MasterPayerBillingBucketPath + BillingReportFormat: !GetAtt DiscoveryResource.BillingReportFormat + # Provisioned outputs (resolved from the AccountResources nested stack) + ResourceOwnerRoleArn: !GetAtt AccountResources.Outputs.ResourceOwnerRoleArn + MasterPayerRoleArn: !GetAtt AccountResources.Outputs.MasterPayerRoleArn + MasterPayerReportS3Bucket: !GetAtt AccountResources.Outputs.MasterPayerReportS3Bucket + MasterPayerReportS3Prefix: !GetAtt AccountResources.Outputs.MasterPayerReportS3Prefix Outputs: - AuditAccount: - Value: !Ref AuditAccount - CloudTrailOwnerAccount: - Value: !Ref CloudTrailOwnerAccount Discovery: - Value: !Ref Discovery - MasterPayerAccount: - Value: !Ref MasterPayerAccount - NotificationFunctionStack: - Value: !If - - ValidReactorCallbackUrl - - !Ref NotificationFunctionStack - - !Ref AWS::NoValue + Value: !Ref DiscoveryResource + AccountResources: + Value: !Ref AccountResources + MasterPayerCurStatus: + Description: Whether a new CUR was created for a master-payer account (or why it was skipped). + Value: !GetAtt AccountResources.Outputs.MasterPayerCurStatus NotifyCloudZeroReactor: Value: !If - ValidReactorCallbackUrl - !Ref NotifyCloudZeroReactor - !Ref AWS::NoValue - ResourceOwnerAccount: - Value: !Ref ResourceOwnerAccount diff --git a/services/connected_account_dev.yaml b/services/connected_account_dev.yaml index 8f8ab47..7d217ad 100644 --- a/services/connected_account_dev.yaml +++ b/services/connected_account_dev.yaml @@ -1,6 +1,9 @@ # -*- coding: utf-8 -*- # Copyright (c) 2016-present, CloudZero, Inc. All rights reserved. # Licensed under the BSD-style license. See LICENSE file in the project root for full license information. +# +# NOTE: This file is identical to connected_account.yaml except that `Stage` defaults +# to `dev`. Keep the two in sync -- the only intended difference is the Stage default. AWSTemplateFormatVersion: '2010-09-09' Description: CloudZero Connected Account Template @@ -42,11 +45,22 @@ Parameters: Default: 'latest' Description: | Version to target when deploying the stack. `latest` should be used by default. + Stage: + Type: String + Default: 'dev' + AllowedValues: ['prod', 'dev'] + Description: | + Which CloudZero environment to link to. Customers should leave this as `prod`. Mappings: CallbackConfiguration: - dev: + prod: # TODO: Get dynamically for namespaces + ReactorAccountId: '061190967865' + ConnectorsAccountId: '559846027439' + ReactorId: 'f7a07d02-d509-4e8d-9fb9-69c7985a6bd8' + ReactorCallbackUrl: 'https://api.cloudzero.com/accounts/v1/link' + dev: ReactorAccountId: '998146006915' ConnectorsAccountId: '483772923246' ReactorId: '67ad3e83-1ec5-4f4f-b505-c27e2758b990' @@ -54,145 +68,158 @@ Mappings: Conditions: ValidReactorCallbackUrl: !Not - - !Equals [!FindInMap [CallbackConfiguration, dev, ReactorCallbackUrl], 'null'] + - !Equals [!FindInMap [CallbackConfiguration, !Ref Stage, ReactorCallbackUrl], 'null'] Resources: - Discovery: - Type: AWS::CloudFormation::Stack + # --------------------------------------------------------------------------- + # Discovery: inspect this account at deploy time. Inlined here (previously a + # nested stack) so its outputs can be passed as parameters into the single + # AccountResources stack -- CloudFormation Conditions cannot read custom-resource + # outputs directly, so parameterizing them through a nested stack is required. + # --------------------------------------------------------------------------- + DiscoveryFunctionRole: + Type: AWS::IAM::Role Properties: - Tags: - - Key: cloudzero-stack - Value: !Ref AWS::StackName - - Key: cloudzero-reactor-account-id - Value: !FindInMap [CallbackConfiguration, dev, ReactorAccountId] - TemplateURL: !Sub https://s3.amazonaws.com/cz-provision-account/${Version}/services/discovery.yaml - Parameters: - Version: !Sub ${Version} + AssumeRolePolicyDocument: + Version: '2012-10-17' + Statement: + - Effect: Allow + Principal: + Service: lambda.amazonaws.com + Action: sts:AssumeRole + ManagedPolicyArns: + - arn:aws:iam::aws:policy/service-role/AWSLambdaBasicExecutionRole + Policies: + - PolicyName: CZDiscovery + PolicyDocument: + Version: '2012-10-17' + Statement: + - Sid: CZDiscovery20190912 + Effect: Allow + Action: + - s3:ListAllMyBuckets + - cur:DescribeReportDefinitions + - bcm-data-exports:ListExports + - bcm-data-exports:GetExport + - organizations:DescribeOrganization + Resource: '*' - ResourceOwnerAccount: - Type: AWS::CloudFormation::Stack + DiscoveryFunction: + Type: AWS::Lambda::Function Properties: - Tags: - - Key: cloudzero-stack - Value: !Ref AWS::StackName - - Key: cloudzero-reactor-account-id - Value: !FindInMap [CallbackConfiguration, dev, ReactorAccountId] - Parameters: - IsResourceOwnerAccount: !GetAtt Discovery.Outputs.IsResourceOwnerAccount - ExternalId: !Ref ExternalId - ReactorAccountId: !FindInMap [CallbackConfiguration, dev, ReactorAccountId] - ConnectorsAccountId: !FindInMap [CallbackConfiguration, dev, ConnectorsAccountId] - TemplateURL: !Sub https://s3.amazonaws.com/cz-provision-account/${Version}/services/account_type/resource_owner.yaml + Code: + S3Bucket: !Sub 'cz-provision-account-${AWS::Region}' + S3Key: !Sub ${Version}/services/discovery.zip + Handler: src.app.handler + Runtime: python3.12 + MemorySize: 512 + Timeout: 30 + Role: !GetAtt DiscoveryFunctionRole.Arn + Environment: + Variables: + VERSION: '20230523' - CloudTrailOwnerAccount: - Type: AWS::CloudFormation::Stack + DiscoveryResource: + Type: Custom::Discovery Properties: - Tags: - - Key: cloudzero-stack - Value: !Ref AWS::StackName - - Key: cloudzero-reactor-account-id - Value: !FindInMap [CallbackConfiguration, dev, ReactorAccountId] - Parameters: - IsCloudTrailOwnerAccount: !GetAtt Discovery.Outputs.IsCloudTrailOwnerAccount - CloudTrailSNSTopicArn: !GetAtt Discovery.Outputs.CloudTrailSNSTopicArn - ReactorAccountId: !FindInMap [CallbackConfiguration, dev, ReactorAccountId] - TemplateURL: !Sub https://s3.amazonaws.com/cz-provision-account/${Version}/services/account_type/cloudtrail_owner.yaml + ServiceToken: !GetAtt DiscoveryFunction.Arn + AccountId: !Ref AWS::AccountId + Version: !Sub ${Version} - AuditAccount: + # --------------------------------------------------------------------------- + # The single remaining nested stack: provisions resources for whichever account + # type Discovery detected (resource-owner and/or master-payer). Discovery outputs + # are passed in as parameters so the nested stack's Conditions can gate on them. + # --------------------------------------------------------------------------- + AccountResources: Type: AWS::CloudFormation::Stack Properties: Tags: - Key: cloudzero-stack Value: !Ref AWS::StackName - Key: cloudzero-reactor-account-id - Value: !FindInMap [CallbackConfiguration, dev, ReactorAccountId] + Value: !FindInMap [CallbackConfiguration, !Ref Stage, ReactorAccountId] + TemplateURL: !Sub https://s3.amazonaws.com/cz-provision-account/${Version}/services/account_type/account_resources.yaml Parameters: - IsAuditAccount: !GetAtt Discovery.Outputs.IsAuditAccount - AuditCloudTrailBucketName: !GetAtt Discovery.Outputs.AuditCloudTrailBucketName ExternalId: !Ref ExternalId - ReactorAccountId: !FindInMap [CallbackConfiguration, dev, ReactorAccountId] - TemplateURL: !Sub https://s3.amazonaws.com/cz-provision-account/${Version}/services/account_type/audit.yaml + ReactorAccountId: !FindInMap [CallbackConfiguration, !Ref Stage, ReactorAccountId] + ConnectorsAccountId: !FindInMap [CallbackConfiguration, !Ref Stage, ConnectorsAccountId] + IsResourceOwnerAccount: !GetAtt DiscoveryResource.IsResourceOwnerAccount + IsOrganizationMasterAccount: !GetAtt DiscoveryResource.IsOrganizationMasterAccount + IsMasterPayerAccount: !GetAtt DiscoveryResource.IsMasterPayerAccount + MasterPayerBillingBucketName: !GetAtt DiscoveryResource.MasterPayerBillingBucketName + MasterPayerBillingBucketArns: !GetAtt DiscoveryResource.MasterPayerBillingBucketArns - MasterPayerAccount: - Type: AWS::CloudFormation::Stack + # --------------------------------------------------------------------------- + # Notification: POST the provisioning result back to the CloudZero reactor. + # Inlined (previously a nested stack). Receives every value it needs as direct + # properties below, so it no longer reads sibling stack outputs at runtime. + # --------------------------------------------------------------------------- + NotificationFunctionRole: + Type: AWS::IAM::Role + Condition: ValidReactorCallbackUrl Properties: - Tags: - - Key: cloudzero-stack - Value: !Ref AWS::StackName - - Key: cloudzero-reactor-account-id - Value: !FindInMap [CallbackConfiguration, dev, ReactorAccountId] - Parameters: - IsOrganizationMasterAccount: !GetAtt Discovery.Outputs.IsOrganizationMasterAccount - IsMasterPayerAccount: !GetAtt Discovery.Outputs.IsMasterPayerAccount - MasterPayerBillingBucketName: !GetAtt Discovery.Outputs.MasterPayerBillingBucketName - MasterPayerBillingBucketArns: !GetAtt Discovery.Outputs.MasterPayerBillingBucketArns - ExternalId: !Ref ExternalId - ReactorAccountId: !FindInMap [CallbackConfiguration, dev, ReactorAccountId] - ConnectorsAccountId: !FindInMap [CallbackConfiguration, dev, ConnectorsAccountId] - TemplateURL: !Sub https://s3.amazonaws.com/cz-provision-account/${Version}/services/account_type/master_payer.yaml - + AssumeRolePolicyDocument: + Version: '2012-10-17' + Statement: + - Effect: Allow + Principal: + Service: lambda.amazonaws.com + Action: sts:AssumeRole + ManagedPolicyArns: + - arn:aws:iam::aws:policy/service-role/AWSLambdaBasicExecutionRole - NotificationFunctionStack: - Type: AWS::CloudFormation::Stack + NotificationFunction: + Type: AWS::Lambda::Function Condition: ValidReactorCallbackUrl Properties: - Tags: - - Key: cloudzero-stack - Value: !Ref AWS::StackName - - Key: cloudzero-reactor-account-id - Value: !FindInMap [CallbackConfiguration, dev, ReactorAccountId] - Parameters: - ReactorCallbackUrl: !FindInMap [CallbackConfiguration, dev, ReactorCallbackUrl] - Version: !Sub ${Version} - TemplateURL: !Sub https://s3.amazonaws.com/cz-provision-account/${Version}/services/notification.yaml - + Code: + S3Bucket: !Sub 'cz-provision-account-${AWS::Region}' + S3Key: !Sub ${Version}/services/notification.zip + Handler: src.app.handler + Runtime: python3.12 + MemorySize: 512 + Timeout: 30 + Role: !GetAtt NotificationFunctionRole.Arn + Environment: + Variables: + VERSION: '1' NotifyCloudZeroReactor: Type: Custom::NotifyCloudZero Condition: ValidReactorCallbackUrl Properties: - Tags: - - Key: cloudzero-stack - Value: !Ref AWS::StackName - - Key: cloudzero-reactor-account-id - Value: !FindInMap [CallbackConfiguration, dev, ReactorAccountId] - ServiceToken: !GetAtt NotificationFunctionStack.Outputs.Arn + ServiceToken: !GetAtt NotificationFunction.Arn + # Reactor / account metadata AccountId: !Ref AWS::AccountId - Version: !Sub ${Version} - # Parameters from Other Resources in this Stack + Region: !Ref AWS::Region ExternalId: !Ref ExternalId - ReactorCallbackUrl: !FindInMap [CallbackConfiguration, dev, ReactorCallbackUrl] AccountName: !Ref AccountName - Region: !Ref AWS::Region - ReactorId: !FindInMap [CallbackConfiguration, dev, ReactorId] - # References to other stacks; it's easier for _this_ Resource to simply grab the outputs - # programmatically b/c some of the outputs are conditional. - Stacks: - Discovery: !Ref Discovery - ResourceOwnerAccount: !Ref ResourceOwnerAccount - CloudTrailOwnerAccount: !Ref CloudTrailOwnerAccount - AuditAccount: !Ref AuditAccount - MasterPayerAccount: !Ref MasterPayerAccount - LegacyAccount: !Ref ResourceOwnerAccount + ReactorId: !FindInMap [CallbackConfiguration, !Ref Stage, ReactorId] + ReactorCallbackUrl: !FindInMap [CallbackConfiguration, !Ref Stage, ReactorCallbackUrl] + # Discovery flags + billing details (resolved from the Discovery custom resource) + IsResourceOwnerAccount: !GetAtt DiscoveryResource.IsResourceOwnerAccount + IsMasterPayerAccount: !GetAtt DiscoveryResource.IsMasterPayerAccount + IsOrganizationMasterAccount: !GetAtt DiscoveryResource.IsOrganizationMasterAccount + MasterPayerBillingBucketName: !GetAtt DiscoveryResource.MasterPayerBillingBucketName + MasterPayerBillingBucketPath: !GetAtt DiscoveryResource.MasterPayerBillingBucketPath + BillingReportFormat: !GetAtt DiscoveryResource.BillingReportFormat + # Provisioned outputs (resolved from the AccountResources nested stack) + ResourceOwnerRoleArn: !GetAtt AccountResources.Outputs.ResourceOwnerRoleArn + MasterPayerRoleArn: !GetAtt AccountResources.Outputs.MasterPayerRoleArn + MasterPayerReportS3Bucket: !GetAtt AccountResources.Outputs.MasterPayerReportS3Bucket + MasterPayerReportS3Prefix: !GetAtt AccountResources.Outputs.MasterPayerReportS3Prefix Outputs: - AuditAccount: - Value: !Ref AuditAccount - CloudTrailOwnerAccount: - Value: !Ref CloudTrailOwnerAccount Discovery: - Value: !Ref Discovery - MasterPayerAccount: - Value: !Ref MasterPayerAccount - NotificationFunctionStack: - Value: !If - - ValidReactorCallbackUrl - - !Ref NotificationFunctionStack - - !Ref AWS::NoValue + Value: !Ref DiscoveryResource + AccountResources: + Value: !Ref AccountResources + MasterPayerCurStatus: + Description: Whether a new CUR was created for a master-payer account (or why it was skipped). + Value: !GetAtt AccountResources.Outputs.MasterPayerCurStatus NotifyCloudZeroReactor: Value: !If - ValidReactorCallbackUrl - !Ref NotifyCloudZeroReactor - !Ref AWS::NoValue - ResourceOwnerAccount: - Value: !Ref ResourceOwnerAccount diff --git a/services/discovery/src/app.py b/services/discovery/src/app.py index d7f66fd..e34e656 100644 --- a/services/discovery/src/app.py +++ b/services/discovery/src/app.py @@ -2,42 +2,49 @@ # Copyright (c) 2016-present, CloudZero, Inc. All rights reserved. # Licensed under the BSD-style license. See LICENSE file in the project root for full license information. -from pprint import pformat +""" +Discovery custom-resource handler. + +Given the AWS account this stack is deployed into, inspect the account and report +back the facts the connected-account stack needs to decide what to provision: + + * Is this a resource-owner account? (always yes -- every connected account) + * Is this the master-payer account? (standalone account, or org management account) + * Where does this account's CUR live? (classic Cost & Usage Report or BCM Data Export) + +The flow is deliberately linear and side-effect-light: `handler` validates its input, +calls `discover`, validates the output, and sends it back to CloudFormation. `discover` +gathers raw facts from four AWS sources (each isolated so one failure can't sink the +others) and then classifies them. There is no CloudTrail/audit detection -- those +account types are deprecated. +""" + import logging import boto3 from botocore.exceptions import ClientError -from toolz.curried import assoc_in, get_in, keyfilter, merge, pipe, update_in from voluptuous import Any, ExactSequence, Schema, ALLOW_EXTRA, REMOVE_EXTRA from src import cfnresponse logger = logging.getLogger() logger.setLevel(logging.INFO) -ct = boto3.client('cloudtrail') + cur = boto3.client('cur', region_name='us-east-1') # cur is only in us-east-1 bcm = boto3.client('bcm-data-exports', region_name='us-east-1') # bcm-data-exports is only in us-east-1 orgs = boto3.client('organizations') s3 = boto3.client('s3') + DEFAULT_OUTPUT = { - 'AuditCloudTrailBucketPrefix': None, - 'AuditCloudTrailBucketName': None, - 'RemoteCloudTrailBucket': True, - 'CloudTrailSNSTopicArn': None, - 'CloudTrailTrailArn': None, - 'IsOrganizationTrail': None, - 'IsOrganizationMasterAccount': False, - 'VisibleCloudTrailArns': None, - 'IsAuditAccount': False, - 'IsCloudTrailOwnerAccount': False, 'IsResourceOwnerAccount': False, 'IsMasterPayerAccount': False, + 'IsOrganizationMasterAccount': False, + 'IsAccountOutsideOrganization': False, 'MasterPayerBillingBucketName': None, 'MasterPayerBillingBucketPath': None, 'MasterPayerBillingBucketArns': '', 'BillingReportFormat': 'aws', - 'IsAccountOutsideOrganization': False, } @@ -59,91 +66,50 @@ OUTPUT_SCHEMA = Schema({ 'output': { - 'IsAuditAccount': bool, - 'AuditCloudTrailBucketName': Any(None, str), 'IsResourceOwnerAccount': bool, - 'IsCloudTrailOwnerAccount': bool, - 'CloudTrailSNSTopicArn': Any(None, str), 'IsMasterPayerAccount': bool, + 'IsOrganizationMasterAccount': bool, 'MasterPayerBillingBucketName': Any(None, str), 'MasterPayerBillingBucketArns': str, }, }, required=True, extra=ALLOW_EXTRA) -DEFAULT_PAYER_REPORTS = {'is_master_payer': False, 'report_definitions': []} -DEFAULT_DATA_EXPORTS = {'s3_buckets': []} -NOT_IN_ORGANIZATION_RESPONSE = {} - -event_account_id = get_in(['event', 'ResourceProperties', 'AccountId']) -coeffects_traillist = get_in(['coeffects', 'cloudtrail', 'trailList'], default=[]) -coeffects_buckets = get_in(['coeffects', 's3', 'Buckets'], default=[]) -coeffects_payer_reports = get_in(['coeffects', 'cur'], default=DEFAULT_PAYER_REPORTS) -coeffects_data_export_buckets = get_in(['coeffects', 'bcm_data_exports', 's3_buckets'], default=[]) -coeffects_master_account_id = get_in(['coeffects', 'organizations', 'Organization', 'MasterAccountId']) -output_is_organization_master = get_in(['output', 'IsOrganizationMasterAccount']) -output_is_account_outside_organization = get_in(['output', 'IsAccountOutsideOrganization']) - ##################### # -# Coeffects, i.e. from the outside world +# Gather: read raw facts from AWS (each source isolated from the others) # ##################### -def coeffects(world): - return pipe(world, - coeffects_cloudtrail, - coeffects_s3, - coeffects_cur, - coeffects_bcm_data_exports, - coeffects_organizations) - - -def coeffect(name): - def d(f): - def w(world): - data = {} - try: - data = f(world) - except Exception: - logger.warning(f'Failed to get {name} information.', exc_info=True) - return assoc_in(world, ['coeffects', name], data) - return w - return d - - -@coeffect('cloudtrail') -def coeffects_cloudtrail(world): - response = ct.describe_trails() - return keyfilter(lambda x: x in {'trailList'}, response) - - -@coeffect('s3') -def coeffects_s3(world): - response = s3.list_buckets() - return keyfilter(lambda x: x in {'Buckets'}, response) - - -@coeffect('cur') -def coeffects_cur(world): +def list_local_bucket_names(): + """Return the set of S3 bucket names owned by this account.""" + try: + response = s3.list_buckets() + except ClientError: + logger.warning('Failed to list S3 buckets', exc_info=True) + return set() + return {b['Name'] for b in response.get('Buckets', []) if b.get('Name')} + + +def list_cur_report_definitions(): + """Return the classic Cost & Usage Report definitions defined in this account.""" try: - return { - 'report_definitions': cur.describe_report_definitions().get('ReportDefinitions', []), - } + return cur.describe_report_definitions().get('ReportDefinitions', []) except ClientError: logger.warning('Failed to access CUR DescribeReportDefinitions', exc_info=True) - return DEFAULT_PAYER_REPORTS - - -@coeffect('bcm_data_exports') -def coeffects_bcm_data_exports(world): - # CUR 2.0 exports (and any other BCM Data Exports) live here rather than in the - # classic CUR. CloudZero only ingests COST_AND_USAGE_REPORT exports, but we collect - # the S3 destination bucket for *every* export regardless of type so the master-payer - # role's bucket access covers all of them -- a customer can then repoint CloudZero at - # a different export without redeploying this stack. list_exports only returns export - # ARNs, so each export is resolved with get_export to read its destination bucket; - # bucket selection against the account's local buckets happens later in - # get_all_local_cur_bucket_names. + return [] + + +def list_data_export_bucket_names(): + """ + Return the S3 destination buckets of every BCM Data Export in this account. + + CUR 2.0 exports (and any other BCM Data Exports) live here rather than in the + classic CUR. CloudZero only ingests COST_AND_USAGE_REPORT exports, but we collect + the S3 destination bucket for *every* export regardless of type so the master-payer + role's bucket access covers all of them -- a customer can then repoint CloudZero at + a different export without redeploying this stack. `list_exports` only returns export + ARNs, so each export is resolved with `get_export` to read its destination bucket. + """ try: export_arns = [] next_token = None @@ -155,116 +121,41 @@ def coeffects_bcm_data_exports(world): break except ClientError: logger.warning('Failed to access BCM Data Exports ListExports', exc_info=True) - return DEFAULT_DATA_EXPORTS + return [] # Resolve each export independently: a transient failure on one export should not # drop the buckets already resolved from the others, so isolate the get_export call # per export rather than wrapping the whole loop in a single try/except. - s3_buckets = [] + buckets = [] for export_arn in export_arns: try: export = bcm.get_export(ExportArn=export_arn).get('Export', {}) except ClientError: logger.warning(f'Failed to access BCM Data Exports GetExport for {export_arn}', exc_info=True) continue - bucket = get_in(['DestinationConfigurations', 'S3Destination', 'S3Bucket'], export) + bucket = export.get('DestinationConfigurations', {}).get('S3Destination', {}).get('S3Bucket') if bucket: - s3_buckets.append(bucket) - return {'s3_buckets': s3_buckets} + buckets.append(bucket) + return buckets -@coeffect('organizations') -def coeffects_organizations(world): +def get_organization_master_account_id(): + """Return the org's management (master payer) account id, or None if not in an org.""" try: - response = orgs.describe_organization() - return keyfilter(lambda x: x in {'Organization'}, response) + return orgs.describe_organization().get('Organization', {}).get('MasterAccountId') except ClientError: - return NOT_IN_ORGANIZATION_RESPONSE + # The account is not a member of an AWS Organization. + return None ##################### # -# Business Logic +# CUR report selection # ##################### -MINIMUM_CLOUDTRAIL_CONFIGURATION = Schema({ - "S3BucketName": str, - "SnsTopicName": str, - "SnsTopicARN": str, - "IsMultiRegionTrail": True, - "TrailARN": str, -}, extra=ALLOW_EXTRA, required=True) - - -IDEAL_CLOUDTRAIL_CONFIGURATION = MINIMUM_CLOUDTRAIL_CONFIGURATION.extend({ - "IsOrganizationTrail": True, -}, extra=ALLOW_EXTRA, required=True) - - -def safe_check(schema, data): - try: - return schema(data) - except Exception: - logger.debug(f'Data {pformat(data)} did not match schema {schema}', exc_info=True) - return None - - -def keep_valid(schema, xs): - return [ - y for y in [safe_check(schema, x) for x in xs] - if y is not None - ] - - -def get_first_valid_trail(world): - trails = coeffects_traillist(world) - logger.info(f'Found these CloudTrails: {trails}') - valid_trails = keep_valid(IDEAL_CLOUDTRAIL_CONFIGURATION, trails) or keep_valid(MINIMUM_CLOUDTRAIL_CONFIGURATION, trails) - logger.info(f'Found these _valid_ CloudTrails: {valid_trails}') - return valid_trails[0] if valid_trails else {} - - -def discover_audit_account(world): - trail = get_first_valid_trail(world) - trail_bucket = trail.get('S3BucketName') - local_buckets = {x['Name'] for x in coeffects_buckets(world)} - output = { - 'IsAuditAccount': trail_bucket in local_buckets, - 'RemoteCloudTrailBucket': trail_bucket not in local_buckets, - 'AuditCloudTrailBucketName': trail_bucket, - 'AuditCloudTrailBucketPrefix': trail.get('S3KeyPrefix'), - } - return update_in(world, ['output'], lambda x: merge(x or {}, output)) - - -def discover_connected_account(world): - output = { - 'IsResourceOwnerAccount': True, - } - return update_in(world, ['output'], lambda x: merge(x or {}, output)) - - -def get_visible_cloudtrail_arns(world): - visible_trail_arns = [x.get('TrailARN') - for x in coeffects_traillist(world)] - return ','.join(visible_trail_arns) if visible_trail_arns else None - - -def discover_cloudtrail_account(world): - visible_trails = get_visible_cloudtrail_arns(world) - trail = get_first_valid_trail(world) - trail_topic = trail.get('SnsTopicARN') - account_id = trail_topic.split(':')[4] if trail_topic else None - output = { - 'IsCloudTrailOwnerAccount': account_id == event_account_id(world), - 'IsOrganizationTrail': trail.get('IsOrganizationTrail'), - 'CloudTrailSNSTopicArn': trail_topic, - 'CloudTrailTrailArn': trail.get('TrailARN'), - 'VisibleCloudTrailArns': visible_trails, - } - return update_in(world, ['output'], lambda x: merge(x or {}, output)) - - +# A CUR report must match one of these schemas to be ingestable by CloudZero. CSV is +# preferred over Parquet, and within a format an "ideal" (resource-tagged, versioned) +# report is preferred over a "minimum" one. IDEAL_BILLING_REPORT_CSV = Schema({ 'TimeUnit': 'HOURLY', 'Format': 'textORcsv', @@ -320,51 +211,50 @@ def discover_cloudtrail_account(world): ] -def get_first_valid_report_definition(valid_report_definitions, default=None): - return valid_report_definitions[0] if any(valid_report_definitions) else default - - -def _report_to_bucket_info(report): - bucket_name = report.get('S3Bucket') - bucket_path = f"{report.get('S3Prefix', '')}/{report.get('ReportName', '')}" if bucket_name else None - return bucket_name, bucket_path +def matches_schema(schema, data): + """True if `data` validates against the voluptuous `schema`.""" + try: + schema(data) + return True + except Exception: + return False -def get_cur_bucket_if_local(world, report_definitions): - logger.info(f'Found these ReportDefinitions: {report_definitions}') - local_buckets = {x['Name'] for x in coeffects_buckets(world)} +def select_ingest_cur(report_definitions, local_bucket_names): + """ + Pick the single CUR report CloudZero will ingest, preferring CSV over Parquet and + ideal over minimum (see `_CUR_CANDIDATE_TIERS`). Only reports whose bucket is owned + by this account are eligible. + Returns (bucket_name, bucket_path, billing_report_format); all-None / 'aws' when none. + """ for schema, billing_report_format in _CUR_CANDIDATE_TIERS: - local_matches = [r for r in keep_valid(schema, report_definitions) if r['S3Bucket'] in local_buckets] - logger.info(f'CUR tier {billing_report_format}: {local_matches}') - if local_matches: - bucket_name, bucket_path = _report_to_bucket_info(get_first_valid_report_definition(local_matches)) - return bucket_name, bucket_path, billing_report_format - + for report in report_definitions: + if report.get('S3Bucket') in local_bucket_names and matches_schema(schema, report): + bucket_name = report['S3Bucket'] + bucket_path = f"{report.get('S3Prefix', '')}/{report.get('ReportName', '')}" + logger.info(f'Selected ingest CUR ({billing_report_format}) in bucket {bucket_name}') + return bucket_name, bucket_path, billing_report_format return None, None, 'aws' -def get_all_local_cur_bucket_names(world, report_definitions): - # Schema-agnostic on purpose: we enumerate every locally-owned bucket referenced by - # any CUR report, regardless of whether the report's schema matches CloudZero's - # ingest formats (the `_CUR_CANDIDATE_TIERS` filter applied by `get_cur_bucket_if_local`). - # The role needs s3:Get/List on every CUR bucket so the customer can later switch - # CloudZero to a different report without redeploying this stack. A bucket referenced - # by a CUR report is by definition a CUR bucket; the schema filter only governs which - # report CloudZero currently ingests, not which buckets are legitimate CUR storage. - # - # CUR 2.0 data lives in BCM Data Exports rather than the classic CUR, so we also - # include the S3 destination buckets discovered via bcm-data-exports list/get. As - # with classic CUR above, this is deliberately type-agnostic: CloudZero only ingests - # COST_AND_USAGE_REPORT exports, but the role is granted bucket access to every - # export's destination so the customer can switch export types without redeploy. - local_buckets = {x['Name'] for x in coeffects_buckets(world)} +def all_local_billing_bucket_names(report_definitions, data_export_buckets, local_bucket_names): + """ + Every locally-owned bucket referenced by any CUR report or BCM Data Export. + + Deliberately schema-agnostic: a bucket referenced by any billing report/export is + legitimate billing storage, so the master-payer role gets s3:Get/List on all of + them. This lets a customer switch CloudZero between report formats (CSV/Parquet/CUR + 2.0) without redeploying this stack. The schema filter in `select_ingest_cur` only + governs which report CloudZero currently ingests, not which buckets are legitimate. + """ report_buckets = {r['S3Bucket'] for r in report_definitions if isinstance(r.get('S3Bucket'), str)} - data_export_buckets = {b for b in coeffects_data_export_buckets(world) if isinstance(b, str)} - return sorted((report_buckets | data_export_buckets) & local_buckets) + export_buckets = {b for b in data_export_buckets if isinstance(b, str)} + return sorted((report_buckets | export_buckets) & local_bucket_names) def format_bucket_arns(bucket_names): + """Render bucket names as a comma-separated list of `bucket` and `bucket/*` ARNs.""" arns = [] for name in bucket_names: arns.append(f'arn:aws:s3:::{name}') @@ -372,41 +262,38 @@ def format_bucket_arns(bucket_names): return ','.join(arns) -def discover_master_payer_account(world): - is_account_not_in_organization = output_is_account_outside_organization(world) - is_account_organization_master_account = output_is_organization_master(world) - is_master_payer = is_account_not_in_organization or is_account_organization_master_account - report_definitions = coeffects_payer_reports(world)['report_definitions'] - bucket_name, bucket_path, billing_report_format = get_cur_bucket_if_local(world, report_definitions) - all_local_cur_buckets = get_all_local_cur_bucket_names(world, report_definitions) - output = { +##################### +# +# Classification +# +##################### +def discover(account_id): + """Gather account facts and classify the account for CloudZero onboarding.""" + local_bucket_names = list_local_bucket_names() + report_definitions = list_cur_report_definitions() + data_export_buckets = list_data_export_bucket_names() + master_account_id = get_organization_master_account_id() + + is_outside_organization = master_account_id is None + is_organization_master = account_id == master_account_id + # A standalone account (no org) pays its own bill; in an org, the management account + # is the payer. Either way that account owns the consolidated CUR. + is_master_payer = is_outside_organization or is_organization_master + + ingest_bucket, ingest_path, billing_report_format = select_ingest_cur(report_definitions, local_bucket_names) + billing_bucket_arns = format_bucket_arns( + all_local_billing_bucket_names(report_definitions, data_export_buckets, local_bucket_names)) + + return { + 'IsResourceOwnerAccount': True, 'IsMasterPayerAccount': is_master_payer, - 'MasterPayerBillingBucketName': bucket_name, - 'MasterPayerBillingBucketPath': bucket_path, - 'MasterPayerBillingBucketArns': format_bucket_arns(all_local_cur_buckets), + 'IsOrganizationMasterAccount': is_organization_master, + 'IsAccountOutsideOrganization': is_outside_organization, + 'MasterPayerBillingBucketName': ingest_bucket, + 'MasterPayerBillingBucketPath': ingest_path, + 'MasterPayerBillingBucketArns': billing_bucket_arns, 'BillingReportFormat': billing_report_format, } - return update_in(world, ['output'], lambda x: merge(x or {}, output)) - - -def discover_organization_master_account(world): - account_id = event_account_id(world) - master_account_id = coeffects_master_account_id(world) - - output = { - 'IsOrganizationMasterAccount': account_id == master_account_id, - 'IsAccountOutsideOrganization': master_account_id is None, - } - return update_in(world, ['output'], lambda x: merge(x or {}, output)) - - -def discover_account_types(world): - return pipe(world, - discover_audit_account, - discover_connected_account, - discover_cloudtrail_account, - discover_organization_master_account, - discover_master_payer_account) ##################### @@ -416,17 +303,14 @@ def discover_account_types(world): ##################### def handler(event, context, **kwargs): status = cfnresponse.SUCCESS - world = {} + output = DEFAULT_OUTPUT try: logger.info(f'Processing event {event}') - world = pipe({'event': event, 'kwargs': kwargs}, - INPUT_SCHEMA, - coeffects, - discover_account_types, - OUTPUT_SCHEMA) + validated = INPUT_SCHEMA({'event': event}) + account_id = validated['event']['ResourceProperties']['AccountId'] + output = OUTPUT_SCHEMA({'output': discover(account_id)})['output'] except Exception as err: logger.exception(err) finally: - output = world.get('output', DEFAULT_OUTPUT) logger.info(f'Sending output {output}') cfnresponse.send(event, context, status, output, event.get('PhysicalResourceId')) diff --git a/services/discovery/template.yaml b/services/discovery/template.yaml index c39f933..92f075e 100644 --- a/services/discovery/template.yaml +++ b/services/discovery/template.yaml @@ -33,7 +33,6 @@ Resources: - Sid: CZDiscovery20190912 Effect: Allow Action: - - cloudtrail:DescribeTrails - s3:ListAllMyBuckets - cur:DescribeReportDefinitions - bcm-data-exports:ListExports diff --git a/services/discovery/tests/unit/test_app.py b/services/discovery/tests/unit/test_app.py index a06bcab..edddf9a 100644 --- a/services/discovery/tests/unit/test_app.py +++ b/services/discovery/tests/unit/test_app.py @@ -7,8 +7,6 @@ import pytest from botocore.exceptions import ClientError -from voluptuous import All, Schema, ALLOW_EXTRA -from toolz.curried import assoc_in import src.app as app from src import cfnresponse @@ -24,14 +22,7 @@ EXPORT_ARN = f'arn:aws:bcm-data-exports:us-east-1:{LOCAL_ACCOUNT_ID}:export/cur2-export' -LOCAL_TRAIL_ARN = f'arn:aws:cloudtrail:us-east-1:{LOCAL_ACCOUNT_ID}:trail/local-trail' -REMOTE_TRAIL_ARN = f'arn:aws:cloudtrail:us-east-1:{REMOTE_ACCOUNT_ID}:trail/remote-trail' -LOCAL_TOPIC_ARN = f'arn:aws:sns:us-east-1:{LOCAL_ACCOUNT_ID}:local-cloudtrail-topic' -REMOTE_TOPIC_ARN = f'arn:aws:sns:us-east-1:{REMOTE_ACCOUNT_ID}:remote-cloudtrail-topic' - - -# TODO: Replace these fixtures with Voluptuous Schemas + Hypothesis + Property Tests @pytest.fixture() def cfn_event(): return { @@ -49,20 +40,12 @@ def cfn_event(): @pytest.fixture() def describe_organizations_local(): - return { - 'Organization': { - 'MasterAccountId': LOCAL_ACCOUNT_ID - } - } + return {'Organization': {'MasterAccountId': LOCAL_ACCOUNT_ID}} @pytest.fixture() def describe_organizations_remote(): - return { - 'Organization': { - 'MasterAccountId': REMOTE_ACCOUNT_ID - } - } + return {'Organization': {'MasterAccountId': REMOTE_ACCOUNT_ID}} @pytest.fixture() @@ -72,79 +55,9 @@ def describe_organizations_not_in_organization_error(): 'DescribeOrganization') -@pytest.fixture() -def describe_trails_response_local(): - return { - 'trailList': [ - { - 'HasCustomEventSelectors': True, - 'HomeRegion': 'us-east-1', - 'IncludeGlobalServiceEvents': True, - 'IsMultiRegionTrail': True, - 'IsOrganizationTrail': False, - 'LogFileValidationEnabled': True, - 'Name': 'my-local-trail', - 'S3BucketName': LOCAL_BUCKET_NAME, - 'SnsTopicARN': LOCAL_TOPIC_ARN, - 'SnsTopicName': LOCAL_TOPIC_ARN, - 'TrailARN': LOCAL_TRAIL_ARN, - 'S3KeyPrefix': 'trails', - }, - ], - } - - -@pytest.fixture() -def describe_trails_response_remote(): - return { - 'trailList': [ - { - 'HasCustomEventSelectors': True, - 'HomeRegion': 'us-east-1', - 'IncludeGlobalServiceEvents': True, - 'IsMultiRegionTrail': True, - 'IsOrganizationTrail': True, - 'LogFileValidationEnabled': True, - 'Name': 'my-local-trail', - 'S3BucketName': LOCAL_BUCKET_NAME, - 'SnsTopicARN': REMOTE_TOPIC_ARN, - 'SnsTopicName': REMOTE_TOPIC_ARN, - 'TrailARN': REMOTE_TRAIL_ARN, - }, - ], - } - - -@pytest.fixture() -def describe_trails_response_remote_bucket(): - return { - 'trailList': [ - { - 'HasCustomEventSelectors': True, - 'HomeRegion': 'us-east-1', - 'IncludeGlobalServiceEvents': True, - 'IsMultiRegionTrail': True, - 'IsOrganizationTrail': False, - 'LogFileValidationEnabled': True, - 'Name': 'my-local-trail', - 'S3BucketName': REMOTE_BUCKET_NAME, - 'SnsTopicARN': LOCAL_TOPIC_ARN, - 'SnsTopicName': LOCAL_TOPIC_ARN, - 'TrailARN': LOCAL_TRAIL_ARN, - }, - ], - } - - @pytest.fixture() def list_buckets_response(): - return { - 'Buckets': [ - { - 'Name': LOCAL_BUCKET_NAME, - } - ] - } + return {'Buckets': [{'Name': LOCAL_BUCKET_NAME}]} @pytest.fixture() @@ -157,22 +70,18 @@ def describe_report_definitions_response_local(): 'ReportName': 'billing_report', }, { - "ReportName": "valid-local-report", - "TimeUnit": "HOURLY", - "Format": "textORcsv", - "Compression": "GZIP", - "AdditionalSchemaElements": [ - "RESOURCES" - ], - "S3Bucket": LOCAL_BUCKET_NAME, - "S3Prefix": "reports", - "S3Region": "us-east-1", - "AdditionalArtifacts": [ - "REDSHIFT" - ], - "RefreshClosedReports": True, - "ReportVersioning": "CREATE_NEW_REPORT" - } + 'ReportName': 'valid-local-report', + 'TimeUnit': 'HOURLY', + 'Format': 'textORcsv', + 'Compression': 'GZIP', + 'AdditionalSchemaElements': ['RESOURCES'], + 'S3Bucket': LOCAL_BUCKET_NAME, + 'S3Prefix': 'reports', + 'S3Region': 'us-east-1', + 'AdditionalArtifacts': ['REDSHIFT'], + 'RefreshClosedReports': True, + 'ReportVersioning': 'CREATE_NEW_REPORT', + }, ] } @@ -182,21 +91,17 @@ def describe_report_definitions_response_remote(): return { 'ReportDefinitions': [ { - "ReportName": "valid-local-report", - "TimeUnit": "HOURLY", - "Format": "textORcsv", - "Compression": "GZIP", - "AdditionalSchemaElements": [ - "RESOURCES" - ], - "S3Bucket": REMOTE_BUCKET_NAME, - "S3Prefix": "reports", - "S3Region": "us-east-1", - "AdditionalArtifacts": [ - "REDSHIFT" - ], - "RefreshClosedReports": True, - "ReportVersioning": "CREATE_NEW_REPORT" + 'ReportName': 'valid-local-report', + 'TimeUnit': 'HOURLY', + 'Format': 'textORcsv', + 'Compression': 'GZIP', + 'AdditionalSchemaElements': ['RESOURCES'], + 'S3Bucket': REMOTE_BUCKET_NAME, + 'S3Prefix': 'reports', + 'S3Region': 'us-east-1', + 'AdditionalArtifacts': ['REDSHIFT'], + 'RefreshClosedReports': True, + 'ReportVersioning': 'CREATE_NEW_REPORT', } ] } @@ -256,8 +161,8 @@ def describe_report_definitions_response_invalid(): @pytest.fixture() def describe_report_definitions_client_error(): return ClientError({'Error': {'Code': 'AccessDeniedException', - 'Message': 'is not authorized to callDescribeReportDefinitions'}}, - 'GetReportDefinitions') + 'Message': 'is not authorized to call DescribeReportDefinitions'}}, + 'DescribeReportDefinitions') @pytest.fixture(scope='function') @@ -267,7 +172,6 @@ def context(mocker): context.os = {'environ': os.environ} context.prefix = app.__name__ context.mock_cfnresponse_send = mocker.patch(f'{context.prefix}.cfnresponse.send', autospec=True) - context.mock_ct = mocker.patch(f'{context.prefix}.ct', autospec=True) context.mock_cur = mocker.patch(f'{context.prefix}.cur', autospec=True) context.mock_bcm = mocker.patch(f'{context.prefix}.bcm', autospec=True) context.mock_bcm.list_exports.return_value = {'Exports': []} @@ -278,274 +182,84 @@ def context(mocker): mocker.stopall() -IS_MASTER_PAYER = Schema({ - 'IsMasterPayerAccount': True, - 'MasterPayerBillingBucketName': LOCAL_BUCKET_NAME, -}, extra=ALLOW_EXTRA, required=True) - -IS_AUDIT = Schema({ - 'IsAuditAccount': True, -}, extra=ALLOW_EXTRA, required=True) - -IS_CONNECTED = Schema({ - 'IsResourceOwnerAccount': True, -}, extra=ALLOW_EXTRA, required=True) - -IS_CLOUDTRAIL_OWNER = Schema({ - 'CloudTrailSNSTopicArn': LOCAL_TOPIC_ARN, - 'IsCloudTrailOwnerAccount': True, -}, extra=ALLOW_EXTRA, required=True) - -ALL_LOCAL = All(IS_MASTER_PAYER, IS_AUDIT, IS_CONNECTED, IS_CLOUDTRAIL_OWNER) - - -@pytest.mark.unit -def test_handler_all_local(context, cfn_event, describe_trails_response_local, list_buckets_response, describe_report_definitions_response_local, describe_organizations_local): - context.mock_ct.describe_trails.return_value = describe_trails_response_local - context.mock_cur.describe_report_definitions.return_value = describe_report_definitions_response_local - context.mock_orgs.describe_organization.return_value = describe_organizations_local - context.mock_s3.list_buckets.return_value = list_buckets_response - ret = app.handler(cfn_event, None) - assert ret is None - assert context.mock_cfnresponse_send.call_count == 1 - ((_, _, status, output, _), kwargs) = context.mock_cfnresponse_send.call_args +def _output(context): + ((_, _, status, output, _), _) = context.mock_cfnresponse_send.call_args assert status == cfnresponse.SUCCESS - assert output == { - 'AuditCloudTrailBucketPrefix': 'trails', - 'AuditCloudTrailBucketName': LOCAL_BUCKET_NAME, - 'CloudTrailSNSTopicArn': LOCAL_TOPIC_ARN, - 'CloudTrailTrailArn': LOCAL_TRAIL_ARN, - 'VisibleCloudTrailArns': LOCAL_TRAIL_ARN, - 'IsOrganizationTrail': False, - 'IsAuditAccount': True, - 'IsCloudTrailOwnerAccount': True, - 'IsResourceOwnerAccount': True, - 'IsMasterPayerAccount': True, - 'IsOrganizationMasterAccount': True, - 'MasterPayerBillingBucketName': LOCAL_BUCKET_NAME, - 'MasterPayerBillingBucketPath': 'reports/valid-local-report', - 'MasterPayerBillingBucketArns': f'arn:aws:s3:::{LOCAL_BUCKET_NAME},arn:aws:s3:::{LOCAL_BUCKET_NAME}/*', - 'BillingReportFormat': 'aws', - 'RemoteCloudTrailBucket': False, - 'IsAccountOutsideOrganization': False, - } + return output @pytest.mark.unit -def test_handler_non_audit(context, cfn_event, describe_trails_response_remote_bucket, list_buckets_response, describe_report_definitions_response_local, describe_organizations_local): - context.mock_ct.describe_trails.return_value = describe_trails_response_remote_bucket +def test_handler_master_payer_org_master_with_local_cur( + context, cfn_event, list_buckets_response, + describe_report_definitions_response_local, describe_organizations_local, +): context.mock_cur.describe_report_definitions.return_value = describe_report_definitions_response_local context.mock_orgs.describe_organization.return_value = describe_organizations_local context.mock_s3.list_buckets.return_value = list_buckets_response - ret = app.handler(cfn_event, None) - assert ret is None - assert context.mock_cfnresponse_send.call_count == 1 - ((_, _, status, output, _), kwargs) = context.mock_cfnresponse_send.call_args - assert status == cfnresponse.SUCCESS - assert output == { - 'AuditCloudTrailBucketPrefix': None, - 'AuditCloudTrailBucketName': REMOTE_BUCKET_NAME, - 'RemoteCloudTrailBucket': True, - 'CloudTrailSNSTopicArn': LOCAL_TOPIC_ARN, - 'CloudTrailTrailArn': LOCAL_TRAIL_ARN, - 'VisibleCloudTrailArns': LOCAL_TRAIL_ARN, - 'IsOrganizationTrail': False, - 'IsAuditAccount': False, - 'IsCloudTrailOwnerAccount': True, + app.handler(cfn_event, None) + assert _output(context) == { 'IsResourceOwnerAccount': True, 'IsMasterPayerAccount': True, 'IsOrganizationMasterAccount': True, - 'MasterPayerBillingBucketName': LOCAL_BUCKET_NAME, - 'MasterPayerBillingBucketPath': 'reports/valid-local-report', - 'MasterPayerBillingBucketArns': f'arn:aws:s3:::{LOCAL_BUCKET_NAME},arn:aws:s3:::{LOCAL_BUCKET_NAME}/*', - 'BillingReportFormat': 'aws', 'IsAccountOutsideOrganization': False, - } - - -@pytest.mark.unit -def test_handler_remote_organization_trail(context, cfn_event, describe_trails_response_remote, list_buckets_response, describe_report_definitions_response_local, describe_organizations_local): - context.mock_ct.describe_trails.return_value = describe_trails_response_remote - context.mock_cur.describe_report_definitions.return_value = describe_report_definitions_response_local - context.mock_orgs.describe_organization.return_value = describe_organizations_local - context.mock_s3.list_buckets.return_value = list_buckets_response - ret = app.handler(cfn_event, None) - assert ret is None - assert context.mock_cfnresponse_send.call_count == 1 - ((_, _, status, output, _), kwargs) = context.mock_cfnresponse_send.call_args - assert status == cfnresponse.SUCCESS - assert output == { - 'AuditCloudTrailBucketPrefix': None, - 'AuditCloudTrailBucketName': LOCAL_BUCKET_NAME, - 'RemoteCloudTrailBucket': False, - 'CloudTrailSNSTopicArn': REMOTE_TOPIC_ARN, - 'CloudTrailTrailArn': REMOTE_TRAIL_ARN, - 'VisibleCloudTrailArns': REMOTE_TRAIL_ARN, - 'IsOrganizationTrail': True, - 'IsAuditAccount': True, - 'IsCloudTrailOwnerAccount': False, - 'IsResourceOwnerAccount': True, - 'IsMasterPayerAccount': True, - 'IsOrganizationMasterAccount': True, 'MasterPayerBillingBucketName': LOCAL_BUCKET_NAME, 'MasterPayerBillingBucketPath': 'reports/valid-local-report', 'MasterPayerBillingBucketArns': f'arn:aws:s3:::{LOCAL_BUCKET_NAME},arn:aws:s3:::{LOCAL_BUCKET_NAME}/*', 'BillingReportFormat': 'aws', - 'IsAccountOutsideOrganization': False, - } - - -@pytest.mark.unit -def test_handler_master_payer_with_no_valid_reports(context, cfn_event, describe_trails_response_local, list_buckets_response, describe_report_definitions_response_invalid, describe_organizations_local): - context.mock_ct.describe_trails.return_value = describe_trails_response_local - context.mock_cur.describe_report_definitions.return_value = describe_report_definitions_response_invalid - context.mock_orgs.describe_organization.return_value = describe_organizations_local - context.mock_s3.list_buckets.return_value = list_buckets_response - ret = app.handler(cfn_event, None) - assert ret is None - assert context.mock_cfnresponse_send.call_count == 1 - ((_, _, status, output, _), kwargs) = context.mock_cfnresponse_send.call_args - assert status == cfnresponse.SUCCESS - assert output == { - 'AuditCloudTrailBucketPrefix': 'trails', - 'AuditCloudTrailBucketName': LOCAL_BUCKET_NAME, - 'RemoteCloudTrailBucket': False, - 'CloudTrailSNSTopicArn': LOCAL_TOPIC_ARN, - 'CloudTrailTrailArn': LOCAL_TRAIL_ARN, - 'VisibleCloudTrailArns': LOCAL_TRAIL_ARN, - 'IsOrganizationTrail': False, - 'IsAuditAccount': True, - 'IsCloudTrailOwnerAccount': True, - 'IsResourceOwnerAccount': True, - 'IsMasterPayerAccount': True, - 'IsOrganizationMasterAccount': True, - 'MasterPayerBillingBucketName': None, - 'MasterPayerBillingBucketPath': None, - 'MasterPayerBillingBucketArns': f'arn:aws:s3:::{LOCAL_BUCKET_NAME},arn:aws:s3:::{LOCAL_BUCKET_NAME}/*', - 'BillingReportFormat': 'aws', - 'IsAccountOutsideOrganization': False, } @pytest.mark.unit -def test_handler_master_payer_outside_organization(context, cfn_event, describe_trails_response_local, list_buckets_response, describe_report_definitions_response_invalid, describe_organizations_not_in_organization_error): - context.mock_ct.describe_trails.return_value = describe_trails_response_local +def test_handler_master_payer_outside_organization( + context, cfn_event, list_buckets_response, + describe_report_definitions_response_invalid, describe_organizations_not_in_organization_error, +): context.mock_cur.describe_report_definitions.return_value = describe_report_definitions_response_invalid context.mock_orgs.describe_organization.side_effect = describe_organizations_not_in_organization_error context.mock_s3.list_buckets.return_value = list_buckets_response - ret = app.handler(cfn_event, None) - assert ret is None - assert context.mock_cfnresponse_send.call_count == 1 - ((_, _, status, output, _), kwargs) = context.mock_cfnresponse_send.call_args - assert status == cfnresponse.SUCCESS - assert output == { - 'AuditCloudTrailBucketPrefix': 'trails', - 'AuditCloudTrailBucketName': LOCAL_BUCKET_NAME, - 'RemoteCloudTrailBucket': False, - 'CloudTrailSNSTopicArn': LOCAL_TOPIC_ARN, - 'CloudTrailTrailArn': LOCAL_TRAIL_ARN, - 'VisibleCloudTrailArns': LOCAL_TRAIL_ARN, - 'IsOrganizationTrail': False, - 'IsAuditAccount': True, - 'IsCloudTrailOwnerAccount': True, - 'IsResourceOwnerAccount': True, - 'IsMasterPayerAccount': True, - 'IsOrganizationMasterAccount': False, - 'MasterPayerBillingBucketName': None, - 'MasterPayerBillingBucketPath': None, - 'MasterPayerBillingBucketArns': f'arn:aws:s3:::{LOCAL_BUCKET_NAME},arn:aws:s3:::{LOCAL_BUCKET_NAME}/*', - 'BillingReportFormat': 'aws', - 'IsAccountOutsideOrganization': True, - } + app.handler(cfn_event, None) + output = _output(context) + assert output['IsMasterPayerAccount'] is True + assert output['IsAccountOutsideOrganization'] is True + assert output['IsOrganizationMasterAccount'] is False + # No valid ingestable CUR, but the bucket the invalid report references is still granted. + assert output['MasterPayerBillingBucketName'] is None + assert output['MasterPayerBillingBucketArns'] == ( + f'arn:aws:s3:::{LOCAL_BUCKET_NAME},arn:aws:s3:::{LOCAL_BUCKET_NAME}/*' + ) -# This is actually not possible @pytest.mark.unit -def test_handler_master_payer_remote(context, cfn_event, describe_trails_response_local, list_buckets_response, describe_report_definitions_response_remote, describe_organizations_remote): - context.mock_ct.describe_trails.return_value = describe_trails_response_local +def test_handler_not_master_payer_when_org_master_is_remote( + context, cfn_event, list_buckets_response, + describe_report_definitions_response_remote, describe_organizations_remote, +): context.mock_cur.describe_report_definitions.return_value = describe_report_definitions_response_remote context.mock_orgs.describe_organization.return_value = describe_organizations_remote context.mock_s3.list_buckets.return_value = list_buckets_response - ret = app.handler(cfn_event, None) - assert ret is None - assert context.mock_cfnresponse_send.call_count == 1 - ((_, _, status, output, _), kwargs) = context.mock_cfnresponse_send.call_args - assert status == cfnresponse.SUCCESS - assert output == { - 'AuditCloudTrailBucketPrefix': 'trails', - 'AuditCloudTrailBucketName': LOCAL_BUCKET_NAME, - 'RemoteCloudTrailBucket': False, - 'CloudTrailSNSTopicArn': LOCAL_TOPIC_ARN, - 'CloudTrailTrailArn': LOCAL_TRAIL_ARN, - 'VisibleCloudTrailArns': LOCAL_TRAIL_ARN, - 'IsOrganizationTrail': False, - 'IsAuditAccount': True, - 'IsCloudTrailOwnerAccount': True, - 'IsResourceOwnerAccount': True, - 'IsMasterPayerAccount': False, - 'IsOrganizationMasterAccount': False, - 'MasterPayerBillingBucketName': None, - 'MasterPayerBillingBucketPath': None, - 'MasterPayerBillingBucketArns': '', - 'BillingReportFormat': 'aws', - 'IsAccountOutsideOrganization': False, - } + app.handler(cfn_event, None) + output = _output(context) + assert output['IsMasterPayerAccount'] is False + assert output['IsOrganizationMasterAccount'] is False + assert output['MasterPayerBillingBucketName'] is None + assert output['MasterPayerBillingBucketArns'] == '' @pytest.mark.unit -def test_handler_just_resource_owner(context, cfn_event, list_buckets_response, describe_report_definitions_client_error, describe_organizations_remote): - context.mock_ct.describe_trails.return_value = {'trailList': []} +def test_handler_only_resource_owner_when_cur_access_denied( + context, cfn_event, describe_report_definitions_client_error, describe_organizations_remote, +): context.mock_cur.describe_report_definitions.side_effect = describe_report_definitions_client_error context.mock_orgs.describe_organization.return_value = describe_organizations_remote - context.mock_s3.list_buckets.return_value = list_buckets_response - ret = app.handler(cfn_event, None) - assert ret is None - assert context.mock_cfnresponse_send.call_count == 1 - ((_, _, status, output, _), kwargs) = context.mock_cfnresponse_send.call_args - assert status == cfnresponse.SUCCESS - assert output == { - 'AuditCloudTrailBucketPrefix': None, - 'AuditCloudTrailBucketName': None, - 'RemoteCloudTrailBucket': True, - 'CloudTrailSNSTopicArn': None, - 'CloudTrailTrailArn': None, - 'VisibleCloudTrailArns': None, - 'IsOrganizationTrail': None, - 'IsAuditAccount': False, - 'IsCloudTrailOwnerAccount': False, - 'IsResourceOwnerAccount': True, - 'IsMasterPayerAccount': False, - 'IsOrganizationMasterAccount': False, - 'MasterPayerBillingBucketName': None, - 'MasterPayerBillingBucketPath': None, - 'MasterPayerBillingBucketArns': '', - 'BillingReportFormat': 'aws', - 'IsAccountOutsideOrganization': False, - } - - -@pytest.mark.unit -def test_handler_only_connected(context, cfn_event, describe_report_definitions_client_error, describe_organizations_remote): - context.mock_ct.describe_trails.return_value = {'trailList': []} - context.mock_cur.describe_report_definitions.side_effect = describe_report_definitions_client_error context.mock_s3.list_buckets.return_value = {'Buckets': []} - context.mock_orgs.describe_organization.return_value = describe_organizations_remote - ret = app.handler(cfn_event, None) - assert ret is None - assert context.mock_cfnresponse_send.call_count == 1 - ((_, _, status, output, _), kwargs) = context.mock_cfnresponse_send.call_args - assert status == cfnresponse.SUCCESS - assert output == assoc_in(app.DEFAULT_OUTPUT, ['IsResourceOwnerAccount'], True) + app.handler(cfn_event, None) + assert _output(context) == {**app.DEFAULT_OUTPUT, 'IsResourceOwnerAccount': True} @pytest.mark.unit -def test_handler_exception(context): - ret = app.handler({}, None) - assert ret is None - assert context.mock_cfnresponse_send.call_count == 1 - ((_, _, status, output, _), kwargs) = context.mock_cfnresponse_send.call_args - assert status == cfnresponse.SUCCESS - assert output == app.DEFAULT_OUTPUT +def test_handler_exception_returns_default_output(context): + app.handler({}, None) + assert _output(context) == app.DEFAULT_OUTPUT @pytest.mark.unit @@ -554,15 +268,15 @@ def test_handler_exception(context): ({'ReportDefinitions': [PARQUET_REPORT, CSV_REPORT]}, 'aws'), ({'ReportDefinitions': [PARQUET_REPORT, MINIMUM_CSV_REPORT]}, 'aws'), # ideal Parquet + minimum CSV → CSV wins ]) -def test_handler_cur_format_detection(context, cfn_event, describe_trails_response_local, list_buckets_response, describe_organizations_local, report_definitions, expected_format): - context.mock_ct.describe_trails.return_value = describe_trails_response_local +def test_handler_cur_format_detection( + context, cfn_event, list_buckets_response, describe_organizations_local, + report_definitions, expected_format, +): context.mock_cur.describe_report_definitions.return_value = report_definitions context.mock_orgs.describe_organization.return_value = describe_organizations_local context.mock_s3.list_buckets.return_value = list_buckets_response - ret = app.handler(cfn_event, None) - assert ret is None - ((_, _, status, output, _), _) = context.mock_cfnresponse_send.call_args - assert status == cfnresponse.SUCCESS + app.handler(cfn_event, None) + output = _output(context) assert output['BillingReportFormat'] == expected_format assert output['MasterPayerBillingBucketName'] == LOCAL_BUCKET_NAME assert output['IsMasterPayerAccount'] is True @@ -570,73 +284,43 @@ def test_handler_cur_format_detection(context, cfn_event, describe_trails_respon @pytest.fixture() def list_buckets_response_two_local(): - return { - 'Buckets': [ - {'Name': LOCAL_BUCKET_NAME}, - {'Name': SECOND_LOCAL_BUCKET_NAME}, - ] - } - - -@pytest.fixture() -def describe_report_definitions_response_two_local(): - second_report = dict(CSV_REPORT, ReportName='second-cur', S3Bucket=SECOND_LOCAL_BUCKET_NAME, S3Prefix='cz') - return {'ReportDefinitions': [CSV_REPORT, second_report]} + return {'Buckets': [{'Name': LOCAL_BUCKET_NAME}, {'Name': SECOND_LOCAL_BUCKET_NAME}]} @pytest.mark.unit -def test_handler_master_payer_enumerates_all_local_cur_buckets( - context, cfn_event, describe_trails_response_local, - list_buckets_response_two_local, describe_report_definitions_response_two_local, - describe_organizations_local, +def test_handler_enumerates_all_local_cur_buckets( + context, cfn_event, list_buckets_response_two_local, describe_organizations_local, ): - context.mock_ct.describe_trails.return_value = describe_trails_response_local - context.mock_cur.describe_report_definitions.return_value = describe_report_definitions_response_two_local + second_report = dict(CSV_REPORT, ReportName='second-cur', S3Bucket=SECOND_LOCAL_BUCKET_NAME, S3Prefix='cz') + context.mock_cur.describe_report_definitions.return_value = {'ReportDefinitions': [CSV_REPORT, second_report]} context.mock_orgs.describe_organization.return_value = describe_organizations_local context.mock_s3.list_buckets.return_value = list_buckets_response_two_local - ret = app.handler(cfn_event, None) - assert ret is None - ((_, _, status, output, _), _) = context.mock_cfnresponse_send.call_args - assert status == cfnresponse.SUCCESS + app.handler(cfn_event, None) expected = ','.join([ f'arn:aws:s3:::{SECOND_LOCAL_BUCKET_NAME}', f'arn:aws:s3:::{SECOND_LOCAL_BUCKET_NAME}/*', f'arn:aws:s3:::{LOCAL_BUCKET_NAME}', f'arn:aws:s3:::{LOCAL_BUCKET_NAME}/*', ]) - assert output['MasterPayerBillingBucketArns'] == expected - assert output['MasterPayerBillingBucketName'] == LOCAL_BUCKET_NAME - - -@pytest.fixture() -def describe_report_definitions_response_two_local_one_remote(): - second_report = dict(CSV_REPORT, ReportName='second-cur', S3Bucket=SECOND_LOCAL_BUCKET_NAME, S3Prefix='cz') - remote_report = dict(CSV_REPORT, ReportName='remote-cur', S3Bucket=REMOTE_BUCKET_NAME, S3Prefix='cz') - return {'ReportDefinitions': [CSV_REPORT, second_report, remote_report]} + assert _output(context)['MasterPayerBillingBucketArns'] == expected @pytest.mark.unit -def test_handler_master_payer_excludes_remote_cur_buckets( - context, cfn_event, describe_trails_response_local, - list_buckets_response_two_local, describe_report_definitions_response_two_local_one_remote, - describe_organizations_local, +def test_handler_excludes_remote_cur_buckets( + context, cfn_event, list_buckets_response_two_local, describe_organizations_local, ): - context.mock_ct.describe_trails.return_value = describe_trails_response_local - context.mock_cur.describe_report_definitions.return_value = describe_report_definitions_response_two_local_one_remote + second_report = dict(CSV_REPORT, ReportName='second-cur', S3Bucket=SECOND_LOCAL_BUCKET_NAME, S3Prefix='cz') + remote_report = dict(CSV_REPORT, ReportName='remote-cur', S3Bucket=REMOTE_BUCKET_NAME, S3Prefix='cz') + context.mock_cur.describe_report_definitions.return_value = { + 'ReportDefinitions': [CSV_REPORT, second_report, remote_report] + } context.mock_orgs.describe_organization.return_value = describe_organizations_local context.mock_s3.list_buckets.return_value = list_buckets_response_two_local - ret = app.handler(cfn_event, None) - assert ret is None - ((_, _, status, output, _), _) = context.mock_cfnresponse_send.call_args - assert status == cfnresponse.SUCCESS - expected = ','.join([ - f'arn:aws:s3:::{SECOND_LOCAL_BUCKET_NAME}', - f'arn:aws:s3:::{SECOND_LOCAL_BUCKET_NAME}/*', - f'arn:aws:s3:::{LOCAL_BUCKET_NAME}', - f'arn:aws:s3:::{LOCAL_BUCKET_NAME}/*', - ]) - assert output['MasterPayerBillingBucketArns'] == expected - assert f'arn:aws:s3:::{REMOTE_BUCKET_NAME}' not in output['MasterPayerBillingBucketArns'] + app.handler(cfn_event, None) + arns = _output(context)['MasterPayerBillingBucketArns'] + assert f'arn:aws:s3:::{REMOTE_BUCKET_NAME}' not in arns + assert f'arn:aws:s3:::{SECOND_LOCAL_BUCKET_NAME}' in arns + assert f'arn:aws:s3:::{LOCAL_BUCKET_NAME}' in arns def _list_exports_response(*export_arns): @@ -660,104 +344,69 @@ def _get_export_response(bucket_name): } -@pytest.fixture() -def list_buckets_response_local_and_data_export(): - return { - 'Buckets': [ - {'Name': LOCAL_BUCKET_NAME}, - {'Name': DATA_EXPORT_BUCKET_NAME}, - ] - } - - @pytest.mark.unit -def test_handler_master_payer_includes_local_cur2_data_export_bucket( - context, cfn_event, describe_trails_response_local, - list_buckets_response_local_and_data_export, describe_report_definitions_response_local, - describe_organizations_local, +def test_handler_includes_local_cur2_data_export_bucket( + context, cfn_event, describe_report_definitions_response_local, describe_organizations_local, ): - context.mock_ct.describe_trails.return_value = describe_trails_response_local context.mock_cur.describe_report_definitions.return_value = describe_report_definitions_response_local context.mock_bcm.list_exports.return_value = _list_exports_response(EXPORT_ARN) context.mock_bcm.get_export.return_value = _get_export_response(DATA_EXPORT_BUCKET_NAME) context.mock_orgs.describe_organization.return_value = describe_organizations_local - context.mock_s3.list_buckets.return_value = list_buckets_response_local_and_data_export - ret = app.handler(cfn_event, None) - assert ret is None - ((_, _, status, output, _), _) = context.mock_cfnresponse_send.call_args - assert status == cfnresponse.SUCCESS + context.mock_s3.list_buckets.return_value = { + 'Buckets': [{'Name': LOCAL_BUCKET_NAME}, {'Name': DATA_EXPORT_BUCKET_NAME}] + } + app.handler(cfn_event, None) expected = ','.join([ f'arn:aws:s3:::{DATA_EXPORT_BUCKET_NAME}', f'arn:aws:s3:::{DATA_EXPORT_BUCKET_NAME}/*', f'arn:aws:s3:::{LOCAL_BUCKET_NAME}', f'arn:aws:s3:::{LOCAL_BUCKET_NAME}/*', ]) - assert output['MasterPayerBillingBucketArns'] == expected + assert _output(context)['MasterPayerBillingBucketArns'] == expected context.mock_bcm.get_export.assert_called_once_with(ExportArn=EXPORT_ARN) @pytest.mark.unit -def test_handler_master_payer_includes_all_export_buckets_regardless_of_type( - context, cfn_event, describe_trails_response_local, - describe_report_definitions_response_local, describe_organizations_local, +def test_handler_includes_all_export_buckets_regardless_of_type( + context, cfn_event, describe_report_definitions_response_local, describe_organizations_local, ): - # CloudZero only ingests COST_AND_USAGE_REPORT exports, but the IAM policy must grant - # bucket access to every export's destination regardless of type. The discovery code - # never inspects the export type, so two exports pointing at two local buckets both - # get covered. second_export_arn = f'arn:aws:bcm-data-exports:us-east-1:{LOCAL_ACCOUNT_ID}:export/focus-export' export_buckets = {EXPORT_ARN: DATA_EXPORT_BUCKET_NAME, second_export_arn: SECOND_LOCAL_BUCKET_NAME} - context.mock_ct.describe_trails.return_value = describe_trails_response_local context.mock_cur.describe_report_definitions.return_value = describe_report_definitions_response_local context.mock_bcm.list_exports.return_value = _list_exports_response(EXPORT_ARN, second_export_arn) context.mock_bcm.get_export.side_effect = lambda ExportArn: _get_export_response(export_buckets[ExportArn]) context.mock_orgs.describe_organization.return_value = describe_organizations_local context.mock_s3.list_buckets.return_value = { - 'Buckets': [ - {'Name': LOCAL_BUCKET_NAME}, - {'Name': DATA_EXPORT_BUCKET_NAME}, - {'Name': SECOND_LOCAL_BUCKET_NAME}, - ] + 'Buckets': [{'Name': LOCAL_BUCKET_NAME}, {'Name': DATA_EXPORT_BUCKET_NAME}, {'Name': SECOND_LOCAL_BUCKET_NAME}] } - ret = app.handler(cfn_event, None) - assert ret is None - ((_, _, status, output, _), _) = context.mock_cfnresponse_send.call_args - assert status == cfnresponse.SUCCESS - arns = output['MasterPayerBillingBucketArns'] + app.handler(cfn_event, None) + arns = _output(context)['MasterPayerBillingBucketArns'] for bucket in (LOCAL_BUCKET_NAME, DATA_EXPORT_BUCKET_NAME, SECOND_LOCAL_BUCKET_NAME): assert f'arn:aws:s3:::{bucket}' in arns assert f'arn:aws:s3:::{bucket}/*' in arns @pytest.mark.unit -def test_handler_master_payer_excludes_remote_cur2_data_export_bucket( - context, cfn_event, describe_trails_response_local, - list_buckets_response, describe_report_definitions_response_local, - describe_organizations_local, +def test_handler_excludes_remote_cur2_data_export_bucket( + context, cfn_event, list_buckets_response, + describe_report_definitions_response_local, describe_organizations_local, ): - context.mock_ct.describe_trails.return_value = describe_trails_response_local context.mock_cur.describe_report_definitions.return_value = describe_report_definitions_response_local context.mock_bcm.list_exports.return_value = _list_exports_response(EXPORT_ARN) context.mock_bcm.get_export.return_value = _get_export_response(REMOTE_BUCKET_NAME) context.mock_orgs.describe_organization.return_value = describe_organizations_local context.mock_s3.list_buckets.return_value = list_buckets_response - ret = app.handler(cfn_event, None) - assert ret is None - ((_, _, status, output, _), _) = context.mock_cfnresponse_send.call_args - assert status == cfnresponse.SUCCESS - assert output['MasterPayerBillingBucketArns'] == ( - f'arn:aws:s3:::{LOCAL_BUCKET_NAME},arn:aws:s3:::{LOCAL_BUCKET_NAME}/*' - ) - assert f'arn:aws:s3:::{REMOTE_BUCKET_NAME}' not in output['MasterPayerBillingBucketArns'] + app.handler(cfn_event, None) + arns = _output(context)['MasterPayerBillingBucketArns'] + assert arns == f'arn:aws:s3:::{LOCAL_BUCKET_NAME},arn:aws:s3:::{LOCAL_BUCKET_NAME}/*' + assert f'arn:aws:s3:::{REMOTE_BUCKET_NAME}' not in arns @pytest.mark.unit -def test_handler_master_payer_survives_bcm_data_exports_access_denied( - context, cfn_event, describe_trails_response_local, - list_buckets_response, describe_report_definitions_response_local, - describe_organizations_local, +def test_handler_survives_bcm_data_exports_access_denied( + context, cfn_event, list_buckets_response, + describe_report_definitions_response_local, describe_organizations_local, ): - context.mock_ct.describe_trails.return_value = describe_trails_response_local context.mock_cur.describe_report_definitions.return_value = describe_report_definitions_response_local context.mock_bcm.list_exports.side_effect = ClientError( {'Error': {'Code': 'AccessDeniedException', 'Message': 'not authorized to call ListExports'}}, @@ -765,83 +414,53 @@ def test_handler_master_payer_survives_bcm_data_exports_access_denied( ) context.mock_orgs.describe_organization.return_value = describe_organizations_local context.mock_s3.list_buckets.return_value = list_buckets_response - ret = app.handler(cfn_event, None) - assert ret is None - ((_, _, status, output, _), _) = context.mock_cfnresponse_send.call_args - assert status == cfnresponse.SUCCESS - assert output['MasterPayerBillingBucketArns'] == ( + app.handler(cfn_event, None) + assert _output(context)['MasterPayerBillingBucketArns'] == ( f'arn:aws:s3:::{LOCAL_BUCKET_NAME},arn:aws:s3:::{LOCAL_BUCKET_NAME}/*' ) @pytest.mark.unit -def test_handler_master_payer_paginates_list_exports( - context, cfn_event, describe_trails_response_local, - describe_report_definitions_response_local, describe_organizations_local, +def test_handler_paginates_list_exports( + context, cfn_event, describe_report_definitions_client_error, describe_organizations_local, ): - # list_exports is paginated: a NextToken on the first page must drive a second call, - # and exports from every page must be resolved. The two exports live on separate pages. - second_export_arn = f'arn:aws:bcm-data-exports:us-east-1:{LOCAL_ACCOUNT_ID}:export/focus-export' + second_export_arn = f'arn:aws:bcm-data-exports:us-east-1:{LOCAL_ACCOUNT_ID}:export/page-two' export_buckets = {EXPORT_ARN: DATA_EXPORT_BUCKET_NAME, second_export_arn: SECOND_LOCAL_BUCKET_NAME} - context.mock_ct.describe_trails.return_value = describe_trails_response_local - context.mock_cur.describe_report_definitions.return_value = describe_report_definitions_response_local - first_page = _list_exports_response(EXPORT_ARN) - first_page['NextToken'] = 'page-2' - second_page = _list_exports_response(second_export_arn) - context.mock_bcm.list_exports.side_effect = [first_page, second_page] + context.mock_cur.describe_report_definitions.side_effect = describe_report_definitions_client_error + context.mock_bcm.list_exports.side_effect = [ + {'Exports': [{'ExportArn': EXPORT_ARN}], 'NextToken': 'page-2'}, + {'Exports': [{'ExportArn': second_export_arn}]}, + ] context.mock_bcm.get_export.side_effect = lambda ExportArn: _get_export_response(export_buckets[ExportArn]) context.mock_orgs.describe_organization.return_value = describe_organizations_local context.mock_s3.list_buckets.return_value = { - 'Buckets': [ - {'Name': LOCAL_BUCKET_NAME}, - {'Name': DATA_EXPORT_BUCKET_NAME}, - {'Name': SECOND_LOCAL_BUCKET_NAME}, - ] + 'Buckets': [{'Name': DATA_EXPORT_BUCKET_NAME}, {'Name': SECOND_LOCAL_BUCKET_NAME}] } - ret = app.handler(cfn_event, None) - assert ret is None - ((_, _, status, output, _), _) = context.mock_cfnresponse_send.call_args - assert status == cfnresponse.SUCCESS + app.handler(cfn_event, None) + arns = _output(context)['MasterPayerBillingBucketArns'] + assert f'arn:aws:s3:::{DATA_EXPORT_BUCKET_NAME}' in arns + assert f'arn:aws:s3:::{SECOND_LOCAL_BUCKET_NAME}' in arns assert context.mock_bcm.list_exports.call_count == 2 - context.mock_bcm.list_exports.assert_any_call(NextToken='page-2') - arns = output['MasterPayerBillingBucketArns'] - for bucket in (DATA_EXPORT_BUCKET_NAME, SECOND_LOCAL_BUCKET_NAME): - assert f'arn:aws:s3:::{bucket}' in arns - assert f'arn:aws:s3:::{bucket}/*' in arns @pytest.mark.unit -def test_handler_master_payer_isolates_per_export_get_export_failure( - context, cfn_event, describe_trails_response_local, - describe_report_definitions_response_local, describe_organizations_local, +def test_handler_isolates_per_export_get_export_failure( + context, cfn_event, describe_report_definitions_client_error, describe_organizations_local, ): - # A get_export failure on a single export must not drop buckets already resolved from - # the other exports: the failing export is skipped and the rest still get covered. - failing_export_arn = f'arn:aws:bcm-data-exports:us-east-1:{LOCAL_ACCOUNT_ID}:export/broken-export' + bad_export_arn = f'arn:aws:bcm-data-exports:us-east-1:{LOCAL_ACCOUNT_ID}:export/broken' def get_export(ExportArn): - if ExportArn == failing_export_arn: - raise ClientError( - {'Error': {'Code': 'InternalServerException', 'Message': 'transient failure'}}, - 'GetExport', - ) + if ExportArn == bad_export_arn: + raise ClientError({'Error': {'Code': 'InternalFailure', 'Message': 'boom'}}, 'GetExport') return _get_export_response(DATA_EXPORT_BUCKET_NAME) - context.mock_ct.describe_trails.return_value = describe_trails_response_local - context.mock_cur.describe_report_definitions.return_value = describe_report_definitions_response_local - context.mock_bcm.list_exports.return_value = _list_exports_response(failing_export_arn, EXPORT_ARN) + context.mock_cur.describe_report_definitions.side_effect = describe_report_definitions_client_error + context.mock_bcm.list_exports.return_value = _list_exports_response(bad_export_arn, EXPORT_ARN) context.mock_bcm.get_export.side_effect = get_export context.mock_orgs.describe_organization.return_value = describe_organizations_local - context.mock_s3.list_buckets.return_value = { - 'Buckets': [ - {'Name': LOCAL_BUCKET_NAME}, - {'Name': DATA_EXPORT_BUCKET_NAME}, - ] - } - ret = app.handler(cfn_event, None) - assert ret is None - ((_, _, status, output, _), _) = context.mock_cfnresponse_send.call_args - assert status == cfnresponse.SUCCESS - arns = output['MasterPayerBillingBucketArns'] - assert f'arn:aws:s3:::{DATA_EXPORT_BUCKET_NAME}' in arns - assert f'arn:aws:s3:::{DATA_EXPORT_BUCKET_NAME}/*' in arns + context.mock_s3.list_buckets.return_value = {'Buckets': [{'Name': DATA_EXPORT_BUCKET_NAME}]} + app.handler(cfn_event, None) + # The healthy export's bucket is still granted; the failing one is skipped, not fatal. + assert _output(context)['MasterPayerBillingBucketArns'] == ( + f'arn:aws:s3:::{DATA_EXPORT_BUCKET_NAME},arn:aws:s3:::{DATA_EXPORT_BUCKET_NAME}/*' + ) diff --git a/services/notification/src/app.py b/services/notification/src/app.py index 1e66e18..c9a3221 100644 --- a/services/notification/src/app.py +++ b/services/notification/src/app.py @@ -2,59 +2,34 @@ # Copyright (c) 2016-present, CloudZero, Inc. All rights reserved. # Licensed under the BSD-style license. See LICENSE file in the project root for full license information. -import logging +""" +NotifyCloudZero custom-resource handler. + +Posts the result of account provisioning back to the CloudZero reactor. The parent +stack passes every value this needs directly as resource properties (resolved from +`!GetAtt` of the Discovery resource and the AccountResources nested stack), so this +handler no longer reads sibling CloudFormation stack outputs at runtime -- it just +reshapes the properties into the reactor payload and POSTs it. + +IMPORTANT: the reactor payload (`account-link-provisioned` / `-deprovisioned`) is a +FIXED external contract. Do not add, rename, or drop keys. The audit and +cloudtrail-owner account types are deprecated, but their keys remain in the payload +and are emitted as null / no-op values. +""" + import json +import logging -import boto3 import urllib3 -from toolz.curried import assoc_in, get_in, keyfilter, merge, pipe, update_in -from voluptuous import Any, Invalid, Match, Schema, ALLOW_EXTRA, REMOVE_EXTRA +from voluptuous import Any, Match, Schema, ALLOW_EXTRA, REMOVE_EXTRA from src import cfnresponse -cfn = boto3.resource('cloudformation') http = urllib3.PoolManager() logger = logging.getLogger() logger.setLevel(logging.INFO) - -DEFAULT_CFN_COEFFECT = { - 'AuditAccount': { - 'RoleArn': 'null', - }, - 'CloudTrailOwnerAccount': { - 'SQSQueueArn': 'null', - 'SQSQueuePolicyName': 'null' - }, - 'Discovery': { - 'AuditCloudTrailBucketName': 'null', - 'AuditCloudTrailBucketPrefix': 'null', - 'CloudTrailSNSTopicArn': 'null', - 'CloudTrailTrailArn': 'null', - 'VisibleCloudTrailArns': 'null', - 'IsAuditAccount': 'false', - 'IsCloudTrailOwnerAccount': 'false', - 'IsMasterPayerAccount': 'false', - 'IsOrganizationMasterAccount': 'false', - 'IsOrganizationTrail': 'null', - 'IsResourceOwnerAccount': 'false', - 'MasterPayerBillingBucketName': 'null', - 'MasterPayerBillingBucketPath': 'null', - 'BillingReportFormat': 'aws', - 'RemoteCloudTrailBucket': 'true', - }, - 'MasterPayerAccount': { - 'RoleArn': 'null', - 'ReportS3Bucket': 'null', - 'ReportS3Prefix': 'null', - }, - 'ResourceOwnerAccount': { - 'RoleArn': 'null' - }, - 'LegacyAccount': { - 'RoleArn': 'null' - } -} +DEFAULT_BILLING_REPORT_FORMAT = 'aws' ##################### @@ -66,75 +41,40 @@ 'event': { 'RequestType': Any('Create', 'Delete', 'Update'), 'ResourceProperties': { + # Reactor / account metadata 'ExternalId': str, 'ReactorCallbackUrl': str, 'AccountName': str, 'ReactorId': str, 'AccountId': str, 'Region': str, - 'Stacks': { - 'Discovery': str, - 'ResourceOwnerAccount': str, - 'CloudTrailOwnerAccount': str, - 'AuditAccount': str, - 'MasterPayerAccount': str, - 'LegacyAccount': str, - } + # Discovery flags (arrive as the strings 'true' / 'false') + 'IsResourceOwnerAccount': str, + 'IsMasterPayerAccount': str, + 'IsOrganizationMasterAccount': str, + # Provisioned role ARNs ('null' when that account type wasn't provisioned) + 'ResourceOwnerRoleArn': str, + 'MasterPayerRoleArn': str, + # Master-payer billing details + 'MasterPayerBillingBucketName': str, + 'MasterPayerBillingBucketPath': str, + 'MasterPayerReportS3Bucket': str, + 'MasterPayerReportS3Prefix': str, + 'BillingReportFormat': str, }, 'ResponseURL': str, - 'StackId': str + 'StackId': str, } }, required=True, extra=REMOVE_EXTRA) -BOOLEAN_STRING = Schema(Any('null', 'true', 'false')) ARN = Schema(Match(r'^arn:(?:aws|aws-cn|aws-us-gov):([a-z0-9-]+):' r'((?:[a-z0-9-]*)|global):(\d{12}|aws)*:(.+$)$')) -NULLABLE_ARN = Schema(Any('null', ARN)) -NULLABLE_STRING = Schema(Any('null', str)) - -CFN_COEFFECT_SCHEMA = Schema({ - 'AuditAccount': { - 'RoleArn': NULLABLE_ARN - }, - 'CloudTrailOwnerAccount': { - 'SQSQueueArn': NULLABLE_ARN, - 'SQSQueuePolicyName': NULLABLE_STRING, - }, - 'Discovery': { - 'AuditCloudTrailBucketName': NULLABLE_STRING, - 'AuditCloudTrailBucketPrefix': NULLABLE_STRING, - 'CloudTrailSNSTopicArn': NULLABLE_ARN, - 'CloudTrailTrailArn': NULLABLE_ARN, - 'VisibleCloudTrailArns': NULLABLE_STRING, - 'IsAuditAccount': BOOLEAN_STRING, - 'IsCloudTrailOwnerAccount': BOOLEAN_STRING, - 'IsMasterPayerAccount': BOOLEAN_STRING, - 'IsOrganizationMasterAccount': BOOLEAN_STRING, - 'IsOrganizationTrail': BOOLEAN_STRING, - 'IsResourceOwnerAccount': BOOLEAN_STRING, - 'MasterPayerBillingBucketName': NULLABLE_STRING, - 'MasterPayerBillingBucketPath': NULLABLE_STRING, - 'BillingReportFormat': NULLABLE_STRING, - 'RemoteCloudTrailBucket': BOOLEAN_STRING, - }, - 'MasterPayerAccount': { - 'RoleArn': NULLABLE_ARN, - 'ReportS3Bucket': NULLABLE_STRING, - 'ReportS3Prefix': NULLABLE_STRING, - }, - 'ResourceOwnerAccount': { - 'RoleArn': NULLABLE_ARN, - }, - 'LegacyAccount': { - 'RoleArn': NULLABLE_ARN, - } -}, required=True, extra=ALLOW_EXTRA) - - NONEABLE_ARN = Schema(Any(None, ARN)) NONEABLE_BOOL = Schema(Any(None, bool)) NONEABLE_STRING = Schema(Any(None, str)) LINK_ROLE = Schema({'role_arn': NONEABLE_ARN}) + +# The reactor payload contract. FIXED -- do not change its shape. ACCOUNT_LINK_PROVISIONED = Schema({ 'data': { 'metadata': { @@ -174,59 +114,22 @@ } }, required=True, extra=ALLOW_EXTRA) -OUTPUT_SCHEMA = Schema({ - 'output': ACCOUNT_LINK_PROVISIONED, -}, required=True, extra=ALLOW_EXTRA) - - -request_type = get_in(['event', 'RequestType']) -properties = get_in(['event', 'ResourceProperties']) -stacks = get_in(['event', 'ResourceProperties', 'Stacks']) -reactor_callback_url = get_in(['event', 'ResourceProperties', 'ReactorCallbackUrl']) -supported_metadata = {'Region', 'ExternalId', 'AccountId', 'AccountName', 'ReactorId', 'ReactorCallbackUrl'} -callback_metadata = keyfilter(lambda x: x in supported_metadata) -default_metadata = { - 'version': '1', - 'message_source': 'cfn', -} +OUTPUT_SCHEMA = Schema({'output': ACCOUNT_LINK_PROVISIONED}, required=True, extra=ALLOW_EXTRA) ##################### # -# Coeffects, i.e. from the outside world +# Property coercion (CloudFormation passes every property as a string) # ##################### -def coeffects(world): - return pipe(world, - coeffects_cfn) - - -def coeffect(name): - def d(f): - def w(world): - data = {} - try: - data = f(world) - except Exception: - logger.warning(f'Failed to get {name} information.', exc_info=True) - return assoc_in(world, ['coeffects', name], data) - return w - return d +def to_value(s): + """Treat 'null'/empty as absent; otherwise return the string unchanged.""" + return None if s in (None, '', 'null') else s -def outputs_to_dict(outputs): - return { - output['OutputKey']: output['OutputValue'] - for output in outputs or [] - } - - -@coeffect('cloudformation') -def coeffects_cfn(world): - return { - key: outputs_to_dict(cfn.Stack(name).outputs) - for key, name in stacks(world, default={}).items() - } +def to_bool(s): + """Coerce a CloudFormation string flag to a strict bool ('true' -> True, else False).""" + return str(s).lower() == 'true' ##################### @@ -234,109 +137,65 @@ def coeffects_cfn(world): # Business Logic # ##################### -def notify_cloudzero(world): - return pipe(world, - validate_cfn_coeffect, - prepare_output) +def build_payload(properties, message_type): + """Reshape the resource properties into the fixed reactor payload.""" + resource_owner_role_arn = to_value(properties['ResourceOwnerRoleArn']) + master_payer_role_arn = to_value(properties['MasterPayerRoleArn']) + # An existing CUR reports its bucket via discovery; a freshly created CUR reports it + # via the master-payer report outputs. Prefer the discovered one. + billing_bucket_name = to_value(properties['MasterPayerBillingBucketName']) or \ + to_value(properties['MasterPayerReportS3Bucket']) + billing_bucket_path = to_value(properties['MasterPayerBillingBucketPath']) or \ + to_value(properties['MasterPayerReportS3Prefix']) - -def validate_cfn_coeffect(world): - cfn_coeffect = get_in(['coeffects', 'cloudformation'], world) - try: - return update_in(world, ['valid_cfn'], - lambda x: merge(x or {}, CFN_COEFFECT_SCHEMA(cfn_coeffect))) - except Invalid: - logger.warning(cfn_coeffect) - logger.warning('CloudFormation Coeffects are not valid; using defaults', exc_info=True) - return update_in(world, ['valid_cfn'], - lambda x: merge(x or {}, DEFAULT_CFN_COEFFECT)) - - -def null_to_none(s): - return None if s == 'null' else s - - -def string_to_bool(s): - """ - Convert String to Bool - - >>> string_to_bool('True') - True - - >>> string_to_bool('true') - True - - >>> string_to_bool('False') - False - - >>> string_to_bool('false') - False - - >>> string_to_bool('null') - - >>> string_to_bool(None) - - >>> string_to_bool('') - - """ - if not s: - return None - return None if s.lower() == 'null' else s.lower() == 'true' - - -def prepare_output(world): - valid_cfn = get_in(['valid_cfn'], world) - metadata = callback_metadata(properties(world)) - message_type = 'account-link-provisioned' if request_type(world) in {'Create', 'Update'} else 'account-link-deprovisioned' - visible_cloudtrail_arns_string = null_to_none(get_in(['Discovery', 'VisibleCloudTrailArns'], valid_cfn)) - visible_cloudtrail_arns = visible_cloudtrail_arns_string.split(',') if visible_cloudtrail_arns_string else None - master_payer_billing_bucket_name = (null_to_none(get_in(['Discovery', 'MasterPayerBillingBucketName'], valid_cfn)) or - null_to_none(get_in(['MasterPayerAccount', 'ReportS3Bucket'], valid_cfn))) - master_payer_billing_bucket_path = (null_to_none(get_in(['Discovery', 'MasterPayerBillingBucketPath'], valid_cfn)) or - null_to_none(get_in(['MasterPayerAccount', 'ReportS3Prefix'], valid_cfn))) - output = { - **default_metadata, + return { + 'version': '1', + 'message_source': 'cfn', 'message_type': message_type, 'data': { 'metadata': { - 'cloud_region': metadata['Region'], - 'external_id': metadata['ExternalId'], - 'cloud_account_id': metadata['AccountId'], - 'cz_account_name': metadata['AccountName'], - 'reactor_id': metadata['ReactorId'], - 'reactor_callback_url': metadata['ReactorCallbackUrl'], - 'billing_report_format': null_to_none(get_in(['Discovery', 'BillingReportFormat'], valid_cfn)) or 'aws', + 'cloud_region': properties['Region'], + 'external_id': properties['ExternalId'], + 'cloud_account_id': properties['AccountId'], + 'cz_account_name': properties['AccountName'], + 'reactor_id': properties['ReactorId'], + 'reactor_callback_url': properties['ReactorCallbackUrl'], + 'billing_report_format': to_value(properties['BillingReportFormat']) or DEFAULT_BILLING_REPORT_FORMAT, }, 'links': { - 'audit': {'role_arn': null_to_none(get_in(['AuditAccount', 'RoleArn'], valid_cfn))}, - 'cloudtrail_owner': { - 'sqs_queue_arn': null_to_none(get_in(['CloudTrailOwnerAccount', 'SQSQueueArn'], valid_cfn)), - 'sqs_queue_policy_name': null_to_none(get_in(['CloudTrailOwnerAccount', 'SQSQueuePolicyName'], valid_cfn)), - }, - 'master_payer': {'role_arn': null_to_none(get_in(['MasterPayerAccount', 'RoleArn'], valid_cfn))}, - 'resource_owner': {'role_arn': null_to_none(get_in(['ResourceOwnerAccount', 'RoleArn'], valid_cfn))}, - 'legacy': {'role_arn': null_to_none(get_in(['LegacyAccount', 'RoleArn'], valid_cfn))}, + # audit + cloudtrail_owner are deprecated: their slots remain in the + # fixed contract but are always null/no-op now. + 'audit': {'role_arn': None}, + 'cloudtrail_owner': {'sqs_queue_arn': None, 'sqs_queue_policy_name': None}, + 'master_payer': {'role_arn': master_payer_role_arn}, + 'resource_owner': {'role_arn': resource_owner_role_arn}, + 'legacy': {'role_arn': resource_owner_role_arn}, }, 'discovery': { - 'audit_cloudtrail_bucket_name': null_to_none(get_in(['Discovery', 'AuditCloudTrailBucketName'], valid_cfn)), - 'audit_cloudtrail_bucket_prefix': null_to_none(get_in(['Discovery', 'AuditCloudTrailBucketPrefix'], valid_cfn)), - 'cloudtrail_sns_topic_arn': null_to_none(get_in(['Discovery', 'CloudTrailSNSTopicArn'], valid_cfn)), - 'cloudtrail_trail_arn': null_to_none(get_in(['Discovery', 'CloudTrailTrailArn'], valid_cfn)), - - 'is_audit_account': string_to_bool(get_in(['Discovery', 'IsAuditAccount'], valid_cfn)), - 'is_cloudtrail_owner_account': string_to_bool(get_in(['Discovery', 'IsCloudTrailOwnerAccount'], valid_cfn)), - 'is_master_payer_account': string_to_bool(get_in(['Discovery', 'IsMasterPayerAccount'], valid_cfn)), - 'is_organization_master_account': string_to_bool(get_in(['Discovery', 'IsOrganizationMasterAccount'], valid_cfn)), - 'is_organization_trail': string_to_bool(get_in(['Discovery', 'IsOrganizationTrail'], valid_cfn)), - 'is_resource_owner_account': string_to_bool(get_in(['Discovery', 'IsResourceOwnerAccount'], valid_cfn)), - 'master_payer_billing_bucket_name': master_payer_billing_bucket_name, - 'master_payer_billing_bucket_path': master_payer_billing_bucket_path, - 'remote_cloudtrail_bucket': string_to_bool(get_in(['Discovery', 'RemoteCloudTrailBucket'], valid_cfn)), - 'visible_cloudtrail_arns': visible_cloudtrail_arns, + # Deprecated cloudtrail/audit discovery fields -- retained as null/no-op + # for contract stability (see module docstring). + 'audit_cloudtrail_bucket_name': None, + 'audit_cloudtrail_bucket_prefix': None, + 'cloudtrail_sns_topic_arn': None, + 'cloudtrail_trail_arn': None, + 'is_audit_account': False, + 'is_cloudtrail_owner_account': False, + 'is_organization_trail': None, + 'remote_cloudtrail_bucket': True, + 'visible_cloudtrail_arns': None, + # Live discovery values + 'is_master_payer_account': to_bool(properties['IsMasterPayerAccount']), + 'is_organization_master_account': to_bool(properties['IsOrganizationMasterAccount']), + 'is_resource_owner_account': to_bool(properties['IsResourceOwnerAccount']), + 'master_payer_billing_bucket_name': billing_bucket_name, + 'master_payer_billing_bucket_path': billing_bucket_path, } } } - return update_in(world, ['output'], lambda x: merge(x or {}, output)) + + +def message_type_for(request_type): + return 'account-link-provisioned' if request_type in {'Create', 'Update'} else 'account-link-deprovisioned' ##################### @@ -344,31 +203,10 @@ def prepare_output(world): # Effects, i.e. changes to the outside world # ##################### -def effects(world): - return pipe(world, - effects_reactor_callback) - - -def effect(name): - def d(f): - def w(world): - data = {} - try: - data = f(world) - except Exception: - logger.warning(f'Failed to effect {name} change.', exc_info=True) - return assoc_in(world, ['effects', name], data) - return w - return d - - -@effect('reactor') -def effects_reactor_callback(world): - url = reactor_callback_url(world) - data = get_in(['output'], world) - data_string = json.dumps(data) - logger.info(f'Posting to {url} this data: {data_string}') - response = http.request('POST', url, body=data_string.encode('utf-8')) +def post_to_reactor(url, payload): + body = json.dumps(payload) + logger.info(f'Posting to {url} this data: {body}') + response = http.request('POST', url, body=body.encode('utf-8')) response_text = response.data.decode('utf-8') logger.info(f'response {response.status}; text {response_text}') assert response.status == 200 @@ -382,18 +220,15 @@ def effects_reactor_callback(world): ##################### def handler(event, context, **kwargs): status = cfnresponse.SUCCESS - world = {} + payload = {} try: logger.info(f'Processing event {json.dumps(event)}') - world = pipe({'event': event, 'kwargs': kwargs}, - INPUT_SCHEMA, - coeffects, - notify_cloudzero, - effects, - OUTPUT_SCHEMA) + validated = INPUT_SCHEMA({'event': event})['event'] + properties = validated['ResourceProperties'] + payload = build_payload(properties, message_type_for(validated['RequestType'])) + OUTPUT_SCHEMA({'output': payload}) # validate the fixed contract before sending + post_to_reactor(properties['ReactorCallbackUrl'], payload) except Exception as err: logger.exception(err) finally: - output = world.get('output') - logger.info(f'Sending output {output}') - cfnresponse.send(event, context, status, output, event.get('PhysicalResourceId')) + cfnresponse.send(event, context, status, payload, event.get('PhysicalResourceId')) diff --git a/services/notification/tests/unit/test_app.py b/services/notification/tests/unit/test_app.py index ccd7f2f..872bb8f 100644 --- a/services/notification/tests/unit/test_app.py +++ b/services/notification/tests/unit/test_app.py @@ -3,116 +3,55 @@ # Licensed under the BSD-style license. See LICENSE file in the project root for full license information. import os -import random +import json from collections import namedtuple import pytest -import json import src.app as app from src import cfnresponse -EXPECTED_URL = 'url' - - -# TODO: Replace these fixtures with Voluptuous Schemas + Hypothesis + Property Tests -@pytest.fixture(scope='function') -def cfn_event(): +EXPECTED_URL = 'https://api.cloudzero.com/accounts/v1/link' +ACCOUNT_ID = '123456789012' +RESOURCE_OWNER_ROLE_ARN = f'arn:aws:iam::{ACCOUNT_ID}:role/cloudzero/resource-owner' +MASTER_PAYER_ROLE_ARN = f'arn:aws:iam::{ACCOUNT_ID}:role/cloudzero/master-payer' + + +def make_event(request_type='Create', **overrides): + properties = { + 'AccountId': ACCOUNT_ID, + 'Region': 'us-east-1', + 'ExternalId': 'ext-id', + 'ReactorCallbackUrl': EXPECTED_URL, + 'AccountName': 'my-account', + 'ReactorId': 'reactor-1', + 'IsResourceOwnerAccount': 'true', + 'IsMasterPayerAccount': 'false', + 'IsOrganizationMasterAccount': 'false', + 'ResourceOwnerRoleArn': 'null', + 'MasterPayerRoleArn': 'null', + 'MasterPayerBillingBucketName': 'null', + 'MasterPayerBillingBucketPath': 'null', + 'MasterPayerReportS3Bucket': 'null', + 'MasterPayerReportS3Prefix': 'null', + 'BillingReportFormat': 'null', + } + properties.update(overrides) return { 'LogicalResourceId': 'some logical resource id', 'PhysicalResourceId': None, 'RequestId': 'some request id', - 'RequestType': 'Create', - 'ResourceProperties': { - 'AccountId': 'str', - 'Region': 'str', - 'ExternalId': 'str', - 'ReactorCallbackUrl': EXPECTED_URL, - 'AccountName': 'str', - 'ReactorId': 'str', - 'Stacks': { - 'AuditAccount': 'stack-arn', - 'CloudTrailOwnerAccount': 'stack-arn', - 'Discovery': 'stack-arn', - 'MasterPayerAccount': 'stack-arn', - 'ResourceOwnerAccount': 'stack-arn', - 'LegacyAccount': 'stack-arn', - } - }, + 'RequestType': request_type, + 'ResourceProperties': properties, 'ResponseURL': 'https://cfn.amazonaws.com/callback', 'StackId': 'some-cfn-stack-id', } -def random_bool(): - return bool(random.getrandbits(1)) - - -def nullable_arn(): - return 'arn:aws:lambda:us-east-1:999999999999:function:name' if random_bool() else 'null' - - -def nullable_string(): - return 'foo' if random_bool() else 'null' - - -def boolean_string(): - return 'True' if random_bool() else 'False' - - -@pytest.fixture(scope='function') -def cfn_coeffect(): - return { - 'AuditAccount': { - 'RoleArn': nullable_arn() - }, - 'CloudTrailOwnerAccount': { - 'SQSQueueArn': nullable_arn(), - 'SQSQueuePolicyName': nullable_string(), - }, - 'Discovery': { - 'AuditCloudTrailBucketName': nullable_string(), - 'AuditCloudTrailBucketPrefix': nullable_string(), - 'CloudTrailSNSTopicArn': nullable_arn(), - 'CloudTrailTrailArn': nullable_arn(), - 'VisibleCloudTrailArns': nullable_string(), - 'IsAuditAccount': boolean_string(), - 'IsCloudTrailOwnerAccount': boolean_string(), - 'IsMasterPayerAccount': boolean_string(), - 'IsOrganizationMasterAccount': boolean_string(), - 'IsOrganizationTrail': boolean_string(), - 'IsResourceOwnerAccount': boolean_string(), - 'MasterPayerBillingBucketName': nullable_string(), - 'MasterPayerBillingBucketPath': nullable_string(), - 'BillingReportFormat': nullable_string(), - 'RemoteCloudTrailBucket': boolean_string(), - }, - 'MasterPayerAccount': { - 'RoleArn': nullable_arn(), - 'ReportS3Bucket': nullable_string(), - 'ReportS3Prefix': nullable_string(), - }, - 'ResourceOwnerAccount': { - 'RoleArn': nullable_arn(), - }, - 'LegacyAccount': { - 'RoleArn': nullable_arn(), - } - } - - -class Response: - def __init__(self, status_code, json_data={}, data={}): - self.status_code = status_code - self._json = json_data - self._data = {} - self.text = json.dumps(self._json) - - def json(self): - return self._json - - def data(self): - return self._data +class FakeResponse: + def __init__(self, status=200, body=b'ok'): + self.status = status + self.data = body @pytest.fixture(scope='function') @@ -123,26 +62,51 @@ def context(mocker): context.prefix = app.__name__ context.mock_cfnresponse_send = mocker.patch(f'{context.prefix}.cfnresponse.send', autospec=True) context.mock_http = mocker.patch(f'{context.prefix}.http') - context.mock_cfn = mocker.patch(f'{context.prefix}.cfn', autospec=True) + context.mock_http.request.return_value = FakeResponse() yield context os.environ = orig_env mocker.stopall() -@pytest.mark.unit -def test_handler_no_cfn_coeffects(context, cfn_event): - response = Response(200) - context.mock_http.return_value = response - ret = app.handler(cfn_event, None) - assert ret is None - assert context.mock_cfnresponse_send.call_count == 1 - assert context.mock_http.request.call_count == 1 +def _sent_output(context): ((_, _, status, output, _), _) = context.mock_cfnresponse_send.call_args + assert status == cfnresponse.SUCCESS + return output + + +@pytest.mark.unit +def test_handler_posts_fixed_contract_payload(context): + event = make_event( + IsMasterPayerAccount='true', + IsOrganizationMasterAccount='true', + ResourceOwnerRoleArn=RESOURCE_OWNER_ROLE_ARN, + MasterPayerRoleArn=MASTER_PAYER_ROLE_ARN, + MasterPayerBillingBucketName='cur-bucket', + MasterPayerBillingBucketPath='cloudzero/report', + BillingReportFormat='aws', + ) + app.handler(event, None) expected = { 'version': '1', 'message_source': 'cfn', 'message_type': 'account-link-provisioned', 'data': { + 'metadata': { + 'cloud_region': 'us-east-1', + 'external_id': 'ext-id', + 'cloud_account_id': ACCOUNT_ID, + 'cz_account_name': 'my-account', + 'reactor_id': 'reactor-1', + 'reactor_callback_url': EXPECTED_URL, + 'billing_report_format': 'aws', + }, + 'links': { + 'audit': {'role_arn': None}, + 'cloudtrail_owner': {'sqs_queue_arn': None, 'sqs_queue_policy_name': None}, + 'master_payer': {'role_arn': MASTER_PAYER_ROLE_ARN}, + 'resource_owner': {'role_arn': RESOURCE_OWNER_ROLE_ARN}, + 'legacy': {'role_arn': RESOURCE_OWNER_ROLE_ARN}, + }, 'discovery': { 'audit_cloudtrail_bucket_name': None, 'audit_cloudtrail_bucket_prefix': None, @@ -150,55 +114,42 @@ def test_handler_no_cfn_coeffects(context, cfn_event): 'cloudtrail_trail_arn': None, 'is_audit_account': False, 'is_cloudtrail_owner_account': False, - 'is_master_payer_account': False, 'is_organization_trail': None, - 'is_organization_master_account': False, - 'is_resource_owner_account': False, - 'master_payer_billing_bucket_name': None, - 'master_payer_billing_bucket_path': None, 'remote_cloudtrail_bucket': True, 'visible_cloudtrail_arns': None, + 'is_master_payer_account': True, + 'is_organization_master_account': True, + 'is_resource_owner_account': True, + 'master_payer_billing_bucket_name': 'cur-bucket', + 'master_payer_billing_bucket_path': 'cloudzero/report', }, - 'metadata': { - 'cloud_region': 'str', - 'cz_account_name': 'str', - 'cloud_account_id': 'str', - 'reactor_callback_url': EXPECTED_URL, - 'external_id': 'str', - 'reactor_id': 'str', - 'billing_report_format': 'aws', - }, - 'links': { - 'audit': {'role_arn': None}, - 'legacy': {'role_arn': None}, - 'master_payer': {'role_arn': None}, - 'resource_owner': {'role_arn': None}, - 'cloudtrail_owner': { - 'sqs_queue_arn': None, - 'sqs_queue_policy_name': None, - }, - } - } + }, } - assert status == cfnresponse.SUCCESS - assert output == expected + assert _sent_output(context) == expected (args, kwargs) = context.mock_http.request.call_args assert args == ('POST', EXPECTED_URL) assert json.loads(kwargs['body']) == expected @pytest.mark.unit -def test_prepare_output(context, cfn_event, cfn_coeffect): - world = { - 'event': cfn_event, - 'valid_cfn': cfn_coeffect, - } - is_master_payer_account = bool(cfn_coeffect['Discovery']['IsMasterPayerAccount'] == 'True') - raw_format = cfn_coeffect['Discovery']['BillingReportFormat'] - expected_billing_format = 'aws' if raw_format == 'null' else raw_format - new_world = app.prepare_output(world) - assert new_world['output']['data']['discovery']['is_master_payer_account'] == is_master_payer_account - assert new_world['output']['data']['metadata']['billing_report_format'] == expected_billing_format +def test_handler_deprecated_audit_cloudtrail_fields_are_null(context): + app.handler(make_event(), None) + data = _sent_output(context)['data'] + assert data['links']['audit'] == {'role_arn': None} + assert data['links']['cloudtrail_owner'] == {'sqs_queue_arn': None, 'sqs_queue_policy_name': None} + assert data['discovery']['is_audit_account'] is False + assert data['discovery']['is_cloudtrail_owner_account'] is False + assert data['discovery']['cloudtrail_sns_topic_arn'] is None + assert data['discovery']['audit_cloudtrail_bucket_name'] is None + + +@pytest.mark.unit +def test_handler_legacy_aliases_resource_owner_role(context): + event = make_event(ResourceOwnerRoleArn=RESOURCE_OWNER_ROLE_ARN) + app.handler(event, None) + links = _sent_output(context)['data']['links'] + assert links['resource_owner']['role_arn'] == RESOURCE_OWNER_ROLE_ARN + assert links['legacy']['role_arn'] == RESOURCE_OWNER_ROLE_ARN @pytest.mark.unit @@ -207,11 +158,69 @@ def test_prepare_output(context, cfn_event, cfn_coeffect): ('null', 'aws'), ('aws', 'aws'), ]) -def test_prepare_output_billing_format(context, cfn_event, cfn_coeffect, raw_format, expected): - cfn_coeffect['Discovery']['BillingReportFormat'] = raw_format - world = { - 'event': cfn_event, - 'valid_cfn': cfn_coeffect, - } - new_world = app.prepare_output(world) - assert new_world['output']['data']['metadata']['billing_report_format'] == expected +def test_handler_billing_format_fallback(context, raw_format, expected): + app.handler(make_event(BillingReportFormat=raw_format), None) + assert _sent_output(context)['data']['metadata']['billing_report_format'] == expected + + +@pytest.mark.unit +def test_handler_billing_bucket_falls_back_to_created_report(context): + # No existing discovered bucket, but a CUR was freshly created -> use the report outputs. + event = make_event( + MasterPayerBillingBucketName='null', + MasterPayerBillingBucketPath='null', + MasterPayerReportS3Bucket='new-cur-bucket', + MasterPayerReportS3Prefix='cloudzero/cloudzero-cur-hourly-csv', + ) + app.handler(event, None) + discovery = _sent_output(context)['data']['discovery'] + assert discovery['master_payer_billing_bucket_name'] == 'new-cur-bucket' + assert discovery['master_payer_billing_bucket_path'] == 'cloudzero/cloudzero-cur-hourly-csv' + + +@pytest.mark.unit +def test_handler_delete_sends_deprovisioned(context): + app.handler(make_event(request_type='Delete'), None) + assert _sent_output(context)['message_type'] == 'account-link-deprovisioned' + + +@pytest.mark.unit +def test_handler_posts_once_and_responds_success(context): + app.handler(make_event(), None) + assert context.mock_http.request.call_count == 1 + assert context.mock_cfnresponse_send.call_count == 1 + + +# ---- cfnresponse (vendored AWS helper that ships in the Lambda) ---- + +CfnContext = namedtuple('CfnContext', ['log_stream_name']) + +CFN_RESPONSE_EVENT = { + 'ResponseURL': 'https://cfn.amazonaws.com/presigned', + 'StackId': 'stack-id', + 'RequestId': 'request-id', + 'LogicalResourceId': 'logical-id', +} + + +@pytest.mark.unit +def test_cfnresponse_send_puts_response_body(mocker): + mock_http = mocker.patch(f'{cfnresponse.__name__}.http') + mock_http.request.return_value = FakeResponse() + mock_http.request.return_value.reason = 'OK' + cfnresponse.send(CFN_RESPONSE_EVENT, CfnContext('log-stream'), cfnresponse.SUCCESS, {'k': 'v'}) + (args, kwargs) = mock_http.request.call_args + assert args[0] == 'PUT' + assert args[1] == CFN_RESPONSE_EVENT['ResponseURL'] + body = json.loads(kwargs['body']) + assert body['Status'] == cfnresponse.SUCCESS + assert body['PhysicalResourceId'] == 'log-stream' # falls back to log_stream_name + assert body['Data'] == {'k': 'v'} + + +@pytest.mark.unit +def test_cfnresponse_send_swallows_http_errors(mocker): + mock_http = mocker.patch(f'{cfnresponse.__name__}.http') + mock_http.request.side_effect = Exception('network down') + # Must not raise -- a failed callback should never fail the custom resource. + cfnresponse.send(CFN_RESPONSE_EVENT, CfnContext('log-stream'), cfnresponse.FAILED, {}, physicalResourceId='pid') From 0f5f29c3b7cf4ee59545335d0b94b4addf9d5723 Mon Sep 17 00:00:00 2001 From: Erik Peterson Date: Tue, 16 Jun 2026 14:34:10 -0400 Subject: [PATCH 4/6] Address PR review: redact sensitive logs, harden BCM error handling - CodeQL (py/clear-text-logging-sensitive-data): stop logging the full event / payload / output in the discovery and notification handlers; they carry ExternalId and account-identifying fields. Log only non-sensitive summary fields. - Broaden the BCM Data Exports list/get catch to BotoCoreError so connectivity errors degrade gracefully (per review feedback). - Use lazy %s logging for the per-export GetExport warning. - Add tests for an export with no destination bucket (the if-bucket guard). - Drop now-unused toolz from discovery/notification requirements. Author: Erik Peterson --- services/discovery/requirements.txt | 1 - services/discovery/src/app.py | 19 ++++++++++++------- services/discovery/tests/unit/test_app.py | 17 +++++++++++++++++ services/notification/requirements.txt | 1 - services/notification/src/app.py | 8 +++++--- 5 files changed, 34 insertions(+), 12 deletions(-) diff --git a/services/discovery/requirements.txt b/services/discovery/requirements.txt index a578c59..9ba4611 100644 --- a/services/discovery/requirements.txt +++ b/services/discovery/requirements.txt @@ -4,4 +4,3 @@ boto3>=1.34.91 urllib3>=2.6.3 voluptuous>=0.14.2 -toolz>=0.12.1 diff --git a/services/discovery/src/app.py b/services/discovery/src/app.py index e34e656..3882ef0 100644 --- a/services/discovery/src/app.py +++ b/services/discovery/src/app.py @@ -22,7 +22,7 @@ import logging import boto3 -from botocore.exceptions import ClientError +from botocore.exceptions import BotoCoreError, ClientError from voluptuous import Any, ExactSequence, Schema, ALLOW_EXTRA, REMOVE_EXTRA from src import cfnresponse @@ -119,19 +119,21 @@ def list_data_export_bucket_names(): next_token = response.get('NextToken') if not next_token: break - except ClientError: + except (ClientError, BotoCoreError): logger.warning('Failed to access BCM Data Exports ListExports', exc_info=True) return [] # Resolve each export independently: a transient failure on one export should not # drop the buckets already resolved from the others, so isolate the get_export call - # per export rather than wrapping the whole loop in a single try/except. + # per export rather than wrapping the whole loop in a single try/except. Catch + # BotoCoreError too (e.g. EndpointConnectionError) so connectivity blips degrade + # gracefully rather than failing the stack. buckets = [] for export_arn in export_arns: try: export = bcm.get_export(ExportArn=export_arn).get('Export', {}) - except ClientError: - logger.warning(f'Failed to access BCM Data Exports GetExport for {export_arn}', exc_info=True) + except (ClientError, BotoCoreError): + logger.warning('Failed to access BCM Data Exports GetExport for %s', export_arn, exc_info=True) continue bucket = export.get('DestinationConfigurations', {}).get('S3Destination', {}).get('S3Bucket') if bucket: @@ -305,12 +307,15 @@ def handler(event, context, **kwargs): status = cfnresponse.SUCCESS output = DEFAULT_OUTPUT try: - logger.info(f'Processing event {event}') + # Avoid logging the full event/output: they carry account-identifying fields + # that CodeQL flags as clear-text logging of sensitive data. + logger.info('Processing %s discovery request', event.get('RequestType')) validated = INPUT_SCHEMA({'event': event}) account_id = validated['event']['ResourceProperties']['AccountId'] output = OUTPUT_SCHEMA({'output': discover(account_id)})['output'] except Exception as err: logger.exception(err) finally: - logger.info(f'Sending output {output}') + logger.info('Discovery complete: IsMasterPayerAccount=%s IsResourceOwnerAccount=%s', + output.get('IsMasterPayerAccount'), output.get('IsResourceOwnerAccount')) cfnresponse.send(event, context, status, output, event.get('PhysicalResourceId')) diff --git a/services/discovery/tests/unit/test_app.py b/services/discovery/tests/unit/test_app.py index edddf9a..433084c 100644 --- a/services/discovery/tests/unit/test_app.py +++ b/services/discovery/tests/unit/test_app.py @@ -443,6 +443,23 @@ def test_handler_paginates_list_exports( assert context.mock_bcm.list_exports.call_count == 2 +@pytest.mark.unit +def test_handler_skips_export_with_no_destination_bucket( + context, cfn_event, describe_report_definitions_client_error, describe_organizations_local, +): + # An export whose S3Destination has no S3Bucket must be silently skipped, not added + # as an empty/None bucket. + no_bucket_export = {'Export': {'ExportArn': EXPORT_ARN, 'Name': 'no-bucket', + 'DestinationConfigurations': {'S3Destination': {'S3Prefix': 'x'}}}} + context.mock_cur.describe_report_definitions.side_effect = describe_report_definitions_client_error + context.mock_bcm.list_exports.return_value = _list_exports_response(EXPORT_ARN) + context.mock_bcm.get_export.return_value = no_bucket_export + context.mock_orgs.describe_organization.return_value = describe_organizations_local + context.mock_s3.list_buckets.return_value = {'Buckets': [{'Name': DATA_EXPORT_BUCKET_NAME}]} + app.handler(cfn_event, None) + assert _output(context)['MasterPayerBillingBucketArns'] == '' + + @pytest.mark.unit def test_handler_isolates_per_export_get_export_failure( context, cfn_event, describe_report_definitions_client_error, describe_organizations_local, diff --git a/services/notification/requirements.txt b/services/notification/requirements.txt index a578c59..9ba4611 100644 --- a/services/notification/requirements.txt +++ b/services/notification/requirements.txt @@ -4,4 +4,3 @@ boto3>=1.34.91 urllib3>=2.6.3 voluptuous>=0.14.2 -toolz>=0.12.1 diff --git a/services/notification/src/app.py b/services/notification/src/app.py index c9a3221..b0106e9 100644 --- a/services/notification/src/app.py +++ b/services/notification/src/app.py @@ -204,11 +204,13 @@ def message_type_for(request_type): # ##################### def post_to_reactor(url, payload): + # Do not log the payload or full event: they carry the ExternalId and other + # account-identifying fields that CodeQL flags as clear-text logging of sensitive data. body = json.dumps(payload) - logger.info(f'Posting to {url} this data: {body}') + logger.info('Posting %s to the reactor', payload.get('message_type')) response = http.request('POST', url, body=body.encode('utf-8')) response_text = response.data.decode('utf-8') - logger.info(f'response {response.status}; text {response_text}') + logger.info('Reactor responded with status %s', response.status) assert response.status == 200 return response_text @@ -222,7 +224,7 @@ def handler(event, context, **kwargs): status = cfnresponse.SUCCESS payload = {} try: - logger.info(f'Processing event {json.dumps(event)}') + logger.info('Processing %s notification', event.get('RequestType')) validated = INPUT_SCHEMA({'event': event})['event'] properties = validated['ResourceProperties'] payload = build_payload(properties, message_type_for(validated['RequestType'])) From 82983fdfc455a8b0600c3f599f02f7be4dbaeda4 Mon Sep 17 00:00:00 2001 From: Erik Peterson Date: Tue, 16 Jun 2026 14:41:35 -0400 Subject: [PATCH 5/6] Fix remaining CodeQL clear-text-logging alerts The redacted logs still read tainted values: the notification payload dict carries ExternalId (so any field read is flagged) and the discovery log interpolated the CUR bucket name. Log a static message when posting to the reactor, and log only the (literal) billing report format when selecting a CUR. Author: Erik Peterson --- services/discovery/src/app.py | 3 ++- services/notification/src/app.py | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/services/discovery/src/app.py b/services/discovery/src/app.py index 3882ef0..eb5ae61 100644 --- a/services/discovery/src/app.py +++ b/services/discovery/src/app.py @@ -235,7 +235,8 @@ def select_ingest_cur(report_definitions, local_bucket_names): if report.get('S3Bucket') in local_bucket_names and matches_schema(schema, report): bucket_name = report['S3Bucket'] bucket_path = f"{report.get('S3Prefix', '')}/{report.get('ReportName', '')}" - logger.info(f'Selected ingest CUR ({billing_report_format}) in bucket {bucket_name}') + # Log only the format literal, not the bucket name (avoid logging account data). + logger.info('Selected ingest CUR with format %s', billing_report_format) return bucket_name, bucket_path, billing_report_format return None, None, 'aws' diff --git a/services/notification/src/app.py b/services/notification/src/app.py index b0106e9..1fb83d6 100644 --- a/services/notification/src/app.py +++ b/services/notification/src/app.py @@ -207,7 +207,9 @@ def post_to_reactor(url, payload): # Do not log the payload or full event: they carry the ExternalId and other # account-identifying fields that CodeQL flags as clear-text logging of sensitive data. body = json.dumps(payload) - logger.info('Posting %s to the reactor', payload.get('message_type')) + # Static message only: the payload dict carries ExternalId, so reading any field of + # it into a log is flagged by CodeQL as clear-text logging of sensitive data. + logger.info('Posting notification to the reactor') response = http.request('POST', url, body=body.encode('utf-8')) response_text = response.data.decode('utf-8') logger.info('Reactor responded with status %s', response.status) From 963e74f7f2bbc81b4f083c7b4ea7009e3a18f827 Mon Sep 17 00:00:00 2001 From: Erik Peterson Date: Tue, 16 Jun 2026 14:48:30 -0400 Subject: [PATCH 6/6] Drop CUR-format log line that trips CodeQL false positive CodeQL's clear-text-logging 'private data' heuristic flags the variable name billing_report_format (contains 'billing'), not its value (a literal like 'aws'). The log carried no real value beyond the format already present in the discovery output, so remove it rather than fight the scanner. Author: Erik Peterson --- services/discovery/src/app.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/services/discovery/src/app.py b/services/discovery/src/app.py index eb5ae61..5647d57 100644 --- a/services/discovery/src/app.py +++ b/services/discovery/src/app.py @@ -235,8 +235,6 @@ def select_ingest_cur(report_definitions, local_bucket_names): if report.get('S3Bucket') in local_bucket_names and matches_schema(schema, report): bucket_name = report['S3Bucket'] bucket_path = f"{report.get('S3Prefix', '')}/{report.get('ReportName', '')}" - # Log only the format literal, not the bucket name (avoid logging account data). - logger.info('Selected ingest CUR with format %s', billing_report_format) return bucket_name, bucket_path, billing_report_format return None, None, 'aws'