From 76187cd5550f4344be5e311cba23abc941dcb3b8 Mon Sep 17 00:00:00 2001 From: Iakov GAN <82834333+iakov-aws@users.noreply.github.com> Date: Wed, 23 Aug 2023 15:58:59 +0300 Subject: [PATCH 1/6] doc and test improvements (#603) --- README.md | 14 ++++-- cid/cli.py | 11 ++++- cid/commands/init_qs.py | 33 ++++++------- cid/helpers/quicksight/__init__.py | 23 +++++---- .../create-qs-subscription.bats | 47 +++++++++++++++++++ 5 files changed, 95 insertions(+), 33 deletions(-) create mode 100644 cid/test/bats/20-init-quicksight/create-qs-subscription.bats diff --git a/README.md b/README.md index 6869f922..a5bf473d 100644 --- a/README.md +++ b/README.md @@ -79,6 +79,14 @@ cid-cmd status cid-cmd share ``` +#### Initialize Amazon QuickSight +One time action to intialize Amazon QuickSight Enerprise Edition. + +```bash +cid-cmd initqs +``` + + #### Delete Dashboard and all dependencies unused by other Delete Dashboards and all dependencies unused by other CID-managed dashboards.(including QuickSight datasets, Athena views and tables) ```bash @@ -129,9 +137,7 @@ CID offers a set of Terraform modules to deploy CUR replicaion and CID dashboard ## Rights Management -The ownership of CID is usually with the FinOps team, who do not have administrative access. However, they require specific privileges to install and operate CID -dashboards. To assist the Admin team in granting the necessary privileges to the CID owners, a CFN template is provided. This template, located at -[CFN template](cfn-templates/cid-admin-policies.yaml), takes an IAM role name as a parameter and adds the required policies to the role. +The ownership of CID is usually with the FinOps team, who do not have administrative access. However, they require specific privileges to install and operate CID dashboards. To assist the Admin team in granting the necessary privileges to the CID owners, a CFN template is provided. This template, located at [CFN template](cfn-templates/cid-admin-policies.yaml), takes an IAM role name as a parameter and adds the required policies to the role. ## Troubleshooting and Support @@ -143,6 +149,6 @@ cid-cmd -vv [command] This will produce a log file in the same directory that were at the tile of launch of cid-cmd. -:heavy_exclamation_mark:Inspect the produced debug log for any sensitive information and anonymise it. +:heavy_exclamation_mark:Inspect the produced debug log for any sensitive information and anonymize it. We encourage you to open [new issue](https://github.com/aws-samples/aws-cudos-framework-deployment/issues/new) with description of the problem and attached debug log file. \ No newline at end of file diff --git a/cid/cli.py b/cid/cli.py index 1da41629..eb5d408f 100644 --- a/cid/cli.py +++ b/cid/cli.py @@ -225,9 +225,18 @@ def share(ctx, dashboard_id, **kwargs): ctx.obj.share(dashboard_id) +@click.option('-v', '--verbose', count=True) +@click.option('-y', '--yes', help='confirm all', is_flag=True, default=False) @cid_command def initqs(ctx, **kwargs): - """Initialize QuickSight resources for deployment""" + """Initialize Amazon QuickSight + + \b + + --enable-quicksight-enterprise (yes|no) Confirm the activation of QuickSight + --account-name NAME Unique QuickSight account name (Unique across all AWS users) + --notification-email EMAIL User's email for QuickSight notificaitons + """ ctx.obj.initqs(**kwargs) diff --git a/cid/commands/init_qs.py b/cid/commands/init_qs.py index 32f7fac1..a3c97e11 100644 --- a/cid/commands/init_qs.py +++ b/cid/commands/init_qs.py @@ -25,7 +25,7 @@ def __init__(self, cid, **kwargs): def execute(self, *args, **kwargs): """Execute the initilization""" - self._create_quicksight_enterprise_subscription() # No tagging available + self._create_quicksight_enterprise_subscription() def _create_quicksight_enterprise_subscription(self): """Enable QuickSight Enterprise if not enabled already""" @@ -50,7 +50,13 @@ def _create_quicksight_enterprise_subscription(self): for counter in range(MAX_ITERATIONS): email = self._get_email_for_quicksight() account_name = self._get_account_name_for_quicksight() - params = self._get_quicksight_params(email, account_name) + params = { + 'Edition': 'ENTERPRISE', + 'AuthenticationMethod': 'IAM_AND_QUICKSIGHT', + 'AwsAccountId': self.cid.base.account_id, + 'AccountName': account_name, + 'NotificationEmail': email, + } try: response = self.cid.qs.client.create_account_subscription(**params) logger.debug(f'create_account_subscription resp: {response}') @@ -59,29 +65,19 @@ def _create_quicksight_enterprise_subscription(self): break except Exception as exc: #pylint: disable=broad-exception-caught cid_print(f'\tQuickSight Edition...\tError ({exc}). Please, try again or press CTRL + C to interrupt.') - unset_parameter('qs-account-name') - unset_parameter('qs-notification-email') + unset_parameter('account-name') + unset_parameter('notification-email') if counter == MAX_ITERATIONS - 1: raise CidCritical('Quicksight setup failed') from exc while self.cid.qs.edition(fresh=True) not in ('ENTERPRISE', 'ENTERPRISE_AND_Q'): time.sleep(5) cid_print(f'\tQuickSight Edition is {self.cid.qs.edition()}.') - def _get_quicksight_params(self, email, account_name): - """Create dictionary of quicksight subscription initialization parameters""" - return { - 'Edition': 'ENTERPRISE', - 'AuthenticationMethod': 'IAM_AND_QUICKSIGHT', - 'AwsAccountId': self.cid.base.account_id, - 'AccountName': account_name, # Should be a parameter with a reasonable default - 'NotificationEmail': email, # Read the value from account parameters as a default - } - def _get_account_name_for_quicksight(self): """Get the account name for quicksight""" for _ in range(MAX_ITERATIONS): account_name = get_parameter( - 'qs-account-name', + 'account-name', message=( '\n\tPlease, choose a descriptive name for your QuickSight account. ' 'This will be used later to share it with your users. This can NOT be changed later.' @@ -91,15 +87,16 @@ def _get_account_name_for_quicksight(self): if account_name: return account_name print('\t The account name must not be empty. Please, try again.') - unset_parameter('qs-account-name') + unset_parameter('account-name') else: #pylint: disable=W0120:useless-else-on-loop raise CidCritical('Failed to read QuickSight Account Name') + def _get_email_for_quicksight(self): """Get email for quicksight""" for _ in range(MAX_ITERATIONS): email = get_parameter( - 'qs-notification-email', + 'notification-email', message=( 'Amazon QuickSight needs your email address to send notifications ' 'regarding your Amazon QuickSight account.' @@ -109,6 +106,6 @@ def _get_email_for_quicksight(self): if '@' in email and '.' in email: return email cid_print(f'\t{email} does not seem to be a valid email. Please, try again.') - unset_parameter('qs-notification-email') + unset_parameter('notification-email') else: #pylint: disable=W0120:useless-else-on-loop raise CidCritical('Failed to read email') diff --git a/cid/helpers/quicksight/__init__.py b/cid/helpers/quicksight/__init__.py index 388c1160..0ebf0577 100644 --- a/cid/helpers/quicksight/__init__.py +++ b/cid/helpers/quicksight/__init__.py @@ -115,6 +115,9 @@ def edition(self, fresh: bool=False) -> str: """ if fresh or not hasattr(self, '_subscription_info'): self._subscription_info = self.describe_account_subscription() + status = self._subscription_info.get('AccountSubscriptionStatus') + if status != 'ACCOUNT_CREATED': + return None return self._subscription_info.get('Edition') @property @@ -177,16 +180,16 @@ def describe_account_subscription(self) -> dict: # in case the account doesn't have Enterprise edition logger.info('Insufficient privileges to describe account subscription, working around') try: - self.client.list_dashboards(AwsAccountId=self.account_id).get('AccountInfo') - result = {'Edition': 'ENTERPRISE'} - except self.client.exceptions.UnsupportedUserEditionException as e: - logger.debug(f'UnsupportedUserEditionException means edition is STANDARD: {e}') - result = {'Edition': 'STANDARD'} - except self.client.exceptions.ResourceNotFoundException as e: - logger.debug(e, exc_info=True) - logger.info('QuickSight not activated') - except Exception as e: - logger.debug(e, exc_info=True) + self.client.list_dashboards(AwsAccountId=self.account_id) + result = {'Edition': 'ENTERPRISE', 'AccountSubscriptionStatus': 'ACCOUNT_CREATED'} + except self.client.exceptions.UnsupportedUserEditionException as exc: + logger.debug(f'UnsupportedUserEditionException means edition is STANDARD: {exc}') + result = {'Edition': 'STANDARD', 'AccountSubscriptionStatus': 'ACCOUNT_CREATED'} + except self.client.exceptions.ResourceNotFoundException as exc: + logger.debug(exc, exc_info=True) + logger.info('QuickSight not activated?') + except Exception as exc: + logger.debug(exc, exc_info=True) return result diff --git a/cid/test/bats/20-init-quicksight/create-qs-subscription.bats b/cid/test/bats/20-init-quicksight/create-qs-subscription.bats new file mode 100644 index 00000000..6a52c0bd --- /dev/null +++ b/cid/test/bats/20-init-quicksight/create-qs-subscription.bats @@ -0,0 +1,47 @@ +#!/bin/bats + +account_id=$(aws sts get-caller-identity --query "Account" --output text ) +BATS_TEST_TIMEOUT=300 + +# Helper function for waiting for the requred SubscriptionStatus status. +# TODO: add timeout +function wait_subscription { + status=$1 + until (aws quicksight describe-account-subscription \ + --aws-account-id $account_id \ + --query AccountInfo.AccountSubscriptionStatus | grep -m 1 $status); + do : + sleep 5; + done +} + +@test "Delete Account Subscription" { + aws quicksight update-account-settings \ + --aws-account-id $account_id \ + --default-namespace default \ + --no-termination-protection-enabled + aws quicksight delete-account-subscription --aws-account-id $account_id +} + +@test "Waiting for SubscriptionStatus = UNSUBSCRIBED (can take 2 minutes)" { + wait_subscription "UNSUBSCRIBED" +} + +@test "Run cid-cmd initqs (can take 1 minute)" { + run timeout 300 cid-cmd -vv initqs \ + --enable-quicksight-enterprise yes \ + --account-name $account_id \ + --notification-email 'aaa@bb.com' + + [ "$status" -eq 0 ] +} + +@test "SubscriptionStatus is ACCOUNT_CREATED" { + wait_subscription "ACCOUNT_CREATED" +} + +@test "Edition is ENTERPRISE" { + aws quicksight describe-account-subscription \ + --aws-account-id $account_id \ + --query AccountInfo.Edition | grep "ENTERPRISE" +} From 76fffb4be55340e6ee194379291855c380092c36 Mon Sep 17 00:00:00 2001 From: Iakov GAN <82834333+iakov-aws@users.noreply.github.com> Date: Wed, 23 Aug 2023 16:11:53 +0300 Subject: [PATCH 2/6] fix ThrottlingException (#604) --- cid/helpers/quicksight/__init__.py | 13 +++++++++---- cid/utils.py | 5 +++-- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/cid/helpers/quicksight/__init__.py b/cid/helpers/quicksight/__init__.py index 0ebf0577..7ed5b5f1 100644 --- a/cid/helpers/quicksight/__init__.py +++ b/cid/helpers/quicksight/__init__.py @@ -732,10 +732,10 @@ def select_folder(self): def describe_dashboard(self, poll: bool=False, **kwargs) -> Union[None, Dashboard]: """ Describes an AWS QuickSight dashboard Keyword arguments: - DashboardId - + DashboardId + poll_interval """ - poll_interval = kwargs.get('poll_interval', 1) + poll_interval = kwargs.get('poll_interval', 5) try: dashboard: Dashboard = None current_status = None @@ -747,7 +747,12 @@ def describe_dashboard(self, poll: bool=False, **kwargs) -> Union[None, Dashboar time.sleep(poll_interval) elif poll: logger.info(f'Polling for dashboard {kwargs.get("DashboardId")}') - response = self.client.describe_dashboard(AwsAccountId=self.account_id, **kwargs).get('Dashboard') + try: + response = self.client.describe_dashboard(AwsAccountId=self.account_id, **kwargs).get('Dashboard') + except self.client.exceptions.ThrottlingException: + logger.debug('Got ThrottlingException will sleep for 5 sec') + time.sleep(5) + continue logger.debug(response) dashboard = Dashboard(response) current_status = dashboard.version.get('Status') diff --git a/cid/utils.py b/cid/utils.py index c2063b88..17845a7b 100644 --- a/cid/utils.py +++ b/cid/utils.py @@ -4,12 +4,12 @@ import logging import platform from typing import Any, Dict -import requests from functools import lru_cache as cache from collections.abc import Iterable -from boto3.session import Session +import requests import questionary +from boto3.session import Session from botocore.exceptions import NoCredentialsError, CredentialRetrievalError, NoRegionError, ProfileNotFound from cid.exceptions import CidCritical @@ -257,3 +257,4 @@ def unset_parameter(param_name): value = params[param_name] del params[param_name] logger.info(f'Cleared {param_name}={value}, from parameters') + From fed7f945db971d0719af34705caf5348c47e76f1 Mon Sep 17 00:00:00 2001 From: Iakov GAN <82834333+iakov-aws@users.noreply.github.com> Date: Wed, 23 Aug 2023 16:14:42 +0300 Subject: [PATCH 3/6] add parameters via athena (#605) * add parameters via athena * wip * better error handling --- cid/common.py | 38 ++++++++++++++++++++++++++++---------- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/cid/common.py b/cid/common.py index ea3755cb..d74407a3 100644 --- a/cid/common.py +++ b/cid/common.py @@ -223,8 +223,10 @@ def getPlugin(self, plugin) -> dict: return self.plugins.get(plugin) - def get_definition(self, type: str, name: str=None, id: str=None) -> dict: - """ return resource definition that matches parameters """ + def get_definition(self, type: str, name: str=None, id: str=None, noparams: bool=False) -> dict: + """ return resource definition that matches parameters + :noparams: do not process parameters as they may not exist by this time + """ res = None if type not in ['dashboard', 'dataset', 'view', 'schedule']: raise ValueError(f'{type} is not a valid definition type') @@ -240,7 +242,7 @@ def get_definition(self, type: str, name: str=None, id: str=None) -> dict: break # template - if isinstance(res, dict): + if isinstance(res, dict) and not noparams: name = name or res.get('name') params = self.get_template_parameters(res.get('parameters', {}), param_prefix=f'{type}-{name}-') # FIXME: can be recursive? @@ -300,7 +302,7 @@ def load_resources(self): self.resources = self.resources_with_global_parameters(self.resources) - def get_template_parameters(self, parameters: dict, param_prefix: str='', others: dict={}): + def get_template_parameters(self, parameters: dict, param_prefix: str='', others: dict=None): """ Get template parameters. """ params = get_parameters() for key, value in parameters.items(): @@ -314,10 +316,21 @@ def get_template_parameters(self, parameters: dict, param_prefix: str='', others message=f"Required parameter: {key} ({value.get('description')})", choices=self.cur.tag_and_cost_category_fields + ["'none'"], ) + elif isinstance(value, dict) and value.get('type') == 'athena': + if 'query' not in value: + raise CidCritical(f'Failed fetching parameter {prefix}{key}: paramter with type ahena must have query value.') + query = value['query'] + try: + res = self.athena.query(query)[0] + except (self.athena.client.exceptions.ClientError, CidError, CidCritical) as exc: + raise CidCritical(f'Failed fetching parameter {prefix}{key}: {exc}') from exc + if not res: + raise CidCritical(f'Failed fetching parameter {prefix}{key}, {value}. Athena returns empty result') + params[key] = res[0] elif isinstance(value, dict): params[key] = value.get('value') - while params[key] == None: - if value.get('silentDefault') != None and get_parameters().get(key) == None: + while params[key] is None: + if value.get('silentDefault') is not None and get_parameters().get(key) is None: params[key] = value.get('silentDefault') else: params[key] = get_parameter( @@ -328,7 +341,7 @@ def get_template_parameters(self, parameters: dict, param_prefix: str='', others ) else: raise CidCritical(f'Unknown parameter type for "{key}". Must be a string or a dict with value or with default key') - return always_merger.merge(params, others) + return always_merger.merge(params, others or {}) @command @@ -1083,7 +1096,7 @@ def create_datasets(self, _datasets: list, known_datasets: dict={}, recursive: b print('\nLooking by DataSetId defined in template...', end='') for dataset_name in missing_datasets[:]: try: - dataset_definition = self.get_definition(type='dataset', name=dataset_name) + dataset_definition = self.get_definition(type='dataset', name=dataset_name, noparams=True) raw_template = self.get_data_from_definition('dataset', dataset_definition) if raw_template: ds = self.qs.describe_dataset(raw_template.get('DataSetId')) @@ -1339,8 +1352,13 @@ def create_or_update_dataset(self, dataset_definition: dict, dataset_id: str=Non columns_tpl, ) logger.debug(columns_tpl) - - compiled_dataset = json.loads(template.safe_substitute(columns_tpl)) + compiled_dataset_text = template.safe_substitute(columns_tpl) + try: + compiled_dataset = json.loads(compiled_dataset_text) + except json.JSONDecodeError as exc: + logger.error('The json of dataset is not correct. Please check parameters of the dasbhoard.') + logger.debug(compiled_dataset_text) + raise if dataset_id: compiled_dataset.update({'DataSetId': dataset_id}) From 0d0a0a8ba5a84c32f343a8fee327be4c61f34d9b Mon Sep 17 00:00:00 2001 From: Iakov GAN <82834333+iakov-aws@users.noreply.github.com> Date: Wed, 23 Aug 2023 16:16:06 +0300 Subject: [PATCH 4/6] bump version (#600) --- cfn-templates/cid-cfn.yml | 2 +- cid/_version.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cfn-templates/cid-cfn.yml b/cfn-templates/cid-cfn.yml index 3004ed13..67de84c8 100644 --- a/cfn-templates/cid-cfn.yml +++ b/cfn-templates/cid-cfn.yml @@ -136,7 +136,7 @@ Parameters: CidVersion: Type: String MinLength: 5 - Default: 0.2.24 + Default: 0.2.25 Description: A version of CID package Suffix: Type: String diff --git a/cid/_version.py b/cid/_version.py index cf706eca..fb9bbfc5 100644 --- a/cid/_version.py +++ b/cid/_version.py @@ -1,2 +1,2 @@ -__version__ = '0.2.24' +__version__ = '0.2.25' From 6cac154656bc52de45cc7d3327b0ae3fdffed9cd Mon Sep 17 00:00:00 2001 From: Rudy Krol Date: Thu, 24 Aug 2023 12:14:56 +0200 Subject: [PATCH 5/6] Add us-gov-west-1 to the list of green regions (#608) --- .../sustainability-proxy-metrics.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dashboards/sustainability-proxy-metrics/sustainability-proxy-metrics.yaml b/dashboards/sustainability-proxy-metrics/sustainability-proxy-metrics.yaml index 5216c438..df78783c 100644 --- a/dashboards/sustainability-proxy-metrics/sustainability-proxy-metrics.yaml +++ b/dashboards/sustainability-proxy-metrics/sustainability-proxy-metrics.yaml @@ -744,7 +744,7 @@ views: , ROW ('eu-north-1', 'Sweden', 'Stockholm', '59.33', '18.07', 1) , ROW ('eu-west-1', 'Ireland', 'Dublin', '53.28', '-7.71', 1) , ROW ('us-east-2', 'USA', 'Ohio', '40.36', '-82.91', 1) - , ROW ('us-gov-west-1', 'USA', 'Oregon', '39.53', '-119.88', 0) + , ROW ('us-gov-west-1', 'USA', 'Oregon', '39.53', '-119.88', 1) , ROW ('us-west-1', 'USA', 'N. California', '36.55', '-119.89', 1) , ROW ('us-west-2', 'USA', 'Oregon', '43.82', '-120.33', 1) , ROW ('ap-east-1', 'Hong Kong', 'Hong Kong', '22.28', '114.15', 0) From f083a167783a0e8239ec5b42f947e4ccd54f1bdf Mon Sep 17 00:00:00 2001 From: Rudy Krol Date: Fri, 25 Aug 2023 08:32:08 +0200 Subject: [PATCH 6/6] Fix sus_geo_region_athena_view with all costs on last rolling month (#609) --- .../sustainability-proxy-metrics.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dashboards/sustainability-proxy-metrics/sustainability-proxy-metrics.yaml b/dashboards/sustainability-proxy-metrics/sustainability-proxy-metrics.yaml index df78783c..691ec04b 100644 --- a/dashboards/sustainability-proxy-metrics/sustainability-proxy-metrics.yaml +++ b/dashboards/sustainability-proxy-metrics/sustainability-proxy-metrics.yaml @@ -1021,7 +1021,7 @@ views: FROM (${athena_database_name}.${cur_table_name} cur INNER JOIN sus_aws_regions ON ("product_region" = sus_aws_regions.region_name)) - WHERE ((((product_region <> '') AND (product_region <> 'global')) AND ((product_operation LIKE 'RunInstances%') AND (("bill_billing_period_start_date" >= ("date_trunc"('month', current_timestamp) - INTERVAL '7' MONTH)) AND (CAST("concat"("year", '-', "month", '-01') AS date) >= ("date_trunc"('month', current_date) - INTERVAL '7' MONTH))))) AND (line_item_usage_start_date > (current_date - INTERVAL '1' MONTH))) + WHERE ((product_region <> '') AND (product_region <> 'global')) AND (CAST("concat"("year", '-', "month", '-01') AS date) >= ("date_trunc"('month', current_date) - INTERVAL '1' MONTH)) AND (line_item_usage_start_date > (current_date - INTERVAL '1' MONTH)) GROUP BY bill_payer_account_id, line_item_usage_start_date, product_region, region_city, region_latitude, is95PercentRenewable, region_longitude sus_network: dependsOn: