Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve CDK spoke account handling #9269

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

connelldave
Copy link
Contributor

Fix #9248 - handle more than one "spoke" account in CDK. Added to documentation.

Reduced deployed IAM role permissions for safety: removed iam:* and other write-enabling permissions.

@castrapel
Copy link
Contributor

IAM/S3/SQS/SNS mutation permissions are needed for ConsoleMe self-service and cross-account permission mutation to work, so that change may cause confusion. Perhaps we could add a read-only configuration for folks that aren't ready to deploy fully?

@patricksanders
Copy link
Collaborator

Agreed with @castrapel. I see three options here, in ascending order of work/involvement:

  1. Remove the policy changes from this PR (the quickest way to get this change merged)
  2. A read-only deployment option that uses the version of the policy in this PR
  3. Update ConsoleMe to support distinct roles for logins vs mutation, such as ConsoleMeTrustRole and ConsoleMeManagementRole, with an option to not deploy the latter for read-only deployments

@connelldave connelldave force-pushed the issue_9248_duplicate_spoke_construct_name branch from c879f86 to a6c33ac Compare October 29, 2021 10:33
@connelldave
Copy link
Contributor Author

Hello both - thanks for the feedback. I fixed the CDK deployment while first exploring the project to get going - the second commit is naive on my part.

I've removed the IAM commit so the basic fix can be merged and potentially come back to options 2 and 3 when I understand the codebase better :)

@connelldave connelldave force-pushed the issue_9248_duplicate_spoke_construct_name branch from a6c33ac to 1973bd4 Compare October 29, 2021 10:39
@castrapel
Copy link
Contributor

@avishayil how does this look to you?

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.

There is already a Construct with name 'ConsoleMeSpoke' in App
3 participants