Cloudwatch dev env fixes #1695

ben851 · 2024-12-09T16:24:47Z

Summary | Résumé

Adding fixes for the dev env delete/recreate with cloudwatch enabled

Related Issues | Cartes liées

https://app.zenhub.com/workspaces/notify-planning-core-6411dfb7c95fb80014e0cab0/issues/gh/cds-snc/notification-planning-core/480

Before merging this PR

Read code suggestions left by the
cds-ai-codereviewer bot. Address
valid suggestions and shortly write down reasons to not address others. To help
with the classification of the comments, please use these reactions on each of the
comments made by the AI review:

Classification	Reaction	Emoticon
Useful	+1	👍
Noisy	eyes	👀
Hallucination	confused	😕
Wrong but teachable	rocket	🚀
Wrong and incorrect	-1	👎

The classifications will be extracted and summarized into an analysis of how helpful
or not the AI code review really is.

Test instructions | Instructions pour tester la modification

Tested locally - validate next week that dev env creates properly.

Release Instructions | Instructions pour le déploiement

None.

Reviewer checklist | Liste de vérification du réviseur

This PR does not break existing functionality.
This PR does not violate GCNotify's privacy policies.
This PR does not raise new security concerns. Refer to our GC Notify Risk Register document on our Google drive.
This PR does not significantly alter performance.
Additional required documentation resulting of these changes is covered (such as the README, setup instructions, a related ADR or the technical documentation).

⚠ If boxes cannot be checked off before merging the PR, they should be moved to the "Release Instructions" section with appropriate steps required to verify before release. For example, changes to celery code may require tests on staging to verify that performance has not been affected.

[review]

github-actions · 2024-12-09T16:25:33Z

scripts/deleteEnvironment.sh

-aws logs delete-query-definition --query-definition-id $(aws logs describe-query-definitions --query 'queryDefinitions[?name==`Lambda Statistics - ses_to_sqs_email_callbacks`]'.queryDefinitionId --output text)
-aws logs delete-query-definition --query-definition-id $(aws logs describe-query-definitions --query 'queryDefinitions[?name==`Lambda Statistics - ses_receiving_emails`]'.queryDefinitionId --output text --region us-east-1) --region us-east-1
-aws logs delete-query-definition --query-definition-id $(aws logs describe-query-definitions --query 'queryDefinitions[?name==`API / Services going over daily rate limits`]'.queryDefinitionId --output text)
+aws logs delete-log-group --log-group-name '/aws/eks/notification-canada-ca-dev-eks-cluster/cluster'


The aws logs delete-log-group command should include a check to ensure the log group exists before attempting to delete it. This will prevent errors if the log group does not exist.

github-actions · 2024-12-09T16:25:33Z

scripts/deleteEnvironment.sh

-aws logs delete-query-definition --query-definition-id $(aws logs describe-query-definitions --query 'queryDefinitions[?name==`Lambda Statistics - ses_receiving_emails`]'.queryDefinitionId --output text --region us-east-1) --region us-east-1
-aws logs delete-query-definition --query-definition-id $(aws logs describe-query-definitions --query 'queryDefinitions[?name==`API / Services going over daily rate limits`]'.queryDefinitionId --output text)
+aws logs delete-log-group --log-group-name '/aws/eks/notification-canada-ca-dev-eks-cluster/cluster'
+aws logs delete-log-group --log-group-name '/aws/rds/cluster/notification-canada-ca-dev-cluster/postgresql'


The aws logs delete-log-group command should include a check to ensure the log group exists before attempting to delete it. This will prevent errors if the log group does not exist.

github-actions · 2024-12-09T16:25:33Z

scripts/deleteEnvironment.sh

+aws logs delete-log-group --log-group-name '/aws/eks/notification-canada-ca-dev-eks-cluster/cluster'
+aws logs delete-log-group --log-group-name '/aws/rds/cluster/notification-canada-ca-dev-cluster/postgresql'
+
+QUERIES=$(aws logs describe-query-definitions --query 'queryDefinitions[].queryDefinitionId' --output text)


Consider adding a check to ensure that QUERIES is not empty before entering the for loop. This will prevent unnecessary iterations if there are no query definitions to delete.

github-actions · 2024-12-09T16:25:33Z

scripts/deleteEnvironment.sh

@@ -138,5 +139,11 @@
  echo "Done."
 done

+QUERIES=$(aws logs describe-query-definitions --query 'queryDefinitions[].queryDefinitionId' --output text)


Consider adding a check to ensure aws logs describe-query-definitions returns a non-empty result before proceeding with the deletion loop. This can prevent unnecessary operations if there are no query definitions to delete.

Not necessary for what we're doing here.

github-actions · 2024-12-09T16:25:33Z

scripts/deleteEnvironment.sh

+  echo "Deleting cloudwatch query $query"
+  aws logs delete-query-definition --query-definition-id $query
+done
+
 echo "Done."
 echo "Account $ACCOUNT_ID has been cleaned up."


Ensure that the script ends with a newline to follow POSIX standards and avoid potential issues with some tools that might not handle files without a newline at the end correctly.

sastels · 2024-12-09T16:31:18Z

aws/eks/cloudwatch_log.tf

-  to = aws_cloudwatch_log_group.blazer[0]
-  id = "blazer"
-}
-


We don't need this for staging / prod?

github-actions · 2024-12-10T18:46:30Z

staging: eks

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

Plan: 0 to add, 1 to change, 0 to destroy

Show summary

CHANGE	NAME
update	`module.sentinel_forwarder.aws_lambda_function.sentinel_forwarder`

Show plan

Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # module.sentinel_forwarder.aws_lambda_function.sentinel_forwarder will be updated in-place
  ~ resource "aws_lambda_function" "sentinel_forwarder" {
        id                             = "sentinel-cloud-watch-forwarder"
      ~ layers                         = [
          ~ (sensitive value),
        ]
        tags                           = {
            "CostCentre" = "notification-canada-ca-staging"
        }
        # (28 unchanged attributes hidden)

        # (4 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 results

WARN - plan.json - main - Cloudwatch log metric pattern is invalid: ["aws_cloudwatch_log_metric_filter.celery-error[0]"]
WARN - plan.json - main - Cloudwatch log metric pattern is invalid: ["aws_cloudwatch_log_metric_filter.scanfiles-timeout[0]"]
WARN - plan.json - main - Missing Common Tags: ["aws_acm_certificate.client_vpn"]
WARN - plan.json - main - Missing Common Tags: ["aws_acm_certificate.notification-canada-ca"]
WARN - plan.json - main - Missing Common Tags: ["aws_acm_certificate.notification-canada-ca-alt[0]"]
WARN - plan.json - main - Missing Common Tags: ["aws_alb.notification-canada-ca"]
WARN - plan.json - main - Missing Common Tags: ["aws_alb_listener.internal_alb_tls"]
WARN - plan.json - main - Missing Common Tags: ["aws_alb_listener.notification-canada-ca"]
WARN - plan.json - main - Missing Common Tags: ["aws_alb_target_group.internal_nginx_http"]
WARN - plan.json - main - Missing Common Tags: ["aws_alb_target_group.notification-canada-ca-admin"]
WARN - plan.json - main - Missing Common Tags: ["aws_alb_target_group.notification-canada-ca-api"]
WARN - plan.json - main - Missing Common Tags: ["aws_alb_target_group.notification-canada-ca-document"]
WARN - plan.json - main - Missing Common Tags: ["aws_alb_target_group.notification-canada-ca-document-api"]
WARN - plan.json - main - Missing Common Tags: ["aws_alb_target_group.notification-canada-ca-documentation"]
WARN - plan.json - main - Missing Common Tags: ["aws_cloudwatch_log_group.blazer[0]"]
WARN - plan.json - main - Missing Common Tags: ["aws_cloudwatch_log_group.notification-canada-ca-eks-application-logs[0]"]
WARN - plan.json - main - Missing Common Tags: ["aws_cloudwatch_log_group.notification-canada-ca-eks-cluster-logs[0]"]
WARN - plan.json - main - Missing Common Tags: ["aws_cloudwatch_log_group.notification-canada-ca-eks-prometheus-logs[0]"]
WARN - plan.json - main - Missing Common Tags: ["aws_cloudwatch_metric_alarm.admin-evicted-pods[0]"]
WARN - plan.json - main - Missing Common Tags:...

ben851 added 5 commits December 4, 2024 14:49

formatting

65d8999

[review]

dashboard fix

1a37963

Merge branch 'main' of github.com:cds-snc/notification-terraform

480f13b

Merge branch 'main' of github.com:cds-snc/notification-terraform

31e0784

Dev cloudwatch fixes

5504992

[review]

ben851 requested a review from sastels December 9, 2024 16:24

ben851 requested a review from jimleroyer as a code owner December 9, 2024 16:24

github-actions bot reviewed Dec 9, 2024

View reviewed changes

new line

8f40580

sastels reviewed Dec 9, 2024

View reviewed changes

aws/eks/cloudwatch_log.tf

to = aws_cloudwatch_log_group.blazer[0]

id = "blazer"

}

Copy link

Contributor

sastels Dec 9, 2024

Choose a reason for hiding this comment

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

We don't need this for staging / prod?

sastels approved these changes Dec 9, 2024

View reviewed changes

ben851 mentioned this pull request Dec 10, 2024

[CHORE] Fix Dev cds-snc/notification-planning-core#480

Open

2 tasks

Merge branch 'main' into cloudwatch-dev-env-fixes

bc635a1

ben851 merged commit 7e5bdf7 into main Dec 10, 2024
29 checks passed

ben851 deleted the cloudwatch-dev-env-fixes branch December 10, 2024 18:56

ben851 mentioned this pull request Dec 17, 2024

tf release 2.17.36 #1701

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cloudwatch dev env fixes #1695

Cloudwatch dev env fixes #1695

ben851 commented Dec 9, 2024 •

edited by sastels

Loading

github-actions bot Dec 9, 2024

ben851 Dec 9, 2024

github-actions bot Dec 9, 2024

ben851 Dec 9, 2024

github-actions bot Dec 9, 2024

ben851 Dec 9, 2024

github-actions bot Dec 9, 2024

ben851 Jan 9, 2025

github-actions bot Dec 9, 2024

sastels Dec 9, 2024

github-actions bot commented Dec 10, 2024

Cloudwatch dev env fixes #1695

Cloudwatch dev env fixes #1695

Conversation

ben851 commented Dec 9, 2024 • edited by sastels Loading

Summary | Résumé

Related Issues | Cartes liées

Before merging this PR

Test instructions | Instructions pour tester la modification

Release Instructions | Instructions pour le déploiement

Reviewer checklist | Liste de vérification du réviseur

github-actions bot Dec 9, 2024

Choose a reason for hiding this comment

ben851 Dec 9, 2024

Choose a reason for hiding this comment

github-actions bot Dec 9, 2024

Choose a reason for hiding this comment

ben851 Dec 9, 2024

Choose a reason for hiding this comment

github-actions bot Dec 9, 2024

Choose a reason for hiding this comment

ben851 Dec 9, 2024

Choose a reason for hiding this comment

github-actions bot Dec 9, 2024

Choose a reason for hiding this comment

ben851 Jan 9, 2025

Choose a reason for hiding this comment

github-actions bot Dec 9, 2024

Choose a reason for hiding this comment

sastels Dec 9, 2024

Choose a reason for hiding this comment

github-actions bot commented Dec 10, 2024

staging: eks

ben851 commented Dec 9, 2024 •

edited by sastels

Loading