Skip to content
This repository has been archived by the owner on Jul 26, 2022. It is now read-only.

feat(aws): add region support to ssm and sm #475

Merged
merged 1 commit into from
Oct 7, 2020

Conversation

moolen
Copy link
Member

@moolen moolen commented Sep 1, 2020

This PR adds multi-region support for ssm and sm #354

I feel like we should (somehow) cache & reuse the ssm/sm client object so we don't assume a role in each _get call. E.g. by using the hash of the ExternalSecret or ${namespace}/${name}/${metadata.generation} as cache key for the client object.

@moolen
Copy link
Member Author

moolen commented Sep 11, 2020

@silasbw do you have some time to look into this?

@Flydiverny
Copy link
Member

Yo! I merged #454 which caused a bunch of conflicts here.
I think caching client is probably good, we just merged a PR for vault doing something similar.

@moolen moolen force-pushed the feat/aws-region-support branch from e31bc57 to f3ea0ba Compare October 6, 2020 07:24
@moolen
Copy link
Member Author

moolen commented Oct 6, 2020

Thanks for the update @Flydiverny, i rebased my changes.

Copy link
Member

@Flydiverny Flydiverny left a comment

Choose a reason for hiding this comment

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

Changes looks good to me, but one question. If I pass a region, is this for only specifying the region for the secret? Which region is the IAM role in? Currently I think the IAM role would be assumed using the default region. But I could be wrong about that. What would be the expected behavior?

@moolen
Copy link
Member Author

moolen commented Oct 7, 2020

Thanks for the feedback! True, documentation is lacking. The readme is already really long, we should consider using something to generate static pages and serve them using github pages (i'll open a issue for that). Let me try to outline the mechanics:

Take this slightly modified example from the AWS docs:

{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Effect": "Allow",
            "Action": [
                "secretsmanager:GetResourcePolicy",
                "secretsmanager:GetSecretValue",
                "secretsmanager:DescribeSecret",
                "secretsmanager:ListSecretVersionIds"
            ],
            "Resource": [
                "arn:aws:secretsmanager:us-west-2:111122223333:secret:aes128-1a2b3c",
                "arn:aws:secretsmanager:us-central-1:111122223333:secret:aes192-4D5e6F",
                "arn:aws:secretsmanager:us-east-1:111122223333:secret:aes256-7g8H9i"
            ]
        }
    ]
}

IAM roles are global, the attached policies grant access to certain resources in certain regions (or *).
When we construct the secretsManager/SSM client through _clientFactory it needs to know from what region to fetch the secret or parameter from. This is what the region property tries to solve.

If it is not defined explicitly it falls back to use (see docs):

  • environment variables AWS_REGION or AWS_DEFAULT_REGION (that's what's most likely in use - see helm chart )
  • shared credentials file ~/.aws/config
  • instance metadata service API (docs) if it runs on EC2

So, essentially, if the region property is set it will take precedence over the above chain.

@Flydiverny
Copy link
Member

Yes, we should definitely set up some docs. Was planning to try setting something up with docusaurus but didn't get around to it.

Think we can merge this as is then.

@Flydiverny Flydiverny merged commit 0b35441 into external-secrets:master Oct 7, 2020
@Niksko Niksko mentioned this pull request Oct 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants