Multiaccount action capability

[stefangusa]

Fixes #70

It offers support not only for multi-region but also for multi-account actions.

[m4dcoder]

I am not familiar with multi-account access in AWS and so need some more explanation. The change here allows user to pass in an account_id and region when executing an AWS action. User will only need a single pair of access key and secret to be able to assume different role?

[blag]

@m4dcoder brings up an excellent point and it really got me thinking. You generally want to do the minimum amount of work in a try block, and I think this mixes two separate operations that could both throw. If possible, I would break these up into two try/except blocks:

        try:
            assumed_role = self.session.client('sts').assume_role(
                RoleArn=self.cross_roles_arns[account_id],
                RoleSessionName='StackStormEvents'
            )
        except ClientError:
            self.logger.error('Could not assume role on account with id: %s', account_id)
            raise

        try:
            self.credentials.update({
                'aws_access_key_id': assumed_role["Credentials"]["AccessKeyId"],
                'aws_secret_access_key': assumed_role["Credentials"]["SecretAccessKey"],
                'security_token': assumed_role["Credentials"]["SessionToken"]
            })
        except KeyError:
            self.logger.error('Could not find cross region role ARN in the config file.')
            raise

But that also brings up the except blocks themselves. Is it really useful to add those small tidbits to the logs? Generally logging should add more context to the logs. Exactly what went wrong is already captured by exception tracebacks, which are logged anyway, so these log statements really don't contribute anything more that is actionable to a StackStorm administrator.

I would either remove the except blocks entirely, let the exceptions with their tracebacks bubble up (where they will be logged anyway), and let ST2 administrators infer the cause from the logs, or I would go the opposite direction and include as much information as I could in the logs before reraising the exception.

Useful log messages should:

1. Clearly explain what was attempted and didn't work.
   Example: "Failed to assume role as account %d." (it's going to be obvious from the format of the account_id that it's an account ID, so you don't need to explicitly state that)
2. Elucidate a high-level reason of what it was doing when it failed.
   Example: "when using the AWS session client"
3. And give one or more possible breadcrumbs to the solution.
   Example: "You may want to check the roles configured for the AWS pack and ensure that the %d is still valid."

If you can save a StackStorm administrator from having to Google for a solution, or digging through Stack Overflow posts, or querying our Slack community, they will love you (and StackStorm).

[stefangusa]

@blag I understood the issue regarding the format of the try/catch block but I have to highlight that its new structure could only be:

       try:
            assumed_role = self.session.client('sts').assume_role(
                RoleArn=self.cross_roles_arns[account_id],
                RoleSessionName='StackStormEvents'
            )
        except ClientError:
            self.logger.error('Could not assume role on account with id: %s', account_id)
            raise
        except KeyError:
            self.logger.error('Could not find cross region role ARN in the config file.')
            raise

        self.credentials.update({
                'aws_access_key_id': assumed_role["Credentials"]["AccessKeyId"],
                'aws_secret_access_key': assumed_role["Credentials"]["SecretAccessKey"],
                'security_token': assumed_role["Credentials"]["SessionToken"]
        })

This happens because the KeyError could only be encountered if the cross_role_arn is missing and not in the other case as the boto3 documentation [1] says that if the assume role operation succeeds (so does not return a ClientError) it must contain a dictionary containing all those keys.

Regarding the logging errors, I agree they could be more accurate.

I will make the changes these days and hope they will meet the expectations.

Until then, could you also take a look on the #87 PR which implements a similar capability for the sensor part?

Thank you!

[blag]

Looking good, just a few small tweaks.

[stefangusa]

@blag I have squashed the new commit in the older one but I have changed the quoting style.

[blag]

LGTM, thanks!

[blag]

Closes #101.