Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Terraform to provision Cognito resources, rather than shell script #157

Merged
merged 13 commits into from
Apr 10, 2018

Conversation

bleggett
Copy link
Contributor

@bleggett bleggett commented Apr 9, 2018

Just putting this here so it can be reviewed, as this contains a Terraform version bump it probably should not be merged until the 1.4.1. release goes out.

This

  • Moves most of the manual shell-script-based Cognito provisioning into Terraform's AWS Cognito module.
  • Updates to the most recent Terraform version (11.6), adds terraform init to automatically fetch and install external provider modules (aws and null)
  • adds the now-required, top-level providers.tf to lock down the above-mentioned external provider versions.
  • Captures the terraform apply exit code and automatically runs terraform destroy for all resources created up to that point in AWS if that command fails, so we don't leave stray AWS resources out there.

Hardcoded the AWS provider version to ">= 1.14", since 1.14 has this required fix.

@suryajak suryajak changed the base branch from v1.4.1 to v1.4.6 April 9, 2018 18:38
default_email_option = "CONFIRM_WITH_LINK"
}
password_policy = {
minimum_length = 6
Copy link
Contributor

Choose a reason for hiding this comment

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

Update this to 8 (Gitlab requires 8 characters and Jazz UI enforces it)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. It might be a good idea to strengthen the requirements for symbols and mixed case as well, but I was just trying to match what was already defined.

@@ -1,15 +0,0 @@
#!/bin/bash
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this not required anymore because terraform destroy will take care of it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct.

if [ $? -gt 0 ]
then
date
print_error "$message....Failed"
Copy link
Contributor

Choose a reason for hiding this comment

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

Finally we can rollback on failures, super cool!

Copy link
Contributor Author

@bleggett bleggett Apr 10, 2018

Choose a reason for hiding this comment

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

It seems to work OK in my testing. It doesn't undo everything, but it will destroy all AWS resources Terraform created, which is most of what people actually would want since those will cost money.

@suryajak suryajak merged commit c9bfab7 into tmobile:v1.4.6 Apr 10, 2018
@bleggett
Copy link
Contributor Author

I think this fixes #154 to some degree. It doesn't fail immediately, but it will note the failure and roll back AWS resource creation.

bleggett pushed a commit to bleggett/jazz-installer that referenced this pull request Nov 29, 2018
Use Terraform to provision Cognito resources, rather than shell script

Former-commit-id: c9bfab7
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.

2 participants