Extracted SNS topic definition to its own module because it will be r… #156
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
patheard
reviewed
Dec 16, 2021
patheard
reviewed
Dec 16, 2021
patheard
reviewed
Dec 16, 2021
patheard
reviewed
Dec 16, 2021
patheard
reviewed
Dec 16, 2021
patheard
reviewed
Dec 16, 2021
patheard
reviewed
Dec 16, 2021
patheard
reviewed
Dec 16, 2021
patheard
reviewed
Dec 16, 2021
patheard
reviewed
Dec 16, 2021
patheard
requested changes
Dec 16, 2021
There was a problem hiding this comment.
Looking good @craigzour! I had a few fixes for you but then I think you'll be good to merge.
One other thing is now that you've created the sns module, you can get rid of the two KMS key variables in the alarms module since they're not needed there anymore:
https://github.com/cds-snc/forms-terraform/blob/main/aws/alarms/inputs.tf#L21-L29
…equired by the app module for an upcoming feature
craigzour
force-pushed
the
feature/extract-sns-definition-into-separated-module
branch
from
372415a to
5a01721
Compare
4 tasks
⚠ Terrform update availableTerraform: 1.1.1 (using 1.0.10)
Terragrunt: 0.35.14 (using 0.35.6) |
|
Note that the SNS topics were migrated manually to avoid errors around trying to create topics that already existed. The reason this was required is because of order of operations:
The state migration commands used were: cd env/staging/sns
terragrunt import aws_sns_topic.alert_critical arn:aws:sns:ca-central-1:687401027353:alert-critical
terragrunt import aws_sns_topic.alert_warning arn:aws:sns:ca-central-1:687401027353:alert-warning
terragrunt import aws_sns_topic.alert_warning_us_east arn:aws:sns:us-east-1:687401027353:alert-warning
terragrunt import aws_sns_topic.alert_ok arn:aws:sns:ca-central-1:687401027353:alert-ok
terragrunt import aws_sns_topic.alert_ok_us_east arn:aws:sns:us-east-1:687401027353:alert-ok
cd env/staging/alarms
terragrunt state rm aws_sns_topic.alert_critical
terragrunt state rm aws_sns_topic.alert_warning
terragrunt state rm aws_sns_topic.alert_warning_us_east
terragrunt state rm aws_sns_topic.alert_ok
terragrunt state rm aws_sns_topic.alert_ok_us_east
terragrunt import aws_sns_topic_subscription.topic_critical arn:aws:sns:ca-central-1:687401027353:alert-critical:9eb13517-91ba-4866-9e39-6f9e190deb96
terragrunt import aws_sns_topic_policy.cloudwatch_events_critical_sns arn:aws:sns:ca-central-1:687401027353:alert-critical
terragrunt import aws_lambda_permission.notify_slack_critical NotifySlackSNS/AllowExecutionFromSNSCriticalAlert |
Staging: app✅ Terraform Format: Plan: 0 to add, 1 to change, 0 to destroyShow planResource actions are indicated with the following symbols:
~ update in-place
Terraform will perform the following actions:
# aws_iam_policy.lambda_kms will be updated in-place
~ resource "aws_iam_policy" "lambda_kms" {
id = "arn:aws:iam::687401027353:policy/lambda_kms"
name = "lambda_kms"
~ policy = jsonencode(
~ {
~ Statement = [
~ {
~ Action = [
- "kms:Decrypt",
- "kms:Encrypt",
"kms:GenerateDataKey",
+ "kms:Encrypt",
+ "kms:Decrypt",
]
~ Resource = [
- "arn:aws:kms:ca-central-1:687401027353:key/1f3edb85-9eac-4da9-8c7c-43a68e339ede",
- "arn:aws:kms:ca-central-1:687401027353:key/c5c2a1c2-c092-4fa1-8daf-3414f3511b1d",
] -> "arn:aws:kms:ca-central-1:687401027353:key/1f3edb85-9eac-4da9-8c7c-43a68e339ede"
~ Sid = "VisualEditor0" -> ""
# (1 unchanged element hidden)
},
]
# (1 unchanged element hidden)
}
)
tags = {
"CostCentre" = "forms-platform-staging"
"Terraform" = "true"
}
# (5 unchanged attributes hidden)
}
Plan: 0 to add, 1 to change, 0 to destroy.
─────────────────────────────────────────────────────────────────────────────
Saved the plan to: plan.tfplan
To perform exactly these actions, run the following command to apply:
terraform apply "plan.tfplan"
Show Conftest results9 tests, 9 passed, 0 warnings, 0 failures, 0 exceptions
|
Staging: alarms✅ Terraform Format: Plan: 1 to add, 2 to change, 1 to destroyShow planResource actions are indicated with the following symbols:
~ update in-place
-/+ destroy and then create replacement
Terraform will perform the following actions:
# aws_lambda_permission.notify_slack_critical must be replaced
-/+ resource "aws_lambda_permission" "notify_slack_critical" {
~ function_name = "arn:aws:lambda:ca-central-1:687401027353:function:NotifySlackSNS" -> "NotifySlackSNS" # forces replacement
~ id = "AllowExecutionFromSNSCriticalAlert" -> (known after apply)
# (4 unchanged attributes hidden)
}
# aws_sns_topic_policy.cloudwatch_events_critical_sns will be updated in-place
~ resource "aws_sns_topic_policy" "cloudwatch_events_critical_sns" {
id = "arn:aws:sns:ca-central-1:687401027353:alert-critical"
~ policy = jsonencode(
~ {
- Id = "__default_policy_ID" -> null
~ Statement = [
~ {
~ Action = [
- "SNS:GetTopicAttributes",
+ "SNS:Subscribe",
"SNS:SetTopicAttributes",
- "SNS:AddPermission",
"SNS:RemovePermission",
- "SNS:DeleteTopic",
- "SNS:Subscribe",
- "SNS:ListSubscriptionsByTopic",
- "SNS:Publish",
"SNS:Receive",
+ "SNS:Publish",
+ "SNS:ListSubscriptionsByTopic",
+ "SNS:GetTopicAttributes",
+ "SNS:DeleteTopic",
+ "SNS:AddPermission",
]
~ Condition = {
~ StringEquals = {
~ AWS:SourceOwner = "687401027353" -> [
+ "687401027353",
]
}
}
~ Resource = "arn:aws:sns:ca-central-1:687401027353:alert-critical" -> "*"
~ Sid = "__default_statement_ID" -> "SNS_Default_Policy"
# (2 unchanged elements hidden)
},
+ {
+ Action = "sns:Publish"
+ Effect = "Allow"
+ Principal = {
+ Service = "events.amazonaws.com"
}
+ Resource = "*"
+ Sid = "SNS_Publish_statement"
},
]
~ Version = "2008-10-17" -> "2012-10-17"
}
)
# (2 unchanged attributes hidden)
}
# aws_sns_topic_subscription.topic_critical will be updated in-place
~ resource "aws_sns_topic_subscription" "topic_critical" {
+ confirmation_timeout_in_minutes = 1
+ endpoint_auto_confirms = false
id = "arn:aws:sns:ca-central-1:687401027353:alert-critical:9eb13517-91ba-4866-9e39-6f9e190deb96"
# (8 unchanged attributes hidden)
}
Plan: 1 to add, 2 to change, 1 to destroy.
─────────────────────────────────────────────────────────────────────────────
Saved the plan to: plan.tfplan
To perform exactly these actions, run the following command to apply:
terraform apply "plan.tfplan"
Show Conftest resultsWARN - plan.json - main - Missing Common Tags: ["aws_cloudwatch_metric_alarm.alb_ddos"]
WARN - plan.json - main - Missing Common Tags: ["aws_cloudwatch_metric_alarm.route53_ddos"]
10 tests, 8 passed, 2 warnings, 0 failures, 0 exceptions
|
Staging: load_testing✅ Terraform Format: Plan: 0 to add, 1 to change, 0 to destroyShow planResource actions are indicated with the following symbols:
~ update in-place
Terraform will perform the following actions:
# aws_lambda_function.load_testing will be updated in-place
~ resource "aws_lambda_function" "load_testing" {
id = "LoadTesting"
~ last_modified = "2021-12-08T21:16:31.000+0000" -> (known after apply)
~ source_code_hash = "KmUj+g2OTcuuzCytFbpnofKL5b+4Rk42a+XIMFIxfvA=" -> "I9IuopapsDa3Kw40Uy6A/lk83EYWetduATNj6is/pwo="
tags = {
"CostCentre" = "forms-platform-staging"
"Terraform" = "true"
}
# (19 unchanged attributes hidden)
# (2 unchanged blocks hidden)
}
Plan: 0 to add, 1 to change, 0 to destroy.
─────────────────────────────────────────────────────────────────────────────
Saved the plan to: plan.tfplan
To perform exactly these actions, run the following command to apply:
terraform apply "plan.tfplan"
Show Conftest results9 tests, 9 passed, 0 warnings, 0 failures, 0 exceptions
|
patheard
approved these changes
Dec 17, 2021
craigzour
deleted the
feature/extract-sns-definition-into-separated-module
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
…equired by the app module for an upcoming feature
Summary | Résumé
Reviewer checklist | Liste de vérification du réviseur
This is a suggested checklist of questions reviewers might ask during their
review | Voici une suggestion de liste de vérification comprenant des questions
que les réviseurs pourraient poser pendant leur examen :
langues officielles?
une baisse de la quantité de code couvert par les tests automatisés?
fonctionnalité existante?
que ça devrait être divisé en de plus petites demandes de tirage (« pull
requests ») afin de réduire le risque lié aux modifications?
modification de la politique de confidentialité?
préoccupations liées à la sécurité?
façon importante la performance?
risque d’utiliser des dépendances ajoutées?
setup, etc.) | Faudra-t-il mettre à jour la documentation à la suite de ce
changement (fichier README, etc.)?