aws-ia / terraform-aws-control_tower_account_factory

AWS Control Tower Account Factory
Apache License 2.0
630 stars 418 forks source link

Confusing variable names for AWSAFTExection #459

Open hacker65536 opened 4 months ago

hacker65536 commented 4 months ago

Terraform Version & Prov:

AFT Version: 1.12.2

Terraform Version & Provider Versions Please provide the outputs of terraform version and terraform providers from within your AFT environment

terraform version

{Replace me}

terraform providers

{Replace me}

Bug Description

I think aft_admin_role_arn should be aft_exec_role_arn.

$ aws ssm get-parameters-by-path --path "/aft/resources/iam/" --query 'Parameters[*].Name.[@]' --output text
/aft/resources/iam/aft-administrator-role-name
/aft/resources/iam/aft-execution-role-name
/aft/resources/iam/aft-session-name
$ git grep --name-only "aft_admin_role_arn"
examples/multiple-account-customizations/account-customization-dev/terraform/backend.jinja
examples/multiple-account-customizations/account-customization-prod/terraform/backend.jinja
examples/multiple-regions-customization/multiple-regions/terraform/backend.jinja
modules/aft-code-repositories/buildspecs/ct-aft-account-provisioning-customizations.yml
modules/aft-code-repositories/buildspecs/ct-aft-account-request.yml
modules/aft-customizations/buildspecs/aft-account-customizations-terraform.yml
modules/aft-customizations/buildspecs/aft-global-customizations-terraform.yml
modules/aft-iam-roles/outputs.tf
sources/aft-customizations-repos/aft-account-customizations/ACCOUNT_TEMPLATE/terraform/backend.jinja
sources/aft-customizations-repos/aft-account-provisioning-customizations/terraform/aft-providers.jinja
sources/aft-customizations-repos/aft-account-provisioning-customizations/terraform/backend.jinja
sources/aft-customizations-repos/aft-account-request/terraform/aft-providers.jinja
sources/aft-customizations-repos/aft-account-request/terraform/backend.jinja
sources/aft-customizations-repos/aft-global-customizations/terraform/backend.jinja

To Reproduce Steps to reproduce the behavior:

  1. Go to '...'
  2. Click on '....'
  3. Scroll down to '....'
  4. See error

Expected behavior A clear and concise description of what you expected to happen.

Related Logs Provide any related logs or error messages to help explain your problem.

Additional context Add any other context about the problem here.

snebhu3 commented 3 months ago

@hacker65536 thank you for reaching out. Please may you elaborate on why you would require this change of variable names? You could read more on AFT required roles here.

hacker65536 commented 3 months ago

@snebhu3

Sorry I am not explaining it well. This is about buildspec.yaml and jinja template.

I think the variable name aft_admin_role_arn should be aft_exec_role_arn because the actual content of the variable name aft_admin_role_arn is arn:aws:iam::*:role/AWSAFTExecution.

hacker65536 commented 3 months ago

e.g.

https://github.com/aws-ia/terraform-aws-control_tower_account_factory/blob/aee036db9d4926d60fd48a04fbc1d70917749456/modules/aft-code-repositories/buildspecs/ct-aft-account-provisioning-customizations.yml#L70