STS cross-account access

[philchristensen]

At Logicworks we had a crucial need to look up service limits across many different client accounts, and this tool was the perfect fit. The only thing missing was a more practical way to query multiple accounts than specifying access keys via environment variables.

I was able to fairly trivially implement STS AssumeRole support, allowing you to setup your environment variables with a IAM user that has sts:AssumeRole privileges. The primary change was a switch to using the connect_to_region() function to connect to the API, which accepts the access keys and security token you get from STS.

To use this in code, you should already have created an IAM account with sts:AssumeRole, and set your environment up up using the normal methods (env variables, config file, what have you). Then you simply pass the region, an account_id (a 12 digit string), and account_role, the name of an IAM Role defined in the destination account. Alternatively, you can use the new -r, -A and -R flags to pass those things in at the CLI.

I fixed the tests it broke, but I haven't yet been able to write any tests. I'm not entirely sure how. All that has changed is a line or two in the connect() methods, and there has been the addition of two methods to the _AwsService base class, connect_via() and _get_sts_token(), but I'm not clear how to add tests for them without requiring use of an actual AWS account.

This is a pretty useful patch, though, so I wanted to at least make it available.

[philchristensen]

Not sure the best way to fix the error. It's a single issue with a docstring. It reads:

    def connect_via(self, driver):
        """
        Connect to API if not already connected; set self.conn
        Use STS to assume a role as another user if self.account_id has been set

        :param driver: the Boto sub-module to use to call connect_to_region()
        :type driver: module
        """

It's not recognizing 'module' as a valid type. But that's the result of calling type() on a module object.

[landscape-bot]

Repository health decreased by 0.22% when pulling 9111da6 on Logicworks:sts-cross-account-access into ae37971 on jantman:develop.

• 12 new problems were found (including 0 errors and 0 code smells).
• 1 problem was fixed (including 0 errors and 0 code smells).

[landscape-bot]

Repository health decreased by 0.06% when pulling 9f3c798 on Logicworks:sts-cross-account-access into ae37971 on jantman:develop.

• 4 new problems were found (including 0 errors and 0 code smells).
• 1 problem was fixed (including 0 errors and 0 code smells).

[jantman]

@philchristensen I definitely want to get this tested and merged, but I'm trying to give some attention to another project of mine, so it might be a little while before I can.

The gist of what still needs to be done (by either you or I) for this to be merge-able is:

1. Unit tests for this. In general, all of the unit tests use mock and patch to test for calls to boto and simulate the return values. I know that testing - especially using a particular style - can be be a real pain to get the hang of, so I can probably add the tests.
2. The generated sphinx documentation (the .rst files under docs/source that end up on readthedocs) needs to be updated for this, both the Development documentation (how to add new services) and the end-user docs about regions and accounts.
3. I'd really like confirmation from someone else that this works for them, so I'll either need to setup STS somehow and test it myself, or find someone else who can verify it.

Thanks!!!!!!!!!!!!!

[jantman]

As to the docstring error with connect_via, passing modules around is a bit taboo, especially since Boto occasionally reorganizes their code and module hierarchy. What I'd recommend doing instead is passing the reference to the actual connect_to_region() method, i.e.:

in autoscaling.py on line 60, change self.conn = self.connect_via(boto.ec2.autoscale) to self.conn = self.connect_via(boto.ec2.autoscale.connect_to_region). Then in _AwsService instead of doing driver.connect_to_region(args) we just do driver(args).

In this case, our doc string will change to :type driver: method. I'm not sure if that will validate or not. If it doesn't, don't worry about it, I'll figure it out. I know I've had this problem before with sphinx type validation, but I don't remember how to fix it...

[philchristensen]

That all sounds good. I'm about to leave for vacation for a week, but I should be able to get to it before Thursday, when I leave

[landscape-bot]

Repository health decreased by 0.06% when pulling 5981ea0 on Logicworks:sts-cross-account-access into ae37971 on jantman:develop.

• 4 new problems were found (including 0 errors and 0 code smells).
• 1 problem was fixed (including 0 errors and 0 code smells).

[jantman]

@philchristensen ok, thanks so much. I'll try to look at this over the weekend and see if I can get Sphinx and the tests happy. Interestingly, codecov.io doesn't seem to be commenting on PRs properly, but coverage did decrease a bit. I'll look into that too.

[philchristensen]

Sorry I haven't made any headway in the unit tests, but I realized I needed to make the TrustedAdvisor stuff use regions/accounts also. It's working properly with the latest commit.

[jantman]

Sorry I've let this sit for so long, it's really inexcusable. I'm going to do some work tonight on trying to get it merged.

[philchristensen]

It's totally excusable! But thanks for the update...

[jantman]

Just for my own reference, and in case I need to call it quits for the night, the collected comments that I could find from above:

• unit tests for all changes
• perform (unfortunately manual) sanity checks for correct operation; have someone test operation with STS
• full code coverage
• ensure that we're passing connect methods, not driver classes
• ensure Trusted Advisor is properly updated
• docs:
  • validate generated documentation
  • end-user docs for how to use STS and how to use a region (without STS)
  • developer documentation for how all this works
  • developer doc updates for adding services

Assuming I can get all this done and run through the release checklist, I'll probably cut 0.1.3 with these fixes and push the rest of the stuff in that milestone to 0.1.4.

[jantman]

I believe I've finished up the code fix-ups and unit tests; as far as I can tell, the only things left from the above list are documentation updates, and testing operation with STS, which I can't do.

@philchristensen is there any chance you could pull down/install the "pr64" branch and test that it still works right with STS?

[jantman]

I've confirmed that this works with STS.

To do:

• documentation
• add support for external_id parameter for the assume_role() call
• test support with external_id

[jantman]

I went a bit too far down a rabbit hole, this is taking longer than I'd expected. As of be65975, remaining to do:

• update development docs for adding services for these changes.
• re-test with plain STS and external_id
• consider refactoring the connection stuff, since TrustedAdvisor is now a subclass of _AwsService which seems wrong, and might mess up class loading. Maybe we should just have a Connectable mixin for this.

[codecov-io]

Current coverage is 100.00%

Merging #64 into master will not affect coverage as of be65975

@@            master     #64   diff @@
======================================
  Files           17      16     -1
  Stmts         1170    1227    +57
  Branches       175     184     +9
  Methods          0       0       
======================================
+ Hit           1170    1227    +57
  Partial          0       0       
  Missed           0       0       

Review entire Coverage Diff as of be65975

Powered by Codecov. Updated on successful CI builds.

[jantman]

I'm closing this in favor of #77 that pulls in my work from last night.