Skip to content

Add utility for creating a client assuming a role, migrate some inter-service communication to new client#7990

Merged
dfangl merged 9 commits intomasterfrom
iam/inter-service-enforcement
Mar 29, 2023
Merged

Add utility for creating a client assuming a role, migrate some inter-service communication to new client#7990
dfangl merged 9 commits intomasterfrom
iam/inter-service-enforcement

Conversation

@dfangl
Copy link
Member

@dfangl dfangl commented Mar 28, 2023

Motivation

  • Assuming a role and using a client with the returned credentials is a pattern we often need across the code base, especially for the use of service roles or service-linked roles. The new method provides a simple interface to do this in one call.
  • To enforce IAM between some services, we need to use the new aws client, and also handle the exceptions (it should fail with a warning, but should not crash anything important if a request fails).

Changes

connect_to.with_assumed_role

A new method for the aws client factories.

Usage:

logs_client = connect_to
    .with_assumed_role(role_arn="arn:aws:iam::000000000000:role/test-role", service_principal="lambda")
    .logs

This call will assume the given role as the given service, and return a service level client factory, where various clients can be created using the returned credentials.

The assume role call will be done immediately, the other clients instantiated lazily as they are requested.

Client migrations

Several important inter-service calls between lambda, sqs and sns are migrated to the new clients.
This migration is ongoing, and not complete yet.

During this, I silently killed SYNCHRONOUS_SNS_EVENTS, as it is scheduled to remove with 2.0 anyway.

Fix check if installed check outside of installer lock

This one is a tricky one.
Basically, we are circumventing the lock if the is_installed() method already returns true. But, the default is just checking if a directory is created, which can be true during the installation as well. Moving this entirely behind the lock avoids one thread installing while the other one is checking, and therefore avoids starting incompletely installed dependencies.

This seems to be the root cause for the failing test_kinesis_firehose_elasticsearch_s3_backup test, which is so annoying. It is basically trying to start elasticsearch while we are still installing plugins -> class loading failure.

@github-actions
Copy link

github-actions bot commented Mar 28, 2023

LocalStack integration with Pro

1 866 tests  ±0   1 675 ✔️ ±0   1h 21m 52s ⏱️ - 3m 11s
       1 suites ±0      191 💤 ±0 
       1 files   ±0          0 ±0 

Results for commit 0743e29. ± Comparison against base commit 62254ed.

♻️ This comment has been updated with latest results.

@dfangl dfangl removed request for bentsku and thrau March 28, 2023 17:17
Copy link
Member

@dominikschubert dominikschubert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 🚀

I like the new pattern. One issue I'm starting to see a bit is the manual specification of the service principal as a string literal. Maybe it would make sense to keep a list of all current AWS service principals and use an enum for these. Sometimes they really aren't as intuitive on AWS as they should be 😬 (e.g. states for stepfunctions)

@dfangl dfangl force-pushed the iam/inter-service-enforcement branch from f9a80f1 to 0c15734 Compare March 28, 2023 20:40
@dfangl dfangl force-pushed the iam/inter-service-enforcement branch from 0c15734 to d936065 Compare March 29, 2023 09:31
@dfangl dfangl requested review from alexrashed and removed request for viren-nadkarni March 29, 2023 11:52
Copy link
Member

@alexrashed alexrashed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The package installer API change looks good to me! Thanks for tackling this! 🚀

@dfangl dfangl merged commit 6dd1c41 into master Mar 29, 2023
@dfangl dfangl deleted the iam/inter-service-enforcement branch March 29, 2023 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants