cds-snc / notification-planning-core

Project planning for GC Notify Core Team
0 stars 0 forks source link

AWS Github Actions Lambda Role is Administrator #270

Open ben851 opened 6 months ago

ben851 commented 6 months ago

Describe the bug

SEV-2

The aws role being assumed by github actions, notification-lambdas-apply is a full administrator role across the entire account. We should really lock this down to the permissions and resources needed by github actions.

Additionally, the IAM for this is not in terraform, so we should add it there as while doing this

CalvinRodo commented 6 months ago

Just for context the IAM for this is in Terraform, it's managed globally by the Internal SRE team as part of the account vending process.

These roles were put in place to make it easier for teams to implement OIDC and prevent teams from having to do the initial creation of OIDC rules using their own credentials or worse yet skip using OIDC and use AWS Access Keys instead for their Terraform work.

You can see the terraform that creates the plan and apply roles here: https://github.com/cds-snc/aft-global-customizations/blob/main/terraform/oidc_terraform_roles.tf

Also the OIDC Identity Provider for Github is also centrally deployed to all accounts on account vend here: https://github.com/cds-snc/aft-global-customizations/blob/main/terraform/oicd_roles.tf

The Notification-Production repos that have these roles can be seen here: https://github.com/cds-snc/aft-account-request/blob/main/terraform/platform.tf#L115-L117

The Notification-Staging repos that have these roles can be seen here: https://github.com/cds-snc/aft-account-request/blob/main/terraform/staging.tf#L84-L86

Not saying that you shouldn't make the switch to your own managed OIDC role and tighten up Role to just what is need but just that this is not necessarily a bug but a feature put in place to make life a bit easier when starting up new projects.


PLEASE NOTE: You will need to remove the repos you don't want these roles vended to from the aft-account-request repo, or you will run into an issue where a resource is managed in two Terraform Instances, if you don't do this work you will run into issues where your Terraform is overridden Internal SRE re-applies all global configurations.

Alternatively since the Readonly is pretty safe (though it does allow for reading of AWS Secrets in your PRs), you could ask Internal SRE if they can change the functionality and split up the apply and read-only role creation so you can have the reaodnly without the apply and thus saving you from having to manage the read-only role yourself, (which is here: https://github.com/cds-snc/aft-global-customizations/blob/main/terraform/oidc_terraform_roles.tf#L66-L151)

Another possibility is to pull out the plan role and add it to the Terraform modules repo so that both the global customization and notify can take advantage of the work we already did on that role.