feat: added tf infra for AWS ami account #6517

richardcase · 2024-03-04T16:29:52Z

This adds AWS infra provisioning using terraform for the AWS account to be used to publish CAPA AMIs.

It adds the following:

IAM Group for the CAPA maintainers
- With the group having admin access in the account
IAM user for each maintainer of CAPA
- Users added to the maintainer IAM group
IAM user for image builder automation
- Minimum permissions as set out by packer for publishing AMIs
- Access key encrypted with pgp key

k8s-ci-robot · 2024-03-04T16:29:55Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

nrb · 2024-03-04T17:36:24Z

infra/aws/terraform/cncf-k8s-infra-aws-capa-ami/iam-capa-maintainers-users.tf

+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+


Is there an automated way to keep this file in sync with the team membership?

We could look at doing something for that in the future

Maybe using .tfvars?

salasberryfin

Only nitpicking on the README.

infra/aws/terraform/cncf-k8s-infra-aws-capa-ami/README.md

Danil-Grigorev · 2024-05-03T13:05:53Z

Thanks for addressing it @richardcase
/lgtm

bryantbiggs · 2024-05-22T12:06:28Z

infra/aws/terraform/cncf-k8s-infra-aws-capa-ami/iam-capa-maintainers-users.tf

+*/
+
+
+resource "aws_iam_user" "ankitasw" {


seeing IAM users like this in 2024 is a bit sad - is this because we do not have SSO setup?

bryantbiggs · 2024-05-22T12:07:36Z

infra/aws/terraform/cncf-k8s-infra-aws-capa-ami/providers.tf

+*/
+
+terraform {
+  backend "s3" {}


where is state stored, or how is the bucket identified to store the state?

richardcase · 2024-07-04T11:02:14Z

@bryantbiggs - thats for the review comments. I've been AFK for the last ~5 weeks on a break. I'll pick this up again and will get back to you asap on the comments.

richardcase · 2024-08-06T07:55:49Z

We could potentially use IAM Identity centre for the AMI account(819546954734) instead of using IAM users/groups directly. We have identity centre enabled for the management account (see here.

Off the top of my head this would mean roughly:

Creating a new IAM Identity centre group for CAPA maintainers
Adding IAM identity centre users for each maintainer (with there email address) and assign to the group
- Ideally ensuring that the OTP option is enabled on create user
- Enforce MFA
Assign a "Admin" permission for account 819546954734 to the CAPA maintainer group

seeing IAM users like this in 2024 is a bit sad - is this because we do not have SSO setup?

@bryantbiggs - do you think that using IAM Identity Centre would be better than whats is currently in this PR? It still requires us to create/list the users in TF but at least we are not relying on the password output and a human to send them to the respective maintainer.

bryantbiggs · 2024-08-06T11:55:59Z

yes, SSO would be better. no matter what, we'll have to map users somewhere at the end of the day - but with SSO, we avoid the use of long lived credentials and it will give us the ability to map users to multiple accounts (as needed)

cc @upodroid

richardcase · 2024-08-06T11:58:19Z

Thanks @bryantbiggs 🙇 I will refactor this to use IAM Identity Centre instead

sftim · 2024-08-06T17:02:06Z

infra/aws/terraform/cncf-k8s-infra-aws-capa-ami/iam-capa-maintainers-group.tf

+
+resource "aws_iam_group_policy_attachment" "maintainer-admin" {
+  group      = aws_iam_group.maintainers.name
+  policy_arn = "arn:aws:iam::aws:policy/AdministratorAccess"


Is there a least-privilege alternative to this access that we can grant?

Yes for sure. How about initially we scope the "admin" down to only the services that are used for AMI publishing. The original thinking was that more elevated permissions would be needed initially whilst we work out all the automation / diagnose issues.

Ultimately, we could make it so it's just the build agent (prow or GHA) that has access.

TBH, I don't even agree with giving AdministratorAccess to Prow.

The more i think about it (based on your feedback), the maintainers don't need access to the account. Just whatever CI we use. I'll also re-think this based on that.

sftim · 2024-08-06T17:03:16Z