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

Refactoring Terraform For Scratch Deployment #721

Closed
wants to merge 8 commits into from
Closed

Conversation

ben851
Copy link
Contributor

@ben851 ben851 commented Apr 20, 2023

Summary | Résumé

Lots of changes here.

  • Had to update AWS Provider and modify s3 bucket creation due to this breaking change by AWS. This needs to be verified in staging.
  • Removed reference to static cloud based sensor satellite bucket, and instead created this bucket via TF. We will need to import this in staging and production, and further changes may be necessary to configure the bucket to forward to the CBS.
  • Made the athena workgroup name a variable - it looks like the name "primary" is created by default in the AWS account. The default name is "primary" but for scratch environment, this can be overridden (and has to be since the primary already exists).
  • Set the monthly spending limit for scratch to $1

Test instructions | Instructions pour tester la modification

  • Against scratch env, terragrunt apply -var-file ../scratch.tfvars
  • Terragrunt plan against staging/prod and import as necessary.

@ben851 ben851 requested a review from sastels April 20, 2023 19:50
@ben851 ben851 requested a review from jimleroyer as a code owner April 20, 2023 19:50
@ben851
Copy link
Contributor Author

ben851 commented Apr 20, 2023

The failing CI tests are because the aws provider in staging has to be upgraded to 4.0, which requires a terragrunt init --upgrade command. Don't want to do this yet.

Copy link
Member

@CalvinRodo CalvinRodo left a comment

Choose a reason for hiding this comment

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

Just some non-blocking comments pointing you to our Terraform Modules.

acl = "log-delivery-write"
}

resource "aws_s3_bucket" "cbs_sensor_bucket" {
Copy link
Member

Choose a reason for hiding this comment

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

Have you seen our Terraform modules?

https://github.com/cds-snc/terraform-modules

We have modules for S3 Buckets that might be useful.

When v5 of the AWS Provider comes out and this type of bucket is finally deprecated I plan on fixing the s3 bucket module so it automatically refactors to the new way of building them.

It also configures them in a consistent that meets some of our organizational defined best practices.

It's not 100% perfect so open to any issues that can help us secure and make our orgs infra better.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks good Calvin. I'd probably still proceed with this PR for now, but once we're back into a state where things actually build I'll do some investigation in to what modules we can leverage. Happy to be a contributor to make them as useful and secure as possible 👍

@github-actions
Copy link

Staging: common

❌   Terraform Init: failed
❌   Terraform Validate: failed
✅   Terraform Format: success
❌   Terraform Plan: failed
❌   Conftest: failed

Show Init results
Initializing modules...
Downloading registry.terraform.io/terraform-aws-modules/notify-slack/aws 4.24.0 for notify_slack_critical...
- notify_slack_critical in .terraform/modules/notify_slack_critical
Downloading registry.terraform.io/terraform-aws-modules/lambda/aws 2.27.1 for notify_slack_critical.lambda...
- notify_slack_critical.lambda in .terraform/modules/notify_slack_critical.lambda
Downloading registry.terraform.io/terraform-aws-modules/notify-slack/aws 4.24.0 for notify_slack_general...
- notify_slack_general in .terraform/modules/notify_slack_general
Downloading registry.terraform.io/terraform-aws-modules/lambda/aws 2.27.1 for notify_slack_general.lambda...
- notify_slack_general.lambda in .terraform/modules/notify_slack_general.lambda
Downloading registry.terraform.io/terraform-aws-modules/notify-slack/aws 4.24.0 for notify_slack_ok...
- notify_slack_ok in .terraform/modules/notify_slack_ok
Downloading registry.terraform.io/terraform-aws-modules/lambda/aws 2.27.1 for notify_slack_ok.lambda...
- notify_slack_ok.lambda in .terraform/modules/notify_slack_ok.lambda
Downloading registry.terraform.io/terraform-aws-modules/notify-slack/aws 4.24.0 for notify_slack_warning...
- notify_slack_warning in .terraform/modules/notify_slack_warning
Downloading registry.terraform.io/terraform-aws-modules/lambda/aws 2.27.1 for notify_slack_warning.lambda...
- notify_slack_warning.lambda in .terraform/modules/notify_slack_warning.lambda
Downloading git::https://github.com/cds-snc/terraform-modules.git?ref=v5.0.0 for s3_scan_objects...
- s3_scan_objects in .terraform/modules/s3_scan_objects/S3_scan_object

Initializing the backend...

Successfully configured the backend "s3"! Terraform will automatically
use this backend unless the backend configuration changes.

Initializing provider plugins...
- Reusing previous version of hashicorp/aws from the dependency lock file
- Reusing previous version of hashicorp/local from the dependency lock file
- Reusing previous version of hashicorp/null from the dependency lock file
- Reusing previous version of hashicorp/external from the dependency lock file
- Reusing previous version of hashicorp/archive from the dependency lock file
- Installing hashicorp/local v2.1.0...
- Installed hashicorp/local v2.1.0 (signed by HashiCorp)
- Installing hashicorp/null v3.1.0...
- Installed hashicorp/null v3.1.0 (signed by HashiCorp)
- Installing hashicorp/external v2.2.0...
- Installed hashicorp/external v2.2.0 (signed by HashiCorp)
- Installing hashicorp/archive v2.2.0...
- Installed hashicorp/archive v2.2.0 (signed by HashiCorp)

Error: Failed to query available provider packages

Could not retrieve the list of available versions for provider hashicorp/aws:
locked provider registry.terraform.io/hashicorp/aws 3.74.2 does not match
configured version constraint >= 3.61.0, ~> 4.0; must use terraform init
-upgrade to allow selection of new versions

time=2023-04-24T17:32:06Z level=error msg=1 error occurred:
	* exit status 1


Show Validate results
Error: registry.terraform.io/hashicorp/aws: there is no package for registry.terraform.io/hashicorp/aws 3.74.2 cached in .terraform/providers

time=2023-04-24T17:32:07Z level=error msg=1 error occurred:
	* exit status 1


Show plan
Error: Required plugins are not installed

The installed provider plugins are not consistent with the packages selected
in the dependency lock file:
  - registry.terraform.io/hashicorp/aws: there is no package for registry.terraform.io/hashicorp/aws 3.74.2 cached in .terraform/providers

Terraform uses external plugins to integrate with a variety of different
infrastructure services. To download the plugins required for this
configuration, run:
  terraform init
time=2023-04-24T17:32:08Z level=error msg=1 error occurred:
	* exit status 1


Copy link
Member

@jimleroyer jimleroyer left a comment

Choose a reason for hiding this comment

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

LGTM

acl = "log-delivery-write"
}

resource "aws_s3_bucket" "cbs_sensor_bucket" {
Copy link
Member

Choose a reason for hiding this comment

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

Right now the CBS bucket is being managed here for all accounts:
https://github.com/cds-snc/cloud-based-sensor

If you want to bring it into this repo you'll need to do a bit of IAM setup and add replication rules so it can send objects to the log archive account's bucket:
https://github.com/cds-snc/cloud-based-sensor/tree/main/terragrunt/aws/satellite_bucket

Happy to chat about this if you want more details.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks Pat - based on this info I think I will refactor this.

@ben851
Copy link
Contributor Author

ben851 commented Apr 25, 2023

I'm going to close this PR for now - I'm going to look at refactoring the code for the CBS Satellite buckets.

@ben851 ben851 closed this Apr 25, 2023
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.

4 participants