Skip to content

Conversation

@igordust
Copy link

This feature is the implementation of the feature request #541.

It adds a new parameter workflow_type to the account request module.
The parameter can be valued:

  • apply (default)
  • apply-with-approval

This parameter is saved with all other parameters in the DynamoDB table and is passed to the step function for the account creation and the customizations pipeline.

The apply value maintains unchanged the current behaviour, the customizations pipeline is created with the usual 3 stages.

When you specify apply-with-approval, the resulting customizations pipeline will have the following stages:

  1. Source
  2. Global-Customizations-1 (plan)
  3. Global-Customizations-2 (approval)
  4. Global-Customizations-3 (apply)
  5. Account-Customizations-1 (plan)
  6. Account-Customizations-2 (approval)
  7. Account-Customizations-3 (apply)

@ikerei
Copy link

ikerei commented Mar 31, 2025

+1 This is a valuable enhancement request that would significantly improve AFT's safety and flexibility for core account management. The proposed optional workflow with plan/approve/apply stages aligns with infrastructure-as-code best practices, especially for critical accounts that can impact the entire landing zone.

The rationale provided around protecting core shared services (TGW, IPAM, etc.) is compelling - having the ability to preview and approve changes before they're applied would help prevent unintended consequences across the landing zone. Keeping this as an optional feature maintains AFT's simplicity for standard account vending while adding necessary controls for sensitive environments.

This addresses a clear need expressed by multiple users previously. I support prioritizing this enhancement.

@tyron
Copy link

tyron commented Apr 1, 2025

This is a great initiative, I've been looking for something like that and thinking about developing in house. Thanks!

From a Terraform perspective, when the plan and apply are executed in different stages, your plan should include a -out=FILENAME (reference), file which should be passed to your terraform apply. Otherwise, you are at risk of changes being introduced after your "plan" has finished but still waiting for approval -- when the "apply" step executes, it would just perform all actions, regardless if they have been reviewed.

I don't have much experience with CodePipeline, but I believe you can use output artifacts for that.

@igordust
Copy link
Author

igordust commented Apr 3, 2025

This is a great initiative, I've been looking for something like that and thinking about developing in house. Thanks!

I hope that the feature will be merged, obviously you can develop it by yourself, but I would recommend to stick to the official releases of AFT, that are provided and officially supported by the AWS Control Tower service team. In case of a custom release, probably you would have difficulties in being supported in case of problems...

From a Terraform perspective, when the plan and apply are executed in different stages, your plan should include a -out=FILENAME (reference), file which should be passed to your terraform apply. Otherwise, you are at risk of changes being introduced after your "plan" has finished but still waiting for approval -- when the "apply" step executes, it would just perform all actions, regardless if they have been reviewed.

I don't have much experience with CodePipeline, but I believe you can use output artifacts for that.

You're right, Codepipeline artifacts are the way to save the plan (and also the .terraform folder) and have it ready for the apply phase. I already did it in different terraform pipelines with Codepipeline. I deliberately decided to not introduce it in this PR to reduce the number of changes, but for sure this could be an improvement that can be introduced later on.

@pkleanthous
Copy link

Could someone from the AFT team review this PR?
We’d really like to include this as part of our CI/CD workflows.

Thank you 🙏

@ebachle
Copy link

ebachle commented May 14, 2025

We ended up making quite the contraption in CI, where we'd essentially pull down the state file from HCP terraform and manipulate the backend stanzas so that there would be local runs and we could post the plans in the PR comments. It's helped us somewhat, but this would be a much preferable solution.

/opt/aft/bin/terraform plan -no-color
elif [[ "apply" == ${ACTION} ]] ; then
/opt/aft/bin/terraform apply -no-color --auto-approve
elif [[ -z "${ACTION}"" ]] ; then
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the double quote after ${ACTION} intended?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nope, my bad, fixing it

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this last condition in the case the account doesn't have set the ACTION env variable, in case the user hasn't updated the account request

@meraj-kashi
Copy link

Is there any update on this feature?

@petrosmelachrinos
Copy link

For local testing, someone may find my response in this thread #153 useful.

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.

7 participants