finos / terraform-aws-cfi-eks

Apache License 2.0
0 stars 1 forks source link

Move iam-required-permissions.md into a terraform file. #15

Open thinkl33t opened 1 year ago

thinkl33t commented 1 year ago

Feature Request

Description of Problem:

The iam permissions file we have is a markdown file, the contents of which needs to be copy-pasted by an admin into the webui, onto the permissions of our deployment account. This has a couple of issues:

1) copy-pasting things is error prone 2) we don't have an identified upstream for this information, so when updating upstream terraform modules there is a risk that our permissions are not synched

Potential Solutions:

1) We should provide this file in a format that can be directly run in terraform, by a user with administrative permissions. 2) We should identify the upstream source of this information so it can be updated when our aws upstream is.

thinkl33t commented 1 year ago

https://github.com/terraform-aws-modules/terraform-aws-eks/blob/master/docs/irsa_integration.md is the new version of this, and describes exactly what i suggested :grin:

eddie-knight commented 1 year ago

I may be jumping into the comments too quickly here, but my concern is regarding organization of the IaC for this proposed solution. This approach will presumably result in the creation of a config that is used independently of the child module featured in this repo... so where should it live? Do we keep it as a sub-repo here? Or do we create a separate repo just for it? Or do we create a monorepo of similar configs for all of the IaC related to service accounts?

I agree with the need for automating the service account creation 100%, but I want to make sure we have a think around how best to organize it. @ml4 @thinkl33t @AdrianHammond @abdullahgarcia -- can any of y'all speak on this topic?

ml4 commented 1 year ago

IAM policy can be injected into resource definitions using:

all of which obviate the need to have extraneous modules for the storage of such. As it happens, all of these mean that any interpolation of such policy at plan time with references to as-yet non-existing resources mean that FINOS members may be troubled by the fact that Sentinel will thus not be able to parse such resource definitions. I'm actually handling this problem this sprint. The definition of policy as a separate module would not solve this problem because the modules are downloaded and then the graph computed on that basis so any interpolation members make would cause the same problem.

The fact that there is no current interpolation is currently immaterial because the approach we make here sets a precedent for future similar approaches.

ml4 commented 1 year ago

In the short term at least, I would propose we use a data source to pass the policy in, and remediate the notion of handling future interpolation issues then.

thinkl33t commented 1 year ago

Given the IAM policy being created, and the permission set required isn't ours, but is a requirement from the hashicorp aws module, i think it would be better to link to their recommended way of doing it as part of the getting started / initial setup, and not duplicate it here at all?